Merge pull request #136 from DaveLiddament/feature/results-parsesrs-c…

…an-report-sa-tool-failure ADD ability for results parser to throw exception if SA tool reports errors
DaveLiddament · Jan 11, 2024 · fb1b031 · fb1b031
2 parents b13ea39 + 4065c5e
commit fb1b031
Show file tree

Hide file tree

Showing 15 changed files with 241 additions and 0 deletions.
diff --git a/docs/NewResultsParser.md b/docs/NewResultsParser.md
@@ -368,3 +368,12 @@ If the relative paths are not relative then use the following code for creating
             new LineNumber($lineAsInt)
         );
 ```
+
+## Passing on errors from the static analysis tool
+
+If the static analysis tool reports an error that means it could not run successfully, then throw the `ErrorReportedByStaticAnalysisTool` exception from the `convertFromString` method.
+
+This will cause SARB to report the error to the user and exit with a non-zero exit code. 
+See the [`PhpstanJsonResultsParser`](../src/Plugins/ResultsParsers/PhpstanJsonResultsParser/PhpstanJsonResultsParser.php) for an example of this.
+
+
diff --git a/src/Domain/Creator/BaseLineCreatorInterface.php b/src/Domain/Creator/BaseLineCreatorInterface.php
@@ -12,6 +12,7 @@
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\HistoryAnalyser\HistoryAnalyserException;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\HistoryAnalyser\HistoryFactory;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\ResultsParser\AnalysisResultsImportException;
+use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\ResultsParser\ErrorReportedByStaticAnalysisTool;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\ResultsParser\ResultsParser;
 
 interface BaseLineCreatorInterface
@@ -21,6 +22,7 @@ interface BaseLineCreatorInterface
      * @throws BaseLineImportException
      * @throws AnalysisResultsImportException
      * @throws HistoryAnalyserException
+     * @throws ErrorReportedByStaticAnalysisTool
      */
     public function createBaseLine(
         HistoryFactory $historyFactory,

diff --git a/src/Domain/Pruner/ResultsPruner.php b/src/Domain/Pruner/ResultsPruner.php
@@ -13,6 +13,7 @@
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\HistoryAnalyser\HistoryAnalyserException;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\ResultsParser\AnalysisResultsImporter;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\ResultsParser\AnalysisResultsImportException;
+use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\ResultsParser\ErrorReportedByStaticAnalysisTool;
 
 class ResultsPruner implements ResultsPrunerInterface
 {
@@ -44,6 +45,7 @@ public function __construct(
      * @throws FileAccessException
      * @throws AnalysisResultsImportException
      * @throws HistoryAnalyserException
+     * @throws ErrorReportedByStaticAnalysisTool
      */
     public function getPrunedResults(
         BaseLineFileName $baseLineFileName,

diff --git a/src/Domain/ResultsParser/AnalysisResultsImporter.php b/src/Domain/ResultsParser/AnalysisResultsImporter.php
@@ -12,6 +12,7 @@ class AnalysisResultsImporter
 {
     /**
      * @throws AnalysisResultsImportException
+     * @throws ErrorReportedByStaticAnalysisTool
      */
     public function import(
         ResultsParser $resultsParser,

diff --git a/src/Domain/ResultsParser/ErrorReportedByStaticAnalysisTool.php b/src/Domain/ResultsParser/ErrorReportedByStaticAnalysisTool.php
@@ -0,0 +1,22 @@
+<?php
+
+/**
+ * Static Analysis Results Baseliner (sarb).
+ *
+ * (c) Dave Liddament
+ *
+ * For the full copyright and licence information please view the LICENSE file distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace DaveLiddament\StaticAnalysisResultsBaseliner\Domain\ResultsParser;
+
+use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Common\SarbException;
+
+/**
+ * Thrown if the static analysis tool reports an error.
+ */
+final class ErrorReportedByStaticAnalysisTool extends SarbException
+{
+}
diff --git a/src/Domain/ResultsParser/ResultsParser.php b/src/Domain/ResultsParser/ResultsParser.php
@@ -23,6 +23,7 @@ interface ResultsParser
      *
      * @throws ParseAtLocationException
      * @throws InvalidContentTypeException
+     * @throws ErrorReportedByStaticAnalysisTool
      */
     public function convertFromString(string $resultsAsString, ProjectRoot $projectRoot): AnalysisResults;
 

diff --git a/src/Domain/Utils/ArrayUtils.php b/src/Domain/Utils/ArrayUtils.php
@@ -126,6 +126,22 @@ public static function assertArray(mixed $entity): void
         }
     }
 
+    /**
+     * @psalm-assert array<array-key,string> $array
+     *
+     * @param array<mixed> $array
+     *
+     * @throws ArrayParseException
+     */
+    public static function assertArrayOfStrings(array $array): void
+    {
+        foreach ($array as $key => $value) {
+            if (!is_string($value)) {
+                throw ArrayParseException::invalidType((string) $key, 'string');
+            }
+        }
+    }
+
     /**
      * Extracts a integer value from an array, however the integer is in string format.
      *

diff --git a/src/Framework/Command/internal/ErrorReporter.php b/src/Framework/Command/internal/ErrorReporter.php
@@ -9,6 +9,7 @@
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\File\FileAccessException;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\HistoryAnalyser\HistoryAnalyserException;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\ResultsParser\AnalysisResultsImportException;
+use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\ResultsParser\ErrorReportedByStaticAnalysisTool;
 use Symfony\Component\Console\Output\OutputInterface;
 
 class ErrorReporter
@@ -37,6 +38,10 @@ public static function reportError(OutputInterface $output, \Throwable $throwabl
             OutputWriter::writeToStdError($output, $e->getMessage(), true);
 
             return 15;
+        } catch (ErrorReportedByStaticAnalysisTool $e) {
+            OutputWriter::writeToStdError($output, $e->getMessage(), true);
+
+            return 16;
         } catch (\Throwable $e) {
             // This should never happen. All exceptions should extend SarbException
             OutputWriter::writeToStdError($output, "Unexpected critical error: {$e->getMessage()}", true);

diff --git a/src/Plugins/ResultsParsers/PhpstanJsonResultsParser/PhpstanJsonResultsParser.php b/src/Plugins/ResultsParsers/PhpstanJsonResultsParser/PhpstanJsonResultsParser.php
@@ -22,6 +22,7 @@
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\ResultsParser\AnalysisResult;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\ResultsParser\AnalysisResults;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\ResultsParser\AnalysisResultsBuilder;
+use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\ResultsParser\ErrorReportedByStaticAnalysisTool;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\ResultsParser\Identifier;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\ResultsParser\ResultsParser;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Utils\ArrayParseException;
@@ -42,6 +43,7 @@ class PhpstanJsonResultsParser implements ResultsParser
     private const TYPE = 'message';
     private const FILES = 'files';
     private const MESSAGES = 'messages';
+    private const ERRORS = 'errors';
 
     /**
      * @var FqcnRemover
@@ -59,6 +61,22 @@ public function convertFromString(string $resultsAsString, ProjectRoot $projectR
 
         $analysisResultsBuilder = new AnalysisResultsBuilder();
 
+        try {
+            $errors = ArrayUtils::getArrayValue($analysisResultsAsArray, self::ERRORS);
+        } catch (ArrayParseException $e) {
+            throw ParseAtLocationException::issueParsing($e, 'Root node');
+        }
+
+        if (count($errors) > 0) {
+            try {
+                ArrayUtils::assertArrayOfStrings($errors);
+            } catch (ArrayParseException $e) {
+                throw ParseAtLocationException::issueParsing($e, self::ERRORS);
+            }
+            $errorMessage = 'PHPStan failed with errors:'.\PHP_EOL.implode("\n", $errors);
+            throw new ErrorReportedByStaticAnalysisTool($errorMessage);
+        }
+
         try {
             $filesErrors = ArrayUtils::getArrayValue($analysisResultsAsArray, self::FILES);
         } catch (ArrayParseException $e) {

diff --git a/tests/Unit/Core/Utils/ArrayUtilsTest.php b/tests/Unit/Core/Utils/ArrayUtilsTest.php
@@ -142,4 +142,20 @@ public function testGetOptionalStringWithIncorrectType(): void
         $this->expectException(ArrayParseException::class);
         ArrayUtils::getOptionalStringValue(self::TEST_ARRAY, self::AGE_KEY);
     }
+
+    public function testAssertArrayOfStringsOnArrayContainingNonStringValues(): void
+    {
+        $this->expectException(ArrayParseException::class);
+        ArrayUtils::assertArrayOfStrings(self::TEST_ARRAY);
+    }
+
+    /** @doesNotPerformAssertions  */
+    public function testAssertArrayOfStringsOnArrayContainingOnlyStrings(): void
+    {
+        $array = [
+            'foo',
+            'bar',
+        ];
+        ArrayUtils::assertArrayOfStrings($array);
+    }
 }
diff --git a/tests/Unit/Framework/Command/CreateBaseLineCommandTest.php b/tests/Unit/Framework/Command/CreateBaseLineCommandTest.php
@@ -8,6 +8,7 @@
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Common\ProjectRoot;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\HistoryAnalyser\HistoryFactory;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\HistoryAnalyser\UnifiedDiffParser\Parser;
+use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\ResultsParser\ErrorReportedByStaticAnalysisTool;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\ResultsParser\ResultsParser;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Framework\Command\CreateBaseLineCommand;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Framework\Container\HistoryFactoryRegistry;
@@ -224,6 +225,26 @@ public function testSimulateThrowable(): void
         $this->assertReturnCode(100, $commandTester);
     }
 
+    public function testSimulateStaticAnalysisToolFailed(): void
+    {
+        $commandTester = $this->createCommandTester(
+            $this->historyFactoryStub,
+            $this->defaultResultsParser,
+            self::BASELINE_FILENAME,
+            null,
+            new ErrorReportedByStaticAnalysisTool('Tool failed'),
+        );
+
+        $commandTester->execute([
+            self::HISTORY_ANALYSER => HistoryFactoryStub::CODE,
+            self::BASELINE_FILE_ARGUMENT => self::BASELINE_FILENAME,
+            self::PROJECT_ROOT => '/tmp',
+        ]);
+
+        $this->assertReturnCode(16, $commandTester);
+        $this->assertResponseContains('Tool failed', $commandTester);
+    }
+
     private function createCommandTester(
         HistoryFactory $expectedHistoryFactory,
         ResultsParser $expectedResultsParser,

diff --git a/tests/Unit/Plugins/ResultsParsers/PhpstanJsonResultsParser/PhpstanJsonResultsParserTest.php b/tests/Unit/Plugins/ResultsParsers/PhpstanJsonResultsParser/PhpstanJsonResultsParserTest.php
@@ -7,6 +7,7 @@
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Common\ProjectRoot;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Common\Severity;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\File\InvalidContentTypeException;
+use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\ResultsParser\ErrorReportedByStaticAnalysisTool;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Utils\FqcnRemover;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Utils\ParseAtLocationException;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Plugins\ResultsParsers\PhpstanJsonResultsParser\PhpstanJsonResultsParser;
@@ -101,6 +102,8 @@ public function invalidFileProvider(): array
             ['phpstan/phpstan-invalid-missing-file.json'],
             ['phpstan/phpstan-invalid-missing-files.json'],
             ['phpstan/phpstan-invalid-missing-line.json'],
+            ['phpstan/phpstan-invalid-missing-errors.json'],
+            ['phpstan/phpstan-invalid-errors-not-strings.json'],
         ];
     }
 
@@ -113,4 +116,12 @@ public function testInvalidFileFormat(string $fileName): void
         $this->expectException(ParseAtLocationException::class);
         $this->phpstanJsonResultsParser->convertFromString($fileContents, $this->projectRoot);
     }
+
+    public function testPhpstanReportsErrors(): void
+    {
+        $fileContents = $this->getResource('phpstan/phpstan-with-errors.json');
+        $this->expectException(ErrorReportedByStaticAnalysisTool::class);
+        $this->expectExceptionMessage('PHPStan failed with errors:'.\PHP_EOL.'Error 1'.\PHP_EOL.'Error 2');
+        $this->phpstanJsonResultsParser->convertFromString($fileContents, $this->projectRoot);
+    }
 }
diff --git a/tests/resources/phpstan/phpstan-invalid-errors-not-strings.json b/tests/resources/phpstan/phpstan-invalid-errors-not-strings.json
@@ -0,0 +1,41 @@
+{
+  "totals": {
+    "errors": 0,
+    "file_errors": 3
+  },
+  "files": {
+    "/vagrant/static-analysis-baseliner/src/Domain/BaseLiner/BaseLineImporter.php": {
+      "errors": 1,
+      "messages": [
+        {
+          "message": "Parameter #1 $array of static method DaveLiddament\\StaticAnalysisResultsBaseliner\\Domain\\ResultsParser\\AnalysisResults::fromArray() expects int, array given.",
+          "line": 89,
+          "ignorable": true,
+          "relativePath": "Domain/BaseLiner/BaseLineImporter.php"
+        }
+      ]
+    },
+    "/vagrant/static-analysis-baseliner/src/Domain/ResultsParser/AnalysisResults.php": {
+      "errors": 2,
+      "messages": [
+        {
+          "message": "PHPDoc tag @param for parameter $array with type array is incompatible with native type int",
+          "line": 73,
+          "ignorable": true,
+          "relativePath": "Domain/ResultsParser/AnalysisResults.php"
+        },
+        {
+          "message": "Argument of an invalid type int supplied for foreach, only iterables are supported.",
+          "line": null,
+          "ignorable": true,
+          "relativePath": "Domain/ResultsParser/AnalysisResults.php"
+        }
+      ]
+    }
+  },
+  "errors": [
+    "Error 1",
+    1,
+    true
+  ]
+}
diff --git a/tests/resources/phpstan/phpstan-invalid-missing-errors.json b/tests/resources/phpstan/phpstan-invalid-missing-errors.json
@@ -0,0 +1,36 @@
+{
+  "totals": {
+    "errors": 0,
+    "file_errors": 3
+  },
+  "files": {
+    "/vagrant/static-analysis-baseliner/src/Domain/BaseLiner/BaseLineImporter.php": {
+      "errors": 1,
+      "messages": [
+        {
+          "message": "Parameter #1 $array of static method DaveLiddament\\StaticAnalysisResultsBaseliner\\Domain\\ResultsParser\\AnalysisResults::fromArray() expects int, array given.",
+          "line": 89,
+          "ignorable": true,
+          "relativePath": "Domain/BaseLiner/BaseLineImporter.php"
+        }
+      ]
+    },
+    "/vagrant/static-analysis-baseliner/src/Domain/ResultsParser/AnalysisResults.php": {
+      "errors": 2,
+      "messages": [
+        {
+          "message": "PHPDoc tag @param for parameter $array with type array is incompatible with native type int",
+          "line": 73,
+          "ignorable": true,
+          "relativePath": "Domain/ResultsParser/AnalysisResults.php"
+        },
+        {
+          "message": "Argument of an invalid type int supplied for foreach, only iterables are supported.",
+          "line": null,
+          "ignorable": true,
+          "relativePath": "Domain/ResultsParser/AnalysisResults.php"
+        }
+      ]
+    }
+  }
+}
diff --git a/tests/resources/phpstan/phpstan-with-errors.json b/tests/resources/phpstan/phpstan-with-errors.json
@@ -0,0 +1,40 @@
+{
+  "totals": {
+    "errors": 0,
+    "file_errors": 3
+  },
+  "files": {
+    "/vagrant/static-analysis-baseliner/src/Domain/BaseLiner/BaseLineImporter.php": {
+      "errors": 1,
+      "messages": [
+        {
+          "message": "Parameter #1 $array of static method DaveLiddament\\StaticAnalysisResultsBaseliner\\Domain\\ResultsParser\\AnalysisResults::fromArray() expects int, array given.",
+          "line": 89,
+          "ignorable": true,
+          "relativePath": "Domain/BaseLiner/BaseLineImporter.php"
+        }
+      ]
+    },
+    "/vagrant/static-analysis-baseliner/src/Domain/ResultsParser/AnalysisResults.php": {
+      "errors": 2,
+      "messages": [
+        {
+          "message": "PHPDoc tag @param for parameter $array with type array is incompatible with native type int",
+          "line": 73,
+          "ignorable": true,
+          "relativePath": "Domain/ResultsParser/AnalysisResults.php"
+        },
+        {
+          "message": "Argument of an invalid type int supplied for foreach, only iterables are supported.",
+          "line": null,
+          "ignorable": true,
+          "relativePath": "Domain/ResultsParser/AnalysisResults.php"
+        }
+      ]
+    }
+  },
+  "errors": [
+    "Error 1",
+    "Error 2"
+  ]
+}