From d9ce580a866b51e776a78a6cbe4e28b3571d1c9e Mon Sep 17 00:00:00 2001 From: Pietro Campagnano Date: Sun, 26 Jul 2026 23:18:32 +0200 Subject: [PATCH 1/2] feat: match baseline violations without asking the user how Choosing between matching with or without line numbers was a decision about how the baseline is compared, not about what to enforce, and both answers were wrong somewhere: the default reopened known violations after any edit above them, while --ignore-baseline-linenumbers could not tell two violations of the same rule in the same class apart. Matching is now a single strategy with no knob. Violations still sitting where the baseline recorded them pair first; among what is left, class and reported problem alone decide, so an edit above a violation moves it without making it new, and identical violations in one class stay distinct. When a violation is added among moved ones, it is the added one that is reported. --ignore-baseline-linenumbers and ignoreBaselineLinenumbers() are deprecated: they have no effect and print a notice. generate-baseline always writes line numbers; baselines stored without them keep working and need no regeneration. Closes #662 Refs #636 Co-Authored-By: Claude Opus 5 --- README.md | 12 +-- src/CLI/Baseline.php | 12 +-- src/CLI/CheckHandler.php | 5 +- src/CLI/Command/CommonOptions.php | 9 ++- src/CLI/Config.php | 5 ++ src/CLI/DeprecationNotice.php | 13 +++ src/CLI/GenerateBaselineHandler.php | 2 +- src/CLI/Runner.php | 2 +- src/Rules/Violations.php | 81 +++++++++++++------ tests/E2E/Cli/CheckCommandTest.php | 18 +++-- tests/E2E/Cli/GenerateBaselineCommandTest.php | 8 +- tests/E2E/Cli/PruneBaselineCommandTest.php | 24 ++++-- tests/Unit/CLI/BaselineTest.php | 15 ++-- tests/Unit/Rules/ViolationsTest.php | 56 ++++++++----- 14 files changed, 176 insertions(+), 86 deletions(-) create mode 100644 src/CLI/DeprecationNotice.php diff --git a/README.md b/README.md index 55a86a1c..d08b8120 100644 --- a/README.md +++ b/README.md @@ -144,7 +144,7 @@ Every setting can be passed as a CLI option or set via the corresponding `Config | `--autoload` | `-a` | `autoloadFilePath()` | Autoload file to load before running. Required for all Phar runs. | | `--use-baseline` | `-b` | `baselineFilePath()` | Baseline file path for ignoring known violations. | | `--skip-baseline` | `-k` | `skipBaseline()` | Skips the default baseline even if present. | -| `--ignore-baseline-linenumbers` | `-i` | `ignoreBaselineLinenumbers()` | Matches baseline violations without checking line numbers. | +| `--ignore-baseline-linenumbers` | `-i` | `ignoreBaselineLinenumbers()` | **Deprecated**: has no effect, baseline matching already tolerates moved violations. | | `--config` | `-c` | — | Configuration file to load (default: `phparkitect.php`). | | `--verbose` | `-v` | — | Prints every parsed file instead of the progress bar. | | — | — | `skipParsingCustomAnnotations()` | Disables custom DocBlock annotation parsing (enabled by default). | @@ -165,15 +165,17 @@ When violations get fixed over time, prune the baseline instead of regenerating phparkitect prune-baseline ``` -Pruning only removes entries that no longer match a current violation — it never adds anything. Regenerating snapshots the entire current state, so it would silently legitimize any new violation introduced since the baseline was created; pruning cannot, which makes it safe to run routinely (even automated). Since matching ignores line numbers and the kept entries are saved with their current ones, pruning also refreshes a baseline whose line numbers went stale after refactorings. `check` prints a hint when it detects baseline entries that look fixed. +Pruning only removes entries that no longer match a current violation — it never adds anything. Regenerating snapshots the entire current state, so it would silently legitimize any new violation introduced since the baseline was created; pruning cannot, which makes it safe to run routinely (even automated). Pruning also refreshes a baseline whose line numbers went stale after refactorings, since the kept entries are saved with their current ones. `check` prints a hint when it detects baseline entries that look fixed. -Both `generate-baseline` and `prune-baseline` accept an optional custom file name as argument and the same `--config`, `--target-php-version` and `--autoload` options as `check`; `generate-baseline` also accepts `--ignore-baseline-linenumbers` to write the baseline without line numbers. `prune-baseline` needs no such flag: matching already ignores line numbers, and a baseline stored without line numbers keeps its format when pruned. +Both `generate-baseline` and `prune-baseline` accept an optional custom file name as argument and the same `--config`, `--target-php-version` and `--autoload` options as `check`. > **Note**: baseline generation was previously a `check` option (`check --generate-baseline`); it is now a dedicated command, and the old option fails with a pointer to the new one. -By default the baseline also checks line numbers — a change before the offending line shifts the number and the check fails. Use `--ignore-baseline-linenumbers` to match violations regardless of line number. +#### How baseline entries are matched -> **Warning**: when ignoring line numbers, PHPArkitect cannot detect if the same rule is violated additional times in the same file. +You don't have to choose: a violation is identified by its class and by what it reports, never by where it sits in the file. Entries are matched first by exact position, and whatever is left over is matched within the same class and rule, in file order. A change above the offending line therefore doesn't reopen a known violation, while two violations of the same rule in the same class stay distinct — and when a new one appears among them, it's the new one that gets reported. + +`--ignore-baseline-linenumbers` / `ignoreBaselineLinenumbers()` used to select this behaviour and is now **deprecated**: it has no effect and will be removed in the next major version. Existing baselines keep working and need no regeneration, whether or not they store line numbers. ### Output format diff --git a/src/CLI/Baseline.php b/src/CLI/Baseline.php index b2ffa3cb..743e9295 100644 --- a/src/CLI/Baseline.php +++ b/src/CLI/Baseline.php @@ -41,22 +41,22 @@ public function getViolations(): Violations * given set untouched: what is left to report is in the returned result, * together with the number of baseline entries nothing matched. */ - public function applyTo(Violations $violations, bool $ignoreBaselineLinenumbers): BaselineResult + public function applyTo(Violations $violations): BaselineResult { - $match = $violations->matchAgainst($this->violations, $ignoreBaselineLinenumbers); + $match = $violations->matchAgainst($this->violations); return new BaselineResult($match->new(), $match->stale()->count()); } /** * Shrink-only update: returns a baseline containing only the entries that - * still match a current violation — nothing is ever added. Matching - * ignores line numbers and the current violations are the ones kept, so - * pruning also refreshes line numbers gone stale after refactorings. + * still match a current violation — nothing is ever added. The current + * violations are the ones kept, so pruning also refreshes line numbers + * gone stale after refactorings. */ public function prune(Violations $currentViolations): self { - $prunedViolations = $currentViolations->matchAgainst($this->violations, true)->known(); + $prunedViolations = $currentViolations->matchAgainst($this->violations)->known(); // a baseline stored without line numbers keeps its format if (!$this->hasLineNumbers()) { diff --git a/src/CLI/CheckHandler.php b/src/CLI/CheckHandler.php index 8a5e93f0..06832301 100644 --- a/src/CLI/CheckHandler.php +++ b/src/CLI/CheckHandler.php @@ -32,9 +32,12 @@ public function check( ->autoloadFilePath($options->getAutoloadFilePath()) ->stopOnFailure($options->isStopOnFailure()) ->targetPhpVersion(TargetPhpVersion::create($options->getTargetPhpVersion())) - ->ignoreBaselineLinenumbers($options->isIgnoreBaselineLinenumbers()) ->format($options->getFormat()); + if ($options->isIgnoreBaselineLinenumbers() || $config->isIgnoreBaselineLinenumbers()) { + $output->writeln(DeprecationNotice::IGNORE_BASELINE_LINENUMBERS); + } + $baselineFilePath = $options->getBaselineFilePath(); if (null === $baselineFilePath) { diff --git a/src/CLI/Command/CommonOptions.php b/src/CLI/Command/CommonOptions.php index 91e674e7..e42dd0b7 100644 --- a/src/CLI/Command/CommonOptions.php +++ b/src/CLI/Command/CommonOptions.php @@ -50,9 +50,10 @@ public function addTo(Command $command): void } /** - * Not part of addTo() because not every analysis command takes it: - * prune-baseline always matches ignoring line numbers and preserves - * the baseline's stored format instead. + * Not part of addTo() because prune-baseline never took it. + * + * The option is deprecated and has no effect: it is still registered so + * that the commands accept it and can warn who is passing it. */ public function addIgnoreBaselineLinenumbers(Command $command): void { @@ -60,7 +61,7 @@ public function addIgnoreBaselineLinenumbers(Command $command): void self::IGNORE_BASELINE_LINENUMBERS_PARAM, 'i', InputOption::VALUE_NONE, - 'Ignore line numbers when checking or generating the baseline' + 'Deprecated: has no effect, baseline matching already tolerates moved violations' ); } diff --git a/src/CLI/Config.php b/src/CLI/Config.php index 361a16aa..f6ec0336 100644 --- a/src/CLI/Config.php +++ b/src/CLI/Config.php @@ -120,6 +120,11 @@ public function getBaselineFilePath(): ?string return $this->baselineFilePath; } + /** + * @deprecated baseline matching no longer depends on line numbers; this + * option has no effect and will be removed in the next major + * version. The value is kept only to warn the user about it. + */ public function ignoreBaselineLinenumbers(bool $ignoreBaselineLinenumbers): self { $this->ignoreBaselineLinenumbers = $ignoreBaselineLinenumbers; diff --git a/src/CLI/DeprecationNotice.php b/src/CLI/DeprecationNotice.php new file mode 100644 index 00000000..69c4025f --- /dev/null +++ b/src/CLI/DeprecationNotice.php @@ -0,0 +1,13 @@ +getViolations()); if ($options->isIgnoreBaselineLinenumbers()) { - $baseline = $baseline->withoutLineNumbers(); + $output->writeln(DeprecationNotice::IGNORE_BASELINE_LINENUMBERS); } $this->baselineRepository->save($baseline, $options->getBaselineFilePath()); diff --git a/src/CLI/Runner.php b/src/CLI/Runner.php index 0739bf74..21c76635 100644 --- a/src/CLI/Runner.php +++ b/src/CLI/Runner.php @@ -20,7 +20,7 @@ public function run(Config $config, Baseline $baseline, Progress $progress): Ana { [$violations, $parsingErrors] = $this->doRun($config, $progress); - $baselineResult = $baseline->applyTo($violations, $config->isIgnoreBaselineLinenumbers()); + $baselineResult = $baseline->applyTo($violations); return new AnalysisResult( $baselineResult->getRemainingViolations(), diff --git a/src/Rules/Violations.php b/src/Rules/Violations.php index f15be301..e0068368 100644 --- a/src/Rules/Violations.php +++ b/src/Rules/Violations.php @@ -82,37 +82,29 @@ public function toArray(): array * $baseline, one to one: what matched, what is new and what the baseline * still claims but nothing matches anymore. * - * A violation is identified by its class and by the problem it reports; - * $ignoreLineNumbers decides whether where it sits in the file is part of - * that identity too. + * Pairing happens in two passes: first the violations still sitting where + * the baseline recorded them, then, among what is left, the ones matching + * by class and reported problem alone — an edit above a violation moves it + * without making it a new one. The line number is therefore never part of + * the identity of a violation, only a hint on which entry of a group to + * pair with, so there is nothing for the user to choose here. + * + * When a group of identical violations both moved and grew, which of them + * is reported as new is a guess; how many are is not. */ - public function matchAgainst(self $baseline, bool $ignoreLineNumbers): ViolationsMatch + public function matchAgainst(self $baseline): ViolationsMatch { - $key = $ignoreLineNumbers ? [__CLASS__, 'violationKey'] : [__CLASS__, 'positionKey']; - $unpairedByKey = self::indexBy($baseline->violations, $key); - - $known = []; - $new = []; - $paired = []; - - foreach ($this->violations as $violation) { - $violationKey = $key($violation); - - if ([] === ($unpairedByKey[$violationKey] ?? [])) { - $new[] = $violation; - - continue; - } + [$stillThere, $moved, $paired] = self::pairWith($this->violations, $baseline->violations, [__CLASS__, 'positionKey']); - // the bucket was just checked to be non-empty, so array_pop() returns an index - /** @psalm-suppress PossiblyNullArrayOffset */ - $paired[array_pop($unpairedByKey[$violationKey])] = true; - $known[] = $violation; - } + $unpairedEntries = array_diff_key($baseline->violations, $paired); - $stale = array_diff_key($baseline->violations, $paired); + [$movedAndKnown, $new, $pairedMoved] = self::pairWith($moved, $unpairedEntries, [__CLASS__, 'violationKey']); - return new ViolationsMatch(self::fromArray($known), self::fromArray($new), self::fromArray($stale)); + return new ViolationsMatch( + self::fromArray(array_intersect_key($this->violations, $stillThere + $movedAndKnown)), + self::fromArray($new), + self::fromArray(array_diff_key($unpairedEntries, $pairedMoved)) + ); } public function withoutLineNumbers(): self @@ -135,6 +127,43 @@ public function jsonSerialize(): array return get_object_vars($this); } + /** + * Pairs each violation with an entry carrying the same key, in the order + * both appear in their file, and reports what paired with what. + * + * @param array $violations + * @param array $entries + * @param callable(Violation):string $key + * + * @return array{array, array, array} the paired violations, the unpaired ones, the paired entries + */ + private static function pairWith(array $violations, array $entries, callable $key): array + { + $entriesByKey = self::indexBy($entries, $key); + + $matched = []; + $unpaired = []; + $paired = []; + $pairedPerKey = []; + + foreach ($violations as $idx => $violation) { + $violationKey = $key($violation); + $alreadyPaired = $pairedPerKey[$violationKey] ?? 0; + + if (!isset($entriesByKey[$violationKey][$alreadyPaired])) { + $unpaired[$idx] = $violation; + + continue; + } + + $pairedPerKey[$violationKey] = $alreadyPaired + 1; + $paired[$entriesByKey[$violationKey][$alreadyPaired]] = true; + $matched[$idx] = true; + } + + return [$matched, $unpaired, $paired]; + } + /** * Groups the given violations by the key the callback derives from each * of them, so that only the ones that can possibly match are compared. diff --git a/tests/E2E/Cli/CheckCommandTest.php b/tests/E2E/Cli/CheckCommandTest.php index 79b8952e..e808db72 100644 --- a/tests/E2E/Cli/CheckCommandTest.php +++ b/tests/E2E/Cli/CheckCommandTest.php @@ -179,17 +179,23 @@ public function test_dependencies_should_not_leak_between_files(): void self::assertCommandWasSuccessful($cmdTester); } - public function test_baseline_line_numbers_can_be_ignored(): void + public function test_baseline_matches_violations_whose_line_number_moved(): void { $configFilePath = __DIR__.'/../_fixtures/configIgnoreBaselineLineNumbers.php'; - // No errors when ignoring baseline line numbers - $cmdTester = $this->runCheck($configFilePath, null, __DIR__.'/../_fixtures/line_numbers/baseline.json', false, true); + $cmdTester = $this->runCheck($configFilePath, null, __DIR__.'/../_fixtures/line_numbers/baseline.json'); + self::assertCommandWasSuccessful($cmdTester); + } - // Errors when not ignoring baseline line numbers - $cmdTester = $this->runCheck($configFilePath, null, __DIR__.'/../_fixtures/line_numbers/baseline.json'); - self::assertCommandExitedWithError($cmdTester); + public function test_deprecated_ignore_baseline_linenumbers_warns_and_changes_nothing(): void + { + $configFilePath = __DIR__.'/../_fixtures/configIgnoreBaselineLineNumbers.php'; + + $cmdTester = $this->runCheck($configFilePath, null, __DIR__.'/../_fixtures/line_numbers/baseline.json', false, true); + + self::assertCommandWasSuccessful($cmdTester); + self::assertStringContainsString('is deprecated and has no effect', $cmdTester->getErrorOutput()); } public function test_baseline_reports_stale_violations(): void diff --git a/tests/E2E/Cli/GenerateBaselineCommandTest.php b/tests/E2E/Cli/GenerateBaselineCommandTest.php index 545fe483..f988ab67 100644 --- a/tests/E2E/Cli/GenerateBaselineCommandTest.php +++ b/tests/E2E/Cli/GenerateBaselineCommandTest.php @@ -55,20 +55,20 @@ public function test_creates_the_baseline_with_a_custom_filename(): void self::assertFileDoesNotExist($this->defaultBaselineFilename); } - public function test_can_ignore_line_numbers(): void + public function test_deprecated_ignore_line_numbers_warns_and_still_writes_them(): void { $cmdTester = $this->runGenerateBaseline( - __DIR__.'/../_fixtures/configMvcForYieldBug.php', + __DIR__.'/../_fixtures/configIgnoreBaselineLineNumbers.php', $this->customBaselineFilename, true ); self::assertEquals(self::SUCCESS_CODE, $cmdTester->getStatusCode()); + self::assertStringContainsString('is deprecated and has no effect', $cmdTester->getDisplay()); $baseline = json_decode((string) file_get_contents($this->customBaselineFilename), true); - self::assertCount(1, $baseline['violations']); - self::assertNull($baseline['violations'][0]['line']); + self::assertNotNull($baseline['violations'][0]['line']); } public function test_fails_gracefully_when_the_config_file_does_not_exist(): void diff --git a/tests/E2E/Cli/PruneBaselineCommandTest.php b/tests/E2E/Cli/PruneBaselineCommandTest.php index 26896261..af7a264a 100644 --- a/tests/E2E/Cli/PruneBaselineCommandTest.php +++ b/tests/E2E/Cli/PruneBaselineCommandTest.php @@ -70,22 +70,20 @@ public function test_pruning_an_up_to_date_baseline_changes_nothing(): void } /** - * Pruning has no --ignore-baseline-linenumbers flag: it always matches - * ignoring line numbers and infers the format to save from the baseline - * itself, so a line-numberless workflow has nothing to remember. + * Baselines written by older versions with --ignore-baseline-linenumbers + * store no line number: pruning keeps that format instead of silently + * upgrading the file. */ - public function test_pruning_preserves_a_baseline_generated_without_line_numbers(): void + public function test_pruning_preserves_a_baseline_stored_without_line_numbers(): void { - // this fixture violates a dependency rule, so its violations carry a - // line number: without -i the baseline would store one $configFilePath = __DIR__.'/../_fixtures/configIgnoreBaselineLineNumbers.php'; $this->runCommand([ 'generate-baseline', '--config' => $configFilePath, 'filename' => $this->customBaselineFilename, - '--ignore-baseline-linenumbers' => true, ]); + $this->stripLineNumbersFrom($this->customBaselineFilename); $cmdTester = $this->runCommand(['prune-baseline', '--config' => $configFilePath, 'filename' => $this->customBaselineFilename]); @@ -104,7 +102,6 @@ public function test_pruning_preserves_a_baseline_generated_without_line_numbers 'check', '--config' => $configFilePath, '--use-baseline' => $this->customBaselineFilename, - '--ignore-baseline-linenumbers' => true, ]); self::assertEquals(self::SUCCESS_CODE, $cmdTester->getStatusCode()); @@ -131,4 +128,15 @@ protected function runCommand(array $input): ApplicationTester return $appTester; } + + private function stripLineNumbersFrom(string $baselineFilename): void + { + $baseline = json_decode((string) file_get_contents($baselineFilename), true); + + foreach ($baseline['violations'] as $idx => $violation) { + $baseline['violations'][$idx]['line'] = null; + } + + file_put_contents($baselineFilename, json_encode($baseline)); + } } diff --git a/tests/Unit/CLI/BaselineTest.php b/tests/Unit/CLI/BaselineTest.php index 73ca9983..fac4d4da 100644 --- a/tests/Unit/CLI/BaselineTest.php +++ b/tests/Unit/CLI/BaselineTest.php @@ -25,7 +25,7 @@ public function test_apply_to_removes_baseline_violations_and_counts_stale_entri $current = new Violations(); $current->add($stillPresent); - $result = $baseline->applyTo($current, false); + $result = $baseline->applyTo($current); self::assertCount(0, $result->getRemainingViolations()); self::assertSame(1, $result->getStaleBaselineEntriesCount()); @@ -41,7 +41,7 @@ public function test_apply_to_does_not_mutate_the_given_violations(): void $current = new Violations(); $current->add($violation); - Baseline::fromViolations($baselineViolations)->applyTo($current, false); + Baseline::fromViolations($baselineViolations)->applyTo($current); self::assertCount(1, $current); } @@ -104,14 +104,17 @@ public function test_prune_preserves_a_line_numberless_baseline_format(): void self::assertNull($pruned->getViolations()->get(0)->getLine()); } - public function test_without_line_numbers_returns_a_copy_with_stripped_line_numbers(): void + public function test_apply_to_keeps_a_violation_moved_by_an_edit_above_it(): void { $baselineViolations = new Violations(); $baselineViolations->add(new Violation('App\Controller\Shop', 'should have name end with Controller', 10)); - $stripped = Baseline::fromViolations($baselineViolations)->withoutLineNumbers(); + $current = new Violations(); + $current->add(new Violation('App\Controller\Shop', 'should have name end with Controller', 42)); - self::assertNull($stripped->getViolations()->get(0)->getLine()); - self::assertEquals(10, $baselineViolations->get(0)->getLine()); + $result = Baseline::fromViolations($baselineViolations)->applyTo($current); + + self::assertCount(0, $result->getRemainingViolations()); + self::assertSame(0, $result->getStaleBaselineEntriesCount()); } } diff --git a/tests/Unit/Rules/ViolationsTest.php b/tests/Unit/Rules/ViolationsTest.php index 3a1b4d99..ebdb29cc 100644 --- a/tests/Unit/Rules/ViolationsTest.php +++ b/tests/Unit/Rules/ViolationsTest.php @@ -101,7 +101,7 @@ public function test_match_reports_the_violations_the_baseline_does_not_know(): $violationsBaseline = new Violations(); $violationsBaseline->add($this->violation); - $new = $this->violationStore->matchAgainst($violationsBaseline, false)->new(); + $new = $this->violationStore->matchAgainst($violationsBaseline)->new(); self::assertCount(2, $new); self::assertEquals([ @@ -171,10 +171,10 @@ public function test_match_pairs_when_rule_description_changes(): void 10 )); - self::assertCount(0, $violations->matchAgainst($baseline, false)->new()); + self::assertCount(0, $violations->matchAgainst($baseline)->new()); } - public function test_match_pairs_when_rule_description_changes_ignore_linenumber(): void + public function test_match_pairs_when_rule_description_changes_and_the_violation_moved(): void { $violations = new Violations(); $violations->add(new Violation( @@ -190,7 +190,7 @@ public function test_match_pairs_when_rule_description_changes_ignore_linenumber 10 )); - self::assertCount(0, $violations->matchAgainst($baseline, true)->new()); + self::assertCount(0, $violations->matchAgainst($baseline)->new()); } public function test_match_reports_a_duplicate_the_baseline_only_knows_once(): void @@ -204,7 +204,28 @@ public function test_match_reports_a_duplicate_the_baseline_only_knows_once(): v $violations->add(new Violation('App\Foo', $error, 10, 'src/Foo.php')); $violations->add(new Violation('App\Foo', $error, 10, 'src/Foo.php')); - self::assertCount(1, $violations->matchAgainst($baseline, false)->new(), 'each baseline entry covers one violation, not every identical one'); + self::assertCount(1, $violations->matchAgainst($baseline)->new(), 'each baseline entry covers one violation, not every identical one'); + } + + public function test_match_reports_the_violation_that_was_really_added(): void + { + $error = 'depends on App\Bar, but should depend only on classes in one of these namespaces: App\Domain'; + + $baseline = new Violations(); + $baseline->add(new Violation('App\Foo', $error, 10)); + $baseline->add(new Violation('App\Foo', $error, 20)); + $baseline->add(new Violation('App\Foo', $error, 30)); + + $violations = new Violations(); + $violations->add(new Violation('App\Foo', $error, 10)); + $violations->add(new Violation('App\Foo', $error, 15)); + $violations->add(new Violation('App\Foo', $error, 20)); + $violations->add(new Violation('App\Foo', $error, 30)); + + $new = $violations->matchAgainst($baseline)->new(); + + self::assertCount(1, $new); + self::assertSame(15, $new->get(0)->getLine(), 'the untouched violations pair by position, so what is left is the added one'); } public function test_match_does_not_pair_different_dependency(): void @@ -223,7 +244,7 @@ public function test_match_does_not_pair_different_dependency(): void 10 )); - self::assertCount(1, $violations->matchAgainst($baseline, false)->new()); + self::assertCount(1, $violations->matchAgainst($baseline)->new()); } public function test_match_pairs_self_explanatory_messages(): void @@ -240,7 +261,7 @@ public function test_match_pairs_self_explanatory_messages(): void 'should be final because we want immutability' )); - self::assertCount(0, $violations->matchAgainst($baseline, false)->new()); + self::assertCount(0, $violations->matchAgainst($baseline)->new()); } public function test_match_pairs_self_explanatory_messages_when_because_is_reworded(): void @@ -257,7 +278,7 @@ public function test_match_pairs_self_explanatory_messages_when_because_is_rewor 'should be final because we want immutability' )); - self::assertCount(0, $violations->matchAgainst($baseline, false)->new()); + self::assertCount(0, $violations->matchAgainst($baseline)->new()); } public function test_violation_without_line_number_returns_copy_with_null_line(): void @@ -328,7 +349,7 @@ public function test_remove_violations_from_violations_ignore_linenumber(): void 21 )); - $new = $this->violationStore->matchAgainst($violationsBaseline, true)->new(); + $new = $this->violationStore->matchAgainst($violationsBaseline)->new(); self::assertCount(3, $new); self::assertEquals([ @@ -346,7 +367,7 @@ public function test_match_stale_returns_zero_when_everything_still_occurs(): vo $current = new Violations(); $current->add($this->violation); - self::assertCount(0, $current->matchAgainst($baseline, false)->stale()); + self::assertCount(0, $current->matchAgainst($baseline)->stale()); } public function test_match_stale_counts_fixed_baseline_entries(): void @@ -361,10 +382,10 @@ public function test_match_stale_counts_fixed_baseline_entries(): void $current = new Violations(); $current->add($stillPresent); - self::assertCount(1, $current->matchAgainst($baseline, false)->stale()); + self::assertCount(1, $current->matchAgainst($baseline)->stale()); } - public function test_match_stale_counts_fixed_baseline_entries_ignoring_line_numbers(): void + public function test_match_does_not_call_stale_a_violation_that_only_moved(): void { $stillPresent = new Violation('App\Controller\Shop', 'should have name end with Controller', 10); $fixed = new Violation('App\Controller\Shop', 'should implement AbstractController', 20); @@ -377,8 +398,7 @@ public function test_match_stale_counts_fixed_baseline_entries_ignoring_line_num $current = new Violations(); $current->add($stillPresentMovedLine); - self::assertCount(1, $current->matchAgainst($baseline, true)->stale()); - self::assertCount(2, $current->matchAgainst($baseline, false)->stale()); + self::assertCount(1, $current->matchAgainst($baseline)->stale()); } public function test_match_stale_does_not_mutate_either_set(): void @@ -391,7 +411,7 @@ public function test_match_stale_does_not_mutate_either_set(): void $current = new Violations(); $current->add($stillPresent); - $current->matchAgainst($baseline, true); + $current->matchAgainst($baseline); self::assertCount(1, $baseline); self::assertCount(1, $current); @@ -407,7 +427,7 @@ public function test_match_known_keeps_only_matching_violations_with_this_sets_l $baseline->add(new Violation('App\Controller\Shop', 'should have name end with Controller', 10)); $baseline->add(new Violation('App\Controller\Fixed', 'should have name end with Controller', 3)); - $intersection = $current->matchAgainst($baseline, true)->known(); + $intersection = $current->matchAgainst($baseline)->known(); self::assertCount(1, $intersection); self::assertEquals('App\Controller\Shop', $intersection->get(0)->getFqcn()); @@ -425,7 +445,7 @@ public function test_match_known_matches_each_entry_at_most_once(): void $baseline = new Violations(); $baseline->add($duplicated); - self::assertCount(1, $current->matchAgainst($baseline, true)->known()); + self::assertCount(1, $current->matchAgainst($baseline)->known()); } public function test_match_known_does_not_mutate_either_set(): void @@ -438,7 +458,7 @@ public function test_match_known_does_not_mutate_either_set(): void $baseline = new Violations(); $baseline->add($violation); - $current->matchAgainst($baseline, true); + $current->matchAgainst($baseline); self::assertCount(1, $current); self::assertCount(1, $baseline); From 7a94f3f33f861f3ba9e6b1b8c5330163d55442dc Mon Sep 17 00:00:00 2001 From: Pietro Campagnano Date: Thu, 30 Jul 2026 18:37:17 +0200 Subject: [PATCH 2/2] docs: document the baseline linenumbers deprecation in UPGRADE.md Co-Authored-By: Claude Opus 5 --- UPGRADE.md | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 58cb66be..9b4f0134 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -8,6 +8,30 @@ PHPArkitect, ordered from the most recent version to the oldest. ## 1.3.0 +### `--ignore-baseline-linenumbers` is deprecated + +Baseline matching no longer depends on line numbers: a violation is identified +by its class and by what it reports, so an edit above the offending line does +not reopen a known violation, and two violations of the same rule in the same +class stay distinct. The option that used to select this behaviour has no +effect and prints a deprecation notice, both from the CLI and from +`phparkitect.php`: + +```diff +- phparkitect check --ignore-baseline-linenumbers ++ phparkitect check + +- $config->ignoreBaselineLinenumbers(true); ++ $config; +``` + +It will be removed in the next major version. + +`generate-baseline` now always writes line numbers — the option no longer +strips them. **Existing baselines keep working and need no regeneration**, +whether or not they store line numbers, and `prune-baseline` still preserves +the format it finds. + ### `check --generate-baseline` is now the `generate-baseline` command Generating a baseline was an action disguised as a `check` option: it ran a @@ -24,8 +48,7 @@ a dedicated command: The optional filename is now an argument (still defaulting to `phparkitect-baseline.json`), and the command accepts the same `--config`, -`--target-php-version`, `--autoload` and `--ignore-baseline-linenumbers` -options as before. The check-only options that never affected generation +`--target-php-version` and `--autoload` options as before. The check-only options that never affected generation (`--stop-on-failure`, `--format`, `--use-baseline`, `--skip-baseline`) are no longer accepted.