Skip to content

Commit

Permalink
Ignore global error variables
Browse files Browse the repository at this point in the history
Error variables are variables starting with 'err' or 'Err'.

This implements #2.
  • Loading branch information
leonklingele committed Sep 8, 2018
1 parent 9d4b45f commit 4886830
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 3 deletions.
22 changes: 19 additions & 3 deletions check_no_globals.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,22 @@ import (
"strings"
)

func isWhitelisted(i *ast.Ident) bool {
return i.Name == "_" || looksLikeError(i)
}

// looksLikeError checks if the AST identifier starts with 'err' or 'Err'.
//
// See the following issue on how to further improve this func:
// 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
Expand Down Expand Up @@ -55,11 +71,11 @@ func checkNoGlobals(rootPath string, includeTests bool) ([]string, error) {
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 == "_" {
vn := valueSpec.Names[i]
if isWhitelisted(vn) {
continue
}
message := fmt.Sprintf("%s:%d %s is a global variable", filename, line, name)
message := fmt.Sprintf("%s:%d %s is a global variable", filename, line, vn.Name)
messages = append(messages, message)
}
}
Expand Down
21 changes: 21 additions & 0 deletions check_no_globals_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,19 @@ func TestCheckNoGlobals(t *testing.T) {
path: "testdata/6",
wantMessages: nil,
},
{
path: "testdata/7",
wantMessages: []string{
"testdata/7/code.go:8 myVar is a global variable",
// "testdata/7/code.go:13 errFakeErrorUnexported is a global variable",
// "testdata/7/code.go:14 ErrFakeErrorExported is a global variable",
"testdata/7/code.go:17 myErrVar is a global variable",
"testdata/7/code.go:18 myVarErr is a global variable",
"testdata/7/code.go:19 myVarError is a global variable",
"testdata/7/code.go:20 customErr is a global variable",
"testdata/7/code.go:29 declaredErr is a global variable",
},
},
{
path: ".",
wantMessages: nil,
Expand All @@ -93,6 +106,14 @@ func TestCheckNoGlobals(t *testing.T) {
"testdata/4/subpkg/code_1.go:3 myVar is a global variable",
"testdata/5/code.go:3 myVar1 is a global variable",
"testdata/5/code.go:3 myVar2 is a global variable",
"testdata/7/code.go:8 myVar is a global variable",
// "testdata/7/code.go:13 errFakeErrorUnexported is a global variable",
// "testdata/7/code.go:14 ErrFakeErrorExported is a global variable",
"testdata/7/code.go:17 myErrVar is a global variable",
"testdata/7/code.go:18 myVarErr is a global variable",
"testdata/7/code.go:19 myVarError is a global variable",
"testdata/7/code.go:20 customErr is a global variable",
"testdata/7/code.go:29 declaredErr is a global variable",
},
},
}
Expand Down
34 changes: 34 additions & 0 deletions testdata/7/code.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package code

import (
"errors"
)

// Those are not errors
var myVar = 1

// Those are fake errors which are currently not detected
// because they start with 'err' or 'Err' and we don't
// check if such a variable implements the error interface.
var errFakeErrorUnexported = 1
var ErrFakeErrorExported = 1

// Those errors are not named correctly
var myErrVar = errors.New("myErrVar")
var myVarErr = errors.New("myVarErr")
var myVarError = errors.New("myVarErr")
var customErr = customError{"customErr"}

// Those are actual errors which should be ignored
var errUnexported = errors.New("errUnexported")
var ErrExported = errors.New("ErrExported")
var errCustomUnexported = customError{"errCustomUnexported"}
var ErrCustomExported = customError{"ErrCustomExported"}

// Those actual errors have a declared error type
var declaredErr error = errors.New("declaredErr")
var errDeclared error = errors.New("errDeclared")

type customError struct{ e string }

func (e *customError) Error() string { return e.e }

0 comments on commit 4886830

Please sign in to comment.