diff --git a/README.md b/README.md index 9f4887e..ef26bbc 100644 --- a/README.md +++ b/README.md @@ -68,6 +68,10 @@ ignorePackageGlobs: # function whose call is defined on the given interface. ignoreInterfaceRegexps: - ^(?i)c(?-i)ach(ing|e) + +# ReportInternalErrors determines whether wrapcheck should report errors returned +# from inside the package. +reportInternalErrors: true ``` ## Usage diff --git a/wrapcheck/testdata/config_reportInternalErrors/.wrapcheck.yaml b/wrapcheck/testdata/config_reportInternalErrors/.wrapcheck.yaml new file mode 100644 index 0000000..0e52581 --- /dev/null +++ b/wrapcheck/testdata/config_reportInternalErrors/.wrapcheck.yaml @@ -0,0 +1,12 @@ +reportInternalErrors: true + +ignoreSigs: +- .Errorf( +- errors.New( +- wrap( + +extraIgnoreSigs: +- wrapError( + +ignoreSigRegexps: +- new.*Error\( diff --git a/wrapcheck/testdata/config_reportInternalErrors/main.go b/wrapcheck/testdata/config_reportInternalErrors/main.go new file mode 100644 index 0000000..ffde8db --- /dev/null +++ b/wrapcheck/testdata/config_reportInternalErrors/main.go @@ -0,0 +1,94 @@ +package main + +import ( + "encoding/json" + "errors" + "fmt" +) + +func main() { + do() +} + +func do() error { + // Internal function + if err := fn(); err != nil { + return err // want `package-internal error should be wrapped` + } + if err := fn(); err != nil { + return fmt.Errorf("wrap: %w", err) + } + + // Internal struct method + ss := someStruct{} + if err := ss.someMethod(); err != nil { + return err // want `package-internal error should be wrapped` + } + if err := ss.someMethod(); err != nil { + return fmt.Errorf("wrap: %w", err) + } + + // Interface method + var si someInterface = &someInterfaceImpl{} + if err := si.SomeMethod(); err != nil { + return err // want `error returned from interface method should be wrapped` + } + if err := si.SomeMethod(); err != nil { + return fmt.Errorf("wrap: %w", err) + } + + // External function + if _, err := json.Marshal(struct{}{}); err != nil { + return err // want `error returned from external package is unwrapped` + } + if _, err := json.Marshal(struct{}{}); err != nil { + return fmt.Errorf("wrap: %w", err) + } + + // Ignore sigs + if err := wrap(errors.New("error")); err != nil { + return err + } + + // Extra ignore sigs + if err := wrapError(errors.New("error")); err != nil { + return err + } + + // Ignore sig regexps + if err := newError(errors.New("error")); err != nil { + return err + } + + return nil +} + +func fn() error { + return errors.New("error") +} + +type someStruct struct{} + +func (s *someStruct) someMethod() error { + return errors.New("error") +} + +type someInterface interface { + SomeMethod() error +} + +type someInterfaceImpl struct { + someInterface +} + +func wrap(err error) error { + return fmt.Errorf("wrap: %w", err) +} + +func wrapError(err error) error { + return fmt.Errorf("wrap: %w", err) +} + +func newError(err error) error { + return fmt.Errorf("new error: %w", err) +} diff --git a/wrapcheck/wrapcheck.go b/wrapcheck/wrapcheck.go index 127f7ef..12cd156 100644 --- a/wrapcheck/wrapcheck.go +++ b/wrapcheck/wrapcheck.go @@ -83,6 +83,10 @@ type WrapcheckConfig struct { // 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"` + + // ReportInternalErrors determines whether wrapcheck should report errors returned + // from inside the package. + ReportInternalErrors bool `mapstructure:"reportInternalErrors" yaml:"reportInternalErrors"` } func NewDefaultConfig() WrapcheckConfig { @@ -273,16 +277,31 @@ func reportUnwrapped( pkgGlobs []glob.Glob, ) { + if cfg.ReportInternalErrors { + // Check if the call is package-internal function + fnIdent, ok := call.Fun.(*ast.Ident) + if ok { + fnSig := pass.TypesInfo.ObjectOf(fnIdent).String() + + // Check for ignored signatures + if checkSignature(cfg, regexpsSig, fnSig) { + return + } + + pass.Reportf(tokenPos, "package-internal error should be wrapped: sig: %s", fnSig) + return + } + } + sel, ok := call.Fun.(*ast.SelectorExpr) if !ok { return } - // Check for ignored signatures fnSig := pass.TypesInfo.ObjectOf(sel.Sel).String() - if contains(cfg.IgnoreSigs, fnSig) || contains(cfg.ExtraIgnoreSigs, fnSig) { - return - } else if containsMatch(regexpsSig, fnSig) { + + // Check for ignored signatures + if checkSignature(cfg, regexpsSig, fnSig) { return } @@ -305,6 +324,17 @@ func reportUnwrapped( pass.Reportf(tokenPos, "error returned from external package is unwrapped: sig: %s", fnSig) return } + + // If `ReportInternalErrors` is true, report package-internal errors + if cfg.ReportInternalErrors { + pass.Reportf(tokenPos, "package-internal error should be wrapped: sig: %s", fnSig) + return + } +} + +// checkSignature returns whether the function should be ignored. +func checkSignature(cfg WrapcheckConfig, regexpsSig []*regexp.Regexp, fnSig string) bool { + return contains(cfg.IgnoreSigs, fnSig) || contains(cfg.ExtraIgnoreSigs, fnSig) || containsMatch(regexpsSig, fnSig) } // isInterface returns whether the function call is one defined on an interface.