diff --git a/src/Expression/ForClasses/DependsOnlyOnTheseNamespaces.php b/src/Expression/ForClasses/DependsOnlyOnTheseNamespaces.php index 183ae84d..d3e4396a 100644 --- a/src/Expression/ForClasses/DependsOnlyOnTheseNamespaces.php +++ b/src/Expression/ForClasses/DependsOnlyOnTheseNamespaces.php @@ -9,6 +9,7 @@ use Arkitect\Expression\Description; use Arkitect\Expression\Expression; use Arkitect\Rules\Violation; +use Arkitect\Rules\ViolationMessage; use Arkitect\Rules\Violations; class DependsOnlyOnTheseNamespaces implements Expression @@ -44,7 +45,10 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str if (!$dependency->matchesOneOf(...$this->namespaces)) { $violation = Violation::createWithErrorLine( $theClass->getFQCN(), - $this->describe($theClass, $because)->toString(), + ViolationMessage::withDescription( + $this->describe($theClass, $because), + "depends on {$dependency->getFQCN()->toString()}" + ), $dependency->getLine() ); diff --git a/src/Expression/ForClasses/DocBlockContains.php b/src/Expression/ForClasses/DocBlockContains.php index 66f28b33..aa8df128 100644 --- a/src/Expression/ForClasses/DocBlockContains.php +++ b/src/Expression/ForClasses/DocBlockContains.php @@ -8,6 +8,7 @@ use Arkitect\Expression\Description; use Arkitect\Expression\Expression; use Arkitect\Rules\Violation; +use Arkitect\Rules\ViolationMessage; use Arkitect\Rules\Violations; class DocBlockContains implements Expression @@ -30,7 +31,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str if (!$theClass->containsDocBlock($this->docBlock)) { $violation = Violation::create( $theClass->getFQCN(), - $this->describe($theClass, $because)->toString() + ViolationMessage::selfExplanatory($this->describe($theClass, $because)) ); $violations->add($violation); } diff --git a/src/Expression/ForClasses/DocBlockNotContains.php b/src/Expression/ForClasses/DocBlockNotContains.php index dec7286f..6ecee596 100644 --- a/src/Expression/ForClasses/DocBlockNotContains.php +++ b/src/Expression/ForClasses/DocBlockNotContains.php @@ -8,6 +8,7 @@ use Arkitect\Expression\Description; use Arkitect\Expression\Expression; use Arkitect\Rules\Violation; +use Arkitect\Rules\ViolationMessage; use Arkitect\Rules\Violations; class DocBlockNotContains implements Expression @@ -30,7 +31,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str if ($theClass->containsDocBlock($this->docBlock)) { $violation = Violation::create( $theClass->getFQCN(), - $this->describe($theClass, $because)->toString() + ViolationMessage::selfExplanatory($this->describe($theClass, $because)) ); $violations->add($violation); } diff --git a/src/Expression/ForClasses/Extend.php b/src/Expression/ForClasses/Extend.php index eb11c1be..c8b6d023 100644 --- a/src/Expression/ForClasses/Extend.php +++ b/src/Expression/ForClasses/Extend.php @@ -8,6 +8,7 @@ use Arkitect\Expression\Description; use Arkitect\Expression\Expression; use Arkitect\Rules\Violation; +use Arkitect\Rules\ViolationMessage; use Arkitect\Rules\Violations; class Extend implements Expression @@ -35,7 +36,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str $violation = Violation::create( $theClass->getFQCN(), - $this->describe($theClass, $because)->toString() + ViolationMessage::selfExplanatory($this->describe($theClass, $because)) ); $violations->add($violation); diff --git a/src/Expression/ForClasses/HaveAttribute.php b/src/Expression/ForClasses/HaveAttribute.php index 8ca88cf0..8a542fdf 100644 --- a/src/Expression/ForClasses/HaveAttribute.php +++ b/src/Expression/ForClasses/HaveAttribute.php @@ -7,6 +7,7 @@ use Arkitect\Expression\Description; use Arkitect\Expression\Expression; use Arkitect\Rules\Violation; +use Arkitect\Rules\ViolationMessage; use Arkitect\Rules\Violations; final class HaveAttribute implements Expression @@ -33,7 +34,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str $violations->add( Violation::create( $theClass->getFQCN(), - $this->describe($theClass, $because)->toString() + ViolationMessage::selfExplanatory($this->describe($theClass, $because)) ) ); } diff --git a/src/Expression/ForClasses/HaveNameMatching.php b/src/Expression/ForClasses/HaveNameMatching.php index 42df2bfd..858c3928 100644 --- a/src/Expression/ForClasses/HaveNameMatching.php +++ b/src/Expression/ForClasses/HaveNameMatching.php @@ -9,6 +9,7 @@ use Arkitect\Expression\Description; use Arkitect\Expression\Expression; use Arkitect\Rules\Violation; +use Arkitect\Rules\ViolationMessage; use Arkitect\Rules\Violations; class HaveNameMatching implements Expression @@ -32,7 +33,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str if (!$fqcn->classMatches($this->name)) { $violation = Violation::create( $theClass->getFQCN(), - $this->describe($theClass, $because)->toString() + ViolationMessage::selfExplanatory($this->describe($theClass, $because)) ); $violations->add($violation); } diff --git a/src/Expression/ForClasses/Implement.php b/src/Expression/ForClasses/Implement.php index 3f35c80c..06ead6d6 100644 --- a/src/Expression/ForClasses/Implement.php +++ b/src/Expression/ForClasses/Implement.php @@ -9,6 +9,7 @@ use Arkitect\Expression\Description; use Arkitect\Expression\Expression; use Arkitect\Rules\Violation; +use Arkitect\Rules\ViolationMessage; use Arkitect\Rules\Violations; class Implement implements Expression @@ -37,7 +38,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str if (0 === \count(array_filter($interfaces, $implements))) { $violation = Violation::create( $theClass->getFQCN(), - $this->describe($theClass, $because)->toString() + ViolationMessage::selfExplanatory($this->describe($theClass, $because)) ); $violations->add($violation); } diff --git a/src/Expression/ForClasses/IsAbstract.php b/src/Expression/ForClasses/IsAbstract.php index 8b6d6be7..3a58908e 100644 --- a/src/Expression/ForClasses/IsAbstract.php +++ b/src/Expression/ForClasses/IsAbstract.php @@ -8,6 +8,7 @@ use Arkitect\Expression\Description; use Arkitect\Expression\Expression; use Arkitect\Rules\Violation; +use Arkitect\Rules\ViolationMessage; use Arkitect\Rules\Violations; class IsAbstract implements Expression @@ -25,7 +26,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str $violation = Violation::create( $theClass->getFQCN(), - $this->describe($theClass, $because)->toString() + ViolationMessage::selfExplanatory($this->describe($theClass, $because)) ); $violations->add($violation); diff --git a/src/Expression/ForClasses/IsFinal.php b/src/Expression/ForClasses/IsFinal.php index 30bd23e8..67298158 100644 --- a/src/Expression/ForClasses/IsFinal.php +++ b/src/Expression/ForClasses/IsFinal.php @@ -8,6 +8,7 @@ use Arkitect\Expression\Description; use Arkitect\Expression\Expression; use Arkitect\Rules\Violation; +use Arkitect\Rules\ViolationMessage; use Arkitect\Rules\Violations; class IsFinal implements Expression @@ -25,7 +26,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str $violation = Violation::create( $theClass->getFQCN(), - $this->describe($theClass, $because)->toString() + ViolationMessage::selfExplanatory($this->describe($theClass, $because)) ); $violations->add($violation); diff --git a/src/Expression/ForClasses/IsNotAbstract.php b/src/Expression/ForClasses/IsNotAbstract.php index 8725d435..73eca2fb 100644 --- a/src/Expression/ForClasses/IsNotAbstract.php +++ b/src/Expression/ForClasses/IsNotAbstract.php @@ -8,6 +8,7 @@ use Arkitect\Expression\Description; use Arkitect\Expression\Expression; use Arkitect\Rules\Violation; +use Arkitect\Rules\ViolationMessage; use Arkitect\Rules\Violations; class IsNotAbstract implements Expression @@ -25,7 +26,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str $violation = Violation::create( $theClass->getFQCN(), - $this->describe($theClass, $because)->toString() + ViolationMessage::selfExplanatory($this->describe($theClass, $because)) ); $violations->add($violation); diff --git a/src/Expression/ForClasses/IsNotFinal.php b/src/Expression/ForClasses/IsNotFinal.php index b576ac1f..7557099d 100644 --- a/src/Expression/ForClasses/IsNotFinal.php +++ b/src/Expression/ForClasses/IsNotFinal.php @@ -8,6 +8,7 @@ use Arkitect\Expression\Description; use Arkitect\Expression\Expression; use Arkitect\Rules\Violation; +use Arkitect\Rules\ViolationMessage; use Arkitect\Rules\Violations; class IsNotFinal implements Expression @@ -25,7 +26,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str $violation = Violation::create( $theClass->getFQCN(), - $this->describe($theClass, $because)->toString() + ViolationMessage::selfExplanatory($this->describe($theClass, $because)) ); $violations->add($violation); diff --git a/src/Expression/ForClasses/NotDependsOnTheseNamespaces.php b/src/Expression/ForClasses/NotDependsOnTheseNamespaces.php index 6826c07e..eb5c73a9 100644 --- a/src/Expression/ForClasses/NotDependsOnTheseNamespaces.php +++ b/src/Expression/ForClasses/NotDependsOnTheseNamespaces.php @@ -9,6 +9,7 @@ use Arkitect\Expression\Description; use Arkitect\Expression\Expression; use Arkitect\Rules\Violation; +use Arkitect\Rules\ViolationMessage; use Arkitect\Rules\Violations; class NotDependsOnTheseNamespaces implements Expression @@ -41,7 +42,10 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str if ($dependency->matchesOneOf(...$this->namespaces)) { $violation = Violation::createWithErrorLine( $theClass->getFQCN(), - $this->describe($theClass, $because)->toString(), + ViolationMessage::withDescription( + $this->describe($theClass, $because), + "depends on {$dependency->getFQCN()->toString()}" + ), $dependency->getLine() ); diff --git a/src/Expression/ForClasses/NotExtend.php b/src/Expression/ForClasses/NotExtend.php index 9ea4e377..26539312 100644 --- a/src/Expression/ForClasses/NotExtend.php +++ b/src/Expression/ForClasses/NotExtend.php @@ -8,6 +8,7 @@ use Arkitect\Expression\Description; use Arkitect\Expression\Expression; use Arkitect\Rules\Violation; +use Arkitect\Rules\ViolationMessage; use Arkitect\Rules\Violations; class NotExtend implements Expression @@ -39,7 +40,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str $violation = Violation::create( $theClass->getFQCN(), - $this->describe($theClass, $because)->toString() + ViolationMessage::selfExplanatory($this->describe($theClass, $because)) ); $violations->add($violation); diff --git a/src/Expression/ForClasses/NotHaveDependencyOutsideNamespace.php b/src/Expression/ForClasses/NotHaveDependencyOutsideNamespace.php index fe3b8239..19ac84dd 100644 --- a/src/Expression/ForClasses/NotHaveDependencyOutsideNamespace.php +++ b/src/Expression/ForClasses/NotHaveDependencyOutsideNamespace.php @@ -9,6 +9,7 @@ use Arkitect\Expression\Description; use Arkitect\Expression\Expression; use Arkitect\Rules\Violation; +use Arkitect\Rules\ViolationMessage; use Arkitect\Rules\Violations; class NotHaveDependencyOutsideNamespace implements Expression @@ -47,7 +48,10 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str $violation = Violation::createWithErrorLine( $theClass->getFQCN(), - $this->describe($theClass, $because)->toString(), + ViolationMessage::withDescription( + $this->describe($theClass, $because), + "depends on {$externalDep->getFQCN()->toString()}" + ), $externalDep->getLine() ); $violations->add($violation); diff --git a/src/Expression/ForClasses/NotHaveNameMatching.php b/src/Expression/ForClasses/NotHaveNameMatching.php index 333e509c..05b80f01 100644 --- a/src/Expression/ForClasses/NotHaveNameMatching.php +++ b/src/Expression/ForClasses/NotHaveNameMatching.php @@ -9,6 +9,7 @@ use Arkitect\Expression\Description; use Arkitect\Expression\Expression; use Arkitect\Rules\Violation; +use Arkitect\Rules\ViolationMessage; use Arkitect\Rules\Violations; class NotHaveNameMatching implements Expression @@ -32,7 +33,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str if ($fqcn->classMatches($this->name)) { $violation = Violation::create( $theClass->getFQCN(), - $this->describe($theClass, $because)->toString() + ViolationMessage::selfExplanatory($this->describe($theClass, $because)) ); $violations->add($violation); } diff --git a/src/Expression/ForClasses/NotImplement.php b/src/Expression/ForClasses/NotImplement.php index 4189a3ac..8e6765bf 100644 --- a/src/Expression/ForClasses/NotImplement.php +++ b/src/Expression/ForClasses/NotImplement.php @@ -9,6 +9,7 @@ use Arkitect\Expression\Description; use Arkitect\Expression\Expression; use Arkitect\Rules\Violation; +use Arkitect\Rules\ViolationMessage; use Arkitect\Rules\Violations; class NotImplement implements Expression @@ -37,7 +38,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str if (\count(array_filter($interfaces, $implements)) > 0) { $violation = Violation::create( $theClass->getFQCN(), - $this->describe($theClass, $because)->toString() + ViolationMessage::selfExplanatory($this->describe($theClass, $because)) ); $violations->add($violation); } diff --git a/src/Expression/ForClasses/NotResideInTheseNamespaces.php b/src/Expression/ForClasses/NotResideInTheseNamespaces.php index bf11ea59..23949ff3 100644 --- a/src/Expression/ForClasses/NotResideInTheseNamespaces.php +++ b/src/Expression/ForClasses/NotResideInTheseNamespaces.php @@ -8,6 +8,7 @@ use Arkitect\Expression\Description; use Arkitect\Expression\Expression; use Arkitect\Rules\Violation; +use Arkitect\Rules\ViolationMessage; use Arkitect\Rules\Violations; class NotResideInTheseNamespaces implements Expression @@ -39,7 +40,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str if ($resideInNamespace) { $violation = Violation::create( $theClass->getFQCN(), - $this->describe($theClass, $because)->toString() + ViolationMessage::selfExplanatory($this->describe($theClass, $because)) ); $violations->add($violation); } diff --git a/src/Expression/ForClasses/ResideInOneOfTheseNamespaces.php b/src/Expression/ForClasses/ResideInOneOfTheseNamespaces.php index 91e48d06..5048486d 100644 --- a/src/Expression/ForClasses/ResideInOneOfTheseNamespaces.php +++ b/src/Expression/ForClasses/ResideInOneOfTheseNamespaces.php @@ -8,6 +8,7 @@ use Arkitect\Expression\Description; use Arkitect\Expression\Expression; use Arkitect\Rules\Violation; +use Arkitect\Rules\ViolationMessage; use Arkitect\Rules\Violations; class ResideInOneOfTheseNamespaces implements Expression @@ -39,7 +40,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str if (!$resideInNamespace) { $violation = Violation::create( $theClass->getFQCN(), - $this->describe($theClass, $because)->toString() + ViolationMessage::selfExplanatory($this->describe($theClass, $because)) ); $violations->add($violation); } diff --git a/src/Rules/Violation.php b/src/Rules/Violation.php index 20028f0d..cc716a5b 100644 --- a/src/Rules/Violation.php +++ b/src/Rules/Violation.php @@ -22,14 +22,14 @@ public function __construct(string $fqcn, string $error, ?int $line = null) $this->line = $line; } - public static function create(string $fqcn, string $error): self + public static function create(string $fqcn, ViolationMessage $error): self { - return new self($fqcn, $error); + return new self($fqcn, $error->toString()); } - public static function createWithErrorLine(string $fqcn, string $error, int $line): self + public static function createWithErrorLine(string $fqcn, ViolationMessage $error, int $line): self { - return new self($fqcn, $error, $line); + return new self($fqcn, $error->toString(), $line); } public function getFqcn(): string diff --git a/src/Rules/ViolationMessage.php b/src/Rules/ViolationMessage.php new file mode 100644 index 00000000..eb794046 --- /dev/null +++ b/src/Rules/ViolationMessage.php @@ -0,0 +1,40 @@ +rule = $rule; + $this->violation = $violation; + } + + public static function withDescription(Description $brokenRule, string $description): self + { + return new self($brokenRule->toString(), $description); + } + + public static function selfExplanatory(Description $brokenRule): self + { + return new self($brokenRule->toString(), null); + } + + public function toString(): string + { + if (null === $this->violation) { + return $this->rule; + } + + return "$this->violation, but $this->rule"; + } +} diff --git a/src/Rules/Violations.php b/src/Rules/Violations.php index 1d2094d4..d034f935 100644 --- a/src/Rules/Violations.php +++ b/src/Rules/Violations.php @@ -72,7 +72,8 @@ public function toString(): string * @var Violation[] $violationsByFqcn */ foreach ($violationsCollection as $key => $violationsByFqcn) { - $errors .= "\n".$key.' violates rules'; + $violationForThisFqcn = \count($violationsByFqcn); + $errors .= "\n$key has {$violationForThisFqcn} violations"; foreach ($violationsByFqcn as $violation) { $errors .= "\n ".$violation->getError(); diff --git a/tests/E2E/Cli/CheckCommandTest.php b/tests/E2E/Cli/CheckCommandTest.php index bb3daa7c..ac19497d 100644 --- a/tests/E2E/Cli/CheckCommandTest.php +++ b/tests/E2E/Cli/CheckCommandTest.php @@ -21,22 +21,22 @@ public function test_app_returns_error_with_multiple_violations(): void $expectedErrors = 'ERRORS! -App\Controller\Foo violates rules +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 -App\Controller\ProductsController violates rules +App\Controller\ProductsController has 1 violations should implement ContainerAwareInterface because all controllers should be container aware -App\Controller\UserController violates rules +App\Controller\UserController has 1 violations should implement ContainerAwareInterface because all controllers should be container aware -App\Controller\YieldController violates rules +App\Controller\YieldController has 1 violations should implement ContainerAwareInterface because all controllers should be container aware -App\Domain\Model violates rules - should not depend on classes outside namespace App\Domain because we want protect our domain (on line 14) - should not depend on classes outside namespace App\Domain because we want protect our domain (on line 15)'; +App\Domain\Model has 2 violations + depends on App\Services\UserService, but should not depend on classes outside namespace App\Domain because we want protect our domain (on line 14) + depends on App\Services\CartService, but should not depend on classes outside namespace App\Domain because we want protect our domain (on line 15)'; $this->assertCheckHasErrors($cmdTester, $expectedErrors); } @@ -46,11 +46,11 @@ public function test_app_returns_single_error_because_there_is_stop_on_failure_p $cmdTester = $this->runCheck(__DIR__.'/../_fixtures/configMvc.php', true); $expectedErrors = 'ERRORS! -App\Controller\Foo violates rules +App\Controller\Foo has 1 violations should implement ContainerAwareInterface because all controllers should be container aware'; $this->assertCheckHasErrors($cmdTester, $expectedErrors); - $this->assertCheckHasNoErrorsLike($cmdTester, "App\Controller\ProductsController violates rules"); + $this->assertCheckHasNoErrorsLike($cmdTester, "App\Controller\ProductsController has 1 violations"); } public function test_does_not_explode_if_an_exception_is_thrown(): void @@ -73,7 +73,7 @@ public function test_bug_yield(): void $expectedErrors = 'ERRORS! -App\Controller\Foo violates rules +App\Controller\Foo has 1 violations 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 a83004f4..aa146646 100644 --- a/tests/E2E/PHPUnit/CheckClassHaveAttributeTest.php +++ b/tests/E2E/PHPUnit/CheckClassHaveAttributeTest.php @@ -38,7 +38,7 @@ public function test_controllers_should_have_name_ending_in_controller(): void ->because('its a symfony thing'); $expectedExceptionMessage = ' -App\Controller\Foo violates rules +App\Controller\Foo has 1 violations should have a name that matches *Controller because its a symfony thing'; $this->expectException(ExpectationFailedException::class); diff --git a/tests/E2E/PHPUnit/CheckClassImplementInterfaceTest.php b/tests/E2E/PHPUnit/CheckClassImplementInterfaceTest.php index ac37e8c8..e064833d 100644 --- a/tests/E2E/PHPUnit/CheckClassImplementInterfaceTest.php +++ b/tests/E2E/PHPUnit/CheckClassImplementInterfaceTest.php @@ -25,19 +25,19 @@ public function test_assertion_should_fail_on_broken_rule(): void ->because('i said so'); $expectedExceptionMessage = ' -App\Controller\Foo violates rules +App\Controller\Foo has 1 violations should implement ContainerAwareInterface because i said so -App\Controller\ProductsController violates rules +App\Controller\ProductsController has 1 violations should implement ContainerAwareInterface because i said so -App\Controller\UserController violates rules +App\Controller\UserController has 1 violations should implement ContainerAwareInterface because i said so -App\Controller\YieldController violates rules +App\Controller\YieldController has 1 violations should implement ContainerAwareInterface because i said so -App\Services\UserService violates rules +App\Services\UserService has 1 violations should implement ContainerAwareInterface because i said so'; $this->expectException(ExpectationFailedException::class); diff --git a/tests/E2E/Smoke/RunArkitectBinTest.php b/tests/E2E/Smoke/RunArkitectBinTest.php index 74809d9e..e98b2dae 100644 --- a/tests/E2E/Smoke/RunArkitectBinTest.php +++ b/tests/E2E/Smoke/RunArkitectBinTest.php @@ -22,22 +22,22 @@ public function test_returns_error_with_multiple_violations(): void $expectedErrors = 'ERRORS! -App\Controller\Foo violates rules +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 -App\Controller\ProductsController violates rules +App\Controller\ProductsController has 1 violations should implement ContainerAwareInterface because all controllers should be container aware -App\Controller\UserController violates rules +App\Controller\UserController has 1 violations should implement ContainerAwareInterface because all controllers should be container aware -App\Controller\YieldController violates rules +App\Controller\YieldController has 1 violations should implement ContainerAwareInterface because all controllers should be container aware -App\Domain\Model violates rules - should not depend on classes outside namespace App\Domain because we want protect our domain (on line 14) - should not depend on classes outside namespace App\Domain because we want protect our domain (on line 15)'; +App\Domain\Model has 2 violations + depends on App\Services\UserService, but should not depend on classes outside namespace App\Domain because we want protect our domain (on line 14) + depends on App\Services\CartService, but 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()); $this->assertStringContainsString($expectedErrors, $process->getOutput()); @@ -49,22 +49,22 @@ public function test_returns_error_with_multiple_violations_without_passing_conf $expectedErrors = 'ERRORS! -App\Controller\Foo violates rules +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 -App\Controller\ProductsController violates rules +App\Controller\ProductsController has 1 violations should implement ContainerAwareInterface because all controllers should be container aware -App\Controller\UserController violates rules +App\Controller\UserController has 1 violations should implement ContainerAwareInterface because all controllers should be container aware -App\Controller\YieldController violates rules +App\Controller\YieldController has 1 violations should implement ContainerAwareInterface because all controllers should be container aware -App\Domain\Model violates rules - should not depend on classes outside namespace App\Domain because we want protect our domain (on line 14) - should not depend on classes outside namespace App\Domain because we want protect our domain (on line 15)'; +App\Domain\Model has 2 violations + depends on App\Services\UserService, but should not depend on classes outside namespace App\Domain because we want protect our domain (on line 14) + depends on App\Services\CartService, but should not depend on classes outside namespace App\Domain because we want protect our domain (on line 15)'; $this->assertStringContainsString($expectedErrors, $process->getOutput()); $this->assertEquals(self::ERROR_CODE, $process->getExitCode()); @@ -91,7 +91,7 @@ public function test_bug_yield(): void $expectedErrors = 'ERRORS! -App\Controller\Foo violates rules +App\Controller\Foo has 1 violations should have a name that matches *Controller'; $this->assertEquals(self::ERROR_CODE, $process->getExitCode()); diff --git a/tests/Unit/Expressions/ForClasses/DependsOnlyOnTheseNamespacesTest.php b/tests/Unit/Expressions/ForClasses/DependsOnlyOnTheseNamespacesTest.php index 8389593c..87723c2e 100644 --- a/tests/Unit/Expressions/ForClasses/DependsOnlyOnTheseNamespacesTest.php +++ b/tests/Unit/Expressions/ForClasses/DependsOnlyOnTheseNamespacesTest.php @@ -42,6 +42,10 @@ public function test_it_should_return_true_if_not_depends_on_namespace(): void $dependOnClasses->evaluate($classDescription, $violations, $because); self::assertNotEquals(0, $violations->count()); + self::assertEquals( + 'depends on anotherNamespace\Banana, but should depend only on classes in one of these namespaces: myNamespace because we want to add this rule for our software', + $violations->get(0)->getError() + ); } public function test_it_should_return_true_if_depends_on_class_in_root_namespace(): void diff --git a/tests/Unit/Expressions/ForClasses/NotDependsOnTheseNamespacesTest.php b/tests/Unit/Expressions/ForClasses/NotDependsOnTheseNamespacesTest.php index 6bbd63f0..5e58e7f6 100644 --- a/tests/Unit/Expressions/ForClasses/NotDependsOnTheseNamespacesTest.php +++ b/tests/Unit/Expressions/ForClasses/NotDependsOnTheseNamespacesTest.php @@ -39,8 +39,8 @@ public function test_it_should_return_true_if_not_depends_on_namespace(): void self::assertEquals(1, $violations->count()); $this->assertEquals( - 'should not depend on these namespaces: myNamespace because we want to add this rule for our software', - $notDependOnClasses->describe($classDescription, $because)->toString() + 'depends on myNamespace\Banana, but should not depend on these namespaces: myNamespace because we want to add this rule for our software', + $violations->get(0)->getError() ); } @@ -60,8 +60,8 @@ public function test_it_should_return_true_if_depends_on_class_in_root_namespace self::assertCount(1, $violations); $this->assertEquals( - 'should not depend on these namespaces: myNamespace because we want to add this rule for our software', - $notDependOnClasses->describe($classDescription, $because)->toString() + 'depends on myNamespace\Banana, but should not depend on these namespaces: myNamespace because we want to add this rule for our software', + $violations->get(0)->getError() ); } @@ -80,8 +80,8 @@ public function test_it_should_return_false_if_depends_on_namespace(): void self::assertEquals(2, $violations->count()); $this->assertEquals( - 'should not depend on these namespaces: myNamespace because we want to add this rule for our software', - $notDependOnClasses->describe($classDescription, $because)->toString() + 'depends on myNamespace\Banana, but should not depend on these namespaces: myNamespace because we want to add this rule for our software', + $violations->get(0)->getError() ); } } diff --git a/tests/Unit/Expressions/ViolationDescriptionTest.php b/tests/Unit/Expressions/ViolationDescriptionTest.php new file mode 100644 index 00000000..a0056910 --- /dev/null +++ b/tests/Unit/Expressions/ViolationDescriptionTest.php @@ -0,0 +1,25 @@ +toString(), $description->toString()); + } + + public function test_it_should_prepend_description_when_present(): void + { + $expressionDescription = new Description('should not', 'reasons'); + $description = ViolationMessage::withDescription($expressionDescription, 'it does something'); + self::assertEquals('it does something, but should not because reasons', $description->toString()); + } +} diff --git a/tests/Unit/Rules/ConstraintsTest.php b/tests/Unit/Rules/ConstraintsTest.php index b4abd8ab..6d1c0810 100644 --- a/tests/Unit/Rules/ConstraintsTest.php +++ b/tests/Unit/Rules/ConstraintsTest.php @@ -10,6 +10,7 @@ use Arkitect\Expression\Expression; use Arkitect\Rules\Constraints; use Arkitect\Rules\Violation; +use Arkitect\Rules\ViolationMessage; use Arkitect\Rules\Violations; use PHPUnit\Framework\TestCase; @@ -20,7 +21,7 @@ public function test_it_should_not_add_to_violation_if_constraint_is_not_violate $trueExpression = new class() implements Expression { public function describe(ClassDescription $theClass, string $because): Description { - return new Description(''); + return new Description('', ''); } public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void @@ -54,7 +55,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str { $violation = Violation::create( $theClass->getFQCN(), - $this->describe($theClass, $because)->toString() + ViolationMessage::selfExplanatory($this->describe($theClass, $because)) ); $violations->add($violation); diff --git a/tests/Unit/Rules/RuleCheckerTest.php b/tests/Unit/Rules/RuleCheckerTest.php index f3774f75..937bb978 100644 --- a/tests/Unit/Rules/RuleCheckerTest.php +++ b/tests/Unit/Rules/RuleCheckerTest.php @@ -67,7 +67,7 @@ class FakeRule implements ArchRule { public function check(ClassDescription $classDescription, Violations $violations): void { - $violations->add(Violation::create('fqcn', 'error')); + $violations->add(new Violation('fqcn', 'error')); } } diff --git a/tests/Unit/Rules/ViolationsTest.php b/tests/Unit/Rules/ViolationsTest.php index befe2f4c..31cb49cc 100644 --- a/tests/Unit/Rules/ViolationsTest.php +++ b/tests/Unit/Rules/ViolationsTest.php @@ -25,10 +25,9 @@ protected function setUp(): void $this->violationData = 'violation'; $this->violationStore = new Violations(); - $this->violation = Violation::create( + $this->violation = new Violation( 'App\Controller\ProductController', - 'should implement ContainerInterface', - 1 + 'should implement ContainerInterface' ); $this->violationStore->add($this->violation); } @@ -47,10 +46,9 @@ public function test_add_elements_to_store_and_cant_get_it_if_index_not_valid(): public function test_count(): void { - $violation = Violation::create( + $violation = new Violation( 'App\Controller\Shop', - 'should have name end with Controller', - 2 + 'should have name end with Controller' ); $this->violationStore->add($violation); $this->assertEquals(2, $this->violationStore->count()); @@ -58,18 +56,17 @@ public function test_count(): void public function test_to_string(): void { - $violation = Violation::create( + $violation = new Violation( 'App\Controller\Foo', - 'should have name end with Controller', - 3 + 'should have name end with Controller' ); $this->violationStore->add($violation); $expected = ' -App\Controller\ProductController violates rules +App\Controller\ProductController has 1 violations should implement ContainerInterface -App\Controller\Foo violates rules +App\Controller\Foo has 1 violations should have name end with Controller ';