Add baseline feature (#314)

pfazzi · fain182 · web-flow · commit 3f33637baa17 · 2022-12-08T17:09:20.000+01:00
* WIP add baseline feature

* Refactor test

* Baseline is now json

* Add docs in the readme file

* Improve UX

* Refactor

* rename --set-baseline in --generate-baseline

* generate baseline with default name

* fix style

* baseline with default filename is used automatically

* show which baseline is used

* adds --skip-baseline option

* README updated

Co-authored-by: Pietro Campagnano &lt;hello@pietro.camp&gt;
diff --git a/README.md b/README.md
@@ -58,6 +58,37 @@ phparkitect check --config=/project/yourConfigFile.php
 
 By default, a progress bar will show the status of the ongoing analysis.
 
+### Using a baseline file
+
+If there are a lot of violations in your codebase and you can't fix them now, 
+you can use the baseline feature to instruct the tool to ignore past violations.
+
+To create a baseline file, run the `check` command with the `generate-baseline` parameter as follows:
+
+```
+phparkitect check --generate-baseline
+```
+This will create a `phparkitect-baseline.json`, if you want a different file name you can do it with:
+```
+phparkitect check --generate-baseline=my-baseline.json
+```
+
+It will produce a json file with the current list of violations.  
+
+If is present a baseline file with the default name will be used automatically.
+
+To use a different baseline file, run the `check` command with the `use-baseline` parameter as follows:
+
+```
+phparkitect check --use-baseline=my-baseline.json
+```
+
+To avoid using the default baseline file, you can use the `skip-baseline` option:
+
+```
+phparkitect check --skip-baseline
+```
+
 ## Configuration
 
 Example of configuration file `phparkitect.php`
@@ -93,7 +124,6 @@ return static function (Config $config): void {
 };
 ```
 
-
 # Available rules
 
 **Hint**: If you want to test how a Rule work, you can use the command like `phparkitect debug:expression <RuleName> <arguments>` to check which class satisfy the rule in your current folder.
diff --git a/src/CLI/Command/Check.php b/src/CLI/Command/Check.php
@@ -21,15 +21,17 @@
 class Check extends Command
 {
     private const CONFIG_FILENAME_PARAM = 'config';
-
     private const TARGET_PHP_PARAM = 'target-php-version';
-
     private const STOP_ON_FAILURE_PARAM = 'stop-on-failure';
+    private const USE_BASELINE_PARAM = 'use-baseline';
+    private const SKIP_BASELINE_PARAM = 'skip-baseline';
 
-    private const DEFAULT_FILENAME = 'phparkitect.php';
+    private const GENERATE_BASELINE_PARAM = 'generate-baseline';
+    private const DEFAULT_RULES_FILENAME = 'phparkitect.php';
 
-    private const SUCCESS_CODE = 0;
+    private const DEFAULT_BASELINE_FILENAME = 'phparkitect-baseline.json';
 
+    private const SUCCESS_CODE = 0;
     private const ERROR_CODE = 1;
 
     public function __construct()
@@ -59,6 +61,25 @@ protected function configure(): void
                 's',
                 InputOption::VALUE_NONE,
                 'Stop on failure'
+            )
+            ->addOption(
+                self::GENERATE_BASELINE_PARAM,
+                'g',
+                InputOption::VALUE_OPTIONAL,
+                'Generate a file containing the current errors',
+                false
+            )
+            ->addOption(
+                self::USE_BASELINE_PARAM,
+                'b',
+                InputOption::VALUE_REQUIRED,
+                'Ignore errors in baseline file'
+            )
+            ->addOption(
+                self::SKIP_BASELINE_PARAM,
+                'k',
+                InputOption::VALUE_NONE,
+                'Don\'t use the default baseline'
             );
     }
 
@@ -71,6 +92,21 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         try {
             $verbose = $input->getOption('verbose');
             $stopOnFailure = $input->getOption(self::STOP_ON_FAILURE_PARAM);
+            $useBaseline = $input->getOption(self::USE_BASELINE_PARAM);
+            $skipBaseline = $input->getOption(self::SKIP_BASELINE_PARAM);
+
+            if (true !== $skipBaseline && !$useBaseline && file_exists(self::DEFAULT_BASELINE_FILENAME)) {
+                $useBaseline = self::DEFAULT_BASELINE_FILENAME;
+            }
+
+            if ($useBaseline && !file_exists($useBaseline)) {
+                $output->writeln('<error>Baseline file not found.</error>');
+
+                return self::ERROR_CODE;
+            }
+            $output->writeln('<info>Baseline found: '.$useBaseline.'</info>');
+
+            $generateBaseline = $input->getOption(self::GENERATE_BASELINE_PARAM);
 
             /** @var string|null $phpVersion */
             $phpVersion = $input->getOption('target-php-version');
@@ -93,6 +129,25 @@ protected function execute(InputInterface $input, OutputInterface $output): int
             } catch (FailOnFirstViolationException $e) {
             }
             $violations = $runner->getViolations();
+
+            if (false !== $generateBaseline) {
+                if (null === $generateBaseline) {
+                    $generateBaseline = self::DEFAULT_BASELINE_FILENAME;
+                }
+                $this->saveBaseline($generateBaseline, $violations);
+
+                $output->writeln('<info>Baseline file \''.$generateBaseline.'\'created!</info>');
+                $this->printExecutionTime($output, $startTime);
+
+                return self::SUCCESS_CODE;
+            }
+
+            if ($useBaseline) {
+                $baseline = $this->loadBaseline($useBaseline);
+
+                $violations->remove($baseline);
+            }
+
             if ($violations->count() > 0) {
                 $this->printViolations($violations, $output);
                 $this->printExecutionTime($output, $startTime);
@@ -149,12 +204,22 @@ protected function printExecutionTime(OutputInterface $output, float $startTime)
         $output->writeln('<info>Execution time: '.$executionTime."s</info>\n");
     }
 
+    private function loadBaseline(string $filename): Violations
+    {
+        return Violations::fromJson(file_get_contents($filename));
+    }
+
+    private function saveBaseline(string $filename, Violations $violations): void
+    {
+        file_put_contents($filename, json_encode($violations, \JSON_PRETTY_PRINT));
+    }
+
     private function getConfigFilename(InputInterface $input): string
     {
         $filename = $input->getOption(self::CONFIG_FILENAME_PARAM);
 
         if (null === $filename) {
-            $filename = self::DEFAULT_FILENAME;
+            $filename = self::DEFAULT_RULES_FILENAME;
         }
 
         Assert::file($filename, 'Config file not found');
diff --git a/src/Rules/Violation.php b/src/Rules/Violation.php
@@ -4,7 +4,7 @@
 
 namespace Arkitect\Rules;
 
-class Violation
+class Violation implements \JsonSerializable
 {
     /** @var string */
     private $fqcn;
@@ -46,4 +46,14 @@ public function getLine(): ?int
     {
         return $this->line;
     }
+
+    public function jsonSerialize(): array
+    {
+        return get_object_vars($this);
+    }
+
+    public static function fromJson(array $json): self
+    {
+        return new self($json['fqcn'], $json['error'], $json['line']);
+    }
 }
diff --git a/src/Rules/Violations.php b/src/Rules/Violations.php
@@ -7,7 +7,7 @@
 use Arkitect\Exceptions\FailOnFirstViolationException;
 use Arkitect\Exceptions\IndexNotFoundException;
 
-class Violations implements \IteratorAggregate, \Countable
+class Violations implements \IteratorAggregate, \Countable, \JsonSerializable
 {
     /**
      * @var Violation[]
@@ -24,6 +24,19 @@ public function __construct(bool $stopOnFailure = false)
         $this->stopOnFailure = $stopOnFailure;
     }
 
+    public static function fromJson(string $json): self
+    {
+        $json = json_decode($json, true);
+
+        $instance = new self($json['stopOnFailure']);
+
+        $instance->violations = array_map(function (array $json): Violation {
+            return Violation::fromJson($json);
+        }, $json['violations']);
+
+        return $instance;
+    }
+
     public function add(Violation $violation): void
     {
         $this->violations[] = $violation;
@@ -92,4 +105,20 @@ public function toArray(): array
     {
         return $this->violations;
     }
+
+    public function remove(self $violations): void
+    {
+        $this->violations = array_udiff(
+            $this->violations,
+            $violations->violations,
+            function (Violation $a, Violation $b): int {
+                return $a == $b ? 0 : 1;
+            }
+        );
+    }
+
+    public function jsonSerialize(): array
+    {
+        return get_object_vars($this);
+    }
 }
diff --git a/tests/E2E/Cli/CheckCommandTest.php b/tests/E2E/Cli/CheckCommandTest.php
@@ -14,6 +14,20 @@ class CheckCommandTest extends TestCase
 
     const ERROR_CODE = 1;
 
+    /** @var string */
+    private $customBaselineFilename = __DIR__.'/my-baseline.json';
+    private $defaultBaselineFilename = 'phparkitect-baseline.json';
+
+    protected function tearDown(): void
+    {
+        if (file_exists($this->customBaselineFilename)) {
+            unlink($this->customBaselineFilename);
+        }
+        if (file_exists($this->defaultBaselineFilename)) {
+            unlink($this->defaultBaselineFilename);
+        }
+    }
+
     public function test_app_returns_error_with_multiple_violations(): void
     {
         $cmdTester = $this->runCheck(__DIR__.'/../_fixtures/configMvc.php');
@@ -86,15 +100,77 @@ public function test_bug_yield(): void
         $this->assertCheckHasErrors($cmdTester, $expectedErrors);
     }
 
-    protected function runCheck($configFilePath = null, bool $stopOnFailure = null): ApplicationTester
+    public function test_baseline(): void
+    {
+        $configFilePath = __DIR__.'/../_fixtures/configMvcForYieldBug.php';
+
+        // Produce the baseline
+
+        $this->runCheck($configFilePath, null, null, $this->customBaselineFilename);
+
+        // Check it detects error if baseline is not used
+
+        $cmdTester = $this->runCheck($configFilePath, null, null);
+
+        $this->assertCheckHasErrors($cmdTester);
+
+        // Check it ignores error if baseline is used
+
+        $cmdTester = $this->runCheck($configFilePath, null, $this->customBaselineFilename);
+        $this->assertCheckHasSuccess($cmdTester);
+    }
+
+    public function test_baseline_with_default_filename_is_enabled_automatically(): void
     {
+        $configFilePath = __DIR__.'/../_fixtures/configMvcForYieldBug.php';
+
+        // Produce the baseline
+
+        $this->runCheck($configFilePath, null, null, null);
+
+        // Check it ignores error if baseline is used
+
+        $cmdTester = $this->runCheck($configFilePath, null, null);
+        $this->assertCheckHasSuccess($cmdTester);
+    }
+
+    public function test_you_can_ignore_the_default_baseline(): void
+    {
+        $configFilePath = __DIR__.'/../_fixtures/configMvcForYieldBug.php';
+
+        // Produce the baseline
+        $this->runCheck($configFilePath, null, null, null);
+
+        // Check it ignores the default baseline
+        $cmdTester = $this->runCheck($configFilePath, null, null, false, true);
+        $this->assertCheckHasErrors($cmdTester);
+    }
+
+    protected function runCheck(
+        $configFilePath = null,
+        bool $stopOnFailure = null,
+        ?string $useBaseline = null,
+        $generateBaseline = false,
+        bool $skipBaseline = false
+    ): ApplicationTester {
         $input = ['check'];
         if (null !== $configFilePath) {
             $input['--config'] = $configFilePath;
         }
         if (null !== $stopOnFailure) {
             $input['--stop-on-failure'] = true;
         }
+        if (null !== $useBaseline) {
+            $input['--use-baseline'] = $useBaseline;
+        }
+        if ($skipBaseline) {
+            $input['--skip-baseline'] = true;
+        }
+
+        // false = option not set, null = option set but without value, string = option with value
+        if (false !== $generateBaseline) {
+            $input['--generate-baseline'] = $generateBaseline;
+        }
 
         $app = new PhpArkitectApplication();
         $app->setAutoExit(false);
@@ -127,7 +203,7 @@ protected function assertCheckHasNoErrorsLike(ApplicationTester $applicationTest
 
     protected function assertCheckHasSuccess(ApplicationTester $applicationTester): void
     {
-        $this->assertEquals(self::SUCCESS_CODE, $applicationTester->getStatusCode());
-        $this->assertStringNotContainsString('ERRORS!', $applicationTester->getDisplay());
+        $this->assertEquals(self::SUCCESS_CODE, $applicationTester->getStatusCode(), 'Command failed: '.$applicationTester->getDisplay());
+        $this->assertStringNotContainsString('ERRORS!', $applicationTester->getDisplay(), 'Error message not expected in successful execution');
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -4,7 +4,7 @@`
`4`	`4`
`5`	`5`	`namespace Arkitect\Rules;`
`6`	`6`
`7`		`-class Violation`
	`7`	`+class Violation implements \JsonSerializable`
`8`	`8`	`{`
`9`	`9`	`/** @var string */`
`10`	`10`	`private $fqcn;`
`@@ -46,4 +46,14 @@ public function getLine(): ?int`
`46`	`46`	`{`
`47`	`47`	`return $this->line;`
`48`	`48`	`}`
	`49`	`+`
	`50`	`+ public function jsonSerialize(): array`
	`51`	`+ {`
	`52`	`+ return get_object_vars($this);`
	`53`	`+ }`
	`54`	`+`
	`55`	`+ public static function fromJson(array $json): self`
	`56`	`+ {`
	`57`	`+ return new self($json['fqcn'], $json['error'], $json['line']);`
	`58`	`+ }`
`49`	`59`	`}`