Skip to content
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

Feature/ignore interface regex #25

Merged
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions README.md
Expand Up @@ -52,6 +52,12 @@ ignoreSigRegexps:
ignorePackageGlobs:
- encoding/*
- github.com/pkg/*

# ignoreInterfaceRegexps defines a list of regular expressions which, if matched
# to a underlying interface name, will ignore unwrapped errors returned from a
# function whose call is defined on the given interface.
ignoreInterfaceRegexps:
- ^(?i)c(?-i)ach(ing|e)
```

## Usage
Expand Down
@@ -0,0 +1,2 @@
ignoreInterfaceRegexps:
- (errorer|error)
25 changes: 25 additions & 0 deletions wrapcheck/testdata/config_ignoreInterfaceRegexps/main.go
@@ -0,0 +1,25 @@
package main

import (
"encoding/json"
"strings"
)

type errorer interface {
Decode(v interface{}) error
}

func main() {
d := json.NewDecoder(strings.NewReader("hello world"))
do(d)
}

func do(fn errorer) error {
var str string
err := fn.Decode(&str)
if err != nil {
return err // errorer interface ignored as per `ignoreInterfaceRegexps`
}

return nil
}
102 changes: 72 additions & 30 deletions wrapcheck/wrapcheck.go
Expand Up @@ -5,8 +5,6 @@ import (
"go/ast"
"go/token"
"go/types"
"log"
"os"
"regexp"
"strings"

Expand Down Expand Up @@ -51,11 +49,11 @@ type WrapcheckConfig struct {
// unwrapped.
//
// For example, an ignoreSigRegexp of `[]string{"\.New.*Err\("}`` will ignore errors
// returned from any signture whose method name starts with "New" and ends with "Err"
// returned from any signature whose method name starts with "New" and ends with "Err"
// due to the signature matching the regular expression `\.New.*Err\(`.
//
// Note that this is similar to the ignoreSigs configuration, but provides
// slightly more flexibility in defining rules by which signtures will be
// slightly more flexibility in defining rules by which signatures will be
// ignored.
IgnoreSigRegexps []string `mapstructure:"ignoreSigRegexps" yaml:"ignoreSigRegexps"`

Expand All @@ -71,13 +69,23 @@ type WrapcheckConfig struct {
// ignorePackageGlobs:
// - encoding/*
IgnorePackageGlobs []string `mapstructure:"ignorePackageGlobs" yaml:"ignorePackageGlobs"`

// IgnoreInterfaceRegexps defines a list of regular expressions which, if matched
// to a underlying interface name, will ignore unwrapped errors returned from a
// function whose call is defined on the given interface.
//
// For example, an ignoreInterfaceRegexps of `[]string{"Transac(tor|tion)"}`` will ignore errors
// returned from any function whose call is defined on a interface named 'Transactor'
// or 'Transaction' due to the name matching the regular expression `Transac(tor|tion)`.
IgnoreInterfaceRegexps []string `mapstructure:"ignoreInterfaceRegexps" yaml:"ignoreInterfaceRegexps"`
}

func NewDefaultConfig() WrapcheckConfig {
return WrapcheckConfig{
IgnoreSigs: DefaultIgnoreSigs,
IgnoreSigRegexps: []string{},
IgnorePackageGlobs: []string{},
IgnoreSigs: DefaultIgnoreSigs,
IgnoreSigRegexps: []string{},
IgnorePackageGlobs: []string{},
IgnoreInterfaceRegexps: []string{},
}
}

Expand All @@ -91,7 +99,21 @@ func NewAnalyzer(cfg WrapcheckConfig) *analysis.Analyzer {

func run(cfg WrapcheckConfig) func(*analysis.Pass) (interface{}, error) {
// Precompile the regexps, report the error
ignoreSigRegexp, err := compileRegexps(cfg.IgnoreSigRegexps)
var (
ignoreSigRegexp []*regexp.Regexp
ignoreInterfaceRegexps []*regexp.Regexp
ignorePackageGlobs []glob.Glob
err error
)

ignoreSigRegexp, err = compileRegexps(cfg.IgnoreSigRegexps)
if err == nil {
ignoreInterfaceRegexps, err = compileRegexps(cfg.IgnoreInterfaceRegexps)
}
if err == nil {
ignorePackageGlobs, err = compileGlobs(cfg.IgnorePackageGlobs)

}

return func(pass *analysis.Pass) (interface{}, error) {
if err != nil {
Expand Down Expand Up @@ -120,7 +142,7 @@ func run(cfg WrapcheckConfig) func(*analysis.Pass) (interface{}, error) {
// tuple check is required.

if isError(pass.TypesInfo.TypeOf(expr)) {
reportUnwrapped(pass, retFn, retFn.Pos(), cfg, ignoreSigRegexp)
reportUnwrapped(pass, retFn, retFn.Pos(), cfg, ignoreSigRegexp, ignoreInterfaceRegexps, ignorePackageGlobs)
return true
}

Expand All @@ -138,7 +160,7 @@ func run(cfg WrapcheckConfig) func(*analysis.Pass) (interface{}, error) {
return true
}
if isError(v.Type()) {
reportUnwrapped(pass, retFn, expr.Pos(), cfg, ignoreSigRegexp)
reportUnwrapped(pass, retFn, expr.Pos(), cfg, ignoreSigRegexp, ignoreInterfaceRegexps, ignorePackageGlobs)
return true
}
}
Expand Down Expand Up @@ -200,7 +222,7 @@ func run(cfg WrapcheckConfig) func(*analysis.Pass) (interface{}, error) {
return true
}

reportUnwrapped(pass, call, ident.NamePos, cfg, ignoreSigRegexp)
reportUnwrapped(pass, call, ident.NamePos, cfg, ignoreSigRegexp, ignoreInterfaceRegexps, ignorePackageGlobs)
}

return true
Expand All @@ -213,7 +235,7 @@ func run(cfg WrapcheckConfig) func(*analysis.Pass) (interface{}, error) {

// Report unwrapped takes a call expression and an identifier and reports
// if the call is unwrapped.
func reportUnwrapped(pass *analysis.Pass, call *ast.CallExpr, tokenPos token.Pos, cfg WrapcheckConfig, regexps []*regexp.Regexp) {
func reportUnwrapped(pass *analysis.Pass, call *ast.CallExpr, tokenPos token.Pos, cfg WrapcheckConfig, regexpsSig []*regexp.Regexp, regexpsInter []*regexp.Regexp, pkgGlobs []glob.Glob) {
sel, ok := call.Fun.(*ast.SelectorExpr)
if !ok {
return
Expand All @@ -224,21 +246,26 @@ func reportUnwrapped(pass *analysis.Pass, call *ast.CallExpr, tokenPos token.Pos

if contains(cfg.IgnoreSigs, fnSig) {
return
} else if containsMatch(regexps, fnSig) {
} else if containsMatch(regexpsSig, fnSig) {
return
}

// Check if the underlying type of the "x" in x.y.z is an interface, as
// errors returned from interface types should be wrapped.
// errors returned from interface types should be wrapped, unless ignored
// as per `ignoreInterfaceRegexps`
if isInterface(pass, sel) {
pass.Reportf(tokenPos, "error returned from interface method should be wrapped: sig: %s", fnSig)
return
name := types.TypeString(pass.TypesInfo.TypeOf(sel.X), func(p *types.Package) string { return p.Name() })
if containsMatch(regexpsInter, name) {
} else {
pass.Reportf(tokenPos, "error returned from interface method should be wrapped: sig: %s", fnSig)
return
}
}

// Check whether the function being called comes from another package,
// as functions called across package boundaries which returns errors
// should be wrapped
if isFromOtherPkg(pass, sel, cfg) {
if isFromOtherPkg(pass, sel, pkgGlobs) {
pass.Reportf(tokenPos, "error returned from external package is unwrapped: sig: %s", fnSig)
return
}
Expand All @@ -251,23 +278,14 @@ func isInterface(pass *analysis.Pass, sel *ast.SelectorExpr) bool {
return ok
}

// isFromotherPkg returns whether the function is defined in the pacakge
// isFromotherPkg returns whether the function is defined in the package
// currently under analysis or is considered external. It will ignore packages
// defined in config.IgnorePackageGlobs.
func isFromOtherPkg(pass *analysis.Pass, sel *ast.SelectorExpr, config WrapcheckConfig) bool {
func isFromOtherPkg(pass *analysis.Pass, sel *ast.SelectorExpr, pkgGlobs []glob.Glob) bool {
// The package of the function that we are calling which returns the error
fn := pass.TypesInfo.ObjectOf(sel.Sel)

for _, globString := range config.IgnorePackageGlobs {
g, err := glob.Compile(globString)
if err != nil {
log.Printf("unable to parse glob: %s\n", globString)
os.Exit(1)
}

if g.Match(fn.Pkg().Path()) {
return false
}
if containsMatchGlob(pkgGlobs, fn.Pkg().Path()) {
return false
}

// If it's not a package name, then we should check the selector to make sure
Expand Down Expand Up @@ -350,6 +368,15 @@ func containsMatch(regexps []*regexp.Regexp, el string) bool {
return false
}

func containsMatchGlob(globs []glob.Glob, el string) bool {
for _, g := range globs {
if g.Match(el) {
return true
}
}
return false
}

// isError returns whether or not the provided type interface is an error
func isError(typ types.Type) bool {
if typ == nil {
Expand Down Expand Up @@ -384,3 +411,18 @@ func compileRegexps(regexps []string) ([]*regexp.Regexp, error) {

return compiledRegexps, nil
}

// compileGlobs compiles a set of globs, returning them for use,
// or the first encountered error due to an invalid expression.
func compileGlobs(globs []string) ([]glob.Glob, error) {
var compiledGlobs []glob.Glob
for _, globString := range globs {
glob, err := glob.Compile(globString)
if err != nil {
return nil, fmt.Errorf("unable to compile globs %s: %v\n", glob, err)
}

compiledGlobs = append(compiledGlobs, glob)
}
return compiledGlobs, nil
}