ruleguard/typematch: use external matcher state (#374)

This is the same trick we did with gogrep matcher.
The caller is supposed to pass the state that is not
shared between different threads.

For the most use cases, ruleguard has worker/runner based
concurrency, so it's easy to pass this state from the
worker that owns that state and doesn't share it with
other workers.

Also added some E2E tests that compile a ruleguard binary
with `-race` and run it using all test rules over the ruleguard
own source code. If any of these rules cause a data race,
this test fails.

To avoid the slower test times, I removed the `-race` from
the basic test as they do not involve any concurrent behavior
anyway, so it was just a waste of time.

Fixes #372
Fixes #368

Author: quasilyte

.gitignore

@@ -1,5 +1,5 @@
 .idea
 .vscode
-test-ruleguard
-test-ruleguard-ir
+test-ruleguard.exe
+test-ruleguard-ir.exe
 coverage.txt

Makefile

@@ -14,7 +14,8 @@ build-release:
 		./cmd/ruleguard
 
 test:
-	go test -count 3 -coverpkg=./... -coverprofile=coverage.txt -covermode=atomic -race ./...
+	go test -timeout=10m -count=1 -race -coverpkg=./... -coverprofile=coverage.txt -covermode=atomic ./...
+	go test -count=3 -run=TestE2E ./analyzer
 	cd rules && go test -v .
 	@echo "everything is OK"
 

analyzer/analyzer_test.go

@@ -51,6 +51,15 @@ var tests = []struct {
 	{name: "quickfix", quickfixes: true},
 }
 
+var ruleguardExe string
+
+func TestMain(m *testing.M) {
+	ruleguardExe = buildRuleguard()
+
+	exitCode := m.Run()
+	os.Exit(exitCode)
+}
+
 func TestDirectiveComments(t *testing.T) {
 	testdata := analysistest.TestData()
 	badDirectiveRe := regexp.MustCompile("// want `[^\\\\][^Q].*")
@@ -114,6 +123,31 @@ func TestAnalyzer(t *testing.T) {
 	}
 }
 
+func TestE2E(t *testing.T) {
+	wd, err := os.Getwd()
+	if err != nil {
+		t.Fatal(err)
+	}
+	rootPath := filepath.Join(wd, "..")
+
+	for i := range tests {
+		test := tests[i]
+		t.Run(test.name, func(t *testing.T) {
+			rulesFilename := fmt.Sprintf("./analyzer/testdata/src/%s/rules.go", test.name)
+			cmd := exec.Command(ruleguardExe, "-rules", rulesFilename, "./...") // nolint:gosec
+			cmd.Dir = rootPath
+			cmd.Env = append([]string{}, os.Environ()...)
+			cmd.Env = append(cmd.Env, "GORACE=halt_on_error=1 exitcode=39")
+			out, err := cmd.CombinedOutput()
+			if exitError, ok := err.(*exec.ExitError); ok {
+				if exitError.ExitCode() == 39 {
+					t.Fatalf("found a data race!\n%s", out)
+				}
+			}
+		})
+	}
+}
+
 func TestPrintIR(t *testing.T) {
 	analyzerTemplate := `
 package main
@@ -137,18 +171,6 @@ var rulesFile = %s
 		t.Fatal(err)
 	}
 
-	{
-		args := []string{
-			"build",
-			"-o", "test-ruleguard",
-			filepath.Join(wd, "..", "cmd", "ruleguard"),
-		}
-		out, err := exec.Command("go", args...).CombinedOutput()
-		if err != nil {
-			t.Fatalf("build go-ruleguard: %v: %s", err, out)
-		}
-	}
-
 	for i := range tests {
 		test := tests[i]
 		if test.flags != nil {
@@ -194,14 +216,14 @@ var rulesFile = %s
 				t.Fatal(err)
 			}
 
-			srcRulesCmd := exec.Command(filepath.Join(wd, "test-ruleguard"), "-rules", rulesFilename, "./...") // nolint:gosec
+			srcRulesCmd := exec.Command(ruleguardExe, "-rules", rulesFilename, "./...") // nolint:gosec
 			srcRulesCmd.Dir = filepath.Join(wd, "testdata", "src", test.name)
 			srcOut, _ := srcRulesCmd.CombinedOutput()
 
 			{
 				args := []string{
 					"build",
-					"-o", "test-ruleguard-ir",
+					"-o", "test-ruleguard-ir.exe",
 					mainFile.Name(),
 				}
 				out, err := exec.Command("go", args...).CombinedOutput()
@@ -210,7 +232,7 @@ var rulesFile = %s
 				}
 			}
 
-			irRulesCmd := exec.Command(filepath.Join(wd, "test-ruleguard-ir"), "./...") // nolint:gosec
+			irRulesCmd := exec.Command(filepath.Join(wd, "test-ruleguard-ir.exe"), "./...") // nolint:gosec
 			irRulesCmd.Dir = filepath.Join(wd, "testdata", "src", test.name)
 			irOut, _ := irRulesCmd.CombinedOutput()
 
@@ -221,3 +243,23 @@ var rulesFile = %s
 		})
 	}
 }
+
+func buildRuleguard() string {
+	wd, err := os.Getwd()
+	if err != nil {
+		panic(err)
+	}
+
+	args := []string{
+		"build",
+		"-o", "test-ruleguard.exe",
+		"-race",
+		filepath.Join(wd, "..", "cmd", "ruleguard"),
+	}
+	out, err := exec.Command("go", args...).CombinedOutput()
+	if err != nil {
+		panic(fmt.Sprintf("build go-ruleguard: %v: %s", err, out))
+	}
+
+	return filepath.Join(wd, "test-ruleguard.exe")
+}

analyzer/testdata/src/regression/issue372.go

@@ -0,0 +1,6 @@
+package regression
+
+func _() {
+	_ = map[string]int{}       // want `\Qcreating a map`
+	_ = make(map[int][]string) // want `\Qcreating a map`
+}

analyzer/testdata/src/regression/rules.go

@@ -71,3 +71,9 @@ func issue360(m dsl.Matcher) {
 		Report(`don't use strings.Compare`).
 		At(m["s1"])
 }
+
+func issue372(m dsl.Matcher) {
+	m.Match("$x{}", "make($x)").
+		Where(m["x"].Type.Is("map[$k]$v")).
+		Report("creating a map")
+}

ruleguard/filters.go

@@ -292,11 +292,11 @@ func makeTypeIsFilter(src, varname string, underlying bool, pat *typematch.Patte
 		return func(params *filterParams) matchFilterResult {
 			if list, ok := params.subNode(varname).(gogrep.ExprSlice); ok {
 				return exprListFilterApply(src, list, func(x ast.Expr) bool {
-					return pat.MatchIdentical(params.typeofNode(x).Underlying())
+					return pat.MatchIdentical(params.typematchState, params.typeofNode(x).Underlying())
 				})
 			}
 			typ := params.typeofNode(params.subNode(varname)).Underlying()
-			if pat.MatchIdentical(typ) {
+			if pat.MatchIdentical(params.typematchState, typ) {
 				return filterSuccess
 			}
 			return filterFailure(src)
@@ -306,11 +306,11 @@ func makeTypeIsFilter(src, varname string, underlying bool, pat *typematch.Patte
 	return func(params *filterParams) matchFilterResult {
 		if list, ok := params.subNode(varname).(gogrep.ExprSlice); ok {
 			return exprListFilterApply(src, list, func(x ast.Expr) bool {
-				return pat.MatchIdentical(params.typeofNode(x))
+				return pat.MatchIdentical(params.typematchState, params.typeofNode(x))
 			})
 		}
 		typ := params.typeofNode(params.subNode(varname))
-		if pat.MatchIdentical(typ) {
+		if pat.MatchIdentical(params.typematchState, typ) {
 			return filterSuccess
 		}
 		return filterFailure(src)

ruleguard/gorule.go

@@ -7,6 +7,7 @@ import (
 	"regexp"
 
 	"github.com/quasilyte/go-ruleguard/ruleguard/quasigo"
+	"github.com/quasilyte/go-ruleguard/ruleguard/typematch"
 	"github.com/quasilyte/gogrep"
 	"github.com/quasilyte/gogrep/nodetag"
 )
@@ -60,6 +61,7 @@ type filterParams struct {
 
 	importer       *goImporter
 	gogrepSubState *gogrep.MatcherState
+	typematchState *typematch.MatcherState
 
 	match    matchData
 	nodePath *nodePath

ruleguard/runner.go

@@ -17,6 +17,7 @@ import (
 
 	"github.com/quasilyte/go-ruleguard/ruleguard/goutil"
 	"github.com/quasilyte/go-ruleguard/ruleguard/profiling"
+	"github.com/quasilyte/go-ruleguard/ruleguard/typematch"
 	"github.com/quasilyte/gogrep"
 	"github.com/quasilyte/gogrep/nodetag"
 )
@@ -80,9 +81,10 @@ func newRulesRunner(ctx *RunContext, buildContext *build.Context, state *engineS
 		nodePath:       newNodePath(),
 		truncateLen:    ctx.TruncateLen,
 		filterParams: filterParams{
-			env:      state.env.GetEvalEnv(),
-			importer: importer,
-			ctx:      ctx,
+			typematchState: typematch.NewMatcherState(),
+			env:            state.env.GetEvalEnv(),
+			importer:       importer,
+			ctx:            ctx,
 		},
 	}
 	if ctx.TruncateLen == 0 {