Indent violations output

Lendable · Oct 1, 2023 · 7939e6d · 7939e6d
1 parent 5cc1d6c
commit 7939e6d
Show file tree

Hide file tree

Showing 8 changed files with 95 additions and 87 deletions.
diff --git a/src/Rules/Violations.php b/src/Rules/Violations.php
@@ -6,6 +6,7 @@
 
 use Arkitect\Exceptions\FailOnFirstViolationException;
 use Arkitect\Exceptions\IndexNotFoundException;
+use Arkitect\Shared\String\IndentationHelper;
 
 class Violations implements \IteratorAggregate, \Countable, \JsonSerializable
 {
@@ -86,20 +87,22 @@ public function toString(): string
          */
         foreach ($violationsCollection as $key => $violationsByFqcn) {
             $violationForThisFqcn = \count($violationsByFqcn);
-            $errors .= "\n$key has {$violationForThisFqcn} violations";
+            $errors .= "\n$key has {$violationForThisFqcn} violations\n";
 
+            $violationDescription = '';
             foreach ($violationsByFqcn as $violation) {
-                $errors .= "\n";
-                $errors .= $violation->getError();
+                $violationDescription .= "\n";
+                $violationDescription .= $violation->getError();
 
                 if (null !== $violation->getLine()) {
-                    $errors .= ' (on line '.$violation->getLine().')';
+                    $violationDescription .= ' (on line '.$violation->getLine().')';
                 }
-                $errors .= "\n";
+                $violationDescription .= "\n";
             }
+            $errors .= IndentationHelper::indent(trim($violationDescription))."\n";
         }
 
-        return $errors;
+        return IndentationHelper::clearEmptyLines($errors);
     }
 
     public function toArray(): array

diff --git a/src/Shared/String/IndentationHelper.php b/src/Shared/String/IndentationHelper.php
@@ -10,4 +10,9 @@ public static function indent(string $text, int $spaces = 2): string
     {
         return preg_replace('/^/m', str_repeat(' ', $spaces), $text);
     }
+
+    public static function clearEmptyLines(string $text): string
+    {
+        return preg_replace('/^\s+$/m', '', $text);
+    }
 }
diff --git a/tests/E2E/Cli/CheckCommandTest.php b/tests/E2E/Cli/CheckCommandTest.php
@@ -35,34 +35,34 @@ public function test_app_returns_error_with_multiple_violations(): void
         $expectedErrors = 'ERRORS!
 
 App\Controller\Foo has 2 violations
-should have a name that matches *Controller
-because we want uniform naming
+  should have a name that matches *Controller
+  because we want uniform naming
 
-should implement ContainerAwareInterface
-because all controllers should be container aware
+  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
+  should implement ContainerAwareInterface
+  because all controllers should be container aware
 
 App\Controller\UserController has 1 violations
-should implement ContainerAwareInterface
-because all controllers should be container aware
+  should implement ContainerAwareInterface
+  because all controllers should be container aware
 
 App\Controller\YieldController has 1 violations
-should implement ContainerAwareInterface
-because all controllers should be container aware
+  should implement ContainerAwareInterface
+  because all controllers should be container aware
 
 App\Domain\Model has 2 violations
-depends on App\Services\UserService
-from the rule
-should not depend on classes outside namespace App\Domain
-because we want protect our domain (on line 14)
+  depends on App\Services\UserService
+  from the rule
+  should not depend on classes outside namespace App\Domain
+  because we want protect our domain (on line 14)
 
-depends on App\Services\CartService
-from the rule
-should not depend on classes outside namespace App\Domain
-because we want protect our domain (on line 15)';
+  depends on App\Services\CartService
+  from the rule
+  should not depend on classes outside namespace App\Domain
+  because we want protect our domain (on line 15)';
 
         $this->assertCheckHasErrors($cmdTester, $expectedErrors);
     }
@@ -73,8 +73,8 @@ public function test_app_returns_single_error_because_there_is_stop_on_failure_p
 
         $expectedErrors = '
 App\Controller\Foo has 1 violations
-should implement ContainerAwareInterface
-because all controllers should be container aware';
+  should implement ContainerAwareInterface
+  because all controllers should be container aware';
 
         $this->assertCheckHasErrors($cmdTester, $expectedErrors);
         $this->assertCheckHasNoErrorsLike($cmdTester, "App\Controller\ProductsController has 1 violations");
@@ -109,7 +109,7 @@ public function test_bug_yield(): void
         $expectedErrors = 'ERRORS!
 
 App\Controller\Foo has 1 violations
-should have a name that matches *Controller';
+  should have a name that matches *Controller';
 
         $this->assertCheckHasErrors($cmdTester, $expectedErrors);
     }

diff --git a/tests/E2E/PHPUnit/CheckClassHaveAttributeTest.php b/tests/E2E/PHPUnit/CheckClassHaveAttributeTest.php
@@ -39,8 +39,8 @@ public function test_controllers_should_have_name_ending_in_controller(): void
 
         $expectedExceptionMessage = '
 App\Controller\Foo has 1 violations
-should have a name that matches *Controller
-because its a symfony thing';
+  should have a name that matches *Controller
+  because its a symfony thing';
 
         $this->expectException(ExpectationFailedException::class);
         $this->expectExceptionMessage($expectedExceptionMessage);

diff --git a/tests/E2E/PHPUnit/CheckClassImplementInterfaceTest.php b/tests/E2E/PHPUnit/CheckClassImplementInterfaceTest.php
@@ -26,24 +26,24 @@ public function test_assertion_should_fail_on_broken_rule(): void
 
         $expectedExceptionMessage = '
 App\Controller\Foo has 1 violations
-should implement ContainerAwareInterface
-because i said so
+  should implement ContainerAwareInterface
+  because i said so
 
 App\Controller\ProductsController has 1 violations
-should implement ContainerAwareInterface
-because i said so
+  should implement ContainerAwareInterface
+  because i said so
 
 App\Controller\UserController has 1 violations
-should implement ContainerAwareInterface
-because i said so
+  should implement ContainerAwareInterface
+  because i said so
 
 App\Controller\YieldController has 1 violations
-should implement ContainerAwareInterface
-because i said so
+  should implement ContainerAwareInterface
+  because i said so
 
 App\Services\UserService has 1 violations
-should implement ContainerAwareInterface
-because i said so';
+  should implement ContainerAwareInterface
+  because i said so';
 
         $this->expectException(ExpectationFailedException::class);
         $this->expectExceptionMessage($expectedExceptionMessage);

diff --git a/tests/E2E/Smoke/RunArkitectBinTest.php b/tests/E2E/Smoke/RunArkitectBinTest.php
@@ -23,34 +23,34 @@ public function test_returns_error_with_multiple_violations(): void
         $expectedErrors = 'ERRORS!
 
 App\Controller\Foo has 2 violations
-should have a name that matches *Controller
-because we want uniform naming
+  should have a name that matches *Controller
+  because we want uniform naming
 
-should implement ContainerAwareInterface
-because all controllers should be container aware
+  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
+  should implement ContainerAwareInterface
+  because all controllers should be container aware
 
 App\Controller\UserController has 1 violations
-should implement ContainerAwareInterface
-because all controllers should be container aware
+  should implement ContainerAwareInterface
+  because all controllers should be container aware
 
 App\Controller\YieldController has 1 violations
-should implement ContainerAwareInterface
-because all controllers should be container aware
+  should implement ContainerAwareInterface
+  because all controllers should be container aware
 
 App\Domain\Model has 2 violations
-depends on App\Services\UserService
-from the rule
-should not depend on classes outside namespace App\Domain
-because we want protect our domain (on line 14)
-
-depends on App\Services\CartService
-from the rule
-should not depend on classes outside namespace App\Domain
-because we want protect our domain (on line 15)
+  depends on App\Services\UserService
+  from the rule
+  should not depend on classes outside namespace App\Domain
+  because we want protect our domain (on line 14)
+
+  depends on App\Services\CartService
+  from the rule
+  should not depend on classes outside namespace App\Domain
+  because we want protect our domain (on line 15)
 ';
 
         $this->assertEquals(self::ERROR_CODE, $process->getExitCode());
@@ -64,34 +64,34 @@ public function test_returns_error_with_multiple_violations_without_passing_conf
         $expectedErrors = 'ERRORS!
 
 App\Controller\Foo has 2 violations
-should have a name that matches *Controller
-because we want uniform naming
+  should have a name that matches *Controller
+  because we want uniform naming
 
-should implement ContainerAwareInterface
-because all controllers should be container aware
+  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
+  should implement ContainerAwareInterface
+  because all controllers should be container aware
 
 App\Controller\UserController has 1 violations
-should implement ContainerAwareInterface
-because all controllers should be container aware
+  should implement ContainerAwareInterface
+  because all controllers should be container aware
 
 App\Controller\YieldController has 1 violations
-should implement ContainerAwareInterface
-because all controllers should be container aware
+  should implement ContainerAwareInterface
+  because all controllers should be container aware
 
 App\Domain\Model has 2 violations
-depends on App\Services\UserService
-from the rule
-should not depend on classes outside namespace App\Domain
-because we want protect our domain (on line 14)
-
-depends on App\Services\CartService
-from the rule
-should not depend on classes outside namespace App\Domain
-because we want protect our domain (on line 15)
+  depends on App\Services\UserService
+  from the rule
+  should not depend on classes outside namespace App\Domain
+  because we want protect our domain (on line 14)
+
+  depends on App\Services\CartService
+  from the rule
+  should not depend on classes outside namespace App\Domain
+  because we want protect our domain (on line 15)
 ';
 
         $this->assertStringContainsString($expectedErrors, $process->getOutput());
@@ -120,7 +120,7 @@ public function test_bug_yield(): void
         $expectedErrors = 'ERRORS!
 
 App\Controller\Foo has 1 violations
-should have a name that matches *Controller';
+  should have a name that matches *Controller';
 
         $this->assertEquals(self::ERROR_CODE, $process->getExitCode());
         $this->assertStringContainsString($expectedErrors, $process->getOutput());

diff --git a/tests/Unit/Expressions/ForClasses/NotDependsOnTheseExpressionsTest.php b/tests/Unit/Expressions/ForClasses/NotDependsOnTheseExpressionsTest.php
@@ -85,13 +85,13 @@ public function test_it_should_see_violations_only_outside_exclusions(): void
         );
         self::assertStringContainsString(
             <<<TXT
-NOT resides in one of these namespaces: Arkitect\Tests\Fixtures\ComponentA\
-OR
-NOT (
-  resides in one of these namespaces: Arkitect\Tests\Fixtures\ComponentC\
-  AND
-  not resides in one of these namespaces: Arkitect\Tests\Fixtures\ComponentC\ComponentCA\
-)
+  NOT resides in one of these namespaces: Arkitect\Tests\Fixtures\ComponentA\
+  OR
+  NOT (
+    resides in one of these namespaces: Arkitect\Tests\Fixtures\ComponentC\
+    AND
+    not resides in one of these namespaces: Arkitect\Tests\Fixtures\ComponentC\ComponentCA\
+  )
 TXT
             ,
             $violationsText

diff --git a/tests/Unit/Rules/ViolationsTest.php b/tests/Unit/Rules/ViolationsTest.php
@@ -59,10 +59,10 @@ public function test_to_string(): void
         $this->violationStore->add($violation);
         $expected = '
 App\Controller\ProductController has 1 violations
-should implement ContainerInterface
+  should implement ContainerInterface
 
 App\Controller\Foo has 1 violations
-should have name end with Controller
+  should have name end with Controller
 ';
 
         $this->assertEquals($expected, $this->violationStore->toString());