diff --git a/README.md b/README.md index daadbc168..d6a671532 100644 --- a/README.md +++ b/README.md @@ -534,6 +534,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a | [`comment-spacings`](./RULES_DESCRIPTIONS.md#comment-spacings) | []string | Warns on malformed comments | no | no | | [`redundant-import-alias`](./RULES_DESCRIPTIONS.md#redundant-import-alias) | n/a | Warns on import aliases matching the imported package name | no | no | | [`import-alias-naming`](./RULES_DESCRIPTIONS.md#import-alias-naming) | string (defaults to ^[a-z][a-z0-9]{0,}$) | Conventions around the naming of import aliases. | no | no | +| [`enforce-map-style`](./RULES_DESCRIPTIONS.md#enforce-map-style) | string (defaults to "any") | Enforces consistent usage of `make(map[type]type)` or `map[type]type{}` for map initialization. Does not affect `make(map[type]type, size)` constructions. | no | no | ## Configurable rules diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index 4561e200e..16bc629a4 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -332,6 +332,22 @@ _Description_: Sometimes `gofmt` is not enough to enforce a common formatting of _Configuration_: N/A +## enforce-map-style + +_Description_: This rule enforces consistent usage of `make(map[type]type)` or `map[type]type{}` for map initialization. It does not affect `make(map[type]type, size)` constructions as well as `map[type]type{k1: v1}`. + +_Configuration_: (string) Specifies the enforced style for map initialization. The options are: +- "any": No enforcement (default). +- "make": Enforces the usage of `make(map[type]type)`. +- "literal": Enforces the usage of `map[type]type{}`. + +Example: + +```toml +[rule.enforce-map-style] + arguments = ["make"] +``` + ## error-naming _Description_: By convention, for the sake of readability, variables of type `error` must be named with the prefix `err`. diff --git a/config/config.go b/config/config.go index 438545942..b75608289 100644 --- a/config/config.go +++ b/config/config.go @@ -89,6 +89,7 @@ var allRules = append([]lint.Rule{ &rule.IfReturnRule{}, &rule.RedundantImportAlias{}, &rule.ImportAliasNamingRule{}, + &rule.EnforceMapStyleRule{}, }, defaultRules...) var allFormatters = []lint.Formatter{ diff --git a/rule/enforce-map-style.go b/rule/enforce-map-style.go new file mode 100644 index 000000000..ae27b654f --- /dev/null +++ b/rule/enforce-map-style.go @@ -0,0 +1,164 @@ +package rule + +import ( + "fmt" + "go/ast" + "sync" + + "github.com/mgechev/revive/lint" +) + +type enforceMapStyleType string + +const ( + enforceMapStyleTypeAny enforceMapStyleType = "any" + enforceMapStyleTypeMake enforceMapStyleType = "make" + enforceMapStyleTypeLiteral enforceMapStyleType = "literal" +) + +func mapStyleFromString(s string) (enforceMapStyleType, error) { + switch s { + case string(enforceMapStyleTypeAny), "": + return enforceMapStyleTypeAny, nil + case string(enforceMapStyleTypeMake): + return enforceMapStyleTypeMake, nil + case string(enforceMapStyleTypeLiteral): + return enforceMapStyleTypeLiteral, nil + default: + return enforceMapStyleTypeAny, fmt.Errorf( + "invalid map style: %s (expecting one of %v)", + s, + []enforceMapStyleType{ + enforceMapStyleTypeAny, + enforceMapStyleTypeMake, + enforceMapStyleTypeLiteral, + }, + ) + } +} + +// EnforceMapStyleRule implements a rule to enforce `make(map[type]type)` over `map[type]type{}`. +type EnforceMapStyleRule struct { + configured bool + enforceMapStyle enforceMapStyleType + sync.Mutex +} + +func (r *EnforceMapStyleRule) configure(arguments lint.Arguments) { + r.Lock() + defer r.Unlock() + + if r.configured { + return + } + r.configured = true + + if len(arguments) < 1 { + r.enforceMapStyle = enforceMapStyleTypeAny + return + } + + enforceMapStyle, ok := arguments[0].(string) + if !ok { + panic(fmt.Sprintf("Invalid argument '%v' for 'enforce-map-style' rule. Expecting string, got %T", arguments[0], arguments[0])) + } + + var err error + r.enforceMapStyle, err = mapStyleFromString(enforceMapStyle) + + if err != nil { + panic(fmt.Sprintf("Invalid argument to the enforce-map-style rule: %v", err)) + } +} + +// Apply applies the rule to given file. +func (r *EnforceMapStyleRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + r.configure(arguments) + + if r.enforceMapStyle == enforceMapStyleTypeAny { + // this linter is not configured + return nil + } + + var failures []lint.Failure + + astFile := file.AST + ast.Inspect(astFile, func(n ast.Node) bool { + switch v := n.(type) { + case *ast.CompositeLit: + if r.enforceMapStyle != enforceMapStyleTypeMake { + return true + } + + if !r.isMapType(v.Type) { + return true + } + + if len(v.Elts) > 0 { + // not an empty map + return true + } + + failures = append(failures, lint.Failure{ + Confidence: 1, + Node: v, + Category: "style", + Failure: "use make(map[type]type) instead of map[type]type{}", + }) + case *ast.CallExpr: + if r.enforceMapStyle != enforceMapStyleTypeLiteral { + // skip any function calls, even if it's make(map[type]type) + // we don't want to report it if literals are not enforced + return true + } + + ident, ok := v.Fun.(*ast.Ident) + if !ok || ident.Name != "make" { + return true + } + + if len(v.Args) != 1 { + // skip make(map[type]type, size) and invalid empty declarations + return true + } + + if !r.isMapType(v.Args[0]) { + // not a map type + return true + } + + failures = append(failures, lint.Failure{ + Confidence: 1, + Node: v.Args[0], + Category: "style", + Failure: "use map[type]type{} instead of make(map[type]type)", + }) + } + return true + }) + + return failures +} + +// Name returns the rule name. +func (r *EnforceMapStyleRule) Name() string { + return "enforce-map-style" +} + +func (r *EnforceMapStyleRule) isMapType(v ast.Expr) bool { + switch t := v.(type) { + case *ast.MapType: + return true + case *ast.Ident: + if t.Obj == nil { + return false + } + typeSpec, ok := t.Obj.Decl.(*ast.TypeSpec) + if !ok { + return false + } + return r.isMapType(typeSpec.Type) + default: + return false + } +} diff --git a/test/enforce-map-style_test.go b/test/enforce-map-style_test.go new file mode 100644 index 000000000..21013bbd8 --- /dev/null +++ b/test/enforce-map-style_test.go @@ -0,0 +1,24 @@ +package test + +import ( + "testing" + + "github.com/mgechev/revive/lint" + "github.com/mgechev/revive/rule" +) + +func TestEnforceMapStyle_any(t *testing.T) { + testRule(t, "enforce-map-style-any", &rule.EnforceMapStyleRule{}) +} + +func TestEnforceMapStyle_make(t *testing.T) { + testRule(t, "enforce-map-style-make", &rule.EnforceMapStyleRule{}, &lint.RuleConfig{ + Arguments: []interface{}{"make"}, + }) +} + +func TestEnforceMapStyle_literal(t *testing.T) { + testRule(t, "enforce-map-style-literal", &rule.EnforceMapStyleRule{}, &lint.RuleConfig{ + Arguments: []interface{}{"literal"}, + }) +} diff --git a/testdata/enforce-map-style-any.go b/testdata/enforce-map-style-any.go new file mode 100644 index 000000000..3eec12301 --- /dev/null +++ b/testdata/enforce-map-style-any.go @@ -0,0 +1,13 @@ +package fixtures + +func somefn() { + m1 := make(map[string]string) + m2 := map[string]string{} + m3 := make(map[string]string, 10) + m4 := map[string]string{"k1": "v1"} + + _ = m1 + _ = m2 + _ = m3 + _ = m4 +} diff --git a/testdata/enforce-map-style-literal.go b/testdata/enforce-map-style-literal.go new file mode 100644 index 000000000..a64d2e410 --- /dev/null +++ b/testdata/enforce-map-style-literal.go @@ -0,0 +1,51 @@ +package fixtures + +func somefn() { + m0 := make(map[string]string, 10) + m1 := make(map[string]string) // MATCH /use map[type]type{} instead of make(map[type]type)/ + m2 := map[string]string{} + m3 := map[string]string{"k1": "v1", "k2": "v2"} + + _ = m0 + _ = m1 + _ = m2 + _ = m3 +} + +type Map map[string]string + +func somefn2() { + m0 := make(Map, 10) + m1 := make(Map) // MATCH /use map[type]type{} instead of make(map[type]type)/ + m2 := Map{} + m3 := Map{"k1": "v1", "k2": "v2"} + + _ = m0 + _ = m1 + _ = m2 + _ = m3 +} + +type MapMap Map + +func somefn3() { + m0 := make(MapMap, 10) + m1 := make(MapMap) // MATCH /use map[type]type{} instead of make(map[type]type)/ + m2 := MapMap{} + m3 := MapMap{"k1": "v1", "k2": "v2"} + + _ = m0 + _ = m1 + _ = m2 + _ = m3 +} + +func somefn4() { + m1 := map[string]map[string]string{} + m1["el0"] = make(map[string]string, 10) + m1["el1"] = make(map[string]string) // MATCH /use map[type]type{} instead of make(map[type]type)/ + m1["el2"] = map[string]string{} + m1["el3"] = map[string]string{"k1": "v1", "k2": "v2"} + + _ = m1 +} diff --git a/testdata/enforce-map-style-make.go b/testdata/enforce-map-style-make.go new file mode 100644 index 000000000..e990dce11 --- /dev/null +++ b/testdata/enforce-map-style-make.go @@ -0,0 +1,51 @@ +package fixtures + +func somefn() { + m0 := make(map[string]string, 10) + m1 := make(map[string]string) + m2 := map[string]string{} // MATCH /use make(map[type]type) instead of map[type]type{}/ + m3 := map[string]string{"k1": "v1", "k2": "v2"} + + _ = m0 + _ = m1 + _ = m2 + _ = m3 +} + +type Map map[string]string + +func somefn2() { + m0 := make(Map, 10) + m1 := make(Map) + m2 := Map{} // MATCH /use make(map[type]type) instead of map[type]type{}/ + m3 := Map{"k1": "v1", "k2": "v2"} + + _ = m0 + _ = m1 + _ = m2 + _ = m3 +} + +type MapMap Map + +func somefn3() { + m0 := make(MapMap, 10) + m1 := make(MapMap) + m2 := MapMap{} // MATCH /use make(map[type]type) instead of map[type]type{}/ + m3 := MapMap{"k1": "v1", "k2": "v2"} + + _ = m0 + _ = m1 + _ = m2 + _ = m3 +} + +func somefn4() { + m1 := map[string]map[string]string{} // MATCH /use make(map[type]type) instead of map[type]type{}/ + m1["el0"] = make(map[string]string, 10) + m1["el1"] = make(map[string]string) + m1["el2"] = map[string]string{} // MATCH /use make(map[type]type) instead of map[type]type{}/ + m1["el3"] = map[string]string{"k1": "v1", "k2": "v2"} + + _ = m1 +}