fixup!

phparkitect · Sep 18, 2023 · 2f39bc5 · 2f39bc5
1 parent 972b305
commit 2f39bc5
Show file tree

Hide file tree

Showing 4 changed files with 181 additions and 67 deletions.
diff --git a/src/RuleBuilders/Architecture/Architecture.php b/src/RuleBuilders/Architecture/Architecture.php
@@ -3,31 +3,29 @@
 
 namespace Arkitect\RuleBuilders\Architecture;
 
-use Arkitect\Expression\Boolean\Andx;
-use Arkitect\Expression\Boolean\Not;
+use Arkitect\Expression\Boolean\Orx;
 use Arkitect\Expression\Expression;
-use Arkitect\Expression\ForClasses\DependsOnlyOnTheseNamespaces;
-use Arkitect\Expression\ForClasses\NotDependsOnTheseNamespaces;
+use Arkitect\Expression\ForClasses\DependsOnlyOnTheseExpressions;
 use Arkitect\Expression\ForClasses\ResideInOneOfTheseNamespaces;
 use Arkitect\Rules\Rule;
 
 class Architecture implements Component, DefinedBy, Where, MayDependOnComponents, MayDependOnAnyComponent, ShouldNotDependOnAnyComponent, ShouldOnlyDependOnComponents, Rules
 {
     /** @var string */
     private $componentName;
-    /** @var array<string, string> */
+    /** @var array<string, Expression|string> */
     private $componentSelectors;
     /** @var array<string, string[]> */
     private $allowedDependencies;
     /** @var array<string, string[]> */
-    private $componentDependsOnlyOnTheseNamespaces;
+    private $componentDependsOnlyOnTheseComponents;
 
     private function __construct()
     {
         $this->componentName = '';
         $this->componentSelectors = [];
         $this->allowedDependencies = [];
-        $this->componentDependsOnlyOnTheseNamespaces = [];
+        $this->componentDependsOnlyOnTheseComponents = [];
     }
 
     public static function withComponents(): Component
@@ -72,7 +70,7 @@ public function shouldNotDependOnAnyComponent()
 
     public function shouldOnlyDependOnComponents(string ...$componentNames)
     {
-        $this->componentDependsOnlyOnTheseNamespaces[$this->componentName] = $componentNames;
+        $this->componentDependsOnlyOnTheseComponents[$this->componentName] = $componentNames;
 
         return $this;
     }
@@ -93,63 +91,61 @@ public function mayDependOnAnyComponent()
 
     public function rules(): iterable
     {
-        $layerNames = array_keys($this->componentSelectors);
-
         foreach ($this->componentSelectors as $name => $selector) {
             if (isset($this->allowedDependencies[$name])) {
-                $forbiddenComponents = array_diff($layerNames, [$name], $this->allowedDependencies[$name]);
-
-                if (!empty($forbiddenComponents)) {
-                    yield Rule::allClasses()
-                        ->that(\is_string($selector) ? new ResideInOneOfTheseNamespaces($selector) : $selector)
-                        ->should($this->createForbiddenExpression($forbiddenComponents))
-                        ->because('of component architecture');
-                }
+                yield Rule::allClasses()
+                    ->that(\is_string($selector) ? new ResideInOneOfTheseNamespaces($selector) : $selector)
+                    ->should($this->createAllowedExpression(
+                        array_merge([$name], $this->allowedDependencies[$name])
+                    ))
+                    ->because('of component architecture');
             }
 
-            if (!isset($this->componentDependsOnlyOnTheseNamespaces[$name])) {
-                continue;
+            if (isset($this->componentDependsOnlyOnTheseComponents[$name])) {
+                yield Rule::allClasses()
+                    ->that(\is_string($selector) ? new ResideInOneOfTheseNamespaces($selector) : $selector)
+                    ->should($this->createAllowedExpression($this->componentDependsOnlyOnTheseComponents[$name]))
+                    ->because('of component architecture');
             }
+        }
+    }
 
-            $allowedDependencies = array_map(function (string $componentName): string {
-                return $this->componentSelectors[$componentName];
-            }, $this->componentDependsOnlyOnTheseNamespaces[$name]);
+    private function createAllowedExpression(array $components): Expression
+    {
+        $namespaceSelectors = $this->extractComponentsNamespaceSelectors($components);
+
+        $expressionSelectors = $this->extractComponentExpressionSelectors($components);
+
+        if ([] === $namespaceSelectors && [] === $expressionSelectors) {
+            return new Orx(); // always true
+        }
 
-            yield Rule::allClasses()
-                ->that(new ResideInOneOfTheseNamespaces($selector))
-                ->should(new DependsOnlyOnTheseNamespaces(...$allowedDependencies))
-                ->because('of component architecture');
+        if ([] !== $namespaceSelectors) {
+            $expressionSelectors[] = new ResideInOneOfTheseNamespaces(...$namespaceSelectors);
         }
+
+        return new DependsOnlyOnTheseExpressions(...$expressionSelectors);
     }
 
-    public function createForbiddenExpression(array $forbiddenComponents): Expression
+    private function extractComponentsNamespaceSelectors(array $components): array
     {
-        $forbiddenNamespaceSelectors = array_filter(
+        return array_filter(
             array_map(function (string $componentName): ?string {
                 $selector = $this->componentSelectors[$componentName];
 
                 return \is_string($selector) ? $selector : null;
-            }, $forbiddenComponents)
+            }, $components)
         );
+    }
 
-        $forbiddenExpressionSelectors = array_filter(
+    private function extractComponentExpressionSelectors(array $components): array
+    {
+        return array_filter(
             array_map(function (string $componentName): ?Expression {
                 $selector = $this->componentSelectors[$componentName];
 
                 return \is_string($selector) ? null : $selector;
-            }, $forbiddenComponents)
+            }, $components)
         );
-
-        $forbiddenExpressionList = [];
-        if ([] !== $forbiddenNamespaceSelectors) {
-            $forbiddenExpressionList[] = new NotDependsOnTheseNamespaces(...$forbiddenNamespaceSelectors);
-        }
-        if ([] !== $forbiddenExpressionSelectors) {
-            $forbiddenExpressionList[] = new Not(new Andx(...$forbiddenExpressionSelectors));
-        }
-
-        return 1 === \count($forbiddenExpressionList)
-            ? array_pop($forbiddenExpressionList)
-            : new Andx(...$forbiddenExpressionList);
     }
 }
diff --git a/tests/Fixtures/Fruit/AnimalFruit.php b/tests/Fixtures/Fruit/AnimalFruit.php
@@ -0,0 +1,20 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Arkitect\Tests\Fixtures\Fruit;
+
+use Arkitect\Tests\Fixtures\Animal\Cat;
+
+final class AnimalFruit extends Banana
+{
+    /**
+     * @var Cat
+     */
+    private $cat;
+
+    public function __construct(Cat $cat)
+    {
+        $this->cat = $cat;
+    }
+}
diff --git a/tests/Unit/Architecture/ArchitectureTest.php b/tests/Unit/Architecture/ArchitectureTest.php
@@ -3,10 +3,7 @@
 
 namespace Arkitect\Tests\Unit\Architecture;
 
-use Arkitect\Expression\Boolean\Andx;
-use Arkitect\Expression\Boolean\Not;
-use Arkitect\Expression\ForClasses\DependsOnlyOnTheseNamespaces;
-use Arkitect\Expression\ForClasses\NotDependsOnTheseNamespaces;
+use Arkitect\Expression\ForClasses\DependsOnlyOnTheseExpressions;
 use Arkitect\Expression\ForClasses\ResideInOneOfTheseNamespaces;
 use Arkitect\RuleBuilders\Architecture\Architecture;
 use Arkitect\Rules\Rule;
@@ -30,15 +27,26 @@ public function test_layered_architecture(): void
         $expectedRules = [
             Rule::allClasses()
                 ->that(new ResideInOneOfTheseNamespaces('App\*\Domain\*'))
-                ->should(new NotDependsOnTheseNamespaces('App\*\Application\*', 'App\*\Infrastructure\*'))
+                ->should(new DependsOnlyOnTheseExpressions(
+                    new ResideInOneOfTheseNamespaces('App\*\Domain\*')
+                ))
                 ->because('of component architecture'),
             Rule::allClasses()
                 ->that(new ResideInOneOfTheseNamespaces('App\*\Application\*'))
-                ->should(new NotDependsOnTheseNamespaces('App\*\Infrastructure\*'))
+                ->should(new DependsOnlyOnTheseExpressions(
+                    new ResideInOneOfTheseNamespaces('App\*\Application\*', 'App\*\Domain\*')
+                ))
+                ->because('of component architecture'),
+            Rule::allClasses()
+                ->that(new ResideInOneOfTheseNamespaces('App\*\Infrastructure\*'))
+                ->should(new DependsOnlyOnTheseExpressions(
+                    new ResideInOneOfTheseNamespaces('App\*\Infrastructure\*', 'App\*\Domain\*', 'App\*\Application\*')
+                ))
                 ->because('of component architecture'),
         ];
 
-        self::assertEquals($expectedRules, iterator_to_array($rules));
+        $actualRules = iterator_to_array($rules);
+        self::assertEquals($expectedRules, $actualRules);
     }
 
     public function test_layered_architecture_with_expression(): void
@@ -58,20 +66,26 @@ public function test_layered_architecture_with_expression(): void
         $expectedRules = [
             Rule::allClasses()
                 ->that(new ResideInOneOfTheseNamespaces('App\*\Domain\*'))
-                ->should(new Not(new Andx(
-                    new ResideInOneOfTheseNamespaces('App\*\Application\*'),
-                    new ResideInOneOfTheseNamespaces('App\*\Infrastructure\*')
-                )))
+                ->should(new DependsOnlyOnTheseExpressions(
+                    new ResideInOneOfTheseNamespaces('App\*\Domain\*')
+                ))
                 ->because('of component architecture'),
             Rule::allClasses()
                 ->that(new ResideInOneOfTheseNamespaces('App\*\Application\*'))
-                ->should(new Not(new Andx(
-                    new ResideInOneOfTheseNamespaces('App\*\Infrastructure\*')
-                )))
+                ->should(new DependsOnlyOnTheseExpressions(
+                    new ResideInOneOfTheseNamespaces('App\*\Application\*', 'App\*\Domain\*')
+                ))
+                ->because('of component architecture'),
+            Rule::allClasses()
+                ->that(new ResideInOneOfTheseNamespaces('App\*\Infrastructure\*'))
+                ->should(new DependsOnlyOnTheseExpressions(
+                    new ResideInOneOfTheseNamespaces('App\*\Infrastructure\*', 'App\*\Domain\*', 'App\*\Application\*')
+                ))
                 ->because('of component architecture'),
         ];
 
-        self::assertEquals($expectedRules, iterator_to_array($rules));
+        $actualRules = iterator_to_array($rules);
+        self::assertEquals($expectedRules, $actualRules);
     }
 
     public function test_layered_architecture_with_mix_of_namespace_and_expression(): void
@@ -90,20 +104,26 @@ public function test_layered_architecture_with_mix_of_namespace_and_expression()
         $expectedRules = [
             Rule::allClasses()
                 ->that(new ResideInOneOfTheseNamespaces('App\*\Domain\*'))
-                ->should(new Andx(
-                    new NotDependsOnTheseNamespaces('App\*\Infrastructure\*'),
-                    new Not(new Andx(
-                        new ResideInOneOfTheseNamespaces('App\*\Application\*')
-                    ))
+                ->should(new DependsOnlyOnTheseExpressions(
+                    new ResideInOneOfTheseNamespaces('App\*\Domain\*')
                 ))
                 ->because('of component architecture'),
             Rule::allClasses()
                 ->that(new ResideInOneOfTheseNamespaces('App\*\Application\*'))
-                ->should(new NotDependsOnTheseNamespaces('App\*\Infrastructure\*'))
+                ->should(new DependsOnlyOnTheseExpressions(
+                    new ResideInOneOfTheseNamespaces('App\*\Application\*', 'App\*\Domain\*')
+                ))
+                ->because('of component architecture'),
+            Rule::allClasses()
+                ->that(new ResideInOneOfTheseNamespaces('App\*\Infrastructure\*'))
+                ->should(new DependsOnlyOnTheseExpressions(
+                    new ResideInOneOfTheseNamespaces('App\*\Domain\*', 'App\*\Application\*', 'App\*\Infrastructure\*')
+                ))
                 ->because('of component architecture'),
         ];
 
-        self::assertEquals($expectedRules, iterator_to_array($rules));
+        $actualRules = iterator_to_array($rules);
+        self::assertEquals($expectedRules, $actualRules);
     }
 
     public function test_layered_architecture_with_depends_only_on_components(): void
@@ -117,7 +137,7 @@ public function test_layered_architecture_with_depends_only_on_components(): voi
         $expectedRules = [
             Rule::allClasses()
                 ->that(new ResideInOneOfTheseNamespaces('App\*\Domain\*'))
-                ->should(new DependsOnlyOnTheseNamespaces('App\*\Domain\*'))
+                ->should(new DependsOnlyOnTheseExpressions(new ResideInOneOfTheseNamespaces('App\*\Domain\*')))
                 ->because('of component architecture'),
         ];
 

diff --git a/tests/Unit/Rules/RuleCheckerTest.php b/tests/Unit/Rules/RuleCheckerTest.php
@@ -4,15 +4,26 @@
 namespace Arkitect\Tests\Unit\Rules;
 
 use Arkitect\Analyzer\ClassDescription;
+use Arkitect\Analyzer\FileParserFactory;
 use Arkitect\Analyzer\Parser;
 use Arkitect\ClassSet;
 use Arkitect\ClassSetRules;
 use Arkitect\CLI\Progress\VoidProgress;
 use Arkitect\CLI\Runner;
+use Arkitect\CLI\TargetPhpVersion;
+use Arkitect\Expression\ForClasses\HaveNameMatching;
+use Arkitect\Expression\ForClasses\Implement;
+use Arkitect\Expression\ForClasses\ResideInOneOfTheseNamespaces;
 use Arkitect\Rules\DSL\ArchRule;
 use Arkitect\Rules\ParsingErrors;
+use Arkitect\Rules\Rule;
 use Arkitect\Rules\Violation;
 use Arkitect\Rules\Violations;
+use Arkitect\Tests\Fixtures\Animal\AnimalInterface;
+use Arkitect\Tests\Fixtures\Fruit\AnimalFruit;
+use Arkitect\Tests\Fixtures\Fruit\CavendishBanana;
+use Arkitect\Tests\Fixtures\Fruit\DwarfCavendishBanana;
+use Arkitect\Tests\Fixtures\Fruit\FruitInterface;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Finder\SplFileInfo;
 
@@ -37,6 +48,73 @@ public function test_should_run_parse_on_all_files_in_class_set(): void
 
         self::assertCount(3, $violations);
     }
+
+    public function test_can_exclude_files_or_directories_from_multiple_dir_class_set_with_no_violations(): void
+    {
+        $classSet = ClassSet::fromDir(\FIXTURES_PATH);
+
+        $rules[] = Rule::allClasses()
+            ->except(FruitInterface::class, CavendishBanana::class, DwarfCavendishBanana::class, AnimalFruit::class)
+            ->that(new ResideInOneOfTheseNamespaces('Arkitect\Tests\Fixtures\Fruit'))
+            ->should(new Implement(FruitInterface::class))
+            ->because('this tests that string exceptions fail');
+
+        $rules[] = Rule::allClasses()
+            ->exceptExpression(new HaveNameMatching('*TestCase'))
+            ->that(new ResideInOneOfTheseNamespaces('Arkitect\Tests\Fixtures\Animal'))
+            ->should(new Implement(AnimalInterface::class))
+            ->because('this tests that expression exceptions fail');
+
+        $runner = new Runner();
+
+        $runner->check(
+            ClassSetRules::create($classSet, ...$rules),
+            new VoidProgress(),
+            FileParserFactory::createFileParser(TargetPhpVersion::create(null)),
+            $violations = new Violations(),
+            new ParsingErrors()
+        );
+
+        self::assertCount(0, $violations);
+    }
+
+    public function test_can_exclude_files_or_directories_from_multiple_dir_class_set_with_violations(): void
+    {
+        $classSet = ClassSet::fromDir(\FIXTURES_PATH);
+
+        $rules[] = Rule::allClasses()
+            ->except(FruitInterface::class, CavendishBanana::class, AnimalFruit::class)
+            ->that(new ResideInOneOfTheseNamespaces('Arkitect\Tests\Fixtures\Fruit'))
+            ->should(new Implement(FruitInterface::class))
+            ->because('this tests that string exceptions fail');
+
+        $rules[] = Rule::allClasses()
+            ->exceptExpression(new HaveNameMatching('*NotExistingSoItFails'))
+            ->that(new ResideInOneOfTheseNamespaces('Arkitect\Tests\Fixtures\Animal'))
+            ->should(new Implement(AnimalInterface::class))
+            ->because('this tests that expression exceptions fail');
+
+        $runner = new Runner();
+
+        $runner->check(
+            ClassSetRules::create($classSet, ...$rules),
+            new VoidProgress(),
+            FileParserFactory::createFileParser(TargetPhpVersion::create(null)),
+            $violations = new Violations(),
+            new ParsingErrors()
+        );
+
+        self::assertCount(2, $violations);
+        $expectedViolations = "Arkitect\Tests\Fixtures\Animal\CatTestCase has 1 violations
+            should implement Arkitect\Tests\Fixtures\Animal\AnimalInterface because this tests
+            that expression exceptions fail Arkitect\Tests\Fixtures\Fruit\DwarfCavendishBanana has 1 violations
+            should implement Arkitect\Tests\Fixtures\Fruit\FruitInterface because
+            this tests that string exceptions fail";
+        self::assertEquals(
+            preg_replace('/\s+/', ' ', $expectedViolations),
+            preg_replace('/\s+/', ' ', trim($violations->toString()))
+        );
+    }
 }
 
 class FakeClassSet extends ClassSet