Skip to content

Basic type alias panic on shouldBeCopied() #813

@potatogopher

Description

@potatogopher

When generating DeepCopy, DeepCopyInto and DeepCopyObject, a panic occurs with the following error:

panic: interface conversion: types.Type is *types.Basic, not *types.Named

goroutine 1 [running]:
sigs.k8s.io/controller-tools/pkg/deepcopy.shouldBeCopied(0x140002584c0, 0x14002327080)
	/Users/me/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/deepcopy/traverse.go:622

This occurs when using an alias with a basic type:

type MyAlias = string

Things pass when using the following:

type MyAlias string

While checking the internals it seems like the intention is to skip types that are eventually basic but nothing about types that are "immediately" basic.

I wanted to propose adding a check for if typeInfo is a basic type before getting the underlying pointer of the named type to support an alias.

func shouldBeCopied(pkg *loader.Package, info *markers.TypeInfo) bool {
	if !ast.IsExported(info.Name) {
		return false
	}

	typeInfo := pkg.TypesInfo.TypeOf(info.RawSpec.Name)
	if typeInfo == types.Typ[types.Invalid] {
		pkg.AddError(loader.ErrFromNode(fmt.Errorf("unknown type: %s", info.Name), info.RawSpec))
		return false
	}

+	if _, isBasic := typeInfo.(*types.Basic); isBasic {
+		return false
+	}

	// according to gengo, everything named is an alias, except for an alias to a pointer,
	// which is just a pointer, afaict.  Just roll with it.
	if asPtr, isPtr := typeInfo.(*types.Named).Underlying().(*types.Pointer); isPtr {
		typeInfo = asPtr
	}

	lastType := typeInfo
	if _, isNamed := typeInfo.(*types.Named); isNamed {
		// if it has a manual deepcopy or deepcopyinto, we're fine
		if hasAnyDeepCopyMethod(pkg, typeInfo) {
			return true
		}

		for underlyingType := typeInfo.Underlying(); underlyingType != lastType; lastType, underlyingType = underlyingType, underlyingType.Underlying() {
			// if it has a manual deepcopy or deepcopyinto, we're fine
			if hasAnyDeepCopyMethod(pkg, underlyingType) {
				return true
			}

			// aliases to other things besides basics need copy methods
			// (basics can be straight-up shallow-copied)
			if _, isBasic := underlyingType.(*types.Basic); !isBasic {
				return true
			}
		}
	}

	// structs are the only thing that's not a basic that's copiable by default
	_, isStruct := lastType.(*types.Struct)
	return isStruct
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    lifecycle/frozenIndicates that an issue or PR should not be auto-closed due to staleness.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions