Repository: VKCOM/noverify
Pull request #1237 (2024-10-22 17:05:15)

linter: base system for on path rule filtering
tied with [prototype](https://github.com/VKCOM/noverify/pull/1236).


Patch:
diff --git a/src/linter/conf.go b/src/linter/conf.go
index 9055f2e..d720736 100644
--- a/src/linter/conf.go
+++ b/src/linter/conf.go
@@ -1,8 +1,10 @@
 package linter
 
 import (
+	"path/filepath"
 	"regexp"
 	"runtime"
+	"strings"
 	"time"
 
 	"github.com/VKCOM/php-parser/pkg/version"
@@ -38,8 +40,13 @@ type Config struct {
 	// Rules is a set of dynamically loaded linter diagnostics.
 	Rules *rules.Set
 
+	// PathRules is a set of specific rules for paths.
+	PathRules *RuleNode
+
 	// settings
 
+	ProjectPath string
+
 	StubsDir string
 	Debug    bool
 
@@ -68,6 +75,54 @@ type Config struct {
 	StrictMixed bool
 }
 
+type RuleNode struct {
+	Children map[string]*RuleNode // Child nodes (subdirectories and files)
+	Enabled  map[string]bool      // Rules enabled at this level
+	Disabled map[string]bool      // Disabled rules at this level
+}
+
+func NewRuleNode() *RuleNode {
+	return &RuleNode{
+		Children: make(map[string]*RuleNode),
+		Enabled:  make(map[string]bool),
+		Disabled: make(map[string]bool),
+	}
+}
+
+type PathRuleSet struct {
+	Enabled  map[string]bool // Rules that are enabled for this path
+	Disabled map[string]bool // Rules that are disabled for this path
+}
+
+func BuildRuleTree(pathRules map[string]*PathRuleSet) *RuleNode {
+	root := NewRuleNode()
+
+	for path, ruleSet := range pathRules {
+		normalizedPath := filepath.ToSlash(filepath.Clean(path))
+		parts := strings.Split(normalizedPath, "/")
+		currentNode := root
+
+		for _, part := range parts {
+			if part == "" {
+				continue
+			}
+			if _, exists := currentNode.Children[part]; !exists {
+				currentNode.Children[part] = NewRuleNode()
+			}
+			currentNode = currentNode.Children[part]
+		}
+
+		for rule := range ruleSet.Enabled {
+			currentNode.Enabled[rule] = true
+		}
+		for rule := range ruleSet.Disabled {
+			currentNode.Disabled[rule] = true
+		}
+	}
+
+	return root
+}
+
 func NewConfig(ver string) *Config {
 	reg := &CheckersRegistry{
 		info: map[string]CheckerInfo{},
@@ -79,6 +134,7 @@ func NewConfig(ver string) *Config {
 	return &Config{
 		SrcInput:       inputs.NewDefaultSourceInput(),
 		Rules:          rules.NewSet(),
+		PathRules:      NewRuleNode(),
 		MaxConcurrency: runtime.NumCPU(),
 		IsDiscardVar:   isUnderscore,
 		Checkers:       reg,
diff --git a/src/linter/root.go b/src/linter/root.go
index 1f3c4ae..8f36140 100644
--- a/src/linter/root.go
+++ b/src/linter/root.go
@@ -1518,6 +1518,34 @@ func (d *rootWalker) ReportPHPDoc(phpDocLocation PHPDocLocation, level int, chec
 	d.ReportLocation(loc, level, checkName, msg, args...)
 }
 
+func IsRuleEnabledForPath(root *RuleNode, filePath string, checkRule string) bool {
+	normalizedPath := filepath.ToSlash(filepath.Clean(filePath))
+	parts := strings.Split(normalizedPath, "/")
+	currentNode := root
+
+	// Starting with global state. We have guarantee while parsing config that rule is `on` and exist
+	ruleState := true
+
+	for _, part := range parts {
+		if part == "" {
+			continue
+		}
+		if node, exists := currentNode.Children[part]; exists {
+			if node.Disabled[checkRule] {
+				ruleState = false // Disable on this path
+			}
+			if node.Enabled[checkRule] {
+				ruleState = true // Enable on this path
+			}
+			currentNode = node
+		} else {
+			break
+		}
+	}
+
+	return ruleState
+}
+
 func (d *rootWalker) Report(n ir.Node, level int, checkName, msg string, args ...interface{}) {
 	var pos position.Position
 
@@ -1606,6 +1634,14 @@ func (d *rootWalker) ReportLocation(loc ir.Location, level int, checkName, msg s
 		}
 	}
 
+	filePath := d.file.Name()
+
+	rootNode := d.config.PathRules
+
+	if !IsRuleEnabledForPath(rootNode, filePath, checkName) {
+		return
+	}
+
 	d.reports = append(d.reports, &Report{
 		CheckName: checkName,
 		Context:   string(contextLine),


Test patch:
diff --git a/src/tests/infra/pathRulesSet_config_test.go b/src/tests/infra/pathRulesSet_config_test.go
new file mode 100644
index 0000000..df027b2
--- /dev/null
+++ b/src/tests/infra/pathRulesSet_config_test.go
@@ -0,0 +1,285 @@
+package checkers
+
+import (
+	"testing"
+
+	"github.com/VKCOM/noverify/src/linter"
+	"github.com/VKCOM/noverify/src/linttest"
+)
+
+func pathRulesSetInit(t *testing.T) *linttest.Suite {
+	t.Helper()
+	linterConfig := linter.NewConfig("8.1")
+	linterConfig.ProjectPath = "dev/"
+	linterConfig.StrictMixed = true
+
+	linterConfig.PathRules = linter.BuildRuleTree(map[string]*linter.PathRuleSet{
+		"disable-emptyStmt": {
+			Enabled:  map[string]bool{},
+			Disabled: map[string]bool{"emptyStmt": true},
+		},
+		"enable-emptyStmt": {
+			Enabled: map[string]bool{
+				"emptyStmt": true,
+			},
+			Disabled: map[string]bool{},
+		},
+		"mixed/foo.php": {
+			Enabled: map[string]bool{
+				"emptyStmt": true,
+			},
+			Disabled: map[string]bool{
+				"undefinedFunction": true,
+			},
+		},
+	})
+
+	var suite = linttest.NewSuite(t)
+	suite.IgnoreUndeclaredChecks()
+	suite.UseConfig(linterConfig)
+	return suite
+}
+
+func TestDisablePath(t *testing.T) {
+	test := pathRulesSetInit(t)
+	code := `<?php
+require_once 'foo.php';;
+        `
+	test.AddNamedFile("disable-emptyStmt/foo.php", code)
+
+	test.RunAndMatch()
+}
+
+func TestEnablePath(t *testing.T) {
+	test := pathRulesSetInit(t)
+	code := `<?php
+require_once 'foo.php';;
+        `
+	test.AddNamedFile("dev/enable-emptyStmt/foo.php", code)
+
+	test.Expect = []string{
+		`Semicolon (;) is not needed here, it can be safely removed at`,
+	}
+	test.RunAndMatch()
+}
+
+func TestMixedRulesPath(t *testing.T) {
+	test := pathRulesSetInit(t)
+	mergedCode := `<?php
+require_once 'foo.php';;
+function function_exists($name) { return 1 == 2; }
+
+function f($cond) {
+  if (!function_exists('\foo')) {
+    \foo();
+  }
+  if ($cond && !function_exists('bar')) {
+    bar("a", "b");
+  }
+  if ($cond || !function_exists('a\b\baz')) {
+    a\b\baz(1);
+  }
+}
+`
+
+	test.AddNamedFile("mixed/foo.php", mergedCode)
+
+	test.Expect = []string{
+		"Semicolon (;) is not needed here, it can be safely removed at",
+	}
+	test.RunAndMatch()
+}
+
+func TestIsRuleEnabledWithAbsolutePaths(t *testing.T) {
+	pathRules := map[string]*linter.PathRuleSet{
+		"/A": {
+			Enabled:  map[string]bool{"rule1": true},
+			Disabled: map[string]bool{},
+		},
+		"/A/B": {
+			Enabled:  map[string]bool{},
+			Disabled: map[string]bool{"rule1": true},
+		},
+		"/A/B/C": {
+			Enabled:  map[string]bool{"rule2": true},
+			Disabled: map[string]bool{},
+		},
+	}
+
+	ruleTree := linter.BuildRuleTree(pathRules)
+
+	tests := []struct {
+		name      string
+		filePath  string
+		checkRule string
+		want      bool
+	}{
+		{
+			name:      "Absolute path with rule1 disabled in /A/B",
+			filePath:  "/A/B/D/file.php",
+			checkRule: "rule1",
+			want:      false,
+		},
+		{
+			name:      "Absolute path with rule2 enabled in /A/B/C",
+			filePath:  "/A/B/C/file.php",
+			checkRule: "rule2",
+			want:      true,
+		},
+		{
+			name:      "Absolute path with rule1 enabled in /A",
+			filePath:  "/A/X/Y/file.php",
+			checkRule: "rule1",
+			want:      true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := linter.IsRuleEnabledForPath(ruleTree, tt.filePath, tt.checkRule)
+			if got != tt.want {
+				t.Errorf("IsRuleEnabledForPath(%q, %q) = %v; want %v", tt.filePath, tt.checkRule, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestIsRuleEnabledWithRelativeComponents(t *testing.T) {
+	pathRules := map[string]*linter.PathRuleSet{
+		"A/B/C": {
+			Enabled:  map[string]bool{"rule1": true},
+			Disabled: map[string]bool{},
+		},
+	}
+
+	ruleTree := linter.BuildRuleTree(pathRules)
+
+	tests := []struct {
+		name      string
+		filePath  string
+		checkRule string
+		want      bool
+	}{
+		{
+			name:      "Path with '.' components",
+			filePath:  "A/B/./C/file.php",
+			checkRule: "rule1",
+			want:      true,
+		},
+		{
+			name:      "Path with '..' components",
+			filePath:  "A/B/D/../C/file.php",
+			checkRule: "rule1",
+			want:      true,
+		},
+		{
+			name:      "Path leading to different directory due to '..'",
+			filePath:  "A/B/C/../D/file.php",
+			checkRule: "rule1",
+			want:      true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := linter.IsRuleEnabledForPath(ruleTree, tt.filePath, tt.checkRule)
+			if got != tt.want {
+				t.Errorf("IsRuleEnabledForPath(%q, %q) = %v; want %v", tt.filePath, tt.checkRule, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestIsRuleEnabledWithLeadingAndTrailingSlashes(t *testing.T) {
+	pathRules := map[string]*linter.PathRuleSet{
+		"/A/B/C/": {
+			Enabled:  map[string]bool{"rule1": true},
+			Disabled: map[string]bool{},
+		},
+	}
+
+	ruleTree := linter.BuildRuleTree(pathRules)
+
+	tests := []struct {
+		name      string
+		filePath  string
+		checkRule string
+		want      bool
+	}{
+		{
+			name:      "Path with trailing slash in rule and file path",
+			filePath:  "/A/B/C/file.php",
+			checkRule: "rule1",
+			want:      true,
+		},
+		{
+			name:      "Path with trailing slash in rule only",
+			filePath:  "/A/B/C/file.php",
+			checkRule: "rule1",
+			want:      true,
+		},
+		{
+			name:      "Path with trailing slash in file path only",
+			filePath:  "/A/B/C//file.php",
+			checkRule: "rule1",
+			want:      true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := linter.IsRuleEnabledForPath(ruleTree, tt.filePath, tt.checkRule)
+			if got != tt.want {
+				t.Errorf("IsRuleEnabledForPath(%q, %q) = %v; want %v", tt.filePath, tt.checkRule, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestIsRuleEnabledWithConflictingRules(t *testing.T) {
+	pathRules := map[string]*linter.PathRuleSet{
+		"A": {
+			Enabled:  map[string]bool{"rule1": true},
+			Disabled: map[string]bool{},
+		},
+		"A/B": {
+			Enabled:  map[string]bool{},
+			Disabled: map[string]bool{"rule1": true},
+		},
+		"A/B/C": {
+			Enabled:  map[string]bool{"rule1": true},
+			Disabled: map[string]bool{},
+		},
+	}
+
+	ruleTree := linter.BuildRuleTree(pathRules)
+
+	tests := []struct {
+		name      string
+		filePath  string
+		checkRule string
+		want      bool
+	}{
+		{
+			name:      "Rule1 re-enabled in A/B/C after being disabled in A/B",
+			filePath:  "A/B/C/file.php",
+			checkRule: "rule1",
+			want:      true,
+		},
+		{
+			name:      "Rule1 remains disabled in A/B/D",
+			filePath:  "A/B/D/file.php",
+			checkRule: "rule1",
+			want:      false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := linter.IsRuleEnabledForPath(ruleTree, tt.filePath, tt.checkRule)
+			if got != tt.want {
+				t.Errorf("IsRuleEnabled(%q, %q) = %v; want %v", tt.filePath, tt.checkRule, got, tt.want)
+			}
+		})
+	}
+}