Skip to content

Commit ff2e115

Browse files
authored
all: implement sub-matches support (#363)
Fixes #28
1 parent cac87d4 commit ff2e115

File tree

9 files changed

+172
-21
lines changed

9 files changed

+172
-21
lines changed

analyzer/testdata/src/matching/file.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package matching
22

33
func sink(args ...interface{}) {}
4+
func expensive() {}
45

56
func multiexpr() (int, int, int) {
67
sink(1, 1) // want `\Qrepeated expression in list`
@@ -52,3 +53,74 @@ func rangeClause() {
5253
}
5354
}
5455
}
56+
57+
func submatchContains() {
58+
{
59+
type Book struct {
60+
AuthorID int
61+
}
62+
var books []Book
63+
m := make(map[int][]Book)
64+
for _, b := range books {
65+
m[b.AuthorID] = append(m[b.AuthorID], b) // want `\Qm[b.AuthorID] contains b`
66+
}
67+
}
68+
69+
{
70+
var b1 []byte
71+
var b2 []byte
72+
copy(b1, b2)
73+
copy(b1[:], b2) // want `\Qcopy() contains a slicing operation`
74+
copy(b1, b2[:]) // want `\Qcopy() contains a slicing operation`
75+
copy(b1[:], b2[:]) // want `\Qcopy() contains a slicing operation`
76+
}
77+
78+
_ = func() error {
79+
var err error
80+
sink(err) // want `\Qsink(err) call not followed by return err`
81+
return nil
82+
}
83+
_ = func() error {
84+
var err error
85+
sink(err)
86+
return err
87+
}
88+
_ = func() error {
89+
var err2 error
90+
sink(err2) // want `\Qsink(err2) call not followed by return err2`
91+
return nil
92+
}
93+
_ = func() error {
94+
var err2 error
95+
sink(err2)
96+
return err2
97+
}
98+
99+
for { // want `\Qexpensive call inside a loop`
100+
expensive()
101+
}
102+
var cond bool
103+
for { // want `\Qexpensive call inside a loop`
104+
if cond {
105+
expensive()
106+
}
107+
}
108+
for {
109+
if cond {
110+
}
111+
}
112+
for { // want `\Qexpensive call inside a loop`
113+
for { // want `\Qexpensive call inside a loop`
114+
expensive()
115+
}
116+
}
117+
118+
{
119+
type object struct {
120+
val int
121+
}
122+
var objects []object
123+
if objects != nil && objects[0].val != 0 { // want `\Qnil check may not be enough to access objects[0], check for len`
124+
}
125+
}
126+
}

analyzer/testdata/src/matching/rules.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,24 @@ func testRules(m dsl.Matcher) {
1111
m.Match(`range $x[:]`).
1212
Where(m["x"].Type.Is(`[]$_`)).
1313
Report(`redundant slicing of a range expression`)
14+
15+
m.Match(`$lhs = append($lhs, $x)`).
16+
Where(m["lhs"].Contains(`$x`)).
17+
Report(`$lhs contains $x`)
18+
19+
m.Match(`copy($*args)`).
20+
Where(m["args"].Contains(`$_[:]`)).
21+
Report(`copy() contains a slicing operation`)
22+
23+
m.Match(`sink($err); $x`).
24+
Where(!m["x"].Contains(`return $err`) && m["err"].Type.Is(`error`)).
25+
Report(`sink($err) call not followed by return $err`)
26+
27+
m.Match(`for { $*body }`).
28+
Where(m["body"].Contains(`expensive()`)).
29+
Report(`expensive call inside a loop`)
30+
31+
m.Match(`$x != nil && $y`).
32+
Where(m["y"].Contains(`$x[0]`)).
33+
Report(`nil check may not be enough to access $x[0], check for len`)
1434
}

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ go 1.17
55
require (
66
github.com/go-toolsmith/astcopy v1.0.0
77
github.com/google/go-cmp v0.5.6
8-
github.com/quasilyte/go-ruleguard/dsl v0.3.13
8+
github.com/quasilyte/go-ruleguard/dsl v0.3.14
99
github.com/quasilyte/go-ruleguard/rules v0.0.0-20211022131956-028d6511ab71
10-
github.com/quasilyte/gogrep v0.0.0-20220104185649-039753a3dd32
10+
github.com/quasilyte/gogrep v0.0.0-20220120141003-628d8b3623b5
1111
golang.org/x/tools v0.1.9-0.20211228192929-ee1ca4ffc4da
1212
)
1313

go.sum

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,15 @@ github.com/quasilyte/go-ruleguard v0.3.1-0.20210203134552-1b5a410e1cc8/go.mod h1
1212
github.com/quasilyte/go-ruleguard/dsl v0.3.0/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU=
1313
github.com/quasilyte/go-ruleguard/dsl v0.3.13 h1:WmtzUkp28TMarzfBCogPf7plyI/2gsNsj8CgZ9ihPCM=
1414
github.com/quasilyte/go-ruleguard/dsl v0.3.13/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU=
15+
github.com/quasilyte/go-ruleguard/dsl v0.3.14 h1:diesHrFHZ6rxuFltuwiW7NRQaqUIypuSSmUulRCNuqM=
16+
github.com/quasilyte/go-ruleguard/dsl v0.3.14/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU=
1517
github.com/quasilyte/go-ruleguard/rules v0.0.0-20201231183845-9e62ed36efe1/go.mod h1:7JTjp89EGyU1d6XfBiXihJNG37wB2VRkd125Q1u7Plc=
1618
github.com/quasilyte/go-ruleguard/rules v0.0.0-20211022131956-028d6511ab71 h1:CNooiryw5aisadVfzneSZPswRWvnVW8hF1bS/vo8ReI=
1719
github.com/quasilyte/go-ruleguard/rules v0.0.0-20211022131956-028d6511ab71/go.mod h1:4cgAphtvu7Ftv7vOT2ZOYhC6CvBxZixcasr8qIOTA50=
1820
github.com/quasilyte/gogrep v0.0.0-20220104185649-039753a3dd32 h1:fJhpG5LYGnHZqUIDULZkvQKJfdtAefNrkoiGCezlr7g=
1921
github.com/quasilyte/gogrep v0.0.0-20220104185649-039753a3dd32/go.mod h1:wSEyW6O61xRV6zb6My3HxrQ5/8ke7NE2OayqCHa3xRM=
22+
github.com/quasilyte/gogrep v0.0.0-20220120141003-628d8b3623b5 h1:PDWGei+Rf2bBiuZIbZmM20J2ftEy9IeUCHA8HbQqed8=
23+
github.com/quasilyte/gogrep v0.0.0-20220120141003-628d8b3623b5/go.mod h1:wSEyW6O61xRV6zb6My3HxrQ5/8ke7NE2OayqCHa3xRM=
2024
github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
2125
github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
2226
github.com/yuin/goldmark v1.4.1/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=

ruleguard/filters.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,28 @@ func makeAddressableFilter(src, varname string) filterFunc {
159159
}
160160
}
161161

162+
func makeVarContainsFilter(src, varname string, pat *gogrep.Pattern) filterFunc {
163+
// TODO: use a shared state here as well?
164+
state := gogrep.NewMatcherState()
165+
return func(params *filterParams) matchFilterResult {
166+
state.CapturePreset = params.match.CaptureList()
167+
matched := false
168+
gogrep.Walk(params.subNode(varname), func(n ast.Node) bool {
169+
if matched {
170+
return false
171+
}
172+
pat.MatchNode(&state, n, func(m gogrep.MatchData) {
173+
matched = true
174+
})
175+
return true
176+
})
177+
if matched {
178+
return filterSuccess
179+
}
180+
return filterFailure(src)
181+
}
182+
}
183+
162184
func makeCustomVarFilter(src, varname string, fn *quasigo.Func) filterFunc {
163185
return func(params *filterParams) matchFilterResult {
164186
// TODO(quasilyte): what if bytecode function panics due to the programming error?

ruleguard/ir/filter_op.gen.go

Lines changed: 19 additions & 13 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

ruleguard/ir/gen_filter_op.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ func main() {
6262
{name: "VarTypeHasMethod", comment: "m[$Value].Type.HasMethod($Args[0])", valueType: "string", flags: flagHasVar},
6363
{name: "VarTextMatches", comment: "m[$Value].Text.Matches($Args[0])", valueType: "string", flags: flagHasVar},
6464

65+
{name: "VarContains", comment: "m[$Value].Contains($Args[0])", valueType: "string", flags: flagHasVar},
66+
6567
{name: "Deadcode", comment: "m.Deadcode()"},
6668

6769
{name: "GoVersionEq", comment: "m.GoVersion().Eq($Value)", valueType: "string"},

ruleguard/ir_loader.go

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,8 @@ func (l *irLoader) loadRule(group *ir.RuleGroup, rule *ir.Rule) error {
272272
}
273273

274274
info := filterInfo{
275-
Vars: make(map[string]struct{}),
275+
Vars: make(map[string]struct{}),
276+
group: group,
276277
}
277278
if rule.WhereExpr.IsValid() {
278279
filter, err := l.newFilter(rule.WhereExpr, &info)
@@ -313,10 +314,7 @@ func (l *irLoader) loadCommentRule(resultProto goRule, rule *ir.Rule, src string
313314
return nil
314315
}
315316

316-
func (l *irLoader) loadSyntaxRule(group *ir.RuleGroup, resultProto goRule, filterInfo filterInfo, rule *ir.Rule, src string, line int) error {
317-
result := resultProto
318-
result.line = line
319-
317+
func (l *irLoader) gogrepCompile(group *ir.RuleGroup, src string) (*gogrep.Pattern, gogrep.PatternInfo, error) {
320318
var imports map[string]string
321319
if len(group.Imports) != 0 {
322320
imports = make(map[string]string)
@@ -332,7 +330,14 @@ func (l *irLoader) loadSyntaxRule(group *ir.RuleGroup, resultProto goRule, filte
332330
WithTypes: true,
333331
Imports: imports,
334332
}
335-
pat, info, err := gogrep.Compile(gogrepConfig)
333+
return gogrep.Compile(gogrepConfig)
334+
}
335+
336+
func (l *irLoader) loadSyntaxRule(group *ir.RuleGroup, resultProto goRule, filterInfo filterInfo, rule *ir.Rule, src string, line int) error {
337+
result := resultProto
338+
result.line = line
339+
340+
pat, info, err := l.gogrepCompile(group, src)
336341
if err != nil {
337342
return l.errorf(rule.Line, err, "parse match pattern")
338343
}
@@ -721,6 +726,14 @@ func (l *irLoader) newFilter(filter ir.FilterExpr, info *filterInfo) (matchFilte
721726
}
722727
result.fn = makeFileNameMatchesFilter(result.src, re)
723728

729+
case ir.FilterVarContainsOp:
730+
src := filter.Args[0].Value.(string)
731+
pat, _, err := l.gogrepCompile(info.group, src)
732+
if err != nil {
733+
return result, l.errorf(filter.Line, err, "parse contains pattern")
734+
}
735+
result.fn = makeVarContainsFilter(result.src, filter.Value.(string), pat)
736+
724737
case ir.FilterVarFilterOp:
725738
funcName := filter.Args[0].Value.(string)
726739
userFn := l.state.env.GetFunc(l.file.PkgPath, funcName)
@@ -839,4 +852,6 @@ func (l *irLoader) newBinaryExprFilter(filter ir.FilterExpr, info *filterInfo) (
839852

840853
type filterInfo struct {
841854
Vars map[string]struct{}
855+
856+
group *ir.RuleGroup
842857
}

ruleguard/irconv/irconv.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -661,6 +661,16 @@ func (conv *converter) convertFilterExprImpl(e ast.Expr) ir.FilterExpr {
661661
case "File.Name.Matches":
662662
return ir.FilterExpr{Op: ir.FilterFileNameMatchesOp, Value: conv.parseStringArg(e.Args[0])}
663663

664+
case "Contains":
665+
pat := conv.parseStringArg(e.Args[0])
666+
return ir.FilterExpr{
667+
Op: ir.FilterVarContainsOp,
668+
Value: op.varName,
669+
Args: []ir.FilterExpr{
670+
{Op: ir.FilterStringOp, Value: pat},
671+
},
672+
}
673+
664674
case "Filter":
665675
funcName, ok := e.Args[0].(*ast.Ident)
666676
if !ok {

0 commit comments

Comments
 (0)