Skip to content
This repository has been archived by the owner on May 25, 2021. It is now read-only.

Commit

Permalink
Support "ignore" comment (#8)
Browse files Browse the repository at this point in the history
* addition: update go.sum

* wip: update manual to implement

* wip: create model and test

* wip: create option comment parser

* addition: change receiver name

* wip: get comment map

* implemented

* skip printing ignored issue
  • Loading branch information
kyoh86 authored Nov 9, 2019
1 parent 60dd71c commit c2a9a45
Show file tree
Hide file tree
Showing 8 changed files with 358 additions and 42 deletions.
39 changes: 39 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,40 @@ And also, scopelint supports the following options:
* The `--vendor` flag enables checking in the `vendor` directories (if you DO NOT it, set `--no-vendor` flag)
* The `--test` flag enables checking in the `*_test.go` files" (if you DO NOT it, set `--no-test` flag)

### Nolint

To ignore issues from use an option comment like //scopelint:ignore.
For example, if it's used inline (not from the beginning of the line), it ignores issues only for the line.

```go
var copies []*string
for _, val := range values {
copies = append(copies, &val) //scopelint:ignore
}
```

To ignore issues for the block of code put it on the beginning of a line before the block:

```go
var copies []*string
//scopelint:ignore
for _, val := range values {
copies = append(copies, &val)
}
```

Also, you can exclude all issues in a file by:

```go
//scopelint:ignore
package pkg
```

You may add a comment explaining or justifying why //scopelint:ignore is being used on the same line as the flag itself:

```go
//scopelint:ignore // any comment
```

### Use with gometalinter

Expand All @@ -106,6 +140,11 @@ scopelint can be used with [gometalinter](https://github.com/alecthomas/gometali
scopelint returns 1 if any problems were found in the checked files.
It returns 2 if there were any other failures.

## Known Issues

- False positive for for table tests [#4](https://github.com/kyoh86/scopelint/issues/4)
- I won't fix it because scopelint supports ignore them all. [=> nolint](#Nolint)

## TODO

- Write tests
Expand Down
1 change: 1 addition & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc h1:cAKDfWh5Vpd
github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf h1:qet1QNfXsQxTZqLG4oE62mJzwPIB8+Tee4RNCL9ulrY=
github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/mattn/go-isatty v0.0.6 h1:SrwhHcpV4nWrMGdNcC2kXpMfcBVYGDuTArqyhocJgvA=
github.com/mattn/go-isatty v0.0.6/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s=
Expand Down
3 changes: 3 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ func lintFiles(filenames ...string) {
return
}
for _, p := range ps {
if p.Ignored {
continue
}
fmt.Printf("%v: %s\n", p.Position, p.Text)
problems++
}
Expand Down
93 changes: 51 additions & 42 deletions scopelint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,12 @@ func (l *Linter) LintFiles(files map[string][]byte) ([]Problem, error) {
return nil, fmt.Errorf("%s is in package %s, not %s", filename, astFile.Name.Name, pkgName)
}
pkg.Files[filename] = &File{
Package: pkg,
ASTFile: astFile,
FileSet: pkg.FileSet,
Source: src,
Filename: filename,
Package: pkg,
ASTFile: astFile,
FileSet: pkg.FileSet,
Source: src,
Filename: filename,
CommentMap: ast.NewCommentMap(pkg.FileSet, astFile, astFile.Comments),
}
}
return pkg.lint(), nil
Expand Down Expand Up @@ -76,11 +77,12 @@ func (p *Package) lint() []Problem {

// File represents a File being linted.
type File struct {
Package *Package
ASTFile *ast.File
FileSet *token.FileSet
Source []byte
Filename string
Package *Package
ASTFile *ast.File
FileSet *token.FileSet
Source []byte
Filename string
CommentMap ast.CommentMap
}

func (f *File) lint() {
Expand All @@ -98,20 +100,34 @@ type Node struct {
DangerObjects map[*ast.Object]int
UnsafeObjects map[*ast.Object]int
SkipFuncs map[*ast.FuncLit]int
Ignore bool
}

// 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).
func (f *Node) Visit(node ast.Node) ast.Visitor {
func (n *Node) Visit(node ast.Node) ast.Visitor {
next := *n
if node == nil {
return &next
}
CGS_LOOP:
for _, cg := range n.File.CommentMap[node] {
for _, com := range cg.List {
if hasOptionComment(com.Text, "ignore") {
next.Ignore = true
break CGS_LOOP
}
}
}
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] = 0
n.UnsafeObjects[tlh.Obj] = 0
}
}
}
Expand All @@ -120,73 +136,65 @@ func (f *Node) Visit(node ast.Node) ast.Visitor {
// Memory variables declarated in range statement
switch k := typedNode.Key.(type) {
case *ast.Ident:
f.UnsafeObjects[k.Obj] = 0
n.UnsafeObjects[k.Obj] = 0
}
switch v := typedNode.Value.(type) {
case *ast.Ident:
f.UnsafeObjects[v.Obj] = 0
n.UnsafeObjects[v.Obj] = 0
}

case *ast.UnaryExpr:
if typedNode.Op == token.AND {
switch ident := typedNode.X.(type) {
case *ast.Ident:
if _, unsafe := f.UnsafeObjects[ident.Obj]; unsafe {
if _, unsafe := n.UnsafeObjects[ident.Obj]; unsafe {
ref := ""
f.errorf(ident, 1, link(ref), category("range-scope"), "Using a reference for the variable on range scope %q", ident.Name)
n.errorf(ident, 1, n.Ignore, link(ref), category("range-scope"), "Using a reference for the variable on range scope %q", ident.Name)
}
}
}

case *ast.Ident:
if _, obj := f.DangerObjects[typedNode.Obj]; obj {
if _, obj := n.DangerObjects[typedNode.Obj]; obj {
// It is the naked variable in scope of range statement.
ref := ""
f.errorf(node, 1, link(ref), category("range-scope"), "Using the variable on range scope %q in function literal", typedNode.Name)
n.errorf(node, 1, n.Ignore, link(ref), category("range-scope"), "Using the variable on range scope %q in function literal", typedNode.Name)
break
}

case *ast.CallExpr:
// Ignore func literals that'll be called immediately.
switch funcLit := typedNode.Fun.(type) {
case *ast.FuncLit:
f.SkipFuncs[funcLit] = 0
n.SkipFuncs[funcLit] = 0
}

case *ast.FuncLit:
if _, skip := f.SkipFuncs[typedNode]; !skip {
if _, skip := n.SkipFuncs[typedNode]; !skip {
dangers := map[*ast.Object]int{}
for d := range f.DangerObjects {
for d := range n.DangerObjects {
dangers[d] = 0
}
for u := range f.UnsafeObjects {
for u := range n.UnsafeObjects {
dangers[u] = 0
f.UnsafeObjects[u]++
}
return &Node{
File: f.File,
DangerObjects: dangers,
UnsafeObjects: f.UnsafeObjects,
SkipFuncs: f.SkipFuncs,
n.UnsafeObjects[u]++
}
next.DangerObjects = dangers
return &next
}

case *ast.ReturnStmt:
unsafe := map[*ast.Object]int{}
for u := range f.UnsafeObjects {
if f.UnsafeObjects[u] == 0 {
for u := range n.UnsafeObjects {
if n.UnsafeObjects[u] == 0 {
continue
}
unsafe[u] = f.UnsafeObjects[u]
}
return &Node{
File: f.File,
DangerObjects: f.DangerObjects,
UnsafeObjects: unsafe,
SkipFuncs: f.SkipFuncs,
unsafe[u] = n.UnsafeObjects[u]
}
next.UnsafeObjects = unsafe
return &next
}
return f
return &next
}

type link string
Expand All @@ -195,18 +203,19 @@ type category string
// 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.
func (f *File) errorf(n ast.Node, confidence float64, args ...interface{}) *Problem {
func (f *File) errorf(n ast.Node, confidence float64, ignore bool, args ...interface{}) *Problem {
pos := f.FileSet.Position(n.Pos())
if pos.Filename == "" {
pos.Filename = f.Filename
}
return f.Package.errorfAt(pos, confidence, args...)
return f.Package.errorfAt(pos, confidence, ignore, args...)
}

func (p *Package) errorfAt(pos token.Position, confidence float64, args ...interface{}) *Problem {
func (p *Package) errorfAt(pos token.Position, confidence float64, ignore bool, args ...interface{}) *Problem {
problem := Problem{
Position: pos,
Confidence: confidence,
Ignored: ignore,
}
if pos.Filename != "" {
// The file might not exist in our mapping if a //line directive was encountered.
Expand Down
Loading

0 comments on commit c2a9a45

Please sign in to comment.