Skip to content

fix #243: support Scopelint linter #274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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()
}
}