-
Notifications
You must be signed in to change notification settings - Fork 284
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat: add support for enforce-slice-style rule (#900)
- Loading branch information
1 parent
5c69df7
commit 5009140
Showing
8 changed files
with
446 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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"}, | ||
}) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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) | ||
m3 := make([]string, 0, 0) | ||
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) | ||
m3 := make(Slice, 0, 0) | ||
m4 := Slice{} | ||
m5 := Slice{"v1", "v2"} | ||
|
||
_ = m0 | ||
_ = m1 | ||
_ = m2 | ||
_ = m3 | ||
_ = m4 | ||
_ = m5 | ||
} | ||
|
||
type SliceSlice Slice | ||
|
||
func somefn3() { | ||
m0 := make(SliceSlice, 10) | ||
m1 := make(SliceSlice, 0, 10) | ||
m2 := make(SliceSlice, 0) | ||
m3 := make(SliceSlice, 0, 0) | ||
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) | ||
m1["el3"] = make([]string, 0, 0) | ||
m1["el4"] = []string{} | ||
m1["el5"] = []string{"v1", "v2"} | ||
|
||
_ = m1 | ||
} |
Oops, something went wrong.