Short-circuit "element" updates in one place (#1635)

hack/generator/pkg/astmodel/array_type.go

@@ -29,6 +29,19 @@ func (array *ArrayType) Element() Type {
 	return array.element
 }
 
+// WithElement returns an ArrayType with the specified element.
+// the benefit of this is it allows reusing the same value if the
+// element type is the same
+func (array *ArrayType) WithElement(t Type) *ArrayType {
+	if array.element.Equals(t) {
+		return array
+	}
+
+	result := *array
+	result.element = t
+	return &result
+}
+
 // assert we implemented Type correctly
 var _ Type = (*ArrayType)(nil)
 

hack/generator/pkg/astmodel/flagged_type.go

@@ -86,6 +86,10 @@ func (ft *FlaggedType) WithoutFlag(flag TypeFlag) Type {
 
 // WithElement returns the flagged type with the same flags but a different element
 func (ft *FlaggedType) WithElement(t Type) *FlaggedType {
+	if ft.element.Equals(t) {
+		return ft // short-circuit
+	}
+
 	var flags []TypeFlag
 	for f := range ft.flags {
 		flags = append(flags, f)

hack/generator/pkg/astmodel/map_type.go

@@ -30,6 +30,26 @@ func (m *MapType) ValueType() Type {
 	return m.value
 }
 
+func (m *MapType) WithKeyType(t Type) *MapType {
+	if m.key.Equals(t) {
+		return m
+	}
+
+	result := *m
+	result.key = t
+	return &result
+}
+
+func (m *MapType) WithValueType(t Type) *MapType {
+	if m.value.Equals(t) {
+		return m
+	}
+
+	result := *m
+	result.value = t
+	return &result
+}
+
 // NewMapType creates a new map with the specified key and value types
 func NewMapType(key Type, value Type) *MapType {
 	return &MapType{key, value}

hack/generator/pkg/astmodel/optional_type.go

@@ -19,16 +19,32 @@ type OptionalType struct {
 	element Type
 }
 
-var _ Type = &OptionalType{}
+// type assertions
+var (
+	_ Type     = &OptionalType{}
+	_ MetaType = &OptionalType{}
+)
 
-var _ MetaType = &OptionalType{}
+// cache of commonly-used values
+var cachedOptionals = map[Type]*OptionalType{
+	BoolType:   {BoolType},
+	FloatType:  {FloatType},
+	IntType:    {IntType},
+	StringType: {StringType},
+	UInt32Type: {UInt32Type},
+	UInt64Type: {UInt64Type},
+}
 
 // NewOptionalType creates a new optional type that may or may not have the specified 'element' type
 func NewOptionalType(element Type) Type {
 	if isTypeOptional(element) {
 		return element
 	}
 
+	if result, ok := cachedOptionals[element]; ok {
+		return result
+	}
+
 	return &OptionalType{element}
 }
 
@@ -52,6 +68,24 @@ func (optional *OptionalType) Element() Type {
 	return optional.element
 }
 
+func (optional *OptionalType) WithElement(t Type) Type {
+	if optional.element.Equals(t) {
+		return optional
+	}
+
+	if isTypeOptional(t) {
+		return t
+	}
+
+	if cached, ok := cachedOptionals[t]; ok {
+		return cached
+	}
+
+	result := *optional
+	result.element = t
+	return &result
+}
+
 func (optional *OptionalType) AsDeclarations(codeGenerationContext *CodeGenerationContext, declContext DeclarationContext) []dst.Decl {
 	return AsSimpleDeclarations(codeGenerationContext, declContext, optional)
 }

hack/generator/pkg/astmodel/resource_type.go

@@ -153,6 +153,10 @@ func (resource *ResourceType) IsStorageVersion() bool {
 
 // WithSpec returns a new resource that has the specified spec type
 func (resource *ResourceType) WithSpec(specType Type) *ResourceType {
+	if TypeEquals(resource.spec, specType) {
+		return resource // short-circuit
+	}
+
 	if specResource, ok := specType.(*ResourceType); ok {
 		// type is a resource, take its SpecType instead
 		// so we don't nest resources
@@ -166,6 +170,10 @@ func (resource *ResourceType) WithSpec(specType Type) *ResourceType {
 
 // WithStatus returns a new resource that has the specified status type
 func (resource *ResourceType) WithStatus(statusType Type) *ResourceType {
+	if TypeEquals(resource.status, statusType) {
+		return resource // short-circuit
+	}
+
 	if specResource, ok := statusType.(*ResourceType); ok {
 		// type is a resource, take its StatusType instead
 		// so we don't nest resources

hack/generator/pkg/astmodel/type_visitor.go

@@ -121,11 +121,7 @@ func IdentityVisitOfArrayType(this *TypeVisitor, it *ArrayType, ctx interface{})
 		return nil, errors.Wrap(err, "failed to visit type of array")
 	}
 
-	if newElement == it.element {
-		return it, nil // short-circuit
-	}
-
-	return NewArrayType(newElement), nil
+	return it.WithElement(newElement), nil
 }
 
 func IdentityVisitOfPrimitiveType(_ *TypeVisitor, it *PrimitiveType, _ interface{}) (Type, error) {
@@ -186,11 +182,7 @@ func IdentityVisitOfMapType(this *TypeVisitor, it *MapType, ctx interface{}) (Ty
 		return nil, errors.Wrapf(err, "failed to visit map value type %q", it.value)
 	}
 
-	if visitedKey == it.key && visitedValue == it.value {
-		return it, nil // short-circuit
-	}
-
-	return NewMapType(visitedKey, visitedValue), nil
+	return it.WithKeyType(visitedKey).WithValueType(visitedValue), nil
 }
 
 func IdentityVisitOfEnumType(_ *TypeVisitor, it *EnumType, _ interface{}) (Type, error) {
@@ -205,11 +197,7 @@ func IdentityVisitOfOptionalType(this *TypeVisitor, it *OptionalType, ctx interf
 		return nil, errors.Wrapf(err, "failed to visit optional element type %q", it.element)
 	}
 
-	if visitedElement == it.element {
-		return it, nil // short-circuit
-	}
-
-	return NewOptionalType(visitedElement), nil
+	return it.WithElement(visitedElement), nil
 }
 
 func IdentityVisitOfResourceType(this *TypeVisitor, it *ResourceType, ctx interface{}) (Type, error) {
@@ -241,7 +229,6 @@ func IdentityVisitOfOneOfType(this *TypeVisitor, it *OneOfType, ctx interface{})
 		newTypes = append(newTypes, newType)
 		return nil
 	})
-
 	if err != nil {
 		return nil, err
 	}
@@ -264,7 +251,6 @@ func IdentityVisitOfAllOfType(this *TypeVisitor, it *AllOfType, ctx interface{})
 		newTypes = append(newTypes, newType)
 		return nil
 	})
-
 	if err != nil {
 		return nil, err
 	}

hack/generator/pkg/astmodel/validated_type.go

@@ -156,6 +156,16 @@ func (v *ValidatedType) ElementType() Type {
 	return v.element
 }
 
+func (v *ValidatedType) WithElement(t Type) Type {
+	if v.element.Equals(t) {
+		return v
+	}
+
+	result := *v
+	result.element = t
+	return &result
+}
+
 func (v *ValidatedType) Validations() Validations {
 	return v.validations
 }

hack/generator/pkg/codegen/pipeline/convert_allof_and_oneof_to_objects.go

@@ -407,7 +407,7 @@ func (s synthesizer) handleArrayArray(leftArray *astmodel.ArrayType, rightArray
 		return nil, err
 	}
 
-	return astmodel.NewArrayType(intersected), nil
+	return leftArray.WithElement(intersected), nil
 }
 
 func (s synthesizer) handleObjectObject(leftObj *astmodel.ObjectType, rightObj *astmodel.ObjectType) (astmodel.Type, error) {

hack/generator/pkg/codegen/pipeline/status_augment.go

@@ -160,17 +160,12 @@ func flattenAugmenter(allTypes astmodel.ReadonlyTypes) augmenter {
 		merger.Add(func(spec *astmodel.OptionalType, status astmodel.Type) (astmodel.Type, error) {
 			// we ignore optionality when matching things up, since there are
 			// discordances between the JSON Schema/Swagger as some teams have handcrafted them
-			newSpecElement, err := merger.Merge(spec.Element(), status)
+			newElement, err := merger.Merge(spec.Element(), status)
 			if err != nil {
 				return nil, err
 			}
 
-			// return original type if not changed
-			if newSpecElement.Equals(spec.Element()) {
-				return spec, nil
-			}
-
-			return astmodel.NewOptionalType(newSpecElement), nil
+			return spec.WithElement(newElement), nil
 		})
 
 		// handle (Type, Optional)
@@ -183,17 +178,12 @@ func flattenAugmenter(allTypes astmodel.ReadonlyTypes) augmenter {
 		// handle (FlaggedType, Type); there is no need to handle the opposite direction
 		// as the swagger types won’t be flagged
 		merger.Add(func(spec *astmodel.FlaggedType, status astmodel.Type) (astmodel.Type, error) {
-			newSpec, err := merger.Merge(spec.Element(), status)
+			newElement, err := merger.Merge(spec.Element(), status)
 			if err != nil {
 				return nil, err
 			}
 
-			// return original type if not changed
-			if newSpec.Equals(spec.Element()) {
-				return spec, nil
-			}
-
-			return spec.WithElement(newSpec), nil
+			return spec.WithElement(newElement), nil
 		})
 
 		// handle (Map, Map) by matching up the keys and values
@@ -208,27 +198,17 @@ func flattenAugmenter(allTypes astmodel.ReadonlyTypes) augmenter {
 				return nil, err
 			}
 
-			// return original type if not changed
-			if keyResult.Equals(spec.KeyType()) && valueResult.Equals(spec.ValueType()) {
-				return spec, nil
-			}
-
-			return astmodel.NewMapType(keyResult, valueResult), nil
+			return spec.WithKeyType(keyResult).WithValueType(valueResult), nil
 		})
 
 		// handle (Array, Array) by matching up the inner types
 		merger.Add(func(spec, status *astmodel.ArrayType) (astmodel.Type, error) {
-			newSpecElement, err := merger.Merge(spec.Element(), status.Element())
+			newElement, err := merger.Merge(spec.Element(), status.Element())
 			if err != nil {
 				return nil, err
 			}
 
-			// return original type if not changed
-			if newSpecElement.Equals(spec.Element()) {
-				return spec, nil
-			}
-
-			return astmodel.NewArrayType(newSpecElement), nil
+			return spec.WithElement(newElement), nil
 		})
 
 		// safety check, (AllOf, AllOf) should not occur