diff --git a/Gopkg.lock b/Gopkg.lock index 52e66e53771d..138233d283f3 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -555,7 +555,7 @@ [[projects]] branch = "master" - digest = "1:aac721c5ec779073b35cf7a51b7764d13f6db2243d650965d2c71469e8994e5b" + digest = "1:c09b8b20f35b802fa418b0715e3147d6366b706c6003bbdef9df78272ffe80a5" name = "golang.org/x/tools" packages = [ "go/ast/astutil", diff --git a/README.md b/README.md index 1f2294813bc5..659ec9f900a3 100644 --- a/README.md +++ b/README.md @@ -199,6 +199,7 @@ lll: Reports long lines [fast: true] unparam: Reports unused function parameters [fast: false] nakedret: Finds naked returns in functions greater than a specified function length [fast: true] prealloc: Finds slice declarations that could potentially be preallocated [fast: true] +scopelint: Scopelint checks for unpinned variables in go programs [fast: true] ``` Pass `-E/--enable` to enable linter and `-D/--disable` to disable: @@ -398,6 +399,7 @@ golangci-lint help linters - [unparam](https://github.com/mvdan/unparam) - Reports unused function parameters - [nakedret](https://github.com/alexkohler/nakedret) - Finds naked returns in functions greater than a specified function length - [prealloc](https://github.com/alexkohler/prealloc) - Finds slice declarations that could potentially be preallocated +- [scopelint](https://github.com/kyoh86/scopelint) - Scopelint checks for unpinned variables in go programs ## Configuration @@ -807,6 +809,7 @@ Thanks to developers and authors of used linters: - [client9](https://github.com/client9) - [walle](https://github.com/walle) - [alexkohler](https://github.com/alexkohler) +- [kyoh86](https://github.com/kyoh86) ## Changelog diff --git a/pkg/golinters/scopelint.go b/pkg/golinters/scopelint.go new file mode 100644 index 000000000000..de71baef8fe0 --- /dev/null +++ b/pkg/golinters/scopelint.go @@ -0,0 +1,139 @@ +package golinters + +import ( + "context" + "fmt" + "go/ast" + "go/token" + + "github.com/golangci/golangci-lint/pkg/lint/linter" + "github.com/golangci/golangci-lint/pkg/result" +) + +type Scopelint struct{} + +func (Scopelint) Name() string { + return "scopelint" +} + +func (Scopelint) Desc() string { + return "Scopelint checks for unpinned variables in go programs" +} + +func (lint Scopelint) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { + var res []result.Issue + + for _, f := range lintCtx.ASTCache.GetAllValidFiles() { + n := Node{ + fset: f.Fset, + dangerObjects: map[*ast.Object]struct{}{}, + unsafeObjects: map[*ast.Object]struct{}{}, + skipFuncs: map[*ast.FuncLit]struct{}{}, + issues: &res, + } + ast.Walk(&n, f.F) + } + + return res, nil +} + +// The code below is copy-pasted from https://github.com/kyoh86/scopelint + +// Node represents a Node being linted. +type Node struct { + fset *token.FileSet + dangerObjects map[*ast.Object]struct{} + unsafeObjects map[*ast.Object]struct{} + skipFuncs map[*ast.FuncLit]struct{} + issues *[]result.Issue +} + +// Visit method is invoked for each node encountered by Walk. +// If the result visitor w is not nil, Walk visits each of the children +// of node with the visitor w, followed by a call of w.Visit(nil). +//nolint:gocyclo +func (f *Node) Visit(node ast.Node) ast.Visitor { + switch typedNode := node.(type) { + case *ast.ForStmt: + switch init := typedNode.Init.(type) { + case *ast.AssignStmt: + for _, lh := range init.Lhs { + switch tlh := lh.(type) { + case *ast.Ident: + f.unsafeObjects[tlh.Obj] = struct{}{} + } + } + } + + case *ast.RangeStmt: + // Memory variables declarated in range statement + switch k := typedNode.Key.(type) { + case *ast.Ident: + f.unsafeObjects[k.Obj] = struct{}{} + } + switch v := typedNode.Value.(type) { + case *ast.Ident: + f.unsafeObjects[v.Obj] = struct{}{} + } + + case *ast.UnaryExpr: + if typedNode.Op == token.AND { + switch ident := typedNode.X.(type) { + case *ast.Ident: + if _, unsafe := f.unsafeObjects[ident.Obj]; unsafe { + f.errorf(ident, "Using a reference for the variable on range scope %s", formatCode(ident.Name, nil)) + } + } + } + + case *ast.Ident: + if _, obj := f.dangerObjects[typedNode.Obj]; obj { + // It is the naked variable in scope of range statement. + f.errorf(node, "Using the variable on range scope %s in function literal", formatCode(typedNode.Name, nil)) + break + } + + case *ast.CallExpr: + // Ignore func literals that'll be called immediately. + switch funcLit := typedNode.Fun.(type) { + case *ast.FuncLit: + f.skipFuncs[funcLit] = struct{}{} + } + + case *ast.FuncLit: + if _, skip := f.skipFuncs[typedNode]; !skip { + dangers := map[*ast.Object]struct{}{} + for d := range f.dangerObjects { + dangers[d] = struct{}{} + } + for u := range f.unsafeObjects { + dangers[u] = struct{}{} + } + return &Node{ + fset: f.fset, + dangerObjects: dangers, + unsafeObjects: f.unsafeObjects, + skipFuncs: f.skipFuncs, + issues: f.issues, + } + } + } + return f +} + +// The variadic arguments may start with link and category types, +// and must end with a format string and any arguments. +// It returns the new Problem. +//nolint:interfacer +func (f *Node) errorf(n ast.Node, format string, args ...interface{}) { + pos := f.fset.Position(n.Pos()) + f.errorfAt(pos, format, args...) +} + +func (f *Node) errorfAt(pos token.Position, format string, args ...interface{}) { + *f.issues = append(*f.issues, result.Issue{ + Pos: pos, + Text: fmt.Sprintf(format, args...), + FromLinter: Scopelint{}.Name(), + }) +} diff --git a/pkg/lint/lintersdb/enabled_set_test.go b/pkg/lint/lintersdb/enabled_set_test.go index de4144ad14ca..bd33dc097f13 100644 --- a/pkg/lint/lintersdb/enabled_set_test.go +++ b/pkg/lint/lintersdb/enabled_set_test.go @@ -83,6 +83,7 @@ func TestGetEnabledLintersSet(t *testing.T) { m := NewManager() es := NewEnabledSet(m, NewValidator(m), nil, nil) for _, c := range cases { + c := c t.Run(c.name, func(t *testing.T) { defaultLinters := []linter.Config{} for _, ln := range c.def { diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index 30dc135351de..94369b14a7a8 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -49,6 +49,7 @@ func (m Manager) GetLinterConfig(name string) *linter.Config { func enableLinterConfigs(lcs []linter.Config, isEnabled func(lc *linter.Config) bool) []linter.Config { var ret []linter.Config for _, lc := range lcs { + lc := lc lc.EnabledByDefault = isEnabled(&lc) ret = append(ret, lc) } @@ -186,6 +187,10 @@ func (Manager) GetAllSupportedLinterConfigs() []linter.Config { WithPresets(linter.PresetPerformance). WithSpeed(8). WithURL("https://github.com/alexkohler/prealloc"), + linter.NewConfig(golinters.Scopelint{}). + WithPresets(linter.PresetBugs). + WithSpeed(8). + WithURL("https://github.com/kyoh86/scopelint"), } isLocalRun := os.Getenv("GOLANGCI_COM_RUN") == "" diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index f67e7c601251..a955075b934c 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -222,6 +222,7 @@ func (r Runner) processLintResults(inCh <-chan lintRes) <-chan lintRes { // finalize processors: logging, clearing, no heavy work here for _, p := range r.Processors { + p := p sw.TrackStage(p.Name(), func() { p.Finish() }) @@ -273,6 +274,7 @@ func (r *Runner) processIssues(issues []result.Issue, sw *timeutils.Stopwatch) [ for _, p := range r.Processors { var newIssues []result.Issue var err error + p := p sw.TrackStage(p.Name(), func() { newIssues, err = p.Process(issues) }) diff --git a/pkg/printers/tab.go b/pkg/printers/tab.go index 51ad0d707b4c..5045d771c335 100644 --- a/pkg/printers/tab.go +++ b/pkg/printers/tab.go @@ -33,6 +33,7 @@ func (p *Tab) Print(ctx context.Context, issues <-chan result.Issue) error { w := tabwriter.NewWriter(logutils.StdOut, 0, 0, 2, ' ', 0) for i := range issues { + i := i p.printIssue(&i, w) } diff --git a/pkg/printers/text.go b/pkg/printers/text.go index 1b2a61db60e0..c0cc5e2c844f 100644 --- a/pkg/printers/text.go +++ b/pkg/printers/text.go @@ -38,6 +38,7 @@ func (p Text) SprintfColored(ca color.Attribute, format string, args ...interfac func (p *Text) Print(ctx context.Context, issues <-chan result.Issue) error { for i := range issues { + i := i p.printIssue(&i) if !p.printIssuedLine { diff --git a/pkg/result/processors/nolint.go b/pkg/result/processors/nolint.go index 87c8702b7479..6bac3956cc46 100644 --- a/pkg/result/processors/nolint.go +++ b/pkg/result/processors/nolint.go @@ -148,6 +148,7 @@ func (e *rangeExpander) Visit(node ast.Node) ast.Visitor { var foundRange *ignoredRange for _, r := range e.inlineRanges { if r.To == nodeStartLine-1 && nodeStartPos.Column == r.col { + r := r foundRange = &r break } diff --git a/pkg/result/processors/nolint_test.go b/pkg/result/processors/nolint_test.go index 6ba0778bda94..69b2231d427a 100644 --- a/pkg/result/processors/nolint_test.go +++ b/pkg/result/processors/nolint_test.go @@ -153,6 +153,7 @@ func TestNolintAliases(t *testing.T) { p := newTestNolintProcessor(getOkLogger(ctrl)) for _, line := range []int{47, 49, 51} { + line := line t.Run(fmt.Sprintf("line-%d", line), func(t *testing.T) { processAssertEmpty(t, p, newNolintFileIssue(line, "gosec")) }) diff --git a/pkg/result/processors/utils.go b/pkg/result/processors/utils.go index 7c636a852a47..9d8ff9343dc0 100644 --- a/pkg/result/processors/utils.go +++ b/pkg/result/processors/utils.go @@ -9,6 +9,7 @@ import ( func filterIssues(issues []result.Issue, filter func(i *result.Issue) bool) []result.Issue { retIssues := make([]result.Issue, 0, len(issues)) for _, i := range issues { + i := i if filter(&i) { retIssues = append(retIssues, i) } @@ -20,6 +21,7 @@ func filterIssues(issues []result.Issue, filter func(i *result.Issue) bool) []re func filterIssuesErr(issues []result.Issue, filter func(i *result.Issue) (bool, error)) ([]result.Issue, error) { retIssues := make([]result.Issue, 0, len(issues)) for _, i := range issues { + i := i ok, err := filter(&i) if err != nil { return nil, fmt.Errorf("can't filter issue %#v: %s", i, err) @@ -36,6 +38,7 @@ func filterIssuesErr(issues []result.Issue, filter func(i *result.Issue) (bool, func transformIssues(issues []result.Issue, transform func(i *result.Issue) *result.Issue) []result.Issue { retIssues := make([]result.Issue, 0, len(issues)) for _, i := range issues { + i := i newI := transform(&i) if newI != nil { retIssues = append(retIssues, *newI) diff --git a/test/run_test.go b/test/run_test.go index 4cb2371e1371..4bd8c6abec55 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -368,6 +368,7 @@ func TestEnabledLinters(t *testing.T) { } for _, c := range cases { + c := c t.Run(c.name, func(t *testing.T) { runArgs := []string{"-v"} if !c.noImplicitFast { diff --git a/test/testdata/scopelint.go b/test/testdata/scopelint.go new file mode 100644 index 000000000000..fdaed5337a40 --- /dev/null +++ b/test/testdata/scopelint.go @@ -0,0 +1,17 @@ +// args: -Escopelint +package testdata + +import "fmt" + +func ScopelintTest() { + values := []string{"a", "b", "c"} + var funcs []func() + for _, val := range values { + funcs = append(funcs, func() { + fmt.Println(val) // ERROR "Using the variable on range scope `val` in function literal" + }) + } + for _, f := range funcs { + f() + } +}