Skip to content

Commit

Permalink
refactor: avoid duplicated exitFunctions map (#1145)
Browse files Browse the repository at this point in the history
  • Loading branch information
alexandear authored Nov 28, 2024
1 parent d2778f3 commit 777abc9
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 34 deletions.
23 changes: 4 additions & 19 deletions rule/deep_exit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
16 changes: 1 addition & 15 deletions rule/unconditional_recursion.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
}
Expand Down
19 changes: 19 additions & 0 deletions rule/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]
}
32 changes: 32 additions & 0 deletions rule/utils_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}

0 comments on commit 777abc9

Please sign in to comment.