feature: add redundant-test-main-exit rule (#1208)

README.md

@@ -560,6 +560,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | [`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   |
+| [`redundant-test-main-exit`](./RULES_DESCRIPTIONS.md#redundant-test-main-exit) |  n/a   | Suggests removing `Exit` call in `TestMain` function for test files |    no    |  no   |
 
 ## Configurable rules
 

RULES_DESCRIPTIONS.md

@@ -66,6 +66,7 @@ List of all available rules.
   - [redefines-builtin-id](#redefines-builtin-id)
   - [redundant-import-alias](#redundant-import-alias)
   - [redundant-build-tag](#redundant-build-tag)
+  - [redundant-test-main-exit](#redundant-test-main-exit)
   - [string-format](#string-format)
   - [string-of-int](#string-of-int)
   - [struct-tag](#struct-tag)
@@ -284,7 +285,7 @@ _Configuration_: N/A
 _Description_: This rule warns on some common mistakes when using `defer` statement. It currently alerts on the following situations:
 
 | name              | description                                                                                                                                                                                     |
-|-------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| ----------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
 | call-chain        | even if deferring call-chains of the form `foo()()` is valid, it does not helps code understanding (only the last call is deferred)                                                             |
 | loop              | deferring inside loops can be misleading (deferred functions are not executed at the end of the loop iteration but of the current function) and it could lead to exhausting the execution stack |
 | method-call       | deferring a call to a method can lead to subtle bugs if the method does not have a pointer receiver                                                                                             |
@@ -809,6 +810,14 @@ _Description_: This rule warns about redundant build tag comments `// +build` wh
 
 _Configuration_: N/A
 
+## redundant-test-main-exit
+
+_Description_: This rule warns about redundant `Exit` calls in the `TestMain` function, as the Go test runner automatically handles program termination starting from Go 1.15.
+
+_Configuration_: N/A
+
+_Note_: This rule is irrelevant for Go versions below 1.15.
+
 ## string-format
 
 _Description_: This rule allows you to configure a list of regular expressions that string literals in certain function calls are checked against.

cli/main_test.go

@@ -14,7 +14,7 @@ func TestMain(m *testing.M) {
 	os.Unsetenv("USERPROFILE")
 	os.Unsetenv("XDG_CONFIG_HOME")
 	AppFs = afero.NewMemMapFs()
-	os.Exit(m.Run())
+	m.Run()
 }
 
 func TestXDGConfigDirIsPreferredFirst(t *testing.T) {

config/config.go

@@ -100,6 +100,7 @@ var allRules = append([]lint.Rule{
 	&rule.FilenameFormatRule{},
 	&rule.RedundantBuildTagRule{},
 	&rule.UseErrorsNewRule{},
+	&rule.RedundantTestMainExitRule{},
 }, defaultRules...)
 
 // allFormatters is a list of all available formatters to output the linting results.

lint/package.go

@@ -35,6 +35,7 @@ var (
 	trueValue  = 1
 	falseValue = 2
 
+	go115 = goversion.Must(goversion.NewVersion("1.15"))
 	go121 = goversion.Must(goversion.NewVersion("1.21"))
 	go122 = goversion.Must(goversion.NewVersion("1.22"))
 )
@@ -194,6 +195,11 @@ func (p *Package) lint(rules []Rule, config Config, failures chan Failure) error
 	return eg.Wait()
 }
 
+// IsAtLeastGo115 returns true if the Go version for this package is 1.15 or higher, false otherwise
+func (p *Package) IsAtLeastGo115() bool {
+	return p.goVersion.GreaterThanOrEqual(go115)
+}
+
 // IsAtLeastGo121 returns true if the Go version for this package is 1.21 or higher, false otherwise
 func (p *Package) IsAtLeastGo121() bool {
 	return p.goVersion.GreaterThanOrEqual(go121)

rule/redundant_test_main_exit.go

@@ -0,0 +1,79 @@
+package rule
+
+import (
+	"fmt"
+	"go/ast"
+
+	"github.com/mgechev/revive/lint"
+)
+
+// RedundantTestMainExitRule suggests removing Exit call in TestMain function for test files.
+type RedundantTestMainExitRule struct{}
+
+// Apply applies the rule to given file.
+func (*RedundantTestMainExitRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+	var failures []lint.Failure
+
+	if !file.IsTest() || !file.Pkg.IsAtLeastGo115() {
+		// skip analysis for non-test files or for Go versions before 1.15
+		return failures
+	}
+
+	onFailure := func(failure lint.Failure) {
+		failures = append(failures, failure)
+	}
+
+	w := &lintRedundantTestMainExit{onFailure: onFailure}
+	ast.Walk(w, file.AST)
+	return failures
+}
+
+// Name returns the rule name.
+func (*RedundantTestMainExitRule) Name() string {
+	return "redundant-test-main-exit"
+}
+
+type lintRedundantTestMainExit struct {
+	onFailure func(lint.Failure)
+}
+
+func (w *lintRedundantTestMainExit) Visit(node ast.Node) ast.Visitor {
+	if fd, ok := node.(*ast.FuncDecl); ok {
+		if fd.Name.Name != "TestMain" {
+			return nil // skip analysis for other functions than TestMain
+		}
+
+		return w
+	}
+
+	se, ok := node.(*ast.ExprStmt)
+	if !ok {
+		return w
+	}
+	ce, ok := se.X.(*ast.CallExpr)
+	if !ok {
+		return w
+	}
+
+	fc, ok := ce.Fun.(*ast.SelectorExpr)
+	if !ok {
+		return w
+	}
+	id, ok := fc.X.(*ast.Ident)
+	if !ok {
+		return w
+	}
+
+	pkg := id.Name
+	fn := fc.Sel.Name
+	if isCallToExitFunction(pkg, fn) {
+		w.onFailure(lint.Failure{
+			Confidence: 1,
+			Node:       ce,
+			Category:   lint.FailureCategoryStyle,
+			Failure:    fmt.Sprintf("redundant call to %s.%s in TestMain function, the test runner will handle it automatically as of Go 1.15", pkg, fn),
+		})
+	}
+
+	return w
+}

test/redundant_test_main_exit_test.go

@@ -0,0 +1,13 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/mgechev/revive/rule"
+)
+
+func TestRedundantTestMainExit(t *testing.T) {
+	testRule(t, "go1.15/redundant_test_main_exit_test", &rule.RedundantTestMainExitRule{})
+	testRule(t, "go1.15/redundant_test_main_exit", &rule.RedundantTestMainExitRule{})
+	testRule(t, "redundant_test_main_exit_test", &rule.RedundantTestMainExitRule{})
+}

testdata/go1.15/go.mod

@@ -0,0 +1,3 @@
+module github.com/mgechev/revive/testdata
+
+go 1.15

testdata/go1.15/redundant_test_main_exit.go

@@ -0,0 +1,23 @@
+package fixtures
+
+import (
+	"fmt"
+	"os"
+	"testing"
+)
+
+func TestMain(m *testing.M) {
+	setup()
+	i := m.Run()
+	teardown()
+	// must not match because this is not a test file
+	os.Exit(i)
+}
+
+func setup() {
+	fmt.Println("Setup")
+}
+
+func teardown() {
+	fmt.Println("Teardown")
+}

testdata/go1.15/redundant_test_main_exit_test.go

@@ -0,0 +1,38 @@
+package fixtures
+
+import (
+	"fmt"
+	"os"
+	"syscall"
+	"testing"
+)
+
+func TestMain(m *testing.M) {
+	setup()
+	i := m.Run()
+	teardown()
+	os.Exit(i)      // MATCH /redundant call to os.Exit in TestMain function, the test runner will handle it automatically as of Go 1.15/
+	syscall.Exit(i) // MATCH /redundant call to syscall.Exit in TestMain function, the test runner will handle it automatically as of Go 1.15/
+}
+
+func setup() {
+	fmt.Println("Setup")
+}
+
+func teardown() {
+	fmt.Println("Teardown")
+}
+
+func Test_function(t *testing.T) {
+	t.Error("Fail")
+}
+
+func Test_os_exit(t *testing.T) {
+	// must not match because this is not TestMain function
+	os.Exit(1)
+}
+
+func Test_syscall_exit(t *testing.T) {
+	// must not match because this is not TestMain function
+	syscall.Exit(1)
+}

testdata/redundant_test_main_exit_test.go

@@ -0,0 +1,27 @@
+package fixtures
+
+import (
+	"fmt"
+	"os"
+	"testing"
+)
+
+func TestMain(m *testing.M) {
+	setup()
+	i := m.Run()
+	teardown()
+	// must not match because the go version of this module is less than 1.15
+	os.Exit(i)
+}
+
+func setup() {
+	fmt.Println("Setup")
+}
+
+func teardown() {
+	fmt.Println("Teardown")
+}
+
+func Test_function(t *testing.T) {
+	t.Error("Fail")
+}