diff --git a/CLAUDE.md b/CLAUDE.md index 8af46efb..cdd4a732 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..55a86a1c 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` 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 74a8ebd0..2e8a787c 100644 --- a/src/CLI/Baseline.php +++ b/src/CLI/Baseline.php @@ -1,30 +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 @@ -45,55 +56,36 @@ 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 + public function prune(Violations $currentViolations): self { - if (!$filePath && file_exists($defaultFilePath)) { - $filePath = $defaultFilePath; - } + $prunedViolations = $currentViolations->intersection($this->violations); - return $filePath ?: null; - } - - public static function empty(): self - { - return new self(new Violations(), ''); - } - - public static function create(bool $skipBaseline, ?string $baselineFilePath): self - { - if ($skipBaseline || null === $baselineFilePath) { - return self::empty(); + // a baseline stored without line numbers keeps its format + if (!$this->hasLineNumbers()) { + $prunedViolations = $prunedViolations->withoutLineNumbers(); } - return self::loadFromFile($baselineFilePath); + return new self($prunedViolations); } - public static function loadFromFile(string $filename): self + public function withoutLineNumbers(): self { - if (!file_exists($filename)) { - throw new \RuntimeException("Baseline file '$filename' not found."); - } - - $contents = file_get_contents($filename); - - if (false === $contents) { - throw new \RuntimeException("Baseline file '$filename' could not be read."); - } - - return new self( - Violations::fromJson($contents), - $filename - ); + return new self($this->violations->withoutLineNumbers()); } - public static function save(string $filename, Violations $violations, bool $ignoreLineNumbers = false): void + private function hasLineNumbers(): bool { - if ($ignoreLineNumbers) { - $violations = $violations->withoutLineNumbers(); + foreach ($this->violations as $violation) { + if (null !== $violation->getLine()) { + return true; + } } - file_put_contents($filename, Json::encode($violations)); + return false; } } diff --git a/src/CLI/BaselineFileRepository.php b/src/CLI/BaselineFileRepository.php new file mode 100644 index 00000000..48f11518 --- /dev/null +++ b/src/CLI/BaselineFileRepository.php @@ -0,0 +1,41 @@ +getViolations())); + } + + /** + * 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) ? $this->load($filename) : null; + } +} diff --git a/src/CLI/CheckHandler.php b/src/CLI/CheckHandler.php index 8fcaa6c1..80b62ac5 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( @@ -30,14 +32,19 @@ 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()); - $baseline = Baseline::create($config->isSkipBaseline(), $config->getBaselineFilePath()); + // 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 ? null : $this->baselineRepository->loadIfPresent($baselineFilePath); - $baseline->getFilename() && $output->writeln("Baseline file '{$baseline->getFilename()}' found"); + if (null === $baseline) { + $baseline = Baseline::empty(); + } else { + $output->writeln("Baseline file '$baselineFilePath' found"); + } $output->writeln("Config file '{$options->getConfigFilePath()}' found\n"); @@ -74,7 +81,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/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 b70a1937..f199d94b 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(); } @@ -80,6 +80,7 @@ protected function configure(): void ); $this->commonOptions->addTo($this); + $this->commonOptions->addIgnoreBaselineLinenumbers($this); } protected function isRunningAsPhar(): bool @@ -138,17 +139,31 @@ protected function execute(InputInterface $input, OutputInterface $output): int protected function parseOptions(InputInterface $input): CheckOptions { - $useBaseline = (string) $input->getOption(self::USE_BASELINE_PARAM); - return new 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), - 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 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 + { + if ((bool) $input->getOption(self::SKIP_BASELINE_PARAM)) { + return null; + } + + $useBaseline = (string) $input->getOption(self::USE_BASELINE_PARAM); + + return '' !== $useBaseline ? $useBaseline : BaselineFileRepository::DEFAULT_FILENAME; + } } 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 8cb26e44..82ea91aa 100644 --- a/src/CLI/Command/GenerateBaseline.php +++ b/src/CLI/Command/GenerateBaseline.php @@ -4,14 +4,13 @@ namespace Arkitect\CLI\Command; -use Arkitect\CLI\Baseline; +use Arkitect\CLI\BaselineFileRepository; use Arkitect\CLI\GenerateBaselineHandler; use Arkitect\CLI\GenerateBaselineOptions; use Arkitect\CLI\Runner; 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 @@ -32,7 +31,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,10 +44,11 @@ protected function configure(): void self::FILENAME_ARG, InputArgument::OPTIONAL, 'The baseline file to create', - Baseline::DEFAULT_FILENAME + BaselineFileRepository::DEFAULT_FILENAME ); $this->commonOptions->addTo($this); + $this->commonOptions->addIgnoreBaselineLinenumbers($this); } protected function isRunningAsPhar(): bool @@ -61,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 new file mode 100644 index 00000000..29a2ef50 --- /dev/null +++ b/src/CLI/Command/PruneBaseline.php @@ -0,0 +1,99 @@ +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); + + 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), + 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..76eb2002 --- /dev/null +++ b/src/CLI/PruneBaselineHandler.php @@ -0,0 +1,49 @@ +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()); + + $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..3f162dbf --- /dev/null +++ b/src/CLI/PruneBaselineOptions.php @@ -0,0 +1,40 @@ +configFilePath; + } + + public function getTargetPhpVersion(): ?string + { + return $this->targetPhpVersion; + } + + public function getAutoloadFilePath(): ?string + { + return $this->autoloadFilePath; + } + + 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/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 new file mode 100644 index 00000000..b71d3a1b --- /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->getDisplay()); + + $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->getDisplay()); + } + + 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->getDisplay()); + } + + 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..b2a1749f --- /dev/null +++ b/tests/Unit/CLI/BaselineFileRepositoryTest.php @@ -0,0 +1,63 @@ +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_load_if_present_is_null_when_the_file_is_absent(): void + { + self::assertNull((new BaselineFileRepository())->loadIfPresent($this->baselineFilePath)); + } + + 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::fromViolations($violations), $this->baselineFilePath); + + $loaded = $repository->loadIfPresent($this->baselineFilePath); + + self::assertNotNull($loaded); + self::assertEquals($violations, $loaded->getViolations()); + } +} diff --git a/tests/Unit/CLI/BaselineTest.php b/tests/Unit/CLI/BaselineTest.php new file mode 100644 index 00000000..b5062be4 --- /dev/null +++ b/tests/Unit/CLI/BaselineTest.php @@ -0,0 +1,102 @@ +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_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(); + $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..de3902e4 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( @@ -101,7 +102,6 @@ private function createOptions( targetPhpVersion: null, stopOnFailure: false, baselineFilePath: $baselineFilePath, - skipBaseline: false, ignoreBaselineLinenumbers: false, format: 'text', autoloadFilePath: null, 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..fca5a105 --- /dev/null +++ b/tests/Unit/CLI/PruneBaselineHandlerTest.php @@ -0,0 +1,102 @@ +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, + 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); + } }