Ensure type declarations are picked up across file boundaries

JoelSpeed · JoelSpeed · commit 7fd5702794ad · 2025-04-30T18:12:12.000+01:00
diff --git a/pkg/analysis/helpers/inspector/analyzer.go b/pkg/analysis/helpers/inspector/analyzer.go
@@ -19,7 +19,6 @@ import (
 	"reflect"
 
 	"golang.org/x/tools/go/analysis"
-	"golang.org/x/tools/go/analysis/passes/inspect"
 	astinspector "golang.org/x/tools/go/ast/inspector"
 
 	kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors"
@@ -36,15 +35,12 @@ var Analyzer = &analysis.Analyzer{
 	Name:       name,
 	Doc:        "Provides common functionality for analyzers that need to inspect fields and struct",
 	Run:        run,
-	Requires:   []*analysis.Analyzer{inspect.Analyzer, extractjsontags.Analyzer, markers.Analyzer},
+	Requires:   []*analysis.Analyzer{extractjsontags.Analyzer, markers.Analyzer},
 	ResultType: reflect.TypeOf(newInspector(nil, nil, nil)),
 }
 
 func run(pass *analysis.Pass) (any, error) {
-	astinspector, ok := pass.ResultOf[inspect.Analyzer].(*astinspector.Inspector)
-	if !ok {
-		return nil, kalerrors.ErrCouldNotGetInspector
-	}
+	astInspector := astinspector.New(pass.Files)
 
 	jsonTags, ok := pass.ResultOf[extractjsontags.Analyzer].(extractjsontags.StructFieldTags)
 	if !ok {
@@ -56,5 +52,5 @@ func run(pass *analysis.Pass) (any, error) {
 		return nil, kalerrors.ErrCouldNotGetMarkers
 	}
 
-	return newInspector(astinspector, jsonTags, markersAccess), nil
+	return newInspector(astInspector, jsonTags, markersAccess), nil
 }
diff --git a/pkg/analysis/integers/testdata/src/a/a.go b/pkg/analysis/integers/testdata/src/a/a.go
@@ -86,6 +86,10 @@ type Integers struct {
 	InvalidMapIntToString map[int]string // want "field InvalidMapIntToString map key should not use an int, int8 or int16. Use int32 or int64 depending on bounding requirements"
 
 	InvalidMapUIntToString map[uint]string // want "field InvalidMapUIntToString map key should not use unsigned integers, use only int32 or int64 and apply validation to ensure the value is positive"
+
+	InvalidIntFromAnotherFile IntB // want "field InvalidIntFromAnotherFile type IntB should not use an int, int8 or int16. Use int32 or int64 depending on bounding requirements"
+
+	InvalidSliceIntAliasFromAnotherFile InvalidSliceIntAliasB // want "field InvalidSliceIntAliasFromAnotherFile type InvalidSliceIntAliasB array element should not use an int, int8 or int16. Use int32 or int64 depending on bounding requirements"
 }
 
 // DoNothing is used to check that the analyser doesn't report on methods.
diff --git a/pkg/analysis/integers/testdata/src/a/b.go b/pkg/analysis/integers/testdata/src/a/b.go
@@ -0,0 +1,6 @@
+package a
+
+// IntB is being used to show a type from a different file.
+type IntB int // want "type IntB should not use an int, int8 or int16. Use int32 or int64 depending on bounding requirements"
+
+type InvalidSliceIntAliasB []int // want "type InvalidSliceIntAliasB array element should not use an int, int8 or int16. Use int32 or int64 depending on bounding requirements"
diff --git a/pkg/analysis/maxlength/analyzer.go b/pkg/analysis/maxlength/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"
 )
 
 const (
@@ -74,13 +75,13 @@ func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markers.Mar
 }
 
 func checkIdent(pass *analysis.Pass, ident *ast.Ident, node ast.Node, aliases []*ast.TypeSpec, markersAccess markers.Markers, prefix, marker string, needsMaxLength func(markers.MarkerSet) bool) {
-	if ident.Obj == nil { // Built-in type
+	if utils.IsBasicType(pass, ident) { // Built-in type
 		checkString(pass, ident, node, aliases, markersAccess, prefix, marker, needsMaxLength)
 
 		return
 	}
 
-	tSpec, ok := ident.Obj.Decl.(*ast.TypeSpec)
+	tSpec, ok := utils.LookupTypeSpec(pass, ident)
 	if !ok {
 		return
 	}
diff --git a/pkg/analysis/maxlength/testdata/src/a/a.go b/pkg/analysis/maxlength/testdata/src/a/a.go
@@ -9,6 +9,13 @@ type MaxLength struct {
 
 	StringAliasWithMaxLengthOnAlias StringAliasWithMaxLength
 
+	StringAliasFromAnotherFile StringAliasB // want "field StringAliasFromAnotherFile type StringAliasB must have a maximum length, add kubebuilder:validation:MaxLength marker"
+
+	// +kubebuilder:validation:MaxLength:=128
+	StringAliasFromAnotherFileWithMaxLengthOnField StringAliasB
+
+	StringAliasWithMaxLengthFromAnotherFile StringAliasWithMaxLengthB
+
 	StringWithoutMaxLength string // want "field StringWithoutMaxLength must have a maximum length, add kubebuilder:validation:MaxLength marker"
 
 	StringAliasWithoutMaxLength StringAlias // want "field StringAliasWithoutMaxLength type StringAlias must have a maximum length, add kubebuilder:validation:MaxLength marker"
diff --git a/pkg/analysis/maxlength/testdata/src/a/b.go b/pkg/analysis/maxlength/testdata/src/a/b.go
@@ -0,0 +1,8 @@
+package a
+
+// StringAliasB is a string without a MaxLength.
+type StringAliasB string
+
+// StringAliasWithMaxLengthB is a string with a MaxLength.
+// +kubebuilder:validation:MaxLength:=512
+type StringAliasWithMaxLengthB string
diff --git a/pkg/analysis/nobools/testdata/src/a/a.go b/pkg/analysis/nobools/testdata/src/a/a.go
@@ -32,6 +32,10 @@ type Bools struct {
 	InvalidMapBoolToString map[bool]string // want "field InvalidMapBoolToString map key should not use a bool. Use a string type with meaningful constant values as an enum."
 
 	InvalidMapBoolPtrToString map[*bool]string // want "field InvalidMapBoolPtrToString map key pointer should not use a bool. Use a string type with meaningful constant values as an enum."
+
+	InvalidBoolAliasFromAnotherFile BoolAliasB // want "field InvalidBoolAliasFromAnotherFile type BoolAliasB should not use a bool. Use a string type with meaningful constant values as an enum."
+
+	InvalidBoolPtrAliasFromAnotherFile *BoolAliasB // want "field InvalidBoolPtrAliasFromAnotherFile pointer type BoolAliasB should not use a bool. Use a string type with meaningful constant values as an enum."
 }
 
 // DoNothing is used to check that the analyser doesn't report on methods.
diff --git a/pkg/analysis/nobools/testdata/src/a/b.go b/pkg/analysis/nobools/testdata/src/a/b.go
@@ -0,0 +1,5 @@
+package a
+
+type BoolAliasB bool // want "type BoolAliasB should not use a bool. Use a string type with meaningful constant values as an enum."
+
+type BoolAliasPtrB *bool // want "type BoolAliasPtrB pointer should not use a bool. Use a string type with meaningful constant values as an enum."
diff --git a/pkg/analysis/nofloats/testdata/src/a/a.go b/pkg/analysis/nofloats/testdata/src/a/a.go
@@ -56,6 +56,10 @@ type Floats struct {
 	InvalidMapFloat32PtrToString map[*float32]string // want "field InvalidMapFloat32PtrToString map key pointer should not use a float value because they cannot be reliably round-tripped."
 
 	InvalidMapFloat64PtrToString map[*float64]string // want "field InvalidMapFloat64PtrToString map key pointer should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidFloat32AliasFromAnotherFile Float32AliasB // want "field InvalidFloat32AliasFromAnotherFile type Float32AliasB should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidFloat32PtrAliasFromAnotherFile Float32AliasPtrB // want "field InvalidFloat32PtrAliasFromAnotherFile type Float32AliasPtrB pointer should not use a float value because they cannot be reliably round-tripped."
 }
 
 // DoNothingFloat32 is used to check that the analyser doesn't report on methods.
diff --git a/pkg/analysis/nofloats/testdata/src/a/b.go b/pkg/analysis/nofloats/testdata/src/a/b.go
@@ -0,0 +1,5 @@
+package a
+
+type Float32AliasB float32 // want "type Float32AliasB should not use a float value because they cannot be reliably round-tripped."
+
+type Float32AliasPtrB *float32 // want "type Float32AliasPtrB pointer should not use a float value because they cannot be reliably round-tripped."
diff --git a/pkg/analysis/nomaps/testdata/src/a/a.go b/pkg/analysis/nomaps/testdata/src/a/a.go
@@ -38,6 +38,13 @@ type NoMapsTestStructWithDefiningType struct {
 	MapIntString          MapIntString          `json:"mapIntString"`          // want "MapIntString should not use a map type, use a list type with a unique name/identifier instead"
 }
 
+type NoMapsTestStructWithDefiningTypeAcrossFiles struct {
+	MapStringComponent    MapStringComponentB    `json:"mapStringComponent"`    // want "MapStringComponent should not use a map type, use a list type with a unique name/identifier instead"
+	PtrMapStringComponent PtrMapStringComponentB `json:"ptrMapStringComponent"` // want "PtrMapStringComponent should not use a map type, use a list type with a unique name/identifier instead"
+	MapStringInt          MapStringIntB          `json:"mapStringInt"`          // want "MapStringInt should not use a map type, use a list type with a unique name/identifier instead"
+	MapIntString          MapIntStringB          `json:"mapIntString"`          // want "MapIntString should not use a map type, use a list type with a unique name/identifier instead"
+}
+
 type NoMapsTestStructWithAlias struct {
 	MapStringComponentAlias           MapStringComponentAlias           `json:"mapStringComponentAlias"`           // want "MapStringComponentAlias should not use a map type, use a list type with a unique name/identifier instead"
 	MapStringPtrComponentAlias        MapStringPtrComponentAlias        `json:"mapStringPtrComponentAlias"`        // want "MapStringPtrComponentAlias should not use a map type, use a list type with a unique name/identifier instead"
diff --git a/pkg/analysis/nomaps/testdata/src/a/b.go b/pkg/analysis/nomaps/testdata/src/a/b.go
@@ -0,0 +1,8 @@
+package a
+
+type (
+	MapStringComponentB    map[string]Component
+	PtrMapStringComponentB *map[string]Component
+	MapStringIntB          map[string]int
+	MapIntStringB          map[int]string
+)
diff --git a/pkg/analysis/utils/testdata/src/a/a.go b/pkg/analysis/utils/testdata/src/a/a.go
@@ -26,6 +26,8 @@ type Integers struct {
 	StringAliasSlice []StringAlias // want "field StringAliasSlice array element type StringAlias is a string"
 
 	StringAliasPtrSlice []*StringAlias // want "field StringAliasPtrSlice array element pointer type StringAlias is a string"
+
+	StringAliasFromAnotherFile StringAliasB // want "field StringAliasFromAnotherFile type StringAliasB is a string"
 }
 
 type StringAlias string // want "type StringAlias is a string"
diff --git a/pkg/analysis/utils/testdata/src/a/b.go b/pkg/analysis/utils/testdata/src/a/b.go
@@ -0,0 +1,3 @@
+package a
+
+type StringAliasB string // want "type StringAliasB is a string"
diff --git a/pkg/analysis/utils/type_check.go b/pkg/analysis/utils/type_check.go
@@ -100,13 +100,13 @@ func (t *typeChecker) checkTypeExpr(pass *analysis.Pass, typeExpr ast.Expr, node
 // checkIdent calls the checkFunc with the ident, when we have hit a built-in type.
 // If the ident is not a built in, we look at the underlying type until we hit a built-in type.
 func (t *typeChecker) checkIdent(pass *analysis.Pass, ident *ast.Ident, node ast.Node, prefix string) {
-	if ident.Obj == nil || ident.Obj.Decl == nil {
+	if IsBasicType(pass, ident) {
 		// We've hit a built-in type, no need to check further.
 		t.checkFunc(pass, ident, node, prefix)
 		return
 	}
 
-	tSpec, ok := ident.Obj.Decl.(*ast.TypeSpec)
+	tSpec, ok := LookupTypeSpec(pass, ident)
 	if !ok {
 		return
 	}
diff --git a/pkg/analysis/utils/utils.go b/pkg/analysis/utils/utils.go
@@ -0,0 +1,99 @@
+/*
+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
+
+import (
+	"go/ast"
+	"go/token"
+	"go/types"
+
+	"golang.org/x/tools/go/analysis"
+)
+
+// IsBasicType checks if the type of the given identifier is a basic type.
+// Basic types are types like int, string, bool, etc.
+func IsBasicType(pass *analysis.Pass, ident *ast.Ident) bool {
+	_, ok := pass.TypesInfo.TypeOf(ident).(*types.Basic)
+	return ok
+}
+
+// LookupTypeSpec is used to search for the type spec of a given identifier.
+// It will first check to see if the ident has an Obj, and if so, it will return the type spec
+// from the Obj. If the Obj is nil, it will search through the files in the package to find the
+// type spec that matches the identifier's position.
+func LookupTypeSpec(pass *analysis.Pass, ident *ast.Ident) (*ast.TypeSpec, bool) {
+	if ident.Obj != nil && ident.Obj.Decl != nil {
+		// The identifier has an Obj, we can use it to find the type spec.
+		if tSpec, ok := ident.Obj.Decl.(*ast.TypeSpec); ok {
+			return tSpec, true
+		}
+	}
+
+	namedType, ok := pass.TypesInfo.TypeOf(ident).(*types.Named)
+	if !ok {
+		return nil, false
+	}
+
+	if !isInPassPackage(pass, namedType) {
+		// The identifier is not in the pass package, we can't find the type spec.
+		return nil, false
+	}
+
+	tokenFile, astFile := getFilesForType(pass, ident)
+
+	if astFile == nil {
+		// We couldn't match the token.File to the ast.File.
+		return nil, false
+	}
+
+	for n := range ast.Preorder(astFile) {
+		tSpec, ok := n.(*ast.TypeSpec)
+		if !ok {
+			continue
+		}
+
+		// Token files are 1-based, while ast files are 0-based.
+		// We need to adjust the position to match the token file.
+		filePos := tSpec.Pos() - astFile.FileStart + token.Pos(tokenFile.Base())
+
+		if filePos == namedType.Obj().Pos() {
+			return tSpec, true
+		}
+	}
+
+	return nil, false
+}
+
+func getFilesForType(pass *analysis.Pass, ident *ast.Ident) (*token.File, *ast.File) {
+	namedType, ok := pass.TypesInfo.TypeOf(ident).(*types.Named)
+	if !ok {
+		return nil, nil
+	}
+
+	tokenFile := pass.Fset.File(namedType.Obj().Pos())
+
+	for _, astFile := range pass.Files {
+		if astFile.Package == token.Pos(tokenFile.Base()) {
+			return tokenFile, astFile
+		}
+	}
+
+	return tokenFile, nil
+}
+
+func isInPassPackage(pass *analysis.Pass, namedType *types.Named) bool {
+	return namedType.Obj().Pkg() != nil && namedType.Obj().Pkg() == pass.Pkg
+}

Original file line number	Diff line number	Diff line change
`@@ -86,6 +86,10 @@ type Integers struct {`
`86`	`86`	`InvalidMapIntToString map[int]string // want "field InvalidMapIntToString map key should not use an int, int8 or int16. Use int32 or int64 depending on bounding requirements"`
`87`	`87`
`88`	`88`	`InvalidMapUIntToString map[uint]string // want "field InvalidMapUIntToString map key should not use unsigned integers, use only int32 or int64 and apply validation to ensure the value is positive"`
	`89`	`+`
	`90`	`+ InvalidIntFromAnotherFile IntB // want "field InvalidIntFromAnotherFile type IntB should not use an int, int8 or int16. Use int32 or int64 depending on bounding requirements"`
	`91`	`+`
	`92`	`+ InvalidSliceIntAliasFromAnotherFile InvalidSliceIntAliasB // want "field InvalidSliceIntAliasFromAnotherFile type InvalidSliceIntAliasB array element should not use an int, int8 or int16. Use int32 or int64 depending on bounding requirements"`
`89`	`93`	`}`
`90`	`94`
`91`	`95`	`// DoNothing is used to check that the analyser doesn't report on methods.`
Original file line number	Diff line number	Diff line change
`@@ -24,6 +24,7 @@ import (`
`24`	`24`	`"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags"`
`25`	`25`	`"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector"`
`26`	`26`	`"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers"`
	`27`	`+ "sigs.k8s.io/kube-api-linter/pkg/analysis/utils"`
`27`	`28`	`)`
`28`	`29`
`29`	`30`	`const (`
`@@ -74,13 +75,13 @@ func checkField(pass analysis.Pass, field ast.Field, markersAccess markers.Mar`
`74`	`75`	`}`
`75`	`76`
`76`	`77`	`func checkIdent(pass analysis.Pass, ident ast.Ident, node ast.Node, aliases []*ast.TypeSpec, markersAccess markers.Markers, prefix, marker string, needsMaxLength func(markers.MarkerSet) bool) {`
`77`		`- if ident.Obj == nil { // Built-in type`
	`78`	`+ if utils.IsBasicType(pass, ident) { // Built-in type`
`78`	`79`	`checkString(pass, ident, node, aliases, markersAccess, prefix, marker, needsMaxLength)`
`79`	`80`
`80`	`81`	`return`
`81`	`82`	`}`
`82`	`83`
`83`		`- tSpec, ok := ident.Obj.Decl.(*ast.TypeSpec)`
	`84`	`+ tSpec, ok := utils.LookupTypeSpec(pass, ident)`
`84`	`85`	`if !ok {`
`85`	`86`	`return`
`86`	`87`	`}`
Original file line number	Diff line number	Diff line change
`@@ -32,6 +32,10 @@ type Bools struct {`
`32`	`32`	`InvalidMapBoolToString map[bool]string // want "field InvalidMapBoolToString map key should not use a bool. Use a string type with meaningful constant values as an enum."`
`33`	`33`
`34`	`34`	`InvalidMapBoolPtrToString map[*bool]string // want "field InvalidMapBoolPtrToString map key pointer should not use a bool. Use a string type with meaningful constant values as an enum."`
	`35`	`+`
	`36`	`+ InvalidBoolAliasFromAnotherFile BoolAliasB // want "field InvalidBoolAliasFromAnotherFile type BoolAliasB should not use a bool. Use a string type with meaningful constant values as an enum."`
	`37`	`+`
	`38`	`+ InvalidBoolPtrAliasFromAnotherFile *BoolAliasB // want "field InvalidBoolPtrAliasFromAnotherFile pointer type BoolAliasB should not use a bool. Use a string type with meaningful constant values as an enum."`
`35`	`39`	`}`
`36`	`40`
`37`	`41`	`// DoNothing is used to check that the analyser doesn't report on methods.`
Original file line number	Diff line number	Diff line change
`@@ -56,6 +56,10 @@ type Floats struct {`
`56`	`56`	`InvalidMapFloat32PtrToString map[*float32]string // want "field InvalidMapFloat32PtrToString map key pointer should not use a float value because they cannot be reliably round-tripped."`
`57`	`57`
`58`	`58`	`InvalidMapFloat64PtrToString map[*float64]string // want "field InvalidMapFloat64PtrToString map key pointer should not use a float value because they cannot be reliably round-tripped."`
	`59`	`+`
	`60`	`+ InvalidFloat32AliasFromAnotherFile Float32AliasB // want "field InvalidFloat32AliasFromAnotherFile type Float32AliasB should not use a float value because they cannot be reliably round-tripped."`
	`61`	`+`
	`62`	`+ InvalidFloat32PtrAliasFromAnotherFile Float32AliasPtrB // want "field InvalidFloat32PtrAliasFromAnotherFile type Float32AliasPtrB pointer should not use a float value because they cannot be reliably round-tripped."`
`59`	`63`	`}`
`60`	`64`
`61`	`65`	`// DoNothingFloat32 is used to check that the analyser doesn't report on methods.`