fixes also readonly and final checks

micheleorselli · micheleorselli · commit 0e4a588b5ac4 · 2025-03-06T23:16:40.000+01:00
diff --git a/src/Expression/ForClasses/IsFinal.php b/src/Expression/ForClasses/IsFinal.php
@@ -18,10 +18,14 @@ public function describe(ClassDescription $theClass, string $because): Descripti
         return new Description("{$theClass->getName()} should be final", $because);
     }
 
+    public function appliesTo(ClassDescription $theClass): bool
+    {
+        return !($theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum() || $theClass->isAbstract());
+    }
+
     public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
     {
-        if ($theClass->isAbstract() || $theClass->isInterface() || $theClass->isFinal() || $theClass->isTrait()
-            || $theClass->isEnum()) {
+        if ($theClass->isFinal()) {
             return;
         }
 
diff --git a/src/Expression/ForClasses/IsNotAbstract.php b/src/Expression/ForClasses/IsNotAbstract.php
@@ -18,6 +18,11 @@ public function describe(ClassDescription $theClass, string $because): Descripti
         return new Description("{$theClass->getName()} should not be abstract", $because);
     }
 
+    public function appliesTo(ClassDescription $theClass): bool
+    {
+        return !($theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum() || $theClass->isFinal());
+    }
+
     public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
     {
         if (!$theClass->isAbstract()) {
diff --git a/src/Expression/ForClasses/IsNotReadonly.php b/src/Expression/ForClasses/IsNotReadonly.php
@@ -18,6 +18,11 @@ public function describe(ClassDescription $theClass, string $because): Descripti
         return new Description("{$theClass->getName()} should not be readonly", $because);
     }
 
+    public function appliesTo(ClassDescription $theClass): bool
+    {
+        return !($theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum());
+    }
+
     public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
     {
         if (!$theClass->isReadonly()) {
diff --git a/src/Expression/ForClasses/IsReadonly.php b/src/Expression/ForClasses/IsReadonly.php
@@ -18,9 +18,14 @@ public function describe(ClassDescription $theClass, string $because): Descripti
         return new Description("{$theClass->getName()} should be readonly", $because);
     }
 
+    public function appliesTo(ClassDescription $theClass): bool
+    {
+        return !($theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum());
+    }
+
     public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
     {
-        if ($theClass->isReadonly() || $theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum()) {
+        if ($theClass->isReadonly()) {
             return;
         }
 
diff --git a/tests/E2E/PHPUnit/CheckClassHaveAttributeTest.php b/tests/E2E/PHPUnit/CheckClassHaveAttributeTest.php
@@ -4,7 +4,6 @@
 
 namespace Arkitect\Tests\E2E\PHPUnit;
 
-use Arkitect\ClassSet;
 use Arkitect\Expression\ForClasses\HaveAttribute;
 use Arkitect\Expression\ForClasses\HaveNameMatching;
 use Arkitect\Expression\ForClasses\ResideInOneOfTheseNamespaces;
@@ -18,14 +17,12 @@ public function test_entities_should_reside_in_app_model(): void
     {
         $runner = TestRunner::create('8.4');
 
-        $set = ClassSet::fromDir(__DIR__.'/../_fixtures/mvc');
-
         $rule = Rule::allClasses()
             ->that(new HaveAttribute('Entity'))
             ->should(new ResideInOneOfTheseNamespaces('App\Model'))
             ->because('we use an ORM');
 
-        $runner->run($set, $rule);
+        $runner->run(__DIR__.'/../_fixtures/mvc', $rule);
 
         $this->assertCount(0, $runner->getViolations());
         $this->assertCount(0, $runner->getParsingErrors());
@@ -35,14 +32,12 @@ public function test_controllers_should_have_name_ending_in_controller(): void
     {
         $runner = TestRunner::create('8.4');
 
-        $set = ClassSet::fromDir(__DIR__.'/../_fixtures/mvc');
-
         $rule = Rule::allClasses()
             ->that(new HaveAttribute('AsController'))
             ->should(new HaveNameMatching('*Controller'))
             ->because('its a symfony thing');
 
-        $runner->run($set, $rule);
+        $runner->run(__DIR__.'/../_fixtures/mvc', $rule);
 
         $this->assertCount(1, $runner->getViolations());
         $this->assertCount(0, $runner->getParsingErrors());
@@ -54,14 +49,12 @@ public function test_controllers_should_have_controller_attribute(): void
     {
         $runner = TestRunner::create('8.4');
 
-        $set = ClassSet::fromDir(__DIR__.'/../_fixtures/mvc');
-
         $rule = Rule::allClasses()
             ->that(new HaveNameMatching('*Controller'))
             ->should(new HaveAttribute('AsController'))
             ->because('it configures the service container');
 
-        $runner->run($set, $rule);
+        $runner->run(__DIR__.'/../_fixtures/mvc', $rule);
 
         $this->assertCount(0, $runner->getViolations());
         $this->assertCount(0, $runner->getParsingErrors());
diff --git a/tests/Integration/CheckAttributeDependencyTest.php b/tests/Integration/CheckAttributeDependencyTest.php
@@ -2,9 +2,8 @@
 
 declare(strict_types=1);
 
-namespace Arkitect\Tests\E2E\PHPUnit;
+namespace Arkitect\Tests\Integration;
 
-use Arkitect\ClassSet;
 use Arkitect\Expression\ForClasses\NotDependsOnTheseNamespaces;
 use Arkitect\Expression\ForClasses\ResideInOneOfTheseNamespaces;
 use Arkitect\Rules\Rule;
@@ -20,14 +19,12 @@ public function test_assertion_should_fail_on_invalid_dependency(): void
 
         $runner = TestRunner::create('8.4');
 
-        $set = ClassSet::fromDir($dir);
-
         $rule = Rule::allClasses()
             ->that(new ResideInOneOfTheseNamespaces('App'))
             ->should(new NotDependsOnTheseNamespaces('App\Invalid'))
             ->because('i said so');
 
-        $runner->run($set, $rule);
+        $runner->run($dir, $rule);
 
         $this->assertCount(1, $runner->getViolations());
         $this->assertCount(0, $runner->getParsingErrors());
diff --git a/tests/Integration/IsAbstractTest.php b/tests/Integration/IsAbstractTest.php
@@ -2,9 +2,8 @@
 
 declare(strict_types=1);
 
-namespace Arkitect\Tests\E2E\PHPUnit;
+namespace Arkitect\Tests\Integration;
 
-use Arkitect\ClassSet;
 use Arkitect\Expression\ForClasses\HaveNameMatching;
 use Arkitect\Expression\ForClasses\IsAbstract;
 use Arkitect\Expression\ForClasses\IsNotAbstract;
@@ -25,14 +24,12 @@ public function test_is_abstract_in_that_should_not_consider_final_traits_enums_
     {
         $runner = TestRunner::create('8.4');
 
-        $set = ClassSet::fromDir($this->createClasses());
-
         $rule = Rule::allClasses()
             ->that(new IsAbstract())
             ->should(new HaveNameMatching('*Abstract'))
             ->because('we want to prefix abstract classes');
 
-        $runner->run($set, $rule);
+        $runner->run($this->createClasses(), $rule);
 
         $this->assertCount(0, $runner->getViolations());
         $this->assertCount(0, $runner->getParsingErrors());
@@ -42,14 +39,12 @@ public function test_is_abstract_in_should_should_consider_final_traits_enums_in
     {
         $runner = TestRunner::create('8.4');
 
-        $set = ClassSet::fromDir($this->createClasses());
-
         $rule = Rule::allClasses()
             ->that(new HaveNameMatching('My*'))
             ->should(new IsAbstract())
             ->because('everything in the app namespace should be abstract');
 
-        $runner->run($set, $rule);
+        $runner->run($this->createClasses(), $rule);
 
         $this->assertCount(4, $runner->getViolations());
         $this->assertCount(0, $runner->getParsingErrors());
@@ -64,14 +59,12 @@ public function test_is_not_abstract_in_should_should_consider_final_traits_enum
     {
         $runner = TestRunner::create('8.4');
 
-        $set = ClassSet::fromDir($this->createClasses());
-
         $rule = Rule::allClasses()
             ->that(new HaveNameMatching('My*'))
             ->should(new IsNotAbstract())
             ->because('everything in the app namespace should be abstract');
 
-        $runner->run($set, $rule);
+        $runner->run($this->createClasses(), $rule);
 
         $this->assertCount(1, $runner->getViolations());
         $this->assertCount(0, $runner->getParsingErrors());
@@ -144,12 +137,8 @@ public function __construct(HappyClass $happy)
             ],
         ];
 
-        $dir = vfsStream::setup('root', null, $structure)->url();
-
         $runner = TestRunner::create('8.4');
 
-        $set = ClassSet::fromDir($dir);
-
         $rule = Rule::allClasses()
             ->that(new ResideInOneOfTheseNamespaces('App\BadCode'))
             ->andThat(new ResideInOneOfTheseNamespaces('App\HappyIsland'))
@@ -161,7 +150,7 @@ public function __construct(HappyClass $happy)
             ->andShould(new IsNotTrait())
             ->because('some reason');
 
-        $runner->run($set, $rule);
+        $runner->run(vfsStream::setup('root', null, $structure)->url(), $rule);
 
         $this->assertCount(0, $runner->getViolations());
         $this->assertCount(0, $runner->getParsingErrors());
diff --git a/tests/Integration/IsFinalTest.php b/tests/Integration/IsFinalTest.php
@@ -0,0 +1,83 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Arkitect\Tests\Integration;
+
+use Arkitect\Expression\ForClasses\HaveNameMatching;
+use Arkitect\Expression\ForClasses\IsFinal;
+use Arkitect\Expression\ForClasses\IsNotFinal;
+use Arkitect\Rules\Rule;
+use Arkitect\Tests\Utils\TestRunner;
+use org\bovigo\vfs\vfsStream;
+use PHPUnit\Framework\TestCase;
+
+class IsFinalTest extends TestCase
+{
+    public function test_is_final_in_that_should_not_consider_abstract_traits_enums_interfaces(): void
+    {
+        $runner = TestRunner::create('8.4');
+
+        $rule = Rule::allClasses()
+            ->that(new IsFinal())
+            ->should(new HaveNameMatching('*Final'))
+            ->because('we want to prefix final classes');
+
+        $runner->run($this->createClasses(), $rule);
+
+        $this->assertCount(0, $runner->getViolations());
+        $this->assertCount(0, $runner->getParsingErrors());
+    }
+
+    public function test_is_final_in_should_should_consider_final_traits_enums_interfaces(): void
+    {
+        $runner = TestRunner::create('8.4');
+
+        $rule = Rule::allClasses()
+            ->that(new HaveNameMatching('My*'))
+            ->should(new IsFinal())
+            ->because('everything in the app namespace should be final');
+
+        $runner->run($this->createClasses(), $rule);
+
+        $this->assertCount(4, $runner->getViolations());
+        $this->assertCount(0, $runner->getParsingErrors());
+
+        $this->assertEquals('App\MyAbstract', $runner->getViolations()->get(0)->getFqcn());
+        $this->assertEquals('App\MyEnum', $runner->getViolations()->get(1)->getFqcn());
+        $this->assertEquals('App\MyInterface', $runner->getViolations()->get(2)->getFqcn());
+        $this->assertEquals('App\MyTrait', $runner->getViolations()->get(3)->getFqcn());
+    }
+
+    public function test_is_not_final_in_should_should_consider_final_traits_enums_interfaces(): void
+    {
+        $runner = TestRunner::create('8.4');
+
+        $rule = Rule::allClasses()
+            ->that(new HaveNameMatching('My*'))
+            ->should(new IsNotFinal())
+            ->because('everything in the app namespace should not be final');
+
+        $runner->run($this->createClasses(), $rule);
+
+        $this->assertCount(1, $runner->getViolations());
+        $this->assertCount(0, $runner->getParsingErrors());
+
+        $this->assertEquals('App\MyFinal', $runner->getViolations()->get(0)->getFqcn());
+    }
+
+    protected function createClasses(): string
+    {
+        $structure = [
+            'App' => [
+                'MyAbstract.php' => '<?php namespace App { abstract class MyAbstract {} };',
+                'MyFinal.php' => '<?php namespace App { final class MyFinal {} };',
+                'MyInterface.php' => '<?php namespace App { interface MyInterface {} };',
+                'MyEnum.php' => '<?php namespace App { enum MyEnum {} };',
+                'MyTrait.php' => '<?php namespace App { trait MyTrait {} };',
+            ],
+        ];
+
+        return vfsStream::setup('root', null, $structure)->url();
+    }
+}
diff --git a/tests/Integration/IsReadonlyTest.php b/tests/Integration/IsReadonlyTest.php
@@ -0,0 +1,81 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Arkitect\Tests\Integration;
+
+use Arkitect\Expression\ForClasses\HaveNameMatching;
+use Arkitect\Expression\ForClasses\IsNotReadonly;
+use Arkitect\Expression\ForClasses\IsReadonly;
+use Arkitect\Rules\Rule;
+use Arkitect\Tests\Utils\TestRunner;
+use org\bovigo\vfs\vfsStream;
+use PHPUnit\Framework\TestCase;
+
+class IsReadonlyTest extends TestCase
+{
+    public function test_is_readonly_in_that_should_not_consider_traits_enums_interfaces(): void
+    {
+        $runner = TestRunner::create('8.4');
+
+        $rule = Rule::allClasses()
+            ->that(new IsReadonly())
+            ->should(new HaveNameMatching('*Readonly'))
+            ->because('we want to prefix readonly classes');
+
+        $runner->run($this->createClasses(), $rule);
+
+        $this->assertCount(0, $runner->getViolations());
+        $this->assertCount(0, $runner->getParsingErrors());
+    }
+
+    public function test_is_readonly_in_should_should_consider_traits_enums_interfaces(): void
+    {
+        $runner = TestRunner::create('8.4');
+
+        $rule = Rule::allClasses()
+            ->that(new HaveNameMatching('My*'))
+            ->should(new IsReadonly())
+            ->because('everything in the app namespace should be readonly');
+
+        $runner->run($this->createClasses(), $rule);
+
+        $this->assertCount(3, $runner->getViolations());
+        $this->assertCount(0, $runner->getParsingErrors());
+
+        $this->assertEquals('App\MyEnum', $runner->getViolations()->get(0)->getFqcn());
+        $this->assertEquals('App\MyInterface', $runner->getViolations()->get(1)->getFqcn());
+        $this->assertEquals('App\MyTrait', $runner->getViolations()->get(2)->getFqcn());
+    }
+
+    public function test_is_not_readonly_in_should_should_consider_traits_enums_interfaces(): void
+    {
+        $runner = TestRunner::create('8.4');
+
+        $rule = Rule::allClasses()
+            ->that(new HaveNameMatching('My*'))
+            ->should(new IsNotReadonly())
+            ->because('everything in the app namespace should not be final');
+
+        $runner->run($this->createClasses(), $rule);
+
+        $this->assertCount(1, $runner->getViolations());
+        $this->assertCount(0, $runner->getParsingErrors());
+
+        $this->assertEquals('App\MyReadonly', $runner->getViolations()->get(0)->getFqcn());
+    }
+
+    protected function createClasses(): string
+    {
+        $structure = [
+            'App' => [
+                'MyInterface.php' => '<?php namespace App { interface MyInterface {} };',
+                'MyEnum.php' => '<?php namespace App { enum MyEnum {} };',
+                'MyTrait.php' => '<?php namespace App { trait MyTrait {} };',
+                'MyReadonly.php' => '<?php namespace App { readonly class MyReadonly {} };',
+            ],
+        ];
+
+        return vfsStream::setup('root', null, $structure)->url();
+    }
+}
diff --git a/tests/Unit/Expressions/ForClasses/IsFinalTest.php b/tests/Unit/Expressions/ForClasses/IsFinalTest.php
diff --git a/tests/Unit/Expressions/ForClasses/IsNotFinalTest.php b/tests/Unit/Expressions/ForClasses/IsNotFinalTest.php
diff --git a/tests/Unit/Expressions/ForClasses/IsReadonlyTest.php b/tests/Unit/Expressions/ForClasses/IsReadonlyTest.php
diff --git a/tests/Utils/TestRunner.php b/tests/Utils/TestRunner.php

Original file line number	Diff line number	Diff line change
`@@ -18,10 +18,14 @@ public function describe(ClassDescription $theClass, string $because): Descripti`
`18`	`18`	`return new Description("{$theClass->getName()} should be final", $because);`
`19`	`19`	`}`
`20`	`20`
	`21`	`+ public function appliesTo(ClassDescription $theClass): bool`
	`22`	`+ {`
	`23`	`+ return !($theClass->isInterface() \|\| $theClass->isTrait() \|\| $theClass->isEnum() \|\| $theClass->isAbstract());`
	`24`	`+ }`
	`25`	`+`
`21`	`26`	`public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void`
`22`	`27`	`{`
`23`		`- if ($theClass->isAbstract() \|\| $theClass->isInterface() \|\| $theClass->isFinal() \|\| $theClass->isTrait()`
`24`		`- \|\| $theClass->isEnum()) {`
	`28`	`+ if ($theClass->isFinal()) {`
`25`	`29`	`return;`
`26`	`30`	`}`
`27`	`31`
Original file line number	Diff line number	Diff line change
`@@ -18,6 +18,11 @@ public function describe(ClassDescription $theClass, string $because): Descripti`
`18`	`18`	`return new Description("{$theClass->getName()} should not be abstract", $because);`
`19`	`19`	`}`
`20`	`20`
	`21`	`+ public function appliesTo(ClassDescription $theClass): bool`
	`22`	`+ {`
	`23`	`+ return !($theClass->isInterface() \|\| $theClass->isTrait() \|\| $theClass->isEnum() \|\| $theClass->isFinal());`
	`24`	`+ }`
	`25`	`+`
`21`	`26`	`public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void`
`22`	`27`	`{`
`23`	`28`	`if (!$theClass->isAbstract()) {`
Original file line number	Diff line number	Diff line change
`@@ -18,9 +18,14 @@ public function describe(ClassDescription $theClass, string $because): Descripti`
`18`	`18`	`return new Description("{$theClass->getName()} should be readonly", $because);`
`19`	`19`	`}`
`20`	`20`
	`21`	`+ public function appliesTo(ClassDescription $theClass): bool`
	`22`	`+ {`
	`23`	`+ return !($theClass->isInterface() \|\| $theClass->isTrait() \|\| $theClass->isEnum());`
	`24`	`+ }`
	`25`	`+`
`21`	`26`	`public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void`
`22`	`27`	`{`
`23`		`- if ($theClass->isReadonly() \|\| $theClass->isInterface() \|\| $theClass->isTrait() \|\| $theClass->isEnum()) {`
	`28`	`+ if ($theClass->isReadonly()) {`
`24`	`29`	`return;`
`25`	`30`	`}`
`26`	`31`