Skip to content

Commit

Permalink
tests and new error
Browse files Browse the repository at this point in the history
  • Loading branch information
Hidanio committed Mar 6, 2025
1 parent 2aa7c55 commit 302a2d2
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 19 deletions.
4 changes: 1 addition & 3 deletions docs/dynamic_rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -492,9 +492,7 @@ eval(${"var"});

##### `@filter`

The `@filter` restriction allows you to restrict the rule by name of matched variable or literal, const or anything else!

More about which can be captured you can read in [phpgrep doc](https://github.com/VKCOM/noverify/blob/master/src/phpgrep/pattern_language.md#matcher-expressions)
The `@filter` restriction allows you to restrict the rule by name of matched variable or literal.

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

Expand Down
24 changes: 17 additions & 7 deletions src/linter/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -1893,7 +1893,14 @@ func (d *rootWalker) runRule(n ir.Node, sc *meta.Scope, rule *rules.Rule) bool {
matched = true
} else {
for _, filterSet := range rule.Filters {
if d.checkFilterSet(&m, sc, filterSet) {

filterMatched, errorFilterSet := d.checkFilterSet(&m, sc, filterSet)
if errorFilterSet != nil {
d.Report(n, rule.Level, rule.Name, "%s", errorFilterSet)
return true
}

if filterMatched {
matched = true
break
}
Expand Down Expand Up @@ -1950,7 +1957,7 @@ func (d *rootWalker) checkTypeFilter(wantType *phpdoc.Type, sc *meta.Scope, nn i
return rules.TypeIsCompatible(wantType.Expr, haveType.Expr)
}

func (d *rootWalker) checkFilterSet(m *phpgrep.MatchData, sc *meta.Scope, filterSet map[string]rules.Filter) bool {
func (d *rootWalker) checkFilterSet(m *phpgrep.MatchData, sc *meta.Scope, filterSet map[string]rules.Filter) (bool, error) {
// TODO: pass custom types here, so both @type and @pure predicates can use it.

for name, filter := range filterSet {
Expand All @@ -1960,24 +1967,27 @@ func (d *rootWalker) checkFilterSet(m *phpgrep.MatchData, sc *meta.Scope, filter
}

if !d.checkTypeFilter(filter.Type, sc, nn) {
return false
return false, nil
}
if filter.Pure && !solver.SideEffectFree(d.scope(), d.ctx.st, nil, nn) {
return false
return false, nil
}
if filter.Regexp != nil {
switch v := nn.(type) {
case *ir.SimpleVar:
if !filter.Regexp.MatchString(v.Name) {
return false
return false, nil
}
case *ir.String:
if !filter.Regexp.MatchString(v.Value) {
return false
return false, nil
}
default:
// logical paradox: we handled it, but does not support that's why here false
return false, fmt.Errorf("applying @filter for construction '%s' does not support. Current supported capturing types are str and var", d.nodeText(nn))
}
}
}

return true
return true, nil
}
53 changes: 44 additions & 9 deletions src/tests/rules/rules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -848,6 +848,40 @@ function testInsecureUrl() {
test.RunRulesTest()
}

func TestFilterStrVarCatching(t *testing.T) {
rfile := `<?php
function catchingDiffTypes() {
/**
* @warning str '$name' warning
* @filter $name ^str_name$
*/
callFunc(${'name:str'});
/**
* @warning var '$name' warning
* @filter $name ^var_name$
*/
callFunc(${'name:var'});
}
`
test := linttest.NewSuite(t)
test.RuleFile = rfile
test.AddFile(`<?php
// For a string literal: captured without quotes inside the filter
callFunc("str_name"); // str '"str_name"' warning
// For variable: name without dollar sign
$var_name = "";
callFunc($var_name); // var '$var_name' warning
`)
test.Expect = []string{
`str '"str_name"' warning`,
`var '$var_name' warning`,
}
test.RunRulesTest()
}

func TestFilterMultiCatching(t *testing.T) {
rfile := `<?php
function catchingDiffTypes() {
Expand Down Expand Up @@ -904,8 +938,8 @@ function catchingDiffTypes() {
* @filter $name ^\$a\+1$
*/
callFunc(${'name:expr'});
}
`
}`

test := linttest.NewSuite(t)
test.RuleFile = rfile
test.AddFile(`<?php
Expand Down Expand Up @@ -942,15 +976,16 @@ function catchingDiffTypes() {
test.Expect = []string{
`str '"str_name"' warning`,
`var '$var_name' warning`,
"call 'funcNane()' warning",
"const 'const_name' warning",
"int '42' warning",
"float '3.14' warning",
"char 'a' warning",
`func 'function() {}' warning`,
"expr '$a+1' warning",
`applying @filter for construction 'funcNane()' does not support. Current supported capturing types are str and var`,
`applying @filter for construction 'const_name' does not support. Current supported capturing types are str and var`,
`applying @filter for construction '42' does not support. Current supported capturing types are str and var`,
`applying @filter for construction '3.14' does not support. Current supported capturing types are str and var`,
`char ''a'' warning`,
`applying @filter for construction 'function() {}' does not support. Current supported capturing types are str and var`,
`applying @filter for construction '$a + 1' does not support. Current supported capturing types are str and var`,
}
test.RunRulesTest()

}

func TestFilterVariableNoWarning(t *testing.T) {
Expand Down

0 comments on commit 302a2d2

Please sign in to comment.