diff --git a/.gitignore b/.gitignore index 8f6e591..aa52fa8 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,5 @@ .idea .vscode -test-ruleguard -test-ruleguard-ir +test-ruleguard.exe +test-ruleguard-ir.exe coverage.txt \ No newline at end of file diff --git a/Makefile b/Makefile index 1cea21a..551a981 100644 --- a/Makefile +++ b/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" diff --git a/analyzer/analyzer_test.go b/analyzer/analyzer_test.go index 3a1938e..6008922 100644 --- a/analyzer/analyzer_test.go +++ b/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") +} diff --git a/analyzer/testdata/src/regression/issue372.go b/analyzer/testdata/src/regression/issue372.go new file mode 100644 index 0000000..520bec6 --- /dev/null +++ b/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` +} diff --git a/analyzer/testdata/src/regression/rules.go b/analyzer/testdata/src/regression/rules.go index 3ee755b..9ab15ac 100644 --- a/analyzer/testdata/src/regression/rules.go +++ b/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") +} diff --git a/ruleguard/filters.go b/ruleguard/filters.go index 607ea27..cd0b135 100644 --- a/ruleguard/filters.go +++ b/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) diff --git a/ruleguard/gorule.go b/ruleguard/gorule.go index 63907ee..2574ac9 100644 --- a/ruleguard/gorule.go +++ b/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 diff --git a/ruleguard/runner.go b/ruleguard/runner.go index 82a40d4..8ad2393 100644 --- a/ruleguard/runner.go +++ b/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 { diff --git a/ruleguard/typematch/typematch.go b/ruleguard/typematch/typematch.go index 4363e6f..b747403 100644 --- a/ruleguard/typematch/typematch.go +++ b/ruleguard/typematch/typematch.go @@ -32,10 +32,32 @@ const ( opNamed ) -type Pattern struct { +type MatcherState struct { typeMatches map[string]types.Type int64Matches map[string]int64 +} + +func NewMatcherState() *MatcherState { + return &MatcherState{ + typeMatches: map[string]types.Type{}, + int64Matches: map[string]int64{}, + } +} + +func (state *MatcherState) reset() { + if len(state.int64Matches) != 0 { + for k := range state.int64Matches { + delete(state.int64Matches, k) + } + } + if len(state.typeMatches) != 0 { + for k := range state.typeMatches { + delete(state.typeMatches, k) + } + } +} +type Pattern struct { root *pattern } @@ -107,9 +129,7 @@ func Parse(ctx *Context, s string) (*Pattern, error) { return nil, fmt.Errorf("can't convert %s type expression", s) } p := &Pattern{ - typeMatches: map[string]types.Type{}, - int64Matches: map[string]int64{}, - root: root, + root: root, } return p, nil } @@ -345,21 +365,12 @@ func parseExpr(ctx *Context, e ast.Expr) *pattern { } // MatchIdentical returns true if the go typ matches pattern p. -func (p *Pattern) MatchIdentical(typ types.Type) bool { - p.reset() - return p.matchIdentical(p.root, typ) -} - -func (p *Pattern) reset() { - if len(p.int64Matches) != 0 { - p.int64Matches = map[string]int64{} - } - if len(p.typeMatches) != 0 { - p.typeMatches = map[string]types.Type{} - } +func (p *Pattern) MatchIdentical(state *MatcherState, typ types.Type) bool { + state.reset() + return p.matchIdentical(state, p.root, typ) } -func (p *Pattern) matchIdenticalFielder(subs []*pattern, f fielder) bool { +func (p *Pattern) matchIdenticalFielder(state *MatcherState, subs []*pattern, f fielder) bool { // TODO: do backtracking. numFields := f.NumFields() @@ -387,7 +398,7 @@ func (p *Pattern) matchIdenticalFielder(subs []*pattern, f fielder) bool { matchAny = false i++ // Lookahead for non-greedy matching. - case i+1 < len(subs) && p.matchIdentical(subs[i+1], f.Field(fieldsMatched).Type()): + case i+1 < len(subs) && p.matchIdentical(state, subs[i+1], f.Field(fieldsMatched).Type()): matchAny = false i += 2 fieldsMatched++ @@ -397,7 +408,7 @@ func (p *Pattern) matchIdenticalFielder(subs []*pattern, f fielder) bool { continue } - if fieldsLeft == 0 || !p.matchIdentical(pat, f.Field(fieldsMatched).Type()) { + if fieldsLeft == 0 || !p.matchIdentical(state, pat, f.Field(fieldsMatched).Type()) { return false } i++ @@ -407,16 +418,16 @@ func (p *Pattern) matchIdenticalFielder(subs []*pattern, f fielder) bool { return numFields == fieldsMatched } -func (p *Pattern) matchIdentical(sub *pattern, typ types.Type) bool { +func (p *Pattern) matchIdentical(state *MatcherState, sub *pattern, typ types.Type) bool { switch sub.op { case opVar: name := sub.value.(string) if name == "_" { return true } - y, ok := p.typeMatches[name] + y, ok := state.typeMatches[name] if !ok { - p.typeMatches[name] = typ + state.typeMatches[name] = typ return true } if y == nil { @@ -432,14 +443,14 @@ func (p *Pattern) matchIdentical(sub *pattern, typ types.Type) bool { if !ok { return false } - return p.matchIdentical(sub.subs[0], typ.Elem()) + return p.matchIdentical(state, sub.subs[0], typ.Elem()) case opSlice: typ, ok := typ.(*types.Slice) if !ok { return false } - return p.matchIdentical(sub.subs[0], typ.Elem()) + return p.matchIdentical(state, sub.subs[0], typ.Elem()) case opArray: typ, ok := typ.(*types.Array) @@ -453,25 +464,25 @@ func (p *Pattern) matchIdentical(sub *pattern, typ types.Type) bool { wantLen = typ.Len() break } - length, ok := p.int64Matches[v] + length, ok := state.int64Matches[v] if ok { wantLen = length } else { - p.int64Matches[v] = typ.Len() + state.int64Matches[v] = typ.Len() wantLen = typ.Len() } case int64: wantLen = v } - return wantLen == typ.Len() && p.matchIdentical(sub.subs[0], typ.Elem()) + return wantLen == typ.Len() && p.matchIdentical(state, sub.subs[0], typ.Elem()) case opMap: typ, ok := typ.(*types.Map) if !ok { return false } - return p.matchIdentical(sub.subs[0], typ.Key()) && - p.matchIdentical(sub.subs[1], typ.Elem()) + return p.matchIdentical(state, sub.subs[0], typ.Key()) && + p.matchIdentical(state, sub.subs[1], typ.Elem()) case opChan: typ, ok := typ.(*types.Chan) @@ -479,7 +490,7 @@ func (p *Pattern) matchIdentical(sub *pattern, typ types.Type) bool { return false } dir := sub.value.(types.ChanDir) - return dir == typ.Dir() && p.matchIdentical(sub.subs[0], typ.Elem()) + return dir == typ.Dir() && p.matchIdentical(state, sub.subs[0], typ.Elem()) case opNamed: typ, ok := typ.(*types.Named) @@ -515,12 +526,12 @@ func (p *Pattern) matchIdentical(sub *pattern, typ types.Type) bool { return false } for i := 0; i < typ.Params().Len(); i++ { - if !p.matchIdentical(params[i], typ.Params().At(i).Type()) { + if !p.matchIdentical(state, params[i], typ.Params().At(i).Type()) { return false } } for i := 0; i < typ.Results().Len(); i++ { - if !p.matchIdentical(results[i], typ.Results().At(i).Type()) { + if !p.matchIdentical(state, results[i], typ.Results().At(i).Type()) { return false } } @@ -535,11 +546,11 @@ func (p *Pattern) matchIdentical(sub *pattern, typ types.Type) bool { params := sub.subs[:numParams] results := sub.subs[numParams:] adapter := tupleFielder{x: typ.Params()} - if !p.matchIdenticalFielder(params, &adapter) { + if !p.matchIdenticalFielder(state, params, &adapter) { return false } adapter.x = typ.Results() - if !p.matchIdenticalFielder(results, &adapter) { + if !p.matchIdenticalFielder(state, results, &adapter) { return false } return true @@ -553,7 +564,7 @@ func (p *Pattern) matchIdentical(sub *pattern, typ types.Type) bool { return false } for i, member := range sub.subs { - if !p.matchIdentical(member, typ.Field(i).Type()) { + if !p.matchIdentical(state, member, typ.Field(i).Type()) { return false } } @@ -564,7 +575,7 @@ func (p *Pattern) matchIdentical(sub *pattern, typ types.Type) bool { if !ok { return false } - if !p.matchIdenticalFielder(sub.subs, typ) { + if !p.matchIdenticalFielder(state, sub.subs, typ) { return false } return true diff --git a/ruleguard/typematch/typematch_test.go b/ruleguard/typematch/typematch_test.go index c616266..3221c4f 100644 --- a/ruleguard/typematch/typematch_test.go +++ b/ruleguard/typematch/typematch_test.go @@ -152,13 +152,14 @@ func TestIdentical(t *testing.T) { {`struct{$*_; $x; $*_; $x; $*_}`, structType(intVar, int32Var, stringVar, intVar)}, } + state := NewMatcherState() for _, test := range tests { pat, err := Parse(testContext, test.expr) if err != nil { t.Errorf("parse('%s'): %v", test.expr, err) continue } - if !pat.MatchIdentical(test.typ) { + if !pat.MatchIdentical(state, test.typ) { t.Errorf("identical('%s', %s): expected a match", test.expr, test.typ.String()) } @@ -258,13 +259,14 @@ func TestIdenticalNegative(t *testing.T) { //{`struct{$*x; int; $*x}`, structType(stringVar, intVar, intVar)}, } + state := NewMatcherState() for _, test := range tests { pat, err := Parse(testContext, test.expr) if err != nil { t.Errorf("parse('%s'): %v", test.expr, err) continue } - if pat.MatchIdentical(test.typ) { + if pat.MatchIdentical(state, test.typ) { t.Errorf("identical('%s', %s): unexpected match", test.expr, test.typ.String()) }