Repository: mgechev/revive
Pull request #1142 (2024-11-20 21:08:03)

adds rule use-errors-new
Implementing a new rule `use-errors-new` that proposes replacing `fmt.Errorf` by `errors.New` when possible.

As indicated in [this issue](https://github.com/googleapis/google-cloud-go/issues/9749), beyond style considerations, `errors.News` is ~4x faster that `fmt.Errorf`

(initial discussion on this PR can be retrieved at #1136)


Patch:
diff --git a/README.md b/README.md
index 6a5aa61..753b27f 100644
--- a/README.md
+++ b/README.md
@@ -94,6 +94,9 @@ Here's how `revive` is different from `golint`:
     - [Default](#default)
     - [Plain](#plain)
     - [Unix](#unix)
+    - [JSON](#json)
+    - [NDJSON](#ndjson)
+    - [Checkstyle](#checkstyle)
     - [SARIF](#sarif)
   - [Extensibility](#extensibility)
     - [Writing a Custom Rule](#writing-a-custom-rule)
@@ -551,6 +554,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
 
@@ -597,9 +601,21 @@ The unix formatter produces the same output as the default formatter but surroun
 
 ![Unix formatter](/assets/formatter-unix.png)
 
+### JSON
+
+The `json` formatter produces output in JSON format.
+
+### NDJSON
+
+The `ndjson` formatter produces output in [`Newline Delimited JSON`](https://github.com/ndjson/ndjson-spec) format.
+
+### Checkstyle
+
+The `checkstyle` formatter produces output in a [Checkstyle-like](https://checkstyle.sourceforge.io/) format.
+
 ### SARIF
 
-The `sarif`  formatter produces outputs in SARIF, for _Static Analysis Results Interchange Format_, a standard JSON-based format for the output of static analysis tools defined and promoted by [OASIS](https://www.oasis-open.org/).
+The `sarif`  formatter produces output in SARIF, for _Static Analysis Results Interchange Format_, a standard JSON-based format for the output of static analysis tools defined and promoted by [OASIS](https://www.oasis-open.org/).
 
 Current supported version of the standard is [SARIF-v2.1.0](https://docs.oasis-open.org/sarif/sarif/v2.1.0/csprd01/sarif-v2.1.0-csprd01.html
 ).
diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md
index 31b0a7e..e084756 100644
--- a/RULES_DESCRIPTIONS.md
+++ b/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.
diff --git a/config/config.go b/config/config.go
index 1da7386..5ff9aa3 100644
--- a/config/config.go
+++ b/config/config.go
@@ -99,18 +99,21 @@ var allRules = append([]lint.Rule{
 	&rule.FileLengthLimitRule{},
 	&rule.FilenameFormatRule{},
 	&rule.RedundantBuildTagRule{},
+	&rule.UseErrorsNewRule{},
 }, defaultRules...)
 
+// allFormatters is a list of all available formatters to output the linting results.
+// Keep the list sorted and in sync with available formatters in README.md.
 var allFormatters = []lint.Formatter{
-	&formatter.Stylish{},
+	&formatter.Checkstyle{},
+	&formatter.Default{},
 	&formatter.Friendly{},
 	&formatter.JSON{},
 	&formatter.NDJSON{},
-	&formatter.Default{},
-	&formatter.Unix{},
-	&formatter.Checkstyle{},
 	&formatter.Plain{},
 	&formatter.Sarif{},
+	&formatter.Stylish{},
+	&formatter.Unix{},
 }
 
 func getFormatters() map[string]lint.Formatter {
diff --git a/rule/redundant_build_tag.go b/rule/redundant_build_tag.go
index cba84f9..52412b9 100644
--- a/rule/redundant_build_tag.go
+++ b/rule/redundant_build_tag.go
@@ -12,7 +12,7 @@ type RedundantBuildTagRule struct{}
 // Apply triggers if an old build tag `// +build` is found after a new one `//go:build`.
 // `//go:build` comments are automatically added by gofmt when Go 1.17+ is used.
 // See https://pkg.go.dev/cmd/go#hdr-Build_constraints
-func (*RedundantBuildTagRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
+func (*RedundantBuildTagRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
 	for _, group := range file.AST.Comments {
 		hasGoBuild := false
 		for _, comment := range group.List {
diff --git a/rule/use_errors_new.go b/rule/use_errors_new.go
new file mode 100644
index 0000000..d21bbf7
--- /dev/null
+++ b/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 patch:
diff --git a/test/use_errors_new_test.go b/test/use_errors_new_test.go
new file mode 100644
index 0000000..135beaa
--- /dev/null
+++ b/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{})
+}
diff --git a/testdata/use_errors_new.go b/testdata/use_errors_new.go
new file mode 100644
index 0000000..9fae226
--- /dev/null
+++ b/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/
+}