Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions wrapcheck/testdata/config_reportInternalErrors/.wrapcheck.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
reportInternalErrors: true

ignoreSigs:
- .Errorf(
- errors.New(
- wrap(

extraIgnoreSigs:
- wrapError(

ignoreSigRegexps:
- new.*Error\(
94 changes: 94 additions & 0 deletions wrapcheck/testdata/config_reportInternalErrors/main.go
Original file line number Diff line number Diff line change
@@ -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)
}
38 changes: 34 additions & 4 deletions wrapcheck/wrapcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}

Expand All @@ -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.
Expand Down