add nomaps linter

Signed-off-by: sivchari <shibuuuu5@gmail.com>

Author: sivchari

Makefile

@@ -43,6 +43,10 @@ vendor: ## Ensure the vendor directory is up to date.
 lint: ## Run golangci-lint over the codebase.
 	${GOLANGCI_LINT} run ./... --timeout 5m -v
 
+.PHONY: lint-fix
+lint-fix: $(GOLANGCI_LINT) ## Run golangci-lint over the codebase and run auto-fixers if supported by the linter
+	${GOLANGCI_LINT} run ./... --timeout 5m -v --fix
+
 .PHONY: test
 test: fmt vet unit ## Run tests.
 

README.md

@@ -239,6 +239,20 @@ Floating-point values cannot be reliably round-tripped without changing and have
 Their use should be avoided as much as possible.
 They should never be used in spec.
 
+## Nomaps
+
+The `nomaps` linter checks the usage of map types.
+
+Maps are discouraged apart from `map[string]string` which is used for labels and annotations in Kubernetes APIs since it's hard to distinguish between structs and maps in spec. Instead of plain map, lists of named subobjects are preferred.
+
+### Configuration
+
+```yaml
+lintersConfig:
+  nomaps:
+    policy: Enforce | AllowStringToStringMaps | Ignore # Determines how the linter should handle maps of simple types. Defaults to AllowStringToStringMaps.
+```
+
 ## Nophase
 
 The `nophase` linter checks that the fields in the API types don't contain a 'Phase', or any field which contains 'Phase' as a substring, e.g MachinePhase.

pkg/analysis/nomaps/analyzer.go

@@ -0,0 +1,111 @@
+package nomaps
+
+import (
+	"errors"
+	"fmt"
+	"go/ast"
+	"go/token"
+	"go/types"
+
+	"github.com/JoelSpeed/kal/pkg/analysis/helpers/extractjsontags"
+	"github.com/JoelSpeed/kal/pkg/analysis/helpers/inspector"
+	"github.com/JoelSpeed/kal/pkg/analysis/helpers/markers"
+	"github.com/JoelSpeed/kal/pkg/config"
+	"golang.org/x/tools/go/analysis"
+)
+
+const (
+	name = "nomaps"
+)
+
+var (
+	errCouldNotGetInspector = errors.New("could not get inspector")
+)
+
+type analyzer struct {
+	policy config.NoMapsPolicy
+}
+
+// newAnalyzer creates a new analyzer.
+func newAnalyzer(cfg config.NoMapsConfig) *analysis.Analyzer {
+	defaultConfig(&cfg)
+
+	a := &analyzer{
+		policy: cfg.Policy,
+	}
+
+	return &analysis.Analyzer{
+		Name:     name,
+		Doc:      "Checks for usage of map types. Maps are discouraged apart from `map[string]string` which is used for labels and annotations. Use a list of named objects instead.",
+		Run:      a.run,
+		Requires: []*analysis.Analyzer{inspector.Analyzer},
+	}
+}
+
+func (a *analyzer) run(pass *analysis.Pass) (any, error) {
+	inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector)
+	if !ok {
+		return nil, errCouldNotGetInspector
+	}
+
+	inspect.InspectFields(func(field *ast.Field, stack []ast.Node, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markers.Markers) {
+		a.checkField(pass, field)
+	})
+
+	return nil, nil //nolint:nilnil
+}
+
+func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field) {
+	stringToStringMapType := types.NewMap(types.Typ[types.String], types.Typ[types.String])
+
+	underlyingType := pass.TypesInfo.TypeOf(field.Type).Underlying()
+
+	if ptr, ok := underlyingType.(*types.Pointer); ok {
+		underlyingType = ptr.Elem().Underlying()
+	}
+
+	m, ok := underlyingType.(*types.Map)
+	if !ok {
+		return
+	}
+
+	if a.policy == config.NoMapsEnforce {
+		report(pass, field.Pos(), field.Names[0].Name)
+		return
+	}
+
+	if a.policy == config.NoMapsAllowStringToStringMaps {
+		if types.Identical(m, stringToStringMapType) {
+			return
+		}
+
+		report(pass, field.Pos(), field.Names[0].Name)
+	}
+
+	if a.policy == config.NoMapsIgnore {
+		key := m.Key().Underlying()
+		_, ok := key.(*types.Basic)
+
+		elm := m.Elem().Underlying()
+		_, ok2 := elm.(*types.Basic)
+
+		if ok && ok2 {
+			return
+		}
+
+		report(pass, field.Pos(), field.Names[0].Name)
+	}
+}
+
+func report(pass *analysis.Pass, pos token.Pos, fieldName string) {
+	pass.Report(analysis.Diagnostic{
+		Pos:     pos,
+		Message: fmt.Sprintf("%s should not use a map type, use a list type with a unique name/identifier instead", fieldName),
+	})
+}
+
+func defaultConfig(cfg *config.NoMapsConfig) {
+	if cfg.Policy == "" {
+		cfg.Policy = config.NoMapsAllowStringToStringMaps
+	}
+}

pkg/analysis/nomaps/analyzer_test.go

@@ -0,0 +1,65 @@
+package nomaps_test
+
+import (
+	"testing"
+
+	"github.com/JoelSpeed/kal/pkg/analysis/nomaps"
+	"github.com/JoelSpeed/kal/pkg/config"
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+func Test(t *testing.T) {
+	testdata := analysistest.TestData()
+
+	a, err := nomaps.Initializer().Init(config.LintersConfig{})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	analysistest.Run(t, testdata, a, "a")
+}
+
+func TestWithEnforce(t *testing.T) {
+	testdata := analysistest.TestData()
+
+	a, err := nomaps.Initializer().Init(config.LintersConfig{
+		NoMaps: config.NoMapsConfig{
+			Policy: config.NoMapsEnforce,
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	analysistest.Run(t, testdata, a, "b")
+}
+
+func TestWithAllowStringToStringMaps(t *testing.T) {
+	testdata := analysistest.TestData()
+
+	a, err := nomaps.Initializer().Init(config.LintersConfig{
+		NoMaps: config.NoMapsConfig{
+			Policy: config.NoMapsAllowStringToStringMaps,
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	analysistest.Run(t, testdata, a, "c")
+}
+
+func TestWithIgnore(t *testing.T) {
+	testdata := analysistest.TestData()
+
+	a, err := nomaps.Initializer().Init(config.LintersConfig{
+		NoMaps: config.NoMapsConfig{
+			Policy: config.NoMapsIgnore,
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	analysistest.Run(t, testdata, a, "d")
+}

pkg/analysis/nomaps/doc.go

@@ -0,0 +1,21 @@
+/*
+nomaps provides a linter to ensure that fields do not use map types.
+
+Maps are discouraged in Kubernetes APIs. It is hard to distinguish between structs and maps in JSON/YAML and as such, lists of named subobjects are preferred over plain map types.
+
+Instead of
+
+  ports:
+    www:
+      containerPort: 80
+
+use
+
+  ports:
+    - name: www
+      containerPort: 80
+
+Lists should use the `+listType=map` and `+listMapKey=name` markers, or equivalent.
+*/
+
+package nomaps

pkg/analysis/nomaps/initializer.go

@@ -0,0 +1,30 @@
+package nomaps
+
+import (
+	"github.com/JoelSpeed/kal/pkg/config"
+	"golang.org/x/tools/go/analysis"
+)
+
+// Initializer returns the AnalyzerInitializer for this
+// Analyzer so that it can be added to the registry.
+func Initializer() initializer {
+	return initializer{}
+}
+
+// intializer implements the AnalyzerInitializer interface.
+type initializer struct{}
+
+// Name returns the name of the Analyzer.
+func (initializer) Name() string {
+	return name
+}
+
+// Init returns the intialized Analyzer.
+func (initializer) Init(cfg config.LintersConfig) (*analysis.Analyzer, error) {
+	return newAnalyzer(cfg.NoMaps), nil
+}
+
+// Default determines whether this Analyzer is on by default, or not.
+func (initializer) Default() bool {
+	return true
+}

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

@@ -0,0 +1,66 @@
+package a
+
+type (
+	MapStringComponent    map[string]Component
+	PtrMapStringComponent *map[string]Component
+	MapStringInt          map[string]int
+	MapIntString          map[int]string
+)
+
+type (
+	MapStringComponentAlias           = map[string]Component
+	MapStringPtrComponentAlias        = *map[string]Component
+	MapStringIntAlias                 = map[string]int
+	DefinedMapStringComponentAlias    = MapStringComponent
+	DefinedMapStringComponentPtrAlias = *MapStringComponent
+)
+
+type (
+	MapStringGenerics[V any]             map[string]V
+	MapIntGenerics[V any]                map[int]V
+	MapComparableKeyString[K comparable] map[K]string
+	MapComparableKeyInt[K comparable]    map[K]int
+)
+
+type NoMapsTestStruct struct {
+	Primitive        int32                 `json:"primitive"`
+	Components       []Component           `json:"components"`
+	MapComponents    map[string]Component  `json:"mapComponents"` // want "MapComponents should not use a map type, use a list type with a unique name/identifier instead"
+	PtrMapComponents *map[string]Component `json:"mapComponents"` // want "MapComponents should not use a map type, use a list type with a unique name/identifier instead"
+	MapStringInt     map[string]int        `json:"mapStringInt"`  // want "MapStringInt should not use a map type, use a list type with a unique name/identifier instead"
+	Labels           map[string]string     `json:"specialCase"`
+}
+
+type NoMapsTestStructWithDefiningType struct {
+	MapStringComponent    MapStringComponent    `json:"mapStringComponent"`    // want "MapStringComponent should not use a map type, use a list type with a unique name/identifier instead"
+	PtrMapStringComponent PtrMapStringComponent `json:"ptrMapStringComponent"` // want "PtrMapStringComponent should not use a map type, use a list type with a unique name/identifier instead"
+	MapStringInt          MapStringInt          `json:"mapStringInt"`          // want "MapStringInt should not use a map type, use a list type with a unique name/identifier instead"
+	MapIntString          MapIntString          `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"
+	MapStringIntAlias                 MapStringIntAlias                 `json:"mapStringIntAlias"`                 // want "MapStringIntAlias should not use a map type, use a list type with a unique name/identifier instead"
+	DefinedMapStringComponentAlias    DefinedMapStringComponentAlias    `json:"definedMapStringComponentAlias"`    // want "DefinedMapStringComponentAlias should not use a map type, use a list type with a unique name/identifier instead"
+	DefinedMapStringComponentPtrAlias DefinedMapStringComponentPtrAlias `json:"definedMapStringComponentPtrAlias"` // want "DefinedMapStringComponentPtrAlias should not use a map type, use a list type with a unique name/identifier instead"
+}
+
+type NoMapsTestStructWithGenerics[K comparable, V any] struct {
+	MapStringGenerics      MapStringGenerics[V]      `json:"mapStringGenerics"`      // want "MapStringGenerics should not use a map type, use a list type with a unique name/identifier instead"
+	MapIntGenerics         MapIntGenerics[V]         `json:"mapIntGenerics"`         // want "MapIntGenerics should not use a map type, use a list type with a unique name/identifier instead"
+	MapComparableKeyString MapComparableKeyString[K] `json:"mapComparableKeyString"` // want "MapComparableKeyString should not use a map type, use a list type with a unique name/identifier instead"
+	MapComparableKeyInt    MapComparableKeyInt[K]    `json:"mapComparableKeyInt"`    // want "MapComparableKeyInt should not use a map type, use a list type with a unique name/identifier instead"
+}
+
+type NoMapsTestStructWithEmbedded struct {
+	NoMapsTestStruct
+	NoMapsTestStructWithDefiningType
+	NoMapsTestStructWithGenerics[string, Component]
+	NoMapsTestStructWithAlias
+}
+
+type Component struct {
+	Key   string `json:"key"`
+	Value int32  `json:"value"`
+}

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

@@ -0,0 +1,66 @@
+package b
+
+type (
+	MapStringComponent    map[string]Component
+	PtrMapStringComponent *map[string]Component
+	MapStringInt          map[string]int
+	MapIntString          map[int]string
+)
+
+type (
+	MapStringComponentAlias           = map[string]Component
+	MapStringPtrComponentAlias        = *map[string]Component
+	MapStringIntAlias                 = map[string]int
+	DefinedMapStringComponentAlias    = MapStringComponent
+	DefinedMapStringComponentPtrAlias = *MapStringComponent
+)
+
+type (
+	MapStringGenerics[V any]             map[string]V
+	MapIntGenerics[V any]                map[int]V
+	MapComparableKeyString[K comparable] map[K]string
+	MapComparableKeyInt[K comparable]    map[K]int
+)
+
+type NoMapsTestStruct struct {
+	Primitive        int32                 `json:"primitive"`
+	Components       []Component           `json:"components"`
+	MapComponents    map[string]Component  `json:"mapComponents"` // want "MapComponents should not use a map type, use a list type with a unique name/identifier instead"
+	PtrMapComponents *map[string]Component `json:"mapComponents"` // want "MapComponents should not use a map type, use a list type with a unique name/identifier instead"
+	MapStringInt     map[string]int        `json:"mapStringInt"`  // want "MapStringInt should not use a map type, use a list type with a unique name/identifier instead"
+	Labels           map[string]string     `json:"specialCase"`   // want "Labels should not use a map type, use a list type with a unique name/identifier instead"
+}
+
+type NoMapsTestStructWithDefiningType struct {
+	MapStringComponent    MapStringComponent    `json:"mapStringComponent"`    // want "MapStringComponent should not use a map type, use a list type with a unique name/identifier instead"
+	PtrMapStringComponent PtrMapStringComponent `json:"ptrMapStringComponent"` // want "PtrMapStringComponent should not use a map type, use a list type with a unique name/identifier instead"
+	MapStringInt          MapStringInt          `json:"mapStringInt"`          // want "MapStringInt should not use a map type, use a list type with a unique name/identifier instead"
+	MapIntString          MapIntString          `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"
+	MapStringIntAlias                 MapStringIntAlias                 `json:"mapStringIntAlias"`                 // want "MapStringIntAlias should not use a map type, use a list type with a unique name/identifier instead"
+	DefinedMapStringComponentAlias    DefinedMapStringComponentAlias    `json:"definedMapStringComponentAlias"`    // want "DefinedMapStringComponentAlias should not use a map type, use a list type with a unique name/identifier instead"
+	DefinedMapStringComponentPtrAlias DefinedMapStringComponentPtrAlias `json:"definedMapStringComponentPtrAlias"` // want "DefinedMapStringComponentPtrAlias should not use a map type, use a list type with a unique name/identifier instead"
+}
+
+type NoMapsTestStructWithGenerics[K comparable, V any] struct {
+	MapStringGenerics      MapStringGenerics[V]      `json:"mapStringGenerics"`      // want "MapStringGenerics should not use a map type, use a list type with a unique name/identifier instead"
+	MapIntGenerics         MapIntGenerics[V]         `json:"mapIntGenerics"`         // want "MapIntGenerics should not use a map type, use a list type with a unique name/identifier instead"
+	MapComparableKeyString MapComparableKeyString[K] `json:"mapComparableKeyString"` // want "MapComparableKeyString should not use a map type, use a list type with a unique name/identifier instead"
+	MapComparableKeyInt    MapComparableKeyInt[K]    `json:"mapComparableKeyInt"`    // want "MapComparableKeyInt should not use a map type, use a list type with a unique name/identifier instead"
+}
+
+type NoMapsTestStructWithEmbedded struct {
+	NoMapsTestStruct
+	NoMapsTestStructWithDefiningType
+	NoMapsTestStructWithGenerics[string, Component]
+	NoMapsTestStructWithAlias
+}
+
+type Component struct {
+	Key   string `json:"key"`
+	Value int32  `json:"value"`
+}