Add ability to check for multiple extends (#432)

* Add ability to check for multiple Extends * Add ability to check for multiple NotExtends
phparkitect · May 28, 2024 · cc81afc · cc81afc
1 parent 26bc759
commit cc81afc
Show file tree

Hide file tree

Showing 6 changed files with 82 additions and 31 deletions.
diff --git a/README.md b/README.md
@@ -186,6 +186,8 @@ $rules[] = Rule::allClasses()
     ->that(new ResideInOneOfTheseNamespaces('App\Controller'))
     ->should(new Extend('App\Controller\AbstractController'))
     ->because('we want to be sure that all controllers extend AbstractController');
+
+You can add multiple parameters, the violation will happen when none of them match
 ```
 
 ### Has an attribute (requires PHP >= 8.0)
@@ -348,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/Extend.php b/src/Expression/ForClasses/Extend.php
@@ -13,25 +13,30 @@
 
 class Extend 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 extend {$this->className}", $because);
+        $desc = implode(', ', $this->classNames);
+
+        return new Description("should extend one of these classes: {$desc}", $because);
     }
 
     public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
     {
         $extends = $theClass->getExtends();
 
-        if (null !== $extends && $extends->matches($this->className)) {
-            return;
+        /** @var string $className */
+        foreach ($this->classNames as $className) {
+            if (null !== $extends && $extends->matches($className)) {
+                return;
+            }
         }
 
         $violation = Violation::create(

diff --git a/src/Expression/ForClasses/NotExtend.php b/src/Expression/ForClasses/NotExtend.php
@@ -13,36 +13,37 @@
 
 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;
-        }
+        /** @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))
+                );
 
-        if ($extends->toString() !== $this->className) {
-            return;
-        }
+                $violations->add($violation);
 
-        $violation = Violation::create(
-            $theClass->getFQCN(),
-            ViolationMessage::selfExplanatory($this->describe($theClass, $because))
-        );
-
-        $violations->add($violation);
+                return;
+            }
+        }
     }
 }
diff --git 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/ExtendTest.php b/tests/Unit/Expressions/ForClasses/ExtendTest.php
@@ -70,7 +70,7 @@ public function test_it_should_return_violation_error_when_class_not_extend(): v
         $extend->evaluate($classDescription, $violations, 'we want to add this rule for our software');
 
         self::assertEquals(1, $violations->count());
-        self::assertEquals('should extend My\BaseClass because we want to add this rule for our software', $violations->get(0)->getError());
+        self::assertEquals('should extend one of these classes: My\BaseClass because we want to add this rule for our software', $violations->get(0)->getError());
     }
 
     public function test_it_should_return_violation_error_if_extend_is_null(): void
@@ -97,6 +97,21 @@ public function test_it_should_return_violation_error_if_extend_is_null(): void
         $extend->evaluate($classDescription, $violations, $because);
 
         self::assertEquals(1, $violations->count());
-        self::assertEquals('should extend My\BaseClass because we want to add this rule for our software', $violationError);
+        self::assertEquals('should extend one of these classes: My\BaseClass because we want to add this rule for our software', $violationError);
+    }
+
+    public function test_it_should_accept_multiple_extends(): void
+    {
+        $extend = new Extend('My\FirstExtend', 'My\SecondExtend');
+
+        $classDescription = (new ClassDescriptionBuilder())
+            ->setClassName('My\Class')
+            ->setExtends('My\SecondExtend', 10)
+            ->build();
+
+        $violations = new Violations();
+        $extend->evaluate($classDescription, $violations, 'because');
+
+        self::assertEquals(0, $violations->count());
     }
 }
diff --git 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);
+    }
 }