Suggest replacing errors.New(fmt.Sprintf(...)) with fmt.Errorf(...).
diff --git a/lint.go b/lint.go
index 64e8049..d679f2f 100644
--- a/lint.go
+++ b/lint.go
@@ -70,6 +70,7 @@
f.lintVarDecls()
f.lintElses()
f.lintRanges()
+ f.lintErrorf()
return f.problems
}
@@ -595,7 +596,26 @@
}
f.errorf(rs.Value, 1, "should omit 2nd value from range; this loop is equivalent to `for %s %s range ...`", f.render(rs.Key), rs.Tok)
+ return true
+ })
+}
+// lintErrorf examines errors.New calls. It complains if its only argument is an fmt.Sprintf invocation.
+func (f *file) lintErrorf() {
+ f.walk(func(node ast.Node) bool {
+ ce, ok := node.(*ast.CallExpr)
+ if !ok {
+ return true
+ }
+ if !isPkgDot(ce.Fun, "errors", "New") || len(ce.Args) != 1 {
+ return true
+ }
+ arg := ce.Args[0]
+ ce, ok = arg.(*ast.CallExpr)
+ if !ok || !isPkgDot(ce.Fun, "fmt", "Sprintf") {
+ return true
+ }
+ f.errorf(node, 1, "should replace errors.New(fmt.Sprintf(...)) with fmt.Errorf(...)")
return true
})
}
@@ -646,6 +666,11 @@
return ok && id.Name == ident
}
+func isPkgDot(expr ast.Expr, pkg, name string) bool {
+ sel, ok := expr.(*ast.SelectorExpr)
+ return ok && isIdent(sel.X, pkg) && isIdent(sel.Sel, name)
+}
+
func isIntLiteral(expr ast.Expr) bool {
// Either a BasicLit with Kind token.INT,
// or some combination of a UnaryExpr with Op token.SUB (for "-<lit>")
diff --git a/testdata/errorf.go b/testdata/errorf.go
new file mode 100644
index 0000000..768fb8c
--- /dev/null
+++ b/testdata/errorf.go
@@ -0,0 +1,22 @@
+// Test for not using fmt.Errorf.
+
+// Package foo ...
+package foo
+
+import (
+ "errors"
+ "fmt"
+)
+
+func f(x int) error {
+ if x > 10 {
+ return errors.New(fmt.Sprintf("something %d", x)) // MATCH /should replace.*errors\.New\(fmt\.Sprintf\(\.\.\.\)\).*fmt\.Errorf\(\.\.\.\)/
+ }
+ if x > 5 {
+ return errors.New(g("blah")) // ok
+ }
+ if x > 4 {
+ return errors.New("something else") // ok
+ }
+ return nil
+}
