Skip to content

Commit

Permalink
Merge pull request #8 from bwplotka/issue7
Browse files Browse the repository at this point in the history
Added support for unwanted function; added more tests.
  • Loading branch information
fatih authored Mar 7, 2020
2 parents 96e0241 + 1112ee0 commit 7f19c3a
Show file tree
Hide file tree
Showing 23 changed files with 484 additions and 158 deletions.
32 changes: 19 additions & 13 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
# faillint [![](https://github.com/fatih/faillint/workflows/build/badge.svg)](https://github.com/fatih/faillint/actions)

Faillint is a simple Go linter that fails when a specific set of import paths
are used. It's meant to be used in CI/CD environments to catch rules you want
to enforce in your projects.
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.

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.
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)

Expand All @@ -22,37 +23,42 @@ go get github.com/fatih/faillint
`faillint` works on a file, directory or a Go package:

```sh
$ faillint -paths "errors" foo.go # pass a file
$ faillint -paths "errors" ./... # recursively analyze all files
$ faillint -paths "errors" github.com/fatih/gomodifytags # or pass a package
$ faillint -paths "errors,fmt.{Errorf}" foo.go # pass a file
$ faillint -paths "errors,fmt.{Errorf}" ./... # recursively analyze all files
$ faillint -paths "errors,fmt.{Errorf}" github.com/fatih/gomodifytags # or pass a package
```

By default, `faillint` will not check any import paths. You need to explicitly
define it with the `-paths` flag, which is comma-separated list. Some examples are:

```
# fail if the errors package is used
# Fail if the errors package is used.
-paths "errors"
# fail if the old context package is imported
# Fail if the old context package is imported.
-paths "golang.org/x/net/context"
# fail both on stdlib log and errors package to enforce other internal libraries
# Fail both on stdlib log and errors package to enforce other internal libraries.
-paths "log,errors"
```
# Fail if any of Print, Printf of Println function were used from fmt library.
-paths "fmt.{Print,Printf,Println}"
```

If you have a preferred import path to suggest, append the suggestion after a `=` character:

```
# fail if the errors package is used and suggest to use github.com/pkg/errors
# Fail if the errors package is used and suggest to use github.com/pkg/errors.
-paths "errors=github.com/pkg/errors"
# fail for the old context import path and suggest to use the stdlib context
# Fail for the old context import path and suggest to use the stdlib context.
-paths "golang.org/x/net/context=context"
# fail both on stdlib log and errors package to enforce other libraries
# Fail both on stdlib log and errors package to enforce other libraries.
-paths "log=go.uber.org/zap,errors=github.com/pkg/errors"
# Fail on fmt.Errorf and suggest the Errorf function from github.compkg/errors instead.
-paths "fmt.{Errorf}=github.com/pkg/errors.{Errorf}"
```

## Example
Expand Down
177 changes: 114 additions & 63 deletions faillint/faillint.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,134 +5,185 @@ package faillint
import (
"fmt"
"go/ast"
"go/token"
"regexp"
"strconv"
"strings"
"unicode"

"golang.org/x/tools/go/analysis"
)

// pathsRegexp represents a regexp that is used to parse -paths flag.
// It parses flag content in set of 3 subgroups:
//
// * import: Mandatory part. Go import path in URL format to be unwanted or have unwanted declarations.
// * declarations: Optional declarations in `{ }`. If set, using the import is allowed expect give declarations.
// * suggestion: Optional suggestion to print when unwanted import or declaration is found.
//
var pathsRegexp = regexp.MustCompile(`(?P<import>[\w/.-]+[\w])(\.?{(?P<declarations>[\w-,]+)}|)(=(?P<suggestion>[\w/.-]+[\w](\.?{[\w-,]+}|))|)`)

type faillint struct {
paths string // -paths flag
ignoretests bool // -ignore-tests flag
}

// Analyzer global instance of the linter (if possible use NewAnalyzer)
// Analyzer is a global instance of the linter.
// DEPRECATED: Use faillint.New instead.
var Analyzer = NewAnalyzer()

// NewAnalyzer create a faillint analyzer
// NewAnalyzer create a faillint analyzer.
func NewAnalyzer() *analysis.Analyzer {
f := faillint{
paths: "",
ignoretests: false,
}
a := &analysis.Analyzer{
Name: "faillint",
Doc: "report unwanted import path usages",
Doc: "Report unwanted import path or exported declaration usages",
Run: f.run,
RunDespiteErrors: true,
}

a.Flags.StringVar(&f.paths, "paths", "", "import paths to fail")
a.Flags.StringVar(&f.paths, "paths", "", `import paths or exported declarations (i.e: functions, constant, types or variables) to fail.
E.g. errors=github.com/pkg/errors,fmt.{Errorf}=github.com/pkg/errors.{Errorf},fmt.{Println,Print,Printf},github.com/prometheus/client_golang/prometheus.{DefaultGatherer,MustRegister}`)
a.Flags.BoolVar(&f.ignoretests, "ignore-tests", false, "ignore all _test.go files")
return a
}

// Run is the runner for an analysis pass
func (f *faillint) run(pass *analysis.Pass) (interface{}, error) {
if f.paths == "" {
return nil, nil
func trimAllWhitespaces(str string) string {
var b strings.Builder
b.Grow(len(str))
for _, ch := range str {
if !unicode.IsSpace(ch) {
b.WriteRune(ch)
}
}
return b.String()
}

p := strings.Split(f.paths, ",")

suggestions := make(map[string]string, len(p))
imports := make([]string, 0, len(p))

for _, s := range p {
imps := strings.Split(s, "=")
type path struct {
imp string
decls []string
sugg string
}

imp := imps[0]
suggest := ""
if len(imps) == 2 {
suggest = imps[1]
func parsePaths(paths string) []path {
pathGroups := pathsRegexp.FindAllStringSubmatch(trimAllWhitespaces(paths), -1)

parsed := make([]path, 0, len(pathGroups))
for _, group := range pathGroups {
p := path{}
for i, name := range pathsRegexp.SubexpNames() {
switch name {
case "import":
p.imp = group[i]
case "suggestion":
p.sugg = group[i]
case "declarations":
if group[i] == "" {
break
}
p.decls = strings.Split(group[i], ",")
}
}

imports = append(imports, imp)
suggestions[imp] = suggest
parsed = append(parsed, p)
}
return parsed
}

// run is the runner for an analysis pass.
func (f *faillint) run(pass *analysis.Pass) (interface{}, error) {
if f.paths == "" {
return nil, nil
}
for _, file := range pass.Files {
if f.ignoretests && strings.Contains(pass.Fset.File(file.Package).Name(), "_test.go") {
continue
}
for _, path := range imports {
imp := usesImport(file, path)
if imp == nil {
for _, path := range parsePaths(f.paths) {
specs := importSpec(file, path.imp)
if len(specs) == 0 {
continue
}

impPath := importPath(imp)

msg := fmt.Sprintf("package %q shouldn't be imported", impPath)
if s := suggestions[impPath]; s != "" {
msg += fmt.Sprintf(", suggested: %q", s)
for _, spec := range specs {
usages := importUsages(file, spec)
if len(usages) == 0 {
continue
}

if _, ok := usages[unspecifiedUsage]; ok || len(path.decls) == 0 {
// File using unwanted import. Report.
msg := fmt.Sprintf("package %q shouldn't be imported", importPath(spec))
if path.sugg != "" {
msg += fmt.Sprintf(", suggested: %q", path.sugg)
}
pass.Reportf(spec.Path.Pos(), msg)
continue
}

// Not all usages are forbidden. Report only unwanted declarations.
for _, declaration := range path.decls {
positions, ok := usages[declaration]
if !ok {
continue
}
msg := fmt.Sprintf("declaration %q from package %q shouldn't be used", declaration, importPath(spec))
if path.sugg != "" {
msg += fmt.Sprintf(", suggested: %q", path.sugg)
}
for _, pos := range positions {
pass.Reportf(pos, msg)
}
}
}

pass.Reportf(imp.Path.Pos(), msg)
}
}

return nil, nil
}

// usesImport reports whether a given import is used.
func usesImport(f *ast.File, path string) *ast.ImportSpec {
spec := importSpec(f, path)
if spec == nil {
return nil
}
const unspecifiedUsage = "unspecified"

name := spec.Name.String()
switch name {
// importUsages reports all exported declaration used for a given import.
func importUsages(f *ast.File, spec *ast.ImportSpec) map[string][]token.Pos {
importRef := spec.Name.String()
switch importRef {
case "<nil>":
// If the package name is not explicitly specified,
importRef, _ = strconv.Unquote(spec.Path.Value)
// If the package importRef is not explicitly specified,
// make an educated guess. This is not guaranteed to be correct.
lastSlash := strings.LastIndex(path, "/")
if lastSlash == -1 {
name = path
} else {
name = path[lastSlash+1:]
lastSlash := strings.LastIndex(importRef, "/")
if lastSlash != -1 {
importRef = importRef[lastSlash+1:]
}
case "_", ".":
// Not sure if this import is used - err on the side of caution.
return spec
// Not sure if this import is used - on the side of caution, report special "unspecified" usage.
return map[string][]token.Pos{unspecifiedUsage: nil}
}
usages := map[string][]token.Pos{}

var used bool
ast.Inspect(f, func(n ast.Node) bool {
sel, ok := n.(*ast.SelectorExpr)
if ok && isTopName(sel.X, name) {
used = true
if !ok {
return true
}
if isTopName(sel.X, importRef) {
usages[sel.Sel.Name] = append(usages[sel.Sel.Name], sel.Sel.NamePos)
}
return true
})

if used {
return spec
}

return nil
return usages
}

// importSpec returns the import spec if f imports path,
// or nil otherwise.
func importSpec(f *ast.File, path string) *ast.ImportSpec {
// importSpecs returns all import specs for f import statements importing path.
func importSpec(f *ast.File, path string) (imports []*ast.ImportSpec) {
for _, s := range f.Imports {
if importPath(s) == path {
return s
imports = append(imports, s)
}
}
return nil
return imports
}

// importPath returns the unquoted import path of s,
Expand Down
Loading

0 comments on commit 7f19c3a

Please sign in to comment.