diff --git a/analyzer/testdata/src/matching/file.go b/analyzer/testdata/src/matching/file.go index f3167848..eefe6b36 100644 --- a/analyzer/testdata/src/matching/file.go +++ b/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` @@ -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` + } + } +} diff --git a/analyzer/testdata/src/matching/rules.go b/analyzer/testdata/src/matching/rules.go index a591aa8d..111ad2ab 100644 --- a/analyzer/testdata/src/matching/rules.go +++ b/analyzer/testdata/src/matching/rules.go @@ -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`) } diff --git a/go.mod b/go.mod index dc53ab93..995632ec 100644 --- a/go.mod +++ b/go.mod @@ -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 ) diff --git a/go.sum b/go.sum index 53d00239..9cd6ca3f 100644 --- a/go.sum +++ b/go.sum @@ -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= diff --git a/ruleguard/filters.go b/ruleguard/filters.go index 713caaf4..f2656d12 100644 --- a/ruleguard/filters.go +++ b/ruleguard/filters.go @@ -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? diff --git a/ruleguard/ir/filter_op.gen.go b/ruleguard/ir/filter_op.gen.go index c1f1d655..766d8e85 100644 --- a/ruleguard/ir/filter_op.gen.go +++ b/ruleguard/ir/filter_op.gen.go @@ -116,55 +116,59 @@ const ( // $Value type: string FilterVarTextMatchesOp FilterOp = 30 + // m[$Value].Contains($Args[0]) + // $Value type: string + FilterVarContainsOp FilterOp = 31 + // m.Deadcode() - FilterDeadcodeOp FilterOp = 31 + FilterDeadcodeOp FilterOp = 32 // m.GoVersion().Eq($Value) // $Value type: string - FilterGoVersionEqOp FilterOp = 32 + FilterGoVersionEqOp FilterOp = 33 // m.GoVersion().LessThan($Value) // $Value type: string - FilterGoVersionLessThanOp FilterOp = 33 + FilterGoVersionLessThanOp FilterOp = 34 // m.GoVersion().GreaterThan($Value) // $Value type: string - FilterGoVersionGreaterThanOp FilterOp = 34 + FilterGoVersionGreaterThanOp FilterOp = 35 // m.GoVersion().LessEqThan($Value) // $Value type: string - FilterGoVersionLessEqThanOp FilterOp = 35 + FilterGoVersionLessEqThanOp FilterOp = 36 // m.GoVersion().GreaterEqThan($Value) // $Value type: string - FilterGoVersionGreaterEqThanOp FilterOp = 36 + FilterGoVersionGreaterEqThanOp FilterOp = 37 // m.File.Imports($Value) // $Value type: string - FilterFileImportsOp FilterOp = 37 + FilterFileImportsOp FilterOp = 38 // m.File.PkgPath.Matches($Value) // $Value type: string - FilterFilePkgPathMatchesOp FilterOp = 38 + FilterFilePkgPathMatchesOp FilterOp = 39 // m.File.Name.Matches($Value) // $Value type: string - FilterFileNameMatchesOp FilterOp = 39 + FilterFileNameMatchesOp FilterOp = 40 // $Value holds a function name // $Value type: string - FilterFilterFuncRefOp FilterOp = 40 + FilterFilterFuncRefOp FilterOp = 41 // $Value holds a string constant // $Value type: string - FilterStringOp FilterOp = 41 + FilterStringOp FilterOp = 42 // $Value holds an int64 constant // $Value type: int64 - FilterIntOp FilterOp = 42 + FilterIntOp FilterOp = 43 // m[`$$`].Node.Parent().Is($Args[0]) - FilterRootNodeParentIsOp FilterOp = 43 + FilterRootNodeParentIsOp FilterOp = 44 ) var filterOpNames = map[FilterOp]string{ @@ -199,6 +203,7 @@ var filterOpNames = map[FilterOp]string{ FilterVarTypeImplementsOp: `VarTypeImplements`, FilterVarTypeHasMethodOp: `VarTypeHasMethod`, FilterVarTextMatchesOp: `VarTextMatches`, + FilterVarContainsOp: `VarContains`, FilterDeadcodeOp: `Deadcode`, FilterGoVersionEqOp: `GoVersionEq`, FilterGoVersionLessThanOp: `GoVersionLessThan`, @@ -243,6 +248,7 @@ var filterOpFlags = map[FilterOp]uint64{ FilterVarTypeImplementsOp: flagHasVar, FilterVarTypeHasMethodOp: flagHasVar, FilterVarTextMatchesOp: flagHasVar, + FilterVarContainsOp: flagHasVar, FilterStringOp: flagIsBasicLit, FilterIntOp: flagIsBasicLit, } diff --git a/ruleguard/ir/gen_filter_op.go b/ruleguard/ir/gen_filter_op.go index db1b7d3f..ea3bab9a 100644 --- a/ruleguard/ir/gen_filter_op.go +++ b/ruleguard/ir/gen_filter_op.go @@ -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"}, diff --git a/ruleguard/ir_loader.go b/ruleguard/ir_loader.go index fbdf06cd..e50c8432 100644 --- a/ruleguard/ir_loader.go +++ b/ruleguard/ir_loader.go @@ -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) @@ -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) @@ -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") } @@ -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) @@ -839,4 +852,6 @@ func (l *irLoader) newBinaryExprFilter(filter ir.FilterExpr, info *filterInfo) ( type filterInfo struct { Vars map[string]struct{} + + group *ir.RuleGroup } diff --git a/ruleguard/irconv/irconv.go b/ruleguard/irconv/irconv.go index 0c98455f..f8efd53b 100644 --- a/ruleguard/irconv/irconv.go +++ b/ruleguard/irconv/irconv.go @@ -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 {