Update gochecknoglobals, use source analyzer (#1422)

Author: bombsimon

go.mod

@@ -3,6 +3,7 @@ module github.com/golangci/golangci-lint
 go 1.12
 
 require (
+	4d63.com/gochecknoglobals v0.0.0-20201008074935-acfc0b28355a
 	github.com/Djarvur/go-err113 v0.0.0-20200511133814-5174e21577d5
 	github.com/OpenPeeDeeP/depguard v1.0.1
 	github.com/bombsimon/wsl/v3 v3.1.0
@@ -62,9 +63,10 @@ require (
 	github.com/ultraware/whitespace v0.0.4
 	github.com/uudashr/gocognit v1.0.1
 	github.com/valyala/quicktemplate v1.6.3
-	golang.org/x/tools v0.0.0-20201011145850-ed2f50202694
+	golang.org/x/sys v0.0.0-20201009025420-dfb3f7c4e634 // indirect
+	golang.org/x/tools v0.0.0-20201013201025-64a9e34f3752
 	gopkg.in/yaml.v2 v2.3.0
-	honnef.co/go/tools v0.0.1-2020.1.6
+	honnef.co/go/tools v0.0.1-2020.1.5
 	mvdan.cc/gofumpt v0.0.0-20200802201014-ab5a8192947d
 	mvdan.cc/interfacer v0.0.0-20180901003855-c20040233aed
 	mvdan.cc/lint v0.0.0-20170908181259-adc824a0674b // indirect

go.sum

@@ -1,100 +1,30 @@
 package golinters
 
 import (
-	"fmt"
-	"go/ast"
-	"go/token"
-	"strings"
-	"sync"
-
 	"golang.org/x/tools/go/analysis"
 
+	"4d63.com/gochecknoglobals/checknoglobals"
+
 	"github.com/golangci/golangci-lint/pkg/golinters/goanalysis"
-	"github.com/golangci/golangci-lint/pkg/lint/linter"
-	"github.com/golangci/golangci-lint/pkg/result"
 )
 
-const gochecknoglobalsName = "gochecknoglobals"
-
-//nolint:dupl
 func NewGochecknoglobals() *goanalysis.Linter {
-	var mu sync.Mutex
-	var resIssues []goanalysis.Issue
-
-	analyzer := &analysis.Analyzer{
-		Name: gochecknoglobalsName,
-		Doc:  goanalysis.TheOnlyanalyzerDoc,
-		Run: func(pass *analysis.Pass) (interface{}, error) {
-			var res []goanalysis.Issue
-			for _, file := range pass.Files {
-				fileIssues := checkFileForGlobals(file, pass.Fset)
-				for i := range fileIssues {
-					res = append(res, goanalysis.NewIssue(&fileIssues[i], pass))
-				}
-			}
-			if len(res) == 0 {
-				return nil, nil
-			}
-
-			mu.Lock()
-			resIssues = append(resIssues, res...)
-			mu.Unlock()
-
-			return nil, nil
+	gochecknoglobals := checknoglobals.Analyzer()
+
+	// gochecknoglobals only lints test files if the `-t` flag is passed so we
+	// pass the `t` flag as true to the analyzer before running it. This can be
+	// turned of by using the regular golangci-lint flags such as `--tests` or
+	// `--skip-files`.
+	linterConfig := map[string]map[string]interface{}{
+		gochecknoglobals.Name: {
+			"t": true,
 		},
 	}
-	return goanalysis.NewLinter(
-		gochecknoglobalsName,
-		"Checks that no globals are present in Go code",
-		[]*analysis.Analyzer{analyzer},
-		nil,
-	).WithIssuesReporter(func(*linter.Context) []goanalysis.Issue {
-		return resIssues
-	}).WithLoadMode(goanalysis.LoadModeSyntax)
-}
-
-func checkFileForGlobals(f *ast.File, fset *token.FileSet) []result.Issue {
-	var res []result.Issue
-	for _, decl := range f.Decls {
-		genDecl, ok := decl.(*ast.GenDecl)
-		if !ok {
-			continue
-		}
-		if genDecl.Tok != token.VAR {
-			continue
-		}
-
-		for _, spec := range genDecl.Specs {
-			valueSpec := spec.(*ast.ValueSpec)
-			for _, vn := range valueSpec.Names {
-				if isWhitelisted(vn) {
-					continue
-				}
-
-				res = append(res, result.Issue{
-					Pos:        fset.Position(vn.Pos()),
-					Text:       fmt.Sprintf("%s is a global variable", formatCode(vn.Name, nil)),
-					FromLinter: gochecknoglobalsName,
-				})
-			}
-		}
-	}
-
-	return res
-}
 
-func isWhitelisted(i *ast.Ident) bool {
-	return i.Name == "_" || i.Name == "version" || looksLikeError(i)
-}
-
-// looksLikeError returns true if the AST identifier starts
-// with 'err' or 'Err', or false otherwise.
-//
-// TODO: https://github.com/leighmcculloch/gochecknoglobals/issues/5
-func looksLikeError(i *ast.Ident) bool {
-	prefix := "err"
-	if i.IsExported() {
-		prefix = "Err"
-	}
-	return strings.HasPrefix(i.Name, prefix)
+	return goanalysis.NewLinter(
+		gochecknoglobals.Name,
+		gochecknoglobals.Doc,
+		[]*analysis.Analyzer{gochecknoglobals},
+		linterConfig,
+	).WithLoadMode(goanalysis.LoadModeSyntax)
 }

pkg/golinters/gochecknoglobals.go

@@ -15,7 +15,6 @@ import (
 
 const gochecknoinitsName = "gochecknoinits"
 
-//nolint:dupl
 func NewGochecknoinits() *goanalysis.Linter {
 	var mu sync.Mutex
 	var resIssues []goanalysis.Issue

pkg/golinters/gochecknoinits.go

@@ -4,11 +4,17 @@ package testdata
 import (
 	"errors"
 	"fmt"
+	"regexp"
 )
 
-var noGlobalsVar int // ERROR "`noGlobalsVar` is a global variable"
+var noGlobalsVar int // ERROR "noGlobalsVar is a global variable"
 var ErrSomeType = errors.New("test that global erorrs aren't warned")
 
+var (
+	OnlyDigites = regexp.MustCompile(`^\d+$`)
+	BadNamedErr = errors.New("this is bad") // ERROR "BadNamedErr is a global variable"
+)
+
 func NoGlobals() {
 	fmt.Print(noGlobalsVar)
 }