Skip to content

Commit

Permalink
dynamic rules: new @link tag (#1232)
Browse files Browse the repository at this point in the history
* added new @link tag for dynamic rules
  • Loading branch information
Hidanio authored Dec 5, 2024
1 parent 7929fd3 commit e59350d
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 15 deletions.
30 changes: 17 additions & 13 deletions docs/dynamic_rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,8 @@ The `@path` restriction allows you to restrict the rule by file path.

Thus, the rule will be applied only if there is a substring from `@path` in the file path.

Also, you can use several tags.

For example:

```php
Expand All @@ -394,6 +396,7 @@ function ternarySimplify() {
* @maybe Could rewrite as '$x ?: $y'
* @pure $x
* @path common/
* @path mythical/
*/
$x ? $x : $y;
}
Expand Down Expand Up @@ -580,20 +583,21 @@ There is a simple rule on how to decide whether you need fuzzy matching or not:

Rule related attributes:

| Syntax | Description |
| ------------- | ------------- |
| `@name name` | Set diagnostic name (only outside of the function group). |
| `@error message...` | Set `severity = error` and report text to `message`. |
| `@warning message...` | Set `severity = warning` and report text to `message`. |
| `@maybe message...` | Set `severity = maybe` and report text to `message`. |
| `@fix template...` | Provide a quickfix template for the rule. |
| `@scope scope_kind` | Controls where rule can be applied. `scope_kind` is `all`, `root` or `local`. |
| `@location $var` | Selects a sub-expr from a match by a matcher var that defines report cursor position. |
| Syntax | Description |
|------------------------| ------------- |
| `@name name` | Set diagnostic name (only outside of the function group). |
| `@error message...` | Set `severity = error` and report text to `message`. |
| `@warning message...` | Set `severity = warning` and report text to `message`. |
| `@maybe message...` | Set `severity = maybe` and report text to `message`. |
| `@fix template...` | Provide a quickfix template for the rule. |
| `@scope scope_kind` | Controls where rule can be applied. `scope_kind` is `all`, `root` or `local`. |
| `@location $var` | Selects a sub-expr from a match by a matcher var that defines report cursor position. |
| `@type type_expr $var` | Adds "type equals to" filter, applied to `$var`. |
| `@pure $var` | Adds "side effect free" filter, applied to `$var`. |
| `@or` | Add a new filter set. "Closes" the previous filter set and "opens" a new one. |
| `@strict-syntax` | Sets not to use the normalization of the same constructs. |
| `@path $substr` | If specified, the rule will only work for files that contain `$substr` in the name. |
| `@pure $var` | Adds "side effect free" filter, applied to `$var`. |
| `@or` | Add a new filter set. "Closes" the previous filter set and "opens" a new one. |
| `@strict-syntax` | Sets not to use the normalization of the same constructs. |
| `@path $substr` | If specified, the rule will only work for files that contain `$substr` in the name. |
| `@link link` | If specified, then if there is an error, additional text/link to possible documentation will be displayed. |

Function related attributes:

Expand Down
2 changes: 1 addition & 1 deletion src/linter/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ main();
// 1. Check cache contents length.
//
// If cache encoding changes, there is a very high chance that
// encoded data lengh will change as well.
// encoded data length will change as well.
wantLen := 5953
haveLen := buf.Len()
if haveLen != wantLen {
Expand Down
5 changes: 5 additions & 0 deletions src/linter/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -1780,6 +1780,11 @@ func (d *rootWalker) runRule(n ir.Node, sc *meta.Scope, rule *rules.Rule) bool {
}

message := d.renderRuleMessage(rule.Message, n, m, true)

if rule.Link != "" {
message += " | More about this rule: " + rule.Link
}

d.Report(location, rule.Level, rule.Name, "%s", message)

if d.config.ApplyQuickFixes && rule.Fix != "" {
Expand Down
7 changes: 6 additions & 1 deletion src/rules/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,12 @@ func (p *parser) parseRuleInfo(st ir.Node, labelStmt ir.Node, proto *Rule) (Rule
}
rule.Name = part.Params[0]

case "link":
if len(part.Params) != 1 {
return rule, p.errorf(st, "@link expects exactly 1 param, got %d", len(part.Params))
}
rule.Link = part.Params[0]

case "location":
if len(part.Params) != 1 {
return rule, p.errorf(st, "@location expects exactly 1 params, got %d", len(part.Params))
Expand Down Expand Up @@ -317,7 +323,6 @@ func (p *parser) parseRuleInfo(st ir.Node, labelStmt ir.Node, proto *Rule) (Rule
}
filter.Regexp = regex
filterSet[name] = filter

default:
return rule, p.errorf(st, "unknown attribute @%s on line %d", part.Name(), part.Line())
}
Expand Down
3 changes: 3 additions & 0 deletions src/rules/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ type Rule struct {
// Name tells whether this rule causes critical report.
Name string

// Link to the documentation about rule
Link string

// Matcher is an object that is used to check whether a given AST node
// should trigger a warning that is associated with rule.
Matcher *phpgrep.Matcher
Expand Down
27 changes: 27 additions & 0 deletions src/tests/rules/rules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,33 @@ function f() {
test.RunRulesTest()
}

func TestLinkTag(t *testing.T) {
rfile := `<?php
/**
* @name emptyIf
* @warning suspicious empty body of the if statement
* @scope local
* @link goodrule.com
*/
if ($_);
`

test := linttest.NewSuite(t)
test.RuleFile = rfile
test.AddFile(`<?php
if (123); // No warning
function f() {
if (123); // Warning
}
`)

test.Expect = []string{
` | More about this rule: goodrule.com`,
}
test.RunRulesTest()
}

func TestRootRules(t *testing.T) {
rfile := `<?php
/**
Expand Down

0 comments on commit e59350d

Please sign in to comment.