Skip to content

Commit 74c4ae9

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #196 from JoelSpeed/broader-type-checker
Cleanup: Align more linters to the type checker pattern
2 parents 91d1bdb + c7b56f8 commit 74c4ae9

File tree

13 files changed

+268
-275
lines changed

13 files changed

+268
-275
lines changed

‎pkg/analysis/integers/analyzer.go‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@ import (
1919
"go/ast"
2020

2121
"golang.org/x/tools/go/analysis"
22-
"golang.org/x/tools/go/analysis/passes/inspect"
23-
"golang.org/x/tools/go/ast/inspector"
22+
2423
kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors"
24+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags"
25+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector"
26+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers"
2527
"sigs.k8s.io/kube-api-linter/pkg/analysis/utils"
2628
)
2729

@@ -33,37 +35,35 @@ var Analyzer = &analysis.Analyzer{
3335
Name: name,
3436
Doc: "All integers should be explicit about their size, int32 and int64 should be used over plain int. Unsigned ints are not allowed.",
3537
Run: run,
36-
Requires: []*analysis.Analyzer{inspect.Analyzer},
38+
Requires: []*analysis.Analyzer{inspector.Analyzer},
3739
}
3840

3941
func run(pass *analysis.Pass) (any, error) {
40-
inspect, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
42+
inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector)
4143
if !ok {
4244
return nil, kalerrors.ErrCouldNotGetInspector
4345
}
4446

45-
// Filter to fields so that we can look at fields within structs.
46-
// Filter typespecs so that we can look at type aliases.
47-
nodeFilter := []ast.Node{
48-
(*ast.StructType)(nil),
49-
(*ast.TypeSpec)(nil),
50-
}
47+
typeChecker := utils.NewTypeChecker(utils.IsBasicType, checkIntegers)
5148

52-
typeChecker := utils.NewTypeChecker(checkIntegers)
49+
inspect.InspectFields(func(field *ast.Field, _ extractjsontags.FieldTagInfo, _ markers.Markers, _ string) {
50+
typeChecker.CheckNode(pass, field)
51+
})
5352

54-
// Preorder visits all the nodes of the AST in depth-first order. It calls
55-
// f(n) for each node n before it visits n's children.
56-
//
57-
// We use the filter defined above, ensuring we only look at struct fields and type declarations.
58-
inspect.Preorder(nodeFilter, func(n ast.Node) {
59-
typeChecker.CheckNode(pass, n)
53+
inspect.InspectTypeSpec(func(typeSpec *ast.TypeSpec, markersAccess markers.Markers) {
54+
typeChecker.CheckNode(pass, typeSpec)
6055
})
6156

6257
return nil, nil //nolint:nilnil
6358
}
6459

6560
// checkIntegers looks for known type of integers that do not match the allowed `int32` or `int64` requirements.
66-
func checkIntegers(pass *analysis.Pass, ident *ast.Ident, node ast.Node, prefix string) {
61+
func checkIntegers(pass *analysis.Pass, expr ast.Expr, node ast.Node, prefix string) {
62+
ident, ok := expr.(*ast.Ident)
63+
if !ok {
64+
return
65+
}
66+
6767
switch ident.Name {
6868
case "int32", "int64":
6969
// Valid cases

‎pkg/analysis/nobools/analyzer.go‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@ import (
1919
"go/ast"
2020

2121
"golang.org/x/tools/go/analysis"
22-
"golang.org/x/tools/go/analysis/passes/inspect"
23-
"golang.org/x/tools/go/ast/inspector"
22+
2423
kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors"
24+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags"
25+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector"
26+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers"
2527
"sigs.k8s.io/kube-api-linter/pkg/analysis/utils"
2628
)
2729

@@ -33,36 +35,34 @@ var Analyzer = &analysis.Analyzer{
3335
Name: name,
3436
Doc: "Boolean values cannot evolve over time, use an enum with meaningful values instead",
3537
Run: run,
36-
Requires: []*analysis.Analyzer{inspect.Analyzer},
38+
Requires: []*analysis.Analyzer{inspector.Analyzer},
3739
}
3840

3941
func run(pass *analysis.Pass) (any, error) {
40-
inspect, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
42+
inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector)
4143
if !ok {
4244
return nil, kalerrors.ErrCouldNotGetInspector
4345
}
4446

45-
// Filter to fields so that we can look at fields within structs.
46-
// Filter typespecs so that we can look at type aliases.
47-
nodeFilter := []ast.Node{
48-
(*ast.StructType)(nil),
49-
(*ast.TypeSpec)(nil),
50-
}
47+
typeChecker := utils.NewTypeChecker(utils.IsBasicType, checkBool)
5148

52-
typeChecker := utils.NewTypeChecker(checkBool)
49+
inspect.InspectFields(func(field *ast.Field, _ extractjsontags.FieldTagInfo, _ markers.Markers, _ string) {
50+
typeChecker.CheckNode(pass, field)
51+
})
5352

54-
// Preorder visits all the nodes of the AST in depth-first order. It calls
55-
// f(n) for each node n before it visits n's children.
56-
//
57-
// We use the filter defined above, ensuring we only look at struct fields and type declarations.
58-
inspect.Preorder(nodeFilter, func(n ast.Node) {
59-
typeChecker.CheckNode(pass, n)
53+
inspect.InspectTypeSpec(func(typeSpec *ast.TypeSpec, markersAccess markers.Markers) {
54+
typeChecker.CheckNode(pass, typeSpec)
6055
})
6156

6257
return nil, nil //nolint:nilnil
6358
}
6459

65-
func checkBool(pass *analysis.Pass, ident *ast.Ident, node ast.Node, prefix string) {
60+
func checkBool(pass *analysis.Pass, expr ast.Expr, node ast.Node, prefix string) {
61+
ident, ok := expr.(*ast.Ident)
62+
if !ok {
63+
return
64+
}
65+
6666
if ident.Name == "bool" {
6767
pass.Reportf(node.Pos(), "%s should not use a bool. Use a string type with meaningful constant values as an enum.", prefix)
6868
}

‎pkg/analysis/nodurations/analyzer.go‎

Lines changed: 20 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ limitations under the License.
1717
package nodurations
1818

1919
import (
20-
"fmt"
2120
"go/ast"
2221

2322
"golang.org/x/tools/go/analysis"
@@ -45,78 +44,41 @@ func run(pass *analysis.Pass) (any, error) {
4544
return nil, kalerrors.ErrCouldNotGetInspector
4645
}
4746

48-
inspect.InspectFields(func(field *ast.Field, _ extractjsontags.FieldTagInfo, markersAccess markers.Markers, qualifiedFieldName string) {
49-
checkField(pass, field, qualifiedFieldName)
47+
typeChecker := utils.NewTypeChecker(isDurationType, checkDuration)
48+
49+
inspect.InspectFields(func(field *ast.Field, _ extractjsontags.FieldTagInfo, _ markers.Markers, _ string) {
50+
typeChecker.CheckNode(pass, field)
5051
})
5152

5253
inspect.InspectTypeSpec(func(typeSpec *ast.TypeSpec, markersAccess markers.Markers) {
53-
checkTypeSpec(pass, typeSpec, typeSpec, "type")
54+
typeChecker.CheckNode(pass, typeSpec)
5455
})
5556

5657
return nil, nil //nolint:nilnil
5758
}
5859

59-
func checkField(pass *analysis.Pass, field *ast.Field, qualifiedFieldName string) {
60-
prefix := fmt.Sprintf("field %s", qualifiedFieldName)
61-
62-
checkTypeExpr(pass, field.Type, field, prefix)
63-
}
64-
65-
//nolint:cyclop
66-
func checkTypeExpr(pass *analysis.Pass, typeExpr ast.Expr, node ast.Node, prefix string) {
67-
switch typ := typeExpr.(type) {
68-
case *ast.SelectorExpr:
69-
pkg, ok := typ.X.(*ast.Ident)
70-
if !ok {
71-
return
72-
}
73-
74-
if typ.X == nil || (pkg.Name != "time" && pkg.Name != "metav1") {
75-
return
76-
}
77-
78-
// Array element is not a metav1.Condition.
79-
if typ.Sel == nil || typ.Sel.Name != "Duration" {
80-
return
81-
}
82-
83-
pass.Reportf(node.Pos(), "%s should not use a Duration. Use an integer type with units in the name to avoid the need for clients to implement Go style duration parsing.", prefix)
84-
case *ast.Ident:
85-
checkIdent(pass, typ, node, prefix)
86-
case *ast.StarExpr:
87-
checkTypeExpr(pass, typ.X, node, fmt.Sprintf("%s pointer", prefix))
88-
case *ast.ArrayType:
89-
checkTypeExpr(pass, typ.Elt, node, fmt.Sprintf("%s array element", prefix))
90-
case *ast.MapType:
91-
checkTypeExpr(pass, typ.Key, node, fmt.Sprintf("%s map key", prefix))
92-
checkTypeExpr(pass, typ.Value, node, fmt.Sprintf("%s map value", prefix))
93-
}
94-
}
95-
96-
// checkIdent calls the checkFunc with the ident, when we have hit a built-in type.
97-
// If the ident is not a built in, we look at the underlying type until we hit a built-in type.
98-
func checkIdent(pass *analysis.Pass, ident *ast.Ident, node ast.Node, prefix string) {
99-
if utils.IsBasicType(pass, ident) {
100-
// We've hit a built-in type, no need to check further.
101-
return
60+
func isDurationType(pass *analysis.Pass, expr ast.Expr) bool {
61+
typ, ok := expr.(*ast.SelectorExpr)
62+
if !ok {
63+
return false
10264
}
10365

104-
tSpec, ok := utils.LookupTypeSpec(pass, ident)
66+
pkg, ok := typ.X.(*ast.Ident)
10567
if !ok {
106-
return
68+
return false
10769
}
10870

109-
// The field is using a type alias, check if the alias is an int.
110-
checkTypeSpec(pass, tSpec, node, fmt.Sprintf("%s type", prefix))
111-
}
71+
if typ.X == nil || (pkg.Name != "time" && pkg.Name != "metav1") {
72+
return false
73+
}
11274

113-
func checkTypeSpec(pass *analysis.Pass, tSpec *ast.TypeSpec, node ast.Node, prefix string) {
114-
if tSpec.Name == nil {
115-
return
75+
if typ.Sel == nil || typ.Sel.Name != "Duration" {
76+
return false
11677
}
11778

118-
typeName := tSpec.Name.Name
119-
prefix = fmt.Sprintf("%s %s", prefix, typeName)
79+
return true
80+
}
12081

121-
checkTypeExpr(pass, tSpec.Type, node, prefix)
82+
func checkDuration(pass *analysis.Pass, expr ast.Expr, node ast.Node, prefix string) {
83+
pass.Reportf(node.Pos(), "%s should not use a Duration. Use an integer type with units in the name to avoid the need for clients to implement Go style duration parsing.", prefix)
12284
}

‎pkg/analysis/nofloats/analyzer.go‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@ import (
1919
"go/ast"
2020

2121
"golang.org/x/tools/go/analysis"
22-
"golang.org/x/tools/go/analysis/passes/inspect"
23-
"golang.org/x/tools/go/ast/inspector"
22+
2423
kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors"
24+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags"
25+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector"
26+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers"
2527
"sigs.k8s.io/kube-api-linter/pkg/analysis/utils"
2628
)
2729

@@ -33,36 +35,34 @@ var Analyzer = &analysis.Analyzer{
3335
Name: name,
3436
Doc: "Float values cannot be reliably round-tripped without changing and have varying precisions and representations across languages and architectures.",
3537
Run: run,
36-
Requires: []*analysis.Analyzer{inspect.Analyzer},
38+
Requires: []*analysis.Analyzer{inspector.Analyzer},
3739
}
3840

3941
func run(pass *analysis.Pass) (any, error) {
40-
inspect, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
42+
inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector)
4143
if !ok {
4244
return nil, kalerrors.ErrCouldNotGetInspector
4345
}
4446

45-
// Filter to structs so that we can look at fields within structs.
46-
// Filter typespecs so that we can look at type aliases.
47-
nodeFilter := []ast.Node{
48-
(*ast.StructType)(nil),
49-
(*ast.TypeSpec)(nil),
50-
}
47+
typeChecker := utils.NewTypeChecker(utils.IsBasicType, checkFloat)
5148

52-
typeChecker := utils.NewTypeChecker(checkFloat)
49+
inspect.InspectFields(func(field *ast.Field, _ extractjsontags.FieldTagInfo, _ markers.Markers, _ string) {
50+
typeChecker.CheckNode(pass, field)
51+
})
5352

54-
// Preorder visits all the nodes of the AST in depth-first order. It calls
55-
// f(n) for each node n before it visits n's children.
56-
//
57-
// We use the filter defined above, ensuring we only look at struct fields and type declarations.
58-
inspect.Preorder(nodeFilter, func(n ast.Node) {
59-
typeChecker.CheckNode(pass, n)
53+
inspect.InspectTypeSpec(func(typeSpec *ast.TypeSpec, markersAccess markers.Markers) {
54+
typeChecker.CheckNode(pass, typeSpec)
6055
})
6156

6257
return nil, nil //nolint:nilnil
6358
}
6459

65-
func checkFloat(pass *analysis.Pass, ident *ast.Ident, node ast.Node, prefix string) {
60+
func checkFloat(pass *analysis.Pass, expr ast.Expr, node ast.Node, prefix string) {
61+
ident, ok := expr.(*ast.Ident)
62+
if !ok {
63+
return
64+
}
65+
6666
if ident.Name == "float32" || ident.Name == "float64" {
6767
pass.Reportf(node.Pos(), "%s should not use a float value because they cannot be reliably round-tripped.", prefix)
6868
}

0 commit comments

Comments
 (0)