From 38a94b1397b4a7bb9127cccab9d00a7f90047937 Mon Sep 17 00:00:00 2001 From: Richardas Kuchinskas Date: Sun, 28 Jul 2024 02:53:27 +0300 Subject: [PATCH 1/5] added new @link tag for dynamic rules, test --- src/linter/cache.go | 70 ++++++++++++++++++----------------- src/linter/cache_test.go | 6 +-- src/linter/root.go | 5 +++ src/rules/parser.go | 16 +++++++- src/rules/rules.go | 3 ++ src/tests/rules/rules_test.go | 27 ++++++++++++++ 6 files changed, 89 insertions(+), 38 deletions(-) diff --git a/src/linter/cache.go b/src/linter/cache.go index 528fb0318..5c5d08d4c 100644 --- a/src/linter/cache.go +++ b/src/linter/cache.go @@ -16,40 +16,42 @@ import ( // cacheVersions is a magic number that helps to distinguish incompatible caches. // // Version log: -// 27 - added Static field to meta.FuncInfo -// 28 - array type parsed as mixed[] -// 29 - updated type inference for ClassConstFetch -// 30 - resolve ClassConstFetch to a wrapped type string -// 31 - fixed plus operator type inference for arrays -// 32 - replaced Static:bool with Flags:uint8 in meta.FuncInfo -// 33 - support parsing of array and list -// 34 - support parsing of ?ClassName as "ClassName|null" -// 35 - added Flags:uint8 to meta.ClassInfo -// 36 - added FuncAbstract bit to FuncFlags -// added FuncFinal bit to FuncFlags -// added ClassFinal bit to ClassFlags -// FuncInfo now stores original function name -// ClassInfo now stores original class name -// 37 - added ClassShape bit to ClassFlags -// changed meta.scopeVar bool fields representation -// 38 - replaced TypesMap.immutable:bool with flags:uint8. -// added mapPrecise flag to mark precise type maps. -// 39 - added new field Value in ConstantInfo -// 40 - changed string const value storage (no quotes) -// 41 - const-folding affected const definition values -// 42 - bool-typed consts are now stored in meta info -// 43 - define'd const values stored in cache -// 44 - rename ConstantInfo => ConstInfo -// 45 - added Mixins field to meta.ClassInfo -// 46 - changed the way of inferring the return type of functions and methods -// 47 - forced cache version invalidation due to the #921 -// 48 - renamed meta.TypesMap to types.Map; this affects gob encoding -// 49 - for shape, names are now generated using the keys that make up this shape -// 50 - added Flags field for meta.PropertyInfo -// 51 - added anonymous classes -// 52 - renamed all PhpDoc and Phpdoc with PHPDoc -// 53 - added DeprecationInfo for functions and methods and support for some attributes -// 54 - forced cache version invalidation due to the #1165 +// +// 27 - added Static field to meta.FuncInfo +// 28 - array type parsed as mixed[] +// 29 - updated type inference for ClassConstFetch +// 30 - resolve ClassConstFetch to a wrapped type string +// 31 - fixed plus operator type inference for arrays +// 32 - replaced Static:bool with Flags:uint8 in meta.FuncInfo +// 33 - support parsing of array and list +// 34 - support parsing of ?ClassName as "ClassName|null" +// 35 - added Flags:uint8 to meta.ClassInfo +// 36 - added FuncAbstract bit to FuncFlags +// added FuncFinal bit to FuncFlags +// added ClassFinal bit to ClassFlags +// FuncInfo now stores original function name +// ClassInfo now stores original class name +// 37 - added ClassShape bit to ClassFlags +// changed meta.scopeVar bool fields representation +// 38 - replaced TypesMap.immutable:bool with flags:uint8. +// added mapPrecise flag to mark precise type maps. +// 39 - added new field Value in ConstantInfo +// 40 - changed string const value storage (no quotes) +// 41 - const-folding affected const definition values +// 42 - bool-typed consts are now stored in meta info +// 43 - define'd const values stored in cache +// 44 - rename ConstantInfo => ConstInfo +// 45 - added Mixins field to meta.ClassInfo +// 46 - changed the way of inferring the return type of functions and methods +// 47 - forced cache version invalidation due to the #921 +// 48 - renamed meta.TypesMap to types.Map; this affects gob encoding +// 49 - for shape, names are now generated using the keys that make up this shape +// 50 - added Flags field for meta.PropertyInfo +// 51 - added anonymous classes +// 52 - renamed all PhpDoc and Phpdoc with PHPDoc +// 53 - added DeprecationInfo for functions and methods and support for some attributes +// 54 - forced cache version invalidation due to the #1165 +// 55 - added @link tag -> new field in Rule const cacheVersion = 54 var ( diff --git a/src/linter/cache_test.go b/src/linter/cache_test.go index 5cb1f2775..546de2863 100644 --- a/src/linter/cache_test.go +++ b/src/linter/cache_test.go @@ -148,8 +148,8 @@ main(); // 1. Check cache contents length. // // If cache encoding changes, there is a very high chance that - // encoded data lengh will change as well. - wantLen := 5953 + // encoded data length will change as well. + wantLen := 5952 haveLen := buf.Len() if haveLen != wantLen { t.Errorf("cache len mismatch:\nhave: %d\nwant: %d", haveLen, wantLen) @@ -158,7 +158,7 @@ main(); // 2. Check cache "strings" hash. // // It catches new fields in cached types, field renames and encoding of additional named attributes. - wantStrings := "df69cfe531807fe5e317e5f894ac1ad2a68020edf03a053a30b25c70488b741b6d992d37e7a76ac4b8a760756631ee9ae309ba57e29253ffae896c3492b90939" + wantStrings := "690e77c94ecdd7878de0bf6f6881d786cf1fafa4588f7905f54d700646c4952aad359008ae2dcddb1c7f29163ecee62355d525672090ac30257bc414f690006f" haveStrings := collectCacheStrings(buf.String()) if haveStrings != wantStrings { t.Errorf("cache strings mismatch:\nhave: %q\nwant: %q", haveStrings, wantStrings) diff --git a/src/linter/root.go b/src/linter/root.go index 3c23699ac..261c85fa2 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 ab7e0b827..f18870ba8 100644 --- a/src/rules/parser.go +++ b/src/rules/parser.go @@ -177,6 +177,16 @@ 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)) + } + var link = part.Params[0] + if !isURL(link) { + return rule, p.errorf(st, "@link argument is not link") + } + rule.Link = link + case "location": if len(part.Params) != 1 { return rule, p.errorf(st, "@location expects exactly 1 params, got %d", len(part.Params)) @@ -318,7 +328,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()) } @@ -339,6 +348,11 @@ func (p *parser) parseRuleInfo(st ir.Node, labelStmt ir.Node, proto *Rule) (Rule return rule, nil } +func isURL(str string) bool { + re := regexp.MustCompile(`^(https?|ftp)://[^\s/$.?#].\S*$`) + return re.MatchString(str) +} + func (p *parser) parseRules(stmts []ir.Node, proto *Rule) error { for len(stmts) > 0 { stmt := stmts[0] diff --git a/src/rules/rules.go b/src/rules/rules.go index 8bed8f9ae..c303ecd8e 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 diff --git a/src/tests/rules/rules_test.go b/src/tests/rules/rules_test.go index b3df05852..02a2df5f1 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 := ` Date: Sun, 28 Jul 2024 11:07:32 +0300 Subject: [PATCH 2/5] rewrote link url checker logic --- src/rules/parser.go | 2 +- src/tests/rules/rules_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/rules/parser.go b/src/rules/parser.go index f18870ba8..bf3a39c4f 100644 --- a/src/rules/parser.go +++ b/src/rules/parser.go @@ -349,7 +349,7 @@ func (p *parser) parseRuleInfo(st ir.Node, labelStmt ir.Node, proto *Rule) (Rule } func isURL(str string) bool { - re := regexp.MustCompile(`^(https?|ftp)://[^\s/$.?#].\S*$`) + re := regexp.MustCompile(`^((https?|ftp)://)?[^\s/$.?#].\S*$`) return re.MatchString(str) } diff --git a/src/tests/rules/rules_test.go b/src/tests/rules/rules_test.go index 02a2df5f1..eea8f280b 100644 --- a/src/tests/rules/rules_test.go +++ b/src/tests/rules/rules_test.go @@ -338,7 +338,7 @@ func TestLinkTag(t *testing.T) { * @name emptyIf * @warning suspicious empty body of the if statement * @scope local - * @link https://goodrule.com + * @link goodrule.com */ if ($_); ` @@ -354,7 +354,7 @@ function f() { `) test.Expect = []string{ - ` | More about this rule: https://goodrule.com`, + ` | More about this rule: goodrule.com`, } test.RunRulesTest() } From 4c15f341567723e5ac6d2c8be2b0d6f72f8a9d7a Mon Sep 17 00:00:00 2001 From: Richardas Kuchinskas Date: Sun, 28 Jul 2024 11:24:29 +0300 Subject: [PATCH 3/5] cache test --- src/linter/cache_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/linter/cache_test.go b/src/linter/cache_test.go index 546de2863..de0386b0f 100644 --- a/src/linter/cache_test.go +++ b/src/linter/cache_test.go @@ -149,7 +149,7 @@ main(); // // If cache encoding changes, there is a very high chance that // encoded data length will change as well. - wantLen := 5952 + wantLen := 5953 haveLen := buf.Len() if haveLen != wantLen { t.Errorf("cache len mismatch:\nhave: %d\nwant: %d", haveLen, wantLen) @@ -158,7 +158,7 @@ main(); // 2. Check cache "strings" hash. // // It catches new fields in cached types, field renames and encoding of additional named attributes. - wantStrings := "690e77c94ecdd7878de0bf6f6881d786cf1fafa4588f7905f54d700646c4952aad359008ae2dcddb1c7f29163ecee62355d525672090ac30257bc414f690006f" + wantStrings := "df69cfe531807fe5e317e5f894ac1ad2a68020edf03a053a30b25c70488b741b6d992d37e7a76ac4b8a760756631ee9ae309ba57e29253ffae896c3492b90939" haveStrings := collectCacheStrings(buf.String()) if haveStrings != wantStrings { t.Errorf("cache strings mismatch:\nhave: %q\nwant: %q", haveStrings, wantStrings) From 87eb7ecbeb334d7d00c8b904be049a1404e4a795 Mon Sep 17 00:00:00 2001 From: Richardas Kuchinskas Date: Mon, 5 Aug 2024 14:13:40 +0300 Subject: [PATCH 4/5] removed checker for url --- src/linter/cache.go | 1 - src/rules/parser.go | 11 +---------- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/src/linter/cache.go b/src/linter/cache.go index 5c5d08d4c..79d499e64 100644 --- a/src/linter/cache.go +++ b/src/linter/cache.go @@ -51,7 +51,6 @@ import ( // 52 - renamed all PhpDoc and Phpdoc with PHPDoc // 53 - added DeprecationInfo for functions and methods and support for some attributes // 54 - forced cache version invalidation due to the #1165 -// 55 - added @link tag -> new field in Rule const cacheVersion = 54 var ( diff --git a/src/rules/parser.go b/src/rules/parser.go index bf3a39c4f..f68516637 100644 --- a/src/rules/parser.go +++ b/src/rules/parser.go @@ -181,11 +181,7 @@ func (p *parser) parseRuleInfo(st ir.Node, labelStmt ir.Node, proto *Rule) (Rule if len(part.Params) != 1 { return rule, p.errorf(st, "@link expects exactly 1 param, got %d", len(part.Params)) } - var link = part.Params[0] - if !isURL(link) { - return rule, p.errorf(st, "@link argument is not link") - } - rule.Link = link + rule.Link = part.Params[0] case "location": if len(part.Params) != 1 { @@ -348,11 +344,6 @@ func (p *parser) parseRuleInfo(st ir.Node, labelStmt ir.Node, proto *Rule) (Rule return rule, nil } -func isURL(str string) bool { - re := regexp.MustCompile(`^((https?|ftp)://)?[^\s/$.?#].\S*$`) - return re.MatchString(str) -} - func (p *parser) parseRules(stmts []ir.Node, proto *Rule) error { for len(stmts) > 0 { stmt := stmts[0] From 126619dfeb47bf37c1b5e3e501b68df47a81efec Mon Sep 17 00:00:00 2001 From: Richardas Kuchinskas Date: Tue, 6 Aug 2024 18:40:42 +0300 Subject: [PATCH 5/5] added tag to documentation --- docs/dynamic_rules.md | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/docs/dynamic_rules.md b/docs/dynamic_rules.md index 9674aad2b..58534def1 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: