Skip to content

Commit

Permalink
Update gochecknoglobals, use source analyzer (#1422)
Browse files Browse the repository at this point in the history
  • Loading branch information
bombsimon authored Oct 15, 2020
1 parent 58234f0 commit a8b7b00
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 102 deletions.
6 changes: 4 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ module github.com/golangci/golangci-lint
go 1.12

require (
4d63.com/gochecknoglobals v0.0.0-20201008074935-acfc0b28355a
github.com/Djarvur/go-err113 v0.0.0-20200511133814-5174e21577d5
github.com/OpenPeeDeeP/depguard v1.0.1
github.com/bombsimon/wsl/v3 v3.1.0
Expand Down Expand Up @@ -62,9 +63,10 @@ require (
github.com/ultraware/whitespace v0.0.4
github.com/uudashr/gocognit v1.0.1
github.com/valyala/quicktemplate v1.6.3
golang.org/x/tools v0.0.0-20201011145850-ed2f50202694
golang.org/x/sys v0.0.0-20201009025420-dfb3f7c4e634 // indirect
golang.org/x/tools v0.0.0-20201013201025-64a9e34f3752
gopkg.in/yaml.v2 v2.3.0
honnef.co/go/tools v0.0.1-2020.1.6
honnef.co/go/tools v0.0.1-2020.1.5
mvdan.cc/gofumpt v0.0.0-20200802201014-ab5a8192947d
mvdan.cc/interfacer v0.0.0-20180901003855-c20040233aed
mvdan.cc/lint v0.0.0-20170908181259-adc824a0674b // indirect
Expand Down
16 changes: 11 additions & 5 deletions go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

104 changes: 17 additions & 87 deletions pkg/golinters/gochecknoglobals.go
Original file line number Diff line number Diff line change
@@ -1,100 +1,30 @@
package golinters

import (
"fmt"
"go/ast"
"go/token"
"strings"
"sync"

"golang.org/x/tools/go/analysis"

"4d63.com/gochecknoglobals/checknoglobals"

"github.com/golangci/golangci-lint/pkg/golinters/goanalysis"
"github.com/golangci/golangci-lint/pkg/lint/linter"
"github.com/golangci/golangci-lint/pkg/result"
)

const gochecknoglobalsName = "gochecknoglobals"

//nolint:dupl
func NewGochecknoglobals() *goanalysis.Linter {
var mu sync.Mutex
var resIssues []goanalysis.Issue

analyzer := &analysis.Analyzer{
Name: gochecknoglobalsName,
Doc: goanalysis.TheOnlyanalyzerDoc,
Run: func(pass *analysis.Pass) (interface{}, error) {
var res []goanalysis.Issue
for _, file := range pass.Files {
fileIssues := checkFileForGlobals(file, pass.Fset)
for i := range fileIssues {
res = append(res, goanalysis.NewIssue(&fileIssues[i], pass))
}
}
if len(res) == 0 {
return nil, nil
}

mu.Lock()
resIssues = append(resIssues, res...)
mu.Unlock()

return nil, nil
gochecknoglobals := checknoglobals.Analyzer()

// gochecknoglobals only lints test files if the `-t` flag is passed so we
// pass the `t` flag as true to the analyzer before running it. This can be
// turned of by using the regular golangci-lint flags such as `--tests` or
// `--skip-files`.
linterConfig := map[string]map[string]interface{}{
gochecknoglobals.Name: {
"t": true,
},
}
return goanalysis.NewLinter(
gochecknoglobalsName,
"Checks that no globals are present in Go code",
[]*analysis.Analyzer{analyzer},
nil,
).WithIssuesReporter(func(*linter.Context) []goanalysis.Issue {
return resIssues
}).WithLoadMode(goanalysis.LoadModeSyntax)
}

func checkFileForGlobals(f *ast.File, fset *token.FileSet) []result.Issue {
var res []result.Issue
for _, decl := range f.Decls {
genDecl, ok := decl.(*ast.GenDecl)
if !ok {
continue
}
if genDecl.Tok != token.VAR {
continue
}

for _, spec := range genDecl.Specs {
valueSpec := spec.(*ast.ValueSpec)
for _, vn := range valueSpec.Names {
if isWhitelisted(vn) {
continue
}

res = append(res, result.Issue{
Pos: fset.Position(vn.Pos()),
Text: fmt.Sprintf("%s is a global variable", formatCode(vn.Name, nil)),
FromLinter: gochecknoglobalsName,
})
}
}
}

return res
}

func isWhitelisted(i *ast.Ident) bool {
return i.Name == "_" || i.Name == "version" || 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)
return goanalysis.NewLinter(
gochecknoglobals.Name,
gochecknoglobals.Doc,
[]*analysis.Analyzer{gochecknoglobals},
linterConfig,
).WithLoadMode(goanalysis.LoadModeSyntax)
}
1 change: 0 additions & 1 deletion pkg/golinters/gochecknoinits.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (

const gochecknoinitsName = "gochecknoinits"

//nolint:dupl
func NewGochecknoinits() *goanalysis.Linter {
var mu sync.Mutex
var resIssues []goanalysis.Issue
Expand Down
8 changes: 7 additions & 1 deletion test/testdata/gochecknoglobals.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,17 @@ package testdata
import (
"errors"
"fmt"
"regexp"
)

var noGlobalsVar int // ERROR "`noGlobalsVar` is a global variable"
var noGlobalsVar int // ERROR "noGlobalsVar is a global variable"
var ErrSomeType = errors.New("test that global erorrs aren't warned")

var (
OnlyDigites = regexp.MustCompile(`^\d+$`)
BadNamedErr = errors.New("this is bad") // ERROR "BadNamedErr is a global variable"
)

func NoGlobals() {
fmt.Print(noGlobalsVar)
}
6 changes: 0 additions & 6 deletions test/testdata/testpackage_internal_test.go

This file was deleted.

0 comments on commit a8b7b00

Please sign in to comment.