From dfbee999d52e36d74ad5bcadfa8ad0aa53638f36 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Jul 2026 14:35:47 +0000 Subject: [PATCH 1/8] Add prune-baseline command with shrink-only semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #649. Regenerating the baseline snapshots the entire current state, so it silently legitimizes any violation introduced since the baseline was created. prune-baseline is the safe counterpart: phparkitect prune-baseline [filename] It runs the analysis and keeps only the intersection between the existing baseline and the current violations — the baseline can only shrink. Matching ignores line numbers and the kept entries are the current violations, so pruning also refreshes line numbers gone stale after refactorings. Unlike generation, pruning is safe to automate. The check hint introduced in #638 now points to the new command instead of suggesting a (risky) full regeneration. This change also splits the two responsibilities Baseline used to mix, since pruning needs the full load -> transform -> save lifecycle: - Baseline is now a pure domain object: applyTo(), stale counting, the new prune() (built on the new Violations::intersection(), the complementary operation of Violations::remove(), reusing the same stable-key matching) and withoutLineNumbers(). - BaselineFileRepository owns every filesystem concern: load(), save(), resolveFilePath() and the default filename. Check, generate-baseline and prune-baseline all take it via constructor injection. The command follows the same structure as its siblings, composing CommonOptions and CommandRuntime. --ignore-baseline-linenumbers on prune-baseline strips line numbers from the saved file (matching already ignores them by design). Covered by unit tests (Baseline, BaselineFileRepository, PruneBaselineHandler, Violations::intersection) and E2E tests; README and CLAUDE.md updated. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015K1TKiwtwkW7twBCTz758P --- CLAUDE.md | 4 +- README.md | 10 +- src/CLI/Baseline.php | 75 +++++-------- src/CLI/BaselineFileRepository.php | 42 +++++++ src/CLI/CheckHandler.php | 13 ++- src/CLI/Command/Check.php | 6 +- src/CLI/Command/GenerateBaseline.php | 6 +- src/CLI/Command/PruneBaseline.php | 104 ++++++++++++++++++ src/CLI/GenerateBaselineHandler.php | 18 +-- src/CLI/PhpArkitectApplication.php | 2 + src/CLI/PruneBaselineHandler.php | 53 +++++++++ src/CLI/PruneBaselineOptions.php | 46 ++++++++ src/Rules/Violations.php | 29 +++++ tests/E2E/Cli/CheckCommandTest.php | 2 +- tests/E2E/Cli/PruneBaselineCommandTest.php | 92 ++++++++++++++++ tests/Unit/CLI/BaselineFileRepositoryTest.php | 64 +++++++++++ tests/Unit/CLI/BaselineTest.php | 88 +++++++++++++++ tests/Unit/CLI/CheckHandlerTest.php | 9 +- .../Unit/CLI/GenerateBaselineHandlerTest.php | 7 +- tests/Unit/CLI/PruneBaselineHandlerTest.php | 103 +++++++++++++++++ tests/Unit/Rules/ViolationsTest.php | 47 ++++++++ 21 files changed, 744 insertions(+), 76 deletions(-) create mode 100644 src/CLI/BaselineFileRepository.php create mode 100644 src/CLI/Command/PruneBaseline.php create mode 100644 src/CLI/PruneBaselineHandler.php create mode 100644 src/CLI/PruneBaselineOptions.php create mode 100644 tests/E2E/Cli/PruneBaselineCommandTest.php create mode 100644 tests/Unit/CLI/BaselineFileRepositoryTest.php create mode 100644 tests/Unit/CLI/BaselineTest.php create mode 100644 tests/Unit/CLI/PruneBaselineHandlerTest.php diff --git a/CLAUDE.md b/CLAUDE.md index 8a164aba..6d43c1ee 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -41,13 +41,13 @@ Before committing, `make csfix && make psalm && make test` must all pass (CI gat ## Pipeline (how a `check` run flows) -`bin/phparkitect` → `PhpArkitectApplication` registers the commands in `src/CLI/Command/` (`Check`, `GenerateBaseline`, `Init`, `DebugExpression`). Options shared between analysis-running commands (`--config`, `--target-php-version`, `--autoload`, `--ignore-baseline-linenumbers`) are defined once in `CommonOptions`, and the shared console plumbing (process limits, progress, heading, timing) in `CommandRuntime` — both composed by each command. +`bin/phparkitect` → `PhpArkitectApplication` registers the commands in `src/CLI/Command/` (`Check`, `GenerateBaseline`, `PruneBaseline`, `Init`, `DebugExpression`). Options shared between analysis-running commands (`--config`, `--target-php-version`, `--autoload`, `--ignore-baseline-linenumbers`) are defined once in `CommonOptions`, and the shared console plumbing (process limits, progress, heading, timing) in `CommandRuntime` — both composed by each command. `Check::execute` (`src/CLI/Command/Check.php`) parses the CLI options into a `CheckOptions` value object and delegates to `CheckHandler` (`src/CLI/CheckHandler.php`), the application service that runs the actual flow: 1. `ConfigBuilder` loads the user's `phparkitect.php`, which receives a `Config` and registers `ClassSet`s + `Rule`s. 2. `Runner` (`src/CLI/Runner.php`) iterates every `SplFileInfo` in each `ClassSet`, parses it via `FileParser`, and gets back `ClassDescription`s. 3. Each `Rule` is evaluated against each `ClassDescription`, accumulating `Violations`. -4. `Baseline` (`src/CLI/Baseline.php`) filters out known/ignored violations. +4. `Baseline` (`src/CLI/Baseline.php`, a pure domain object persisted via `BaselineFileRepository`) filters out known/ignored violations. 5. A `Printer` (`text` / `json` / `gitlab`, `src/CLI/Printer/`) renders the result; exit code is non-zero if violations remain. ## Architecture — the two halves diff --git a/README.md b/README.md index 27d12d71..db1ef092 100644 --- a/README.md +++ b/README.md @@ -159,7 +159,15 @@ phparkitect generate-baseline This creates `phparkitect-baseline.json`. Subsequent `check` runs pick it up automatically. Use a custom file name with `phparkitect generate-baseline my-baseline.json`, point `check` to it with `--use-baseline=my-baseline.json`, or skip it entirely with `--skip-baseline`. -The `generate-baseline` command accepts the same `--config`, `--target-php-version`, `--autoload` and `--ignore-baseline-linenumbers` options as `check`. +When violations get fixed over time, prune the baseline instead of regenerating it: + +``` +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. + +Both `generate-baseline` and `prune-baseline` accept an optional custom file name as argument and the same `--config`, `--target-php-version`, `--autoload` and `--ignore-baseline-linenumbers` 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. diff --git a/src/CLI/Baseline.php b/src/CLI/Baseline.php index 07fe4130..7861ea2a 100644 --- a/src/CLI/Baseline.php +++ b/src/CLI/Baseline.php @@ -1,29 +1,41 @@ violations = $violations; - $this->filename = $filename; } - public function getFilename(): string + public static function fromViolations(Violations $violations): self + { + return new self($violations); + } + + public static function empty(): self + { + return new self(new Violations()); + } + + public function getViolations(): Violations { - return $this->filename; + return $this->violations; } public function applyTo(Violations $violations, bool $ignoreBaselineLinenumbers): void @@ -44,49 +56,18 @@ public function getStaleViolationsCount(): int } /** - * @psalm-suppress RiskyTruthyFalsyComparison + * 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. */ - public static function resolveFilePath(?string $filePath, string $defaultFilePath): ?string - { - if (!$filePath && file_exists($defaultFilePath)) { - $filePath = $defaultFilePath; - } - - return $filePath ?: null; - } - - public static function empty(): self - { - return new self(new Violations(), ''); - } - - public static function create(bool $skipBaseline, ?string $baselineFilePath): self + public function prune(Violations $currentViolations): self { - if ($skipBaseline || null === $baselineFilePath) { - return self::empty(); - } - - return self::loadFromFile($baselineFilePath); - } - - public static function loadFromFile(string $filename): self - { - if (!file_exists($filename)) { - throw new \RuntimeException("Baseline file '$filename' not found."); - } - - return new self( - Violations::fromJson(file_get_contents($filename)), - $filename - ); + return new self($currentViolations->intersection($this->violations)); } - public static function save(string $filename, Violations $violations, bool $ignoreLineNumbers = false): void + public function withoutLineNumbers(): self { - if ($ignoreLineNumbers) { - $violations = $violations->withoutLineNumbers(); - } - - file_put_contents($filename, json_encode($violations, \JSON_PRETTY_PRINT)); + return new self($this->violations->withoutLineNumbers()); } } diff --git a/src/CLI/BaselineFileRepository.php b/src/CLI/BaselineFileRepository.php new file mode 100644 index 00000000..82b0ac0a --- /dev/null +++ b/src/CLI/BaselineFileRepository.php @@ -0,0 +1,42 @@ +getViolations(), \JSON_PRETTY_PRINT)); + } + + /** + * @psalm-suppress RiskyTruthyFalsyComparison + */ + public static function resolveFilePath(?string $filePath): ?string + { + if (!$filePath && file_exists(self::DEFAULT_FILENAME)) { + $filePath = self::DEFAULT_FILENAME; + } + + return $filePath ?: null; + } +} diff --git a/src/CLI/CheckHandler.php b/src/CLI/CheckHandler.php index 8fcaa6c1..aa4acc11 100644 --- a/src/CLI/CheckHandler.php +++ b/src/CLI/CheckHandler.php @@ -16,8 +16,10 @@ */ final class CheckHandler { - public function __construct(private Runner $runner) - { + public function __construct( + private Runner $runner, + private BaselineFileRepository $baselineRepository, + ) { } public function check( @@ -35,9 +37,10 @@ public function check( ->skipBaseline($options->isSkipBaseline()) ->format($options->getFormat()); - $baseline = Baseline::create($config->isSkipBaseline(), $config->getBaselineFilePath()); + $baselineFilePath = $config->isSkipBaseline() ? null : $config->getBaselineFilePath(); + $baseline = null === $baselineFilePath ? Baseline::empty() : $this->baselineRepository->load($baselineFilePath); - $baseline->getFilename() && $output->writeln("Baseline file '{$baseline->getFilename()}' found"); + null !== $baselineFilePath && $output->writeln("Baseline file '$baselineFilePath' found"); $output->writeln("Config file '{$options->getConfigFilePath()}' found\n"); @@ -74,7 +77,7 @@ private function printStaleBaselineViolations(Baseline $baseline, OutputInterfac $verb = 1 === $staleViolationsCount ? 'looks' : 'look'; $pronoun = 1 === $staleViolationsCount ? 'it' : 'them'; $noun = 1 === $staleViolationsCount ? 'violation' : 'violations'; - $output->writeln("💡 {$staleViolationsCount} {$noun} in the baseline {$verb} fixed — regenerate the baseline to remove {$pronoun}"); + $output->writeln("💡 {$staleViolationsCount} {$noun} in the baseline {$verb} fixed — run `phparkitect prune-baseline` to remove {$pronoun}"); } } } diff --git a/src/CLI/Command/Check.php b/src/CLI/Command/Check.php index b70a1937..b1490476 100644 --- a/src/CLI/Command/Check.php +++ b/src/CLI/Command/Check.php @@ -4,7 +4,7 @@ namespace Arkitect\CLI\Command; -use Arkitect\CLI\Baseline; +use Arkitect\CLI\BaselineFileRepository; use Arkitect\CLI\CheckHandler; use Arkitect\CLI\CheckOptions; use Arkitect\CLI\Runner; @@ -37,7 +37,7 @@ public function __construct(?CheckHandler $handler = null) parent::__construct('check'); - $this->handler = $handler ?? new CheckHandler(new Runner()); + $this->handler = $handler ?? new CheckHandler(new Runner(), new BaselineFileRepository()); $this->runtime = new CommandRuntime(); } @@ -144,7 +144,7 @@ protected function parseOptions(InputInterface $input): CheckOptions configFilePath: $this->commonOptions->configFilePath($input), targetPhpVersion: $this->commonOptions->targetPhpVersion($input), stopOnFailure: (bool) $input->getOption(self::STOP_ON_FAILURE_PARAM), - baselineFilePath: Baseline::resolveFilePath($useBaseline, Baseline::DEFAULT_FILENAME), + baselineFilePath: BaselineFileRepository::resolveFilePath($useBaseline), skipBaseline: (bool) $input->getOption(self::SKIP_BASELINE_PARAM), ignoreBaselineLinenumbers: $this->commonOptions->isIgnoreBaselineLinenumbers($input), format: (string) $input->getOption(self::FORMAT_PARAM), diff --git a/src/CLI/Command/GenerateBaseline.php b/src/CLI/Command/GenerateBaseline.php index 8cb26e44..bdfab54c 100644 --- a/src/CLI/Command/GenerateBaseline.php +++ b/src/CLI/Command/GenerateBaseline.php @@ -4,7 +4,7 @@ namespace Arkitect\CLI\Command; -use Arkitect\CLI\Baseline; +use Arkitect\CLI\BaselineFileRepository; use Arkitect\CLI\GenerateBaselineHandler; use Arkitect\CLI\GenerateBaselineOptions; use Arkitect\CLI\Runner; @@ -32,7 +32,7 @@ public function __construct(?GenerateBaselineHandler $handler = null) parent::__construct('generate-baseline'); - $this->handler = $handler ?? new GenerateBaselineHandler(new Runner()); + $this->handler = $handler ?? new GenerateBaselineHandler(new Runner(), new BaselineFileRepository()); $this->runtime = new CommandRuntime(); } @@ -45,7 +45,7 @@ protected function configure(): void self::FILENAME_ARG, InputArgument::OPTIONAL, 'The baseline file to create', - Baseline::DEFAULT_FILENAME + BaselineFileRepository::DEFAULT_FILENAME ); $this->commonOptions->addTo($this); diff --git a/src/CLI/Command/PruneBaseline.php b/src/CLI/Command/PruneBaseline.php new file mode 100644 index 00000000..a0516183 --- /dev/null +++ b/src/CLI/Command/PruneBaseline.php @@ -0,0 +1,104 @@ +commonOptions = new CommonOptions(); + + parent::__construct('prune-baseline'); + + $this->handler = $handler ?? new PruneBaselineHandler(new Runner(), new BaselineFileRepository()); + $this->runtime = new CommandRuntime(); + } + + protected function configure(): void + { + $this + ->setDescription('Remove from the baseline the violations that no longer exist, without adding anything.') + ->setHelp('This command runs the analysis and keeps in the baseline only the entries that still match a current violation. It never adds entries, so unlike regenerating the baseline it cannot hide new violations, and it refreshes stale line numbers.') + ->addArgument( + self::FILENAME_ARG, + InputArgument::OPTIONAL, + 'The baseline file to prune', + BaselineFileRepository::DEFAULT_FILENAME + ); + + $this->commonOptions->addTo($this); + } + + protected function isRunningAsPhar(): bool + { + return '' !== \Phar::running(); + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + $this->runtime->raiseLimits(); + $startTime = microtime(true); + + // we write everything on STDERR to be consistent with the check command + $output = $output instanceof ConsoleOutputInterface ? $output->getErrorOutput() : $output; + + try { + $verbose = (bool) $input->getOption('verbose'); + $options = $this->parseOptions($input); + + if ($this->isRunningAsPhar() && null === $options->getAutoloadFilePath()) { + $output->writeln('❌ The --autoload option is required when running phparkitect as a PHAR'); + + return self::FAILURE; + } + + $this->runtime->printHeadingLine($this, $output); + $this->commonOptions->requireAutoload($input, $output); + + $progress = $this->runtime->createProgress($output, $verbose); + + $this->handler->pruneBaseline($options, $progress, $output); + + return self::SUCCESS; + } catch (\Throwable $e) { + $output->writeln("❌ {$e->getMessage()}"); + + return self::FAILURE; + } finally { + $this->runtime->printExecutionTime($output, $startTime); + } + } + + protected function parseOptions(InputInterface $input): PruneBaselineOptions + { + return new PruneBaselineOptions( + configFilePath: $this->commonOptions->configFilePath($input), + targetPhpVersion: $this->commonOptions->targetPhpVersion($input), + autoloadFilePath: $this->commonOptions->autoloadFilePath($input), + ignoreBaselineLinenumbers: $this->commonOptions->isIgnoreBaselineLinenumbers($input), + baselineFilePath: (string) $input->getArgument(self::FILENAME_ARG), + ); + } +} diff --git a/src/CLI/GenerateBaselineHandler.php b/src/CLI/GenerateBaselineHandler.php index 420dc2c1..9fe72c57 100644 --- a/src/CLI/GenerateBaselineHandler.php +++ b/src/CLI/GenerateBaselineHandler.php @@ -15,8 +15,10 @@ */ final class GenerateBaselineHandler { - public function __construct(private Runner $runner) - { + public function __construct( + private Runner $runner, + private BaselineFileRepository $baselineRepository, + ) { } public function generateBaseline(GenerateBaselineOptions $options, Progress $progress, OutputInterface $output): void @@ -29,11 +31,13 @@ public function generateBaseline(GenerateBaselineOptions $options, Progress $pro $result = $this->runner->baseline($config, $progress); - Baseline::save( - $options->getBaselineFilePath(), - $result->getViolations(), - $options->isIgnoreBaselineLinenumbers() - ); + $baseline = Baseline::fromViolations($result->getViolations()); + + if ($options->isIgnoreBaselineLinenumbers()) { + $baseline = $baseline->withoutLineNumbers(); + } + + $this->baselineRepository->save($baseline, $options->getBaselineFilePath()); $output->writeln("ℹ️ Baseline file '{$options->getBaselineFilePath()}' created!"); } diff --git a/src/CLI/PhpArkitectApplication.php b/src/CLI/PhpArkitectApplication.php index 8d778dae..264c465c 100644 --- a/src/CLI/PhpArkitectApplication.php +++ b/src/CLI/PhpArkitectApplication.php @@ -8,6 +8,7 @@ use Arkitect\CLI\Command\DebugExpression; use Arkitect\CLI\Command\GenerateBaseline; use Arkitect\CLI\Command\Init; +use Arkitect\CLI\Command\PruneBaseline; class PhpArkitectApplication extends \Symfony\Component\Console\Application { @@ -29,6 +30,7 @@ public function __construct() $this->$addMethod(new Check()); $this->$addMethod(new GenerateBaseline()); + $this->$addMethod(new PruneBaseline()); $this->$addMethod(new Init()); $this->$addMethod(new DebugExpression()); } diff --git a/src/CLI/PruneBaselineHandler.php b/src/CLI/PruneBaselineHandler.php new file mode 100644 index 00000000..2eb15bec --- /dev/null +++ b/src/CLI/PruneBaselineHandler.php @@ -0,0 +1,53 @@ +baselineRepository->load($options->getBaselineFilePath()); + + $output->writeln("Baseline file '{$options->getBaselineFilePath()}' found"); + + $config = ConfigBuilder::loadFromFile($options->getConfigFilePath()) + ->autoloadFilePath($options->getAutoloadFilePath()) + ->targetPhpVersion(TargetPhpVersion::create($options->getTargetPhpVersion())); + + $output->writeln("Config file '{$options->getConfigFilePath()}' found\n"); + + $result = $this->runner->baseline($config, $progress); + + $prunedBaseline = $baseline->prune($result->getViolations()); + + if ($options->isIgnoreBaselineLinenumbers()) { + $prunedBaseline = $prunedBaseline->withoutLineNumbers(); + } + + $this->baselineRepository->save($prunedBaseline, $options->getBaselineFilePath()); + + $keptCount = $prunedBaseline->getViolations()->count(); + $removedCount = $baseline->getViolations()->count() - $keptCount; + + $output->writeln("ℹ️ Baseline file '{$options->getBaselineFilePath()}' pruned: {$removedCount} removed, {$keptCount} kept"); + } +} diff --git a/src/CLI/PruneBaselineOptions.php b/src/CLI/PruneBaselineOptions.php new file mode 100644 index 00000000..a6fa9946 --- /dev/null +++ b/src/CLI/PruneBaselineOptions.php @@ -0,0 +1,46 @@ +configFilePath; + } + + public function getTargetPhpVersion(): ?string + { + return $this->targetPhpVersion; + } + + public function getAutoloadFilePath(): ?string + { + return $this->autoloadFilePath; + } + + public function isIgnoreBaselineLinenumbers(): bool + { + return $this->ignoreBaselineLinenumbers; + } + + public function getBaselineFilePath(): string + { + return $this->baselineFilePath; + } +} diff --git a/src/Rules/Violations.php b/src/Rules/Violations.php index 6cbc3680..86d80def 100644 --- a/src/Rules/Violations.php +++ b/src/Rules/Violations.php @@ -143,6 +143,35 @@ public function countUnmatchedIn(self $current, bool $ignoreLineNumbers): int return $unmatched; } + /** + * Returns a new set with the violations from this set (typically the + * current run) that have a matching entry in $other (typically the + * baseline). Matching uses the stable violation key and ignores line + * numbers, so the returned violations carry this set's line numbers. + * Each entry in $other matches at most one violation, so duplicated + * violations are kept only as many times as they appear in $other. + */ + public function intersection(self $other): self + { + $result = new self(); + + $otherViolations = $other->violations; + foreach ($this->violations as $violation) { + foreach ($otherViolations as $idx => $otherViolation) { + if ( + $otherViolation->getFqcn() === $violation->getFqcn() + && self::extractViolationKey($otherViolation->getError()) === self::extractViolationKey($violation->getError()) + ) { + $result->add($violation); + unset($otherViolations[$idx]); + continue 2; + } + } + } + + return $result; + } + public function withoutLineNumbers(): self { $copy = new self(); diff --git a/tests/E2E/Cli/CheckCommandTest.php b/tests/E2E/Cli/CheckCommandTest.php index b4da6896..813b5c81 100644 --- a/tests/E2E/Cli/CheckCommandTest.php +++ b/tests/E2E/Cli/CheckCommandTest.php @@ -212,7 +212,7 @@ public function test_baseline_reports_stale_violations(): void $cmdTester = $this->runCheck($configFilePath, null, $this->customBaselineFilename); self::assertCommandWasSuccessful($cmdTester); - self::assertStringContainsString('💡 1 violation in the baseline looks fixed — regenerate the baseline to remove it', $cmdTester->getErrorOutput()); + self::assertStringContainsString('💡 1 violation in the baseline looks fixed — run `phparkitect prune-baseline` to remove it', $cmdTester->getErrorOutput()); } public function test_json_format_output_errors(): void diff --git a/tests/E2E/Cli/PruneBaselineCommandTest.php b/tests/E2E/Cli/PruneBaselineCommandTest.php new file mode 100644 index 00000000..7601f380 --- /dev/null +++ b/tests/E2E/Cli/PruneBaselineCommandTest.php @@ -0,0 +1,92 @@ +customBaselineFilename)) { + unlink($this->customBaselineFilename); + } + } + + public function test_prunes_fixed_violations_and_keeps_the_still_existing_ones(): void + { + $configFilePath = __DIR__.'/../_fixtures/configMvcForYieldBug.php'; + + // Produce a baseline matching the current violation + $this->runCommand(['generate-baseline', '--config' => $configFilePath, 'filename' => $this->customBaselineFilename]); + + // Add a fake, already-fixed entry + $baseline = json_decode((string) file_get_contents($this->customBaselineFilename), true); + $baseline['violations'][] = [ + 'fqcn' => 'App\Controller\AlreadyFixed', + 'error' => 'should have a name that matches *Controller because all controllers should be end name with Controller', + 'line' => 1, + 'filePath' => 'Controller/AlreadyFixed.php', + ]; + file_put_contents($this->customBaselineFilename, json_encode($baseline)); + + $cmdTester = $this->runCommand(['prune-baseline', '--config' => $configFilePath, 'filename' => $this->customBaselineFilename]); + + self::assertEquals(self::SUCCESS_CODE, $cmdTester->getStatusCode()); + self::assertStringContainsString("ℹ️ Baseline file '{$this->customBaselineFilename}' pruned: 1 removed, 1 kept", $cmdTester->getErrorOutput()); + + $pruned = json_decode((string) file_get_contents($this->customBaselineFilename), true); + + self::assertCount(1, $pruned['violations']); + self::assertSame('App\Controller\Foo', $pruned['violations'][0]['fqcn']); + + // The pruned baseline still makes the check pass + $cmdTester = $this->runCommand(['check', '--config' => $configFilePath, '--use-baseline' => $this->customBaselineFilename]); + + self::assertEquals(self::SUCCESS_CODE, $cmdTester->getStatusCode()); + self::assertStringNotContainsString('💡', $cmdTester->getErrorOutput()); + } + + public function test_pruning_an_up_to_date_baseline_changes_nothing(): void + { + $configFilePath = __DIR__.'/../_fixtures/configMvcForYieldBug.php'; + + $this->runCommand(['generate-baseline', '--config' => $configFilePath, 'filename' => $this->customBaselineFilename]); + + $cmdTester = $this->runCommand(['prune-baseline', '--config' => $configFilePath, 'filename' => $this->customBaselineFilename]); + + self::assertEquals(self::SUCCESS_CODE, $cmdTester->getStatusCode()); + self::assertStringContainsString('pruned: 0 removed, 1 kept', $cmdTester->getErrorOutput()); + } + + public function test_fails_gracefully_when_the_baseline_file_does_not_exist(): void + { + $configFilePath = __DIR__.'/../_fixtures/configMvcForYieldBug.php'; + + $cmdTester = $this->runCommand(['prune-baseline', '--config' => $configFilePath, 'filename' => $this->customBaselineFilename]); + + self::assertEquals(self::ERROR_CODE, $cmdTester->getStatusCode()); + self::assertStringContainsString('not found', $cmdTester->getErrorOutput()); + } + + protected function runCommand(array $input): ApplicationTester + { + $app = new PhpArkitectApplication(); + $app->setAutoExit(false); + + $appTester = new ApplicationTester($app); + $appTester->run($input, ['capture_stderr_separately' => true]); + + return $appTester; + } +} diff --git a/tests/Unit/CLI/BaselineFileRepositoryTest.php b/tests/Unit/CLI/BaselineFileRepositoryTest.php new file mode 100644 index 00000000..3ce9d30f --- /dev/null +++ b/tests/Unit/CLI/BaselineFileRepositoryTest.php @@ -0,0 +1,64 @@ +baselineFilePath)) { + unlink($this->baselineFilePath); + } + } + + public function test_save_and_load_round_trip(): void + { + $violations = new Violations(); + $violations->add(new Violation('App\Controller\Shop', 'should have name end with Controller', 10, 'Controller/Shop.php')); + + $repository = new BaselineFileRepository(); + $repository->save(Baseline::fromViolations($violations), $this->baselineFilePath); + + $loaded = $repository->load($this->baselineFilePath); + + self::assertEquals($violations, $loaded->getViolations()); + } + + public function test_load_throws_when_the_file_does_not_exist(): void + { + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage("Baseline file 'not-a-real-file.json' not found."); + + (new BaselineFileRepository())->load('not-a-real-file.json'); + } + + public function test_resolve_file_path_prefers_the_explicit_path(): void + { + self::assertSame('custom.json', BaselineFileRepository::resolveFilePath('custom.json')); + } + + public function test_resolve_file_path_is_null_when_nothing_is_given_and_no_default_exists(): void + { + // the default baseline is resolved against the current working + // directory, so move to an empty one to make the test deterministic + $cwd = (string) getcwd(); + chdir(sys_get_temp_dir()); + + try { + self::assertNull(BaselineFileRepository::resolveFilePath('')); + self::assertNull(BaselineFileRepository::resolveFilePath(null)); + } finally { + chdir($cwd); + } + } +} diff --git a/tests/Unit/CLI/BaselineTest.php b/tests/Unit/CLI/BaselineTest.php new file mode 100644 index 00000000..bde05c5f --- /dev/null +++ b/tests/Unit/CLI/BaselineTest.php @@ -0,0 +1,88 @@ +add($stillPresent); + $baselineViolations->add($alreadyFixed); + + $baseline = Baseline::fromViolations($baselineViolations); + + $current = new Violations(); + $current->add($stillPresent); + + $baseline->applyTo($current, false); + + self::assertCount(0, $current); + self::assertSame(1, $baseline->getStaleViolationsCount()); + } + + public function test_prune_keeps_only_entries_matching_a_current_violation(): void + { + $stillPresent = new Violation('App\Controller\Shop', 'should have name end with Controller', 10); + $alreadyFixed = new Violation('App\Controller\Fixed', 'should have name end with Controller', 3); + + $baselineViolations = new Violations(); + $baselineViolations->add($stillPresent); + $baselineViolations->add($alreadyFixed); + + $current = new Violations(); + $current->add($stillPresent); + + $pruned = Baseline::fromViolations($baselineViolations)->prune($current); + + self::assertCount(1, $pruned->getViolations()); + self::assertEquals('App\Controller\Shop', $pruned->getViolations()->get(0)->getFqcn()); + } + + public function test_prune_never_adds_new_violations(): void + { + $newViolation = new Violation('App\Controller\NewViolation', 'should implement ContainerInterface', 5); + + $current = new Violations(); + $current->add($newViolation); + + $pruned = Baseline::empty()->prune($current); + + self::assertCount(0, $pruned->getViolations()); + } + + public function test_prune_refreshes_stale_line_numbers(): void + { + $baselineViolations = new Violations(); + $baselineViolations->add(new Violation('App\Controller\Shop', 'should have name end with Controller', 10)); + + $current = new Violations(); + $current->add(new Violation('App\Controller\Shop', 'should have name end with Controller', 42)); + + $pruned = Baseline::fromViolations($baselineViolations)->prune($current); + + self::assertCount(1, $pruned->getViolations()); + self::assertEquals(42, $pruned->getViolations()->get(0)->getLine()); + } + + public function test_without_line_numbers_returns_a_copy_with_stripped_line_numbers(): void + { + $baselineViolations = new Violations(); + $baselineViolations->add(new Violation('App\Controller\Shop', 'should have name end with Controller', 10)); + + $stripped = Baseline::fromViolations($baselineViolations)->withoutLineNumbers(); + + self::assertNull($stripped->getViolations()->get(0)->getLine()); + self::assertEquals(10, $baselineViolations->get(0)->getLine()); + } +} diff --git a/tests/Unit/CLI/CheckHandlerTest.php b/tests/Unit/CLI/CheckHandlerTest.php index b6d8d3ca..cf318ad6 100644 --- a/tests/Unit/CLI/CheckHandlerTest.php +++ b/tests/Unit/CLI/CheckHandlerTest.php @@ -4,6 +4,7 @@ namespace Arkitect\Tests\Unit\CLI; +use Arkitect\CLI\BaselineFileRepository; use Arkitect\CLI\CheckHandler; use Arkitect\CLI\CheckOptions; use Arkitect\CLI\GenerateBaselineHandler; @@ -26,7 +27,7 @@ protected function tearDown(): void public function test_check_reports_violations(): void { - $handler = new CheckHandler(new Runner()); + $handler = new CheckHandler(new Runner(), new BaselineFileRepository()); $output = new BufferedOutput(); $violationsOutput = new BufferedOutput(); @@ -44,7 +45,7 @@ public function test_check_reports_violations(): void public function test_check_reports_success_when_there_are_no_violations(): void { - $handler = new CheckHandler(new Runner()); + $handler = new CheckHandler(new Runner(), new BaselineFileRepository()); $output = new BufferedOutput(); $violationsOutput = new BufferedOutput(); @@ -61,7 +62,7 @@ public function test_check_reports_success_when_there_are_no_violations(): void public function test_check_applies_the_baseline(): void { - $generateBaselineHandler = new GenerateBaselineHandler(new Runner()); + $generateBaselineHandler = new GenerateBaselineHandler(new Runner(), new BaselineFileRepository()); $output = new BufferedOutput(); $generateBaselineHandler->generateBaseline( @@ -76,7 +77,7 @@ public function test_check_applies_the_baseline(): void $output ); - $handler = new CheckHandler(new Runner()); + $handler = new CheckHandler(new Runner(), new BaselineFileRepository()); $result = $handler->check( $this->createOptions( diff --git a/tests/Unit/CLI/GenerateBaselineHandlerTest.php b/tests/Unit/CLI/GenerateBaselineHandlerTest.php index 52f59d3a..2c9ec334 100644 --- a/tests/Unit/CLI/GenerateBaselineHandlerTest.php +++ b/tests/Unit/CLI/GenerateBaselineHandlerTest.php @@ -4,6 +4,7 @@ namespace Arkitect\Tests\Unit\CLI; +use Arkitect\CLI\BaselineFileRepository; use Arkitect\CLI\GenerateBaselineHandler; use Arkitect\CLI\GenerateBaselineOptions; use Arkitect\CLI\Progress\VoidProgress; @@ -24,7 +25,7 @@ protected function tearDown(): void public function test_generate_baseline_writes_the_violations_to_a_file(): void { - $handler = new GenerateBaselineHandler(new Runner()); + $handler = new GenerateBaselineHandler(new Runner(), new BaselineFileRepository()); $output = new BufferedOutput(); $handler->generateBaseline( @@ -44,7 +45,7 @@ public function test_generate_baseline_writes_the_violations_to_a_file(): void public function test_generate_baseline_can_omit_line_numbers(): void { - $handler = new GenerateBaselineHandler(new Runner()); + $handler = new GenerateBaselineHandler(new Runner(), new BaselineFileRepository()); $output = new BufferedOutput(); $handler->generateBaseline( @@ -64,7 +65,7 @@ public function test_generate_baseline_can_omit_line_numbers(): void public function test_generate_baseline_writes_an_empty_baseline_when_there_are_no_violations(): void { - $handler = new GenerateBaselineHandler(new Runner()); + $handler = new GenerateBaselineHandler(new Runner(), new BaselineFileRepository()); $output = new BufferedOutput(); $handler->generateBaseline( diff --git a/tests/Unit/CLI/PruneBaselineHandlerTest.php b/tests/Unit/CLI/PruneBaselineHandlerTest.php new file mode 100644 index 00000000..ffe918d1 --- /dev/null +++ b/tests/Unit/CLI/PruneBaselineHandlerTest.php @@ -0,0 +1,103 @@ +baselineFilePath)) { + unlink($this->baselineFilePath); + } + } + + public function test_prune_removes_fixed_entries_and_keeps_the_still_existing_ones(): void + { + $this->writeBaseline([ + // still occurring, but with a stale line number + ['fqcn' => 'App\Foo', 'error' => 'should have a name that matches *Controller because we want uniform naming', 'line' => 99, 'filePath' => 'src/Foo.php'], + // already fixed, must be pruned + ['fqcn' => 'App\AlreadyFixed', 'error' => 'should have a name that matches *Controller because we want uniform naming', 'line' => 1, 'filePath' => 'src/AlreadyFixed.php'], + ]); + + $handler = new PruneBaselineHandler(new Runner(), new BaselineFileRepository()); + $output = new BufferedOutput(); + + $handler->pruneBaseline( + $this->createOptions(configFilePath: __DIR__.'/_fixtures/checkhandler/configWithViolations.php'), + new VoidProgress(), + $output + ); + + self::assertStringContainsString("ℹ️ Baseline file '{$this->baselineFilePath}' pruned: 1 removed, 1 kept", $output->fetch()); + + $baseline = json_decode((string) file_get_contents($this->baselineFilePath), true); + + self::assertCount(1, $baseline['violations']); + self::assertSame('App\Foo', $baseline['violations'][0]['fqcn']); + // the line number is refreshed with the current one + self::assertNotSame(99, $baseline['violations'][0]['line']); + } + + public function test_prune_never_adds_current_violations_to_an_empty_baseline(): void + { + $this->writeBaseline([]); + + $handler = new PruneBaselineHandler(new Runner(), new BaselineFileRepository()); + $output = new BufferedOutput(); + + $handler->pruneBaseline( + $this->createOptions(configFilePath: __DIR__.'/_fixtures/checkhandler/configWithViolations.php'), + new VoidProgress(), + $output + ); + + self::assertStringContainsString('pruned: 0 removed, 0 kept', $output->fetch()); + + $baseline = json_decode((string) file_get_contents($this->baselineFilePath), true); + + self::assertSame([], $baseline['violations']); + } + + public function test_prune_fails_when_the_baseline_file_does_not_exist(): void + { + $handler = new PruneBaselineHandler(new Runner(), new BaselineFileRepository()); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage("Baseline file '{$this->baselineFilePath}' not found."); + + $handler->pruneBaseline( + $this->createOptions(configFilePath: __DIR__.'/_fixtures/checkhandler/configWithViolations.php'), + new VoidProgress(), + new BufferedOutput() + ); + } + + private function writeBaseline(array $violations): void + { + file_put_contents($this->baselineFilePath, json_encode(['violations' => $violations])); + } + + private function createOptions(string $configFilePath): PruneBaselineOptions + { + return new PruneBaselineOptions( + configFilePath: $configFilePath, + targetPhpVersion: null, + autoloadFilePath: null, + ignoreBaselineLinenumbers: false, + baselineFilePath: $this->baselineFilePath, + ); + } +} diff --git a/tests/Unit/Rules/ViolationsTest.php b/tests/Unit/Rules/ViolationsTest.php index 86edded8..9baa8174 100644 --- a/tests/Unit/Rules/ViolationsTest.php +++ b/tests/Unit/Rules/ViolationsTest.php @@ -392,4 +392,51 @@ public function test_count_unmatched_in_does_not_mutate_either_set(): void self::assertCount(1, $baseline); self::assertCount(1, $current); } + + public function test_intersection_keeps_only_matching_violations_with_this_sets_line_numbers(): void + { + $current = new Violations(); + $current->add(new Violation('App\Controller\Shop', 'should have name end with Controller', 25)); + $current->add(new Violation('App\Controller\NewViolation', 'should implement ContainerInterface', 5)); + + $baseline = new Violations(); + $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->intersection($baseline); + + self::assertCount(1, $intersection); + self::assertEquals('App\Controller\Shop', $intersection->get(0)->getFqcn()); + self::assertEquals(25, $intersection->get(0)->getLine()); + } + + public function test_intersection_matches_each_entry_at_most_once(): void + { + $duplicated = new Violation('App\Controller\Shop', 'should have name end with Controller', 10); + + $current = new Violations(); + $current->add($duplicated); + $current->add($duplicated); + + $baseline = new Violations(); + $baseline->add($duplicated); + + self::assertCount(1, $current->intersection($baseline)); + } + + public function test_intersection_does_not_mutate_either_set(): void + { + $violation = new Violation('App\Controller\Shop', 'should have name end with Controller', 10); + + $current = new Violations(); + $current->add($violation); + + $baseline = new Violations(); + $baseline->add($violation); + + $current->intersection($baseline); + + self::assertCount(1, $current); + self::assertCount(1, $baseline); + } } From d68a418ec1a26562289b86d69426959ba07b598e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Jul 2026 16:52:47 +0000 Subject: [PATCH 2/8] Drop --ignore-baseline-linenumbers from prune-baseline, preserve the stored format instead MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On prune-baseline the flag did not control matching (pruning always matches ignoring line numbers — that is how it refreshes stale ones): it only stripped line numbers from the saved file. That made it a footgun for the line-numberless workflow (generate-baseline -i + check -i): forgetting it on a prune would silently convert the baseline back to a line-numbered file, reintroducing exactly the drift the user opted out of. Instead of an output-format flag, Baseline::prune() now preserves the stored format: a baseline whose entries have no line numbers stays line-numberless after pruning. No flag to remember, and pruning can never make the baseline worse. CommonOptions::addTo() loses the -i option, which moves to a separate addIgnoreBaselineLinenumbers() composed only by check and generate-baseline, where the option actually means something. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015K1TKiwtwkW7twBCTz758P --- README.md | 2 +- src/CLI/Baseline.php | 20 +++++++++++++++++++- src/CLI/Command/Check.php | 1 + src/CLI/Command/CommonOptions.php | 21 +++++++++++++++------ src/CLI/Command/GenerateBaseline.php | 1 + src/CLI/Command/PruneBaseline.php | 1 - src/CLI/PruneBaselineHandler.php | 4 ---- src/CLI/PruneBaselineOptions.php | 6 ------ tests/Unit/CLI/BaselineTest.php | 14 ++++++++++++++ tests/Unit/CLI/PruneBaselineHandlerTest.php | 1 - 10 files changed, 51 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index db1ef092..55a86a1c 100644 --- a/README.md +++ b/README.md @@ -167,7 +167,7 @@ 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. -Both `generate-baseline` and `prune-baseline` accept an optional custom file name as argument and the same `--config`, `--target-php-version`, `--autoload` and `--ignore-baseline-linenumbers` options as `check`. +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. > **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. diff --git a/src/CLI/Baseline.php b/src/CLI/Baseline.php index 7861ea2a..2e8a787c 100644 --- a/src/CLI/Baseline.php +++ b/src/CLI/Baseline.php @@ -63,11 +63,29 @@ public function getStaleViolationsCount(): int */ public function prune(Violations $currentViolations): self { - return new self($currentViolations->intersection($this->violations)); + $prunedViolations = $currentViolations->intersection($this->violations); + + // a baseline stored without line numbers keeps its format + if (!$this->hasLineNumbers()) { + $prunedViolations = $prunedViolations->withoutLineNumbers(); + } + + return new self($prunedViolations); } public function withoutLineNumbers(): self { return new self($this->violations->withoutLineNumbers()); } + + private function hasLineNumbers(): bool + { + foreach ($this->violations as $violation) { + if (null !== $violation->getLine()) { + return true; + } + } + + return false; + } } diff --git a/src/CLI/Command/Check.php b/src/CLI/Command/Check.php index b1490476..d06fdc45 100644 --- a/src/CLI/Command/Check.php +++ b/src/CLI/Command/Check.php @@ -80,6 +80,7 @@ protected function configure(): void ); $this->commonOptions->addTo($this); + $this->commonOptions->addIgnoreBaselineLinenumbers($this); } protected function isRunningAsPhar(): bool diff --git a/src/CLI/Command/CommonOptions.php b/src/CLI/Command/CommonOptions.php index 320d4159..654f63fb 100644 --- a/src/CLI/Command/CommonOptions.php +++ b/src/CLI/Command/CommonOptions.php @@ -46,15 +46,24 @@ public function addTo(Command $command): void 'a', InputOption::VALUE_REQUIRED, 'Specify an autoload file to use', - ) - ->addOption( - self::IGNORE_BASELINE_LINENUMBERS_PARAM, - 'i', - InputOption::VALUE_NONE, - 'Ignore line numbers when checking or generating the baseline' ); } + /** + * 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. + */ + public function addIgnoreBaselineLinenumbers(Command $command): void + { + $command->addOption( + self::IGNORE_BASELINE_LINENUMBERS_PARAM, + 'i', + InputOption::VALUE_NONE, + 'Ignore line numbers when checking or generating the baseline' + ); + } + public function configFilePath(InputInterface $input): string { return (string) $input->getOption(self::CONFIG_PARAM); diff --git a/src/CLI/Command/GenerateBaseline.php b/src/CLI/Command/GenerateBaseline.php index bdfab54c..0869d3ef 100644 --- a/src/CLI/Command/GenerateBaseline.php +++ b/src/CLI/Command/GenerateBaseline.php @@ -49,6 +49,7 @@ protected function configure(): void ); $this->commonOptions->addTo($this); + $this->commonOptions->addIgnoreBaselineLinenumbers($this); } protected function isRunningAsPhar(): bool diff --git a/src/CLI/Command/PruneBaseline.php b/src/CLI/Command/PruneBaseline.php index a0516183..4787f0be 100644 --- a/src/CLI/Command/PruneBaseline.php +++ b/src/CLI/Command/PruneBaseline.php @@ -97,7 +97,6 @@ protected function parseOptions(InputInterface $input): PruneBaselineOptions configFilePath: $this->commonOptions->configFilePath($input), targetPhpVersion: $this->commonOptions->targetPhpVersion($input), autoloadFilePath: $this->commonOptions->autoloadFilePath($input), - ignoreBaselineLinenumbers: $this->commonOptions->isIgnoreBaselineLinenumbers($input), baselineFilePath: (string) $input->getArgument(self::FILENAME_ARG), ); } diff --git a/src/CLI/PruneBaselineHandler.php b/src/CLI/PruneBaselineHandler.php index 2eb15bec..76eb2002 100644 --- a/src/CLI/PruneBaselineHandler.php +++ b/src/CLI/PruneBaselineHandler.php @@ -39,10 +39,6 @@ public function pruneBaseline(PruneBaselineOptions $options, Progress $progress, $prunedBaseline = $baseline->prune($result->getViolations()); - if ($options->isIgnoreBaselineLinenumbers()) { - $prunedBaseline = $prunedBaseline->withoutLineNumbers(); - } - $this->baselineRepository->save($prunedBaseline, $options->getBaselineFilePath()); $keptCount = $prunedBaseline->getViolations()->count(); diff --git a/src/CLI/PruneBaselineOptions.php b/src/CLI/PruneBaselineOptions.php index a6fa9946..3f162dbf 100644 --- a/src/CLI/PruneBaselineOptions.php +++ b/src/CLI/PruneBaselineOptions.php @@ -14,7 +14,6 @@ public function __construct( private string $configFilePath, private ?string $targetPhpVersion, private ?string $autoloadFilePath, - private bool $ignoreBaselineLinenumbers, private string $baselineFilePath, ) { } @@ -34,11 +33,6 @@ public function getAutoloadFilePath(): ?string return $this->autoloadFilePath; } - public function isIgnoreBaselineLinenumbers(): bool - { - return $this->ignoreBaselineLinenumbers; - } - public function getBaselineFilePath(): string { return $this->baselineFilePath; diff --git a/tests/Unit/CLI/BaselineTest.php b/tests/Unit/CLI/BaselineTest.php index bde05c5f..b5062be4 100644 --- a/tests/Unit/CLI/BaselineTest.php +++ b/tests/Unit/CLI/BaselineTest.php @@ -75,6 +75,20 @@ public function test_prune_refreshes_stale_line_numbers(): void self::assertEquals(42, $pruned->getViolations()->get(0)->getLine()); } + public function test_prune_preserves_a_line_numberless_baseline_format(): void + { + $baselineViolations = new Violations(); + $baselineViolations->add(new Violation('App\Controller\Shop', 'should have name end with Controller', null)); + + $current = new Violations(); + $current->add(new Violation('App\Controller\Shop', 'should have name end with Controller', 42)); + + $pruned = Baseline::fromViolations($baselineViolations)->prune($current); + + self::assertCount(1, $pruned->getViolations()); + self::assertNull($pruned->getViolations()->get(0)->getLine()); + } + public function test_without_line_numbers_returns_a_copy_with_stripped_line_numbers(): void { $baselineViolations = new Violations(); diff --git a/tests/Unit/CLI/PruneBaselineHandlerTest.php b/tests/Unit/CLI/PruneBaselineHandlerTest.php index ffe918d1..fca5a105 100644 --- a/tests/Unit/CLI/PruneBaselineHandlerTest.php +++ b/tests/Unit/CLI/PruneBaselineHandlerTest.php @@ -96,7 +96,6 @@ private function createOptions(string $configFilePath): PruneBaselineOptions configFilePath: $configFilePath, targetPhpVersion: null, autoloadFilePath: null, - ignoreBaselineLinenumbers: false, baselineFilePath: $this->baselineFilePath, ); } From 85c8e81507b8e286fa223e02adaf1a3032222202 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Jul 2026 17:10:55 +0000 Subject: [PATCH 3/8] Write generate-baseline and prune-baseline output to stdout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stderr redirect exists in check because stdout is reserved for the violations payload (json/gitlab formats piped to files). The baseline commands have no stdout payload — their console output is all diagnostics — so redirecting it to stderr was cargo cult that made 'phparkitect generate-baseline > log.txt' capture nothing. They now write to the output they receive, like any Symfony command; progress bars still go to stderr via Symfony's own ConsoleOutputInterface handling. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015K1TKiwtwkW7twBCTz758P --- src/CLI/Command/GenerateBaseline.php | 4 ---- src/CLI/Command/PruneBaseline.php | 4 ---- tests/E2E/Cli/GenerateBaselineCommandTest.php | 6 +++--- tests/E2E/Cli/PruneBaselineCommandTest.php | 6 +++--- 4 files changed, 6 insertions(+), 14 deletions(-) diff --git a/src/CLI/Command/GenerateBaseline.php b/src/CLI/Command/GenerateBaseline.php index 0869d3ef..82ea91aa 100644 --- a/src/CLI/Command/GenerateBaseline.php +++ b/src/CLI/Command/GenerateBaseline.php @@ -11,7 +11,6 @@ use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Output\ConsoleOutputInterface; use Symfony\Component\Console\Output\OutputInterface; class GenerateBaseline extends Command @@ -62,9 +61,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int $this->runtime->raiseLimits(); $startTime = microtime(true); - // we write everything on STDERR to be consistent with the check command - $output = $output instanceof ConsoleOutputInterface ? $output->getErrorOutput() : $output; - try { $verbose = (bool) $input->getOption('verbose'); $options = $this->parseOptions($input); diff --git a/src/CLI/Command/PruneBaseline.php b/src/CLI/Command/PruneBaseline.php index 4787f0be..29a2ef50 100644 --- a/src/CLI/Command/PruneBaseline.php +++ b/src/CLI/Command/PruneBaseline.php @@ -11,7 +11,6 @@ use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Output\ConsoleOutputInterface; use Symfony\Component\Console\Output\OutputInterface; class PruneBaseline extends Command @@ -61,9 +60,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int $this->runtime->raiseLimits(); $startTime = microtime(true); - // we write everything on STDERR to be consistent with the check command - $output = $output instanceof ConsoleOutputInterface ? $output->getErrorOutput() : $output; - try { $verbose = (bool) $input->getOption('verbose'); $options = $this->parseOptions($input); diff --git a/tests/E2E/Cli/GenerateBaselineCommandTest.php b/tests/E2E/Cli/GenerateBaselineCommandTest.php index d47da99d..545fe483 100644 --- a/tests/E2E/Cli/GenerateBaselineCommandTest.php +++ b/tests/E2E/Cli/GenerateBaselineCommandTest.php @@ -33,7 +33,7 @@ public function test_creates_the_baseline_with_the_default_filename(): void $cmdTester = $this->runGenerateBaseline(__DIR__.'/../_fixtures/configMvcForYieldBug.php'); self::assertEquals(self::SUCCESS_CODE, $cmdTester->getStatusCode()); - self::assertStringContainsString("ℹ️ Baseline file '{$this->defaultBaselineFilename}' created!", $cmdTester->getErrorOutput()); + self::assertStringContainsString("ℹ️ Baseline file '{$this->defaultBaselineFilename}' created!", $cmdTester->getDisplay()); self::assertFileExists($this->defaultBaselineFilename); $baseline = json_decode((string) file_get_contents($this->defaultBaselineFilename), true); @@ -50,7 +50,7 @@ public function test_creates_the_baseline_with_a_custom_filename(): void ); self::assertEquals(self::SUCCESS_CODE, $cmdTester->getStatusCode()); - self::assertStringContainsString("ℹ️ Baseline file '{$this->customBaselineFilename}' created!", $cmdTester->getErrorOutput()); + self::assertStringContainsString("ℹ️ Baseline file '{$this->customBaselineFilename}' created!", $cmdTester->getDisplay()); self::assertFileExists($this->customBaselineFilename); self::assertFileDoesNotExist($this->defaultBaselineFilename); } @@ -76,7 +76,7 @@ public function test_fails_gracefully_when_the_config_file_does_not_exist(): voi $cmdTester = $this->runGenerateBaseline('not-a-real-config.php'); self::assertEquals(self::ERROR_CODE, $cmdTester->getStatusCode()); - self::assertStringContainsString('not found', $cmdTester->getErrorOutput()); + self::assertStringContainsString('not found', $cmdTester->getDisplay()); self::assertFileDoesNotExist($this->defaultBaselineFilename); } diff --git a/tests/E2E/Cli/PruneBaselineCommandTest.php b/tests/E2E/Cli/PruneBaselineCommandTest.php index 7601f380..b71d3a1b 100644 --- a/tests/E2E/Cli/PruneBaselineCommandTest.php +++ b/tests/E2E/Cli/PruneBaselineCommandTest.php @@ -43,7 +43,7 @@ public function test_prunes_fixed_violations_and_keeps_the_still_existing_ones() $cmdTester = $this->runCommand(['prune-baseline', '--config' => $configFilePath, 'filename' => $this->customBaselineFilename]); self::assertEquals(self::SUCCESS_CODE, $cmdTester->getStatusCode()); - self::assertStringContainsString("ℹ️ Baseline file '{$this->customBaselineFilename}' pruned: 1 removed, 1 kept", $cmdTester->getErrorOutput()); + self::assertStringContainsString("ℹ️ Baseline file '{$this->customBaselineFilename}' pruned: 1 removed, 1 kept", $cmdTester->getDisplay()); $pruned = json_decode((string) file_get_contents($this->customBaselineFilename), true); @@ -66,7 +66,7 @@ public function test_pruning_an_up_to_date_baseline_changes_nothing(): void $cmdTester = $this->runCommand(['prune-baseline', '--config' => $configFilePath, 'filename' => $this->customBaselineFilename]); self::assertEquals(self::SUCCESS_CODE, $cmdTester->getStatusCode()); - self::assertStringContainsString('pruned: 0 removed, 1 kept', $cmdTester->getErrorOutput()); + self::assertStringContainsString('pruned: 0 removed, 1 kept', $cmdTester->getDisplay()); } public function test_fails_gracefully_when_the_baseline_file_does_not_exist(): void @@ -76,7 +76,7 @@ public function test_fails_gracefully_when_the_baseline_file_does_not_exist(): v $cmdTester = $this->runCommand(['prune-baseline', '--config' => $configFilePath, 'filename' => $this->customBaselineFilename]); self::assertEquals(self::ERROR_CODE, $cmdTester->getStatusCode()); - self::assertStringContainsString('not found', $cmdTester->getErrorOutput()); + self::assertStringContainsString('not found', $cmdTester->getDisplay()); } protected function runCommand(array $input): ApplicationTester From 6dcd5617e36b9a5f79de4b1e02997e2a7a4c6224 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Jul 2026 17:24:03 +0000 Subject: [PATCH 4/8] Rename BaselineFileRepository::resolveFilePath to findDefaultFilePath MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old name hid both decisions the method took: 'resolve' did not say it probes the filesystem, and the nullable-in/nullable-out signature mixed two different concerns — the explicit-path-wins precedence (command policy) and the default-file auto-detection (persistence knowledge). The precedence now reads inline in Check::parseOptions, and the repository keeps only its own piece with a name that says what it does: findDefaultFilePath() returns the default baseline file or null when none exists. The RiskyTruthyFalsyComparison suppression dies with the falsy-parameter check. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015K1TKiwtwkW7twBCTz758P --- src/CLI/BaselineFileRepository.php | 11 +++---- src/CLI/Command/Check.php | 2 +- tests/Unit/CLI/BaselineFileRepositoryTest.php | 33 ++++++++++++++----- 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/src/CLI/BaselineFileRepository.php b/src/CLI/BaselineFileRepository.php index 82b0ac0a..5a73a5ec 100644 --- a/src/CLI/BaselineFileRepository.php +++ b/src/CLI/BaselineFileRepository.php @@ -29,14 +29,11 @@ public function save(Baseline $baseline, string $filename): void } /** - * @psalm-suppress RiskyTruthyFalsyComparison + * The default baseline file, or null when none exists — this is what + * lets `check` pick up a generated baseline automatically. */ - public static function resolveFilePath(?string $filePath): ?string + public static function findDefaultFilePath(): ?string { - if (!$filePath && file_exists(self::DEFAULT_FILENAME)) { - $filePath = self::DEFAULT_FILENAME; - } - - return $filePath ?: null; + return file_exists(self::DEFAULT_FILENAME) ? self::DEFAULT_FILENAME : null; } } diff --git a/src/CLI/Command/Check.php b/src/CLI/Command/Check.php index d06fdc45..ee184c5e 100644 --- a/src/CLI/Command/Check.php +++ b/src/CLI/Command/Check.php @@ -145,7 +145,7 @@ protected function parseOptions(InputInterface $input): CheckOptions configFilePath: $this->commonOptions->configFilePath($input), targetPhpVersion: $this->commonOptions->targetPhpVersion($input), stopOnFailure: (bool) $input->getOption(self::STOP_ON_FAILURE_PARAM), - baselineFilePath: BaselineFileRepository::resolveFilePath($useBaseline), + baselineFilePath: '' !== $useBaseline ? $useBaseline : BaselineFileRepository::findDefaultFilePath(), skipBaseline: (bool) $input->getOption(self::SKIP_BASELINE_PARAM), ignoreBaselineLinenumbers: $this->commonOptions->isIgnoreBaselineLinenumbers($input), format: (string) $input->getOption(self::FORMAT_PARAM), diff --git a/tests/Unit/CLI/BaselineFileRepositoryTest.php b/tests/Unit/CLI/BaselineFileRepositoryTest.php index 3ce9d30f..0b390866 100644 --- a/tests/Unit/CLI/BaselineFileRepositoryTest.php +++ b/tests/Unit/CLI/BaselineFileRepositoryTest.php @@ -42,23 +42,40 @@ public function test_load_throws_when_the_file_does_not_exist(): void (new BaselineFileRepository())->load('not-a-real-file.json'); } - public function test_resolve_file_path_prefers_the_explicit_path(): void + public function test_find_default_file_path_is_null_when_no_default_baseline_exists(): void { - self::assertSame('custom.json', BaselineFileRepository::resolveFilePath('custom.json')); + $this->inEmptyDirectory(static function (): void { + self::assertNull(BaselineFileRepository::findDefaultFilePath()); + }); } - public function test_resolve_file_path_is_null_when_nothing_is_given_and_no_default_exists(): void + public function test_find_default_file_path_returns_the_default_baseline_when_it_exists(): void + { + $this->inEmptyDirectory(static function (): void { + file_put_contents(BaselineFileRepository::DEFAULT_FILENAME, '{"violations": []}'); + + self::assertSame(BaselineFileRepository::DEFAULT_FILENAME, BaselineFileRepository::findDefaultFilePath()); + + unlink(BaselineFileRepository::DEFAULT_FILENAME); + }); + } + + /** + * The default baseline is looked up in the current working directory, + * so run the callback from a fresh empty one to make tests deterministic. + */ + private function inEmptyDirectory(callable $test): void { - // the default baseline is resolved against the current working - // directory, so move to an empty one to make the test deterministic $cwd = (string) getcwd(); - chdir(sys_get_temp_dir()); + $directory = sys_get_temp_dir().'/arkitect-baseline-test-'.uniqid(); + mkdir($directory); + chdir($directory); try { - self::assertNull(BaselineFileRepository::resolveFilePath('')); - self::assertNull(BaselineFileRepository::resolveFilePath(null)); + $test(); } finally { chdir($cwd); + rmdir($directory); } } } From 5a653050cee932e255c585f741531fc7fe12e3e5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Jul 2026 18:52:44 +0000 Subject: [PATCH 5/8] Make the default-baseline lookup a predicate: hasDefaultBaseline() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit findDefaultFilePath() returned ?string but carried a single bit: the non-null value was always DEFAULT_FILENAME, never a different path. So it was a yes/no question dressed as a finder. It becomes hasDefaultBaseline(): bool, a pure persistence predicate, and Check maps 'default exists' to the DEFAULT_FILENAME path — keeping the path-selection policy in the command layer, consistent with where the explicit-wins precedence already lives. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015K1TKiwtwkW7twBCTz758P --- src/CLI/BaselineFileRepository.php | 8 ++++---- src/CLI/Command/Check.php | 3 ++- tests/Unit/CLI/BaselineFileRepositoryTest.php | 8 ++++---- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/CLI/BaselineFileRepository.php b/src/CLI/BaselineFileRepository.php index 5a73a5ec..1e3991f6 100644 --- a/src/CLI/BaselineFileRepository.php +++ b/src/CLI/BaselineFileRepository.php @@ -29,11 +29,11 @@ public function save(Baseline $baseline, string $filename): void } /** - * The default baseline file, or null when none exists — this is what - * lets `check` pick up a generated baseline automatically. + * Whether the default baseline file exists — this is what lets `check` + * pick up a generated baseline automatically. */ - public static function findDefaultFilePath(): ?string + public static function hasDefaultBaseline(): bool { - return file_exists(self::DEFAULT_FILENAME) ? self::DEFAULT_FILENAME : null; + return file_exists(self::DEFAULT_FILENAME); } } diff --git a/src/CLI/Command/Check.php b/src/CLI/Command/Check.php index ee184c5e..c99b9a2b 100644 --- a/src/CLI/Command/Check.php +++ b/src/CLI/Command/Check.php @@ -140,12 +140,13 @@ protected function execute(InputInterface $input, OutputInterface $output): int protected function parseOptions(InputInterface $input): CheckOptions { $useBaseline = (string) $input->getOption(self::USE_BASELINE_PARAM); + $defaultBaselineFilePath = BaselineFileRepository::hasDefaultBaseline() ? BaselineFileRepository::DEFAULT_FILENAME : null; return new CheckOptions( configFilePath: $this->commonOptions->configFilePath($input), targetPhpVersion: $this->commonOptions->targetPhpVersion($input), stopOnFailure: (bool) $input->getOption(self::STOP_ON_FAILURE_PARAM), - baselineFilePath: '' !== $useBaseline ? $useBaseline : BaselineFileRepository::findDefaultFilePath(), + baselineFilePath: '' !== $useBaseline ? $useBaseline : $defaultBaselineFilePath, skipBaseline: (bool) $input->getOption(self::SKIP_BASELINE_PARAM), ignoreBaselineLinenumbers: $this->commonOptions->isIgnoreBaselineLinenumbers($input), format: (string) $input->getOption(self::FORMAT_PARAM), diff --git a/tests/Unit/CLI/BaselineFileRepositoryTest.php b/tests/Unit/CLI/BaselineFileRepositoryTest.php index 0b390866..06182373 100644 --- a/tests/Unit/CLI/BaselineFileRepositoryTest.php +++ b/tests/Unit/CLI/BaselineFileRepositoryTest.php @@ -42,19 +42,19 @@ public function test_load_throws_when_the_file_does_not_exist(): void (new BaselineFileRepository())->load('not-a-real-file.json'); } - public function test_find_default_file_path_is_null_when_no_default_baseline_exists(): void + public function test_has_default_baseline_is_false_when_no_default_baseline_exists(): void { $this->inEmptyDirectory(static function (): void { - self::assertNull(BaselineFileRepository::findDefaultFilePath()); + self::assertFalse(BaselineFileRepository::hasDefaultBaseline()); }); } - public function test_find_default_file_path_returns_the_default_baseline_when_it_exists(): void + public function test_has_default_baseline_is_true_when_the_default_baseline_exists(): void { $this->inEmptyDirectory(static function (): void { file_put_contents(BaselineFileRepository::DEFAULT_FILENAME, '{"violations": []}'); - self::assertSame(BaselineFileRepository::DEFAULT_FILENAME, BaselineFileRepository::findDefaultFilePath()); + self::assertTrue(BaselineFileRepository::hasDefaultBaseline()); unlink(BaselineFileRepository::DEFAULT_FILENAME); }); From 0427d10369f3798585ba88aabf2b3ff3b1138bd6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Jul 2026 19:04:35 +0000 Subject: [PATCH 6/8] Resolve the check baseline choice once, into a single path-or-empty value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 'Which baseline to subtract' was represented twice — as ?string baselineFilePath and bool skipBaseline — and resolved in two layers: parseOptions handled --use-baseline and the default file, while CheckHandler handled skip (skip ? null : path). skip is just another route to 'empty baseline', i.e. baselineFilePath = null, so the two fields encoded the same decision. The decision now lives in one place, Check::resolveBaselineFilePath(), which reads top-to-bottom as the precedence it implements: --skip-baseline forces empty; an explicit --use-baseline wins over the auto-detected default; a missing default just means empty. Its output is the single ?string baselineFilePath, where null means 'empty baseline'. CheckOptions loses the redundant skipBaseline field, and CheckHandler drops the baselineFilePath/skipBaseline pass-through it wrote into Config and immediately read back (nothing else reads them from Config) — it now loads-or-empties straight from the resolved value. No behavior change: the CLI still resolves exactly as before. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015K1TKiwtwkW7twBCTz758P --- src/CLI/CheckHandler.php | 6 +++--- src/CLI/CheckOptions.php | 6 ------ src/CLI/Command/Check.php | 26 +++++++++++++++++++++----- tests/Unit/CLI/CheckHandlerTest.php | 1 - 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/src/CLI/CheckHandler.php b/src/CLI/CheckHandler.php index aa4acc11..7480b10d 100644 --- a/src/CLI/CheckHandler.php +++ b/src/CLI/CheckHandler.php @@ -32,12 +32,12 @@ public function check( ->autoloadFilePath($options->getAutoloadFilePath()) ->stopOnFailure($options->isStopOnFailure()) ->targetPhpVersion(TargetPhpVersion::create($options->getTargetPhpVersion())) - ->baselineFilePath($options->getBaselineFilePath()) ->ignoreBaselineLinenumbers($options->isIgnoreBaselineLinenumbers()) - ->skipBaseline($options->isSkipBaseline()) ->format($options->getFormat()); - $baselineFilePath = $config->isSkipBaseline() ? null : $config->getBaselineFilePath(); + // null baselineFilePath means: use an empty baseline (no --use-baseline, + // no default file on disk, or --skip-baseline) + $baselineFilePath = $options->getBaselineFilePath(); $baseline = null === $baselineFilePath ? Baseline::empty() : $this->baselineRepository->load($baselineFilePath); null !== $baselineFilePath && $output->writeln("Baseline file '$baselineFilePath' found"); diff --git a/src/CLI/CheckOptions.php b/src/CLI/CheckOptions.php index b351c55c..39f81cc9 100644 --- a/src/CLI/CheckOptions.php +++ b/src/CLI/CheckOptions.php @@ -15,7 +15,6 @@ public function __construct( private ?string $targetPhpVersion, private bool $stopOnFailure, private ?string $baselineFilePath, - private bool $skipBaseline, private bool $ignoreBaselineLinenumbers, private string $format, private ?string $autoloadFilePath, @@ -42,11 +41,6 @@ public function getBaselineFilePath(): ?string return $this->baselineFilePath; } - public function isSkipBaseline(): bool - { - return $this->skipBaseline; - } - public function isIgnoreBaselineLinenumbers(): bool { return $this->ignoreBaselineLinenumbers; diff --git a/src/CLI/Command/Check.php b/src/CLI/Command/Check.php index c99b9a2b..0c95b82a 100644 --- a/src/CLI/Command/Check.php +++ b/src/CLI/Command/Check.php @@ -139,18 +139,34 @@ protected function execute(InputInterface $input, OutputInterface $output): int protected function parseOptions(InputInterface $input): CheckOptions { - $useBaseline = (string) $input->getOption(self::USE_BASELINE_PARAM); - $defaultBaselineFilePath = BaselineFileRepository::hasDefaultBaseline() ? BaselineFileRepository::DEFAULT_FILENAME : null; - return new CheckOptions( configFilePath: $this->commonOptions->configFilePath($input), targetPhpVersion: $this->commonOptions->targetPhpVersion($input), stopOnFailure: (bool) $input->getOption(self::STOP_ON_FAILURE_PARAM), - baselineFilePath: '' !== $useBaseline ? $useBaseline : $defaultBaselineFilePath, - skipBaseline: (bool) $input->getOption(self::SKIP_BASELINE_PARAM), + baselineFilePath: $this->resolveBaselineFilePath($input), ignoreBaselineLinenumbers: $this->commonOptions->isIgnoreBaselineLinenumbers($input), format: (string) $input->getOption(self::FORMAT_PARAM), autoloadFilePath: $this->commonOptions->autoloadFilePath($input), ); } + + /** + * The single "which baseline" decision: the file path to load, or null + * for an empty baseline. --skip-baseline forces empty; an explicit + * --use-baseline wins over the auto-detected default baseline, whose + * absence just means "empty". + */ + private function resolveBaselineFilePath(InputInterface $input): ?string + { + if ((bool) $input->getOption(self::SKIP_BASELINE_PARAM)) { + return null; + } + + $useBaseline = (string) $input->getOption(self::USE_BASELINE_PARAM); + if ('' !== $useBaseline) { + return $useBaseline; + } + + return BaselineFileRepository::hasDefaultBaseline() ? BaselineFileRepository::DEFAULT_FILENAME : null; + } } diff --git a/tests/Unit/CLI/CheckHandlerTest.php b/tests/Unit/CLI/CheckHandlerTest.php index cf318ad6..de3902e4 100644 --- a/tests/Unit/CLI/CheckHandlerTest.php +++ b/tests/Unit/CLI/CheckHandlerTest.php @@ -102,7 +102,6 @@ private function createOptions( targetPhpVersion: null, stopOnFailure: false, baselineFilePath: $baselineFilePath, - skipBaseline: false, ignoreBaselineLinenumbers: false, format: 'text', autoloadFilePath: null, From b9142ccbec85f5022d5b29a09f120fdbde7fdaf3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 08:49:22 +0000 Subject: [PATCH 7/8] Stop probing the filesystem to resolve the baseline path for check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit baselineFilePath is a field — a value — but resolveBaselineFilePath was stuffing filesystem logic into it: 'the default path, but only if the file exists'. That is why hasDefaultBaseline() had to probe disk at parse time; a path field secretly carried an existence fact. Now the field holds a plain path: --use-baseline, or the default phparkitect-baseline.json (null only for --skip-baseline). Existence is decided where loading happens: CheckHandler loads the baseline if the file is there and falls back to an empty one otherwise — the baseline is optional for check, so an absent file just means nothing to ignore. hasDefaultBaseline() is replaced by a general BaselineFileRepository:: exists(), used by the handler for both the fallback and the 'found' message. Behavior change: check --use-baseline=missing.json no longer errors, it runs with an empty baseline — consistent with a missing default, and it fails in the safe direction (all violations shown). prune-baseline keeps its strict load: there a missing baseline is still an error. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015K1TKiwtwkW7twBCTz758P --- src/CLI/BaselineFileRepository.php | 8 ++--- src/CLI/CheckHandler.php | 11 +++--- src/CLI/Command/Check.php | 13 +++---- tests/Unit/CLI/BaselineFileRepositoryTest.php | 36 ++++--------------- 4 files changed, 20 insertions(+), 48 deletions(-) diff --git a/src/CLI/BaselineFileRepository.php b/src/CLI/BaselineFileRepository.php index 1e3991f6..5120e28f 100644 --- a/src/CLI/BaselineFileRepository.php +++ b/src/CLI/BaselineFileRepository.php @@ -28,12 +28,8 @@ public function save(Baseline $baseline, string $filename): void file_put_contents($filename, json_encode($baseline->getViolations(), \JSON_PRETTY_PRINT)); } - /** - * Whether the default baseline file exists — this is what lets `check` - * pick up a generated baseline automatically. - */ - public static function hasDefaultBaseline(): bool + public function exists(string $filename): bool { - return file_exists(self::DEFAULT_FILENAME); + return file_exists($filename); } } diff --git a/src/CLI/CheckHandler.php b/src/CLI/CheckHandler.php index 7480b10d..04e5da79 100644 --- a/src/CLI/CheckHandler.php +++ b/src/CLI/CheckHandler.php @@ -35,12 +35,15 @@ public function check( ->ignoreBaselineLinenumbers($options->isIgnoreBaselineLinenumbers()) ->format($options->getFormat()); - // null baselineFilePath means: use an empty baseline (no --use-baseline, - // no default file on disk, or --skip-baseline) + // the baseline is optional for check: an absent file just means there + // is nothing to ignore, so we fall back to an empty baseline $baselineFilePath = $options->getBaselineFilePath(); - $baseline = null === $baselineFilePath ? Baseline::empty() : $this->baselineRepository->load($baselineFilePath); + $baseline = Baseline::empty(); - null !== $baselineFilePath && $output->writeln("Baseline file '$baselineFilePath' found"); + if (null !== $baselineFilePath && $this->baselineRepository->exists($baselineFilePath)) { + $baseline = $this->baselineRepository->load($baselineFilePath); + $output->writeln("Baseline file '$baselineFilePath' found"); + } $output->writeln("Config file '{$options->getConfigFilePath()}' found\n"); diff --git a/src/CLI/Command/Check.php b/src/CLI/Command/Check.php index 0c95b82a..f199d94b 100644 --- a/src/CLI/Command/Check.php +++ b/src/CLI/Command/Check.php @@ -151,10 +151,10 @@ protected function parseOptions(InputInterface $input): CheckOptions } /** - * The single "which baseline" decision: the file path to load, or null - * for an empty baseline. --skip-baseline forces empty; an explicit - * --use-baseline wins over the auto-detected default baseline, whose - * absence just means "empty". + * The baseline file check should use: an explicit --use-baseline, or the + * default phparkitect-baseline.json. Null when --skip-baseline opts out. + * Whether that file exists is decided at load time — the baseline is + * optional for check, so an absent one just means nothing to ignore. */ private function resolveBaselineFilePath(InputInterface $input): ?string { @@ -163,10 +163,7 @@ private function resolveBaselineFilePath(InputInterface $input): ?string } $useBaseline = (string) $input->getOption(self::USE_BASELINE_PARAM); - if ('' !== $useBaseline) { - return $useBaseline; - } - return BaselineFileRepository::hasDefaultBaseline() ? BaselineFileRepository::DEFAULT_FILENAME : null; + return '' !== $useBaseline ? $useBaseline : BaselineFileRepository::DEFAULT_FILENAME; } } diff --git a/tests/Unit/CLI/BaselineFileRepositoryTest.php b/tests/Unit/CLI/BaselineFileRepositoryTest.php index 06182373..320cae4c 100644 --- a/tests/Unit/CLI/BaselineFileRepositoryTest.php +++ b/tests/Unit/CLI/BaselineFileRepositoryTest.php @@ -42,40 +42,16 @@ public function test_load_throws_when_the_file_does_not_exist(): void (new BaselineFileRepository())->load('not-a-real-file.json'); } - public function test_has_default_baseline_is_false_when_no_default_baseline_exists(): void + public function test_exists_is_false_when_the_file_is_absent(): void { - $this->inEmptyDirectory(static function (): void { - self::assertFalse(BaselineFileRepository::hasDefaultBaseline()); - }); + self::assertFalse((new BaselineFileRepository())->exists($this->baselineFilePath)); } - public function test_has_default_baseline_is_true_when_the_default_baseline_exists(): void + public function test_exists_is_true_when_the_file_is_present(): void { - $this->inEmptyDirectory(static function (): void { - file_put_contents(BaselineFileRepository::DEFAULT_FILENAME, '{"violations": []}'); - - self::assertTrue(BaselineFileRepository::hasDefaultBaseline()); - - unlink(BaselineFileRepository::DEFAULT_FILENAME); - }); - } - - /** - * The default baseline is looked up in the current working directory, - * so run the callback from a fresh empty one to make tests deterministic. - */ - private function inEmptyDirectory(callable $test): void - { - $cwd = (string) getcwd(); - $directory = sys_get_temp_dir().'/arkitect-baseline-test-'.uniqid(); - mkdir($directory); - chdir($directory); + $repository = new BaselineFileRepository(); + $repository->save(Baseline::empty(), $this->baselineFilePath); - try { - $test(); - } finally { - chdir($cwd); - rmdir($directory); - } + self::assertTrue($repository->exists($this->baselineFilePath)); } } From 6e2adf1b7242844a5a5355fde6b68400cfd454fa Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 08:57:52 +0000 Subject: [PATCH 8/8] Replace repository exists() with loadIfPresent(): ?Baseline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit exists() was a thin file_exists() wrapper with a single caller, which then did check-then-load: exists() followed by load(). Fold both into one repository capability — loadIfPresent() returns the baseline, or null when the file is absent — so CheckHandler makes one call, there is no TOCTOU gap, and the method name states the optional-load intent. load() stays strict for prune-baseline; loadIfPresent() is the optional variant for check, where a missing baseline just means nothing to ignore. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015K1TKiwtwkW7twBCTz758P --- src/CLI/BaselineFileRepository.php | 9 +++++++-- src/CLI/CheckHandler.php | 7 ++++--- tests/Unit/CLI/BaselineFileRepositoryTest.php | 16 +++++++++++----- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/CLI/BaselineFileRepository.php b/src/CLI/BaselineFileRepository.php index 5120e28f..ba370d68 100644 --- a/src/CLI/BaselineFileRepository.php +++ b/src/CLI/BaselineFileRepository.php @@ -28,8 +28,13 @@ public function save(Baseline $baseline, string $filename): void file_put_contents($filename, json_encode($baseline->getViolations(), \JSON_PRETTY_PRINT)); } - public function exists(string $filename): bool + /** + * The baseline at $filename, or null when the file is absent — for + * callers (like check) where the baseline is optional and a missing + * one just means "nothing to ignore". + */ + public function loadIfPresent(string $filename): ?Baseline { - return file_exists($filename); + return file_exists($filename) ? $this->load($filename) : null; } } diff --git a/src/CLI/CheckHandler.php b/src/CLI/CheckHandler.php index 04e5da79..80b62ac5 100644 --- a/src/CLI/CheckHandler.php +++ b/src/CLI/CheckHandler.php @@ -38,10 +38,11 @@ public function check( // the baseline is optional for check: an absent file just means there // is nothing to ignore, so we fall back to an empty baseline $baselineFilePath = $options->getBaselineFilePath(); - $baseline = Baseline::empty(); + $baseline = null === $baselineFilePath ? null : $this->baselineRepository->loadIfPresent($baselineFilePath); - if (null !== $baselineFilePath && $this->baselineRepository->exists($baselineFilePath)) { - $baseline = $this->baselineRepository->load($baselineFilePath); + if (null === $baseline) { + $baseline = Baseline::empty(); + } else { $output->writeln("Baseline file '$baselineFilePath' found"); } diff --git a/tests/Unit/CLI/BaselineFileRepositoryTest.php b/tests/Unit/CLI/BaselineFileRepositoryTest.php index 320cae4c..b2a1749f 100644 --- a/tests/Unit/CLI/BaselineFileRepositoryTest.php +++ b/tests/Unit/CLI/BaselineFileRepositoryTest.php @@ -42,16 +42,22 @@ public function test_load_throws_when_the_file_does_not_exist(): void (new BaselineFileRepository())->load('not-a-real-file.json'); } - public function test_exists_is_false_when_the_file_is_absent(): void + public function test_load_if_present_is_null_when_the_file_is_absent(): void { - self::assertFalse((new BaselineFileRepository())->exists($this->baselineFilePath)); + self::assertNull((new BaselineFileRepository())->loadIfPresent($this->baselineFilePath)); } - public function test_exists_is_true_when_the_file_is_present(): void + public function test_load_if_present_returns_the_baseline_when_the_file_is_present(): void { + $violations = new Violations(); + $violations->add(new Violation('App\Controller\Shop', 'should have name end with Controller', 10, 'Controller/Shop.php')); + $repository = new BaselineFileRepository(); - $repository->save(Baseline::empty(), $this->baselineFilePath); + $repository->save(Baseline::fromViolations($violations), $this->baselineFilePath); + + $loaded = $repository->loadIfPresent($this->baselineFilePath); - self::assertTrue($repository->exists($this->baselineFilePath)); + self::assertNotNull($loaded); + self::assertEquals($violations, $loaded->getViolations()); } }