Repository: mgechev/revive
Pull request #1145 (2024-11-25 19:55:25)

refactor: avoid duplicated exitFunctions map
The map `exitFunctions` is duplicated in `rule/deep_exit.go` and `rule/unconditional_recursion.go`, so I moved this `var` to `rule/utils.go`. Additionally, I extracted `isCallToExitFunction` to a separate function.


Patch:
diff --git a/rule/deep_exit.go b/rule/deep_exit.go
index 7b3dd0f..ef81eb8 100644
--- a/rule/deep_exit.go
+++ b/rule/deep_exit.go
@@ -17,20 +17,7 @@ func (*DeepExitRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
 		failures = append(failures, failure)
 	}
 
-	exitFunctions := map[string]map[string]bool{
-		"os":      {"Exit": true},
-		"syscall": {"Exit": true},
-		"log": {
-			"Fatal":   true,
-			"Fatalf":  true,
-			"Fatalln": true,
-			"Panic":   true,
-			"Panicf":  true,
-			"Panicln": true,
-		},
-	}
-
-	w := lintDeepExit{onFailure, exitFunctions, file.IsTest()}
+	w := lintDeepExit{onFailure: onFailure, isTestFile: file.IsTest()}
 	ast.Walk(w, file.AST)
 	return failures
 }
@@ -41,9 +28,8 @@ func (*DeepExitRule) Name() string {
 }
 
 type lintDeepExit struct {
-	onFailure     func(lint.Failure)
-	exitFunctions map[string]map[string]bool
-	isTestFile    bool
+	onFailure  func(lint.Failure)
+	isTestFile bool
 }
 
 func (w lintDeepExit) Visit(node ast.Node) ast.Visitor {
@@ -75,8 +61,7 @@ func (w lintDeepExit) Visit(node ast.Node) ast.Visitor {
 
 	pkg := id.Name
 	fn := fc.Sel.Name
-	isACallToExitFunction := w.exitFunctions[pkg] != nil && w.exitFunctions[pkg][fn]
-	if isACallToExitFunction {
+	if isCallToExitFunction(pkg, fn) {
 		w.onFailure(lint.Failure{
 			Confidence: 1,
 			Node:       ce,
diff --git a/rule/unconditional_recursion.go b/rule/unconditional_recursion.go
index d806b67..271c3fa 100644
--- a/rule/unconditional_recursion.go
+++ b/rule/unconditional_recursion.go
@@ -152,19 +152,6 @@ func (w *lintUnconditionalRecursionRule) updateFuncStatus(node ast.Node) {
 	w.currentFunc.seenConditionalExit = w.hasControlExit(node)
 }
 
-var exitFunctions = map[string]map[string]bool{
-	"os":      {"Exit": true},
-	"syscall": {"Exit": true},
-	"log": {
-		"Fatal":   true,
-		"Fatalf":  true,
-		"Fatalln": true,
-		"Panic":   true,
-		"Panicf":  true,
-		"Panicln": true,
-	},
-}
-
 func (lintUnconditionalRecursionRule) hasControlExit(node ast.Node) bool {
 	// isExit returns true if the given node makes control exit the function
 	isExit := func(node ast.Node) bool {
@@ -187,8 +174,7 @@ func (lintUnconditionalRecursionRule) hasControlExit(node ast.Node) bool {
 
 			functionName := se.Sel.Name
 			pkgName := id.Name
-			isCallToExitFunction := exitFunctions[pkgName] != nil && exitFunctions[pkgName][functionName]
-			if isCallToExitFunction {
+			if isCallToExitFunction(pkgName, functionName) {
 				return true
 			}
 		}
diff --git a/rule/utils.go b/rule/utils.go
index a22219e..ecba547 100644
--- a/rule/utils.go
+++ b/rule/utils.go
@@ -11,6 +11,20 @@ import (
 	"github.com/mgechev/revive/lint"
 )
 
+// exitFunctions is a map of std packages and functions that are considered as exit functions.
+var exitFunctions = map[string]map[string]bool{
+	"os":      {"Exit": true},
+	"syscall": {"Exit": true},
+	"log": {
+		"Fatal":   true,
+		"Fatalf":  true,
+		"Fatalln": true,
+		"Panic":   true,
+		"Panicf":  true,
+		"Panicln": true,
+	},
+}
+
 func isCgoExported(f *ast.FuncDecl) bool {
 	if f.Recv != nil || f.Doc == nil {
 		return false
@@ -102,3 +116,8 @@ var directiveCommentRE = regexp.MustCompile("^//(line |extern |export |[a-z0-9]+
 func isDirectiveComment(line string) bool {
 	return directiveCommentRE.MatchString(line)
 }
+
+// isCallToExitFunction checks if the function call is a call to an exit function.
+func isCallToExitFunction(pkgName, functionName string) bool {
+	return exitFunctions[pkgName] != nil && exitFunctions[pkgName][functionName]
+}


Test patch:
diff --git a/rule/utils_test.go b/rule/utils_test.go
new file mode 100644
index 0000000..dabae07
--- /dev/null
+++ b/rule/utils_test.go
@@ -0,0 +1,32 @@
+package rule
+
+import (
+	"fmt"
+	"testing"
+)
+
+func TestIsCallToExitFunction(t *testing.T) {
+	tests := []struct {
+		pkgName      string
+		functionName string
+		expected     bool
+	}{
+		{"os", "Exit", true},
+		{"syscall", "Exit", true},
+		{"log", "Fatal", true},
+		{"log", "Fatalf", true},
+		{"log", "Fatalln", true},
+		{"log", "Panic", true},
+		{"log", "Panicf", true},
+		{"log", "Print", false},
+		{"fmt", "Errorf", false},
+	}
+
+	for _, tt := range tests {
+		t.Run(fmt.Sprintf("%s.%s", tt.pkgName, tt.functionName), func(t *testing.T) {
+			if got := isCallToExitFunction(tt.pkgName, tt.functionName); got != tt.expected {
+				t.Errorf("isCallToExitFunction(%s, %s) = %v; want %v", tt.pkgName, tt.functionName, got, tt.expected)
+			}
+		})
+	}
+}