diff --git a/pkg/analysis/commentstart/analyzer.go b/pkg/analysis/commentstart/analyzer.go index e9492c73..b6744f13 100644 --- a/pkg/analysis/commentstart/analyzer.go +++ b/pkg/analysis/commentstart/analyzer.go @@ -27,6 +27,7 @@ import ( "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags" "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector" "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers" + "sigs.k8s.io/kube-api-linter/pkg/analysis/utils" ) const name = "commentstart" @@ -58,10 +59,8 @@ func checkField(pass *analysis.Pass, field *ast.Field, tagInfo extractjsontags.F return } - var fieldName string - if len(field.Names) > 0 { - fieldName = field.Names[0].Name - } else { + fieldName := utils.FieldName(field) + if fieldName == "" { fieldName = types.ExprString(field.Type) } diff --git a/pkg/analysis/helpers/inspector/analyzer_test.go b/pkg/analysis/helpers/inspector/analyzer_test.go index d8318b7b..9f9b1d34 100644 --- a/pkg/analysis/helpers/inspector/analyzer_test.go +++ b/pkg/analysis/helpers/inspector/analyzer_test.go @@ -25,6 +25,7 @@ import ( "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags" "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector" "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers" + "sigs.k8s.io/kube-api-linter/pkg/analysis/utils" ) func TestInspector(t *testing.T) { @@ -49,14 +50,7 @@ func run(pass *analysis.Pass) (any, error) { } inspect.InspectFields(func(field *ast.Field, stack []ast.Node, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markers.Markers) { - var fieldName string - if len(field.Names) > 0 { - fieldName = field.Names[0].Name - } else if ident, ok := field.Type.(*ast.Ident); ok { - fieldName = ident.Name - } - - pass.Reportf(field.Pos(), "field: %v", fieldName) + pass.Reportf(field.Pos(), "field: %v", utils.FieldName(field)) if jsonTagInfo.Name != "" { pass.Reportf(field.Pos(), "json tag: %v", jsonTagInfo.Name) diff --git a/pkg/analysis/jsontags/analyzer.go b/pkg/analysis/jsontags/analyzer.go index 6487f00c..958bae39 100644 --- a/pkg/analysis/jsontags/analyzer.go +++ b/pkg/analysis/jsontags/analyzer.go @@ -24,6 +24,7 @@ import ( "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags" "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector" "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers" + "sigs.k8s.io/kube-api-linter/pkg/analysis/utils" "sigs.k8s.io/kube-api-linter/pkg/config" "golang.org/x/tools/go/analysis" @@ -75,13 +76,13 @@ func (a *analyzer) run(pass *analysis.Pass) (any, error) { } func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field, tagInfo extractjsontags.FieldTagInfo) { - var prefix string - if len(field.Names) > 0 && field.Names[0] != nil { - prefix = fmt.Sprintf("field %s", field.Names[0].Name) - } else if ident, ok := field.Type.(*ast.Ident); ok { - prefix = fmt.Sprintf("embedded field %s", ident.Name) + prefix := "field %s" + if len(field.Names) == 0 || field.Names[0] == nil { + prefix = "embedded field %s" } + prefix = fmt.Sprintf(prefix, utils.FieldName(field)) + if tagInfo.Missing { pass.Reportf(field.Pos(), "%s is missing json tag", prefix) return diff --git a/pkg/analysis/maxlength/analyzer.go b/pkg/analysis/maxlength/analyzer.go index 72a44306..82b01fb6 100644 --- a/pkg/analysis/maxlength/analyzer.go +++ b/pkg/analysis/maxlength/analyzer.go @@ -55,11 +55,11 @@ func run(pass *analysis.Pass) (any, error) { } func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markershelper.Markers) { - if len(field.Names) == 0 || field.Names[0] == nil { + fieldName := utils.FieldName(field) + if fieldName == "" { return } - fieldName := field.Names[0].Name prefix := fmt.Sprintf("field %s", fieldName) checkTypeExpr(pass, field.Type, field, nil, markersAccess, prefix, markers.KubebuilderMaxLengthMarker, needsStringMaxLength) diff --git a/pkg/analysis/nomaps/analyzer.go b/pkg/analysis/nomaps/analyzer.go index 6469cd8d..99ac9e52 100644 --- a/pkg/analysis/nomaps/analyzer.go +++ b/pkg/analysis/nomaps/analyzer.go @@ -26,6 +26,7 @@ import ( "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags" "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector" "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers" + "sigs.k8s.io/kube-api-linter/pkg/analysis/utils" "sigs.k8s.io/kube-api-linter/pkg/config" ) @@ -81,7 +82,7 @@ func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field) { } if a.policy == config.NoMapsEnforce { - report(pass, field.Pos(), field.Names[0].Name) + report(pass, field.Pos(), utils.FieldName(field)) return } @@ -90,7 +91,7 @@ func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field) { return } - report(pass, field.Pos(), field.Names[0].Name) + report(pass, field.Pos(), utils.FieldName(field)) } if a.policy == config.NoMapsIgnore { @@ -104,7 +105,7 @@ func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field) { return } - report(pass, field.Pos(), field.Names[0].Name) + report(pass, field.Pos(), utils.FieldName(field)) } } diff --git a/pkg/analysis/nophase/analyzer.go b/pkg/analysis/nophase/analyzer.go index 73e04ba7..c7201aed 100644 --- a/pkg/analysis/nophase/analyzer.go +++ b/pkg/analysis/nophase/analyzer.go @@ -24,6 +24,7 @@ import ( "golang.org/x/tools/go/ast/inspector" kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors" "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags" + "sigs.k8s.io/kube-api-linter/pkg/analysis/utils" ) const name = "nophase" @@ -68,7 +69,7 @@ func run(pass *analysis.Pass) (any, error) { return } - fieldName := field.Names[0].Name + fieldName := utils.FieldName(field) // First check if the struct field name contains 'phase' if strings.Contains(strings.ToLower(fieldName), "phase") { diff --git a/pkg/analysis/optionalorrequired/analyzer.go b/pkg/analysis/optionalorrequired/analyzer.go index b45f1ce4..d996592d 100644 --- a/pkg/analysis/optionalorrequired/analyzer.go +++ b/pkg/analysis/optionalorrequired/analyzer.go @@ -24,6 +24,7 @@ import ( "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags" "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector" markershelper "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers" + "sigs.k8s.io/kube-api-linter/pkg/analysis/utils" "sigs.k8s.io/kube-api-linter/pkg/config" "sigs.k8s.io/kube-api-linter/pkg/markers" ) @@ -107,13 +108,13 @@ func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field, fieldMarker return } - var prefix string - if len(field.Names) > 0 && field.Names[0] != nil { - prefix = fmt.Sprintf("field %s", field.Names[0].Name) - } else if ident, ok := field.Type.(*ast.Ident); ok { - prefix = fmt.Sprintf("embedded field %s", ident.Name) + prefix := "field %s" + if len(field.Names) == 0 || field.Names[0] == nil { + prefix = "embedded field %s" } + prefix = fmt.Sprintf(prefix, utils.FieldName(field)) + hasPrimaryOptional := fieldMarkers.Has(a.primaryOptionalMarker) hasPrimaryRequired := fieldMarkers.Has(a.primaryRequiredMarker) diff --git a/pkg/analysis/requiredfields/analyzer.go b/pkg/analysis/requiredfields/analyzer.go index e2902cc3..380f8225 100644 --- a/pkg/analysis/requiredfields/analyzer.go +++ b/pkg/analysis/requiredfields/analyzer.go @@ -25,6 +25,7 @@ import ( "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags" "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector" markershelper "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers" + "sigs.k8s.io/kube-api-linter/pkg/analysis/utils" "sigs.k8s.io/kube-api-linter/pkg/config" "sigs.k8s.io/kube-api-linter/pkg/markers" ) @@ -71,12 +72,11 @@ func (a *analyzer) run(pass *analysis.Pass) (any, error) { } func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field, fieldMarkers markershelper.MarkerSet, fieldTagInfo extractjsontags.FieldTagInfo) { - if field == nil || len(field.Names) == 0 { + fieldName := utils.FieldName(field) + if fieldName == "" { return } - fieldName := field.Names[0].Name - if !fieldMarkers.Has(markers.RequiredMarker) && !fieldMarkers.Has(markers.KubebuilderRequiredMarker) { // The field is not marked required, so we don't need to check it. return diff --git a/pkg/analysis/utils/type_check.go b/pkg/analysis/utils/type_check.go index 4c56c783..d6b8e1b4 100644 --- a/pkg/analysis/utils/type_check.go +++ b/pkg/analysis/utils/type_check.go @@ -62,11 +62,11 @@ func (t *typeChecker) CheckNode(pass *analysis.Pass, node ast.Node) { } func (t *typeChecker) checkField(pass *analysis.Pass, field *ast.Field) { - if field == nil || len(field.Names) == 0 || field.Names[0] == nil { + fieldName := FieldName(field) + if fieldName == "" { return } - fieldName := field.Names[0].Name prefix := fmt.Sprintf("field %s", fieldName) t.checkTypeExpr(pass, field.Type, field, prefix) diff --git a/pkg/analysis/utils/utils.go b/pkg/analysis/utils/utils.go index dd5e44d1..09439b91 100644 --- a/pkg/analysis/utils/utils.go +++ b/pkg/analysis/utils/utils.go @@ -77,6 +77,26 @@ func LookupTypeSpec(pass *analysis.Pass, ident *ast.Ident) (*ast.TypeSpec, bool) return nil, false } +// FieldName returns the name of the field. If the field has a name, it returns that name. +// If the field is embedded and it can be converted to an identifier, it returns the name of the identifier. +// If it doesn't have a name and can't be converted to an identifier, it returns an empty string. +func FieldName(field *ast.Field) string { + if len(field.Names) > 0 && field.Names[0] != nil { + return field.Names[0].Name + } + + switch typ := field.Type.(type) { + case *ast.Ident: + return typ.Name + case *ast.StarExpr: + if ident, ok := typ.X.(*ast.Ident); ok { + return ident.Name + } + } + + return "" +} + func getFilesForType(pass *analysis.Pass, ident *ast.Ident) (*token.File, *ast.File) { namedType, ok := pass.TypesInfo.TypeOf(ident).(*types.Named) if !ok { diff --git a/pkg/analysis/utils/utils_suite_test.go b/pkg/analysis/utils/utils_suite_test.go new file mode 100644 index 00000000..ed097331 --- /dev/null +++ b/pkg/analysis/utils/utils_suite_test.go @@ -0,0 +1,28 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package utils_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestValidation(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Utils") +} diff --git a/pkg/analysis/utils/utils_test.go b/pkg/analysis/utils/utils_test.go new file mode 100644 index 00000000..8eb7b6ed --- /dev/null +++ b/pkg/analysis/utils/utils_test.go @@ -0,0 +1,75 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package utils_test + +import ( + "go/ast" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "sigs.k8s.io/kube-api-linter/pkg/analysis/utils" +) + +var _ = Describe("FieldName", func() { + type fieldNameInput struct { + field *ast.Field + want string + } + + DescribeTable("Should extract the field name", func(in fieldNameInput) { + Expect(utils.FieldName(in.field)).To(Equal(in.want), "expect to match the extracted field name") + }, + Entry("field has Names", fieldNameInput{ + field: &ast.Field{ + Names: []*ast.Ident{ + { + Name: "foo", + }, + }, + }, + want: "foo", + }), + Entry("field has no Names, but is an Ident", fieldNameInput{ + field: &ast.Field{ + Type: &ast.Ident{ + Name: "foo", + }, + }, + want: "foo", + }), + Entry("field has no Names, but is a StarExpr with an Ident", fieldNameInput{ + field: &ast.Field{ + Type: &ast.StarExpr{ + X: &ast.Ident{ + Name: "foo", + }, + }, + }, + want: "foo", + }), + Entry("field has no Names, and is not an Ident or StarExpr", fieldNameInput{ + field: &ast.Field{ + Type: &ast.ArrayType{ + Elt: &ast.Ident{ + Name: "foo", + }, + }, + }, + want: "", + }), + ) +})