dev: review and clean linter tests (#3139)

.golangci.yml

@@ -136,7 +136,6 @@ issues:
     - path: pkg/lint/lintersdb/manager.go
       text: "SA1019: (.+).(GoVersion|LangVersion) is deprecated: use the global `run.go` instead."
 
-
 run:
   timeout: 5m
   go: '1.17' # TODO(ldez): we force to use an old version of Go for the CI and the tests.

pkg/result/processors/sort_results.go

@@ -90,10 +90,8 @@ var (
 
 type ByName struct{ next comparator }
 
-//nolint:golint
 func (cmp ByName) Next() comparator { return cmp.next }
 
-//nolint:golint
 func (cmp ByName) Compare(a, b *result.Issue) compareResult {
 	var res compareResult
 
@@ -110,10 +108,8 @@ func (cmp ByName) Compare(a, b *result.Issue) compareResult {
 
 type ByLine struct{ next comparator }
 
-//nolint:golint
 func (cmp ByLine) Next() comparator { return cmp.next }
 
-//nolint:golint
 func (cmp ByLine) Compare(a, b *result.Issue) compareResult {
 	var res compareResult
 
@@ -130,10 +126,8 @@ func (cmp ByLine) Compare(a, b *result.Issue) compareResult {
 
 type ByColumn struct{ next comparator }
 
-//nolint:golint
 func (cmp ByColumn) Next() comparator { return cmp.next }
 
-//nolint:golint
 func (cmp ByColumn) Compare(a, b *result.Issue) compareResult {
 	var res compareResult
 

test/enabled_linters_test.go

@@ -10,72 +10,11 @@ import (
 	"github.com/golangci/golangci-lint/test/testshared"
 )
 
-func inSlice(s []string, v string) bool {
-	for _, sv := range s {
-		if sv == v {
-			return true
-		}
-	}
-
-	return false
-}
-
-func getEnabledByDefaultFastLintersExcept(except ...string) []string {
-	m := lintersdb.NewManager(nil, nil)
-	ebdl := m.GetAllEnabledByDefaultLinters()
-	var ret []string
-	for _, lc := range ebdl {
-		if lc.IsSlowLinter() {
-			continue
-		}
-
-		if !inSlice(except, lc.Name()) {
-			ret = append(ret, lc.Name())
-		}
-	}
-
-	return ret
-}
-
-func getAllFastLintersWith(with ...string) []string {
-	linters := lintersdb.NewManager(nil, nil).GetAllSupportedLinterConfigs()
-	ret := append([]string{}, with...)
-	for _, lc := range linters {
-		if lc.IsSlowLinter() {
-			continue
-		}
-		ret = append(ret, lc.Name())
-	}
-
-	return ret
-}
-
-func getEnabledByDefaultLinters() []string {
-	ebdl := lintersdb.NewManager(nil, nil).GetAllEnabledByDefaultLinters()
-	var ret []string
-	for _, lc := range ebdl {
-		ret = append(ret, lc.Name())
-	}
-
-	return ret
-}
-
-func getEnabledByDefaultFastLintersWith(with ...string) []string {
-	ebdl := lintersdb.NewManager(nil, nil).GetAllEnabledByDefaultLinters()
-	ret := append([]string{}, with...)
-	for _, lc := range ebdl {
-		if lc.IsSlowLinter() {
-			continue
-		}
-
-		ret = append(ret, lc.Name())
-	}
-
-	return ret
-}
-
 //nolint:funlen
 func TestEnabledLinters(t *testing.T) {
+	// require to display the message "Active x linters: [x,y]"
+	t.Setenv("GL_TEST_RUN", "1")
+
 	cases := []struct {
 		name           string
 		cfg            string
@@ -93,13 +32,13 @@ func TestEnabledLinters(t *testing.T) {
 			enabledLinters: getEnabledByDefaultFastLintersExcept("govet"),
 		},
 		{
-			name: "enable golint in config",
+			name: "enable revive in config",
 			cfg: `
 			linters:
 				enable:
-					- golint
+					- revive
 			`,
-			enabledLinters: getEnabledByDefaultFastLintersWith("golint"),
+			enabledLinters: getEnabledByDefaultFastLintersWith("revive"),
 		},
 		{
 			name:           "disable govet in cmd",
@@ -107,14 +46,14 @@ func TestEnabledLinters(t *testing.T) {
 			enabledLinters: getEnabledByDefaultFastLintersExcept("govet"),
 		},
 		{
-			name: "enable gofmt in cmd and enable golint in config",
+			name: "enable gofmt in cmd and enable revive in config",
 			args: []string{"-Egofmt"},
 			cfg: `
 			linters:
 				enable:
-					- golint
+					- revive
 			`,
-			enabledLinters: getEnabledByDefaultFastLintersWith("golint", "gofmt"),
+			enabledLinters: getEnabledByDefaultFastLintersWith("revive", "gofmt"),
 		},
 		{
 			name: "fast option in config",
@@ -195,3 +134,67 @@ func TestEnabledLinters(t *testing.T) {
 		})
 	}
 }
+
+func inSlice(s []string, v string) bool {
+	for _, sv := range s {
+		if sv == v {
+			return true
+		}
+	}
+
+	return false
+}
+
+func getEnabledByDefaultFastLintersExcept(except ...string) []string {
+	m := lintersdb.NewManager(nil, nil)
+	ebdl := m.GetAllEnabledByDefaultLinters()
+	var ret []string
+	for _, lc := range ebdl {
+		if lc.IsSlowLinter() {
+			continue
+		}
+
+		if !inSlice(except, lc.Name()) {
+			ret = append(ret, lc.Name())
+		}
+	}
+
+	return ret
+}
+
+func getAllFastLintersWith(with ...string) []string {
+	linters := lintersdb.NewManager(nil, nil).GetAllSupportedLinterConfigs()
+	ret := append([]string{}, with...)
+	for _, lc := range linters {
+		if lc.IsSlowLinter() {
+			continue
+		}
+		ret = append(ret, lc.Name())
+	}
+
+	return ret
+}
+
+func getEnabledByDefaultLinters() []string {
+	ebdl := lintersdb.NewManager(nil, nil).GetAllEnabledByDefaultLinters()
+	var ret []string
+	for _, lc := range ebdl {
+		ret = append(ret, lc.Name())
+	}
+
+	return ret
+}
+
+func getEnabledByDefaultFastLintersWith(with ...string) []string {
+	ebdl := lintersdb.NewManager(nil, nil).GetAllEnabledByDefaultLinters()
+	ret := append([]string{}, with...)
+	for _, lc := range ebdl {
+		if lc.IsSlowLinter() {
+			continue
+		}
+
+		ret = append(ret, lc.Name())
+	}
+
+	return ret
+}

test/run_test.go

@@ -2,7 +2,6 @@ package test
 
 import (
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -14,8 +13,6 @@ import (
 
 const minimalPkg = "minimalpkg"
 
-const skipDirsCommand = "testdata_etc/,pkg/golinters/goanalysis/(checker|passes)"
-
 func TestAutogeneratedNoIssues(t *testing.T) {
 	testshared.NewRunnerBuilder(t).
 		WithTargetPath(testdataDir, "autogenerated").
@@ -85,11 +82,9 @@ func TestTimeout(t *testing.T) {
 }
 
 func TestTimeoutInConfig(t *testing.T) {
-	type tc struct {
+	cases := []struct {
 		cfg string
-	}
-
-	cases := []tc{
+	}{
 		{
 			cfg: `
 				run:
@@ -118,7 +113,6 @@ func TestTimeoutInConfig(t *testing.T) {
 		// Run with disallowed option set only in config
 		testshared.NewRunnerBuilder(t).
 			WithConfig(c.cfg).
-			WithArgs("--skip-dirs", skipDirsCommand).
 			WithTargetPath(testdataDir, minimalPkg).
 			Runner().
 			Run().
@@ -202,104 +196,8 @@ func TestCgoWithIssues(t *testing.T) {
 	}
 }
 
-func TestUnsafeOk(t *testing.T) {
-	testshared.NewRunnerBuilder(t).
-		WithNoConfig().
-		WithArgs("--enable-all").
-		WithTargetPath(testdataDir, "unsafe").
-		Runner().
-		Install().
-		Run().
-		ExpectNoIssues()
-}
-
-func TestGovetCustomFormatter(t *testing.T) {
-	testshared.NewRunnerBuilder(t).
-		WithTargetPath(testdataDir, "govet_custom_formatter").
-		Runner().
-		Install().
-		Run().
-		ExpectNoIssues()
-}
-
-func TestLineDirectiveProcessedFilesLiteLoading(t *testing.T) {
-	output := strings.Join([]string{
-		"testdata/quicktemplate/hello.qtpl.go:26:1: exported function `StreamHello` should have comment or be unexported (golint)",
-		"testdata/quicktemplate/hello.qtpl.go:50:1: exported function `Hello` should have comment or be unexported (golint)",
-		"testdata/quicktemplate/hello.qtpl.go:39:1: exported function `WriteHello` should have comment or be unexported (golint)",
-	}, "\n")
-
-	testshared.NewRunnerBuilder(t).
-		WithNoConfig().
-		WithArgs(
-			"--print-issued-lines=false",
-			"--exclude-use-default=false",
-			"-Egolint",
-		).
-		WithTargetPath(testdataDir, "quicktemplate").
-		Runner().
-		Install().
-		Run().
-		ExpectExitCode(exitcodes.IssuesFound).ExpectOutputEq(output + "\n")
-}
-
-func TestSortedResults(t *testing.T) {
-	testCases := []struct {
-		opt  string
-		want string
-	}{
-		{
-			opt: "--sort-results=false",
-			want: "testdata/sort_results/main.go:15:13: Error return value is not checked (errcheck)" + "\n" +
-				"testdata/sort_results/main.go:12:5: var `db` is unused (unused)",
-		},
-		{
-			opt: "--sort-results=true",
-			want: "testdata/sort_results/main.go:12:5: var `db` is unused (unused)" + "\n" +
-				"testdata/sort_results/main.go:15:13: Error return value is not checked (errcheck)",
-		},
-	}
-
-	testshared.InstallGolangciLint(t)
-
-	for _, test := range testCases {
-		test := test
-		t.Run(test.opt, func(t *testing.T) {
-			t.Parallel()
-
-			testshared.NewRunnerBuilder(t).
-				WithNoConfig().
-				WithArgs("--print-issued-lines=false", test.opt).
-				WithTargetPath(testdataDir, "sort_results").
-				Runner().
-				Run().
-				ExpectExitCode(exitcodes.IssuesFound).ExpectOutputEq(test.want + "\n")
-		})
-	}
-}
-
-func TestLineDirectiveProcessedFilesFullLoading(t *testing.T) {
-	output := strings.Join([]string{
-		"testdata/quicktemplate/hello.qtpl.go:26:1: exported function `StreamHello` should have comment or be unexported (golint)",
-		"testdata/quicktemplate/hello.qtpl.go:50:1: exported function `Hello` should have comment or be unexported (golint)",
-		"testdata/quicktemplate/hello.qtpl.go:39:1: exported function `WriteHello` should have comment or be unexported (golint)",
-	}, "\n")
-
-	testshared.NewRunnerBuilder(t).
-		WithNoConfig().
-		WithArgs(
-			"--print-issued-lines=false",
-			"--exclude-use-default=false",
-			"-Egolint,govet",
-		).
-		WithTargetPath(testdataDir, "quicktemplate").
-		Runner().
-		Install().
-		Run().
-		ExpectExitCode(exitcodes.IssuesFound).ExpectOutputEq(output + "\n")
-}
-
-func TestLintFilesWithLineDirective(t *testing.T) {
+// https://pkg.go.dev/cmd/compile#hdr-Compiler_Directives
+func TestLineDirective(t *testing.T) {
 	testshared.InstallGolangciLint(t)
 
 	testCases := []struct {
@@ -396,6 +294,110 @@ func TestLintFilesWithLineDirective(t *testing.T) {
 	}
 }
 
+// https://pkg.go.dev/cmd/compile#hdr-Compiler_Directives
+func TestLineDirectiveProcessedFiles(t *testing.T) {
+	testCases := []struct {
+		desc     string
+		args     []string
+		target   string
+		expected []string
+	}{
+		{
+			desc: "lite loading",
+			args: []string{
+				"--print-issued-lines=false",
+				"--exclude-use-default=false",
+				"-Erevive",
+			},
+			target: "quicktemplate",
+			expected: []string{
+				"testdata/quicktemplate/hello.qtpl.go:10:1: package-comments: should have a package comment (revive)",
+				"testdata/quicktemplate/hello.qtpl.go:26:1: exported: exported function StreamHello should have comment or be unexported (revive)",
+				"testdata/quicktemplate/hello.qtpl.go:39:1: exported: exported function WriteHello should have comment or be unexported (revive)",
+				"testdata/quicktemplate/hello.qtpl.go:50:1: exported: exported function Hello should have comment or be unexported (revive)",
+			},
+		},
+		{
+			desc: "full loading",
+			args: []string{
+				"--print-issued-lines=false",
+				"--exclude-use-default=false",
+				"-Erevive,govet",
+			},
+			target: "quicktemplate",
+			expected: []string{
+				"testdata/quicktemplate/hello.qtpl.go:10:1: package-comments: should have a package comment (revive)",
+				"testdata/quicktemplate/hello.qtpl.go:26:1: exported: exported function StreamHello should have comment or be unexported (revive)",
+				"testdata/quicktemplate/hello.qtpl.go:39:1: exported: exported function WriteHello should have comment or be unexported (revive)",
+				"testdata/quicktemplate/hello.qtpl.go:50:1: exported: exported function Hello should have comment or be unexported (revive)",
+			},
+		},
+	}
+
+	for _, test := range testCases {
+		test := test
+		t.Run(test.desc, func(t *testing.T) {
+			t.Parallel()
+
+			testshared.NewRunnerBuilder(t).
+				WithNoConfig().
+				WithArgs(test.args...).
+				WithTargetPath(testdataDir, test.target).
+				Runner().
+				Install().
+				Run().
+				ExpectExitCode(exitcodes.IssuesFound).
+				ExpectOutputContains(test.expected...)
+		})
+	}
+}
+
+func TestUnsafeOk(t *testing.T) {
+	testshared.NewRunnerBuilder(t).
+		WithNoConfig().
+		WithArgs("--enable-all").
+		WithTargetPath(testdataDir, "unsafe").
+		Runner().
+		Install().
+		Run().
+		ExpectNoIssues()
+}
+
+func TestSortedResults(t *testing.T) {
+	testCases := []struct {
+		opt  string
+		want string
+	}{
+		{
+			opt: "--sort-results=false",
+			want: "testdata/sort_results/main.go:15:13: Error return value is not checked (errcheck)" + "\n" +
+				"testdata/sort_results/main.go:12:5: var `db` is unused (unused)",
+		},
+		{
+			opt: "--sort-results=true",
+			want: "testdata/sort_results/main.go:12:5: var `db` is unused (unused)" + "\n" +
+				"testdata/sort_results/main.go:15:13: Error return value is not checked (errcheck)",
+		},
+	}
+
+	testshared.InstallGolangciLint(t)
+
+	for _, test := range testCases {
+		test := test
+		t.Run(test.opt, func(t *testing.T) {
+			t.Parallel()
+
+			testshared.NewRunnerBuilder(t).
+				WithNoConfig().
+				WithArgs("--print-issued-lines=false", test.opt).
+				WithTargetPath(testdataDir, "sort_results").
+				Runner().
+				Run().
+				ExpectExitCode(exitcodes.IssuesFound).ExpectOutputEq(test.want + "\n")
+		})
+	}
+}
+
 func TestSkippedDirsNoMatchArg(t *testing.T) {
 	dir := filepath.Join(testdataDir, "skipdirs", "skip_me", "nested")
 
@@ -404,15 +406,15 @@ func TestSkippedDirsNoMatchArg(t *testing.T) {
 		WithArgs(
 			"--print-issued-lines=false",
 			"--skip-dirs", dir,
-			"-Egolint",
+			"-Erevive",
 		).
 		WithTargetPath(dir).
 		Runner().
 		Install().
 		Run().
 		ExpectExitCode(exitcodes.IssuesFound).
-		ExpectOutputEq("testdata/skipdirs/skip_me/nested/with_issue.go:8:9: `if` block ends with " +
-			"a `return` statement, so drop this `else` and outdent its block (golint)\n")
+		ExpectOutputEq("testdata/skipdirs/skip_me/nested/with_issue.go:8:9: " +
+			"indent-error-flow: if block ends with a return statement, so drop this else and outdent its block (revive)\n")
 }
 
 func TestSkippedDirsTestdata(t *testing.T) {
@@ -420,7 +422,7 @@ func TestSkippedDirsTestdata(t *testing.T) {
 		WithNoConfig().
 		WithArgs(
 			"--print-issued-lines=false",
-			"-Egolint",
+			"-Erevive",
 		).
 		WithTargetPath(testdataDir, "skipdirs", "...").
 		Runner().
@@ -511,7 +513,6 @@ func TestEnableAllFastAndEnableCanCoexist(t *testing.T) {
 
 			testshared.NewRunnerBuilder(t).
 				WithNoConfig().
-				WithArgs("--skip-dirs", skipDirsCommand).
 				WithArgs(test.args...).
 				WithTargetPath(testdataDir, minimalPkg).
 				Runner().
@@ -541,13 +542,14 @@ func TestAbsPathDirAnalysis(t *testing.T) {
 		WithNoConfig().
 		WithArgs(
 			"--print-issued-lines=false",
-			"-Egolint",
+			"-Erevive",
 		).
 		WithTargetPath(absDir).
 		Runner().
 		Install().
 		Run().
-		ExpectHasIssue("`if` block ends with a `return` statement")
+		ExpectHasIssue("testdata_etc/abspath/with_issue.go:8:9: " +
+			"indent-error-flow: if block ends with a return statement, so drop this else and outdent its block (revive)")
 }
 
 func TestAbsPathFileAnalysis(t *testing.T) {
@@ -559,22 +561,20 @@ func TestAbsPathFileAnalysis(t *testing.T) {
 		WithNoConfig().
 		WithArgs(
 			"--print-issued-lines=false",
-			"-Egolint",
+			"-Erevive",
 		).
 		WithTargetPath(absDir).
 		Runner().
 		Install().
 		Run().
-		ExpectHasIssue("`if` block ends with a `return` statement")
+		ExpectHasIssue("indent-error-flow: if block ends with a return statement, so drop this else and outdent its block (revive)")
 }
 
 func TestDisallowedOptionsInConfig(t *testing.T) {
-	type tc struct {
+	cases := []struct {
 		cfg    string
 		option string
-	}
-
-	cases := []tc{
+	}{
 		{
 			cfg: `
 				ruN:
@@ -618,7 +618,6 @@ func TestDisallowedOptionsInConfig(t *testing.T) {
 		// Run with disallowed option set only in config
 		testshared.NewRunnerBuilder(t).
 			WithConfig(c.cfg).
-			WithArgs("--skip-dirs", skipDirsCommand).
 			WithTargetPath(testdataDir, minimalPkg).
 			Runner().
 			Run().
@@ -633,7 +632,6 @@ func TestDisallowedOptionsInConfig(t *testing.T) {
 		// Run with disallowed option set only in command-line
 		testshared.NewRunnerBuilder(t).
 			WithNoConfig().
-			WithArgs("--skip-dirs", skipDirsCommand).
 			WithArgs(args...).
 			WithTargetPath(testdataDir, minimalPkg).
 			Runner().
@@ -644,7 +642,6 @@ func TestDisallowedOptionsInConfig(t *testing.T) {
 
 		testshared.NewRunnerBuilder(t).
 			WithConfig(c.cfg).
-			WithArgs("--skip-dirs", skipDirsCommand).
 			WithArgs(args...).
 			WithTargetPath(testdataDir, minimalPkg).
 			Runner().

test/testdata/configs/errcheck_exclude.txt

@@ -0,0 +1 @@
+io/ioutil.ReadFile

test/testdata/configs/errcheck_exclude.yml

@@ -1,4 +1,4 @@
 linters-settings:
   errcheck:
     check-blank: true
-    exclude: testdata/errcheck/exclude.txt
+    exclude: errcheck_exclude.txt

test/testdata/errcheck/exclude.txt

@@ -1 +0,0 @@
-io/ioutil.ReadFile

test/testdata/errcheck_exclude_functions.go

@@ -1,5 +1,5 @@
 //golangcitest:args -Eerrcheck
-//golangcitest:config_path testdata/errcheck/exclude_functions.yml
+//golangcitest:config_path testdata/configs/exclude_functions.yml
 package testdata
 
 import (

test/testdata/errcheck_ignore.go

@@ -1,5 +1,5 @@
 //golangcitest:args -Eerrcheck
-//golangcitest:config_path testdata/errcheck/ignore_config.yml
+//golangcitest:config_path testdata/configs/ignore_config.yml
 package testdata
 
 import (

test/testdata/govet_custom_formatter.go

@@ -1,3 +1,5 @@
+//golangcitest:args -Egovet
+//golangcitest:expected_exitcode 0
 package main
 
 import (

test/testshared/runner.go

@@ -301,10 +301,13 @@ func (r *RunnerResult) ExpectOutputRegexp(s string) *RunnerResult {
 	return r
 }
 
-func (r *RunnerResult) ExpectOutputContains(s string) *RunnerResult {
+func (r *RunnerResult) ExpectOutputContains(s ...string) *RunnerResult {
 	r.tb.Helper()
 
-	assert.Contains(r.tb, r.output, normalizeFilePath(s), "exit code is %d", r.exitCode)
+	for _, expected := range s {
+		assert.Contains(r.tb, r.output, normalizeFilePath(expected), "exit code is %d", r.exitCode)
+	}
+
 	return r
 }