Skip to content

Commit ea8b325

Browse files
committed
optimization: new optimization for fmt.Errorf("constant")
1 parent 6cce219 commit ea8b325

File tree

4 files changed

+51
-9
lines changed

4 files changed

+51
-9
lines changed

analyzer/analyzer.go

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,15 @@ func New() *analysis.Analyzer {
3535
}
3636

3737
func (n *perfSprint) run(pass *analysis.Pass) (interface{}, error) {
38-
var fmtSprintObj, fmtSprintfObj types.Object
38+
var fmtSprintObj, fmtSprintfObj, fmtErrorfObj types.Object
3939
for _, pkg := range pass.Pkg.Imports() {
4040
if pkg.Path() == "fmt" {
4141
fmtSprintObj = pkg.Scope().Lookup("Sprint")
4242
fmtSprintfObj = pkg.Scope().Lookup("Sprintf")
43+
fmtErrorfObj = pkg.Scope().Lookup("Errorf")
4344
}
4445
}
45-
if fmtSprintfObj == nil {
46+
if fmtSprintfObj == nil && fmtSprintObj == nil && fmtErrorfObj == nil {
4647
return nil, nil
4748
}
4849

@@ -65,6 +66,11 @@ func (n *perfSprint) run(pass *analysis.Pass) (interface{}, error) {
6566
err error
6667
)
6768
switch {
69+
case calledObj == fmtErrorfObj && len(call.Args) == 1:
70+
fn = "fmt.Errorf"
71+
verb = "%s"
72+
value = call.Args[0]
73+
6874
case calledObj == fmtSprintObj && len(call.Args) == 1:
6975
fn = "fmt.Sprint"
7076
verb = "%v"
@@ -100,7 +106,7 @@ func (n *perfSprint) run(pass *analysis.Pass) (interface{}, error) {
100106

101107
var d *analysis.Diagnostic
102108
switch {
103-
case isBasicType(valueType, types.String) && oneOf(verb, "%v", "%s"):
109+
case isBasicType(valueType, types.String) && oneOf(verb, "%v", "%s") && fn != "fmt.Errorf":
104110
d = &analysis.Diagnostic{
105111
Pos: call.Pos(),
106112
End: call.End(),
@@ -117,6 +123,23 @@ func (n *perfSprint) run(pass *analysis.Pass) (interface{}, error) {
117123
},
118124
}
119125

126+
case isBasicType(valueType, types.String) && oneOf(verb, "%v", "%s") && fn == "fmt.Errorf":
127+
d = &analysis.Diagnostic{
128+
Pos: call.Pos(),
129+
End: call.End(),
130+
Message: fn + " can be replaced with errors.New",
131+
SuggestedFixes: []analysis.SuggestedFix{
132+
{
133+
Message: "Use errors.New",
134+
TextEdits: []analysis.TextEdit{{
135+
Pos: call.Pos(),
136+
End: value.Pos(),
137+
NewText: []byte("errors.New("),
138+
}},
139+
},
140+
},
141+
}
142+
120143
case types.Implements(valueType, errIface) && oneOf(verb, "%v", "%s"):
121144
errMethodCall := formatNode(pass.Fset, value) + ".Error()"
122145
d = &analysis.Diagnostic{

analyzer/replacements_bench_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package analyzer_test
33
import (
44
"context"
55
"encoding/hex"
6+
"errors"
67
"fmt"
78
"math"
89
"strconv"
@@ -49,6 +50,20 @@ func BenchmarkErrorFormatting(b *testing.B) {
4950
})
5051
}
5152

53+
func BenchmarkFormattingError(b *testing.B) {
54+
b.Run("fmt.Errorf", func(b *testing.B) {
55+
for n := 0; n < b.N; n++ {
56+
_ = fmt.Errorf("onlystring")
57+
}
58+
})
59+
60+
b.Run("errors.New", func(b *testing.B) {
61+
for n := 0; n < b.N; n++ {
62+
_ = errors.New("onlystring")
63+
}
64+
})
65+
}
66+
5267
func BenchmarkBoolFormatting(b *testing.B) {
5368
b.Run("fmt.Sprint", func(b *testing.B) {
5469
for n := 0; n < b.N; n++ {

analyzer/testdata/src/p/p.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ func positive() {
2020
fmt.Sprintf("%s", s) // want "fmt.Sprintf can be replaced with just using the string"
2121
fmt.Sprintf("%v", s) // want "fmt.Sprintf can be replaced with just using the string"
2222
fmt.Sprint(s) // want "fmt.Sprint can be replaced with just using the string"
23+
fmt.Errorf("hello") // want "fmt.Errorf can be replaced with errors.New"
2324

2425
var err error
2526
fmt.Sprintf("%s", errSentinel) // want "fmt.Sprintf can be replaced with errSentinel.Error()"
@@ -180,6 +181,7 @@ func negative() {
180181
fmt.Printf("%v")
181182
fmt.Printf("%d", 42)
182183
fmt.Printf("%s %d", "hello", 42)
184+
fmt.Errorf("this is %s", "complex")
183185

184186
fmt.Fprint(os.Stdout, "%d", 42)
185187
fmt.Fprintf(os.Stdout, "test")

analyzer/testdata/src/p/p.go.golden

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,13 @@ var errSentinel = errors.New("connection refused")
1414

1515
func positive() {
1616
var s string
17-
"hello" // want "fmt.Sprintf can be replaced with just using the string"
18-
"hello" // want "fmt.Sprintf can be replaced with just using the string"
19-
"hello" // want "fmt.Sprint can be replaced with just using the string"
20-
s // want "fmt.Sprintf can be replaced with just using the string"
21-
s // want "fmt.Sprintf can be replaced with just using the string"
22-
s // want "fmt.Sprint can be replaced with just using the string"
17+
"hello" // want "fmt.Sprintf can be replaced with just using the string"
18+
"hello" // want "fmt.Sprintf can be replaced with just using the string"
19+
"hello" // want "fmt.Sprint can be replaced with just using the string"
20+
s // want "fmt.Sprintf can be replaced with just using the string"
21+
s // want "fmt.Sprintf can be replaced with just using the string"
22+
s // want "fmt.Sprint can be replaced with just using the string"
23+
errors.New("hello") // want "fmt.Errorf can be replaced with errors.New"
2324

2425
var err error
2526
errSentinel.Error() // want "fmt.Sprintf can be replaced with errSentinel.Error()"
@@ -180,6 +181,7 @@ func negative() {
180181
fmt.Printf("%v")
181182
fmt.Printf("%d", 42)
182183
fmt.Printf("%s %d", "hello", 42)
184+
fmt.Errorf("this is %s", "complex")
183185

184186
fmt.Fprint(os.Stdout, "%d", 42)
185187
fmt.Fprintf(os.Stdout, "test")

0 commit comments

Comments
 (0)