From 7939e6d22960631126fbcf2ac01f611125a2b9d2 Mon Sep 17 00:00:00 2001 From: Herberto Graca Date: Sun, 1 Oct 2023 21:54:14 +0200 Subject: [PATCH] Indent violations output --- src/Rules/Violations.php | 15 ++-- src/Shared/String/IndentationHelper.php | 5 ++ tests/E2E/Cli/CheckCommandTest.php | 42 +++++----- .../PHPUnit/CheckClassHaveAttributeTest.php | 4 +- .../CheckClassImplementInterfaceTest.php | 20 ++--- tests/E2E/Smoke/RunArkitectBinTest.php | 78 +++++++++---------- .../NotDependsOnTheseExpressionsTest.php | 14 ++-- tests/Unit/Rules/ViolationsTest.php | 4 +- 8 files changed, 95 insertions(+), 87 deletions(-) diff --git a/src/Rules/Violations.php b/src/Rules/Violations.php index a784e552..ac9dc65b 100644 --- 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 index 5c1113b0..9dc4f6ff 100644 --- 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 index 50e53f70..43ff02f8 100644 --- 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 index 81949f4f..62edfed7 100644 --- 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 index 9cfab1e1..93564f3c 100644 --- 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 index 9eab5a24..65bb6052 100644 --- 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 index 05c36648..870d9c3a 100644 --- 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( <<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());