From 17a7ffa42374937bfecabfb8d2efbd4db0c26741 Mon Sep 17 00:00:00 2001 From: Leigh McCulloch Date: Sun, 9 Sep 2018 04:13:59 +0000 Subject: [PATCH] Update gochecknoglobals --- .../gochecknoglobals/check_no_globals.go | 36 ++++++++++++++----- .../src/4d63.com/gochecknoglobals/main.go | 10 ++++-- _linters/src/manifest | 2 +- 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/_linters/src/4d63.com/gochecknoglobals/check_no_globals.go b/_linters/src/4d63.com/gochecknoglobals/check_no_globals.go index c1155337..ed54f780 100644 --- a/_linters/src/4d63.com/gochecknoglobals/check_no_globals.go +++ b/_linters/src/4d63.com/gochecknoglobals/check_no_globals.go @@ -10,7 +10,23 @@ import ( "strings" ) -func checkNoGlobals(rootPath string) ([]string, error) { +func isWhitelisted(i *ast.Ident) bool { + return i.Name == "_" || 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) +} + +func checkNoGlobals(rootPath string, includeTests bool) ([]string, error) { const recursiveSuffix = string(filepath.Separator) + "..." recursive := false if strings.HasSuffix(rootPath, recursiveSuffix) { @@ -33,6 +49,9 @@ func checkNoGlobals(rootPath string) ([]string, error) { if !strings.HasSuffix(path, ".go") { return nil } + if !includeTests && strings.HasSuffix(path, "_test.go") { + return nil + } fset := token.NewFileSet() file, err := parser.ParseFile(fset, path, nil, 0) @@ -50,14 +69,15 @@ func checkNoGlobals(rootPath string) ([]string, error) { } filename := fset.Position(genDecl.TokPos).Filename line := fset.Position(genDecl.TokPos).Line - valueSpec := genDecl.Specs[0].(*ast.ValueSpec) - for i := 0; i < len(valueSpec.Names); i++ { - name := valueSpec.Names[i].Name - if name == "_" { - continue + for _, spec := range genDecl.Specs { + valueSpec := spec.(*ast.ValueSpec) + for _, vn := range valueSpec.Names { + if isWhitelisted(vn) { + continue + } + message := fmt.Sprintf("%s:%d %s is a global variable", filename, line, vn.Name) + messages = append(messages, message) } - message := fmt.Sprintf("%s:%d %s is a global variable", filename, line, name) - messages = append(messages, message) } } return nil diff --git a/_linters/src/4d63.com/gochecknoglobals/main.go b/_linters/src/4d63.com/gochecknoglobals/main.go index ec9d953f..57d2d533 100644 --- a/_linters/src/4d63.com/gochecknoglobals/main.go +++ b/_linters/src/4d63.com/gochecknoglobals/main.go @@ -7,9 +7,11 @@ import ( ) func main() { - flagPrintHelp := flag.Bool("help", false, "") + flagPrintHelp := flag.Bool("h", false, "Print help") + flagIncludeTests := flag.Bool("t", false, "Include tests") flag.Usage = func() { - fmt.Fprintf(os.Stderr, "Usage: gochecknoglobals [path] [path] ...\n") + fmt.Fprintf(os.Stderr, "Usage: gochecknoglobals [-t] [path] [path] ...\n") + flag.PrintDefaults() } flag.Parse() @@ -18,6 +20,8 @@ func main() { return } + includeTests := *flagIncludeTests + paths := flag.Args() if len(paths) == 0 { paths = []string{"./..."} @@ -26,7 +30,7 @@ func main() { exitWithError := false for _, path := range paths { - messages, err := checkNoGlobals(path) + messages, err := checkNoGlobals(path, includeTests) for _, message := range messages { fmt.Fprintf(os.Stdout, "%s\n", message) exitWithError = true diff --git a/_linters/src/manifest b/_linters/src/manifest index 11a30409..bf8798f9 100644 --- a/_linters/src/manifest +++ b/_linters/src/manifest @@ -5,7 +5,7 @@ "importpath": "4d63.com/gochecknoglobals", "repository": "https://github.com/leighmcculloch/gochecknoglobals", "vcs": "git", - "revision": "9a66a4a931c82990a7eb42e057408ac5f2bb6bee", + "revision": "5090db600a84f7a401cb031f4e8e2e420b0e1ecd", "branch": "master", "notests": true },