Skip to content

Commit d2778f3

Browse files
chavacavachavacava
andauthored
adds rule use-errors-new (#1142)
Co-authored-by: chavacava <[email protected]>
1 parent e92a356 commit d2778f3

File tree

6 files changed

+93
-0
lines changed

6 files changed

+93
-0
lines changed

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
554554
| [`file-length-limit`](./RULES_DESCRIPTIONS.md#file-length-limit) | map (optional)| Enforces a maximum number of lines per file | no | no |
555555
| [`filename-format`](./RULES_DESCRIPTIONS.md#filename-format) | regular expression (optional) | Enforces the formatting of filenames | no | no |
556556
| [`redundant-build-tag`](./RULES_DESCRIPTIONS.md#redundant-build-tag) | n/a | Warns about redundant `// +build` comment lines | no | no |
557+
| [`use-errors-new`](./RULES_DESCRIPTIONS.md#use-errors-new) | n/a | Spots calls to `fmt.Errorf` that can be replaced by `errors.New` | no | no |
557558

558559
## Configurable rules
559560

RULES_DESCRIPTIONS.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ List of all available rules.
8282
- [unused-parameter](#unused-parameter)
8383
- [unused-receiver](#unused-receiver)
8484
- [use-any](#use-any)
85+
- [use-errors-new](#use-errors-new)
8586
- [useless-break](#useless-break)
8687
- [var-declaration](#var-declaration)
8788
- [var-naming](#var-naming)
@@ -987,6 +988,13 @@ _Description_: Since Go 1.18, `interface{}` has an alias: `any`. This rule propo
987988
988989
_Configuration_: N/A
989990
991+
## use-errors-new
992+
993+
_Description_: This rules identifies calls to `fmt.Errorf` that can be safely replaced by, the more efficient, `errors.New`.
994+
995+
_Configuration_: N/A
996+
997+
990998
## useless-break
991999
9921000
_Description_: This rule warns on useless `break` statements in case clauses of switch and select statements. Go, unlike other programming languages like C, only executes statements of the selected case while ignoring the subsequent case clauses.

config/config.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ var allRules = append([]lint.Rule{
9999
&rule.FileLengthLimitRule{},
100100
&rule.FilenameFormatRule{},
101101
&rule.RedundantBuildTagRule{},
102+
&rule.UseErrorsNewRule{},
102103
}, defaultRules...)
103104

104105
// allFormatters is a list of all available formatters to output the linting results.

rule/use_errors_new.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
package rule
2+
3+
import (
4+
"go/ast"
5+
6+
"github.com/mgechev/revive/lint"
7+
)
8+
9+
// UseErrorsNewRule spots calls to fmt.Errorf that can be replaced by errors.New.
10+
type UseErrorsNewRule struct{}
11+
12+
// Apply applies the rule to given file.
13+
func (*UseErrorsNewRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
14+
var failures []lint.Failure
15+
16+
walker := lintFmtErrorf{
17+
onFailure: func(failure lint.Failure) {
18+
failures = append(failures, failure)
19+
},
20+
}
21+
22+
ast.Walk(walker, file.AST)
23+
24+
return failures
25+
}
26+
27+
// Name returns the rule name.
28+
func (*UseErrorsNewRule) Name() string {
29+
return "use-errors-new"
30+
}
31+
32+
type lintFmtErrorf struct {
33+
onFailure func(lint.Failure)
34+
}
35+
36+
func (w lintFmtErrorf) Visit(n ast.Node) ast.Visitor {
37+
funcCall, ok := n.(*ast.CallExpr)
38+
if !ok {
39+
return w // not a function call
40+
}
41+
42+
isFmtErrorf := isPkgDot(funcCall.Fun, "fmt", "Errorf")
43+
if !isFmtErrorf {
44+
return w // not a call to fmt.Errorf
45+
}
46+
47+
if len(funcCall.Args) > 1 {
48+
return w // the use of fmt.Errorf is legit
49+
}
50+
51+
// the call is of the form fmt.Errorf("...")
52+
w.onFailure(lint.Failure{
53+
Category: "errors",
54+
Node: n,
55+
Confidence: 1,
56+
Failure: "replace fmt.Errorf by errors.New",
57+
})
58+
59+
return w
60+
}

test/use_errors_new_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/mgechev/revive/rule"
7+
)
8+
9+
func TestUseErrorsNew(t *testing.T) {
10+
testRule(t, "use_errors_new", &rule.UseErrorsNewRule{})
11+
}

testdata/use_errors_new.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package pkg
2+
3+
import "fmt"
4+
5+
func errorsNew() (int, error) {
6+
fmt.Errorf("repo cannot be nil") // MATCH /replace fmt.Errorf by errors.New/
7+
errs := append(errs, fmt.Errorf("commit cannot be nil")) // MATCH /replace fmt.Errorf by errors.New/
8+
fmt.Errorf("unable to load base repo: %w", err)
9+
fmt.Errorf("Failed to get full commit id for origin/%s: %w", pr.BaseBranch, err)
10+
11+
return 0, fmt.Errorf(msg + "something") // MATCH /replace fmt.Errorf by errors.New/
12+
}

0 commit comments

Comments
 (0)