Repository: VKCOM/noverify
Pull request #1232 (2024-07-28 00:00:05)

dynamic rules: new `@link` tag


Patch:
diff --git a/docs/dynamic_rules.md b/docs/dynamic_rules.md
index 9674aad..58534de 100644
--- a/docs/dynamic_rules.md
+++ b/docs/dynamic_rules.md
@@ -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
@@ -394,6 +396,7 @@ function ternarySimplify() {
    * @maybe Could rewrite as '$x ?: $y'
    * @pure $x
    * @path common/
+   * @path mythical/
    */
   $x ? $x : $y;
 }
@@ -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:
 
diff --git a/src/linter/root.go b/src/linter/root.go
index 3c23699..261c85f 100644
--- a/src/linter/root.go
+++ b/src/linter/root.go
@@ -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 != "" {
diff --git a/src/rules/parser.go b/src/rules/parser.go
index 949bf3b..1749a06 100644
--- a/src/rules/parser.go
+++ b/src/rules/parser.go
@@ -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))
@@ -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())
 		}
diff --git a/src/rules/rules.go b/src/rules/rules.go
index 8bed8f9..c303ecd 100644
--- a/src/rules/rules.go
+++ b/src/rules/rules.go
@@ -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


Test patch:
diff --git a/src/linter/cache_test.go b/src/linter/cache_test.go
index 5cb1f27..de0386b 100644
--- a/src/linter/cache_test.go
+++ b/src/linter/cache_test.go
@@ -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 {
diff --git a/src/tests/rules/rules_test.go b/src/tests/rules/rules_test.go
index b3df058..eea8f28 100644
--- a/src/tests/rules/rules_test.go
+++ b/src/tests/rules/rules_test.go
@@ -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
 /**