Skip to content

Commit

Permalink
fix #243: support Scopelint linter
Browse files Browse the repository at this point in the history
  • Loading branch information
jirfag committed Nov 6, 2018
1 parent ccac35a commit 84c9c65
Show file tree
Hide file tree
Showing 13 changed files with 176 additions and 1 deletion.
2 changes: 1 addition & 1 deletion Gopkg.lock

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

3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
139 changes: 139 additions & 0 deletions pkg/golinters/scopelint.go
Original file line number Diff line number Diff line change
@@ -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(),
})
}
1 change: 1 addition & 0 deletions pkg/lint/lintersdb/enabled_set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions pkg/lint/lintersdb/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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") == ""
Expand Down
2 changes: 2 additions & 0 deletions pkg/lint/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
Expand Down Expand Up @@ -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)
})
Expand Down
1 change: 1 addition & 0 deletions pkg/printers/tab.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
1 change: 1 addition & 0 deletions pkg/printers/text.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions pkg/result/processors/nolint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
1 change: 1 addition & 0 deletions pkg/result/processors/nolint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
})
Expand Down
3 changes: 3 additions & 0 deletions pkg/result/processors/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
Expand All @@ -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)
Expand Down
1 change: 1 addition & 0 deletions test/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
17 changes: 17 additions & 0 deletions test/testdata/scopelint.go
Original file line number Diff line number Diff line change
@@ -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()
}
}

0 comments on commit 84c9c65

Please sign in to comment.