From 50091409c06145e9618287cf3de9e696e94c2dd4 Mon Sep 17 00:00:00 2001 From: Denis Voytyuk <5462781+denisvmedia@users.noreply.github.com> Date: Sat, 23 Sep 2023 09:05:52 +0200 Subject: [PATCH] feat: add support for enforce-slice-style rule (#900) --- README.md | 1 + RULES_DESCRIPTIONS.md | 18 +++ config/config.go | 5 +- rule/enforce-slice-style.go | 193 ++++++++++++++++++++++++ test/enforce-slice-style_test.go | 24 +++ testdata/enforce-slice-style-any.go | 69 +++++++++ testdata/enforce-slice-style-literal.go | 69 +++++++++ testdata/enforce-slice-style-make.go | 69 +++++++++ 8 files changed, 446 insertions(+), 2 deletions(-) create mode 100644 rule/enforce-slice-style.go create mode 100644 test/enforce-slice-style_test.go create mode 100644 testdata/enforce-slice-style-any.go create mode 100644 testdata/enforce-slice-style-literal.go create mode 100644 testdata/enforce-slice-style-make.go diff --git a/README.md b/README.md index cdcd202b7..970a00310 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index 72288b82e..9d8535e58 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -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`. diff --git a/config/config.go b/config/config.go index abd554a9f..1ebd9c4ac 100644 --- a/config/config.go +++ b/config/config.go @@ -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" ) @@ -91,6 +91,7 @@ var allRules = append([]lint.Rule{ &rule.RedundantImportAlias{}, &rule.ImportAliasNamingRule{}, &rule.EnforceMapStyleRule{}, + &rule.EnforceSliceStyleRule{}, }, defaultRules...) var allFormatters = []lint.Formatter{ diff --git a/rule/enforce-slice-style.go b/rule/enforce-slice-style.go new file mode 100644 index 000000000..b3611d51a --- /dev/null +++ b/rule/enforce-slice-style.go @@ -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 + } +} diff --git a/test/enforce-slice-style_test.go b/test/enforce-slice-style_test.go new file mode 100644 index 000000000..ae8e7957e --- /dev/null +++ b/test/enforce-slice-style_test.go @@ -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"}, + }) +} diff --git a/testdata/enforce-slice-style-any.go b/testdata/enforce-slice-style-any.go new file mode 100644 index 000000000..77bd4f36e --- /dev/null +++ b/testdata/enforce-slice-style-any.go @@ -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 +} diff --git a/testdata/enforce-slice-style-literal.go b/testdata/enforce-slice-style-literal.go new file mode 100644 index 000000000..636cd4ff1 --- /dev/null +++ b/testdata/enforce-slice-style-literal.go @@ -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() { + m0 := make(SliceSlice, 10) + m1 := make(SliceSlice, 0, 10) + 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)/ + 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 +} diff --git a/testdata/enforce-slice-style-make.go b/testdata/enforce-slice-style-make.go new file mode 100644 index 000000000..5bee16a69 --- /dev/null +++ b/testdata/enforce-slice-style-make.go @@ -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{} // MATCH /use make([]type) instead of []type{} (or declare nil slice)/ + 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{} // MATCH /use make([]type) instead of []type{} (or declare nil 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{} // MATCH /use make([]type) instead of []type{} (or declare nil slice)/ + m5 := SliceSlice{"v1", "v2"} + + _ = m0 + _ = m1 + _ = m2 + _ = m3 + _ = m4 + _ = m5 +} + +func somefn4() { + m1 := [][]string{} // MATCH /use make([]type) instead of []type{} (or declare nil slice)/ + 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{} // MATCH /use make([]type) instead of []type{} (or declare nil slice)/ + m1["el5"] = []string{"v1", "v2"} + + _ = m1 +}