diff --git a/src/RuleBuilders/Architecture/Architecture.php b/src/RuleBuilders/Architecture/Architecture.php index ab23c935..834ef433 100644 --- a/src/RuleBuilders/Architecture/Architecture.php +++ b/src/RuleBuilders/Architecture/Architecture.php @@ -3,11 +3,9 @@ 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; @@ -15,19 +13,19 @@ class Architecture implements Component, DefinedBy, Where, MayDependOnComponents { /** @var string */ private $componentName; - /** @var array */ + /** @var array */ private $componentSelectors; /** @var array */ private $allowedDependencies; /** @var array */ - 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 new file mode 100644 index 00000000..d2918add --- /dev/null +++ b/tests/Fixtures/Fruit/AnimalFruit.php @@ -0,0 +1,20 @@ +cat = $cat; + } +} diff --git a/tests/Unit/Architecture/ArchitectureTest.php b/tests/Unit/Architecture/ArchitectureTest.php index 4c810be4..9dcb3720 100644 --- 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 index 0e78786b..06d46de8 100644 --- 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