Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use analysis.Analyzer #16

Merged
merged 17 commits into from
Oct 8, 2020
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1 +1 @@
gochecknoglobals
./gochecknoglobals
19 changes: 10 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>
bombsimon marked this conversation as resolved.
Show resolved Hide resolved
leighmcculloch marked this conversation as resolved.
Show resolved Hide resolved
```

```
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.
179 changes: 0 additions & 179 deletions check_no_globals_test.go

This file was deleted.

80 changes: 40 additions & 40 deletions check_no_globals.go → checknoglobals/check_no_globals.go
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -18,6 +18,26 @@ type allowedExpression struct {
SelName string
}

// 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: "Check that there are no global variables",
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:
Expand Down Expand Up @@ -66,37 +86,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 {
Expand All @@ -107,7 +106,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
Expand All @@ -131,14 +129,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
}
25 changes: 25 additions & 0 deletions checknoglobals/check_no_globals_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}
File renamed without changes.
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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"
)
3 changes: 3 additions & 0 deletions checknoglobals/testdata/src/2/code.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package code

var myVar = 0 // want "myVar is a global variable"
3 changes: 3 additions & 0 deletions checknoglobals/testdata/src/2/code_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package code_test

var myTestVar = 0 // want "myTestVar is a global variable"
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ func someCode() bool {
return yourVar
}

var theVar = true
var theVar = true // want "theVar is a global variable"
3 changes: 3 additions & 0 deletions checknoglobals/testdata/src/3/code_1.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package code

var myVar = "global" // want "myVar is a global variable"
Loading