Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for enforce-map-style rule #895

Merged
merged 10 commits into from
Sep 17, 2023
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions RULES_DESCRIPTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
1 change: 1 addition & 0 deletions config/config.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package config

Check warning on line 1 in config/config.go

View workflow job for this annotation

GitHub Actions / Lint

should have a package comment

import (
"errors"
Expand Down Expand Up @@ -89,6 +89,7 @@
&rule.IfReturnRule{},
&rule.RedundantImportAlias{},
&rule.ImportAliasNamingRule{},
&rule.EnforceMapStyleRule{},
}, defaultRules...)

var allFormatters = []lint.Formatter{
Expand Down
164 changes: 164 additions & 0 deletions rule/enforce-map-style.go
Original file line number Diff line number Diff line change
@@ -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
}
}
24 changes: 24 additions & 0 deletions test/enforce-map-style_test.go
Original file line number Diff line number Diff line change
@@ -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"},
})
}
13 changes: 13 additions & 0 deletions testdata/enforce-map-style-any.go
Original file line number Diff line number Diff line change
@@ -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
}
51 changes: 51 additions & 0 deletions testdata/enforce-map-style-literal.go
Original file line number Diff line number Diff line change
@@ -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
}
51 changes: 51 additions & 0 deletions testdata/enforce-map-style-make.go
Original file line number Diff line number Diff line change
@@ -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
}
Loading