Simplify & Improve Definition/Type interfaces (#107)

Summary:
* `controller-gen` can now run without crashing
* all nested enums and structs are pulled out and named
* we generate names for everything that is named in a schema, not just structs/enums
* nomenclature should be a bit clearer
----
Details;
* `DefinitionName` renamed to `TypeName`
* `Definition` interface renamed to `TypeDefiner` (golangy!) and simplified, it has the `TypeName` of the type it is defining and the `Type` that it will be bound to. `AsDeclarations` is on this type.
  - Add `SimpleTypeDefiner` for when we want to give names to simple types that aren't structs/enums. This is functionality we didn't have before (see next point as well).
* `CreateRelatedDefinitions` is renamed and split/massaged to `CreateDefinitions` which will create both the primary definition and any associated definitions. 
This latter thing fixes a few issues:
  - previously we only named structs & enums. now a `refHandler` can give a name to anything like a map, etc (this is actually used a lot).
  - removes the special-casing of structs that we had in `refHandler` and at the root node
  - removes the mutation of `canonicalName` in `EnumType`
  - this is also necessary to enforce that internal structs are always named, which is needed to actually run controller-gen properly.
* There is also `CreateInternalDefinitions` which walks the innards of a type and creates any enums/structs that must be named in order to be used. With this change `controller-gen` can now generate CRDs without crashing.

hack/generator/.gitignore

@@ -1 +1,2 @@
-apis/
+apis/
+config/crd/bases/

hack/generator/pkg/astmodel/array_type.go

@@ -35,7 +35,7 @@ func (array *ArrayType) RequiredImports() []PackageReference {
 }
 
 // References this type has to the given type
-func (array *ArrayType) References(d *DefinitionName) bool {
+func (array *ArrayType) References(d *TypeName) bool {
 	return array.element.References(d)
 }
 
@@ -52,7 +52,13 @@ func (array *ArrayType) Equals(t Type) bool {
 	return false
 }
 
-// CreateRelatedDefinitions returns any additional definitions that need to be created
-func (array *ArrayType) CreateRelatedDefinitions(ref PackageReference, namehint string, idFactory IdentifierFactory) []Definition {
-	return array.element.CreateRelatedDefinitions(ref, namehint, idFactory)
+// CreateInternalDefinitions invokes CreateInternalDefinitions on the inner 'element' type
+func (array *ArrayType) CreateInternalDefinitions(name *TypeName, idFactory IdentifierFactory) (Type, []TypeDefiner) {
+	newElementType, otherTypes := array.element.CreateInternalDefinitions(name, idFactory)
+	return NewArrayType(newElementType), otherTypes
+}
+
+// CreateDefinitions defines a named type for this array type
+func (array *ArrayType) CreateDefinitions(name *TypeName, _ IdentifierFactory, _ bool) (TypeDefiner, []TypeDefiner) {
+	return NewSimpleTypeDefiner(name, array), nil
 }

hack/generator/pkg/astmodel/enum_definition.go

@@ -12,32 +12,34 @@ import (
 
 // EnumDefinition generates the full definition of an enumeration
 type EnumDefinition struct {
-	DefinitionName
-	baseType *EnumType
+	typeName    *TypeName
+	baseType    *EnumType
+	description *string
 }
 
-var _ Definition = (*EnumDefinition)(nil)
+var _ TypeDefiner = (*EnumDefinition)(nil)
 
 // NewEnumDefinition is a factory method for creating new Enum Definitions
-func NewEnumDefinition(name DefinitionName, t *EnumType) *EnumDefinition {
-	return &EnumDefinition{DefinitionName: name, baseType: t}
+func NewEnumDefinition(name *TypeName, t *EnumType) *EnumDefinition {
+	return &EnumDefinition{typeName: name, baseType: t}
 }
 
-// FileNameHint returns a desired name for this enum if it goes into a standalone file
-func (enum *EnumDefinition) FileNameHint() string {
-	return enum.name
-}
-
-// Reference returns the unique name to use for specifying this enumeration
-func (enum *EnumDefinition) Reference() *DefinitionName {
-	return &enum.DefinitionName
+// Name returns the unique name to use for specifying this enumeration
+func (enum *EnumDefinition) Name() *TypeName {
+	return enum.typeName
 }
 
 // Type returns the underlying EnumerationType for this enum
 func (enum *EnumDefinition) Type() Type {
 	return enum.baseType
 }
 
+func (enum *EnumDefinition) WithDescription(description *string) TypeDefiner {
+	result := *enum
+	result.description = description
+	return &result
+}
+
 // AsDeclarations generates the Go code representing this definition
 func (enum *EnumDefinition) AsDeclarations() []ast.Decl {
 	var specs []ast.Spec
@@ -60,7 +62,7 @@ func (enum *EnumDefinition) AsDeclarations() []ast.Decl {
 }
 
 func (enum *EnumDefinition) createBaseDeclaration() ast.Decl {
-	identifier := ast.NewIdent(enum.name)
+	identifier := ast.NewIdent(enum.typeName.name)
 
 	typeSpecification := &ast.TypeSpec{
 		Name: identifier,
@@ -80,9 +82,9 @@ func (enum *EnumDefinition) createBaseDeclaration() ast.Decl {
 
 func (enum *EnumDefinition) createValueDeclaration(value EnumValue) ast.Spec {
 
-	enumIdentifier := ast.NewIdent(enum.name)
+	enumIdentifier := ast.NewIdent(enum.typeName.name)
+	valueIdentifier := ast.NewIdent(enum.typeName.name + value.Identifier)
 
-	valueIdentifier := ast.NewIdent(enum.Name() + value.Identifier)
 	valueLiteral := ast.BasicLit{
 		Kind:  token.STRING,
 		Value: value.Value,
@@ -100,8 +102,3 @@ func (enum *EnumDefinition) createValueDeclaration(value EnumValue) ast.Spec {
 
 	return valueSpec
 }
-
-// CreateRelatedDefinitions returns a set of definitions related to this one
-func (enum *EnumDefinition) CreateRelatedDefinitions(ref PackageReference, namehint string, idFactory IdentifierFactory) []Definition {
-	return enum.baseType.CreateRelatedDefinitions(ref, namehint, idFactory)
-}

hack/generator/pkg/astmodel/enum_type.go

@@ -8,6 +8,8 @@ package astmodel
 import (
 	"go/ast"
 	"sort"
+
+	"k8s.io/klog/v2"
 )
 
 // EnumType represents a set of mutually exclusive predefined options
@@ -16,8 +18,6 @@ type EnumType struct {
 	BaseType *PrimitiveType
 	// Options is the set of all unique values
 	options []EnumValue
-	// canonicalName is our actual name, only available once generated, assigned by CreateRelatedDefinitions()
-	canonicalName DefinitionName
 }
 
 // EnumType must implement the Type interface correctly
@@ -34,20 +34,14 @@ func NewEnumType(baseType *PrimitiveType, options []EnumValue) *EnumType {
 
 // AsType implements Type for EnumType
 func (enum *EnumType) AsType() ast.Expr {
-	return ast.NewIdent(enum.canonicalName.name)
-}
-
-// References indicates whether this Type includes any direct references to the given Type?
-func (enum *EnumType) References(d *DefinitionName) bool {
-	return enum.canonicalName.References(d)
+	// this should "never" happen as we name all enums; warn about it if it does
+	klog.Warning("emitting unnamed enum, something’s awry")
+	return enum.BaseType.AsType()
 }
 
-// CreateRelatedDefinitions returns a definition for our enumeration, with a name based on the referencing property
-func (enum *EnumType) CreateRelatedDefinitions(ref PackageReference, namehint string, idFactory IdentifierFactory) []Definition {
-	identifier := idFactory.CreateEnumIdentifier(namehint)
-	enum.canonicalName = DefinitionName{PackageReference: ref, name: identifier}
-	definition := NewEnumDefinition(enum.canonicalName, enum)
-	return []Definition{definition}
+// References indicates whether this Type includes any direct references to the given Type
+func (enum *EnumType) References(tn *TypeName) bool {
+	return enum.BaseType.References(tn)
 }
 
 // Equals will return true if the supplied type has the same base type and options
@@ -80,7 +74,22 @@ func (enum *EnumType) RequiredImports() []PackageReference {
 	return nil
 }
 
-// Options returns all our options
+// CreateInternalDefinitions defines a named type for this enum and returns that type to be used in place
+// of this "raw" enum type
+func (enum *EnumType) CreateInternalDefinitions(nameHint *TypeName, idFactory IdentifierFactory) (Type, []TypeDefiner) {
+	// an internal enum must always be named:
+	definedEnum, otherTypes := enum.CreateDefinitions(nameHint, idFactory, false)
+	return definedEnum.Name(), append(otherTypes, definedEnum)
+}
+
+// CreateDefinitions defines a named type for this "raw" enum type
+func (enum *EnumType) CreateDefinitions(name *TypeName, idFactory IdentifierFactory, _ bool) (TypeDefiner, []TypeDefiner) {
+	identifier := idFactory.CreateEnumIdentifier(name.name)
+	canonicalName := NewTypeName(name.PackageReference, identifier)
+	return NewEnumDefinition(canonicalName, enum), nil
+}
+
+// Options returns all the enum options
 // A copy of the slice is returned to preserve immutability
 func (enum *EnumType) Options() []EnumValue {
 	return append(enum.options[:0:0], enum.options...)

hack/generator/pkg/astmodel/field_definition.go

@@ -68,6 +68,13 @@ func (field *FieldDefinition) WithDescription(description *string) *FieldDefinit
 	return &result
 }
 
+// WithType clones the field and returns it with a new type
+func (field *FieldDefinition) WithType(newType Type) *FieldDefinition {
+	result := *field
+	result.fieldType = newType
+	return &result
+}
+
 // WithValidation adds the given validation to the field's set of validations
 func (field *FieldDefinition) WithValidation(validation Validation) *FieldDefinition {
 	result := *field
@@ -130,8 +137,3 @@ func (field *FieldDefinition) AsField() *ast.Field {
 func (field *FieldDefinition) Equals(f *FieldDefinition) bool {
 	return field == f || (field.fieldName == f.fieldName && field.fieldType.Equals(f.fieldType))
 }
-
-// CreateRelatedDefinitions returns a set of definitions related to this one
-func (field *FieldDefinition) CreateRelatedDefinitions(ref PackageReference, namehint string, idFactory IdentifierFactory) []Definition {
-	return field.fieldType.CreateRelatedDefinitions(ref, namehint, idFactory)
-}

hack/generator/pkg/astmodel/file_definition.go

@@ -23,16 +23,17 @@ type FileDefinition struct {
 	// the package this file is in
 	PackageReference
 	// definitions to include in this file
-	definitions []Definition
+	definitions []TypeDefiner
 }
 
 // NewFileDefinition creates a file definition containing specified definitions
-func NewFileDefinition(packageRef PackageReference, definitions ...Definition) *FileDefinition {
-	// TODO: check that all definitions are from same package
-	sort.Slice(definitions, func(left int, right int) bool {
-		return definitions[left].FileNameHint() < definitions[right].FileNameHint()
+func NewFileDefinition(packageRef PackageReference, definitions ...TypeDefiner) *FileDefinition {
+
+	sort.Slice(definitions, func(i, j int) bool {
+		return definitions[i].Name().name < definitions[j].Name().name
 	})
 
+	// TODO: check that all definitions are from same package
 	return &FileDefinition{packageRef, definitions}
 }
 
@@ -88,10 +89,10 @@ func (file *FileDefinition) AsAst() ast.Node {
 	// Emit struct registration for each resource:
 	var exprs []ast.Expr
 	for _, defn := range file.definitions {
-		if structDefn, ok := defn.(*StructDefinition); ok && structDefn.StructReference.IsResource() {
+		if structDefn, ok := defn.(*StructDefinition); ok && structDefn.IsResource() {
 			exprs = append(exprs, &ast.UnaryExpr{
 				Op: token.AND,
-				X:  &ast.CompositeLit{Type: structDefn.StructReference.AsType()},
+				X:  &ast.CompositeLit{Type: structDefn.Name().AsType()},
 			})
 		}
 	}

hack/generator/pkg/astmodel/file_definition_test.go

@@ -15,9 +15,9 @@ func Test_NewFileDefinition_GivenValues_InitializesFields(t *testing.T) {
 	g := NewGomegaWithT(t)
 
 	person := NewTestStruct("Person", "fullName", "knownAs", "familyName")
-	file := NewFileDefinition(person.StructReference.PackageReference, &person)
+	file := NewFileDefinition(person.Name().PackageReference, &person)
 
-	g.Expect(file.PackageReference).To(Equal(person.StructReference.PackageReference))
+	g.Expect(file.PackageReference).To(Equal(person.Name().PackageReference))
 	g.Expect(file.definitions).To(HaveLen(1))
 }
 
@@ -27,8 +27,8 @@ func NewTestStruct(name string, fields ...string) StructDefinition {
 		fs = append(fs, NewFieldDefinition(FieldName(n), n, StringType))
 	}
 
-	ref := NewStructReference(name, "group", "2020-01-01", false)
-	definition := NewStructDefinition(ref, NewStructType(fs...))
+	ref := NewTypeName(NewLocalPackageReference("group", "2020-01-01"), name)
+	definition := NewStructDefinition(ref, NewStructType(fs...), false)
 
 	return *definition
 }

hack/generator/pkg/astmodel/function.go

@@ -11,11 +11,12 @@ import (
 
 // Function represents something that is an (unnamed) Go function
 type Function interface {
-	HasImports
-	ReferenceChecker
+	RequiredImports() []PackageReference
+
+	References(name *TypeName)
 
 	// AsFunc renders the current instance as a Go abstract syntax tree
-	AsFunc(receiver *StructReference, methodName string) *ast.FuncDecl
+	AsFunc(receiver *TypeName, methodName string) *ast.FuncDecl
 
 	// Equals determines if this Function is equal to another one
 	Equals(f Function) bool