Skip to content

Commit

Permalink
all: implement sub-matches support (#363)
Browse files Browse the repository at this point in the history
Fixes #28
  • Loading branch information
quasilyte committed Jan 20, 2022
1 parent cac87d4 commit ff2e115
Show file tree
Hide file tree
Showing 9 changed files with 172 additions and 21 deletions.
72 changes: 72 additions & 0 deletions analyzer/testdata/src/matching/file.go
@@ -1,6 +1,7 @@
package matching

func sink(args ...interface{}) {}
func expensive() {}

func multiexpr() (int, int, int) {
sink(1, 1) // want `\Qrepeated expression in list`
Expand Down Expand Up @@ -52,3 +53,74 @@ func rangeClause() {
}
}
}

func submatchContains() {
{
type Book struct {
AuthorID int
}
var books []Book
m := make(map[int][]Book)
for _, b := range books {
m[b.AuthorID] = append(m[b.AuthorID], b) // want `\Qm[b.AuthorID] contains b`
}
}

{
var b1 []byte
var b2 []byte
copy(b1, b2)
copy(b1[:], b2) // want `\Qcopy() contains a slicing operation`
copy(b1, b2[:]) // want `\Qcopy() contains a slicing operation`
copy(b1[:], b2[:]) // want `\Qcopy() contains a slicing operation`
}

_ = func() error {
var err error
sink(err) // want `\Qsink(err) call not followed by return err`
return nil
}
_ = func() error {
var err error
sink(err)
return err
}
_ = func() error {
var err2 error
sink(err2) // want `\Qsink(err2) call not followed by return err2`
return nil
}
_ = func() error {
var err2 error
sink(err2)
return err2
}

for { // want `\Qexpensive call inside a loop`
expensive()
}
var cond bool
for { // want `\Qexpensive call inside a loop`
if cond {
expensive()
}
}
for {
if cond {
}
}
for { // want `\Qexpensive call inside a loop`
for { // want `\Qexpensive call inside a loop`
expensive()
}
}

{
type object struct {
val int
}
var objects []object
if objects != nil && objects[0].val != 0 { // want `\Qnil check may not be enough to access objects[0], check for len`
}
}
}
20 changes: 20 additions & 0 deletions analyzer/testdata/src/matching/rules.go
Expand Up @@ -11,4 +11,24 @@ func testRules(m dsl.Matcher) {
m.Match(`range $x[:]`).
Where(m["x"].Type.Is(`[]$_`)).
Report(`redundant slicing of a range expression`)

m.Match(`$lhs = append($lhs, $x)`).
Where(m["lhs"].Contains(`$x`)).
Report(`$lhs contains $x`)

m.Match(`copy($*args)`).
Where(m["args"].Contains(`$_[:]`)).
Report(`copy() contains a slicing operation`)

m.Match(`sink($err); $x`).
Where(!m["x"].Contains(`return $err`) && m["err"].Type.Is(`error`)).
Report(`sink($err) call not followed by return $err`)

m.Match(`for { $*body }`).
Where(m["body"].Contains(`expensive()`)).
Report(`expensive call inside a loop`)

m.Match(`$x != nil && $y`).
Where(m["y"].Contains(`$x[0]`)).
Report(`nil check may not be enough to access $x[0], check for len`)
}
4 changes: 2 additions & 2 deletions go.mod
Expand Up @@ -5,9 +5,9 @@ go 1.17
require (
github.com/go-toolsmith/astcopy v1.0.0
github.com/google/go-cmp v0.5.6
github.com/quasilyte/go-ruleguard/dsl v0.3.13
github.com/quasilyte/go-ruleguard/dsl v0.3.14
github.com/quasilyte/go-ruleguard/rules v0.0.0-20211022131956-028d6511ab71
github.com/quasilyte/gogrep v0.0.0-20220104185649-039753a3dd32
github.com/quasilyte/gogrep v0.0.0-20220120141003-628d8b3623b5
golang.org/x/tools v0.1.9-0.20211228192929-ee1ca4ffc4da
)

Expand Down
4 changes: 4 additions & 0 deletions go.sum
Expand Up @@ -12,11 +12,15 @@ github.com/quasilyte/go-ruleguard v0.3.1-0.20210203134552-1b5a410e1cc8/go.mod h1
github.com/quasilyte/go-ruleguard/dsl v0.3.0/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU=
github.com/quasilyte/go-ruleguard/dsl v0.3.13 h1:WmtzUkp28TMarzfBCogPf7plyI/2gsNsj8CgZ9ihPCM=
github.com/quasilyte/go-ruleguard/dsl v0.3.13/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU=
github.com/quasilyte/go-ruleguard/dsl v0.3.14 h1:diesHrFHZ6rxuFltuwiW7NRQaqUIypuSSmUulRCNuqM=
github.com/quasilyte/go-ruleguard/dsl v0.3.14/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU=
github.com/quasilyte/go-ruleguard/rules v0.0.0-20201231183845-9e62ed36efe1/go.mod h1:7JTjp89EGyU1d6XfBiXihJNG37wB2VRkd125Q1u7Plc=
github.com/quasilyte/go-ruleguard/rules v0.0.0-20211022131956-028d6511ab71 h1:CNooiryw5aisadVfzneSZPswRWvnVW8hF1bS/vo8ReI=
github.com/quasilyte/go-ruleguard/rules v0.0.0-20211022131956-028d6511ab71/go.mod h1:4cgAphtvu7Ftv7vOT2ZOYhC6CvBxZixcasr8qIOTA50=
github.com/quasilyte/gogrep v0.0.0-20220104185649-039753a3dd32 h1:fJhpG5LYGnHZqUIDULZkvQKJfdtAefNrkoiGCezlr7g=
github.com/quasilyte/gogrep v0.0.0-20220104185649-039753a3dd32/go.mod h1:wSEyW6O61xRV6zb6My3HxrQ5/8ke7NE2OayqCHa3xRM=
github.com/quasilyte/gogrep v0.0.0-20220120141003-628d8b3623b5 h1:PDWGei+Rf2bBiuZIbZmM20J2ftEy9IeUCHA8HbQqed8=
github.com/quasilyte/gogrep v0.0.0-20220120141003-628d8b3623b5/go.mod h1:wSEyW6O61xRV6zb6My3HxrQ5/8ke7NE2OayqCHa3xRM=
github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/yuin/goldmark v1.4.1/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
Expand Down
22 changes: 22 additions & 0 deletions ruleguard/filters.go
Expand Up @@ -159,6 +159,28 @@ func makeAddressableFilter(src, varname string) filterFunc {
}
}

func makeVarContainsFilter(src, varname string, pat *gogrep.Pattern) filterFunc {
// TODO: use a shared state here as well?
state := gogrep.NewMatcherState()
return func(params *filterParams) matchFilterResult {
state.CapturePreset = params.match.CaptureList()
matched := false
gogrep.Walk(params.subNode(varname), func(n ast.Node) bool {
if matched {
return false
}
pat.MatchNode(&state, n, func(m gogrep.MatchData) {
matched = true
})
return true
})
if matched {
return filterSuccess
}
return filterFailure(src)
}
}

func makeCustomVarFilter(src, varname string, fn *quasigo.Func) filterFunc {
return func(params *filterParams) matchFilterResult {
// TODO(quasilyte): what if bytecode function panics due to the programming error?
Expand Down
32 changes: 19 additions & 13 deletions ruleguard/ir/filter_op.gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions ruleguard/ir/gen_filter_op.go
Expand Up @@ -62,6 +62,8 @@ func main() {
{name: "VarTypeHasMethod", comment: "m[$Value].Type.HasMethod($Args[0])", valueType: "string", flags: flagHasVar},
{name: "VarTextMatches", comment: "m[$Value].Text.Matches($Args[0])", valueType: "string", flags: flagHasVar},

{name: "VarContains", comment: "m[$Value].Contains($Args[0])", valueType: "string", flags: flagHasVar},

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

{name: "GoVersionEq", comment: "m.GoVersion().Eq($Value)", valueType: "string"},
Expand Down
27 changes: 21 additions & 6 deletions ruleguard/ir_loader.go
Expand Up @@ -272,7 +272,8 @@ func (l *irLoader) loadRule(group *ir.RuleGroup, rule *ir.Rule) error {
}

info := filterInfo{
Vars: make(map[string]struct{}),
Vars: make(map[string]struct{}),
group: group,
}
if rule.WhereExpr.IsValid() {
filter, err := l.newFilter(rule.WhereExpr, &info)
Expand Down Expand Up @@ -313,10 +314,7 @@ func (l *irLoader) loadCommentRule(resultProto goRule, rule *ir.Rule, src string
return nil
}

func (l *irLoader) loadSyntaxRule(group *ir.RuleGroup, resultProto goRule, filterInfo filterInfo, rule *ir.Rule, src string, line int) error {
result := resultProto
result.line = line

func (l *irLoader) gogrepCompile(group *ir.RuleGroup, src string) (*gogrep.Pattern, gogrep.PatternInfo, error) {
var imports map[string]string
if len(group.Imports) != 0 {
imports = make(map[string]string)
Expand All @@ -332,7 +330,14 @@ func (l *irLoader) loadSyntaxRule(group *ir.RuleGroup, resultProto goRule, filte
WithTypes: true,
Imports: imports,
}
pat, info, err := gogrep.Compile(gogrepConfig)
return gogrep.Compile(gogrepConfig)
}

func (l *irLoader) loadSyntaxRule(group *ir.RuleGroup, resultProto goRule, filterInfo filterInfo, rule *ir.Rule, src string, line int) error {
result := resultProto
result.line = line

pat, info, err := l.gogrepCompile(group, src)
if err != nil {
return l.errorf(rule.Line, err, "parse match pattern")
}
Expand Down Expand Up @@ -721,6 +726,14 @@ func (l *irLoader) newFilter(filter ir.FilterExpr, info *filterInfo) (matchFilte
}
result.fn = makeFileNameMatchesFilter(result.src, re)

case ir.FilterVarContainsOp:
src := filter.Args[0].Value.(string)
pat, _, err := l.gogrepCompile(info.group, src)
if err != nil {
return result, l.errorf(filter.Line, err, "parse contains pattern")
}
result.fn = makeVarContainsFilter(result.src, filter.Value.(string), pat)

case ir.FilterVarFilterOp:
funcName := filter.Args[0].Value.(string)
userFn := l.state.env.GetFunc(l.file.PkgPath, funcName)
Expand Down Expand Up @@ -839,4 +852,6 @@ func (l *irLoader) newBinaryExprFilter(filter ir.FilterExpr, info *filterInfo) (

type filterInfo struct {
Vars map[string]struct{}

group *ir.RuleGroup
}
10 changes: 10 additions & 0 deletions ruleguard/irconv/irconv.go
Expand Up @@ -661,6 +661,16 @@ func (conv *converter) convertFilterExprImpl(e ast.Expr) ir.FilterExpr {
case "File.Name.Matches":
return ir.FilterExpr{Op: ir.FilterFileNameMatchesOp, Value: conv.parseStringArg(e.Args[0])}

case "Contains":
pat := conv.parseStringArg(e.Args[0])
return ir.FilterExpr{
Op: ir.FilterVarContainsOp,
Value: op.varName,
Args: []ir.FilterExpr{
{Op: ir.FilterStringOp, Value: pat},
},
}

case "Filter":
funcName, ok := e.Args[0].(*ast.Ident)
if !ok {
Expand Down

0 comments on commit ff2e115

Please sign in to comment.