[WIP] New rule use-errors-new (propose replacing fmt.Errorf by errors.New when possible)

README.md

@@ -551,6 +551,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | [`file-length-limit`](./RULES_DESCRIPTIONS.md#file-length-limit) | map (optional)| Enforces a maximum number of lines per file |    no    |  no   |
 | [`filename-format`](./RULES_DESCRIPTIONS.md#filename-format) | regular expression (optional) | Enforces the formatting of filenames |   no    |  no   |
 | [`redundant-build-tag`](./RULES_DESCRIPTIONS.md#redundant-build-tag) | n/a   | Warns about redundant `// +build` comment lines |   no    |  no   |
+| [`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   |
 
 ## Configurable rules
 

RULES_DESCRIPTIONS.md

@@ -82,6 +82,7 @@ List of all available rules.
   - [unused-parameter](#unused-parameter)
   - [unused-receiver](#unused-receiver)
   - [use-any](#use-any)
+  - [use-errors-new](#use-errors-new)
   - [useless-break](#useless-break)
   - [var-declaration](#var-declaration)
   - [var-naming](#var-naming)
@@ -987,6 +988,13 @@ _Description_: Since Go 1.18, `interface{}` has an alias: `any`. This rule propo
 
 _Configuration_: N/A
 
+## use-errors-new
+
+_Description_: This rules identifies calls to `fmt.Errorf` that can be safely replaced by, the more efficient, `errors.New`.
+
+_Configuration_: N/A
+
+
 ## useless-break
 
 _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

@@ -98,7 +98,8 @@
 	&rule.CommentsDensityRule{},
 	&rule.FileLengthLimitRule{},
 	&rule.FilenameFormatRule{},
 	&rule.RedundantBuildTagRule{},
+	&rule.UseErrorsNewRule{},
 }, defaultRules...)
 
 var allFormatters = []lint.Formatter{

rule/use_errors_new.go

@@ -0,0 +1,60 @@
+package rule
+
+import (
+	"go/ast"
+
+	"github.com/mgechev/revive/lint"
+)
+
+// UseErrorsNewRule spots calls to fmt.Errorf that can be replaced by errors.New.
+type UseErrorsNewRule struct{}
+
+// Apply applies the rule to given file.
+func (*UseErrorsNewRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+	var failures []lint.Failure
+
+	walker := lintFmtErrorf{
+		onFailure: func(failure lint.Failure) {
+			failures = append(failures, failure)
+		},
+	}
+
+	ast.Walk(walker, file.AST)
+
+	return failures
+}
+
+// Name returns the rule name.
+func (*UseErrorsNewRule) Name() string {
+	return "use-errors-new"
+}
+
+type lintFmtErrorf struct {
+	onFailure func(lint.Failure)
+}
+
+func (w lintFmtErrorf) Visit(n ast.Node) ast.Visitor {
+	funcCall, ok := n.(*ast.CallExpr)
+	if !ok {
+		return w // not a function call
+	}
+
+	isFmtErrorf := isPkgDot(funcCall.Fun, "fmt", "Errorf")
+	if !isFmtErrorf {
+		return w // not a call to fmt.Errorf
+	}
+
+	if len(funcCall.Args) > 1 {
+		return w // the use of fmt.Errorf is legit
+	}
+
+	// the call is of the form fmt.Errorf("...")
+	w.onFailure(lint.Failure{
+		Category:   "errors",
+		Node:       n,
+		Confidence: 1,
+		Failure:    "replace fmt.Errorf by errors.New",
+	})
+
+	return w
+}

test/use_errors_new_test.go

@@ -0,0 +1,11 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/mgechev/revive/rule"
+)
+
+func TestUseErrorsNew(t *testing.T) {
+	testRule(t, "use_errors_new", &rule.UseErrorsNewRule{})
+}

testdata/use_errors_new.go

@@ -0,0 +1,12 @@
+package pkg
+
+import "fmt"
+
+func errorsNew() (int, error) {
+	fmt.Errorf("repo cannot be nil")                         // MATCH /replace fmt.Errorf by errors.New/
+	errs := append(errs, fmt.Errorf("commit cannot be nil")) // MATCH /replace fmt.Errorf by errors.New/
+	fmt.Errorf("unable to load base repo: %w", err)
+	fmt.Errorf("Failed to get full commit id for origin/%s: %w", pr.BaseBranch, err)
+
+	return 0, fmt.Errorf(msg + "something") // MATCH /replace fmt.Errorf by errors.New/
+}