add-constant (new rule) (#39)

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
 

config.go

@@ -56,6 +56,7 @@ var allRules = append([]lint.Rule{
 	&rule.DeepExitRule{},
 	&rule.UnusedParamRule{},
 	&rule.UnreachableCodeRule{},
+	&rule.AddConstantRule{},
 }, defaultRules...)
 
 var allFormatters = []lint.Formatter{

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")
+		}
+	}
+}

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),
+	})
+}

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,
+	})
+}