Add ability to ignore faillint problems (#11)

* Add ability to ignore faillint problems

Linter directive is based on staticcheck.

* Remove reference to staticcheck now that it's more loosely based on staticchecks' linter directives

* Clarify file-ignore placement

* Refactor ignore/fileignore constants

- Rename
- Move
- Comment

* Report missing reasons

* Report out of place file-ignore option

* Add test coverage for hasDirective

* Add valid case coverage to TestHasDirective unit test

* Add test case for variation of package comment

* Loosen restrictions on the placement of the file-ignore directive

* Remove code which can no longer execute

* Switch to staticcheck style directives

.gitignore

@@ -13,3 +13,5 @@
 
 faillint/testdata/pkg
 
+# IDE workspace files
+/.vscode/

README.md

@@ -2,13 +2,13 @@
 
 Faillint is a simple Go linter that fails when a specific set of import paths
 or exported path's functions, constant, vars or types are used. It's meant to be
-used in CI/CD environments to catch rules you want to enforce in your projects. 
+used in CI/CD environments to catch rules you want to enforce in your projects.
 
 As an example, you could enforce the usage of `github.com/pkg/errors` instead
 of the `errors` package. To prevent the usage of the `errors` package, you can
 configure `faillint` to fail whenever someone imports the `errors` package in
-this case. To make sure `fmt.Errorf` is not used for creating errors as well, 
-you can configure to fail on such single function of `fmt` as well. 
+this case. To make sure `fmt.Errorf` is not used for creating errors as well,
+you can configure to fail on such single function of `fmt` as well.
 
 ![faillint](https://user-images.githubusercontent.com/438920/74105802-f7158300-4b15-11ea-8e23-16be5cd3b971.gif)
 
@@ -61,6 +61,61 @@ If you have a preferred import path to suggest, append the suggestion after a `=
 -paths "fmt.{Errorf}=github.com/pkg/errors.{Errorf}"
 ```
 
+### Ignoring problems
+
+If you want to ignore a problem reported by `faillint` you can add a lint directive based on [staticcheck](https://staticcheck.io)'s design.
+
+#### Line-based lint directives
+
+Line-based lint directives can be applied to imports or functions you want to tolerate.  The format is,
+
+```go
+//lint:ignore faillint reason
+```
+
+For example,
+
+```go
+package a
+
+import (
+        //lint:ignore faillint Whatever your reason is.
+        "errors"
+        "fmt" //lint:ignore faillint Whatever your reason is.
+)
+
+func foo() error {
+        //lint:ignore faillint Whatever your reason is.
+        return errors.New("bar!")
+}
+```
+
+#### File-based lint directives
+
+File-based lint directives can be applied to ignore `faillint` problems in a whole file.  The format is,
+
+```go
+//lint:file-ignore faillint reason
+```
+
+This may be placed anywhere in the file but conventionally it should be placed at, or near, the top of the file.
+
+For example,
+
+```go
+//lint:file-ignore faillint This file should be ignored by faillint.
+
+package a
+
+import (
+        "errors"
+)
+
+func foo() error {
+        return errors.New("bar!")
+}
+```
+
 ## Example
 
 Assume we have the following file:
@@ -87,7 +142,7 @@ a.go:4:2: package "errors" shouldn't be imported, suggested: "github.com/pkg/err
 ## The need for this tool?
 
 Most of these checks should be probably detected during the review cycle. But
-it's totally normal to accidentally import them (we're all humans in the end). 
+it's totally normal to accidentally import them (we're all humans in the end).
 
 Second, tools like `goimports` favors certain packages. As an example going
 forward if you decided to use `github.com/pkg/errors` in you project, and write

faillint/faillint.go

@@ -14,6 +14,17 @@ import (
 	"golang.org/x/tools/go/analysis"
 )
 
+const (
+	// ignoreKey is used in a faillint directive to ignore a line-based problem.
+	ignoreKey     = "ignore"
+	// fileIgnoreKey is used in a faillint directive to ignore a whole file.
+	fileIgnoreKey = "file-ignore"
+	// missingReasonTemplate is used when a faillint directive is missing a reason.
+	missingReasonTemplate = "missing reason on faillint:%s directive"
+	// unrecognizedOptionTemplate is used when a faillint directive has an option other than ignore or file-ignore.
+	unrecognizedOptionTemplate = "unrecognized option on faillint directive: %s"
+)
+
 // pathsRegexp represents a regexp that is used to parse -paths flag.
 // It parses flag content in set of 3 subgroups:
 //
@@ -101,13 +112,20 @@ func (f *faillint) run(pass *analysis.Pass) (interface{}, error) {
 		if f.ignoretests && strings.Contains(pass.Fset.File(file.Package).Name(), "_test.go") {
 			continue
 		}
+		if anyHasDirective(pass, file.Comments, fileIgnoreKey) {
+			continue
+		}
+		commentMap := ast.NewCommentMap(pass.Fset, file, file.Comments)
 		for _, path := range parsePaths(f.paths) {
 			specs := importSpec(file, path.imp)
 			if len(specs) == 0 {
 				continue
 			}
 			for _, spec := range specs {
-				usages := importUsages(file, spec)
+				if usageHasDirective(pass, commentMap, spec, spec.Pos(), ignoreKey) {
+					continue
+				}
+				usages := importUsages(pass, commentMap, file, spec)
 				if len(usages) == 0 {
 					continue
 				}
@@ -146,7 +164,7 @@ func (f *faillint) run(pass *analysis.Pass) (interface{}, error) {
 const unspecifiedUsage = "unspecified"
 
 // importUsages reports all exported declaration used for a given import.
-func importUsages(f *ast.File, spec *ast.ImportSpec) map[string][]token.Pos {
+func importUsages(pass *analysis.Pass, commentMap ast.CommentMap, f *ast.File, spec *ast.ImportSpec) map[string][]token.Pos {
 	importRef := spec.Name.String()
 	switch importRef {
 	case "<nil>":
@@ -169,6 +187,9 @@ func importUsages(f *ast.File, spec *ast.ImportSpec) map[string][]token.Pos {
 			return true
 		}
 		if isTopName(sel.X, importRef) {
+			if usageHasDirective(pass, commentMap, n, sel.Sel.NamePos, ignoreKey) {
+				return true
+			}
 			usages[sel.Sel.Name] = append(usages[sel.Sel.Name], sel.Sel.NamePos)
 		}
 		return true
@@ -201,3 +222,73 @@ func isTopName(n ast.Expr, name string) bool {
 	id, ok := n.(*ast.Ident)
 	return ok && id.Name == name && id.Obj == nil
 }
+
+func parseDirective(pass *analysis.Pass, c *ast.Comment) (option string) {
+	s := c.Text
+	if !strings.HasPrefix(s, "//lint:") {
+		return ""
+	}
+	s = strings.TrimPrefix(s, "//lint:")
+	fields := strings.SplitN(s, " ", 3)
+
+	if len(fields) < 2 {
+		return ""
+	}
+
+	if fields[1] != "faillint" {
+		return ""
+	}
+
+	if fields[0] != ignoreKey && fields[0] != fileIgnoreKey {
+		pass.Reportf(c.Pos(), unrecognizedOptionTemplate, fields[0])
+		return ""
+	}
+
+	if len(fields) < 3 {
+		pass.Reportf(c.Pos(), missingReasonTemplate, fields[0])
+		return ""
+	}
+
+	return fields[0]
+}
+
+func anyHasDirective(pass *analysis.Pass, cgs []*ast.CommentGroup, option string) bool {
+	for _, cg := range cgs {
+		if hasDirective(pass, cg, option) {
+			return true
+		}
+	}
+	return false
+}
+
+func hasDirective(pass *analysis.Pass, cg *ast.CommentGroup, option string) bool {
+	if cg == nil {
+		return false
+	}
+	for _, c := range cg.List {
+		if parseDirective(pass, c) == option {
+			return true
+		}
+	}
+	return false
+}
+
+func usageHasDirective(pass *analysis.Pass, cm ast.CommentMap, n ast.Node, p token.Pos, option string) bool {
+	for _, cg := range cm[n] {
+		if hasDirective(pass, cg, ignoreKey) {
+			return true
+		}
+	}
+	// Try to find an "enclosing" node which the ast.CommentMap will
+	// thus have associated comments to this field selector.
+	for node := range cm {
+		if p >= node.Pos() && p <= node.End() {
+			for _, cg := range cm[node] {
+				if hasDirective(pass, cg, option) {
+					return true
+				}
+			}
+		}
+	}
+	return false
+}

faillint/faillint_test.go

@@ -2,6 +2,8 @@ package faillint
 
 import (
 	"fmt"
+	"go/ast"
+	"golang.org/x/tools/go/analysis"
 	"path/filepath"
 	"reflect"
 	"runtime"
@@ -209,6 +211,36 @@ func TestRun(t *testing.T) {
 			dir:   "i",
 			paths: "os.{O_RDONLY,ErrNotExist,File}",
 		},
+		{
+			name:  "unwanted errors package present but it has ignore directive",
+			dir:   "j",
+			paths: "errors,fmt",
+		},
+		{
+			name:  "unwanted errors package present but file has file-ignore directive",
+			dir:   "k",
+			paths: "errors",
+		},
+		{
+			name:  "unwanted errors.New function present but it has ignore directive",
+			dir:   "l",
+			paths: "errors.{New}",
+		},
+		{
+			name:  "multiple unwanted errors.New functions present but one has ignore directive",
+			dir:   "m",
+			paths: "errors.{New}",
+		},
+		{
+			name:  "unwanted errors package present but file has file-ignore directive",
+			dir:   "n",
+			paths: "errors",
+		},
+		{
+			name:  "unwanted errors package present but file has file-ignore directive before package comment",
+			dir:   "o",
+			paths: "errors",
+		},
 	} {
 		t.Run(tcase.name, func(t *testing.T) {
 			f := NewAnalyzer()
@@ -223,3 +255,99 @@ func TestRun(t *testing.T) {
 		})
 	}
 }
+
+func TestHasDirective(t *testing.T) {
+	type input struct {
+		comments []*ast.Comment
+		option   string
+	}
+	type expected struct {
+		out     bool
+		message string
+	}
+	for _, tcase := range []struct {
+		name     string
+		input    input
+		expected expected
+	}{
+		{
+			name: "missing reason on ignore",
+			input: input{
+				comments: []*ast.Comment{
+					{Text: "//lint:ignore faillint"},
+				},
+				option: ignoreKey,
+			},
+			expected: expected{
+				out:     false,
+				message: fmt.Sprintf(missingReasonTemplate, "ignore"),
+			},
+		},
+		{
+			name: "missing reason on file-ignore",
+			input: input{
+				comments: []*ast.Comment{
+					{Text: "//lint:file-ignore faillint"},
+				},
+				option: fileIgnoreKey,
+			},
+			expected: expected{
+				out:     false,
+				message: fmt.Sprintf(missingReasonTemplate, "file-ignore"),
+			},
+		},
+		{
+			name: "valid ignore",
+			input: input{
+				comments: []*ast.Comment{
+					{Text: "//lint:ignore faillint reason"},
+				},
+				option: ignoreKey,
+			},
+			expected: expected{
+				out: true,
+			},
+		},
+		{
+			name: "valid file-ignore",
+			input: input{
+				comments: []*ast.Comment{
+					{Text: "//lint:file-ignore faillint reason"},
+				},
+				option: fileIgnoreKey,
+			},
+			expected: expected{
+				out: true,
+			},
+		},
+		{
+			name: "invalid option on faillint directive",
+			input: input{
+				comments: []*ast.Comment{
+					{Text: "//lint:foo faillint reason"},
+				},
+				option: ignoreKey,
+			},
+			expected: expected{
+				out:     false,
+				message: fmt.Sprintf(unrecognizedOptionTemplate, "foo"),
+			},
+		},
+	} {
+		t.Run(tcase.name, func(t *testing.T) {
+			var diagnostic analysis.Diagnostic
+			pass := analysis.Pass{
+				Report: func(d analysis.Diagnostic) {
+					diagnostic = d
+				},
+			}
+			got := hasDirective(&pass, &ast.CommentGroup{List: tcase.input.comments}, tcase.input.option)
+			if got != tcase.expected.out {
+				t.Errorf("expected hasDirective to return %v, got %v", tcase.expected.out, got)
+			}
+			if diagnostic.Message != tcase.expected.message {
+				t.Errorf("expected diagnostic message: `%s`, got: `%s`", tcase.expected.message, diagnostic.Message)
+			}
+		})
+	}
+}

faillint/testdata/src/j/j.go

@@ -0,0 +1,11 @@
+package j
+
+import (
+	//lint:ignore faillint tolerate this errors import
+	"errors"
+	"fmt" //lint:ignore faillint tolerate this fmt import
+)
+
+func foo() error {
+	return errors.New(fmt.Sprintf("%s!", "bar"))
+}

faillint/testdata/src/k/k.go

@@ -0,0 +1,10 @@
+//lint:file-ignore faillint ignore faillint in this file
+package k
+
+import (
+	"errors"
+)
+
+func foo() error {
+	return errors.New("bar!")
+}