diff --git a/README.md b/README.md index d5c7b3d1..d2e630ee 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,6 @@ * [Requirements](#requirements) * [Installing](#installing) * [Using SARB](#using-sarb) - * [Examples](#examples) * [Further reading](#further-reading) ## Why SARB? @@ -30,10 +29,10 @@ to legacy projects the tools have probably reported thousands of problems. It's unrealistic to fix all but the most critical ones before continuing development. SARB is used to create a baseline of these results. As work on the project -progresses SARB can takes the latest static analysis results, removes +progresses SARB takes the latest static analysis results, removes those issues in the baseline and report the issues raised since the baseline. SARB does this, in conjunction with git, by tracking lines of code between commits. -Currently SARB only supports git but it is possible to [add support for other SCMs](docs/NewHistoryAnalyser.md). +Currently, SARB only supports git, but it is possible to [add support for other SCMs](docs/NewHistoryAnalyser.md). SARB is written in PHP, however it can be used to baseline results for any language and [any static analysis tool](docs/CustomInputFormats.md). @@ -44,7 +43,7 @@ SARB should not be used on greenfield projects. If you're lucky enough to work o ## Requirements -Currently SARB only supports projects that use [git](https://git-scm.com/). +Currently, SARB only supports projects that use [git](https://git-scm.com/). SARB requires PHP >= 7.3 to run. The project being analysed does not need to run PHP 7.3 or even be a PHP project at all. @@ -189,6 +188,16 @@ To get the full history checked out use this: Also don't forget to use the SARB option `--output-format=github`. It will annotate your PR with any issues that have been added since the baseline. +## Gradually improving the codebase + +In an ideal world SARB should not be required. SARB prevents you from adding new issues to your codebase. + +It also provides a `--clean-up` option when running `remove`. +Running SARB with this option will pick out 5 random issues that are still in the baseline. +Challenge your team to fix 5 issues in the baseline every day. +Over a working year that'll be 1000 issues gone from the baseline! +Soon you'll be able to ditch SARB for good! + ## Further Reading diff --git a/src/Domain/Pruner/PrunedResults.php b/src/Domain/Pruner/PrunedResults.php index 89a72d0a..b83feef9 100644 --- a/src/Domain/Pruner/PrunedResults.php +++ b/src/Domain/Pruner/PrunedResults.php @@ -18,15 +18,18 @@ class PrunedResults */ private $prunedResults; /** - * @var int + * @var AnalysisResults */ - private $inputAnalysisResultsCount; + private $inputAnalysisResults; - public function __construct(BaseLine $baseLine, AnalysisResults $prunedResults, int $inputAnalysisResultsCount) - { + public function __construct( + BaseLine $baseLine, + AnalysisResults $prunedResults, + AnalysisResults $inputAnalysisResults + ) { $this->baseLine = $baseLine; $this->prunedResults = $prunedResults; - $this->inputAnalysisResultsCount = $inputAnalysisResultsCount; + $this->inputAnalysisResults = $inputAnalysisResults; } public function getBaseLine(): BaseLine @@ -39,8 +42,8 @@ public function getPrunedResults(): AnalysisResults return $this->prunedResults; } - public function getInputAnalysisResultsCount(): int + public function getInputAnalysisResults(): AnalysisResults { - return $this->inputAnalysisResultsCount; + return $this->inputAnalysisResults; } } diff --git a/src/Domain/Pruner/ResultsPruner.php b/src/Domain/Pruner/ResultsPruner.php index 5f551a6a..969474fa 100644 --- a/src/Domain/Pruner/ResultsPruner.php +++ b/src/Domain/Pruner/ResultsPruner.php @@ -20,7 +20,6 @@ class ResultsPruner implements ResultsPrunerInterface * @var BaseLineImporter */ private $baseLineImporter; - /** * @var BaseLineResultsRemover */ @@ -64,6 +63,6 @@ public function getPrunedResults( $baseLine->getAnalysisResults() ); - return new PrunedResults($baseLine, $outputAnalysisResults, $inputAnalysisResults->getCount()); + return new PrunedResults($baseLine, $outputAnalysisResults, $inputAnalysisResults); } } diff --git a/src/Domain/RandomResultsPicker/RandomResultsPicker.php b/src/Domain/RandomResultsPicker/RandomResultsPicker.php new file mode 100644 index 00000000..17268bce --- /dev/null +++ b/src/Domain/RandomResultsPicker/RandomResultsPicker.php @@ -0,0 +1,45 @@ +randomNumberGenerator = $randomNumberGenerator; + } + + /** + * Returns a random selection of the issues found. + */ + public function getRandomResultsToFix(AnalysisResults $issues): AnalysisResults + { + $randomIssuesToFix = min(self::RANDOM_ISSUES_TO_FIX, $issues->getCount()); + + $randomIssues = new AnalysisResultsBuilder(); + $issuesToPickFrom = $issues->getAnalysisResults(); + + for ($i = 0; $i < $randomIssuesToFix; ++$i) { + $totalRemaining = count($issuesToPickFrom); + $issuePickedIndex = $this->randomNumberGenerator->getRandomNumber($totalRemaining - 1); + $issuePicked = $issuesToPickFrom[$issuePickedIndex]; + $randomIssues->addAnalysisResult($issuePicked); + array_splice($issuesToPickFrom, $issuePickedIndex, 1); + } + + return $randomIssues->build(); + } +} diff --git a/src/Domain/Utils/RandomNumberGenerator.php b/src/Domain/Utils/RandomNumberGenerator.php new file mode 100644 index 00000000..78f22aab --- /dev/null +++ b/src/Domain/Utils/RandomNumberGenerator.php @@ -0,0 +1,16 @@ +outputFormatterLookupService = $outputFormatterLookupService; parent::__construct(self::COMMAND_NAME); $this->resultsPruner = $resultsPruner; + $this->tableOutputFormatter = $tableOutputFormatter; + $this->randomResultsPicker = $randomResultsPicker; } protected function configure(): void @@ -71,6 +85,13 @@ protected function configure(): void TableOutputFormatter::CODE ); + $this->addOption( + self::SHOW_RANDOM_ERRORS, + null, + InputOption::VALUE_NONE, + 'Show a random 5 issues in the baseline to fix' + ); + ProjectRootHelper::configureProjectRootOption($this); BaseLineFileHelper::configureBaseLineFileArgument($this); @@ -83,6 +104,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $outputFormatter = $this->getOutputFormatter($input); $baseLineFileName = BaseLineFileHelper::getBaselineFile($input); $inputAnalysisResultsAsString = CliConfigReader::getStdin($input); + $showRandomIssues = CliConfigReader::getBooleanOption($input, self::SHOW_RANDOM_ERRORS); $prunedResults = $this->resultsPruner->getPrunedResults( $baseLineFileName, @@ -94,7 +116,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int OutputWriter::writeToStdError( $output, - "Latest analysis issue count: {$prunedResults->getInputAnalysisResultsCount()}", + "Latest analysis issue count: {$prunedResults->getInputAnalysisResults()->getCount()}", false ); @@ -113,7 +135,27 @@ protected function execute(InputInterface $input, OutputInterface $output): int $outputAsString = $outputFormatter->outputResults($outputAnalysisResults); $output->writeln($outputAsString); - return $outputAnalysisResults->hasNoIssues() ? 0 : 1; + $returnCode = $outputAnalysisResults->hasNoIssues() ? 0 : 1; + + if ($showRandomIssues && !$prunedResults->getInputAnalysisResults()->hasNoIssues()) { + $randomIssues = $this->randomResultsPicker->getRandomResultsToFix($prunedResults->getInputAnalysisResults()); + + OutputWriter::writeToStdError( + $output, + "\n\nRandom {$randomIssues->getCount()} issues in the baseline to fix...", + false + ); + + $outputAsString = $this->tableOutputFormatter->outputResults($randomIssues); + + OutputWriter::writeToStdError( + $output, + $outputAsString, + false + ); + } + + return $returnCode; } catch (Throwable $throwable) { $returnCode = ErrorReporter::reportError($output, $throwable); diff --git a/src/Framework/Command/internal/ErrorReporter.php b/src/Framework/Command/internal/ErrorReporter.php index 2430fd24..26c6a501 100644 --- a/src/Framework/Command/internal/ErrorReporter.php +++ b/src/Framework/Command/internal/ErrorReporter.php @@ -41,6 +41,7 @@ public static function reportError(OutputInterface $output, Throwable $throwable } catch (Throwable $e) { // This should never happen. All exceptions should extend SarbException OutputWriter::writeToStdError($output, "Unexpected critical error: {$e->getMessage()}", true); + OutputWriter::writeToStdError($output, $e->getTraceAsString(), true); return 100; } diff --git a/src/Plugins/OutputFormatters/TableOutputFormatter.php b/src/Plugins/OutputFormatters/TableOutputFormatter.php index c77d8ff5..ec3cf91b 100644 --- a/src/Plugins/OutputFormatters/TableOutputFormatter.php +++ b/src/Plugins/OutputFormatters/TableOutputFormatter.php @@ -56,7 +56,7 @@ private function addIssuesInTable(BufferedOutput $output, AnalysisResults $analy $currentTable->setHeaders($headings); } - Assert::notNull($currentTable); + Assert::notNull($currentTable, 'No Table object'); $currentTable->addRow([ $analysisResult->getLocation()->getLineNumber()->getLineNumber(), $analysisResult->getMessage(), diff --git a/tests/Unit/Core/RandomResultsPicker/RandomResultsPickerTest.php b/tests/Unit/Core/RandomResultsPicker/RandomResultsPickerTest.php new file mode 100644 index 00000000..76e24c97 --- /dev/null +++ b/tests/Unit/Core/RandomResultsPicker/RandomResultsPickerTest.php @@ -0,0 +1,114 @@ +projectRoot = ProjectRoot::fromCurrentWorkingDirectory('/'); + + $this->randomNumberGenerator = $this->createStub(RandomNumberGenerator::class); + $this->randomResultsPicker = new RandomResultsPicker($this->randomNumberGenerator); + } + + public function testPick5Results(): void + { + $issuesBuilder = new AnalysisResultsBuilder(); + $issue1 = $this->addIssue($issuesBuilder, 'a'); + $issue2 = $this->addIssue($issuesBuilder, 'b'); + $this->addIssue($issuesBuilder, 'c'); + $issue4 = $this->addIssue($issuesBuilder, 'd'); + $this->addIssue($issuesBuilder, 'e'); + $issue6 = $this->addIssue($issuesBuilder, 'f'); + $issue7 = $this->addIssue($issuesBuilder, 'g'); + + $this->setRandomNumbers(5, 1, 0, 3, 1); + + $pickedIssues = $this->randomResultsPicker->getRandomResultsToFix($issuesBuilder->build()); + + $expected = [ + $issue1, + $issue2, + $issue4, + $issue6, + $issue7, + ]; + + $this->assertEquals($expected, $pickedIssues->getAnalysisResults()); + $this->assertSame(5, $pickedIssues->getCount()); + $this->assertFalse($pickedIssues->hasNoIssues()); + } + + public function testPickWhenFewerThan5Results(): void + { + $issuesBuilder = new AnalysisResultsBuilder(); + $issue1 = $this->addIssue($issuesBuilder, 'a'); + $issue2 = $this->addIssue($issuesBuilder, 'b'); + $issue3 = $this->addIssue($issuesBuilder, 'c'); + + $this->setRandomNumbers(2, 0, 0); + + $pickedIssues = $this->randomResultsPicker->getRandomResultsToFix($issuesBuilder->build()); + + $expected = [ + $issue1, + $issue2, + $issue3, + ]; + + $this->assertEquals($expected, $pickedIssues->getAnalysisResults()); + $this->assertSame(3, $pickedIssues->getCount()); + $this->assertFalse($pickedIssues->hasNoIssues()); + } + + private function addIssue(AnalysisResultsBuilder $issuesBuilder, string $string): AnalysisResult + { + $analysisResult = new AnalysisResult( + Location::fromRelativeFileName( + new RelativeFileName($string), + $this->projectRoot, + new LineNumber(10), + ), + new Type("Type-{$string}"), + $string, + [] + ); + + $issuesBuilder->addAnalysisResult($analysisResult); + + return $analysisResult; + } + + private function setRandomNumbers(int ...$numbers): void + { + $this->randomNumberGenerator->method('getRandomNumber')->willReturnOnConsecutiveCalls(...$numbers); + } +} diff --git a/tests/Unit/Framework/Command/RemoveBaseLineCommandTest.php b/tests/Unit/Framework/Command/RemoveBaseLineCommandTest.php index e1e59734..cc93bfe4 100644 --- a/tests/Unit/Framework/Command/RemoveBaseLineCommandTest.php +++ b/tests/Unit/Framework/Command/RemoveBaseLineCommandTest.php @@ -12,9 +12,11 @@ use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Common\Type; use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\OutputFormatter\OutputFormatter; use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Pruner\PrunedResults; +use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\RandomResultsPicker\RandomResultsPicker; use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\ResultsParser\AnalysisResult; use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\ResultsParser\AnalysisResults; use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\ResultsParser\AnalysisResultsBuilder; +use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Utils\RandomNumberGenerator; use DaveLiddament\StaticAnalysisResultsBaseliner\Framework\Command\RemoveBaseLineFromResultsCommand; use DaveLiddament\StaticAnalysisResultsBaseliner\Framework\Container\OutputFormatterRegistry; use DaveLiddament\StaticAnalysisResultsBaseliner\Plugins\GitDiffHistoryAnalyser\GitCommit; @@ -40,6 +42,7 @@ class RemoveBaseLineCommandTest extends TestCase public const BASELINE_FILE_ARGUMENT = 'baseline-file'; public const OUTPUT_FORMAT_OPTION = '--output-format'; public const PROJECT_ROOT = '--project-root'; + public const CLEAN_UP = '--clean-up'; /** * @var ProjectRoot @@ -197,6 +200,27 @@ public function testException(): void $this->assertReturnCode(100, $commandTester); } + public function testCleanUpOptions(): void + { + $commandTester = $this->createCommandTester( + $this->getAnalysisResultsWithXResults(0), + null, + null + ); + + $commandTester->execute([ + self::BASELINE_FILE_ARGUMENT => self::BASELINE_FILENAME, + self::CLEAN_UP => true, + ]); + + $this->assertReturnCode(0, $commandTester); + + $this->assertResponseContains('Random 2 issues in the baseline to fix...', $commandTester); + $this->assertResponseContains('FILE: /FILE_2', $commandTester); + $this->assertResponseContains('| 2 | MESSAGE_1 |', $commandTester); + $this->assertResponseContains('| 2 | MESSAGE_0 |', $commandTester); + } + private function createCommandTester( AnalysisResults $expectedAnalysisResults, ?ProjectRoot $projectRoot, @@ -218,7 +242,7 @@ private function createCommandTester( $prunedResults = new PrunedResults( $baseLine, $expectedAnalysisResults, - 2 + $this->getAnalysisResultsWithXResults(2) ); $mockResultsPruner = new MockResultsPruner( @@ -230,7 +254,9 @@ private function createCommandTester( $command = new RemoveBaseLineFromResultsCommand( $mockResultsPruner, - $this->outputFormatterRegistry + $this->outputFormatterRegistry, + new TableOutputFormatter(), + new RandomResultsPicker(new RandomNumberGenerator()) ); $commandTester = new CommandTester($command);