From 486998e090c656d4be7332b022af935d9819c3d4 Mon Sep 17 00:00:00 2001 From: Godsgift Uloamaka Date: Thu, 9 Oct 2025 21:37:46 +0100 Subject: [PATCH 1/5] feat: extend deep-exit to detect exit-triggering flag usage Signed-off-by: Godsgift Uloamaka --- rule/deep_exit.go | 10 +++++++++- rule/utils.go | 27 +++++++++++++++++++++++++++ testdata/deep_exit.go | 13 +++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/rule/deep_exit.go b/rule/deep_exit.go index 6f7acd305..28bcca0fa 100644 --- a/rule/deep_exit.go +++ b/rule/deep_exit.go @@ -64,13 +64,21 @@ func (w *lintDeepExit) Visit(node ast.Node) ast.Visitor { pkg := id.Name fn := fc.Sel.Name - if isCallToExitFunction(pkg, fn) { + switch { + case isCallToExitFunction(pkg, fn): w.onFailure(lint.Failure{ Confidence: 1, Node: ce, Category: lint.FailureCategoryBadPractice, Failure: fmt.Sprintf("calls to %s.%s only in main() or init() functions", pkg, fn), }) + case isConditionalExitFunction(pkg, fn, ce): + w.onFailure(lint.Failure{ + Confidence: 1, + Node: ce, + Category: lint.FailureCategoryBadPractice, + Failure: fmt.Sprintf("calls to %s.%s with exit-triggering argument only in main() or init() functions", pkg, fn), + }) } return w diff --git a/rule/utils.go b/rule/utils.go index 24a723435..ede01b916 100644 --- a/rule/utils.go +++ b/rule/utils.go @@ -2,6 +2,7 @@ package rule import ( "fmt" + "go/ast" "go/token" "regexp" "strings" @@ -21,6 +22,7 @@ var exitFunctions = map[string]map[string]bool{ "Panicf": true, "Panicln": true, }, + "flag": {"Parse": true}, } func srcLine(src []byte, p token.Position) string { @@ -90,6 +92,31 @@ func isCallToExitFunction(pkgName, functionName string) bool { return exitFunctions[pkgName] != nil && exitFunctions[pkgName][functionName] } +var conditionalExitFunctions = map[string]map[string]func(*ast.CallExpr) bool{ + "flag": { + "NewFlagSet": func(ce *ast.CallExpr) bool { + if len(ce.Args) == 2 { + if arg, ok := ce.Args[1].(*ast.SelectorExpr); ok { + if id, ok := arg.X.(*ast.Ident); ok { + return id.Name == "flag" && arg.Sel.Name == "ExitOnError" + } + } + } + return false + }, + }, +} + +// isConditionalExitFunction checks if the function call is a call to a conditional exit function. +func isConditionalExitFunction(pkgName, functionName string, ce *ast.CallExpr) bool { + if m, ok := conditionalExitFunctions[pkgName]; ok { + if check, ok := m[functionName]; ok { + return check(ce) + } + } + return false +} + // newInternalFailureError returns a slice of Failure with a single internal failure in it. func newInternalFailureError(e error) []lint.Failure { return []lint.Failure{lint.NewInternalFailure(e.Error())} diff --git a/testdata/deep_exit.go b/testdata/deep_exit.go index 8137f4a6b..5d4181322 100644 --- a/testdata/deep_exit.go +++ b/testdata/deep_exit.go @@ -1,6 +1,7 @@ package fixtures import ( + "flag" "log" "os" "syscall" @@ -36,3 +37,15 @@ func TestMain(m *testing.M) { // must match because this is not a test file os.Exit(m.Run()) // MATCH /calls to os.Exit only in main() or init() functions/ } + +func flagParseOutsideMain() { + flag.Parse() // MATCH /calls to flag.Parse only in main() or init() functions/ +} + +func flagNewFlagSetExitOnErrorOutsideMain() { + flag.NewFlagSet("cmd", flag.ExitOnError) // MATCH /calls to flag.NewFlagSet with exit-triggering argument only in main() or init() functions/ +} + +func flagNewFlagSetContinueOnErrorOK() { + flag.NewFlagSet("cmd", flag.ContinueOnError) +} \ No newline at end of file From ea91dc31dec71bfeb5f05af177ed78c0fe55e8be Mon Sep 17 00:00:00 2001 From: Godsgift Uloamaka Date: Fri, 10 Oct 2025 20:58:14 +0100 Subject: [PATCH 2/5] deep-exit: disable flag.Parse() line in cli/main.go to fix CI --- cli/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/main.go b/cli/main.go index 25eaec942..0975a7c6c 100644 --- a/cli/main.go +++ b/cli/main.go @@ -173,7 +173,7 @@ func initConfig() { flag.BoolVar(&versionFlag, "version", false, versionUsage) flag.BoolVar(&setExitStatus, "set_exit_status", false, exitStatusUsage) flag.IntVar(&maxOpenFiles, "max_open_files", 0, maxOpenFilesUsage) - flag.Parse() + flag.Parse() //revive:disable-line:deep-exit } // getVersion returns build info (version, commit, date, and builtBy). From 04aa0fc3a593e3715d1fe135115d824b4d4906b3 Mon Sep 17 00:00:00 2001 From: Oleksandr Redko Date: Mon, 13 Oct 2025 20:21:32 +0300 Subject: [PATCH 3/5] fix lint; move to isCallToExitFunction --- rule/deep_exit.go | 10 +---- rule/redundant_test_main_exit.go | 2 +- rule/unconditional_recursion.go | 4 +- rule/utils.go | 71 ++++++++++++++++---------------- rule/utils_test.go | 39 +++++++++++++----- testdata/deep_exit.go | 4 +- 6 files changed, 68 insertions(+), 62 deletions(-) diff --git a/rule/deep_exit.go b/rule/deep_exit.go index 28bcca0fa..e270a1bae 100644 --- a/rule/deep_exit.go +++ b/rule/deep_exit.go @@ -64,21 +64,13 @@ func (w *lintDeepExit) Visit(node ast.Node) ast.Visitor { pkg := id.Name fn := fc.Sel.Name - switch { - case isCallToExitFunction(pkg, fn): + if isCallToExitFunction(pkg, fn, ce.Args) { w.onFailure(lint.Failure{ Confidence: 1, Node: ce, Category: lint.FailureCategoryBadPractice, Failure: fmt.Sprintf("calls to %s.%s only in main() or init() functions", pkg, fn), }) - case isConditionalExitFunction(pkg, fn, ce): - w.onFailure(lint.Failure{ - Confidence: 1, - Node: ce, - Category: lint.FailureCategoryBadPractice, - Failure: fmt.Sprintf("calls to %s.%s with exit-triggering argument only in main() or init() functions", pkg, fn), - }) } return w diff --git a/rule/redundant_test_main_exit.go b/rule/redundant_test_main_exit.go index 01a90a47c..717969ef6 100644 --- a/rule/redundant_test_main_exit.go +++ b/rule/redundant_test_main_exit.go @@ -66,7 +66,7 @@ func (w *lintRedundantTestMainExit) Visit(node ast.Node) ast.Visitor { pkg := id.Name fn := fc.Sel.Name - if isCallToExitFunction(pkg, fn) { + if isCallToExitFunction(pkg, fn, ce.Args) { w.onFailure(lint.Failure{ Confidence: 1, Node: ce, diff --git a/rule/unconditional_recursion.go b/rule/unconditional_recursion.go index 21fe09fa8..772896c1f 100644 --- a/rule/unconditional_recursion.go +++ b/rule/unconditional_recursion.go @@ -190,9 +190,7 @@ func (*lintUnconditionalRecursionRule) hasControlExit(node ast.Node) bool { functionName := se.Sel.Name pkgName := id.Name - if isCallToExitFunction(pkgName, functionName) { - return true - } + return isCallToExitFunction(pkgName, functionName, n.Args) } return false diff --git a/rule/utils.go b/rule/utils.go index ede01b916..cc272a9eb 100644 --- a/rule/utils.go +++ b/rule/utils.go @@ -11,18 +11,34 @@ import ( ) // 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}, +var exitFunctions = map[string]map[string]func(args []ast.Expr) bool{ + "os": {"Exit": func([]ast.Expr) bool { return true }}, + "syscall": {"Exit": func([]ast.Expr) bool { return true }}, "log": { - "Fatal": true, - "Fatalf": true, - "Fatalln": true, - "Panic": true, - "Panicf": true, - "Panicln": true, + "Fatal": func([]ast.Expr) bool { return true }, + "Fatalf": func([]ast.Expr) bool { return true }, + "Fatalln": func([]ast.Expr) bool { return true }, + "Panic": func([]ast.Expr) bool { return true }, + "Panicf": func([]ast.Expr) bool { return true }, + "Panicln": func([]ast.Expr) bool { return true }, + }, + "flag": { + "Parse": func([]ast.Expr) bool { return true }, + "NewFlagSet": func(args []ast.Expr) bool { + if len(args) != 2 { + return false + } + arg, ok := args[1].(*ast.SelectorExpr) + if !ok { + return false + } + id, ok := arg.X.(*ast.Ident) + if !ok { + return false + } + return id.Name == "flag" && arg.Sel.Name == "ExitOnError" + }, }, - "flag": {"Parse": true}, } func srcLine(src []byte, p token.Position) string { @@ -88,33 +104,16 @@ func isDirectiveComment(line string) bool { } // 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] -} - -var conditionalExitFunctions = map[string]map[string]func(*ast.CallExpr) bool{ - "flag": { - "NewFlagSet": func(ce *ast.CallExpr) bool { - if len(ce.Args) == 2 { - if arg, ok := ce.Args[1].(*ast.SelectorExpr); ok { - if id, ok := arg.X.(*ast.Ident); ok { - return id.Name == "flag" && arg.Sel.Name == "ExitOnError" - } - } - } - return false - }, - }, -} - -// isConditionalExitFunction checks if the function call is a call to a conditional exit function. -func isConditionalExitFunction(pkgName, functionName string, ce *ast.CallExpr) bool { - if m, ok := conditionalExitFunctions[pkgName]; ok { - if check, ok := m[functionName]; ok { - return check(ce) - } +func isCallToExitFunction(pkgName, functionName string, ce []ast.Expr) bool { + m, ok := exitFunctions[pkgName] + if !ok { + return false + } + check, ok := m[functionName] + if !ok { + return false } - return false + return check(ce) } // newInternalFailureError returns a slice of Failure with a single internal failure in it. diff --git a/rule/utils_test.go b/rule/utils_test.go index dabae07e4..ab275e0b9 100644 --- a/rule/utils_test.go +++ b/rule/utils_test.go @@ -2,6 +2,7 @@ package rule import ( "fmt" + "go/ast" "testing" ) @@ -9,23 +10,39 @@ func TestIsCallToExitFunction(t *testing.T) { tests := []struct { pkgName string functionName string + functionArgs []ast.Expr 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}, + {"os", "Exit", nil, true}, + {"syscall", "Exit", nil, true}, + {"log", "Fatal", nil, true}, + {"log", "Fatalf", nil, true}, + {"log", "Fatalln", nil, true}, + {"log", "Panic", nil, true}, + {"log", "Panicf", nil, true}, + {"flag", "Parse", nil, true}, + {"flag", "NewFlagSet", []ast.Expr{ + nil, + &ast.SelectorExpr{ + X: &ast.Ident{Name: "flag"}, + Sel: &ast.Ident{Name: "ExitOnError"}, + }, + }, true}, + {"log", "Print", nil, false}, + {"fmt", "Errorf", nil, false}, + {"flag", "NewFlagSet", []ast.Expr{ + nil, + &ast.SelectorExpr{ + X: &ast.Ident{Name: "flag"}, + Sel: &ast.Ident{Name: "ContinueOnError"}, + }, + }, 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) + if got := isCallToExitFunction(tt.pkgName, tt.functionName, tt.functionArgs); got != tt.expected { + t.Errorf("isCallToExitFunction(%s, %s, %v) = %v; want %v", tt.pkgName, tt.functionName, tt.functionArgs, got, tt.expected) } }) } diff --git a/testdata/deep_exit.go b/testdata/deep_exit.go index 5d4181322..a6ac692fc 100644 --- a/testdata/deep_exit.go +++ b/testdata/deep_exit.go @@ -43,9 +43,9 @@ func flagParseOutsideMain() { } func flagNewFlagSetExitOnErrorOutsideMain() { - flag.NewFlagSet("cmd", flag.ExitOnError) // MATCH /calls to flag.NewFlagSet with exit-triggering argument only in main() or init() functions/ + flag.NewFlagSet("cmd", flag.ExitOnError) // MATCH /calls to flag.NewFlagSet only in main() or init() functions/ } func flagNewFlagSetContinueOnErrorOK() { flag.NewFlagSet("cmd", flag.ContinueOnError) -} \ No newline at end of file +} From 0019be2f260452d9e7c8cf126f9eadec62ca4141 Mon Sep 17 00:00:00 2001 From: Godsgift Uloamaka Date: Wed, 15 Oct 2025 09:54:51 +0100 Subject: [PATCH 4/5] refactor: clean up code in deep-exit rule --- rule/deep_exit.go | 9 ++++++++- rule/utils.go | 40 ++++++++++++++++++++-------------------- testdata/deep_exit.go | 2 +- 3 files changed, 29 insertions(+), 22 deletions(-) diff --git a/rule/deep_exit.go b/rule/deep_exit.go index e270a1bae..ed3e34b53 100644 --- a/rule/deep_exit.go +++ b/rule/deep_exit.go @@ -7,6 +7,7 @@ import ( "unicode" "unicode/utf8" + "github.com/mgechev/revive/internal/astutils" "github.com/mgechev/revive/lint" ) @@ -65,11 +66,17 @@ func (w *lintDeepExit) Visit(node ast.Node) ast.Visitor { pkg := id.Name fn := fc.Sel.Name if isCallToExitFunction(pkg, fn, ce.Args) { + msg := fmt.Sprintf("calls to %s.%s only in main() or init() functions", pkg, fn) + + if pkg == "flag" && fn == "NewFlagSet" && + len(ce.Args) == 2 && astutils.IsPkgDotName(ce.Args[1], "flag", "ExitOnError") { + msg = "calls to flag.NewFlagSet with flag.ExitOnError only in main() or init() functions" + } w.onFailure(lint.Failure{ Confidence: 1, Node: ce, Category: lint.FailureCategoryBadPractice, - Failure: fmt.Sprintf("calls to %s.%s only in main() or init() functions", pkg, fn), + Failure: msg, }) } diff --git a/rule/utils.go b/rule/utils.go index cc272a9eb..90c92d820 100644 --- a/rule/utils.go +++ b/rule/utils.go @@ -7,20 +7,26 @@ import ( "regexp" "strings" + "github.com/mgechev/revive/internal/astutils" "github.com/mgechev/revive/lint" ) +// exitChecker is a function type that checks whether a function call is an exit function. +type exitFuncChecker func(args []ast.Expr) bool + +var alwaysTrue exitFuncChecker = func([]ast.Expr) bool { return true } + // exitFunctions is a map of std packages and functions that are considered as exit functions. -var exitFunctions = map[string]map[string]func(args []ast.Expr) bool{ - "os": {"Exit": func([]ast.Expr) bool { return true }}, - "syscall": {"Exit": func([]ast.Expr) bool { return true }}, +var exitFunctions = map[string]map[string]exitFuncChecker { + "os": {"Exit": alwaysTrue}, + "syscall": {"Exit": alwaysTrue}, "log": { - "Fatal": func([]ast.Expr) bool { return true }, - "Fatalf": func([]ast.Expr) bool { return true }, - "Fatalln": func([]ast.Expr) bool { return true }, - "Panic": func([]ast.Expr) bool { return true }, - "Panicf": func([]ast.Expr) bool { return true }, - "Panicln": func([]ast.Expr) bool { return true }, + "Fatal": alwaysTrue, + "Fatalf": alwaysTrue, + "Fatalln": alwaysTrue, + "Panic": alwaysTrue, + "Panicf": alwaysTrue, + "Panicln": alwaysTrue, }, "flag": { "Parse": func([]ast.Expr) bool { return true }, @@ -28,15 +34,7 @@ var exitFunctions = map[string]map[string]func(args []ast.Expr) bool{ if len(args) != 2 { return false } - arg, ok := args[1].(*ast.SelectorExpr) - if !ok { - return false - } - id, ok := arg.X.(*ast.Ident) - if !ok { - return false - } - return id.Name == "flag" && arg.Sel.Name == "ExitOnError" + return astutils.IsPkgDotName(args[1], "flag", "ExitOnError") }, }, } @@ -104,16 +102,18 @@ func isDirectiveComment(line string) bool { } // isCallToExitFunction checks if the function call is a call to an exit function. -func isCallToExitFunction(pkgName, functionName string, ce []ast.Expr) bool { +func isCallToExitFunction(pkgName, functionName string, callArgs []ast.Expr) bool { m, ok := exitFunctions[pkgName] if !ok { return false } + check, ok := m[functionName] if !ok { return false } - return check(ce) + + return check(callArgs) } // newInternalFailureError returns a slice of Failure with a single internal failure in it. diff --git a/testdata/deep_exit.go b/testdata/deep_exit.go index a6ac692fc..700cb3b38 100644 --- a/testdata/deep_exit.go +++ b/testdata/deep_exit.go @@ -43,7 +43,7 @@ func flagParseOutsideMain() { } func flagNewFlagSetExitOnErrorOutsideMain() { - flag.NewFlagSet("cmd", flag.ExitOnError) // MATCH /calls to flag.NewFlagSet only in main() or init() functions/ + flag.NewFlagSet("cmd", flag.ExitOnError) // MATCH /calls to flag.NewFlagSet with flag.ExitOnError only in main() or init() functions/ } func flagNewFlagSetContinueOnErrorOK() { From af4e96e04d65951e22e95534a0163b3317d652e1 Mon Sep 17 00:00:00 2001 From: Godsgift Uloamaka Date: Wed, 15 Oct 2025 10:10:01 +0100 Subject: [PATCH 5/5] fix-lint: format exitFuncChecker correctly --- rule/utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rule/utils.go b/rule/utils.go index 90c92d820..0c9d7bc95 100644 --- a/rule/utils.go +++ b/rule/utils.go @@ -17,7 +17,7 @@ type exitFuncChecker func(args []ast.Expr) bool var alwaysTrue exitFuncChecker = func([]ast.Expr) bool { return true } // exitFunctions is a map of std packages and functions that are considered as exit functions. -var exitFunctions = map[string]map[string]exitFuncChecker { +var exitFunctions = map[string]map[string]exitFuncChecker{ "os": {"Exit": alwaysTrue}, "syscall": {"Exit": alwaysTrue}, "log": {