diff --git a/config/services.yml b/config/services.yml index f02fe149..e926a4d0 100644 --- a/config/services.yml +++ b/config/services.yml @@ -6,3 +6,8 @@ services: DaveLiddament\StaticAnalysisResultsBaseliner\: resource: '../src/*' + # Value objects and exceptions are not services. Excluding them means a newly type hinted + # value object fails at container compile time rather than at runtime. + exclude: + - '../src/**/*Exception.php' + - '../src/Domain/Common/*' diff --git a/src/Domain/OutputFormatter/InvalidOutputFormatterException.php b/src/Domain/OutputFormatter/InvalidOutputFormatterException.php index 6ffdc118..09b196b9 100644 --- a/src/Domain/OutputFormatter/InvalidOutputFormatterException.php +++ b/src/Domain/OutputFormatter/InvalidOutputFormatterException.php @@ -2,6 +2,8 @@ namespace DaveLiddament\StaticAnalysisResultsBaseliner\Domain\OutputFormatter; -final class InvalidOutputFormatterException extends \Exception +use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Common\SarbException; + +final class InvalidOutputFormatterException extends SarbException { } diff --git a/src/Domain/Pruner/PrunedResults.php b/src/Domain/Pruner/PrunedResults.php index b7d5b453..7f11fae7 100644 --- a/src/Domain/Pruner/PrunedResults.php +++ b/src/Domain/Pruner/PrunedResults.php @@ -28,4 +28,21 @@ public function getInputAnalysisResults(): AnalysisResults { return $this->inputAnalysisResults; } + + /** + * Returns the input results that matched the baseline (i.e. input results minus the pruned ones). + */ + public function getBaseLinedResults(): AnalysisResults + { + $prunedAnalysisResults = $this->prunedResults->getAnalysisResults(); + + $baseLinedResults = []; + foreach ($this->inputAnalysisResults->getAnalysisResults() as $analysisResult) { + if (!in_array($analysisResult, $prunedAnalysisResults, true)) { + $baseLinedResults[] = $analysisResult; + } + } + + return new AnalysisResults($baseLinedResults); + } } diff --git a/src/Domain/Utils/FqcnRemover.php b/src/Domain/Utils/FqcnRemover.php index 232b3397..3c12b843 100644 --- a/src/Domain/Utils/FqcnRemover.php +++ b/src/Domain/Utils/FqcnRemover.php @@ -11,7 +11,7 @@ final class FqcnRemover /** * Removes anything that looks like a FQCN from the string. */ - public function removeRqcn(string $raw): string + public function removeFqcn(string $raw): string { $parts = explode(' ', $raw); $outputParts = []; diff --git a/src/Domain/Utils/StringUtils.php b/src/Domain/Utils/StringUtils.php index d4384036..c00db931 100644 --- a/src/Domain/Utils/StringUtils.php +++ b/src/Domain/Utils/StringUtils.php @@ -33,16 +33,6 @@ public static function removeFromStart(string $needle, string $haystack): string return substr($haystack, $length); } - /** - * Returns true if $haystack ends with $needle. - */ - public static function endsWith(string $needle, string $haystack): bool - { - $length = strlen($needle); - - return substr($haystack, -$length) === $needle; - } - /** * Returns true if line is empty (or contains only white space). */ diff --git a/src/Framework/Command/ListResultsParsesCommand.php b/src/Framework/Command/ListResultsParsesCommand.php index 4bdd4976..c8fbae64 100644 --- a/src/Framework/Command/ListResultsParsesCommand.php +++ b/src/Framework/Command/ListResultsParsesCommand.php @@ -49,7 +49,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $output->writeln( sprintf( - ' Create baseline: %s | sarb create --input-format="%s"', + ' Create baseline: %s | sarb create --input-format="%s" ', $identifier->getToolCommand(), $identifier->getCode(), ), @@ -57,7 +57,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $output->writeln( sprintf( - ' Remove baseline results: %s | sarb remove', + ' Remove baseline results: %s | sarb remove ', $identifier->getToolCommand(), ), ); diff --git a/src/Framework/Command/RemoveBaseLineFromResultsCommand.php b/src/Framework/Command/RemoveBaseLineFromResultsCommand.php index ab867ccd..1a707981 100644 --- a/src/Framework/Command/RemoveBaseLineFromResultsCommand.php +++ b/src/Framework/Command/RemoveBaseLineFromResultsCommand.php @@ -53,6 +53,11 @@ protected function configure(): void { $this->setDescription('Shows issues created since the baseline'); + // The documentation tells users to run "sarb remove". That currently works because + // Symfony matches unambiguous abbreviations, but a real alias means adding another + // remove-* command in the future can not silently break every user's CI. + $this->setAliases(['remove']); + $outputFormatters = $this->outputFormatterLookupService->getIdentifiers(); $this->addOption( self::OUTPUT_FORMAT, @@ -123,8 +128,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int $returnCode = $outputAnalysisResults->hasNoIssues() ? 0 : 1; - if ($showRandomIssues && !$prunedResults->getInputAnalysisResults()->hasNoIssues()) { - $randomIssues = $this->randomResultsPicker->getRandomResultsToFix($prunedResults->getInputAnalysisResults()); + $baseLinedResults = $prunedResults->getBaseLinedResults(); + if ($showRandomIssues && !$baseLinedResults->hasNoIssues()) { + $randomIssues = $this->randomResultsPicker->getRandomResultsToFix($baseLinedResults); OutputWriter::writeToStdError( $output, diff --git a/src/Framework/Container/internal/AddCommandCompilerPass.php b/src/Framework/Container/internal/AddCommandCompilerPass.php index 117bfbba..158e30ee 100644 --- a/src/Framework/Container/internal/AddCommandCompilerPass.php +++ b/src/Framework/Container/internal/AddCommandCompilerPass.php @@ -10,6 +10,7 @@ namespace DaveLiddament\StaticAnalysisResultsBaseliner\Framework\Container\internal; +use Composer\InstalledVersions; use DaveLiddament\StaticAnalysisResultsBaseliner\Framework\Container\Container; use Symfony\Component\Console\Application; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; @@ -18,14 +19,24 @@ final class AddCommandCompilerPass implements CompilerPassInterface { + private const PACKAGE_NAME = 'dave-liddament/sarb'; + public function process(ContainerBuilder $container): void { $container->register(Application::class, Application::class); $definition = $container->getDefinition(Application::class); + $definition->setArguments(['SARB', $this->getVersion()]); $definition->setPublic(true); $taggedServices = $container->findTaggedServiceIds(Container::COMMAND_TAG); foreach (array_keys($taggedServices) as $id) { $definition->addMethodCall('add', [new Reference($id)]); } } + + private function getVersion(): string + { + return InstalledVersions::isInstalled(self::PACKAGE_NAME) + ? (InstalledVersions::getPrettyVersion(self::PACKAGE_NAME) ?? 'UNKNOWN') + : 'UNKNOWN'; + } } diff --git a/src/Plugins/ResultsParsers/PhpstanJsonResultsParser/PhpstanJsonResultsParser.php b/src/Plugins/ResultsParsers/PhpstanJsonResultsParser/PhpstanJsonResultsParser.php index e93f531c..b8c5e479 100644 --- a/src/Plugins/ResultsParsers/PhpstanJsonResultsParser/PhpstanJsonResultsParser.php +++ b/src/Plugins/ResultsParsers/PhpstanJsonResultsParser/PhpstanJsonResultsParser.php @@ -33,7 +33,8 @@ * Handles PHPStan's JSON output. * * NOTE: SARB only deals with errors that are attached to a particular line in a file. - * PHPStan can report general errors (not specific to file). These are ignored by SARB. + * PHPStan can report general errors (not specific to a file). If any of these are present SARB + * stops with an error, as they normally indicate the PHPStan run itself failed. */ final class PhpstanJsonResultsParser implements ResultsParser { @@ -121,7 +122,7 @@ private function convertAnalysisResultFromArray( } $rawType = ArrayUtils::getStringValue($analysisResultAsArray, self::TYPE); - $type = $this->fqcnRemover->removeRqcn($rawType); + $type = $this->fqcnRemover->removeFqcn($rawType); $location = Location::fromAbsoluteFileName( $absoluteFileName, diff --git a/tests/Integration/EndToEndTest.php b/tests/Integration/EndToEndTest.php index 1561e59d..84480f4c 100644 --- a/tests/Integration/EndToEndTest.php +++ b/tests/Integration/EndToEndTest.php @@ -5,7 +5,6 @@ use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Common\ProjectRoot; use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Common\RelativeFileName; use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Utils\Path; -use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Utils\StringUtils; use DaveLiddament\StaticAnalysisResultsBaseliner\Framework\Command\CreateBaseLineCommand; use DaveLiddament\StaticAnalysisResultsBaseliner\Framework\Command\ListHistoryAnalysersCommand; use DaveLiddament\StaticAnalysisResultsBaseliner\Framework\Command\ListResultsParsesCommand; @@ -406,7 +405,7 @@ private function updatePathsInJsonFiles(ProjectRoot $projectRoot): void $files = scandir($directory); $this->assertNotFalse($files); foreach ($files as $file) { - if (StringUtils::endsWith('.json', $file)) { + if (str_ends_with($file, '.json')) { $fullPath = Path::makeAbsolute($file, $directory); $contents = file_get_contents($fullPath); $this->assertNotFalse($contents); diff --git a/tests/Unit/Core/Pruner/PrunedResultsTest.php b/tests/Unit/Core/Pruner/PrunedResultsTest.php new file mode 100644 index 00000000..df1767f8 --- /dev/null +++ b/tests/Unit/Core/Pruner/PrunedResultsTest.php @@ -0,0 +1,88 @@ +baseLinedResult = $this->buildAnalysisResult($projectRoot, '/FILE_1', 1, 'TYPE_1', Severity::error()); + $this->newResult = $this->buildAnalysisResult($projectRoot, '/FILE_2', 2, 'TYPE_2', Severity::error()); + } + + public function testGetters(): void + { + $baseLine = $this->createBaseLine(); + $prunedAnalysisResults = new AnalysisResults([$this->newResult]); + $inputAnalysisResults = new AnalysisResults([$this->baseLinedResult, $this->newResult]); + + $prunedResults = new PrunedResults($baseLine, $prunedAnalysisResults, $inputAnalysisResults); + + $this->assertSame($baseLine, $prunedResults->getBaseLine()); + $this->assertSame($prunedAnalysisResults, $prunedResults->getPrunedResults()); + $this->assertSame($inputAnalysisResults, $prunedResults->getInputAnalysisResults()); + } + + public function testBaseLinedResultsAreInputResultsMinusPrunedResults(): void + { + $prunedResults = new PrunedResults( + $this->createBaseLine(), + new AnalysisResults([$this->newResult]), + new AnalysisResults([$this->baseLinedResult, $this->newResult]), + ); + + $baseLinedResults = $prunedResults->getBaseLinedResults(); + + $this->assertSame([$this->baseLinedResult], $baseLinedResults->getAnalysisResults()); + } + + public function testNoBaseLinedResultsWhenAllInputResultsAreNew(): void + { + $prunedResults = new PrunedResults( + $this->createBaseLine(), + new AnalysisResults([$this->baseLinedResult, $this->newResult]), + new AnalysisResults([$this->baseLinedResult, $this->newResult]), + ); + + $this->assertTrue($prunedResults->getBaseLinedResults()->hasNoIssues()); + } + + private function createBaseLine(): BaseLine + { + $baseLineResultsBuilder = new BaseLineResultsBuilder(); + $baseLineResultsBuilder->add('FILE_1', 1, 'TYPE_1', Severity::error()); + + return new BaseLine( + new HistoryFactoryStub(), + $baseLineResultsBuilder->build(), + new SarbJsonResultsParser(), + new GitCommit('fae40b3d596780ffd746dbd2300d05dcfbd09033'), + ); + } +} diff --git a/tests/Unit/Core/Utils/FqcnRemoverTest.php b/tests/Unit/Core/Utils/FqcnRemoverTest.php index 02d3353a..56c0fffc 100644 --- a/tests/Unit/Core/Utils/FqcnRemoverTest.php +++ b/tests/Unit/Core/Utils/FqcnRemoverTest.php @@ -33,7 +33,7 @@ public static function dataProvider(): array public function testFqcnRemover(string $input, string $expected): void { $fqcnRemove = new FqcnRemover(); - $actual = $fqcnRemove->removeRqcn($input); + $actual = $fqcnRemove->removeFqcn($input); $this->assertEquals($expected, $actual); } } diff --git a/tests/Unit/Core/Utils/StringUtilsTest.php b/tests/Unit/Core/Utils/StringUtilsTest.php index e6f64752..533e2053 100644 --- a/tests/Unit/Core/Utils/StringUtilsTest.php +++ b/tests/Unit/Core/Utils/StringUtilsTest.php @@ -33,21 +33,6 @@ public function testRemoveFromStartExceptionWhenInvalidStart(): void StringUtils::removeFromStart('Bar', 'FooBar'); } - public function testDoesEndWith1(): void - { - $this->assertTrue(StringUtils::endsWith('e', 'abcde')); - } - - public function testDoesEndWith2(): void - { - $this->assertTrue(StringUtils::endsWith('de', 'abcde')); - } - - public function testDoesNotEndWith1(): void - { - $this->assertFalse(StringUtils::endsWith('d', 'abcde')); - } - public function testIsEmpty(): void { $this->assertTrue(StringUtils::isEmptyLine('')); diff --git a/tools/Phpat/LayerTest.php b/tools/Phpat/LayerTest.php index 669dba73..91b9cfe5 100644 --- a/tools/Phpat/LayerTest.php +++ b/tools/Phpat/LayerTest.php @@ -26,6 +26,16 @@ public function testDomainDoesNotDependOnOtherLayers(): Rule ; } + public function testDomainDoesNotDependOnSymfony(): Rule + { + return PHPat::rule() + ->classes(Selector::inNamespace(self::DOMAIN_NAMESPACE)) + ->shouldNotDependOn() + ->classes(Selector::inNamespace('Symfony')) + ->because('Domain code should not depend on the framework') + ; + } + public function testsPluginDoesNotDependOnFrameworkLayer(): Rule { return PHPat::rule()