Add tparallel linter (#1380)

go.mod

@@ -34,6 +34,7 @@ require (
 	github.com/mattn/go-colorable v0.1.7
 	github.com/mitchellh/go-homedir v1.1.0
 	github.com/mitchellh/go-ps v1.0.0
+	github.com/moricho/tparallel v0.2.1
 	github.com/nakabonne/nestif v0.3.0
 	github.com/nishanths/exhaustive v0.0.0-20200811152831-6cf413ae40e0
 	github.com/pkg/errors v0.9.1

pkg/golinters/tparallel.go

@@ -0,0 +1,21 @@
+package golinters
+
+import (
+	"github.com/moricho/tparallel"
+	"golang.org/x/tools/go/analysis"
+
+	"github.com/golangci/golangci-lint/pkg/golinters/goanalysis"
+)
+
+func NewTparallel() *goanalysis.Linter {
+	analyzers := []*analysis.Analyzer{
+		tparallel.Analyzer,
+	}
+
+	return goanalysis.NewLinter(
+		"tparallel",
+		"tparallel detects inappropriate usage of t.Parallel() method in your Go test codes",
+		analyzers,
+		nil,
+	).WithLoadMode(goanalysis.LoadModeTypesInfo)
+}

pkg/lint/lintersdb/manager.go

@@ -56,8 +56,10 @@ func (m *Manager) WithCustomLinters() *Manager {
 }
 
 func (Manager) AllPresets() []string {
-	return []string{linter.PresetBugs, linter.PresetComplexity, linter.PresetFormatting,
-		linter.PresetPerformance, linter.PresetStyle, linter.PresetUnused}
+	return []string{
+		linter.PresetBugs, linter.PresetComplexity, linter.PresetFormatting,
+		linter.PresetPerformance, linter.PresetStyle, linter.PresetUnused,
+	}
 }
 
 func (m Manager) allPresetsSet() map[string]bool {
@@ -305,6 +307,10 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
 			WithPresets(linter.PresetStyle).
 			WithLoadForGoAnalysis().
 			WithURL("https://github.com/ssgreg/nlreturn"),
+		linter.NewConfig(golinters.NewTparallel()).
+			WithPresets(linter.PresetStyle).
+			WithLoadForGoAnalysis().
+			WithURL("https://github.com/moricho/tparallel"),
 		// nolintlint must be last because it looks at the results of all the previous linters for unused nolint directives
 		linter.NewConfig(golinters.NewNoLintLint()).
 			WithPresets(linter.PresetStyle).

test/linters_test.go

@@ -252,11 +252,55 @@ func TestExtractRunContextFromComments(t *testing.T) {
 	assert.Equal(t, []string{"-Egoimports"}, rc.args)
 }
 
-func TestGolintConsumesXTestFiles(t *testing.T) {
-	dir := getTestDataDir("withxtest")
-	const expIssue = "`if` block ends with a `return` statement, so drop this `else` and outdent its block"
+func TestTparallel(t *testing.T) {
+	t.Run("should fail on missing top-level Parallel()", func(t *testing.T) {
+		sourcePath := filepath.Join(testdataDir, "tparallel", "missing_toplevel_test.go")
+		args := []string{
+			"--disable-all", "--print-issued-lines=false", "--print-linter-name=false", "--out-format=line-number", "--enable", "tparallel",
+			sourcePath,
+		}
+		rc := extractRunContextFromComments(t, sourcePath)
+		args = append(args, rc.args...)
+
+		cfg, err := yaml.Marshal(rc.config)
+		assert.NoError(t, err)
+
+		testshared.NewLintRunner(t).RunWithYamlConfig(string(cfg), args...).
+			ExpectHasIssue(
+				"testdata/tparallel/missing_toplevel_test.go:7:6: TestTopLevel should call t.Parallel on the top level as well as its subtests\n",
+			)
+	})
+
+	t.Run("should fail on missing subtest Parallel()", func(t *testing.T) {
+		sourcePath := filepath.Join(testdataDir, "tparallel", "missing_subtest_test.go")
+		args := []string{
+			"--disable-all", "--print-issued-lines=false", "--print-linter-name=false", "--out-format=line-number", "--enable", "tparallel",
+			sourcePath,
+		}
+		rc := extractRunContextFromComments(t, sourcePath)
+		args = append(args, rc.args...)
+
+		cfg, err := yaml.Marshal(rc.config)
+		assert.NoError(t, err)
+
+		testshared.NewLintRunner(t).RunWithYamlConfig(string(cfg), args...).
+			ExpectHasIssue(
+				"testdata/tparallel/missing_subtest_test.go:7:6: TestSubtests's subtests should call t.Parallel\n",
+			)
+	})
+
+	t.Run("should pass on parallel test with no subtests", func(t *testing.T) {
+		sourcePath := filepath.Join(testdataDir, "tparallel", "happy_path_test.go")
+		args := []string{
+			"--disable-all", "--print-issued-lines=false", "--print-linter-name=false", "--out-format=line-number", "--enable", "tparallel",
+			sourcePath,
+		}
+		rc := extractRunContextFromComments(t, sourcePath)
+		args = append(args, rc.args...)
+
+		cfg, err := yaml.Marshal(rc.config)
+		assert.NoError(t, err)
 
-	r := testshared.NewLintRunner(t)
-	r.Run("--no-config", "--disable-all", "-Egolint", dir).ExpectHasIssue(expIssue)
-	r.Run("--no-config", "--disable-all", "-Egolint", filepath.Join(dir, "p_test.go")).ExpectHasIssue(expIssue)
+		testshared.NewLintRunner(t).RunWithYamlConfig(string(cfg), args...).ExpectNoIssues()
+	})
 }

test/testdata/tparallel/happy_path_test.go

@@ -0,0 +1,16 @@
+package testdata
+
+import (
+	"testing"
+)
+
+func TestValidHappyPath(t *testing.T) {
+	t.Parallel()
+	t.Run("", func(t *testing.T) {
+		t.Parallel()
+	})
+}
+
+func TestValidNoSubTest(t *testing.T) {
+	t.Parallel()
+}

test/testdata/tparallel/missing_subtest_test.go

@@ -0,0 +1,12 @@
+package testdata
+
+import (
+	"testing"
+)
+
+func TestSubtests(t *testing.T) {
+	t.Parallel()
+
+	t.Run("", func(t *testing.T) {
+	})
+}

test/testdata/tparallel/missing_toplevel_test.go

@@ -0,0 +1,11 @@
+package testdata
+
+import (
+	"testing"
+)
+
+func TestTopLevel(t *testing.T) {
+	t.Run("", func(t *testing.T) {
+		t.Parallel()
+	})
+}