Skip to content

Commit

Permalink
feat: add support for enforce-map-style rule (#895)
Browse files Browse the repository at this point in the history
Co-authored-by: chavacava <[email protected]>
  • Loading branch information
denisvmedia and chavacava authored Sep 17, 2023
1 parent 7dffe3c commit 356e667
Show file tree
Hide file tree
Showing 8 changed files with 321 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,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 @@ -333,6 +333,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
Expand Up @@ -90,6 +90,7 @@ var allRules = append([]lint.Rule{
&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
}

0 comments on commit 356e667

Please sign in to comment.