Fix handling unordered baseline violations with ignore line numbers o…

…ption (#430)

* Test case for #428

* Handle unordered baseline violations

src/Rules/Violations.php

@@ -112,13 +112,30 @@ public function toArray(): array
      */
     public function remove(self $violations, bool $ignoreBaselineLinenumbers = false): void
     {
-        $comparisonFunction = [__CLASS__, $ignoreBaselineLinenumbers ? 'compareViolationsIgnoreLineNumber' : 'compareViolations'];
+        if (!$ignoreBaselineLinenumbers) {
+            $this->violations = array_values(array_udiff(
+                $this->violations,
+                $violations->violations,
+                [__CLASS__, 'compareViolations']
+            ));
+
+            return;
+        }
+
+        $baselineViolations = $violations->violations;
+        foreach ($this->violations as $idx => $violation) {
+            foreach ($baselineViolations as $baseIdx => $baselineViolation) {
+                if (
+                    $baselineViolation->getFqcn() === $violation->getFqcn()
+                    && $baselineViolation->getError() === $violation->getError()
+                ) {
+                    unset($this->violations[$idx], $baselineViolations[$baseIdx]);
+                    continue 2;
+                }
+            }
+        }
 
-        $this->violations = array_values(array_udiff(
-            $this->violations,
-            $violations->violations,
-            $comparisonFunction
-        ));
+        $this->violations = array_values($this->violations);
     }
 
     public function sort(): void

tests/E2E/Cli/CheckCommandTest.php

@@ -146,12 +146,26 @@ public function test_you_can_ignore_the_default_baseline(): void
         $this->assertCheckHasErrors($cmdTester);
     }
 
+    public function test_baseline_line_numbers_can_be_ignored(): void
+    {
+        $configFilePath = __DIR__.'/../_fixtures/configIgnoreBaselineLineNumbers.php';
+
+        // No errors when ignoring baseline line numbers
+        $cmdTester = $this->runCheck($configFilePath, null, __DIR__.'/../_fixtures/line_numbers/baseline.json', false, false, true);
+        $this->assertCheckHasSuccess($cmdTester);
+
+        // Errors when not ignoring baseline line numbers
+        $cmdTester = $this->runCheck($configFilePath, null, __DIR__.'/../_fixtures/line_numbers/baseline.json');
+        $this->assertCheckHasErrors($cmdTester);
+    }
+
     protected function runCheck(
         $configFilePath = null,
         ?bool $stopOnFailure = null,
         ?string $useBaseline = null,
         $generateBaseline = false,
-        bool $skipBaseline = false
+        bool $skipBaseline = false,
+        bool $ignoreBaselineNumbers = false
     ): ApplicationTester {
         $input = ['check'];
         if (null !== $configFilePath) {
@@ -167,6 +181,10 @@ protected function runCheck(
             $input['--skip-baseline'] = true;
         }
 
+        if ($ignoreBaselineNumbers) {
+            $input['--ignore-baseline-linenumbers'] = true;
+        }
+
         // false = option not set, null = option set but without value, string = option with value
         if (false !== $generateBaseline) {
             $input['--generate-baseline'] = $generateBaseline;

tests/E2E/_fixtures/configIgnoreBaselineLineNumbers.php

@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+use Arkitect\ClassSet;
+use Arkitect\CLI\Config;
+use Arkitect\Expression\ForClasses\DependsOnlyOnTheseNamespaces;
+use Arkitect\Expression\ForClasses\ResideInOneOfTheseNamespaces;
+use Arkitect\Rules\Rule;
+
+return static function (Config $config): void {
+    $rootPath = realpath(__DIR__);
+    $classSet = ClassSet::fromDir("$rootPath/line_numbers");
+
+    $rules = [
+        Rule::allClasses()
+            ->that(new ResideInOneOfTheseNamespaces('App\Application'))
+            ->should(new DependsOnlyOnTheseNamespaces('App\Application'))
+            ->because('That is how I want it'),
+    ];
+
+    $config->add($classSet, ...$rules);
+};

tests/E2E/_fixtures/line_numbers/Application/Foo.php

@@ -0,0 +1,20 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Application;
+
+final class Foo
+{
+    public function doSomething(\App\Ui\Baz $baz): void
+    {
+    }
+
+    public function doSomethingElse(\App\Ui\Bar $bar): void
+    {
+    }
+
+    public function doSomethingElseEvenMore(\App\Ui\Bar $bar): void
+    {
+    }
+}

tests/E2E/_fixtures/line_numbers/baseline.json

@@ -0,0 +1,20 @@
+{
+    "violations": [
+        {
+            "fqcn": "App\\Application\\Foo",
+            "line": 13,
+            "error": "depends on App\\Ui\\Bar, but should depend only on classes in one of these namespaces: App\\Application because That is how I want it"
+        },
+        {
+            "fqcn": "App\\Application\\Foo",
+            "line": 20,
+            "error": "depends on App\\Ui\\Baz, but should depend only on classes in one of these namespaces: App\\Application because That is how I want it"
+        },
+        {
+            "fqcn": "App\\Application\\Foo",
+            "line": 17,
+            "error": "depends on App\\Ui\\Bar, but should depend only on classes in one of these namespaces: App\\Application because That is how I want it"
+        }
+    ],
+    "stopOnFailure": false
+}

tests/Unit/Rules/ViolationsTest.php

@@ -187,11 +187,19 @@ public function test_remove_violations_from_violations_ignore_linenumber(): void
 
         $violation2 = new Violation(
             'App\Controller\Shop',
-            'should implement AbstractController'
+            'should implement AbstractController',
+            21
         );
         $this->violationStore->add($violation2);
 
-        $this->assertCount(3, $this->violationStore->toArray());
+        $violation3 = new Violation(
+            'App\Controller\Shop',
+            'should have name end with Controller',
+            5
+        );
+        $this->violationStore->add($violation3);
+
+        $this->assertCount(4, $this->violationStore->toArray());
 
         $violationsBaseline = new Violations();
         $violationsBaseline->add(new Violation(
@@ -202,10 +210,11 @@ public function test_remove_violations_from_violations_ignore_linenumber(): void
 
         $this->violationStore->remove($violationsBaseline, true);
 
-        $this->assertCount(2, $this->violationStore->toArray());
+        $this->assertCount(3, $this->violationStore->toArray());
         $this->assertEquals([
             $this->violation,
             $violation2,
+            $violation3,
         ], $this->violationStore->toArray());
     }
 }