Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 29 additions & 7 deletions src/Rules/Violations.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -121,21 +121,43 @@ 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()),
];
}

/**
* Comparison method that only checks the namespace and error but ignores the line number.
* 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.
*/
public static function compareViolationsIgnoreLineNumber(Violation $a, Violation $b): int
private static function extractViolationKey(string $error): string
{
if (($a->getFqcn() === $b->getFqcn()) && ($a->getError() === $b->getError())) {
return 0;
$pos = strpos($error, ', but ');
if (false !== $pos) {
return substr($error, 0, $pos);
}

return self::compareViolations($a, $b);
return $error;
}
}
82 changes: 82 additions & 0 deletions tests/Unit/Rules/ViolationsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down