Split out ResourceDefinition from StructDefinition (#159)

This removes the bad-feeling `isResource` parameter from `CreateDefinitions` and cleans up some checks in the package/file generation. It also encodes that only resources can have `isStorageVersion` set which wasn’t enforced by the types before. (I also like that it gets us further away from Go-AST-specific naming.)

hack/generator/pkg/astmodel/array_type.go

@@ -59,6 +59,6 @@ func (array *ArrayType) CreateInternalDefinitions(name *TypeName, idFactory Iden
 }
 
 // CreateDefinitions defines a named type for this array type
-func (array *ArrayType) CreateDefinitions(name *TypeName, _ IdentifierFactory, _ bool) (TypeDefiner, []TypeDefiner) {
+func (array *ArrayType) CreateDefinitions(name *TypeName, _ IdentifierFactory) (TypeDefiner, []TypeDefiner) {
 	return NewSimpleTypeDefiner(name, array), nil
 }

hack/generator/pkg/astmodel/enum_type.go

@@ -78,12 +78,12 @@ func (enum *EnumType) RequiredImports() []*PackageReference {
 // 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)
+	definedEnum, otherTypes := enum.CreateDefinitions(nameHint, idFactory)
 	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) {
+func (enum *EnumType) CreateDefinitions(name *TypeName, idFactory IdentifierFactory) (TypeDefiner, []TypeDefiner) {
 	identifier := idFactory.CreateEnumIdentifier(name.name)
 	canonicalName := NewTypeName(name.PackageReference, identifier)
 	return NewEnumDefinition(canonicalName, enum), nil

hack/generator/pkg/astmodel/file_definition.go

@@ -111,7 +111,7 @@ 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.IsResource() {
+		if structDefn, ok := defn.(*ResourceDefinition); ok {
 			exprs = append(exprs, &ast.UnaryExpr{
 				Op: token.AND,
 				X:  &ast.CompositeLit{Type: structDefn.Name().AsType(codeGenContext)},

hack/generator/pkg/astmodel/file_definition_test.go

@@ -28,7 +28,7 @@ func NewTestStruct(name string, fields ...string) StructDefinition {
 	}
 
 	ref := NewTypeName(*NewLocalPackageReference("group", "2020-01-01"), name)
-	definition := NewStructDefinition(ref, NewStructType().WithFields(fs...), false)
+	definition := NewStructDefinition(ref, NewStructType().WithFields(fs...))
 
 	return *definition
 }

hack/generator/pkg/astmodel/map_type.go

@@ -70,6 +70,6 @@ func (m *MapType) CreateInternalDefinitions(name *TypeName, idFactory Identifier
 }
 
 // CreateDefinitions defines a named type for this MapType
-func (m *MapType) CreateDefinitions(name *TypeName, _ IdentifierFactory, _ bool) (TypeDefiner, []TypeDefiner) {
+func (m *MapType) CreateDefinitions(name *TypeName, _ IdentifierFactory) (TypeDefiner, []TypeDefiner) {
 	return NewSimpleTypeDefiner(name, m), nil
 }

hack/generator/pkg/astmodel/optional_type.go

@@ -64,6 +64,6 @@ func (optional *OptionalType) CreateInternalDefinitions(name *TypeName, idFactor
 }
 
 // CreateDefinitions defines a named type for this OptionalType
-func (optional *OptionalType) CreateDefinitions(name *TypeName, _ IdentifierFactory, _ bool) (TypeDefiner, []TypeDefiner) {
+func (optional *OptionalType) CreateDefinitions(name *TypeName, _ IdentifierFactory) (TypeDefiner, []TypeDefiner) {
 	return NewSimpleTypeDefiner(name, optional), nil
 }

hack/generator/pkg/astmodel/package_definition.go

@@ -94,14 +94,14 @@ func anyReferences(defs []TypeDefiner, defName *TypeName) bool {
 	return false
 }
 
-func partitionDefinitions(definitions []TypeDefiner) (resourceStructs []*StructDefinition, otherDefinitions []TypeDefiner) {
+func partitionDefinitions(definitions []TypeDefiner) (resourceStructs []*ResourceDefinition, otherDefinitions []TypeDefiner) {
 
-	var resources []*StructDefinition
+	var resources []*ResourceDefinition
 	var notResources []TypeDefiner
 
 	for _, def := range definitions {
-		if structDef, ok := def.(*StructDefinition); ok && structDef.IsResource() {
-			resources = append(resources, structDef)
+		if resourceDef, ok := def.(*ResourceDefinition); ok {
+			resources = append(resources, resourceDef)
 		} else {
 			notResources = append(notResources, def)
 		}

hack/generator/pkg/astmodel/primitive_type.go

@@ -64,7 +64,7 @@ func (prim *PrimitiveType) CreateInternalDefinitions(_ *TypeName, _ IdentifierFa
 }
 
 // CreateDefinitions defines a named type for this primitive
-func (prim *PrimitiveType) CreateDefinitions(name *TypeName, _ IdentifierFactory, _ bool) (TypeDefiner, []TypeDefiner) {
+func (prim *PrimitiveType) CreateDefinitions(name *TypeName, _ IdentifierFactory) (TypeDefiner, []TypeDefiner) {
 	return NewSimpleTypeDefiner(name, prim), nil
 }
 

hack/generator/pkg/astmodel/resource.go

@@ -0,0 +1,138 @@
+/*
+ * Copyright (c) Microsoft Corporation.
+ * Licensed under the MIT license.
+ */
+
+package astmodel
+
+import (
+	"go/ast"
+	"go/token"
+)
+
+// CreateResourceDefinitions creates definitions for a resource
+func CreateResourceDefinitions(name *TypeName, specType *StructType, statusType *StructType, idFactory IdentifierFactory) (TypeDefiner, []TypeDefiner) {
+
+	var others []TypeDefiner
+
+	defineStruct := func(suffix string, structType *StructType) *TypeName {
+		definedName := NewTypeName(name.PackageReference, name.Name()+suffix)
+		defined, definedOthers := structType.CreateDefinitions(definedName, idFactory)
+		others = append(append(others, defined), definedOthers...)
+		return definedName
+	}
+
+	var specName *TypeName
+	if specType == nil {
+		panic("spec must always be provided")
+	} else {
+		specName = defineStruct("Spec", specType)
+	}
+
+	var statusName *TypeName
+	if statusType != nil {
+		statusName = defineStruct("Status", statusType)
+	}
+
+	this := &ResourceDefinition{typeName: name, spec: specName, status: statusName, isStorageVersion: false}
+
+	return this, others
+}
+
+// ResourceDefinition represents an ARM resource
+type ResourceDefinition struct {
+	typeName         *TypeName
+	spec             *TypeName
+	status           *TypeName
+	isStorageVersion bool
+	description      *string
+}
+
+// assert that ResourceDefinition implements TypeDefiner
+var _ TypeDefiner = &ResourceDefinition{}
+
+// Name returns the name of the type being defined
+func (definition *ResourceDefinition) Name() *TypeName {
+	return definition.typeName
+}
+
+// Type returns the type to be associated with the name
+func (definition *ResourceDefinition) Type() Type {
+	return definition.spec // TODO BUG: the status is not considered here
+	// TO FIX: consider lifting up the two methods used on the result of this method
+	// (which are References/RequiredImports) into the TypeDefiner interface
+}
+
+// MarkAsStorageVersion marks the resource as the Kubebuilder storage version
+func (definition *ResourceDefinition) MarkAsStorageVersion() *ResourceDefinition {
+	result := *definition
+	result.isStorageVersion = true
+	return &result
+}
+
+// WithDescription replaces the description of the resource
+func (definition *ResourceDefinition) WithDescription(description *string) TypeDefiner {
+	result := *definition
+	result.description = description
+	return &result
+}
+
+// TODO: metav1 import should be added via RequiredImports?
+var typeMetaField = defineField("", "metav1.TypeMeta", "`json:\",inline\"`")
+var objectMetaField = defineField("", "metav1.ObjectMeta", "`json:\"metadata,omitempty\"`")
+
+// AsDeclarations converts the ResourceDefinition to a go declaration
+func (definition *ResourceDefinition) AsDeclarations(codeGenerationContext *CodeGenerationContext) []ast.Decl {
+
+	/*
+		start off with:
+			metav1.TypeMeta   `json:",inline"`
+			metav1.ObjectMeta `json:"metadata,omitempty"`
+
+		then the Spec/Status fields
+	*/
+	fields := []*ast.Field{
+		typeMetaField,
+		objectMetaField,
+		defineField("Spec", definition.spec.name, "`json:\"spec,omitempty\"`"),
+	}
+
+	if definition.status != nil {
+		fields = append(fields, defineField("Status", definition.status.name, "`json:\"spec,omitempty\"`"))
+	}
+
+	resourceIdentifier := ast.NewIdent(definition.typeName.name)
+	resourceTypeSpec := &ast.TypeSpec{
+		Name: resourceIdentifier,
+		Type: &ast.StructType{
+			Fields: &ast.FieldList{List: fields},
+		},
+	}
+
+	comments :=
+		[]*ast.Comment{
+			{
+				Text: "// +kubebuilder:object:root=true\n",
+			},
+		}
+
+	if definition.isStorageVersion {
+		comments = append(comments, &ast.Comment{
+			Text: "// +kubebuilder:storageversion\n",
+		})
+	}
+
+	if definition.description != nil {
+		comments = append(comments, &ast.Comment{
+			Text: "/*" + *definition.description + "*/",
+		})
+	}
+
+	return []ast.Decl{
+		&ast.GenDecl{
+			Tok:   token.TYPE,
+			Specs: []ast.Spec{resourceTypeSpec},
+			Doc:   &ast.CommentGroup{List: comments},
+		},
+	}
+}

hack/generator/pkg/astmodel/struct_definition.go

@@ -10,137 +10,65 @@ import (
 	"go/token"
 )
 
-// StructDefinition encapsulates the definition of a struct
+// StructDefinition encapsulates a complex (object) schema type
 type StructDefinition struct {
-	TypeName         *TypeName
-	StructType       *StructType
-	isResource       bool
-	isStorageVersion bool
-	description      *string
+	typeName    *TypeName
+	structType  *StructType
+	description *string
 }
 
-// IsResource indicates if this is a ARM resource and should be a kubebuilder root
-func (definition *StructDefinition) IsResource() bool {
-	return definition.isResource
+// NewStructDefinition creates a new StructDefinition
+func NewStructDefinition(typeName *TypeName, structType *StructType) *StructDefinition {
+	return &StructDefinition{typeName, structType, nil}
 }
 
-// Ensure StructDefinition implements TypeDefiner interface correctly
-var _ TypeDefiner = (*StructDefinition)(nil)
+// ensure StructDefinition is a TypeDefiner
+var _ TypeDefiner = &StructDefinition{}
 
-// Name provides the struct name
+// Name provides the type name
 func (definition *StructDefinition) Name() *TypeName {
-	return definition.TypeName
+	return definition.typeName
 }
 
-// Type provides the type of the struct
+// Type provides the type being linked to the name
 func (definition *StructDefinition) Type() Type {
-	return definition.StructType
+	return definition.structType
 }
 
-// NewStructDefinition is a factory method for creating a new StructDefinition
-func NewStructDefinition(name *TypeName, structType *StructType, isResource bool) *StructDefinition {
-	return &StructDefinition{name, structType, isResource, false, nil}
-}
-
-// WithDescription adds a description (doc-comment) to the struct
+// WithDescription adds a description (doc-comment) to the definition
 func (definition *StructDefinition) WithDescription(description *string) TypeDefiner {
 	result := *definition
 	result.description = description
 	return &result
 }
 
-// WithIsStorageVersion marks the struct definition as a Kubebuilder storage version (or not)
-func (definition *StructDefinition) WithIsStorageVersion(isStorageVersion bool) *StructDefinition {
-	result := *definition
-	result.isStorageVersion = isStorageVersion
-	return &result
-}
-
-// AsDeclarations generates an AST node representing this struct definition
+// AsDeclarations returns the Go AST declarations for this struct
 func (definition *StructDefinition) AsDeclarations(codeGenerationContext *CodeGenerationContext) []ast.Decl {
-	var identifier *ast.Ident
-	if definition.IsResource() {
-		// if it's a resource then this is the Spec type and we will generate
-		// the non-spec type later:
-		identifier = ast.NewIdent(definition.Name().name + "Spec")
-	} else {
-		identifier = ast.NewIdent(definition.Name().name)
-	}
-
-	typeSpecification := &ast.TypeSpec{
-		Name: identifier,
-		Type: definition.StructType.AsType(codeGenerationContext),
-	}
-
+	identifier := ast.NewIdent(definition.typeName.name)
 	declaration := &ast.GenDecl{
 		Tok: token.TYPE,
 		Doc: &ast.CommentGroup{},
 		Specs: []ast.Spec{
-			typeSpecification,
+			&ast.TypeSpec{
+				Name: identifier,
+				Type: definition.structType.AsType(codeGenerationContext),
+			},
 		},
 	}
 
 	if definition.description != nil {
 		addDocComment(&declaration.Doc.List, *definition.description, 200)
 	}
 
-	declarations := []ast.Decl{declaration}
-
-	if definition.IsResource() {
-		resourceIdentifier := ast.NewIdent(definition.Name().name)
-
-		/*
-			start off with:
-				metav1.TypeMeta   `json:",inline"`
-				metav1.ObjectMeta `json:"metadata,omitempty"`
-
-			then the Spec field
-		*/
-		resourceTypeSpec := &ast.TypeSpec{
-			Name: resourceIdentifier,
-			Type: &ast.StructType{
-				Fields: &ast.FieldList{
-					List: []*ast.Field{
-						typeMetaField,
-						objectMetaField,
-						defineField("Spec", identifier.Name, "`json:\"spec,omitempty\"`"),
-					},
-				},
-			},
-		}
-
-		comments :=
-			[]*ast.Comment{
-				{
-					Text: "// +kubebuilder:object:root=true\n",
-				},
-			}
-
-		if definition.isStorageVersion {
-			comments = append(comments, &ast.Comment{
-				Text: "// +kubebuilder:storageversion\n",
-			})
-		}
-
-		resourceDeclaration := &ast.GenDecl{
-			Tok:   token.TYPE,
-			Specs: []ast.Spec{resourceTypeSpec},
-			Doc:   &ast.CommentGroup{List: comments},
-		}
-
-		declarations = append(declarations, resourceDeclaration)
-	}
-
-	// Append the methods
-	declarations = append(declarations, definition.generateMethodDecls(codeGenerationContext)...)
-
-	return declarations
+	result := []ast.Decl{declaration}
+	result = append(result, definition.generateMethodDecls(codeGenerationContext)...)
+	return result
 }
 
 func (definition *StructDefinition) generateMethodDecls(codeGenerationContext *CodeGenerationContext) []ast.Decl {
 	var result []ast.Decl
-	for methodName, function := range definition.StructType.functions {
-		funcDef := function.AsFunc(codeGenerationContext, definition.Name(), methodName)
+	for methodName, function := range definition.structType.functions {
+		funcDef := function.AsFunc(codeGenerationContext, definition.typeName, methodName)
 		result = append(result, funcDef)
 	}
 
@@ -160,7 +88,3 @@ func defineField(fieldName string, typeName string, tag string) *ast.Field {
 
 	return result
 }
-
-// TODO: metav1 import should be added via RequiredImports?
-var typeMetaField = defineField("", "metav1.TypeMeta", "`json:\",inline\"`")
-var objectMetaField = defineField("", "metav1.ObjectMeta", "`json:\"metadata,omitempty\"`")