diff --git a/README.md b/README.md index c9a6840..d668c52 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/wrapcheck/testdata/config_ignoreInterfaceRegexps/.wrapcheck.yaml b/wrapcheck/testdata/config_ignoreInterfaceRegexps/.wrapcheck.yaml new file mode 100644 index 0000000..425fc92 --- /dev/null +++ b/wrapcheck/testdata/config_ignoreInterfaceRegexps/.wrapcheck.yaml @@ -0,0 +1,2 @@ +ignoreInterfaceRegexps: +- (errorer|error) \ No newline at end of file diff --git a/wrapcheck/testdata/config_ignoreInterfaceRegexps/main.go b/wrapcheck/testdata/config_ignoreInterfaceRegexps/main.go new file mode 100644 index 0000000..d0b06ff --- /dev/null +++ b/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 +} diff --git a/wrapcheck/wrapcheck.go b/wrapcheck/wrapcheck.go index 8a63105..3d492ee 100644 --- a/wrapcheck/wrapcheck.go +++ b/wrapcheck/wrapcheck.go @@ -5,8 +5,6 @@ import ( "go/ast" "go/token" "go/types" - "log" - "os" "regexp" "strings" @@ -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"` @@ -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{}, } } @@ -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 { @@ -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 } @@ -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 } } @@ -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 @@ -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 @@ -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 } @@ -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 @@ -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 { @@ -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 +}