Merge pull request #141 from JoelSpeed/allow-embedded-map-key

Allow map keys from embedded types in ssatags

Author: k8s-ci-robot

pkg/analysis/ssatags/analyzer.go

@@ -200,7 +200,7 @@ func (a *analyzer) validateListMapKeys(pass *analysis.Pass, field *ast.Field, li
 		return
 	}
 
-	structFields := a.getStructFieldsFromField(pass, field)
+	structFields := a.getStructFieldsFromField(pass, jsonTags, field)
 	if structFields == nil {
 		return
 	}
@@ -222,7 +222,7 @@ func (a *analyzer) validateListMapKeys(pass *analysis.Pass, field *ast.Field, li
 	}
 }
 
-func (a *analyzer) getStructFieldsFromField(pass *analysis.Pass, field *ast.Field) *ast.FieldList {
+func (a *analyzer) getStructFieldsFromField(pass *analysis.Pass, jsonTags extractjsontags.StructFieldTags, field *ast.Field) *ast.FieldList {
 	var elementType ast.Expr
 
 	// Get the element type from array or field type
@@ -232,10 +232,10 @@ func (a *analyzer) getStructFieldsFromField(pass *analysis.Pass, field *ast.Fiel
 		elementType = field.Type
 	}
 
-	return a.getStructFieldsFromExpr(pass, elementType)
+	return a.getStructFieldsFromExpr(pass, jsonTags, elementType)
 }
 
-func (a *analyzer) getStructFieldsFromExpr(pass *analysis.Pass, expr ast.Expr) *ast.FieldList {
+func (a *analyzer) getStructFieldsFromExpr(pass *analysis.Pass, jsonTags extractjsontags.StructFieldTags, expr ast.Expr) *ast.FieldList {
 	switch elementType := expr.(type) {
 	case *ast.Ident:
 		typeSpec, ok := utils.LookupTypeSpec(pass, elementType)
@@ -248,9 +248,9 @@ func (a *analyzer) getStructFieldsFromExpr(pass *analysis.Pass, expr ast.Expr) *
 			return nil
 		}
 
-		return structType.Fields
+		return flattenStructFields(pass, jsonTags, structType.Fields)
 	case *ast.StarExpr:
-		return a.getStructFieldsFromExpr(pass, elementType.X)
+		return a.getStructFieldsFromExpr(pass, jsonTags, elementType.X)
 	case *ast.SelectorExpr:
 		return nil
 	}
@@ -288,3 +288,44 @@ func defaultConfig(cfg *SSATagsConfig) {
 		cfg.ListTypeSetUsage = SSATagsListTypeSetUsageWarn
 	}
 }
+
+// flattenStructFields flattens a struct's fields by looking for embedded structs and promoting their fields to the top level.
+// This allows us to correctly check listMapKey markers where the map key is a member of the embedded struct.
+func flattenStructFields(pass *analysis.Pass, jsonTags extractjsontags.StructFieldTags, fields *ast.FieldList) *ast.FieldList {
+	if fields == nil {
+		return nil
+	}
+
+	flattenedFields := &ast.FieldList{}
+
+	for _, field := range fields.List {
+		tagInfo := jsonTags.FieldTags(field)
+		if len(field.Names) > 0 || tagInfo.Name != "" {
+			// Field is not embedded, it has an explicit name.
+			flattenedFields.List = append(flattenedFields.List, field)
+			continue
+		}
+
+		ident, ok := field.Type.(*ast.Ident)
+		if !ok {
+			flattenedFields.List = append(flattenedFields.List, field)
+			continue
+		}
+
+		typeSpec, ok := utils.LookupTypeSpec(pass, ident)
+		if !ok {
+			flattenedFields.List = append(flattenedFields.List, field)
+			continue
+		}
+
+		embeddedStruct, ok := typeSpec.Type.(*ast.StructType)
+		if !ok {
+			flattenedFields.List = append(flattenedFields.List, field)
+			continue
+		}
+
+		flattenedFields.List = append(flattenedFields.List, flattenStructFields(pass, jsonTags, embeddedStruct.Fields).List...)
+	}
+
+	return flattenedFields
+}

pkg/analysis/ssatags/testdata/src/a/a.go

@@ -13,6 +13,23 @@ type SimpleObject struct {
 	Name string `json:"name"`
 }
 
+// ObjectWithEmbedded is an object with an embedded struct.
+// As there's no json tag, the embedded struct is inlined.
+type ObjectWithEmbedded struct {
+	TestObject
+	Embedded string `json:"embedded"`
+}
+
+type ObjectWithInlineEmbedded struct {
+	TestObject `json:",inline"`
+	Embedded   string `json:"embedded"`
+}
+
+type ObjectWithNamedEmbedded struct {
+	TestObject `json:"testObject"`
+	Embedded   string `json:"embedded"`
+}
+
 type String string
 type Int int
 type Object TestObject
@@ -195,6 +212,24 @@ type SSATagsTestSpec struct {
 	// +kubebuilder:pruning:PreserveUnknownFields
 	// +kubebuilder:validation:Schemaless
 	AllOf []JSONSchemaProps `json:"allOf,omitempty"`
+
+	// Object with embedded struct - list map key from embedded struct should be allowed
+	// Without a json tag, the default is to inline the embedded struct.
+	// +listType=map
+	// +listMapKey=name
+	ObjectWithEmbeddedStruct []ObjectWithEmbedded `json:"objectWithEmbeddedStruct,omitempty"`
+
+	// Object with inline embedded struct - list map key from embedded struct should be allowed
+	// With a json tag, the embedded struct is not inlined.
+	// +listType=map
+	// +listMapKey=name
+	ObjectWithInlineEmbeddedStruct []ObjectWithInlineEmbedded `json:"objectWithInlineEmbeddedStruct,omitempty"`
+
+	// Object with named embedded struct - list map key from embedded struct should be allowed
+	// With a json tag, the embedded struct is not inlined.
+	// +listType=map
+	// +listMapKey=name
+	ObjectWithNamedEmbeddedStruct []ObjectWithNamedEmbedded `json:"objectWithNamedEmbeddedStruct,omitempty"` // want "ObjectWithNamedEmbeddedStruct listMapKey \"name\" does not exist as a field in the struct"
 }
 
 // JSONSchemaProps is a placeholder for the JSON schema properties.

pkg/analysis/ssatags/testdata/src/a/a.go.golden

@@ -13,6 +13,23 @@ type SimpleObject struct {
 	Name string `json:"name"`
 }
 
+// ObjectWithEmbedded is an object with an embedded struct.
+// As there's no json tag, the embedded struct is inlined.
+type ObjectWithEmbedded struct {
+	TestObject
+	Embedded string `json:"embedded"`
+}
+
+type ObjectWithInlineEmbedded struct {
+	TestObject `json:",inline"`
+	Embedded string `json:"embedded"`
+}
+
+type ObjectWithNamedEmbedded struct {
+	TestObject `json:"testObject"`
+	Embedded   string `json:"embedded"`
+}
+
 type String string
 type Int int
 type Object TestObject
@@ -194,6 +211,24 @@ type SSATagsTestSpec struct {
 	// +kubebuilder:pruning:PreserveUnknownFields
 	// +kubebuilder:validation:Schemaless
 	AllOf []JSONSchemaProps `json:"allOf,omitempty"`
+
+	// Object with embedded struct - list map key from embedded struct should be allowed
+	// Without a json tag, the default is to inline the embedded struct.
+	// +listType=map
+	// +listMapKey=name
+	ObjectWithEmbeddedStruct []ObjectWithEmbedded `json:"objectWithEmbeddedStruct,omitempty"`
+
+	// Object with inline embedded struct - list map key from embedded struct should be allowed
+	// With a json tag, the embedded struct is not inlined.
+	// +listType=map
+	// +listMapKey=name
+	ObjectWithInlineEmbeddedStruct []ObjectWithInlineEmbedded `json:"objectWithInlineEmbeddedStruct,omitempty"`
+
+	// Object with named embedded struct - list map key from embedded struct should be allowed
+	// With a json tag, the embedded struct is not inlined.
+	// +listType=map
+	// +listMapKey=name
+	ObjectWithNamedEmbeddedStruct []ObjectWithNamedEmbedded `json:"objectWithNamedEmbeddedStruct,omitempty"` // want "ObjectWithNamedEmbeddedStruct listMapKey \"name\" does not exist as a field in the struct"
 }
 
 // JSONSchemaProps is a placeholder for the JSON schema properties.