dynamic rules: paths grouping (#1242)

docs/dynamic_rules.md

@@ -425,6 +425,71 @@ function ternarySimplify() {
 
 This rule will now don't apply to files with the `common/` folder in the path.
 
+##### `@path-group-name`
+
+The `@path-group-name` allow you to group a lot of paths to the named group and give the name. This tag must be first.
+
+```php
+/**
+ * @path-group-name test
+ * @path my/site/ads_
+ * @path your/site/bad
+ */
+_init_test_group_();
+```
+
+Very important note: you must write any function name after declaring `@path-group-name`. In this example this is `_init_test_group_()`
+
+
+#### `@path-group`
+
+After creating group with `@path-group-name` you can use it in your rules by `@path-group`.
+
+
+For example:
+
+```php
+/**
+ * @path-group-name test
+ * @path my/site/ads_
+ * @path your/site/bad
+ */
+_init_test_group_();
+
+/**
+ * @name varEval
+ * @warning don't eval from variable
+ * @path-group test
+ * @path my/site/admin_
+ */
+eval(${"var"});
+```
+
+You can combine @path-group with @path. No conflicts here.
+
+##### `@path-group-exclude`
+
+After creating group with `@path-group-name` you can use it in your rules by `@path-group-exclude`.
+
+``@path-group-exclude`` allow you to add all `@path` in group to exclude. It's the same as if they were written one by one.
+
+```php
+/**
+ * @path-group-name test
+ * @path www/no
+ */
+_init_test_group_();
+
+
+/**
+ * @name varEval
+ * @warning don't eval from variable
+ * @path www/
+ * @path-group-exclude test
+ */
+eval(${"var"});
+```
+
 ##### `@filter`
 
 The `@filter` restriction allows you to restrict the rule by name of matched variable.

src/rules/parser.go

@@ -17,7 +17,7 @@ import (
 	"github.com/VKCOM/noverify/src/utils"
 )
 
-var magicComment = regexp.MustCompile(`\* @(?:warning|error|info|maybe) `)
+var magicComment = regexp.MustCompile(`\* @(?:warning|error|info|maybe|path-group-name) `)
 
 //go:generate stringer -type=parseMode
 type parseMode int
@@ -128,6 +128,44 @@ func (p *parser) tryParseLabeledStmt(stmts []ir.Node, proto *Rule) (bool, error)
 	return true, err
 }
 
+func (p *parser) parseRuleGroups(st ir.Node) bool {
+	comment := p.commentText(st)
+	parsedPhpDoc := phpdoc.Parse(p.typeParser, comment).Parsed
+
+	var groupName string
+	// a little optimisation: if @path-group-name is not first - this is not grouping
+	foundGroup := false
+
+	for _, part := range parsedPhpDoc {
+		rawPart, ok := part.(*phpdoc.RawCommentPart)
+		if !ok {
+			continue
+		}
+
+		switch tagName := rawPart.Name(); tagName {
+		case "path-group-name":
+			groupName = rawPart.ParamsText
+			if pathGroups == nil {
+				pathGroups = make(map[string][]string)
+			}
+			pathGroups[groupName] = []string{}
+			foundGroup = true
+
+		case "path":
+			if !foundGroup {
+				return false // if @path-group-name not first - return
+			}
+			pathGroups[groupName] = append(pathGroups[groupName], rawPart.Params...)
+		default:
+			if !foundGroup {
+				return false
+			}
+		}
+	}
+
+	return foundGroup && pathGroups[groupName] != nil
+}
+
 func (p *parser) parseRuleInfo(st ir.Node, labelStmt ir.Node, proto *Rule) (Rule, error) {
 	var rule Rule
 
@@ -166,6 +204,7 @@ func (p *parser) parseRuleInfo(st ir.Node, labelStmt ir.Node, proto *Rule) (Rule
 
 	for _, part := range phpdoc.Parse(p.typeParser, comment).Parsed {
 		part := part.(*phpdoc.RawCommentPart)
+
 		switch part.Name() {
 		case "name":
 			if len(part.Params) != 1 {
@@ -233,6 +272,13 @@ func (p *parser) parseRuleInfo(st ir.Node, labelStmt ir.Node, proto *Rule) (Rule
 		case "or":
 			rule.Filters = append(rule.Filters, filterSet)
 			filterSet = nil
+
+		case "path-group":
+			if rule.Paths == nil {
+				rule.Paths = make([]string, 0)
+			}
+			paths := pathGroups[part.ParamsText]
+			rule.Paths = append(rule.Paths, paths...)
 		case "path":
 			if len(part.Params) != 1 {
 				return rule, p.errorf(st, "@path expects exactly 1 param, got %d", len(part.Params))
@@ -243,6 +289,15 @@ func (p *parser) parseRuleInfo(st ir.Node, labelStmt ir.Node, proto *Rule) (Rule
 			}
 
 			rule.Paths = append(rule.Paths, part.Params...)
+		case "path-group-exclude":
+			paths := pathGroups[part.ParamsText]
+			if rule.PathExcludes == nil {
+				rule.PathExcludes = make(map[string]bool, 1)
+			}
+
+			for _, path := range paths {
+				rule.PathExcludes[path] = true
+			}
 		case "path-exclude":
 			if len(part.Params) != 1 {
 				return rule, p.errorf(st, "@exclude expects exactly 1 param, got %d", len(part.Params))
@@ -389,6 +444,13 @@ func (p *parser) parseRule(st ir.Node, proto *Rule) error {
 		return nil
 	}
 
+	// if we parsed new group - we can go next comments
+	// function declaring after @path-group does not matter, because we will ignore it
+	// This used only for separating comments and functions
+	if p.parseRuleGroups(st) {
+		return nil
+	}
+
 	rule, err := p.parseRuleInfo(st, nil, proto)
 	if err != nil {
 		return err

src/rules/rules.go

@@ -11,6 +11,8 @@ import (
 
 type Parser struct{}
 
+var pathGroups map[string][]string
+
 func NewParser() *Parser {
 	return &Parser{}
 }

src/rules/util.go

@@ -30,6 +30,10 @@ func formatRule(r *Rule) string {
 		}
 	}
 
+	if r.Link != "" {
+		buf.WriteString(" * @link " + r.Link + "\n")
+	}
+
 	if r.Location != "" {
 		buf.WriteString(" * @location $" + r.Location + "\n")
 	}

src/tests/rules/rules_test.go

@@ -135,6 +135,193 @@ eval(${"var"});
 	test.RunRulesTest()
 }
 
+func TestRulePathGroup(t *testing.T) {
+	rfile := `<?php
+
+/**
+ * @path-group-name test
+ * @path my/site/ads_
+ */
+_init_test_group_();
+
+/**
+ * @name varEval
+ * @warning don't eval from variable
+ * @path-group test
+ * @path my/site/admin_
+ */
+eval(${"var"});
+`
+	test := linttest.NewSuite(t)
+	test.RuleFile = rfile
+	code := `<?php
+          $hello = 'echo 123;';
+          eval($hello);
+          eval('echo 456;');
+        `
+	test.AddNamedFile("/home/john/my/site/foo.php", code)
+	test.AddNamedFile("/home/john/my/site/ads_foo.php", code)
+	test.AddNamedFile("/home/john/my/site/ads_bar.php", code)
+	test.AddNamedFile("/home/john/my/site/admin_table.php", code)
+
+	test.Expect = []string{
+		`don't eval from variable`,
+		`don't eval from variable`,
+		`don't eval from variable`,
+	}
+	test.RunRulesTest()
+}
+
+func TestRulePathGroupExclude(t *testing.T) {
+	rfile := `<?php
+/**
+ * @path-group-name test
+ * @path www/no
+ */
+_init_test_group_();
+
+
+/**
+ * @name varEval
+ * @warning don't eval from variable
+ * @path www/
+ * @path-group-exclude test
+ */
+eval(${"var"});
+`
+	test := linttest.NewSuite(t)
+	test.RuleFile = rfile
+	code := `<?php
+          $hello = 'echo 123;';
+          eval($hello);
+          eval('echo 456;');
+        `
+	test.AddNamedFile("www/no", code)
+
+	test.RunRulesTest()
+}
+
+func TestRuleExcludeWithPathGroupExclude(t *testing.T) {
+	rfile := `<?php
+/**
+ * @path-group-name test
+ * @path www/no
+ */
+_init_test_group_();
+
+
+/**
+ * @name varEval
+ * @warning don't eval from variable
+ * @path www/
+ * @path-group-exclude test
+ * @path-exclude www/bad
+ */
+eval(${"var"});
+`
+	test := linttest.NewSuite(t)
+	test.RuleFile = rfile
+	code := `<?php
+          $hello = 'echo 123;';
+          eval($hello);
+          eval('echo 456;');
+        `
+	test.AddNamedFile("www/no", code)
+	test.AddNamedFile("www/bad", code)
+
+	test.RunRulesTest()
+}
+
+func TestMultiplePathGroupsInitialization(t *testing.T) {
+	rfile := `<?php
+/**
+ * @path-group-name group1
+ * @path www/no
+ * @path www/yes
+ */
+_init_test_group1_();
+
+/**
+ * @path-group-name group2
+ * @path www/no
+ * @path www/yes
+ */
+_init_test_group2_();
+
+/**
+ * @name testRuleGroup1
+ * @warning don't eval from variable: Group1
+ * @path-group group1
+ */
+eval(${"var"});
+`
+	test := linttest.NewSuite(t)
+	test.RuleFile = rfile
+
+	code := `<?php
+          $hello = 'echo 123;';
+          eval($hello);
+        `
+
+	test.AddNamedFile("www/no", code)
+	test.AddNamedFile("www/yes", code)
+
+	test.Expect = []string{
+		`don't eval from variable`,
+		`don't eval from variable`,
+	}
+
+	test.RunRulesTest()
+}
+
+func TestMultiplePathGroupExclude(t *testing.T) {
+	rfile := `<?php
+/**
+ * @path-group-name safe
+ * @path www/safe
+ */
+_init_test_safe_();
+
+/**
+ * @path-group-name dangerous
+ * @path www/dangerous
+ */
+_init_test_dangerous_();
+
+/**
+ * @name testRule
+ * @warning This rule applies only for dangerous
+ * @path www/
+ * @path-group dangerous
+ * @path-group-exclude safe
+ */
+eval(${"var"});
+`
+	test := linttest.NewSuite(t)
+	test.RuleFile = rfile
+
+	codeSafe := `<?php
+          eval('echo safe;');
+        `
+	codeDangerous := `<?php
+          $hello = 'echo 123;';
+          eval($hello);
+        `
+	codeOther := `<?php
+          eval('echo other;');
+        `
+
+	test.AddNamedFile("www/safe/index.php", codeSafe)
+	test.AddNamedFile("www/dangerous/index.php", codeDangerous)
+	test.AddNamedFile("www/other/index.php", codeOther)
+
+	test.Expect = []string{
+		`This rule applies only for dangerous`,
+	}
+
+	test.RunRulesTest()
+}
+
 func TestAnyRules(t *testing.T) {
 	rfile := `<?php
 /**