Add errorf rule

fixtures/errorf.go

@@ -0,0 +1,40 @@
+// Test for not using fmt.Errorf or testing.Errorf.
+
+// Package foo ...
+package foo
+
+import (
+	"errors"
+	"fmt"
+	"testing"
+)
+
+func f(x int) error {
+	if x > 10 {
+		return errors.New(fmt.Sprintf("something %d", x)) // MATCH /should replace errors.New(fmt.Sprintf(...)) with fmt.Errorf(...)/ -> 		return fmt.Errorf("something %d", x)`
+	}
+	if x > 5 {
+		return errors.New(g("blah")) // ok
+	}
+	if x > 4 {
+		return errors.New("something else") // ok
+	}
+	return nil
+}
+
+// TestF is a dummy test
+func TestF(t *testing.T) error {
+	x := 1
+	if x > 10 {
+		return t.Error(fmt.Sprintf("something %d", x)) // MATCH /should replace t.Error(fmt.Sprintf(...)) with t.Errorf(...)/
+	}
+	if x > 5 {
+		return t.Error(g("blah")) // ok
+	}
+	if x > 4 {
+		return t.Error("something else") // ok
+	}
+	return nil
+}
+
+func g(s string) string { return "prefix: " + s }

rule/errorf.go

@@ -0,0 +1,92 @@
+package rule
+
+import (
+	"fmt"
+	"go/ast"
+	"regexp"
+	"strings"
+
+	"github.com/mgechev/revive/lint"
+)
+
+// ErrorfRule lints given else constructs.
+type ErrorfRule struct{}
+
+// Apply applies the rule to given file.
+func (r *ErrorfRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
+	var failures []lint.Failure
+
+	fileAst := file.AST
+	walker := lintErrorf{
+		file:    file,
+		fileAst: fileAst,
+		onFailure: func(failure lint.Failure) {
+			failures = append(failures, failure)
+		},
+	}
+
+	ast.Walk(walker, fileAst)
+
+	return failures
+}
+
+// Name returns the rule name.
+func (r *ErrorfRule) Name() string {
+	return "errorf"
+}
+
+type lintErrorf struct {
+	file      *lint.File
+	fileAst   *ast.File
+	onFailure func(lint.Failure)
+}
+
+func (w lintErrorf) Visit(n ast.Node) ast.Visitor {
+	ce, ok := n.(*ast.CallExpr)
+	if !ok || len(ce.Args) != 1 {
+		return w
+	}
+	isErrorsNew := isPkgDot(ce.Fun, "errors", "New")
+	var isTestingError bool
+	se, ok := ce.Fun.(*ast.SelectorExpr)
+	if ok && se.Sel.Name == "Error" {
+		if typ := w.file.Pkg.TypeOf(se.X); typ != nil {
+			isTestingError = typ.String() == "*testing.T"
+		}
+	}
+	if !isErrorsNew && !isTestingError {
+		return w
+	}
+	arg := ce.Args[0]
+	ce, ok = arg.(*ast.CallExpr)
+	if !ok || !isPkgDot(ce.Fun, "fmt", "Sprintf") {
+		return w
+	}
+	errorfPrefix := "fmt"
+	if isTestingError {
+		errorfPrefix = w.file.Render(se.X)
+	}
+
+	failure := lint.Failure{
+		Category:   "errors",
+		Node:       n,
+		Confidence: 1,
+		Failure:    fmt.Sprintf("should replace %s(fmt.Sprintf(...)) with %s.Errorf(...)", w.file.Render(se), errorfPrefix),
+	}
+
+	m := srcLineWithMatch(w.file, ce, `^(.*)`+w.file.Render(se)+`\(fmt\.Sprintf\((.*)\)\)(.*)$`)
+	if m != nil {
+		failure.ReplacementLine = m[1] + errorfPrefix + ".Errorf(" + m[2] + ")" + m[3]
+	}
+
+	w.onFailure(failure)
+
+	return w
+}
+
+func srcLineWithMatch(file *lint.File, node ast.Node, pattern string) (m []string) {
+	line := srcLine(file.Content(), file.ToPosition(node.Pos()))
+	line = strings.TrimSuffix(line, "\n")
+	rx := regexp.MustCompile(pattern)
+	return rx.FindStringSubmatch(line)
+}

rule/range.go

@@ -3,7 +3,6 @@ package rule
 import (
 	"fmt"
 	"go/ast"
-	"go/token"
 	"strings"
 
 	"github.com/mgechev/revive/lint"
@@ -81,15 +80,3 @@ func indentOf(f *lint.File, node ast.Node) string {
 	}
 	return line // unusual or empty line
 }
-
-func srcLine(src []byte, p token.Position) string {
-	// Run to end of line in both directions if not at line start/end.
-	lo, hi := p.Offset, p.Offset+1
-	for lo > 0 && src[lo-1] != '\n' {
-		lo--
-	}
-	for hi < len(src) && src[hi-1] != '\n' {
-		hi++
-	}
-	return string(src[lo:hi])
-}

rule/utils.go

@@ -3,6 +3,7 @@ package rule
 import (
 	"fmt"
 	"go/ast"
+	"go/token"
 	"go/types"
 	"regexp"
 	"strings"
@@ -133,3 +134,15 @@ 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 srcLine(src []byte, p token.Position) string {
+	// Run to end of line in both directions if not at line start/end.
+	lo, hi := p.Offset, p.Offset+1
+	for lo > 0 && src[lo-1] != '\n' {
+		lo--
+	}
+	for hi < len(src) && src[hi-1] != '\n' {
+		hi++
+	}
+	return string(src[lo:hi])
+}

testutil/lint_test.go

@@ -42,6 +42,7 @@ var rules = []lint.Rule{
 	&rule.ElseRule{},
 	&rule.IfReturnRule{},
 	&rule.RangeRule{},
+	&rule.ErrorfRule{},
 }
 
 func TestAll(t *testing.T) {