Skip to content

Commit 486d5bb

Browse files
authored
feat: add option for reporting internal errors (#60)
1 parent 91a0edf commit 486d5bb

File tree

4 files changed

+144
-4
lines changed

4 files changed

+144
-4
lines changed

README.md

+4
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ ignorePackageGlobs:
6868
# function whose call is defined on the given interface.
6969
ignoreInterfaceRegexps:
7070
- ^(?i)c(?-i)ach(ing|e)
71+
72+
# ReportInternalErrors determines whether wrapcheck should report errors returned
73+
# from inside the package.
74+
reportInternalErrors: true
7175
```
7276
7377
## Usage
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
reportInternalErrors: true
2+
3+
ignoreSigs:
4+
- .Errorf(
5+
- errors.New(
6+
- wrap(
7+
8+
extraIgnoreSigs:
9+
- wrapError(
10+
11+
ignoreSigRegexps:
12+
- new.*Error\(
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
package main
2+
3+
import (
4+
"encoding/json"
5+
"errors"
6+
"fmt"
7+
)
8+
9+
func main() {
10+
do()
11+
}
12+
13+
func do() error {
14+
// Internal function
15+
if err := fn(); err != nil {
16+
return err // want `package-internal error should be wrapped`
17+
}
18+
if err := fn(); err != nil {
19+
return fmt.Errorf("wrap: %w", err)
20+
}
21+
22+
// Internal struct method
23+
ss := someStruct{}
24+
if err := ss.someMethod(); err != nil {
25+
return err // want `package-internal error should be wrapped`
26+
}
27+
if err := ss.someMethod(); err != nil {
28+
return fmt.Errorf("wrap: %w", err)
29+
}
30+
31+
// Interface method
32+
var si someInterface = &someInterfaceImpl{}
33+
if err := si.SomeMethod(); err != nil {
34+
return err // want `error returned from interface method should be wrapped`
35+
}
36+
if err := si.SomeMethod(); err != nil {
37+
return fmt.Errorf("wrap: %w", err)
38+
}
39+
40+
// External function
41+
if _, err := json.Marshal(struct{}{}); err != nil {
42+
return err // want `error returned from external package is unwrapped`
43+
}
44+
if _, err := json.Marshal(struct{}{}); err != nil {
45+
return fmt.Errorf("wrap: %w", err)
46+
}
47+
48+
// Ignore sigs
49+
if err := wrap(errors.New("error")); err != nil {
50+
return err
51+
}
52+
53+
// Extra ignore sigs
54+
if err := wrapError(errors.New("error")); err != nil {
55+
return err
56+
}
57+
58+
// Ignore sig regexps
59+
if err := newError(errors.New("error")); err != nil {
60+
return err
61+
}
62+
63+
return nil
64+
}
65+
66+
func fn() error {
67+
return errors.New("error")
68+
}
69+
70+
type someStruct struct{}
71+
72+
func (s *someStruct) someMethod() error {
73+
return errors.New("error")
74+
}
75+
76+
type someInterface interface {
77+
SomeMethod() error
78+
}
79+
80+
type someInterfaceImpl struct {
81+
someInterface
82+
}
83+
84+
func wrap(err error) error {
85+
return fmt.Errorf("wrap: %w", err)
86+
}
87+
88+
func wrapError(err error) error {
89+
return fmt.Errorf("wrap: %w", err)
90+
}
91+
92+
func newError(err error) error {
93+
return fmt.Errorf("new error: %w", err)
94+
}

wrapcheck/wrapcheck.go

+34-4
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,10 @@ type WrapcheckConfig struct {
8383
// returned from any function whose call is defined on a interface named 'Transactor'
8484
// or 'Transaction' due to the name matching the regular expression `Transac(tor|tion)`.
8585
IgnoreInterfaceRegexps []string `mapstructure:"ignoreInterfaceRegexps" yaml:"ignoreInterfaceRegexps"`
86+
87+
// ReportInternalErrors determines whether wrapcheck should report errors returned
88+
// from inside the package.
89+
ReportInternalErrors bool `mapstructure:"reportInternalErrors" yaml:"reportInternalErrors"`
8690
}
8791

8892
func NewDefaultConfig() WrapcheckConfig {
@@ -273,16 +277,31 @@ func reportUnwrapped(
273277
pkgGlobs []glob.Glob,
274278
) {
275279

280+
if cfg.ReportInternalErrors {
281+
// Check if the call is package-internal function
282+
fnIdent, ok := call.Fun.(*ast.Ident)
283+
if ok {
284+
fnSig := pass.TypesInfo.ObjectOf(fnIdent).String()
285+
286+
// Check for ignored signatures
287+
if checkSignature(cfg, regexpsSig, fnSig) {
288+
return
289+
}
290+
291+
pass.Reportf(tokenPos, "package-internal error should be wrapped: sig: %s", fnSig)
292+
return
293+
}
294+
}
295+
276296
sel, ok := call.Fun.(*ast.SelectorExpr)
277297
if !ok {
278298
return
279299
}
280300

281-
// Check for ignored signatures
282301
fnSig := pass.TypesInfo.ObjectOf(sel.Sel).String()
283-
if contains(cfg.IgnoreSigs, fnSig) || contains(cfg.ExtraIgnoreSigs, fnSig) {
284-
return
285-
} else if containsMatch(regexpsSig, fnSig) {
302+
303+
// Check for ignored signatures
304+
if checkSignature(cfg, regexpsSig, fnSig) {
286305
return
287306
}
288307

@@ -305,6 +324,17 @@ func reportUnwrapped(
305324
pass.Reportf(tokenPos, "error returned from external package is unwrapped: sig: %s", fnSig)
306325
return
307326
}
327+
328+
// If `ReportInternalErrors` is true, report package-internal errors
329+
if cfg.ReportInternalErrors {
330+
pass.Reportf(tokenPos, "package-internal error should be wrapped: sig: %s", fnSig)
331+
return
332+
}
333+
}
334+
335+
// checkSignature returns whether the function should be ignored.
336+
func checkSignature(cfg WrapcheckConfig, regexpsSig []*regexp.Regexp, fnSig string) bool {
337+
return contains(cfg.IgnoreSigs, fnSig) || contains(cfg.ExtraIgnoreSigs, fnSig) || containsMatch(regexpsSig, fnSig)
308338
}
309339

310340
// isInterface returns whether the function call is one defined on an interface.

0 commit comments

Comments
 (0)