sort violations to avoid unnecessary diffs in the baseline (#364)

phparkitect · Mar 14, 2023 · 3fbbcd3 · 3fbbcd3
1 parent 19aecb5
commit 3fbbcd3
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 6 deletions.
diff --git a/src/CLI/Command/Check.php b/src/CLI/Command/Check.php
@@ -129,6 +129,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
             } catch (FailOnFirstViolationException $e) {
             }
             $violations = $runner->getViolations();
+            $violations->sort();
 
             if (false !== $generateBaseline) {
                 if (null === $generateBaseline) {

diff --git a/src/Rules/Violation.php b/src/Rules/Violation.php
@@ -9,12 +9,12 @@ class Violation implements \JsonSerializable
     /** @var string */
     private $fqcn;
 
-    /** @var string */
-    private $error;
-
     /** @var int|null */
     private $line;
 
+    /** @var string */
+    private $error;
+
     public function __construct(string $fqcn, string $error, ?int $line = null)
     {
         $this->fqcn = $fqcn;

diff --git a/src/Rules/Violations.php b/src/Rules/Violations.php
@@ -117,6 +117,13 @@ static function (Violation $a, Violation $b): int {
         ));
     }
 
+    public function sort(): void
+    {
+        usort($this->violations, static function (Violation $v1, Violation $v2): int {
+            return $v1 <=> $v2;
+        });
+    }
+
     public function jsonSerialize(): array
     {
         return get_object_vars($this);

diff --git a/tests/E2E/Cli/CheckCommandTest.php b/tests/E2E/Cli/CheckCommandTest.php
@@ -35,8 +35,8 @@ public function test_app_returns_error_with_multiple_violations(): void
         $expectedErrors = 'ERRORS!
 
 App\Controller\Foo has 2 violations
-  should implement ContainerAwareInterface because all controllers should be container aware
   should have a name that matches *Controller because we want uniform naming
+  should implement ContainerAwareInterface because all controllers should be container aware
 
 App\Controller\ProductsController has 1 violations
   should implement ContainerAwareInterface because all controllers should be container aware

diff --git a/tests/E2E/Smoke/RunArkitectBinTest.php b/tests/E2E/Smoke/RunArkitectBinTest.php
@@ -23,8 +23,8 @@ public function test_returns_error_with_multiple_violations(): void
         $expectedErrors = 'ERRORS!
 
 App\Controller\Foo has 2 violations
-  should implement ContainerAwareInterface because all controllers should be container aware
   should have a name that matches *Controller because we want uniform naming
+  should implement ContainerAwareInterface because all controllers should be container aware
 
 App\Controller\ProductsController has 1 violations
   should implement ContainerAwareInterface because all controllers should be container aware
@@ -50,8 +50,8 @@ public function test_returns_error_with_multiple_violations_without_passing_conf
         $expectedErrors = 'ERRORS!
 
 App\Controller\Foo has 2 violations
-  should implement ContainerAwareInterface because all controllers should be container aware
   should have a name that matches *Controller because we want uniform naming
+  should implement ContainerAwareInterface because all controllers should be container aware
 
 App\Controller\ProductsController has 1 violations
   should implement ContainerAwareInterface because all controllers should be container aware

diff --git a/tests/Unit/Rules/ViolationsTest.php b/tests/Unit/Rules/ViolationsTest.php
@@ -135,4 +135,49 @@ public function test_remove_violations_from_violations(): void
             $violation2,
         ], $this->violationStore->toArray());
     }
+
+    public function test_sort(): void
+    {
+        $violationStore = new Violations();
+        $violation1 = new Violation(
+            'App\Controller\Shop',
+            'AAA',
+            20
+        );
+        $violation2 = new Violation(
+            'App\Controller\Shop',
+            'BBB',
+            10
+        );
+        $violation3 = new Violation(
+            'App\Controller\Shop',
+            'AAA',
+            10
+        );
+        $violation4 = new Violation(
+            'App\Controller\Abc',
+            'CCC',
+            30
+        );
+        $violationStore->add($violation1);
+        $violationStore->add($violation2);
+        $violationStore->add($violation3);
+        $violationStore->add($violation4);
+
+        $this->assertEquals([
+            $violation1,
+            $violation2,
+            $violation3,
+            $violation4,
+        ], $violationStore->toArray());
+
+        $violationStore->sort();
+
+        $this->assertSame([
+            $violation4, // fqcn is most important
+            $violation3, // then line number
+            $violation2, // then error message
+            $violation1,
+        ], $violationStore->toArray());
+    }
 }