Skip to content

Commit 0fada9d

Browse files
doniacldchavacava
andauthored
Update error-strings rule (#608) (#609)
Co-authored-by: SalvadorC <[email protected]>
1 parent 2c895fb commit 0fada9d

File tree

2 files changed

+95
-10
lines changed

2 files changed

+95
-10
lines changed

rule/error-strings.go

+77-10
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,25 @@ type ErrorStringsRule struct{}
1717
func (r *ErrorStringsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
1818
var failures []lint.Failure
1919

20+
var errorFunctions = map[string]map[string]struct{}{
21+
"fmt": {
22+
"Errorf": {},
23+
},
24+
"errors": {
25+
"Errorf": {},
26+
"WithMessage": {},
27+
"Wrap": {},
28+
"New": {},
29+
"WithMessagef": {},
30+
"Wrapf": {},
31+
},
32+
}
33+
2034
fileAst := file.AST
2135
walker := lintErrorStrings{
22-
file: file,
23-
fileAst: fileAst,
36+
file: file,
37+
fileAst: fileAst,
38+
errorFunctions: errorFunctions,
2439
onFailure: func(failure lint.Failure) {
2540
failures = append(failures, failure)
2641
},
@@ -37,24 +52,31 @@ func (r *ErrorStringsRule) Name() string {
3752
}
3853

3954
type lintErrorStrings struct {
40-
file *lint.File
41-
fileAst *ast.File
42-
onFailure func(lint.Failure)
55+
file *lint.File
56+
fileAst *ast.File
57+
errorFunctions map[string]map[string]struct{}
58+
onFailure func(lint.Failure)
4359
}
4460

61+
// Visit browses the AST
4562
func (w lintErrorStrings) Visit(n ast.Node) ast.Visitor {
4663
ce, ok := n.(*ast.CallExpr)
4764
if !ok {
4865
return w
4966
}
50-
if !isPkgDot(ce.Fun, "errors", "New") && !isPkgDot(ce.Fun, "fmt", "Errorf") {
67+
68+
if len(ce.Args) < 1 {
5169
return w
5270
}
53-
if len(ce.Args) < 1 {
71+
72+
// expression matches the known pkg.function
73+
ok = w.match(ce)
74+
if !ok {
5475
return w
5576
}
56-
str, ok := ce.Args[0].(*ast.BasicLit)
57-
if !ok || str.Kind != token.STRING {
77+
78+
str, ok := w.getMessage(ce)
79+
if !ok {
5880
return w
5981
}
6082
s, _ := strconv.Unquote(str.Value) // can assume well-formed Go
@@ -65,7 +87,6 @@ func (w lintErrorStrings) Visit(n ast.Node) ast.Visitor {
6587
if clean {
6688
return w
6789
}
68-
6990
w.onFailure(lint.Failure{
7091
Node: str,
7192
Confidence: conf,
@@ -75,6 +96,52 @@ func (w lintErrorStrings) Visit(n ast.Node) ast.Visitor {
7596
return w
7697
}
7798

99+
// match returns true if the expression corresponds to the known pkg.function
100+
// i.e.: errors.Wrap
101+
func (w lintErrorStrings) match(expr *ast.CallExpr) bool {
102+
sel, ok := expr.Fun.(*ast.SelectorExpr)
103+
if !ok {
104+
return false
105+
}
106+
// retrieve the package
107+
id, ok := sel.X.(*ast.Ident)
108+
functions, ok := w.errorFunctions[id.Name]
109+
if !ok {
110+
return false
111+
}
112+
// retrieve the function
113+
_, ok = functions[sel.Sel.Name]
114+
return ok
115+
}
116+
117+
// getMessage returns the message depending on its position
118+
// returns false if the cast is unsuccessful
119+
func (w lintErrorStrings) getMessage(expr *ast.CallExpr) (s *ast.BasicLit, success bool) {
120+
str, ok := w.checkArg(expr, 0)
121+
if ok {
122+
return str, true
123+
}
124+
if len(expr.Args) < 2 {
125+
return s, false
126+
}
127+
str, ok = w.checkArg(expr, 1)
128+
if !ok {
129+
return s, false
130+
}
131+
return str, true
132+
}
133+
134+
func (lintErrorStrings) checkArg(expr *ast.CallExpr, arg int) (s *ast.BasicLit, success bool) {
135+
str, ok := expr.Args[arg].(*ast.BasicLit)
136+
if !ok {
137+
return s, false
138+
}
139+
if str.Kind != token.STRING {
140+
return s, false
141+
}
142+
return str, true
143+
}
144+
78145
func lintErrorString(s string) (isClean bool, conf float64) {
79146
const basicConfidence = 0.8
80147
const capConfidence = basicConfidence - 0.2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Package foo ...
2+
package foo
3+
4+
import (
5+
"github.com/pkg/errors"
6+
)
7+
8+
// Check for the error strings themselves.
9+
10+
func errorsStrings(x int) error {
11+
var err error
12+
err = errors.Wrap(err, "This %d is too low") // MATCH /error strings should not be capitalized or end with punctuation or a newline/
13+
err = errors.New("This %d is too low") // MATCH /error strings should not be capitalized or end with punctuation or a newline/
14+
err = errors.Wrapf(err, "This %d is too low", x) // MATCH /error strings should not be capitalized or end with punctuation or a newline/
15+
err = errors.WithMessage(err, "This %d is too low") // MATCH /error strings should not be capitalized or end with punctuation or a newline/
16+
err = errors.WithMessagef(err, "This %d is too low", x) // MATCH /error strings should not be capitalized or end with punctuation or a newline/
17+
return err
18+
}

0 commit comments

Comments
 (0)