Feature/ignore interface regex #25
Conversation
added ignoreInterfaceRegexps option to ignore errors returned from a function call defined on a interface
| var str string | ||
| err := fn.Decode(&str) | ||
| if err != nil { | ||
| return err // errorer interface ignored |
There was a problem hiding this comment.
Just a minor suggestion
| return err // errorer interface ignored | |
| return err // errorer interface ignored as per `ignoreInterfaceRegexps` |
README.md
Outdated
| @@ -52,6 +52,13 @@ ignoreSigRegexps: | |||
| ignorePackageGlobs: | |||
| - encoding/* | |||
| - github.com/pkg/* | |||
|
|
|||
| # An array of strings which specify regular expressions of interface names. | |||
There was a problem hiding this comment.
Maybe we could make this a little more succinct. I liked the description on the config struct more, maybe we could use that here.
// 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.
wrapcheck/wrapcheck.go
Outdated
| @@ -91,11 +101,15 @@ 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) | |||
| ignoreSigRegexp, err1 := compileRegexps(cfg.IgnoreSigRegexps) | |||
| ignoreInterfaceRegexps, err2 := compileRegexps(cfg.IgnoreInterfaceRegexps) | |||
There was a problem hiding this comment.
I was thinking, I've actually made some similar changes, still local for now, in order to precompile the globs. Could we maybe update this to something along the lines of:
var (
ignoreSigRegexp []*regexp.Regexp
ignorePackageGlobs []glob.Glob
err error
)
ignoreSigRegexp, err = compileRegexps(cfg.IgnoreSigRegexps)
if err != nil {
ignorePackageGlobs, err = compileGlobs(cfg.IgnorePackageGlobs)
}I will likely refactor this anyway in an upcoming PR, but for now I'd prefer not to have multiple error checks in the returned func below.
wrapcheck/wrapcheck.go
Outdated
| 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. | ||
| if isInterface(pass, sel) { | ||
| pass.Reportf(tokenPos, "error returned from interface method should be wrapped: sig: %s", fnSig) | ||
| return | ||
| if containsMatch(regexpsInter, types.TypeString(pass.TypesInfo.TypeOf(sel.X), func(p *types.Package) string { return p.Name() })) { |
There was a problem hiding this comment.
Could we maybe pull this out of a single line? It's a little dense to read as it is and could be simpler.
|
Thanks for the review! |
|
Thanks for the kind words! |
Hi Tom!
This PR intends to bring flexibility to wrapcheck with errors returned from Interfaces types:
Allows ignore unwrapped errors returned from interfaces. Define regexps rules to explicitly ignore underlying interfaces with mandated methods to ignore, according to the interface names. Even though this could technically be done with thought-out ignoreSigRegexps, this allows to explicitly and cleanly define rules in the config to define sub cases of IsInterface.
PS: fixed typos