Skip to content

Commit c7b56f8

Browse files
JoelSpeedJoelSpeed
committed
Switch NoMaps to using type checker
1 parent af2583e commit c7b56f8

File tree

8 files changed

+176
-151
lines changed

8 files changed

+176
-151
lines changed

‎pkg/analysis/nomaps/analyzer.go‎

Lines changed: 31 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,13 @@ import (
1919
"fmt"
2020
"go/ast"
2121
"go/token"
22-
"go/types"
2322

2423
"golang.org/x/tools/go/analysis"
2524
kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors"
2625
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags"
2726
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector"
2827
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers"
28+
"sigs.k8s.io/kube-api-linter/pkg/analysis/utils"
2929
)
3030

3131
const (
@@ -62,52 +62,51 @@ func (a *analyzer) run(pass *analysis.Pass) (any, error) {
6262
return nil, kalerrors.ErrCouldNotGetInspector
6363
}
6464

65-
inspect.InspectFields(func(field *ast.Field, _ extractjsontags.FieldTagInfo, _ markers.Markers, qualifiedFieldName string) {
66-
a.checkField(pass, field, qualifiedFieldName)
65+
typeChecker := utils.NewTypeChecker(isMap, a.checkMap)
66+
67+
inspect.InspectFields(func(field *ast.Field, _ extractjsontags.FieldTagInfo, _ markers.Markers, _ string) {
68+
typeChecker.CheckNode(pass, field)
69+
})
70+
71+
inspect.InspectTypeSpec(func(typeSpec *ast.TypeSpec, _ markers.Markers) {
72+
typeChecker.CheckNode(pass, typeSpec)
6773
})
6874

6975
return nil, nil //nolint:nilnil
7076
}
7177

72-
func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field, qualifiedFieldName string) {
73-
underlyingType := pass.TypesInfo.TypeOf(field.Type).Underlying()
78+
func isMap(pass *analysis.Pass, expr ast.Expr) bool {
79+
_, ok := expr.(*ast.MapType)
7480

75-
if ptr, ok := underlyingType.(*types.Pointer); ok {
76-
underlyingType = ptr.Elem().Underlying()
77-
}
81+
return ok
82+
}
7883

79-
m, ok := underlyingType.(*types.Map)
84+
func (a *analyzer) checkMap(pass *analysis.Pass, expr ast.Expr, node ast.Node, prefix string) {
85+
mapType, ok := expr.(*ast.MapType)
8086
if !ok {
8187
return
8288
}
8389

84-
if a.policy == NoMapsEnforce {
85-
report(pass, field.Pos(), qualifiedFieldName)
86-
return
87-
}
88-
89-
if a.policy == NoMapsAllowStringToStringMaps {
90-
if types.AssignableTo(m.Elem().Underlying(), types.Typ[types.String]) &&
91-
types.AssignableTo(m.Key().Underlying(), types.Typ[types.String]) {
92-
return
90+
switch a.policy {
91+
case NoMapsEnforce:
92+
report(pass, node.Pos(), prefix)
93+
case NoMapsAllowStringToStringMaps:
94+
if !isStringToStringMap(pass, mapType) {
95+
report(pass, node.Pos(), prefix)
96+
}
97+
case NoMapsIgnore:
98+
if !isBasicMap(pass, mapType) {
99+
report(pass, node.Pos(), prefix)
93100
}
94-
95-
report(pass, field.Pos(), qualifiedFieldName)
96101
}
102+
}
97103

98-
if a.policy == NoMapsIgnore {
99-
key := m.Key().Underlying()
100-
_, ok := key.(*types.Basic)
101-
102-
elm := m.Elem().Underlying()
103-
_, ok2 := elm.(*types.Basic)
104-
105-
if ok && ok2 {
106-
return
107-
}
104+
func isStringToStringMap(pass *analysis.Pass, mapType *ast.MapType) bool {
105+
return utils.IsStringType(pass, mapType.Key) && utils.IsStringType(pass, mapType.Value)
106+
}
108107

109-
report(pass, field.Pos(), qualifiedFieldName)
110-
}
108+
func isBasicMap(pass *analysis.Pass, mapType *ast.MapType) bool {
109+
return utils.IsBasicType(pass, mapType.Key) && utils.IsBasicType(pass, mapType.Value)
111110
}
112111

113112
func report(pass *analysis.Pass, pos token.Pos, fieldName string) {

‎pkg/analysis/nomaps/testdata/src/a/a.go‎

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,63 +1,63 @@
11
package a
22

33
type (
4-
MapStringComponent map[string]Component
5-
PtrMapStringComponent *map[string]Component
6-
MapStringInt map[string]int
7-
MapIntString map[int]string
4+
MapStringComponent map[string]Component // want "type MapStringComponent should not use a map type, use a list type with a unique name/identifier instead"
5+
PtrMapStringComponent *map[string]Component // want "type PtrMapStringComponent pointer should not use a map type, use a list type with a unique name/identifier instead"
6+
MapStringInt map[string]int // want "type MapStringInt should not use a map type, use a list type with a unique name/identifier instead"
7+
MapIntString map[int]string // want "type MapIntString should not use a map type, use a list type with a unique name/identifier instead"
88
)
99

1010
type (
11-
MapStringComponentAlias = map[string]Component
12-
MapStringPtrComponentAlias = *map[string]Component
13-
MapStringIntAlias = map[string]int
14-
DefinedMapStringComponentAlias = MapStringComponent
15-
DefinedMapStringComponentPtrAlias = *MapStringComponent
11+
MapStringComponentAlias = map[string]Component // want "type MapStringComponentAlias should not use a map type, use a list type with a unique name/identifier instead"
12+
MapStringPtrComponentAlias = *map[string]Component // want "type MapStringPtrComponentAlias pointer should not use a map type, use a list type with a unique name/identifier instead"
13+
MapStringIntAlias = map[string]int // want "type MapStringIntAlias should not use a map type, use a list type with a unique name/identifier instead"
14+
DefinedMapStringComponentAlias = MapStringComponent // want "type DefinedMapStringComponentAlias type MapStringComponent should not use a map type, use a list type with a unique name/identifier instead"
15+
DefinedMapStringComponentPtrAlias = *MapStringComponent // want "type DefinedMapStringComponentPtrAlias pointer type MapStringComponent should not use a map type, use a list type with a unique name/identifier instead"
1616
)
1717

1818
type (
19-
MapStringGenerics[V any] map[string]V
20-
MapIntGenerics[V any] map[int]V
21-
MapComparableKeyString[K comparable] map[K]string
22-
MapComparableKeyInt[K comparable] map[K]int
19+
MapStringGenerics[V any] map[string]V // want "type MapStringGenerics should not use a map type, use a list type with a unique name/identifier instead"
20+
MapIntGenerics[V any] map[int]V // want "type MapIntGenerics should not use a map type, use a list type with a unique name/identifier instead"
21+
MapComparableKeyString[K comparable] map[K]string // want "type MapComparableKeyString should not use a map type, use a list type with a unique name/identifier instead"
22+
MapComparableKeyInt[K comparable] map[K]int // want "type MapComparableKeyInt should not use a map type, use a list type with a unique name/identifier instead"
2323
)
2424

2525
type NoMapsTestStruct struct {
2626
Primitive int32 `json:"primitive"`
2727
Components []Component `json:"components"`
2828
MapComponents map[string]Component `json:"mapComponents"` // want "NoMapsTestStruct.MapComponents should not use a map type, use a list type with a unique name/identifier instead"
29-
PtrMapComponents *map[string]Component `json:"ptrMapComponents"` // want "NoMapsTestStruct.PtrMapComponents should not use a map type, use a list type with a unique name/identifier instead"
29+
PtrMapComponents *map[string]Component `json:"ptrMapComponents"` // want "NoMapsTestStruct.PtrMapComponents pointer should not use a map type, use a list type with a unique name/identifier instead"
3030
MapStringInt map[string]int `json:"mapStringInt"` // want "NoMapsTestStruct.MapStringInt should not use a map type, use a list type with a unique name/identifier instead"
3131
Labels map[string]string `json:"specialCase"`
3232
}
3333

3434
type NoMapsTestStructWithDefiningType struct {
35-
MapStringComponent MapStringComponent `json:"mapStringComponent"` // want "NoMapsTestStructWithDefiningType.MapStringComponent should not use a map type, use a list type with a unique name/identifier instead"
36-
PtrMapStringComponent PtrMapStringComponent `json:"ptrMapStringComponent"` // want "NoMapsTestStructWithDefiningType.PtrMapStringComponent should not use a map type, use a list type with a unique name/identifier instead"
37-
MapStringInt MapStringInt `json:"mapStringInt"` // want "NoMapsTestStructWithDefiningType.MapStringInt should not use a map type, use a list type with a unique name/identifier instead"
38-
MapIntString MapIntString `json:"mapIntString"` // want "NoMapsTestStructWithDefiningType.MapIntString should not use a map type, use a list type with a unique name/identifier instead"
35+
MapStringComponent MapStringComponent `json:"mapStringComponent"` // want "NoMapsTestStructWithDefiningType.MapStringComponent type MapStringComponent should not use a map type, use a list type with a unique name/identifier instead"
36+
PtrMapStringComponent PtrMapStringComponent `json:"ptrMapStringComponent"` // want "NoMapsTestStructWithDefiningType.PtrMapStringComponent type PtrMapStringComponent pointer should not use a map type, use a list type with a unique name/identifier instead"
37+
MapStringInt MapStringInt `json:"mapStringInt"` // want "NoMapsTestStructWithDefiningType.MapStringInt type MapStringInt should not use a map type, use a list type with a unique name/identifier instead"
38+
MapIntString MapIntString `json:"mapIntString"` // want "NoMapsTestStructWithDefiningType.MapIntString type MapIntString should not use a map type, use a list type with a unique name/identifier instead"
3939
}
4040

4141
type NoMapsTestStructWithDefiningTypeAcrossFiles struct {
42-
MapStringComponent MapStringComponentB `json:"mapStringComponent"` // want "NoMapsTestStructWithDefiningTypeAcrossFiles.MapStringComponent should not use a map type, use a list type with a unique name/identifier instead"
43-
PtrMapStringComponent PtrMapStringComponentB `json:"ptrMapStringComponent"` // want "NoMapsTestStructWithDefiningTypeAcrossFiles.PtrMapStringComponent should not use a map type, use a list type with a unique name/identifier instead"
44-
MapStringInt MapStringIntB `json:"mapStringInt"` // want "NoMapsTestStructWithDefiningTypeAcrossFiles.MapStringInt should not use a map type, use a list type with a unique name/identifier instead"
45-
MapIntString MapIntStringB `json:"mapIntString"` // want "NoMapsTestStructWithDefiningTypeAcrossFiles.MapIntString should not use a map type, use a list type with a unique name/identifier instead"
42+
MapStringComponent MapStringComponentB `json:"mapStringComponent"` // want "NoMapsTestStructWithDefiningTypeAcrossFiles.MapStringComponent type MapStringComponentB should not use a map type, use a list type with a unique name/identifier instead"
43+
PtrMapStringComponent PtrMapStringComponentB `json:"ptrMapStringComponent"` // want "NoMapsTestStructWithDefiningTypeAcrossFiles.PtrMapStringComponent type PtrMapStringComponentB pointer should not use a map type, use a list type with a unique name/identifier instead"
44+
MapStringInt MapStringIntB `json:"mapStringInt"` // want "NoMapsTestStructWithDefiningTypeAcrossFiles.MapStringInt type MapStringIntB should not use a map type, use a list type with a unique name/identifier instead"
45+
MapIntString MapIntStringB `json:"mapIntString"` // want "NoMapsTestStructWithDefiningTypeAcrossFiles.MapIntString type MapIntStringB should not use a map type, use a list type with a unique name/identifier instead"
4646
}
4747

4848
type NoMapsTestStructWithAlias struct {
49-
MapStringComponentAlias MapStringComponentAlias `json:"mapStringComponentAlias"` // want "NoMapsTestStructWithAlias.MapStringComponentAlias should not use a map type, use a list type with a unique name/identifier instead"
50-
MapStringPtrComponentAlias MapStringPtrComponentAlias `json:"mapStringPtrComponentAlias"` // want "NoMapsTestStructWithAlias.MapStringPtrComponentAlias should not use a map type, use a list type with a unique name/identifier instead"
51-
MapStringIntAlias MapStringIntAlias `json:"mapStringIntAlias"` // want "NoMapsTestStructWithAlias.MapStringIntAlias should not use a map type, use a list type with a unique name/identifier instead"
52-
DefinedMapStringComponentAlias DefinedMapStringComponentAlias `json:"definedMapStringComponentAlias"` // want "NoMapsTestStructWithAlias.DefinedMapStringComponentAlias should not use a map type, use a list type with a unique name/identifier instead"
53-
DefinedMapStringComponentPtrAlias DefinedMapStringComponentPtrAlias `json:"definedMapStringComponentPtrAlias"` // want "NoMapsTestStructWithAlias.DefinedMapStringComponentPtrAlias should not use a map type, use a list type with a unique name/identifier instead"
49+
MapStringComponentAlias MapStringComponentAlias `json:"mapStringComponentAlias"` // want "NoMapsTestStructWithAlias.MapStringComponentAlias type MapStringComponentAlias should not use a map type, use a list type with a unique name/identifier instead"
50+
MapStringPtrComponentAlias MapStringPtrComponentAlias `json:"mapStringPtrComponentAlias"` // want "NoMapsTestStructWithAlias.MapStringPtrComponentAlias type MapStringPtrComponentAlias pointer should not use a map type, use a list type with a unique name/identifier instead"
51+
MapStringIntAlias MapStringIntAlias `json:"mapStringIntAlias"` // want "NoMapsTestStructWithAlias.MapStringIntAlias type MapStringIntAlias should not use a map type, use a list type with a unique name/identifier instead"
52+
DefinedMapStringComponentAlias DefinedMapStringComponentAlias `json:"definedMapStringComponentAlias"` // want "NoMapsTestStructWithAlias.DefinedMapStringComponentAlias type DefinedMapStringComponentAlias type MapStringComponent should not use a map type, use a list type with a unique name/identifier instead"
53+
DefinedMapStringComponentPtrAlias DefinedMapStringComponentPtrAlias `json:"definedMapStringComponentPtrAlias"` // want "NoMapsTestStructWithAlias.DefinedMapStringComponentPtrAlias type DefinedMapStringComponentPtrAlias pointer type MapStringComponent should not use a map type, use a list type with a unique name/identifier instead"
5454
}
5555

5656
type NoMapsTestStructWithGenerics[K comparable, V any] struct {
57-
MapStringGenerics MapStringGenerics[V] `json:"mapStringGenerics"` // want "NoMapsTestStructWithGenerics.MapStringGenerics should not use a map type, use a list type with a unique name/identifier instead"
58-
MapIntGenerics MapIntGenerics[V] `json:"mapIntGenerics"` // want "NoMapsTestStructWithGenerics.MapIntGenerics should not use a map type, use a list type with a unique name/identifier instead"
59-
MapComparableKeyString MapComparableKeyString[K] `json:"mapComparableKeyString"` // want "NoMapsTestStructWithGenerics.MapComparableKeyString should not use a map type, use a list type with a unique name/identifier instead"
60-
MapComparableKeyInt MapComparableKeyInt[K] `json:"mapComparableKeyInt"` // want "NoMapsTestStructWithGenerics.MapComparableKeyInt should not use a map type, use a list type with a unique name/identifier instead"
57+
MapStringGenerics MapStringGenerics[V] `json:"mapStringGenerics"` // want "NoMapsTestStructWithGenerics.MapStringGenerics type MapStringGenerics should not use a map type, use a list type with a unique name/identifier instead"
58+
MapIntGenerics MapIntGenerics[V] `json:"mapIntGenerics"` // want "NoMapsTestStructWithGenerics.MapIntGenerics type MapIntGenerics should not use a map type, use a list type with a unique name/identifier instead"
59+
MapComparableKeyString MapComparableKeyString[K] `json:"mapComparableKeyString"` // want "NoMapsTestStructWithGenerics.MapComparableKeyString type MapComparableKeyString should not use a map type, use a list type with a unique name/identifier instead"
60+
MapComparableKeyInt MapComparableKeyInt[K] `json:"mapComparableKeyInt"` // want "NoMapsTestStructWithGenerics.MapComparableKeyInt type MapComparableKeyInt should not use a map type, use a list type with a unique name/identifier instead"
6161
}
6262

6363
type NoMapsTestStructWithEmbedded struct {

‎pkg/analysis/nomaps/testdata/src/a/b.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
package a
22

33
type (
4-
MapStringComponentB map[string]Component
5-
PtrMapStringComponentB *map[string]Component
6-
MapStringIntB map[string]int
7-
MapIntStringB map[int]string
4+
MapStringComponentB map[string]Component // want "type MapStringComponentB should not use a map type, use a list type with a unique name/identifier instead"
5+
PtrMapStringComponentB *map[string]Component // want "type PtrMapStringComponentB pointer should not use a map type, use a list type with a unique name/identifier instead"
6+
MapStringIntB map[string]int // want "type MapStringIntB should not use a map type, use a list type with a unique name/identifier instead"
7+
MapIntStringB map[int]string // want "type MapIntStringB should not use a map type, use a list type with a unique name/identifier instead"
88
)

0 commit comments

Comments
 (0)