Skip to content

Commit

Permalink
feat: add config to ignore interface (#25)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
guillaumeio committed Mar 23, 2022
1 parent fab0c41 commit 624a3b4
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 30 deletions.
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
}

0 comments on commit 624a3b4

Please sign in to comment.