Merge pull request #124 from DaveLiddament/feature/add-severity

Feature/add severity

README.md

@@ -171,9 +171,19 @@ That's no problem there are 3 methods to [integrate a static analysis tool](docs
 The format for showing issues after the baseline is removed can be specified using `--output-format` option. 
 Possible values are: `table`, `text`, `json` or `github` (for Github actions).
 
+## Ignoring warnings
+
+Some static analysis tools (e.g. PHP Code Sniffer) classify issues wth a severity or either `error` or `warning`.
+By default, SARB will report all of these. If you wish to ignore warnings you can use the `--ignore-warnings` option.
+
+E.g.
+```shell
+vendor/bin/phpcs src --report=json | vendor/bin/sarb remove phpcs.baseline --ignore-warnings
+```
+
 ## SARB with Github Actions
 
-If you're using `actions/checkout@v2` to checkout your code you'll need to add set `fetch-depth` to `0`.
+If you're using `actions/checkout@v2` to check out your code you'll need to add set `fetch-depth` to `0`.
 By default `checkout` only gets that latest state of the code and none of the history. 
 SARB uses git, which needs the full git history, to track file changes since the baseline. 
 To get the full history checked out use this:

docs/NewResultsParser.md

@@ -180,18 +180,22 @@ A valid implementation to do this would be this...
                 $lineAsInt = ArrayUtils::getIntValue($analysisResultAsArray, 'line_from');
                 $typeAsString = ArrayUtils::getStringValue($analysisResultAsArray, 'type');
                 $message = ArrayUtils::getStringValue($analysisResultAsArray, 'message');
+                $severityAsString = ArrayUtils::getStringValue($analysisResultAsArray, 'severity');
 
                 $location = Location::fromAbsoluteFileName(
                     new AbsoluteFileName($fileNameAsString),
                     $projectRoot,
                     new LineNumber($lineAsInt)
                 );
 
+                $severity = ($severityAsString === 'error') ? Severity::error() : Severity::warning();
+
                 $analysisResult =  new AnalysisResult(
                     $location,
                     new Type($typeAsString),
                     $message,
-                    $analysisResultAsArray
+                    $analysisResultAsArray,
+                    $severity
                 );
 
                 $analysisResults->addAnalysisResult($analysisResult);
@@ -215,7 +219,7 @@ We can use SARB's `JsonUtils::toArray` method. This takes a string and returns a
 NOTE: If the file provided is not a JSON representation then `convertFromString` must throw an `InvalidContentTypeException`.
 SARB catches this and asks the user if they submitted the correct file.
 
-The that does this is:
+The code that does this is:
 
 ```php
     try {
@@ -276,19 +280,21 @@ SARB needs to pull out:
  - line number (`line_from` in Psalm's JSON output)
  - type (`type` in Psalm's JSON output)
  - message (`message` in Psalm's JSON output)
+ - severity (`severity` in Psalm's JSON output). NOTE: Report a severity of `error` if there is no concept of severity.
 
 **NOTES:** 
 
 1. `Type` must refer to the type of violation (e.g. `MissingConstructor`). See more about this at [How SARB works](HowSarbWorks.md)
 1. Ideally the file path should be the absolute path. SARB stores the relative path in the baseline file, but the HistoryAnalyser needs the absolute path. If the static analysis tool does not provide an absoute path then a relative path can be used, see [using a relative path](#using-relative-paths).
-
-Here is a the code to pull the information from the array:
+2. 
+Here is the code to pull the information from the array:
 
 ```php
                 $fileNameAsString = ArrayUtils::getStringValue($analysisResultAsArray, 'file_path');
                 $lineAsInt = ArrayUtils::getIntValue($analysisResultAsArray, 'line_from');
                 $typeAsString = ArrayUtils::getStringValue($analysisResultAsArray, 'type');
                 $message = ArrayUtils::getStringValue($analysisResultAsArray, 'message');
+                $severityAsString = ArrayUtils::getStringValue($analysisResultAsArray, 'severity');
 ```
 
 The final piece of information that SARB takes is an array containing all the data from the tool about the particular violation. 
@@ -303,12 +309,15 @@ SARB needs to capture all ths information and create an `AnalysisResult`.
                     $projectRoot,
                     new LineNumber($lineAsInt)
                 );
+
+                $severity = ($severityAsString === 'error') ? Severity::error() : Severity::warning();
 
                 $analysisResult =  new AnalysisResult(
                     $location,
                     new Type($typeAsString),
                     $message,
-                    $analysisResultAsArray
+                    $analysisResultAsArray,
+                    $severity
                 );
 ```
 

docs/SarbFormat.md

@@ -12,7 +12,7 @@ There are 2 options:
 - Write a [Results Parser](NewResultsParser.md) that translates the tool's output into something SARB understands
 - Make the tool output data in the `sarb.json` format
 
-In most cases it is probably easier to add support for outputting the `sarb.json` to the static analysis tool rather then writing a Results Parser.
+In most cases it is probably easier to add support for outputting the `sarb.json` to the static analysis tool rather than writing a Results Parser.
 
 
 ## Format
@@ -23,6 +23,7 @@ The following information is needed for each issue the static analysis tool find
 - Line number of the issue. [integer]
 - Type (e.g. `MixedType`). [string]
 - Message (e.g. `Can not assign $asArray to a mixed type`). [string]
+- Severity either `error` or `warning`. This is optional, if not supplied then `error` is assumed. [string]
 
 It is very important `Type` must not have file name, class name, function or line number in it. 
 
@@ -33,7 +34,8 @@ It is very important `Type` must not have file name, class name, function or lin
     "file": "/home/johnsmith/project/Controller/HomeController.php",
     "line": 10,
     "type": "MixedType",
-    "message" : "Cannot assign $asArray to a mixed type"
+    "message" : "Cannot assign $asArray to a mixed type",
+    "severity": "error"
   },
   ... repeat for each issue ...   
 ]

src/Domain/Analyser/BaseLineResultsRemover.php

@@ -28,12 +28,16 @@ class BaseLineResultsRemover
     public function pruneBaseLine(
         AnalysisResults $latestAnalysisResults,
         HistoryAnalyser $historyAnalyser,
-        BaseLineAnalysisResults $baseLineAnalysisResults
+        BaseLineAnalysisResults $baseLineAnalysisResults,
+        bool $ignoreWarnings
     ): AnalysisResults {
         $prunedAnalysisResultsBuilder = new AnalysisResultsBuilder();
         $baseLineResultsComparator = new BaseLineResultsComparator($baseLineAnalysisResults);
 
         foreach ($latestAnalysisResults->getAnalysisResults() as $analysisResult) {
+            if ($analysisResult->getSeverity()->isWarning() && $ignoreWarnings) {
+                continue;
+            }
             if (!$this->isInHistoricResults($analysisResult, $baseLineResultsComparator, $historyAnalyser)) {
                 $prunedAnalysisResultsBuilder->addAnalysisResult($analysisResult);
             }

src/Domain/BaseLiner/BaseLineAnalysisResult.php

@@ -7,6 +7,7 @@
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Common\LineNumber;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Common\PreviousLocation;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Common\RelativeFileName;
+use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Common\Severity;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Common\Type;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Utils\ArrayParseException;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Utils\ArrayUtils;
@@ -17,6 +18,7 @@ class BaseLineAnalysisResult
     private const FILE_NAME = 'fileName';
     private const TYPE = 'type';
     private const MESSAGE = 'message';
+    private const SEVERITY = 'severity';
 
     /**
      * @var RelativeFileName
@@ -34,6 +36,10 @@ class BaseLineAnalysisResult
      * @var string
      */
     private $message;
+    /**
+     * @var Severity
+     */
+    private $severity;
 
     /**
      * @psalm-param array<mixed> $array
@@ -45,26 +51,34 @@ public static function fromArray(array $array): self
         $lineNumber = new LineNumber(ArrayUtils::getIntValue($array, self::LINE_NUMBER));
         $fileName = new RelativeFileName(ArrayUtils::getStringValue($array, self::FILE_NAME));
         $type = new Type(ArrayUtils::getStringValue($array, self::TYPE));
+        $severity = Severity::fromStringOrNull(ArrayUtils::getOptionalStringValue($array, self::SEVERITY));
         $message = ArrayUtils::getStringValue($array, self::MESSAGE);
 
-        return new self($fileName, $lineNumber, $type, $message);
+        return new self($fileName, $lineNumber, $type, $message, $severity);
     }
 
     public static function make(
         RelativeFileName $fileName,
         LineNumber $lineNumber,
         Type $type,
-        string $message
+        string $message,
+        Severity $severity
     ): self {
-        return new self($fileName, $lineNumber, $type, $message);
+        return new self($fileName, $lineNumber, $type, $message, $severity);
     }
 
-    private function __construct(RelativeFileName $fileName, LineNumber $lineNumber, Type $type, string $message)
-    {
+    private function __construct(
+        RelativeFileName $fileName,
+        LineNumber $lineNumber,
+        Type $type,
+        string $message,
+        Severity $severity
+    ) {
         $this->fileName = $fileName;
         $this->lineNumber = $lineNumber;
         $this->type = $type;
         $this->message = $message;
+        $this->severity = $severity;
     }
 
     public function getFileName(): RelativeFileName
@@ -87,6 +101,11 @@ public function getMessage(): string
         return $this->message;
     }
 
+    public function getSeverity(): Severity
+    {
+        return $this->severity;
+    }
+
     /**
      * @psalm-return array<string,string|int>
      */
@@ -97,11 +116,12 @@ public function asArray(): array
             self::FILE_NAME => $this->getFileName()->getFileName(),
             self::TYPE => $this->type->getType(),
             self::MESSAGE => $this->message,
+            self::SEVERITY => $this->severity->getSeverity(),
         ];
     }
 
     /**
-     * Return true if this matches matches given FileName, LineNumber and type.
+     * Return true if this matches given FileName, LineNumber and type.
      */
     public function isMatch(PreviousLocation $location, Type $type): bool
     {

src/Domain/Common/Severity.php

@@ -0,0 +1,58 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Common;
+
+use Webmozart\Assert\Assert;
+
+class Severity
+{
+    public const WARNING = 'warning';
+    public const ERROR = 'error';
+
+    /**
+     * @var string
+     */
+    private $severity;
+
+    public static function fromStringOrNull(?string $severity): self
+    {
+        if (null === $severity) {
+            return new self(self::ERROR);
+        }
+
+        return new self($severity);
+    }
+
+    public static function error(): self
+    {
+        return new self(self::ERROR);
+    }
+
+    public static function warning(): self
+    {
+        return new self(self::WARNING);
+    }
+
+    private function __construct(string $severity)
+    {
+        Assert::true(self::isValueValid($severity), "Invalid severity: $severity");
+        $this->severity = $severity;
+    }
+
+    public function getSeverity(): string
+    {
+        return $this->severity;
+    }
+
+    public static function isValueValid(string $severity): bool
+    {
+        return in_array($severity, [self::ERROR, self::WARNING], true);
+    }
+
+    public function isWarning(): bool
+    {
+        return self::WARNING === $this->severity;
+    }
+}

src/Domain/Pruner/ResultsPruner.php

@@ -48,7 +48,8 @@ public function __construct(
     public function getPrunedResults(
         BaseLineFileName $baseLineFileName,
         string $analysisResults,
-        ProjectRoot $projectRoot
+        ProjectRoot $projectRoot,
+        bool $ignoreWarnings
     ): PrunedResults {
         $baseLine = $this->baseLineImporter->import($baseLineFileName);
         $resultsParser = $baseLine->getResultsParser();
@@ -60,7 +61,8 @@ public function getPrunedResults(
         $outputAnalysisResults = $this->baseLineResultsRemover->pruneBaseLine(
             $inputAnalysisResults,
             $historyAnalyser,
-            $baseLine->getAnalysisResults()
+            $baseLine->getAnalysisResults(),
+            $ignoreWarnings
         );
 
         return new PrunedResults($baseLine, $outputAnalysisResults, $inputAnalysisResults);

src/Domain/Pruner/ResultsPrunerInterface.php

@@ -15,6 +15,7 @@ interface ResultsPrunerInterface
     public function getPrunedResults(
         BaseLineFileName $baseLineFileName,
         string $analysisResults,
-        ProjectRoot $projectRoot
+        ProjectRoot $projectRoot,
+        bool $ignoreWarnings
     ): PrunedResults;
 }

src/Domain/ResultsParser/AnalysisResult.php

@@ -14,6 +14,7 @@
 
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\BaseLiner\BaseLineAnalysisResult;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Common\Location;
+use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Common\Severity;
 use DaveLiddament\StaticAnalysisResultsBaseliner\Domain\Common\Type;
 
 /**
@@ -42,6 +43,11 @@ class AnalysisResult
      */
     private $fullDetails;
 
+    /**
+     * @var Severity
+     */
+    private $severity;
+
     /**
      * AnalysisResult constructor.
      *
@@ -52,12 +58,13 @@ class AnalysisResult
      *
      * @psalm-param array<mixed> $fullDetails
      */
-    public function __construct(Location $location, Type $type, string $message, array $fullDetails)
+    public function __construct(Location $location, Type $type, string $message, array $fullDetails, Severity $severity)
     {
         $this->location = $location;
         $this->type = $type;
         $this->message = $message;
         $this->fullDetails = $fullDetails;
+        $this->severity = $severity;
     }
 
     public function getLocation(): Location
@@ -83,13 +90,19 @@ public function getMessage(): string
         return $this->message;
     }
 
+    public function getSeverity(): Severity
+    {
+        return $this->severity;
+    }
+
     public function asBaseLineAnalysisResult(): BaseLineAnalysisResult
     {
         return BaseLineAnalysisResult::make(
             $this->location->getRelativeFileName(),
             $this->location->getLineNumber(),
             $this->type,
-            $this->message
+            $this->message,
+            $this->severity
         );
     }
 }

src/Domain/Utils/ArrayParseException.php

@@ -27,4 +27,11 @@ public static function invalidType(string $key, string $expectedType): self
 
         return new self($message);
     }
+
+    public static function invalidValue(string $key, string $value): self
+    {
+        $message = "Value [$value] for [$key] is not valid";
+
+        return new self($message);
+    }
 }