Skip to content

Commit 451d089

Browse files
quasilytequasilyte
authored
ruleguard: give error message when filter uses undefined var (#282)
Fixes #159
1 parent 7b21d77 commit 451d089

11 files changed

+134
-54
lines changed

‎internal/gogrep/compile.go

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,12 @@ type compiler struct {
1919
strict bool
2020
fset *token.FileSet
2121

22+
info *PatternInfo
23+
2224
insideStmtList bool
2325
}
2426

25-
func (c *compiler) Compile(fset *token.FileSet, root ast.Node, strict bool) (p *program, err error) {
27+
func (c *compiler) Compile(fset *token.FileSet, root ast.Node, info *PatternInfo, strict bool) (p *program, err error) {
2628
defer func() {
2729
if err != nil {
2830
return
@@ -38,6 +40,7 @@ func (c *compiler) Compile(fset *token.FileSet, root ast.Node, strict bool) (p *
3840
panic(rv) // Not our panic
3941
}()
4042

43+
c.info = info
4144
c.fset = fset
4245
c.strict = strict
4346
c.prog = &program{
@@ -68,6 +71,12 @@ func (c *compiler) toUint8(n ast.Node, v int) uint8 {
6871
return uint8(v)
6972
}
7073

74+
func (c *compiler) internVar(n ast.Node, s string) uint8 {
75+
c.info.Vars[s] = struct{}{}
76+
index := c.internString(n, s)
77+
return index
78+
}
79+
7180
func (c *compiler) internString(n ast.Node, s string) uint8 {
7281
if index, ok := c.stringIndexes[s]; ok {
7382
return index
@@ -156,7 +165,7 @@ func (c *compiler) compileOptFieldList(n *ast.FieldList) {
156165
} else {
157166
c.emitInst(instruction{
158167
op: opNamedFieldNode,
159-
valueIndex: c.internString(n, info.Name),
168+
valueIndex: c.internVar(n, info.Name),
160169
})
161170
}
162171
return
@@ -406,10 +415,10 @@ func (c *compiler) compileWildIdent(n *ast.Ident, optional bool) {
406415
inst.op = pickOp(optional, opOptNode, opNodeSeq)
407416
case info.Name != "_" && !info.Seq:
408417
inst.op = opNamedNode
409-
inst.valueIndex = c.internString(n, info.Name)
418+
inst.valueIndex = c.internVar(n, info.Name)
410419
default:
411420
inst.op = pickOp(optional, opNamedOptNode, opNamedNodeSeq)
412-
inst.valueIndex = c.internString(n, info.Name)
421+
inst.valueIndex = c.internVar(n, info.Name)
413422
}
414423
c.prog.insts = append(c.prog.insts, inst)
415424
}
@@ -771,7 +780,7 @@ func (c *compiler) compileIfStmt(n *ast.IfStmt) {
771780
if info.Seq {
772781
c.prog.insts = append(c.prog.insts, instruction{
773782
op: pickOp(n.Else == nil, opIfNamedOptStmt, opIfNamedOptElseStmt),
774-
valueIndex: c.internString(ident, info.Name),
783+
valueIndex: c.internVar(ident, info.Name),
775784
})
776785
c.compileStmt(n.Body)
777786
if n.Else != nil {

‎internal/gogrep/compile_error_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func TestCompileError(t *testing.T) {
4949
for input, want := range tests {
5050
fset := token.NewFileSet()
5151
testPattern := unwrapPattern(input)
52-
_, err := Compile(fset, testPattern, isStrict(input))
52+
_, _, err := Compile(fset, testPattern, isStrict(input))
5353
if err == nil {
5454
t.Errorf("compile `%s`: expected error, got none", input)
5555
continue

‎internal/gogrep/compile_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ func TestCompileWildcard(t *testing.T) {
351351
input := test.input
352352
want := test.output
353353
fset := token.NewFileSet()
354-
p, err := Compile(fset, input, false)
354+
p, _, err := Compile(fset, input, false)
355355
if err != nil {
356356
t.Errorf("compile `%s`: %v", input, err)
357357
return
@@ -992,7 +992,8 @@ func TestCompile(t *testing.T) {
992992
fset := token.NewFileSet()
993993
n := testParseNode(t, fset, input)
994994
var c compiler
995-
p, err := c.Compile(fset, n, false)
995+
info := newPatternInfo()
996+
p, err := c.Compile(fset, n, &info, false)
996997
if err != nil {
997998
t.Errorf("compile `%s`: %v", input, err)
998999
return

‎internal/gogrep/gogrep.go

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ type Pattern struct {
3737
m *matcher
3838
}
3939

40+
type PatternInfo struct {
41+
Vars map[string]struct{}
42+
}
43+
4044
func (p *Pattern) NodeTag() nodetag.Value {
4145
return operationInfoTable[p.m.prog.insts[0].op].Tag
4246
}
@@ -55,16 +59,23 @@ func (p *Pattern) Clone() *Pattern {
5559
return &clone
5660
}
5761

58-
func Compile(fset *token.FileSet, src string, strict bool) (*Pattern, error) {
62+
func Compile(fset *token.FileSet, src string, strict bool) (*Pattern, PatternInfo, error) {
63+
info := newPatternInfo()
5964
n, err := parseExpr(fset, src)
6065
if err != nil {
61-
return nil, err
66+
return nil, info, err
6267
}
6368
var c compiler
64-
prog, err := c.Compile(fset, n, strict)
69+
prog, err := c.Compile(fset, n, &info, strict)
6570
if err != nil {
66-
return nil, err
71+
return nil, info, err
6772
}
6873
m := newMatcher(prog)
69-
return &Pattern{m: m}, nil
74+
return &Pattern{m: m}, info, nil
75+
}
76+
77+
func newPatternInfo() PatternInfo {
78+
return PatternInfo{
79+
Vars: map[string]struct{}{},
80+
}
7081
}

‎internal/gogrep/match_perf_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ func BenchmarkMatch(b *testing.B) {
163163
test := tests[i]
164164
b.Run(test.name, func(b *testing.B) {
165165
fset := token.NewFileSet()
166-
pat, err := Compile(fset, test.pat, true)
166+
pat, _, err := Compile(fset, test.pat, true)
167167
if err != nil {
168168
b.Errorf("parse `%s`: %v", test.pat, err)
169169
return

‎internal/gogrep/match_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -852,7 +852,7 @@ func TestMatch(t *testing.T) {
852852
t.Run(fmt.Sprintf("test%d", i), func(t *testing.T) {
853853
fset := token.NewFileSet()
854854
testPattern := unwrapPattern(test.pat)
855-
pat, err := Compile(fset, testPattern, isStrict(test.pat))
855+
pat, _, err := Compile(fset, testPattern, isStrict(test.pat))
856856
if err != nil {
857857
t.Errorf("compile `%s`: %v", test.pat, err)
858858
return

‎ruleguard/ir/filter_op.gen.go

Lines changed: 27 additions & 10 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: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ type opInfo struct {
2020
const (
2121
flagIsBinaryExpr uint64 = 1 << iota
2222
flagIsBasicLit
23+
flagHasVar
2324
)
2425

2526
func main() {
@@ -38,24 +39,24 @@ func main() {
3839
{name: "GtEq", comment: "$Args[0] >= $Args[1]", flags: flagIsBinaryExpr},
3940
{name: "LtEq", comment: "$Args[0] <= $Args[1]", flags: flagIsBinaryExpr},
4041

41-
{name: "VarAddressable", comment: "m[$Value].Addressable", valueType: "string"},
42-
{name: "VarPure", comment: "m[$Value].Pure", valueType: "string"},
43-
{name: "VarConst", comment: "m[$Value].Const", valueType: "string"},
44-
{name: "VarConstSlice", comment: "m[$Value].ConstSlice", valueType: "string"},
45-
{name: "VarText", comment: "m[$Value].Text", valueType: "string"},
46-
{name: "VarLine", comment: "m[$Value].Line", valueType: "string"},
47-
{name: "VarValueInt", comment: "m[$Value].Value.Int()", valueType: "string"},
48-
{name: "VarTypeSize", comment: "m[$Value].Type.Size", valueType: "string"},
49-
50-
{name: "VarFilter", comment: "m[$Value].Filter($Args[0])", valueType: "string"},
51-
{name: "VarNodeIs", comment: "m[$Value].Node.Is($Args[0])", valueType: "string"},
52-
{name: "VarObjectIs", comment: "m[$Value].Object.Is($Args[0])", valueType: "string"},
53-
{name: "VarTypeIs", comment: "m[$Value].Type.Is($Args[0])", valueType: "string"},
54-
{name: "VarTypeUnderlyingIs", comment: "m[$Value].Type.Underlying().Is($Args[0])", valueType: "string"},
55-
{name: "VarTypeConvertibleTo", comment: "m[$Value].Type.ConvertibleTo($Args[0])", valueType: "string"},
56-
{name: "VarTypeAssignableTo", comment: "m[$Value].Type.AssignableTo($Args[0])", valueType: "string"},
57-
{name: "VarTypeImplements", comment: "m[$Value].Type.Implements($Args[0])", valueType: "string"},
58-
{name: "VarTextMatches", comment: "m[$Value].Text.Matches($Args[0])", valueType: "string"},
42+
{name: "VarAddressable", comment: "m[$Value].Addressable", valueType: "string", flags: flagHasVar},
43+
{name: "VarPure", comment: "m[$Value].Pure", valueType: "string", flags: flagHasVar},
44+
{name: "VarConst", comment: "m[$Value].Const", valueType: "string", flags: flagHasVar},
45+
{name: "VarConstSlice", comment: "m[$Value].ConstSlice", valueType: "string", flags: flagHasVar},
46+
{name: "VarText", comment: "m[$Value].Text", valueType: "string", flags: flagHasVar},
47+
{name: "VarLine", comment: "m[$Value].Line", valueType: "string", flags: flagHasVar},
48+
{name: "VarValueInt", comment: "m[$Value].Value.Int()", valueType: "string", flags: flagHasVar},
49+
{name: "VarTypeSize", comment: "m[$Value].Type.Size", valueType: "string", flags: flagHasVar},
50+
51+
{name: "VarFilter", comment: "m[$Value].Filter($Args[0])", valueType: "string", flags: flagHasVar},
52+
{name: "VarNodeIs", comment: "m[$Value].Node.Is($Args[0])", valueType: "string", flags: flagHasVar},
53+
{name: "VarObjectIs", comment: "m[$Value].Object.Is($Args[0])", valueType: "string", flags: flagHasVar},
54+
{name: "VarTypeIs", comment: "m[$Value].Type.Is($Args[0])", valueType: "string", flags: flagHasVar},
55+
{name: "VarTypeUnderlyingIs", comment: "m[$Value].Type.Underlying().Is($Args[0])", valueType: "string", flags: flagHasVar},
56+
{name: "VarTypeConvertibleTo", comment: "m[$Value].Type.ConvertibleTo($Args[0])", valueType: "string", flags: flagHasVar},
57+
{name: "VarTypeAssignableTo", comment: "m[$Value].Type.AssignableTo($Args[0])", valueType: "string", flags: flagHasVar},
58+
{name: "VarTypeImplements", comment: "m[$Value].Type.Implements($Args[0])", valueType: "string", flags: flagHasVar},
59+
{name: "VarTextMatches", comment: "m[$Value].Text.Matches($Args[0])", valueType: "string", flags: flagHasVar},
5960

6061
{name: "Deadcode", comment: "m.Deadcode()"},
6162

@@ -117,6 +118,9 @@ func main() {
117118
if op.flags&flagIsBasicLit != 0 {
118119
parts = append(parts, "flagIsBasicLit")
119120
}
121+
if op.flags&flagHasVar != 0 {
122+
parts = append(parts, "flagHasVar")
123+
}
120124
fmt.Fprintf(&buf, "Filter%sOp: %s,\n", op.name, strings.Join(parts, " | "))
121125
}
122126
buf.WriteString("}\n")

‎ruleguard/ir/ir.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ func (e FilterExpr) IsValid() bool { return e.Op != FilterInvalidOp }
8282

8383
func (e FilterExpr) IsBinaryExpr() bool { return filterOpFlags[e.Op]&flagIsBinaryExpr != 0 }
8484
func (e FilterExpr) IsBasicLit() bool { return filterOpFlags[e.Op]&flagIsBasicLit != 0 }
85+
func (e FilterExpr) HasVar() bool { return filterOpFlags[e.Op]&flagHasVar != 0 }
8586

8687
func (e FilterExpr) String() string {
8788
switch e.Op {
@@ -107,4 +108,5 @@ func (e FilterExpr) String() string {
107108
const (
108109
flagIsBinaryExpr uint64 = 1 << iota
109110
flagIsBasicLit
111+
flagHasVar
110112
)

‎ruleguard/ir_loader.go

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -270,16 +270,19 @@ func (l *irLoader) loadRule(rule ir.Rule) error {
270270
location: rule.LocationVar,
271271
}
272272

273+
info := filterInfo{
274+
Vars: make(map[string]struct{}),
275+
}
273276
if rule.WhereExpr.IsValid() {
274-
filter, err := l.newFilter(rule.WhereExpr)
277+
filter, err := l.newFilter(rule.WhereExpr, &info)
275278
if err != nil {
276279
return err
277280
}
278281
proto.filter = filter
279282
}
280283

281284
for _, pat := range rule.SyntaxPatterns {
282-
if err := l.loadSyntaxRule(proto, rule, pat.Value, pat.Line); err != nil {
285+
if err := l.loadSyntaxRule(proto, info, rule, pat.Value, pat.Line); err != nil {
283286
return err
284287
}
285288
}
@@ -309,16 +312,26 @@ func (l *irLoader) loadCommentRule(resultProto goRule, rule ir.Rule, src string,
309312
return nil
310313
}
311314

312-
func (l *irLoader) loadSyntaxRule(resultProto goRule, rule ir.Rule, src string, line int) error {
315+
func (l *irLoader) loadSyntaxRule(resultProto goRule, filterInfo filterInfo, rule ir.Rule, src string, line int) error {
313316
result := resultProto
314317
result.line = line
315318

316-
pat, err := gogrep.Compile(l.gogrepFset, src, false)
319+
pat, info, err := gogrep.Compile(l.gogrepFset, src, false)
317320
if err != nil {
318321
return l.errorf(rule.Line, err, "parse match pattern")
319322
}
320323
result.pat = pat
321324

325+
for filterVar := range filterInfo.Vars {
326+
if filterVar == "$$" {
327+
continue // OK: a predefined var for the "entire match"
328+
}
329+
_, ok := info.Vars[filterVar]
330+
if !ok {
331+
return l.errorf(rule.Line, nil, "filter refers to a non-existing var %s", filterVar)
332+
}
333+
}
334+
322335
dst := l.res.universal
323336
var dstTags []nodetag.Value
324337
switch tag := pat.NodeTag(); tag {
@@ -441,16 +454,20 @@ func (l *irLoader) unwrapStringExpr(filter ir.FilterExpr) string {
441454
return ""
442455
}
443456

444-
func (l *irLoader) newFilter(filter ir.FilterExpr) (matchFilter, error) {
457+
func (l *irLoader) newFilter(filter ir.FilterExpr, info *filterInfo) (matchFilter, error) {
458+
if filter.HasVar() {
459+
info.Vars[filter.Value.(string)] = struct{}{}
460+
}
461+
445462
if filter.IsBinaryExpr() {
446-
return l.newBinaryExprFilter(filter)
463+
return l.newBinaryExprFilter(filter, info)
447464
}
448465

449466
result := matchFilter{src: filter.Src}
450467

451468
switch filter.Op {
452469
case ir.FilterNotOp:
453-
x, err := l.newFilter(filter.Args[0])
470+
x, err := l.newFilter(filter.Args[0], info)
454471
if err != nil {
455472
return result, err
456473
}
@@ -600,14 +617,14 @@ func (l *irLoader) newFilter(filter ir.FilterExpr) (matchFilter, error) {
600617
return result, nil
601618
}
602619

603-
func (l *irLoader) newBinaryExprFilter(filter ir.FilterExpr) (matchFilter, error) {
620+
func (l *irLoader) newBinaryExprFilter(filter ir.FilterExpr, info *filterInfo) (matchFilter, error) {
604621
if filter.Op == ir.FilterAndOp || filter.Op == ir.FilterOrOp {
605622
result := matchFilter{src: filter.Src}
606-
lhs, err := l.newFilter(filter.Args[0])
623+
lhs, err := l.newFilter(filter.Args[0], info)
607624
if err != nil {
608625
return result, err
609626
}
610-
rhs, err := l.newFilter(filter.Args[1])
627+
rhs, err := l.newFilter(filter.Args[1], info)
611628
if err != nil {
612629
return result, err
613630
}
@@ -631,7 +648,7 @@ func (l *irLoader) newBinaryExprFilter(filter ir.FilterExpr) (matchFilter, error
631648
// Simple commutative ops. Just swap the args.
632649
newFilter := filter
633650
newFilter.Args = []ir.FilterExpr{filter.Args[1], filter.Args[0]}
634-
return l.newBinaryExprFilter(newFilter)
651+
return l.newBinaryExprFilter(newFilter, info)
635652
}
636653
}
637654
}
@@ -697,3 +714,7 @@ func (l *irLoader) newBinaryExprFilter(filter ir.FilterExpr) (matchFilter, error
697714

698715
return result, nil
699716
}
717+
718+
type filterInfo struct {
719+
Vars map[string]struct{}
720+
}

0 commit comments

Comments
 (0)