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 8 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.
159 changes: 91 additions & 68 deletions check_no_globals.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package main
package gochecknoglobals

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,70 @@ type allowedExpression struct {
SelName string
}

// reports is the reports of found global variables that's not valid and will be
// reported to the analysis pass.
type report struct {
name string
pos token.Pos
}

// Analyzer is the analasys analyzer for gochecknoglobals. Ironically enough,
// this is in fact a global variable.
func Analyzer() *analysis.Analyzer {
return &analysis.Analyzer{
Name: "gochecknoglobals",
Doc: "Don't allow global variables",
Run: run,
Flags: flags(),
RunDespiteErrors: true,
}
}

func flags() flag.FlagSet {
flags := flag.NewFlagSet("", flag.ExitOnError)
flags.Bool("t", false, "Include tests")

return *flags
}

func run(pass *analysis.Pass) (interface{}, error) {
runTests := pass.Analyzer.Flags.Lookup("t").Value.(flag.Getter).Get().(bool)

for _, file := range pass.Files {
filename := pass.Fset.Position(file.Pos()).Filename

// Only test .go files, not generated test files.
if !strings.HasSuffix(filename, ".go") {
continue
}

if !runTests && strings.HasSuffix(filename, "_test.go") {
continue
}

for _, decl := range file.Decls {
genDecl, ok := decl.(*ast.GenDecl)
if !ok {
continue
}

if genDecl.Tok != token.VAR {
continue
}

for _, issue := range checkNoGlobals(genDecl) {
pass.Report(analysis.Diagnostic{
Pos: issue.pos,
Category: "global",
Message: fmt.Sprintf("%s is a global variable", issue.name),
})
}
}
}

return nil, nil
}

func isAllowed(v ast.Node) bool {
switch i := v.(type) {
case *ast.Ident:
Expand Down Expand Up @@ -66,79 +130,38 @@ 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(genDecl *ast.GenDecl) []report {
var globalVariables []report

messages := []string{}
for _, spec := range genDecl.Specs {
valueSpec := spec.(*ast.ValueSpec)
onlyAllowedValues := false

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
for _, vn := range valueSpec.Values {
if isAllowed(vn) {
onlyAllowedValues = true
continue
}
return nil
}
if !strings.HasSuffix(path, ".go") {
return nil
}
if !includeTests && strings.HasSuffix(path, "_test.go") {
return nil

onlyAllowedValues = false
break
}

fset := token.NewFileSet()
file, err := parser.ParseFile(fset, path, nil, 0)
if err != nil {
return err
if onlyAllowedValues {
continue
}

for _, decl := range file.Decls {
genDecl, ok := decl.(*ast.GenDecl)
if !ok {
for _, vn := range valueSpec.Names {
if isAllowed(vn) {
continue
}
if genDecl.Tok != token.VAR {
continue
}
filename := fset.Position(genDecl.TokPos).Filename
for _, spec := range genDecl.Specs {
valueSpec := spec.(*ast.ValueSpec)
onlyAllowedValues := false

for _, vn := range valueSpec.Values {
if isAllowed(vn) {
onlyAllowedValues = true
continue
}

onlyAllowedValues = false
break
}

if onlyAllowedValues {
continue
}

for _, vn := range valueSpec.Names {
if isAllowed(vn) {
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)
}
}

globalVariables = append(globalVariables, report{
name: vn.Name,
pos: vn.Pos(),
})
}
return nil
})
}

return messages, err
return globalVariables
}
Loading