From d2778f36f1cad0f9c675c221ddf58b20bcbf5792 Mon Sep 17 00:00:00 2001
From: chavacava <salvadorcavadini+github@gmail.com>
Date: Fri, 22 Nov 2024 08:22:51 +0100
Subject: [PATCH] adds rule use-errors-new (#1142)

Co-authored-by: chavacava <salvador.cavadini@gmail.com>
---
 README.md                   |  1 +
 RULES_DESCRIPTIONS.md       |  8 +++++
 config/config.go            |  1 +
 rule/use_errors_new.go      | 60 +++++++++++++++++++++++++++++++++++++
 test/use_errors_new_test.go | 11 +++++++
 testdata/use_errors_new.go  | 12 ++++++++
 6 files changed, 93 insertions(+)
 create mode 100644 rule/use_errors_new.go
 create mode 100644 test/use_errors_new_test.go
 create mode 100644 testdata/use_errors_new.go

diff --git a/README.md b/README.md
index 1d45fe1..753b27f 100644
--- a/README.md
+++ b/README.md
@@ -554,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
 
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 184831d..5ff9aa3 100644
--- a/config/config.go
+++ b/config/config.go
@@ -99,6 +99,7 @@ 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.
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
+}
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/
+}