From 45fae315d022d856eef4e7cb20ab8ef8e4a30a53 Mon Sep 17 00:00:00 2001 From: Jan Willem Kaper Date: Fri, 17 May 2024 21:43:50 +0200 Subject: [PATCH] Add ability to check for multiple NotExtends --- README.md | 2 ++ src/Expression/ForClasses/NotExtend.php | 36 +++++++++---------- tests/E2E/Cli/DebugExpressionCommandTest.php | 8 ++--- .../Expressions/ForClasses/NotExtendTest.php | 28 ++++++++++++++- 4 files changed, 51 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 0bdba903..15cd4738 100644 --- a/README.md +++ b/README.md @@ -350,6 +350,8 @@ $rules[] = Rule::allClasses() ->that(new ResideInOneOfTheseNamespaces('App\Controller\Admin')) ->should(new NotExtend('App\Controller\AbstractController')) ->because('we want to be sure that all admin controllers not extend AbstractController for security reasons'); + +You can add multiple parameters, the violation will happen when one of them match ``` ### Don't have dependency outside a namespace diff --git a/src/Expression/ForClasses/NotExtend.php b/src/Expression/ForClasses/NotExtend.php index 26539312..38c5063a 100644 --- a/src/Expression/ForClasses/NotExtend.php +++ b/src/Expression/ForClasses/NotExtend.php @@ -13,36 +13,36 @@ class NotExtend implements Expression { - /** @var string */ - private $className; + /** @var string[] */ + private $classNames; - public function __construct(string $className) + public function __construct(string ...$classNames) { - $this->className = $className; + $this->classNames = $classNames; } public function describe(ClassDescription $theClass, string $because): Description { - return new Description("should not extend {$this->className}", $because); + $desc = implode(', ', $this->classNames); + + return new Description("should not extend one of these classes: {$desc}", $because); } public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void { $extends = $theClass->getExtends(); - if (null === $extends) { - return; - } - - if ($extends->toString() !== $this->className) { - return; + /** @var string $className */ + foreach ($this->classNames as $className) { + if (null !== $extends && $extends->matches($className)) { + $violation = Violation::create( + $theClass->getFQCN(), + ViolationMessage::selfExplanatory($this->describe($theClass, $because)) + ); + + $violations->add($violation); + return; + } } - - $violation = Violation::create( - $theClass->getFQCN(), - ViolationMessage::selfExplanatory($this->describe($theClass, $because)) - ); - - $violations->add($violation); } } diff --git a/tests/E2E/Cli/DebugExpressionCommandTest.php b/tests/E2E/Cli/DebugExpressionCommandTest.php index 985f8234..4fa0aee0 100644 --- a/tests/E2E/Cli/DebugExpressionCommandTest.php +++ b/tests/E2E/Cli/DebugExpressionCommandTest.php @@ -36,16 +36,16 @@ public function test_some_classes_found(): void public function test_meaningful_errors_for_too_few_arguments_for_the_expression(): void { $appTester = $this->createAppTester(); - $appTester->run(['debug:expression', 'expression' => 'NotExtend', 'arguments' => [], '--from-dir' => __DIR__.'/../_fixtures/mvc/Domain']); - $this->assertEquals("Error: Too few arguments for 'NotExtend'.\n", $appTester->getDisplay()); + $appTester->run(['debug:expression', 'expression' => 'NotImplement', 'arguments' => [], '--from-dir' => __DIR__.'/../_fixtures/mvc/Domain']); + $this->assertEquals("Error: Too few arguments for 'NotImplement'.\n", $appTester->getDisplay()); $this->assertEquals(2, $appTester->getStatusCode()); } public function test_meaningful_errors_for_too_many_arguments_for_the_expression(): void { $appTester = $this->createAppTester(); - $appTester->run(['debug:expression', 'expression' => 'NotExtend', 'arguments' => ['First', 'Second'], '--from-dir' => __DIR__.'/../_fixtures/mvc/Domain']); - $this->assertEquals("Error: Too many arguments for 'NotExtend'.\n", $appTester->getDisplay()); + $appTester->run(['debug:expression', 'expression' => 'NotImplement', 'arguments' => ['First', 'Second'], '--from-dir' => __DIR__.'/../_fixtures/mvc/Domain']); + $this->assertEquals("Error: Too many arguments for 'NotImplement'.\n", $appTester->getDisplay()); $this->assertEquals(2, $appTester->getStatusCode()); } diff --git a/tests/Unit/Expressions/ForClasses/NotExtendTest.php b/tests/Unit/Expressions/ForClasses/NotExtendTest.php index 61d1ebdd..40c42150 100644 --- a/tests/Unit/Expressions/ForClasses/NotExtendTest.php +++ b/tests/Unit/Expressions/ForClasses/NotExtendTest.php @@ -35,7 +35,7 @@ public function test_it_should_return_violation_error(): void $notExtend->evaluate($classDescription, $violations, $because); self::assertEquals(1, $violations->count()); - self::assertEquals('should not extend My\BaseClass because we want to add this rule for our software', $violationError); + self::assertEquals('should not extend one of these classes: My\BaseClass because we want to add this rule for our software', $violationError); } public function test_it_should_not_return_violation_error_if_extends_another_class(): void @@ -62,4 +62,30 @@ public function test_it_should_not_return_violation_error_if_extends_another_cla self::assertEquals(0, $violations->count()); } + + public function test_it_should_return_violation_error_for_multiple_extends(): void + { + $notExtend = new NotExtend('My\FirstExtend', 'My\SecondExtend'); + + $classDescription = new ClassDescription( + FullyQualifiedClassName::fromString('HappyIsland'), + [], + [], + FullyQualifiedClassName::fromString('My\SecondExtend'), + false, + false, + false, + false, + false, + false + ); + $because = 'we want to add this rule for our software'; + $violationError = $notExtend->describe($classDescription, $because)->toString(); + + $violations = new Violations(); + $notExtend->evaluate($classDescription, $violations, $because); + + self::assertEquals(1, $violations->count()); + self::assertEquals('should not extend one of these classes: My\FirstExtend, My\SecondExtend because we want to add this rule for our software', $violationError); + } }