Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dynamic rules: new @link tag #1232

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -177,6 +177,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 @@ -318,7 +324,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