From 5248ad8ed955d22005b47a1dbfdfa720b0e77ecb Mon Sep 17 00:00:00 2001 From: chavacava Date: Tue, 17 Jul 2018 21:21:27 +0200 Subject: [PATCH] add-constant (new rule) (#39) --- README.md | 1 + config.go | 1 + fixtures/add-constant.go | 16 ++++ rule/add-constant.go | 151 ++++++++++++++++++++++++++++++++++++++ test/add-constant_test.go | 21 ++++++ 5 files changed, 190 insertions(+) create mode 100644 fixtures/add-constant.go create mode 100644 rule/add-constant.go create mode 100644 test/add-constant_test.go diff --git a/README.md b/README.md index 417772b..b64b27b 100644 --- a/README.md +++ b/README.md @@ -221,6 +221,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a | `deep-exit` | n/a | Looks for program exits in funcs other than `main()` or `init()` | no | no | | `unused-parameter` | n/a | Suggests to rename or remove unused function parameters | no | no | | `unreachable-code` | n/a | Warns on unreachable code | no | no | +| `add-constant` | map | Suggests using constant for magic numbers and string literals | no | no | ## Available Formatters diff --git a/config.go b/config.go index 25a7f72..5e34dab 100644 --- a/config.go +++ b/config.go @@ -56,6 +56,7 @@ var allRules = append([]lint.Rule{ &rule.DeepExitRule{}, &rule.UnusedParamRule{}, &rule.UnreachableCodeRule{}, + &rule.AddConstantRule{}, }, defaultRules...) var allFormatters = []lint.Formatter{ diff --git a/fixtures/add-constant.go b/fixtures/add-constant.go new file mode 100644 index 0000000..d240f40 --- /dev/null +++ b/fixtures/add-constant.go @@ -0,0 +1,16 @@ +package fixtures + +func foo(a, b, c, d int) { + a = 1.0 // ignore + b = "ignore" + c = 2 // ignore + println("lit", 12) // MATCH /avoid magic numbers like '12', create a named constant for it/ + if a == 12.50 { // MATCH /avoid magic numbers like '12.50', create a named constant for it/ + if b == "lit" { + c = "lit" // MATCH /string literal "lit" appears, at least, 3 times, create a named constant for it/ + } + for i := 0; i < 1; i++ { + println("lit") + } + } +} diff --git a/rule/add-constant.go b/rule/add-constant.go new file mode 100644 index 0000000..881bbd0 --- /dev/null +++ b/rule/add-constant.go @@ -0,0 +1,151 @@ +package rule + +import ( + "fmt" + "github.com/mgechev/revive/lint" + "go/ast" + "strconv" + "strings" +) + +const ( + defaultStrLitLimit = 2 + kindFLOAT = "FLOAT" + kindINT = "INT" + kindSTRING = "STRING" +) + +type whiteList map[string]map[string]bool + +func newWhiteList() whiteList { + return map[string]map[string]bool{kindINT: map[string]bool{}, kindFLOAT: map[string]bool{}, kindSTRING: map[string]bool{}} +} + +func (wl whiteList) add(kind string, list string) { + elems := strings.Split(list, ",") + for _, e := range elems { + wl[kind][e] = true + } +} + +// AddConstantRule lints unused params in functions. +type AddConstantRule struct{} + +// Apply applies the rule to given file. +func (r *AddConstantRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + strLitLimit := defaultStrLitLimit + var whiteList = newWhiteList() + if len(arguments) > 0 { + args, ok := arguments[0].(map[string]interface{}) + if !ok { + panic(fmt.Sprintf("Invalid argument to the add-constant rule. Expecting a k,v map, got %T", arguments[0])) + } + for k, v := range args { + kind := "" + switch k { + case "allowFloats": + kind = kindFLOAT + fallthrough + case "allowInts": + if kind == "" { + kind = kindINT + } + fallthrough + case "allowStrs": + if kind == "" { + kind = kindSTRING + } + list, ok := v.(string) + if !ok { + panic(fmt.Sprintf("Invalid argument to the add-constant rule, string expected. Got '%v' (%T)", v, v)) + } + whiteList.add(kind, list) + case "maxLitCount": + sl, ok := v.(string) + if !ok { + panic(fmt.Sprintf("Invalid argument to the add-constant rule, expecting string representation of an integer. Got '%v' (%T)", v, v)) + } + + limit, err := strconv.Atoi(sl) + if err != nil { + panic(fmt.Sprintf("Invalid argument to the add-constant rule, expecting string representation of an integer. Got '%v'", v)) + } + strLitLimit = limit + } + } + } + + var failures []lint.Failure + + onFailure := func(failure lint.Failure) { + failures = append(failures, failure) + } + + w := lintAddConstantRule{onFailure: onFailure, strLits: make(map[string]int, 0), strLitLimit: strLitLimit, whiteLst: whiteList} + + ast.Walk(w, file.AST) + + return failures +} + +// Name returns the rule name. +func (r *AddConstantRule) Name() string { + return "add-constant" +} + +type lintAddConstantRule struct { + onFailure func(lint.Failure) + strLits map[string]int + strLitLimit int + whiteLst whiteList +} + +func (w lintAddConstantRule) Visit(node ast.Node) ast.Visitor { + switch n := node.(type) { + case *ast.GenDecl: + return nil // skip declarations + case *ast.BasicLit: + switch kind := n.Kind.String(); kind { + case kindFLOAT, kindINT: + w.checkNumLit(kind, n) + case kindSTRING: + w.checkStrLit(n) + } + } + + return w + +} + +func (w lintAddConstantRule) checkStrLit(n *ast.BasicLit) { + if w.whiteLst[kindSTRING][n.Value] { + return + } + + count := w.strLits[n.Value] + if count >= 0 { + w.strLits[n.Value] = count + 1 + if w.strLits[n.Value] > w.strLitLimit { + w.onFailure(lint.Failure{ + Confidence: 1, + Node: n, + Category: "style", + Failure: fmt.Sprintf("string literal %s appears, at least, %d times, create a named constant for it", n.Value, w.strLits[n.Value]), + }) + w.strLits[n.Value] = -1 // mark it to avoid failing again on the same literal + } + } +} + +func (w lintAddConstantRule) checkNumLit(kind string, n *ast.BasicLit) { + if w.whiteLst[kind][n.Value] { + return + } + + w.onFailure(lint.Failure{ + Confidence: 1, + Node: n, + Category: "style", + Failure: fmt.Sprintf("avoid magic numbers like '%s', create a named constant for it", n.Value), + }) +} diff --git a/test/add-constant_test.go b/test/add-constant_test.go new file mode 100644 index 0000000..e4e4aec --- /dev/null +++ b/test/add-constant_test.go @@ -0,0 +1,21 @@ +package test + +import ( + "testing" + + "github.com/mgechev/revive/lint" + "github.com/mgechev/revive/rule" +) + +func TestAddConstant(t *testing.T) { + args := []interface{}{map[string]interface{}{ + "maxLitCount": "2", + "allowStrs": "\"\"", + "allowInts": "0,1,2", + "allowFloats": "0.0,1.0", + }} + + testRule(t, "add-constant", &rule.AddConstantRule{}, &lint.RuleConfig{ + Arguments: args, + }) +}