support goconst

Gopkg.lock

@@ -74,6 +74,12 @@
   ]
   revision = "f557f368b7f3d00a54ed44eb57b9eca59e85cee1"
 
+[[projects]]
+  name = "github.com/golangci/goconst"
+  packages = ["."]
+  revision = "b67b9035d29a7561902d87eb3757dd490b31c1b1"
+  branch = "master"
+
 [[projects]]
   branch = "master"
   name = "github.com/golangci/gocyclo"
@@ -300,6 +306,6 @@
 [solve-meta]
   analyzer-name = "dep"
   analyzer-version = 1
-  inputs-digest = "3c773b17692c1bc6ce7d565b959908aa95160e5d4f2486721deaff48608c864e"
+  inputs-digest = "33466d605f73dcfcecf89a0d3a46aaa719366433cdd6080d00f8476fe77e747f"
   solver-name = "gps-cdcl"
   solver-version = 1

internal/commands/run.go

@@ -52,7 +52,7 @@ func (e *Executor) initRun() {
 	runCmd.Flags().BoolVar(&rc.Gofmt.Simplify, "gofmt.simplify", true, "Gofmt: simplify code")
 
 	runCmd.Flags().IntVar(&rc.Gocyclo.MinComplexity, "gocyclo.min-complexity",
-		20, "Minimal complexity of function to report it")
+		50, "Minimal complexity of function to report it")
 
 	runCmd.Flags().BoolVar(&rc.Structcheck.CheckExportedFields, "structcheck.exported-fields", false, "Structcheck: report about unused exported struct fields")
 	runCmd.Flags().BoolVar(&rc.Varcheck.CheckExportedFields, "varcheck.exported-fields", false, "Varcheck: report about unused exported variables")
@@ -62,8 +62,14 @@ func (e *Executor) initRun() {
 	runCmd.Flags().BoolVar(&rc.Megacheck.EnableStaticcheck, "megacheck.staticcheck", true, "Megacheck: run Staticcheck sub-linter: staticcheck is go vet on steroids, applying a ton of static analysis checks")
 	runCmd.Flags().BoolVar(&rc.Megacheck.EnableGosimple, "megacheck.gosimple", true, "Megacheck: run Gosimple sub-linter: gosimple is a linter for Go source code that specialises on simplifying code")
 	runCmd.Flags().BoolVar(&rc.Megacheck.EnableUnused, "megacheck.unused", true, "Megacheck: run Unused sub-linter: unused checks Go code for unused constants, variables, functions and types")
+
 	runCmd.Flags().IntVar(&rc.Dupl.Threshold, "dupl.threshold",
-		20, "Minimal threshold to detect copy-paste")
+		20, "Dupl: Minimal threshold to detect copy-paste")
+
+	runCmd.Flags().IntVar(&rc.Goconst.MinStringLen, "goconst.min-len",
+		3, "Goconst: minimum constant string length")
+	runCmd.Flags().IntVar(&rc.Goconst.MinOccurrencesCount, "goconst.min-occurrences",
+		3, "Goconst: minimum occurences of constant string count to trigger issue")
 
 	runCmd.Flags().StringSliceVarP(&rc.EnabledLinters, "enable", "E", []string{}, "Enable specific linter")
 	runCmd.Flags().StringSliceVarP(&rc.DisabledLinters, "disable", "D", []string{}, "Disable specific linter")

pkg/config/config.go

@@ -64,6 +64,10 @@ type Run struct {
 	Dupl struct {
 		Threshold int
 	}
+	Goconst struct {
+		MinStringLen        int
+		MinOccurrencesCount int
+	}
 
 	EnabledLinters    []string
 	DisabledLinters   []string

pkg/enabled_linters.go

@@ -62,15 +62,16 @@ func GetAllSupportedLinterConfigs() []LinterConfig {
 		enabledByDefault(golinters.Govet{}, "Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string", false, false),
 		enabledByDefault(golinters.Errcheck{}, "Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases", true, false),
 		enabledByDefault(golinters.Golint{}, "Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes", false, false),
-		enabledByDefault(golinters.Deadcode{}, "Finds unused code", true, false),
-		enabledByDefault(golinters.Gocyclo{}, "Computes and checks the cyclomatic complexity of functions", false, false),
+		enabledByDefault(golinters.Megacheck{}, "Megacheck: 3 sub-linters in one: staticcheck, gosimple and unused", true, true),
 		enabledByDefault(golinters.Structcheck{}, "Finds unused struct fields", true, false),
 		enabledByDefault(golinters.Varcheck{}, "Finds unused global variables and constants", true, false),
-		enabledByDefault(golinters.Megacheck{}, "Megacheck: 3 sub-linters in one: staticcheck, gosimple and unused", true, true),
-		enabledByDefault(golinters.Dupl{}, "Tool for code clone detection", false, false),
-		enabledByDefault(golinters.Ineffassign{}, "Detects when assignments to existing variables are not used", false, false),
 		enabledByDefault(golinters.Interfacer{}, "Linter that suggests narrower interface types", true, true),
 		enabledByDefault(golinters.Unconvert{}, "Remove unnecessary type conversions", true, false),
+		enabledByDefault(golinters.Ineffassign{}, "Detects when assignments to existing variables are not used", false, false),
+		enabledByDefault(golinters.Dupl{}, "Tool for code clone detection", false, false),
+		enabledByDefault(golinters.Goconst{}, "Finds repeated strings that could be replaced by a constant", false, false),
+		enabledByDefault(golinters.Deadcode{}, "Finds unused code", true, false),
+		enabledByDefault(golinters.Gocyclo{}, "Computes and checks the cyclomatic complexity of functions", false, false),
 
 		disabledByDefault(golinters.Gofmt{}, "Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification", false, false),
 		disabledByDefault(golinters.Gofmt{UseGoimports: true}, "Goimports does everything that gofmt does. Additionally it checks unused imports", false, false),

pkg/golinters/goconst.go

@@ -0,0 +1,44 @@
+package golinters
+
+import (
+	"context"
+	"fmt"
+
+	goconstAPI "github.com/golangci/goconst"
+	"github.com/golangci/golangci-lint/pkg/result"
+)
+
+type Goconst struct{}
+
+func (Goconst) Name() string {
+	return "goconst"
+}
+
+func (lint Goconst) Run(ctx context.Context, lintCtx *Context) (*result.Result, error) {
+	issues, err := goconstAPI.Run(lintCtx.Paths.Files, true,
+		lintCtx.RunCfg().Goconst.MinStringLen,
+		lintCtx.RunCfg().Goconst.MinOccurrencesCount,
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	res := &result.Result{}
+	for _, i := range issues {
+		textBegin := fmt.Sprintf("string %s has %d occurrences", formatCode(i.Str, lintCtx.RunCfg()), i.OccurencesCount)
+		var textEnd string
+		if i.MatchingConst == "" {
+			textEnd = ", make it a constant"
+		} else {
+			textEnd = fmt.Sprintf(", but such constant %s already exists", formatCode(i.MatchingConst, lintCtx.RunCfg()))
+		}
+		res.Issues = append(res.Issues, result.Issue{
+			File:       i.Pos.Filename,
+			LineNumber: i.Pos.Line,
+			Text:       textBegin + textEnd,
+			FromLinter: lint.Name(),
+		})
+	}
+
+	return res, nil
+}

pkg/golinters/linters_test.go

@@ -45,6 +45,6 @@ func installBinary(t *testing.T) {
 
 func testOneSource(t *testing.T, sourcePath string) {
 	goErrchkBin := filepath.Join(runtime.GOROOT(), "test", "errchk")
-	cmd := exec.Command(goErrchkBin, binName, "run", "--enable-all", sourcePath)
+	cmd := exec.Command(goErrchkBin, binName, "run", "--enable-all", "--gocyclo.min-complexity", "20", sourcePath)
 	runGoErrchk(cmd, t)
 }

pkg/golinters/testdata/goconst.go

@@ -0,0 +1,30 @@
+package testdata
+
+import "fmt"
+
+func GoconstA() { // nolint:dupl
+	a := "needconst" // ERROR "string `needconst` has 5 occurrences, make it a constant"
+	fmt.Print(a)
+	b := "needconst"
+	fmt.Print(b)
+	c := "needconst"
+	fmt.Print(c)
+}
+
+func GoconstB() {
+	a := "needconst"
+	fmt.Print(a)
+	b := "needconst"
+	fmt.Print(b)
+}
+
+const AlreadyHasConst = "alreadyhasconst"
+
+func GoconstC() { // nolint:dupl
+	a := "alreadyhasconst" // ERROR "string `alreadyhasconst` has 3 occurrences, but such constant `AlreadyHasConst` already exists"
+	fmt.Print(a)
+	b := "alreadyhasconst"
+	fmt.Print(b)
+	c := "alreadyhasconst"
+	fmt.Print(c)
+}

vendor/github.com/golangci/goconst/LICENSE

@@ -0,0 +1,21 @@
+The MIT License (MIT)
+
+Copyright (c) 2015 Jonathan Gautheron
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.

vendor/github.com/golangci/goconst/README.md

@@ -0,0 +1,49 @@
+# goconst
+
+Find repeated strings that could be replaced by a constant.
+
+### Motivation
+
+There are obvious benefits to using constants instead of repeating strings, mostly to ease maintenance. Cannot argue against changing a single constant versus many strings.
+
+While this could be considered a beginner mistake, across time, multiple packages and large codebases, some repetition could have slipped in.
+
+### Get Started
+
+    $ go get github.com/jgautheron/goconst/cmd/goconst
+    $ goconst ./...
+
+### Usage
+
+```
+Usage:
+
+  goconst ARGS <directory>
+
+Flags:
+
+  -ignore            exclude files matching the given regular expression
+  -ignore-tests      exclude tests from the search (default: true)
+  -min-occurrences   report from how many occurrences (default: 2)
+  -min-length        only report strings with the minimum given length (default: 3)
+  -match-constant    look for existing constants matching the values
+  -numbers           search also for duplicated numbers
+  -min          	   minimum value, only works with -numbers
+  -max          	   maximum value, only works with -numbers
+  -output            output formatting (text or json)
+
+Examples:
+
+  goconst ./...
+  goconst -ignore "yacc|\.pb\." $GOPATH/src/github.com/cockroachdb/cockroach/...
+  goconst -min-occurrences 3 -output json $GOPATH/src/github.com/cockroachdb/cockroach
+  goconst -numbers -min 60 -max 512 .
+```
+
+### Other static analysis tools
+
+- [gogetimports](https://github.com/jgautheron/gogetimports): Get a JSON-formatted list of imports.
+- [usedexports](https://github.com/jgautheron/usedexports): Find exported variables that could be unexported.
+
+### License
+MIT

vendor/github.com/golangci/goconst/parser.go

@@ -0,0 +1,187 @@
+// Package goconst finds repeated strings that could be replaced by a constant.
+//
+// There are obvious benefits to using constants instead of repeating strings,
+// mostly to ease maintenance. Cannot argue against changing a single constant versus many strings.
+// While this could be considered a beginner mistake, across time,
+// multiple packages and large codebases, some repetition could have slipped in.
+package goconst
+
+import (
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"log"
+	"os"
+	"path/filepath"
+	"regexp"
+	"strings"
+)
+
+const (
+	testSuffix = "_test.go"
+)
+
+type Parser struct {
+	// Meant to be passed via New()
+	path, ignore               string
+	ignoreTests, matchConstant bool
+	minLength                  int
+
+	supportedTokens []token.Token
+
+	// Internals
+	strs   Strings
+	consts Constants
+}
+
+// New creates a new instance of the parser.
+// This is your entry point if you'd like to use goconst as an API.
+func New(path, ignore string, ignoreTests, matchConstant, numbers bool, minLength int) *Parser {
+	supportedTokens := []token.Token{token.STRING}
+	if numbers {
+		supportedTokens = append(supportedTokens, token.INT, token.FLOAT)
+	}
+
+	return &Parser{
+		path:            path,
+		ignore:          ignore,
+		ignoreTests:     ignoreTests,
+		matchConstant:   matchConstant,
+		minLength:       minLength,
+		supportedTokens: supportedTokens,
+
+		// Initialize the maps
+		strs:   Strings{},
+		consts: Constants{},
+	}
+}
+
+// ParseTree will search the given path for occurrences that could be moved into constants.
+// If "..." is appended, the search will be recursive.
+func (p *Parser) ParseTree() (Strings, Constants, error) {
+	pathLen := len(p.path)
+	// Parse recursively the given path if the recursive notation is found
+	if pathLen >= 5 && p.path[pathLen-3:] == "..." {
+		filepath.Walk(p.path[:pathLen-3], func(path string, f os.FileInfo, err error) error {
+			if err != nil {
+				log.Println(err)
+				// resume walking
+				return nil
+			}
+
+			if f.IsDir() {
+				p.parseDir(path)
+			}
+			return nil
+		})
+	} else {
+		p.parseDir(p.path)
+	}
+	return p.strs, p.consts, nil
+}
+
+func (p *Parser) parseDir(dir string) error {
+	fset := token.NewFileSet()
+	pkgs, err := parser.ParseDir(fset, dir, func(info os.FileInfo) bool {
+		valid, name := true, info.Name()
+
+		if p.ignoreTests {
+			if strings.HasSuffix(name, testSuffix) {
+				valid = false
+			}
+		}
+
+		if len(p.ignore) != 0 {
+			match, err := regexp.MatchString(p.ignore, dir+name)
+			if err != nil {
+				log.Fatal(err)
+				return true
+			}
+			if match {
+				valid = false
+			}
+		}
+
+		return valid
+	}, 0)
+	if err != nil {
+		return err
+	}
+
+	for _, pkg := range pkgs {
+		for fn, f := range pkg.Files {
+			ast.Walk(&treeVisitor{
+				fileSet:     fset,
+				packageName: pkg.Name,
+				fileName:    fn,
+				p:           p,
+			}, f)
+		}
+	}
+
+	return nil
+}
+
+type Strings map[string][]ExtendedPos
+type Constants map[string]ConstType
+
+type ConstType struct {
+	token.Position
+	Name, packageName string
+}
+
+type ExtendedPos struct {
+	token.Position
+	packageName string
+}
+
+type Issue struct {
+	Pos             token.Position
+	OccurencesCount int
+	Str             string
+	MatchingConst   string
+}
+
+func Run(files []string, matchWithConstants bool, minStringLength, minOccurrences int) ([]Issue, error) {
+	p := New("", "", false, matchWithConstants, false, minStringLength)
+	var issues []Issue
+	fset := token.NewFileSet()
+	for _, path := range files {
+		f, err := parser.ParseFile(fset, path, nil, 0)
+		if err != nil {
+			return nil, err
+		}
+		ast.Walk(&treeVisitor{
+			fileSet:     fset,
+			packageName: "",
+			fileName:    path,
+			p:           p,
+		}, f)
+	}
+
+	for str, item := range p.strs {
+		// Filter out items whose occurrences don't match the min value
+		if len(item) < minOccurrences {
+			delete(p.strs, str)
+		}
+	}
+
+	for str, item := range p.strs {
+		fi := item[0]
+		i := Issue{
+			Pos:             fi.Position,
+			OccurencesCount: len(item),
+			Str:             str,
+		}
+
+		if len(p.consts) != 0 {
+			if cst, ok := p.consts[str]; ok {
+				// const should be in the same package and exported
+				i.MatchingConst = cst.Name
+			}
+		}
+		issues = append(issues, i)
+	}
+
+	return issues, nil
+}

vendor/github.com/golangci/goconst/visitor.go

@@ -0,0 +1,143 @@
+package goconst
+
+import (
+	"go/ast"
+	"go/token"
+	"strings"
+)
+
+// treeVisitor carries the package name and file name
+// for passing it to the imports map, and the fileSet for
+// retrieving the token.Position.
+type treeVisitor struct {
+	p                     *Parser
+	fileSet               *token.FileSet
+	packageName, fileName string
+}
+
+// Visit browses the AST tree for strings that could be potentially
+// replaced by constants.
+// A map of existing constants is built as well (-match-constant).
+func (v *treeVisitor) Visit(node ast.Node) ast.Visitor {
+	if node == nil {
+		return v
+	}
+
+	// A single case with "ast.BasicLit" would be much easier
+	// but then we wouldn't be able to tell in which context
+	// the string is defined (could be a constant definition).
+	switch t := node.(type) {
+	// Scan for constants in an attempt to match strings with existing constants
+	case *ast.GenDecl:
+		if !v.p.matchConstant {
+			return v
+		}
+		if t.Tok != token.CONST {
+			return v
+		}
+
+		for _, spec := range t.Specs {
+			val := spec.(*ast.ValueSpec)
+			for i, str := range val.Values {
+				lit, ok := str.(*ast.BasicLit)
+				if !ok || !v.isSupported(lit.Kind) {
+					continue
+				}
+
+				v.addConst(val.Names[i].Name, lit.Value, val.Names[i].Pos())
+			}
+		}
+
+	// foo := "moo"
+	case *ast.AssignStmt:
+		for _, rhs := range t.Rhs {
+			lit, ok := rhs.(*ast.BasicLit)
+			if !ok || !v.isSupported(lit.Kind) {
+				continue
+			}
+
+			v.addString(lit.Value, rhs.(*ast.BasicLit).Pos())
+		}
+
+	// if foo == "moo"
+	case *ast.BinaryExpr:
+		if t.Op != token.EQL && t.Op != token.NEQ {
+			return v
+		}
+
+		var lit *ast.BasicLit
+		var ok bool
+
+		lit, ok = t.X.(*ast.BasicLit)
+		if ok && v.isSupported(lit.Kind) {
+			v.addString(lit.Value, lit.Pos())
+		}
+
+		lit, ok = t.Y.(*ast.BasicLit)
+		if ok && v.isSupported(lit.Kind) {
+			v.addString(lit.Value, lit.Pos())
+		}
+
+	// case "foo":
+	case *ast.CaseClause:
+		for _, item := range t.List {
+			lit, ok := item.(*ast.BasicLit)
+			if ok && v.isSupported(lit.Kind) {
+				v.addString(lit.Value, lit.Pos())
+			}
+		}
+
+	// return "boo"
+	case *ast.ReturnStmt:
+		for _, item := range t.Results {
+			lit, ok := item.(*ast.BasicLit)
+			if ok && v.isSupported(lit.Kind) {
+				v.addString(lit.Value, lit.Pos())
+			}
+		}
+	}
+
+	return v
+}
+
+// addString adds a string in the map along with its position in the tree.
+func (v *treeVisitor) addString(str string, pos token.Pos) {
+	str = strings.Replace(str, `"`, "", 2)
+
+	// Ignore empty strings
+	if len(str) == 0 {
+		return
+	}
+
+	if len(str) < v.p.minLength {
+		return
+	}
+
+	_, ok := v.p.strs[str]
+	if !ok {
+		v.p.strs[str] = make([]ExtendedPos, 0)
+	}
+	v.p.strs[str] = append(v.p.strs[str], ExtendedPos{
+		packageName: v.packageName,
+		Position:    v.fileSet.Position(pos),
+	})
+}
+
+// addConst adds a const in the map along with its position in the tree.
+func (v *treeVisitor) addConst(name string, val string, pos token.Pos) {
+	val = strings.Replace(val, `"`, "", 2)
+	v.p.consts[val] = ConstType{
+		Name:        name,
+		packageName: v.packageName,
+		Position:    v.fileSet.Position(pos),
+	}
+}
+
+func (v *treeVisitor) isSupported(tk token.Token) bool {
+	for _, s := range v.p.supportedTokens {
+		if tk == s {
+			return true
+		}
+	}
+	return false
+}