Merge pull request #156 from yongruilin/conflictmarkers-more-detail

feat(conflictingmarkers): Make error messages more precise

Author: k8s-ci-robot

pkg/analysis/conflictingmarkers/analyzer.go

@@ -67,7 +67,7 @@ func (a *analyzer) run(pass *analysis.Pass) (any, error) {
 		return nil, kalerrors.ErrCouldNotGetInspector
 	}
 
-	inspect.InspectFields(func(field *ast.Field, stack []ast.Node, _ extractjsontags.FieldTagInfo, markersAccess markers.Markers) {
+	inspect.InspectFields(func(field *ast.Field, _ []ast.Node, _ extractjsontags.FieldTagInfo, markersAccess markers.Markers) {
 		checkField(pass, field, markersAccess, a.conflictSets)
 	})
 
@@ -119,8 +119,15 @@ func reportConflict(pass *analysis.Pass, field *ast.Field, conflictSet ConflictS
 		setDescriptions = append(setDescriptions, fmt.Sprintf("%v", markersList))
 	}
 
+	fieldName := field.Names[0].Name
+	structName := utils.GetStructNameForField(pass, field)
+
+	if structName != "" {
+		fieldName = structName + "." + fieldName
+	}
+
 	message := fmt.Sprintf("field %s has conflicting markers: %s: {%s}. %s",
-		field.Names[0].Name,
+		fieldName,
 		conflictSet.Name,
 		strings.Join(setDescriptions, ", "),
 		conflictSet.Description)

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

@@ -17,31 +17,31 @@ type TestStruct struct {
 	// Conflict: marker1 vs marker3 (both with values)
 	// +marker1:=value1
 	// +marker3:=value2
-	ConflictWithValuesField string `json:"conflictWithValuesField"` // want "field ConflictWithValuesField has conflicting markers: test_conflict: \\{\\[marker1\\], \\[marker3\\]\\}. Test markers conflict with each other"
+	ConflictWithValuesField string `json:"conflictWithValuesField"` // want "field TestStruct.ConflictWithValuesField has conflicting markers: test_conflict: \\{\\[marker1\\], \\[marker3\\]\\}. Test markers conflict with each other"
 
 	// Conflict: marker1 vs marker3 (mixed with and without values)
 	// +marker1
 	// +marker3:=someValue
-	ConflictMixedField string `json:"conflictMixedField"` // want "field ConflictMixedField has conflicting markers: test_conflict: \\{\\[marker1\\], \\[marker3\\]\\}. Test markers conflict with each other"
+	ConflictMixedField string `json:"conflictMixedField"` // want "field TestStruct.ConflictMixedField has conflicting markers: test_conflict: \\{\\[marker1\\], \\[marker3\\]\\}. Test markers conflict with each other"
 
 	// Multiple conflicts with multiple markers in each set:
 	// +marker1
 	// +marker2
 	// +marker3
 	// +marker4
-	MultipleConflictsField string `json:"multipleConflictsField"` // want "field MultipleConflictsField has conflicting markers: test_conflict: \\{\\[marker1 marker2\\], \\[marker3 marker4\\]\\}. Test markers conflict with each other"
+	MultipleConflictsField string `json:"multipleConflictsField"` // want "field TestStruct.MultipleConflictsField has conflicting markers: test_conflict: \\{\\[marker1 marker2\\], \\[marker3 marker4\\]\\}. Test markers conflict with each other"
 
 	// Three-way conflict: marker5 vs marker7 vs marker9
 	// +marker5
 	// +marker7
 	// +marker9
-	ThreeWayConflictField string `json:"threeWayConflictField"` // want "field ThreeWayConflictField has conflicting markers: three_way_conflict: \\{\\[marker5\\], \\[marker7\\], \\[marker9\\]\\}. Three-way conflict between marker sets"
+	ThreeWayConflictField string `json:"threeWayConflictField"` // want "field TestStruct.ThreeWayConflictField has conflicting markers: three_way_conflict: \\{\\[marker5\\], \\[marker7\\], \\[marker9\\]\\}. Three-way conflict between marker sets"
 
 	// Three-way conflict with values
 	// +marker6:=value1
 	// +marker8:=value2
 	// +marker10:=value3
-	ThreeWayConflictWithValuesField string `json:"threeWayConflictWithValuesField"` // want "field ThreeWayConflictWithValuesField has conflicting markers: three_way_conflict: \\{\\[marker6\\], \\[marker8\\], \\[marker10\\]\\}. Three-way conflict between marker sets"
+	ThreeWayConflictWithValuesField string `json:"threeWayConflictWithValuesField"` // want "field TestStruct.ThreeWayConflictWithValuesField has conflicting markers: three_way_conflict: \\{\\[marker6\\], \\[marker8\\], \\[marker10\\]\\}. Three-way conflict between marker sets"
 
 	// Valid field with markers from same set in three-way conflict
 	// +marker5
@@ -55,10 +55,10 @@ type TestStruct struct {
 	// +marker8
 	// +marker9
 	// +marker10
-	ThreeWayMultipleMarkersField string `json:"threeWayMultipleMarkersField"` // want "field ThreeWayMultipleMarkersField has conflicting markers: three_way_conflict: \\{\\[marker5 marker6\\], \\[marker7 marker8\\], \\[marker10 marker9\\]\\}. Three-way conflict between marker sets"
+	ThreeWayMultipleMarkersField string `json:"threeWayMultipleMarkersField"` // want "field TestStruct.ThreeWayMultipleMarkersField has conflicting markers: three_way_conflict: \\{\\[marker5 marker6\\], \\[marker7 marker8\\], \\[marker10 marker9\\]\\}. Three-way conflict between marker sets"
 
 	// Three-way conflict with only subset of sets triggered (sets 1 and 2 only)
 	// +marker5
 	// +marker7
-	SubsetThreeWayConflictField string `json:"subsetThreeWayConflictField"` // want "field SubsetThreeWayConflictField has conflicting markers: three_way_conflict: \\{\\[marker5\\], \\[marker7\\]\\}. Three-way conflict between marker sets"
+	SubsetThreeWayConflictField string `json:"subsetThreeWayConflictField"` // want "field TestStruct.SubsetThreeWayConflictField has conflicting markers: three_way_conflict: \\{\\[marker5\\], \\[marker7\\]\\}. Three-way conflict between marker sets"
 }

pkg/analysis/utils/testdata/src/getstructname/getstructname.go

@@ -0,0 +1,10 @@
+package getstructname
+
+type MyStruct1 struct {
+	Field1 string // want "field Field1 is in struct MyStruct1"
+	Field2 int    // want "field Field2 is in struct MyStruct1"
+}
+
+type MyStruct2 struct {
+	AnotherField bool // want "field AnotherField is in struct MyStruct2"
+}

pkg/analysis/utils/utils.go

@@ -19,6 +19,7 @@ import (
 	"go/ast"
 	"go/token"
 	"go/types"
+	"slices"
 
 	"golang.org/x/tools/go/analysis"
 	"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers"
@@ -348,3 +349,49 @@ func isTypeBasic(t types.Type) bool {
 
 	return false
 }
+
+// GetStructNameForField inspects the AST of the package and returns the name of the struct
+// that contains the field being inspected.
+func GetStructNameForField(pass *analysis.Pass, field *ast.Field) string {
+	for _, file := range pass.Files {
+		var (
+			structName string
+			found      bool
+		)
+
+		ast.Inspect(file, func(n ast.Node) bool {
+			if found {
+				return false
+			}
+
+			typeSpec, ok := n.(*ast.TypeSpec)
+			if !ok {
+				return true
+			}
+
+			structType, ok := typeSpec.Type.(*ast.StructType)
+			if !ok {
+				return true
+			}
+
+			structName = typeSpec.Name.Name
+
+			if structType.Fields == nil {
+				return true
+			}
+
+			if slices.Contains(structType.Fields.List, field) {
+				found = true
+				return false
+			}
+
+			return true
+		})
+
+		if found {
+			return structName
+		}
+	}
+
+	return ""
+}

pkg/analysis/utils/utils_test.go

@@ -20,6 +20,10 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/analysistest"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/ast/inspector"
 
 	"sigs.k8s.io/kube-api-linter/pkg/analysis/utils"
 )
@@ -73,3 +77,45 @@ var _ = Describe("FieldName", func() {
 		}),
 	)
 })
+
+var _ = Describe("GetStructNameForField", func() {
+	It("should find the correct struct name for a field", func() {
+		testdata := analysistest.TestData()
+		analysistest.Run(GinkgoT(), testdata, testGetStructNameAnalyzer(), "getstructname")
+	})
+})
+
+func testGetStructNameAnalyzer() *analysis.Analyzer {
+	return &analysis.Analyzer{
+		Name:     "testgetstructname",
+		Doc:      "test analyzer for GetStructNameForField",
+		Requires: []*analysis.Analyzer{inspect.Analyzer},
+		Run: func(pass *analysis.Pass) (any, error) {
+			inspect, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+			if !ok {
+				return nil, errCouldNotGetInspector
+			}
+
+			nodeFilter := []ast.Node{
+				(*ast.Field)(nil),
+			}
+
+			inspect.Preorder(nodeFilter, func(n ast.Node) {
+				field, ok := n.(*ast.Field)
+				if !ok {
+					return
+				}
+				if len(field.Names) == 0 {
+					return
+				}
+
+				structName := utils.GetStructNameForField(pass, field)
+				if structName != "" {
+					pass.Reportf(field.Pos(), "field %s is in struct %s", field.Names[0].Name, structName)
+				}
+			})
+
+			return nil, nil
+		},
+	}
+}