Skip to content

Commit

Permalink
added @filter for dynamic rules (#1180)
Browse files Browse the repository at this point in the history
  • Loading branch information
i582 authored Jun 20, 2022
1 parent ee98305 commit 4f34e25
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 2 deletions.
20 changes: 20 additions & 0 deletions docs/dynamic_rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,26 @@ function ternarySimplify() {

This rule will now don't apply to files with the `common/` folder in the path.

##### `@filter`

The `@filter` restriction allows you to restrict the rule by name of matched variable.

Thus, the rule will be applied only if there is a matched variable that matches passed regexp.

For example:

```php
function forbiddenIdUsage() {
/**
* @maybe Not use $id
* @filter $id ^id$
*/
$id;
}
```

This rule will match usage if `$id` variable.

#### Underline location (`@location`)

For every warning that NoVerify finds, it underlines the location. However, for dynamic rules, the right place is not always underlined.
Expand Down
7 changes: 7 additions & 0 deletions src/linter/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -1823,6 +1823,13 @@ func (d *rootWalker) checkFilterSet(m *phpgrep.MatchData, sc *meta.Scope, filter
if filter.Pure && !solver.SideEffectFree(d.scope(), d.ctx.st, nil, nn) {
return false
}
if filter.Regexp != nil {
if vr, ok := nn.(*ir.SimpleVar); ok {
if filter.Regexp.MatchString(vr.Name) {
return true
}
}
}
}

return true
Expand Down
24 changes: 24 additions & 0 deletions src/rules/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,30 @@ func (p *parser) parseRuleInfo(st ir.Node, labelStmt ir.Node, proto *Rule) (Rule
filter := filterSet[name]
filter.Pure = true
filterSet[name] = filter
case "filter":
if len(part.Params) != 2 {
return rule, p.errorf(st, "@filter expects exactly 2 param, got %d", len(part.Params))
}
name := part.Params[0]
if !strings.HasPrefix(name, "$") {
return rule, p.errorf(st, "@filter param must be a phpgrep variable")
}
name = strings.TrimPrefix(name, "$")
found := p.checkForVariableInPattern(name, patternStmt, verifiedVars)
if !found {
return rule, p.errorf(st, "@filter contains a reference to a variable %s that is not present in the pattern", name)
}
if filterSet == nil {
filterSet = map[string]Filter{}
}
regexString := part.Params[1]
filter := filterSet[name]
regex, err := regexp.Compile(regexString)
if err != nil {
return rule, p.errorf(st, "@filter %s: can't compile regexp %s", regexString, err)
}
filter.Regexp = regex
filterSet[name] = filter

default:
return rule, p.errorf(st, "unknown attribute @%s on line %d", part.Name(), part.Line())
Expand Down
6 changes: 4 additions & 2 deletions src/rules/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package rules

import (
"io"
"regexp"

"github.com/VKCOM/noverify/src/ir"
"github.com/VKCOM/noverify/src/phpdoc"
Expand Down Expand Up @@ -117,6 +118,7 @@ func (r *Rule) String() string {

// Filter describes constraints that should be applied to a given phpgrep variable.
type Filter struct {
Type *phpdoc.Type
Pure bool
Type *phpdoc.Type
Pure bool
Regexp *regexp.Regexp
}
26 changes: 26 additions & 0 deletions src/tests/rules/rules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,3 +416,29 @@ function bad(string $x) {
}
test.RunRulesTest()
}

func TestRulesFilter(t *testing.T) {
rfile := `<?php
function id_check() {
/**
* @warning Don't use $id variable
* @filter $id ^id$
*/
$id;
}
`
test := linttest.NewSuite(t)
test.RuleFile = rfile
test.AddFile(`<?php
$id = 100;
echo $id;
echo $id == 0;
if ($id == 0) {}
`)
test.Expect = []string{
`Don't use $id variable`,
`Don't use $id variable`,
`Don't use $id variable`,
}
test.RunRulesTest()
}

0 comments on commit 4f34e25

Please sign in to comment.