Use analysis.Analyzer

.gitignore

@@ -1 +1 @@
-gochecknoglobals
+./gochecknoglobals

README.md

@@ -29,22 +29,23 @@ go get 4d63.com/gochecknoglobals
 
 ## Usage
 
-```
-gochecknoglobals
-```
+The linter is built on [Go's analysis package] and does thus support all the
+built in flags and features from this type. The analyzer is executed by
+specifying packages.
+
+[Go's analysis package]: https://pkg.go.dev/golang.org/x/tools/go/analysis
 
 ```
-gochecknoglobals ./...
+gochecknoglobals [package]
 ```
 
 ```
-gochecknoglobals [path] [path] [path] [etc]
+gochecknoglobals ./...
 ```
 
-Add `-t` to include tests.
+By default, test files will not be checked but can be included by adding the
+`-t` flag.
 
 ```
-gochecknoglobals -t [path]
+gochecknoglobals -t [package]
 ```
-
-Note: Paths are only inspected recursively if the Go `/...` recursive path suffix is appended to the path.

check_no_globals.go → checknoglobals/check_no_globals.go

@@ -1,13 +1,13 @@
-package main
+package checknoglobals
 
 import (
+	"flag"
 	"fmt"
 	"go/ast"
-	"go/parser"
 	"go/token"
-	"os"
-	"path/filepath"
 	"strings"
+
+	"golang.org/x/tools/go/analysis"
 )
 
 // allowedExpression is a struct representing packages and methods that will
@@ -18,6 +18,36 @@ type allowedExpression struct {
 	SelName string
 }
 
+const Doc = `check that no global variables exist
+
+This analyzer checks for global variables and errors on any found.
+
+A global variable is a variable declared in package scope and that can be read
+and written to by any function within the package. Global variables can cause
+side effects which are difficult to keep track of. A code in one function may
+change the variables state while another unrelated chunk of code may be
+effected by it.`
+
+// Analyzer provides an Analyzer that checks that there are no global
+// variables, except for errors and variables containing regular
+// expressions.
+func Analyzer() *analysis.Analyzer {
+	return &analysis.Analyzer{
+		Name:             "gochecknoglobals",
+		Doc:              Doc,
+		Run:              checkNoGlobals,
+		Flags:            flags(),
+		RunDespiteErrors: true,
+	}
+}
+
+func flags() flag.FlagSet {
+	flags := flag.NewFlagSet("", flag.ExitOnError)
+	flags.Bool("t", false, "Include tests")
+
+	return *flags
+}
+
 func isAllowed(v ast.Node) bool {
 	switch i := v.(type) {
 	case *ast.Ident:
@@ -66,37 +96,16 @@ func looksLikeError(i *ast.Ident) bool {
 	return strings.HasPrefix(i.Name, prefix)
 }
 
-func checkNoGlobals(rootPath string, includeTests bool) ([]string, error) {
-	const recursiveSuffix = string(filepath.Separator) + "..."
-	recursive := false
-	if strings.HasSuffix(rootPath, recursiveSuffix) {
-		recursive = true
-		rootPath = rootPath[:len(rootPath)-len(recursiveSuffix)]
-	}
+func checkNoGlobals(pass *analysis.Pass) (interface{}, error) {
+	includeTests := pass.Analyzer.Flags.Lookup("t").Value.(flag.Getter).Get().(bool)
 
-	messages := []string{}
-
-	err := filepath.Walk(rootPath, func(path string, info os.FileInfo, err error) error {
-		if err != nil {
-			return err
-		}
-		if info.IsDir() {
-			if !recursive && path != rootPath {
-				return filepath.SkipDir
-			}
-			return nil
-		}
-		if !strings.HasSuffix(path, ".go") {
-			return nil
+	for _, file := range pass.Files {
+		filename := pass.Fset.Position(file.Pos()).Filename
+		if !strings.HasSuffix(filename, ".go") {
+			continue
 		}
-		if !includeTests && strings.HasSuffix(path, "_test.go") {
-			return nil
-		}
-
-		fset := token.NewFileSet()
-		file, err := parser.ParseFile(fset, path, nil, 0)
-		if err != nil {
-			return err
+		if !includeTests && strings.HasSuffix(filename, "_test.go") {
+			continue
 		}
 
 		for _, decl := range file.Decls {
@@ -107,7 +116,6 @@ func checkNoGlobals(rootPath string, includeTests bool) ([]string, error) {
 			if genDecl.Tok != token.VAR {
 				continue
 			}
-			filename := fset.Position(genDecl.TokPos).Filename
 			for _, spec := range genDecl.Specs {
 				valueSpec := spec.(*ast.ValueSpec)
 				onlyAllowedValues := false
@@ -131,14 +139,16 @@ func checkNoGlobals(rootPath string, includeTests bool) ([]string, error) {
 						continue
 					}
 
-					line := fset.Position(vn.Pos()).Line
-					message := fmt.Sprintf("%s:%d %s is a global variable", filename, line, vn.Name)
-					messages = append(messages, message)
+					message := fmt.Sprintf("%s is a global variable", vn.Name)
+					pass.Report(analysis.Diagnostic{
+						Pos:      vn.Pos(),
+						Category: "global",
+						Message:  message,
+					})
 				}
 			}
 		}
-		return nil
-	})
+	}
 
-	return messages, err
+	return nil, nil
 }

checknoglobals/check_no_globals_test.go

@@ -0,0 +1,25 @@
+package checknoglobals
+
+import (
+	"flag"
+	"strconv"
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+func TestCheckNoGlobals(t *testing.T) {
+	testdata := analysistest.TestData()
+	flags := flag.NewFlagSet("", flag.ExitOnError)
+	flags.Bool("t", true, "")
+
+	analyzer := Analyzer()
+	analyzer.Flags = *flags
+
+	for i := 0; i <= 10; i++ {
+		dir := strconv.Itoa(i)
+		t.Run(dir, func(t *testing.T) {
+			analysistest.Run(t, testdata, analyzer, dir)
+		})
+	}
+}

testdata/10/code.go → checknoglobals/testdata/src/10/code.go

@@ -7,7 +7,7 @@ import (
 )
 
 // myVar is just a bad named global var.
-var myVar = 1
+var myVar = 1 // want "myVar is a global variable"
 
 // ErrNotFound is an error and should be OK.
 var ErrNotFound = errors.New("this is error")
@@ -30,5 +30,5 @@ var (
 	PrecompileFour = regexp.MustCompile(`[a-z]{1,3}`)
 	PrecompileFive = regexp.MustCompile(`[a-z]{3,6}`)
 	PrecompileSix  = regexp.MustCompile(`[a-z]{6,9}`)
-	HTTPClient     = http.Client{}
+	HTTPClient     = http.Client{} // want "HTTPClient is a global variable"
 )

checknoglobals/testdata/src/2/code.go

@@ -0,0 +1,3 @@
+package code
+
+var myVar = 0 // want "myVar is a global variable"

checknoglobals/testdata/src/2/code_test.go

@@ -0,0 +1,3 @@
+package code_test
+
+var myTestVar = 0 // want "myTestVar is a global variable"