Skip to content

Commit

Permalink
feat: add support for enforce-slice-style rule
Browse files Browse the repository at this point in the history
  • Loading branch information
denisvmedia committed Sep 22, 2023
1 parent 5c69df7 commit d961ed3
Show file tree
Hide file tree
Showing 8 changed files with 391 additions and 2 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
| [`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 |
| [`enforce-slice-style`](./RULES_DESCRIPTIONS.md#enforce-slice-style) | string (defaults to "any") | Enforces consistent usage of `make([]type, 0)` or `[]type{}` for slice initialization. Does not affect `make(map[type]type, non_zero_len, or_non_zero_cap)` constructions. | no | no |


## Configurable rules
Expand Down
18 changes: 18 additions & 0 deletions RULES_DESCRIPTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,24 @@ Example:
arguments = ["make"]
```

## enforce-slice-style

_Description_: This rule enforces consistent usage of `make([]type, 0)` or `[]type{}` for slice initialization.
It does not affect `make([]type, non_zero_len, or_non_zero_cap)` constructions as well as `[]type{v1}`.
Nil slices are always permitted.

_Configuration_: (string) Specifies the enforced style for slice initialization. The options are:
- "any": No enforcement (default).
- "make": Enforces the usage of `make([]type, 0)`.
- "literal": Enforces the usage of `[]type{}`.

Example:

```toml
[rule.enforce-slice-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
5 changes: 3 additions & 2 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import (
"fmt"
"os"

"github.com/mgechev/revive/formatter"

"github.com/BurntSushi/toml"

"github.com/mgechev/revive/formatter"
"github.com/mgechev/revive/lint"
"github.com/mgechev/revive/rule"
)
Expand Down Expand Up @@ -91,6 +91,7 @@ var allRules = append([]lint.Rule{
&rule.RedundantImportAlias{},
&rule.ImportAliasNamingRule{},
&rule.EnforceMapStyleRule{},
&rule.EnforceSliceStyleRule{},
}, defaultRules...)

var allFormatters = []lint.Formatter{
Expand Down
193 changes: 193 additions & 0 deletions rule/enforce-slice-style.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
package rule

import (
"fmt"
"go/ast"
"sync"

"github.com/mgechev/revive/lint"
)

type enforceSliceStyleType string

const (
enforceSliceStyleTypeAny enforceSliceStyleType = "any"
enforceSliceStyleTypeMake enforceSliceStyleType = "make"
enforceSliceStyleTypeLiteral enforceSliceStyleType = "literal"
)

func sliceStyleFromString(s string) (enforceSliceStyleType, error) {
switch s {
case string(enforceSliceStyleTypeAny), "":
return enforceSliceStyleTypeAny, nil
case string(enforceSliceStyleTypeMake):
return enforceSliceStyleTypeMake, nil
case string(enforceSliceStyleTypeLiteral):
return enforceSliceStyleTypeLiteral, nil
default:
return enforceSliceStyleTypeAny, fmt.Errorf(
"invalid slice style: %s (expecting one of %v)",
s,
[]enforceSliceStyleType{
enforceSliceStyleTypeAny,
enforceSliceStyleTypeMake,
enforceSliceStyleTypeLiteral,
},
)
}
}

// EnforceSliceStyleRule implements a rule to enforce `make([]type)` over `[]type{}`.
type EnforceSliceStyleRule struct {
configured bool
enforceSliceStyle enforceSliceStyleType
sync.Mutex
}

func (r *EnforceSliceStyleRule) configure(arguments lint.Arguments) {
r.Lock()
defer r.Unlock()

if r.configured {
return
}
r.configured = true

if len(arguments) < 1 {
r.enforceSliceStyle = enforceSliceStyleTypeAny
return
}

enforceSliceStyle, ok := arguments[0].(string)
if !ok {
panic(fmt.Sprintf("Invalid argument '%v' for 'enforce-slice-style' rule. Expecting string, got %T", arguments[0], arguments[0]))
}

var err error
r.enforceSliceStyle, err = sliceStyleFromString(enforceSliceStyle)

if err != nil {
panic(fmt.Sprintf("Invalid argument to the enforce-slice-style rule: %v", err))
}
}

// Apply applies the rule to given file.
func (r *EnforceSliceStyleRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
r.configure(arguments)

if r.enforceSliceStyle == enforceSliceStyleTypeAny {
// 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.enforceSliceStyle != enforceSliceStyleTypeMake {
return true
}

if !r.isSliceType(v.Type) {
return true
}

if len(v.Elts) > 0 {
// not an empty slice
return true
}

failures = append(failures, lint.Failure{
Confidence: 1,
Node: v,
Category: "style",
Failure: "use make([]type) instead of []type{} (or declare nil slice)",
})
case *ast.CallExpr:
if r.enforceSliceStyle != enforceSliceStyleTypeLiteral {
// skip any function calls, even if it's make([]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) < 2 {
// skip invalid make declarations
return true
}

if !r.isSliceType(v.Args[0]) {
// not a slice type
return true
}

arg, ok := v.Args[1].(*ast.BasicLit)
if !ok {
// skip invalid make declarations
return true
}

if arg.Value != "0" {
// skip slice with non-zero size
return true
}

if len(v.Args) > 2 {
arg, ok := v.Args[2].(*ast.BasicLit)
if !ok {
// skip invalid make declarations
return true
}

if arg.Value != "0" {
// skip non-zero capacity slice
return true
}
}

failures = append(failures, lint.Failure{
Confidence: 1,
Node: v.Args[0],
Category: "style",
Failure: "use []type{} instead of make([]type, 0) (or declare nil slice)",
})
}
return true
})

return failures
}

// Name returns the rule name.
func (r *EnforceSliceStyleRule) Name() string {
return "enforce-slice-style"
}

func (r *EnforceSliceStyleRule) isSliceType(v ast.Expr) bool {
switch t := v.(type) {
case *ast.ArrayType:
if t.Len != nil {
// array
return false
}
// slice
return true
case *ast.Ident:
if t.Obj == nil {
return false
}
typeSpec, ok := t.Obj.Decl.(*ast.TypeSpec)
if !ok {
return false
}
return r.isSliceType(typeSpec.Type)
default:
return false
}
}
24 changes: 24 additions & 0 deletions test/enforce-slice-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 TestEnforceSliceStyle_any(t *testing.T) {
testRule(t, "enforce-slice-style-any", &rule.EnforceSliceStyleRule{})
}

func TestEnforceSliceStyle_make(t *testing.T) {
testRule(t, "enforce-slice-style-make", &rule.EnforceSliceStyleRule{}, &lint.RuleConfig{
Arguments: []interface{}{"make"},
})
}

func TestEnforceSliceStyle_literal(t *testing.T) {
testRule(t, "enforce-slice-style-literal", &rule.EnforceSliceStyleRule{}, &lint.RuleConfig{
Arguments: []interface{}{"literal"},
})
}
21 changes: 21 additions & 0 deletions testdata/enforce-slice-style-any.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package fixtures

func somefn() {
m1 := make([]string)
m2 := []string{}
m3 := make([]string, 10)
m4 := make([]string, 0, 10)
m5 := []string{"v1"}
m6 := [...]string{}
m7 := [...]string{"v1"}
m8 := [1]string{"v1"}

_ = m1
_ = m2
_ = m3
_ = m4
_ = m5
_ = m6
_ = m7
_ = m8
}
69 changes: 69 additions & 0 deletions testdata/enforce-slice-style-literal.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package fixtures

func somefn() {
m0 := make([]string, 10)
m1 := make([]string, 0, 10)
m2 := make([]string, 0) // MATCH /use []type{} instead of make([]type, 0) (or declare nil slice)/
m3 := make([]string, 0, 0) // MATCH /use []type{} instead of make([]type, 0) (or declare nil slice)/
m4 := []string{}
m5 := []string{"v1", "v2"}
m6 := [8]string{}
m7 := [...]string{}

_ = m0
_ = m1
_ = m2
_ = m3
_ = m4
_ = m5
_ = m6
_ = m7
}

type Slice []string

func somefn2() {
m0 := make(Slice, 10)
m1 := make(Slice, 0, 10)
m2 := make(Slice, 0) // MATCH /use []type{} instead of make([]type, 0) (or declare nil slice)/
m3 := make(Slice, 0, 0) // MATCH /use []type{} instead of make([]type, 0) (or declare nil slice)/
m4 := Slice{}
m5 := Slice{"v1", "v2"}

_ = m0
_ = m1
_ = m2
_ = m3
_ = m4
_ = m5
}

type SliceSlice Slice

func somefn3() {
m2 := make(SliceSlice, 0) // MATCH /use []type{} instead of make([]type, 0) (or declare nil slice)/
m3 := make(SliceSlice, 0, 0) // MATCH /use []type{} instead of make([]type, 0) (or declare nil slice)/
m0 := make(SliceSlice, 10)
m1 := make(SliceSlice, 0, 10)
m4 := SliceSlice{}
m5 := SliceSlice{"v1", "v2"}

_ = m0
_ = m1
_ = m2
_ = m3
_ = m4
_ = m5
}

func somefn4() {
m1 := [][]string{}
m1["el0"] = make([]string, 10)
m1["el1"] = make([]string, 0, 10)
m1["el2"] = make([]string, 0) // MATCH /use []type{} instead of make([]type, 0) (or declare nil slice)/
m1["el3"] = make([]string, 0, 0) // MATCH /use []type{} instead of make([]type, 0) (or declare nil slice)/
m1["el4"] = []string{}
m1["el5"] = []string{"v1", "v2"}

_ = m1
}
Loading

0 comments on commit d961ed3

Please sign in to comment.