From 0558b4d204e6cdc583fd36997f0acbf097196883 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 28 Feb 2026 16:53:04 +0000 Subject: [PATCH 1/2] Fix baseline invalidation when allowed namespaces change (#377) Extract the stable violation-specific part from error messages during baseline comparison, so that changes to rule configuration (e.g. adding or removing allowed namespaces) do not invalidate existing baseline entries. https://claude.ai/code/session_01DPotRHDJzXnmPGPoCVp8Rn --- src/Rules/Violations.php | 43 +++++++++++++-- tests/Unit/Rules/ViolationsTest.php | 82 +++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 3 deletions(-) diff --git a/src/Rules/Violations.php b/src/Rules/Violations.php index 4ef4e206..54ab7261 100644 --- a/src/Rules/Violations.php +++ b/src/Rules/Violations.php @@ -98,7 +98,7 @@ public function remove(self $violations, bool $ignoreBaselineLinenumbers = false foreach ($baselineViolations as $baseIdx => $baselineViolation) { if ( $baselineViolation->getFqcn() === $violation->getFqcn() - && $baselineViolation->getError() === $violation->getError() + && self::extractViolationKey($baselineViolation->getError()) === self::extractViolationKey($violation->getError()) ) { unset($this->violations[$idx], $baselineViolations[$baseIdx]); continue 2; @@ -121,10 +121,24 @@ public function jsonSerialize(): array /** * Comparison method that respects all fields in the violation. + * + * Uses the stable violation key (the part before ', but ') for comparison, + * so that changes to rule configuration (e.g. allowed namespaces) do not + * invalidate existing baseline entries. */ public static function compareViolations(Violation $a, Violation $b): int { - return $a <=> $b; + return [ + $a->getFqcn(), + $a->getLine(), + $a->getFilePath(), + self::extractViolationKey($a->getError()), + ] <=> [ + $b->getFqcn(), + $b->getLine(), + $b->getFilePath(), + self::extractViolationKey($b->getError()), + ]; } /** @@ -132,10 +146,33 @@ public static function compareViolations(Violation $a, Violation $b): int */ public static function compareViolationsIgnoreLineNumber(Violation $a, Violation $b): int { - if (($a->getFqcn() === $b->getFqcn()) && ($a->getError() === $b->getError())) { + if ( + $a->getFqcn() === $b->getFqcn() + && self::extractViolationKey($a->getError()) === self::extractViolationKey($b->getError()) + ) { return 0; } return self::compareViolations($a, $b); } + + /** + * Extracts the stable violation-specific part from an error message. + * + * ViolationMessage produces two formats: + * - withDescription: "$violation, but $ruleDescription" → returns $violation + * - selfExplanatory: "$ruleDescription" (no ", but ") → returns the full string + * + * The rule description may include configuration-dependent values (like namespace lists) + * that change when the rule config is updated. The violation part is always stable. + */ + private static function extractViolationKey(string $error): string + { + $pos = strpos($error, ', but '); + if (false !== $pos) { + return substr($error, 0, $pos); + } + + return $error; + } } diff --git a/tests/Unit/Rules/ViolationsTest.php b/tests/Unit/Rules/ViolationsTest.php index ca0299e8..b5eaf755 100644 --- a/tests/Unit/Rules/ViolationsTest.php +++ b/tests/Unit/Rules/ViolationsTest.php @@ -155,6 +155,88 @@ public function test_sort(): void ], $violationStore->toArray()); } + public function test_remove_violations_matches_when_rule_description_changes(): void + { + $violations = new Violations(); + $violations->add(new Violation( + 'App\Foo', + 'depends on App\Bar, but should depend only on classes in one of these namespaces: App\Domain, App\Shared', + 10 + )); + + $baseline = new Violations(); + $baseline->add(new Violation( + 'App\Foo', + 'depends on App\Bar, but should depend only on classes in one of these namespaces: App\Domain', + 10 + )); + + $violations->remove($baseline); + + self::assertCount(0, $violations); + } + + public function test_remove_violations_matches_when_rule_description_changes_ignore_linenumber(): void + { + $violations = new Violations(); + $violations->add(new Violation( + 'App\Foo', + 'depends on App\Bar, but should depend only on classes in one of these namespaces: App\Domain, App\Shared', + 15 + )); + + $baseline = new Violations(); + $baseline->add(new Violation( + 'App\Foo', + 'depends on App\Bar, but should depend only on classes in one of these namespaces: App\Domain', + 10 + )); + + $violations->remove($baseline, true); + + self::assertCount(0, $violations); + } + + public function test_remove_violations_does_not_match_different_dependency(): void + { + $violations = new Violations(); + $violations->add(new Violation( + 'App\Foo', + 'depends on App\Baz, but should depend only on classes in one of these namespaces: App\Domain', + 10 + )); + + $baseline = new Violations(); + $baseline->add(new Violation( + 'App\Foo', + 'depends on App\Bar, but should depend only on classes in one of these namespaces: App\Domain', + 10 + )); + + $violations->remove($baseline); + + self::assertCount(1, $violations); + } + + public function test_remove_violations_still_works_for_self_explanatory_messages(): void + { + $violations = new Violations(); + $violations->add(new Violation( + 'App\Foo', + 'should be final because we want immutability' + )); + + $baseline = new Violations(); + $baseline->add(new Violation( + 'App\Foo', + 'should be final because we want immutability' + )); + + $violations->remove($baseline); + + self::assertCount(0, $violations); + } + public function test_remove_violations_from_violations_ignore_linenumber(): void { $violation1 = new Violation( From cd5fadd52b1892c4f72935870a7eca7bce8fcfe3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Mar 2026 15:03:05 +0000 Subject: [PATCH 2/2] Remove unused compareViolationsIgnoreLineNumber method https://claude.ai/code/session_01DPotRHDJzXnmPGPoCVp8Rn --- src/Rules/Violations.php | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/Rules/Violations.php b/src/Rules/Violations.php index 54ab7261..47fe5f08 100644 --- a/src/Rules/Violations.php +++ b/src/Rules/Violations.php @@ -141,21 +141,6 @@ public static function compareViolations(Violation $a, Violation $b): int ]; } - /** - * Comparison method that only checks the namespace and error but ignores the line number. - */ - public static function compareViolationsIgnoreLineNumber(Violation $a, Violation $b): int - { - if ( - $a->getFqcn() === $b->getFqcn() - && self::extractViolationKey($a->getError()) === self::extractViolationKey($b->getError()) - ) { - return 0; - } - - return self::compareViolations($a, $b); - } - /** * Extracts the stable violation-specific part from an error message. *