feat: add config to ignore interface (#25)

* feature/ignore-interface-regex

added ignoreInterfaceRegexps option to ignore errors returned from a function call defined on a interface

* Added ReadMe description. Fixed comments and fixed typos

* refactored regexs and globs, changed readme description, plus minor nit picks

Author: guillaumeio

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

wrapcheck/testdata/config_ignoreInterfaceRegexps/.wrapcheck.yaml

@@ -0,0 +1,2 @@
+ignoreInterfaceRegexps:
+- (errorer|error)

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
+}

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
+}