diff --git a/docs/assets/no-violation.svg b/docs/assets/no-violation.svg index 602a9b30..e8ff697e 100644 --- a/docs/assets/no-violation.svg +++ b/docs/assets/no-violation.svg @@ -14,7 +14,7 @@ ➜ prj-ddd vendor/bin/structarmed analyze - StructArmed 0.13.7 — Architecture Enforcement + StructArmed 0.13.8 — Architecture Enforcement =============================================== diff --git a/docs/assets/structarmed-showoff.svg b/docs/assets/structarmed-showoff.svg index 6b1ed31a..c05d1317 100644 --- a/docs/assets/structarmed-showoff.svg +++ b/docs/assets/structarmed-showoff.svg @@ -15,7 +15,7 @@ ➜ prj-ddd vendor/bin/structarmed analyze - StructArmed 0.13.7 — Architecture Enforcement + StructArmed 0.13.8 — Architecture Enforcement =============================================== diff --git a/src/Analyser/Analyser.php b/src/Analyser/Analyser.php index ee8748d9..1ff4b164 100644 --- a/src/Analyser/Analyser.php +++ b/src/Analyser/Analyser.php @@ -34,7 +34,6 @@ use function array_merge; use function array_unique; use function array_values; -use function array_walk; use function count; use function fnmatch; use function getcwd; @@ -86,12 +85,17 @@ public function analyse( $projectRuleViolations = []; $fileAnalysisRules = []; + $layerAwareRules = []; foreach ($rules as $key => $rule) { if (array_key_exists($key, $skippedRuleKeys)) { continue; } + if ($rule instanceof LayerAwareRuleInterface) { + $layerAwareRules[] = $rule; + } + if (! $rule instanceof ProjectRuleInterface) { continue; } @@ -168,31 +172,24 @@ className: $violation->className, $hasRuleset = $ruleset !== []; $scanScopeLayerMap = $hasRuleset ? $this->scanScopeLayerMap($architecture) : []; - /** @var array $layerAwareRules */ - $layerAwareRules = array_filter( - $classRules, - fn(RuleInterface $rule): bool => $rule instanceof LayerAwareRuleInterface - ); $hasLayerAwareRules = $layerAwareRules !== []; $classDependencyMaps = $hasRuleset || $hasLayerAwareRules - ? $this->classDependencyMaps($classNodes) + ? $this->classDependencyMaps($classNodes, $hasRuleset, $hasLayerAwareRules) : [ 'dependencies' => [], 'inheritanceDependencies' => [], 'classLayerMap' => [], 'classPrimaryLayerMap' => [], + 'classNodeMap' => [], ]; $dependencyMap = $classDependencyMaps['dependencies']; $inheritanceDependencyMap = $classDependencyMaps['inheritanceDependencies']; $resolvedInheritedDependencies = []; - if ($hasLayerAwareRules) { - array_walk( - $layerAwareRules, - fn($rule) => $rule->injectClassLayerMap($classDependencyMaps['classLayerMap']) - ); + foreach ($layerAwareRules as $rule) { + $rule->injectClassNodeMap($classDependencyMaps['classNodeMap']); } foreach ($classNodes as $classNode) { @@ -455,17 +452,30 @@ private function ruleSkipMatchers(array $classRules, array $ruleSkipPaths): arra * dependencies: array>, * inheritanceDependencies: array>, * classLayerMap: array>, - * classPrimaryLayerMap: array + * classPrimaryLayerMap: array, + * classNodeMap: array * } */ - private function classDependencyMaps(array $classNodes): array - { + private function classDependencyMaps( + array $classNodes, + bool $collectRulesetMaps, + bool $collectClassNodeMap, + ): array { $dependencyMap = []; $inheritanceDependencyMap = []; $classLayerMap = []; $classPrimaryLayerMap = []; + $classNodeMap = []; foreach ($classNodes as $classNode) { + if ($collectClassNodeMap) { + $classNodeMap[$classNode->className] = $classNode; + } + + if (! $collectRulesetMaps) { + continue; + } + $dependencyMap[$classNode->className] = $classNode->dependencies; $dependencies = [ ...$classNode->implements, @@ -496,6 +506,7 @@ private function classDependencyMaps(array $classNodes): array 'inheritanceDependencies' => $inheritanceDependencyMap, 'classLayerMap' => $classLayerMap, 'classPrimaryLayerMap' => $classPrimaryLayerMap, + 'classNodeMap' => $classNodeMap, ]; } diff --git a/src/Rule/LayerAwareRuleInterface.php b/src/Rule/LayerAwareRuleInterface.php index d1513554..6f275c05 100644 --- a/src/Rule/LayerAwareRuleInterface.php +++ b/src/Rule/LayerAwareRuleInterface.php @@ -4,10 +4,10 @@ namespace Boundwize\StructArmed\Rule; +use Boundwize\StructArmed\Analyser\ClassNode; + interface LayerAwareRuleInterface { - /** - * @param array> $classLayerMap className → layer name(s) - */ - public function injectClassLayerMap(array $classLayerMap): void; + /** @param array $classNodeMap class name → class node */ + public function injectClassNodeMap(array $classNodeMap): void; } diff --git a/src/Rule/Rules/Layer/MayNotDependOnRule.php b/src/Rule/Rules/Layer/MayNotDependOnRule.php index 091edccd..b823c933 100644 --- a/src/Rule/Rules/Layer/MayNotDependOnRule.php +++ b/src/Rule/Rules/Layer/MayNotDependOnRule.php @@ -11,7 +11,6 @@ use Boundwize\StructArmed\Util\Path; use function in_array; -use function is_array; use function sprintf; use function str_contains; use function str_starts_with; @@ -20,8 +19,8 @@ final class MayNotDependOnRule implements MultipleRuleViolationInterface, LayerA { private readonly string $normalisedToPath; - /** @var array>|null */ - private ?array $classLayerMap = null; + /** @var array */ + private array $classNodeMap = []; public function __construct( private readonly string $from, @@ -31,14 +30,10 @@ public function __construct( $this->normalisedToPath = Path::normalise($toPath ?? $to); } - /** @param array> $classLayerMap */ - public function injectClassLayerMap(array $classLayerMap): void + /** @param array $classNodeMap */ + public function injectClassNodeMap(array $classNodeMap): void { - $this->classLayerMap = []; - - foreach ($classLayerMap as $className => $layers) { - $this->classLayerMap[$className] = is_array($layers) ? $layers : [$layers]; - } + $this->classNodeMap = $classNodeMap; } public function appliesTo(ClassNode $classNode): bool @@ -85,13 +80,11 @@ className: $classNode->className, private function isInForbiddenLayer(string $dependency): bool { - // Priority 1: Use class layer map if available - if ($this->classLayerMap !== null) { - $depLayers = $this->classLayerMap[$dependency] ?? null; + // Priority 1: Use the scanned dependency node if available + $dependencyNode = $this->classNodeMap[$dependency] ?? null; - if ($depLayers !== null) { - return in_array($this->to, $depLayers, true); - } + if ($dependencyNode instanceof ClassNode && $dependencyNode->layers !== []) { + return in_array($this->to, $dependencyNode->layers, true); } // Priority 2: Fallback to path matching diff --git a/tests/Analyser/AnalyserTest.php b/tests/Analyser/AnalyserTest.php index 8ac32120..b474a9e5 100644 --- a/tests/Analyser/AnalyserTest.php +++ b/tests/Analyser/AnalyserTest.php @@ -2449,7 +2449,7 @@ public function __construct(private OrderRepository $repo) {} public function testMayNotDependOnRuleDetectsViolationWhenDependencyMatchesSecondaryLayer(): void { // AuthTokenStore must be scanned so it gets a ClassNode with layers - // ['Support', 'Auth'] which is then stored in classLayerMap. + // ['Support', 'Auth'] which is then read from the dependency ClassNode. $basePath = $this->makeTempProject([ 'src/HTTP/LoginController.php' => <<<'PHP' $dependencies */ - private function makeNode(string $layer, array $dependencies = []): ClassNode - { + /** + * @param list $dependencies + * @param list $layers + */ + private function makeNode( + ?string $layer, + array $dependencies = [], + string $className = 'App\\Domain\\OrderService', + array $layers = [], + ): ClassNode { return new ClassNode( - className: 'App\\Domain\\OrderService', + className: $className, file: '/fake.php', line: 1, layer: $layer, @@ -28,6 +35,7 @@ className: 'App\\Domain\\OrderService', isInterface: false, isReadonly: false, dependencies: $dependencies, + layers: $layers, ); } @@ -116,12 +124,14 @@ public function testImplementsLayerAwareRuleInterface(): void ); } - public function testViolatesUsingClassLayerMapWhenInjected(): void + public function testViolatesUsingClassNodeMapWhenInjected(): void { $mayNotDependOnRule = new MayNotDependOnRule(from: 'Domain', to: 'Infrastructure'); - $mayNotDependOnRule->injectClassLayerMap([ - 'App\Infrastructure\Persistence\DoctrineOrderRepository' => 'Infrastructure', - ]); + $dependencyNode = $this->makeNode( + layer: 'Infrastructure', + className: 'App\Infrastructure\Persistence\DoctrineOrderRepository', + ); + $mayNotDependOnRule->injectClassNodeMap([$dependencyNode->className => $dependencyNode]); $classNode = $this->makeNode('Domain', [ 'App\Infrastructure\Persistence\DoctrineOrderRepository', ]); @@ -132,12 +142,11 @@ public function testViolatesUsingClassLayerMapWhenInjected(): void $this->assertStringContainsString('Infrastructure', $violation->message); } - public function testPassesUsingClassLayerMapWhenDependencyNotInForbiddenLayer(): void + public function testPassesUsingClassNodeMapWhenDependencyNotInForbiddenLayer(): void { $mayNotDependOnRule = new MayNotDependOnRule(from: 'Domain', to: 'Infrastructure'); - $mayNotDependOnRule->injectClassLayerMap([ - 'App\Domain\Order' => 'Domain', - ]); + $dependencyNode = $this->makeNode(layer: 'Domain', className: 'App\Domain\Order'); + $mayNotDependOnRule->injectClassNodeMap([$dependencyNode->className => $dependencyNode]); $classNode = $this->makeNode('Domain', [ 'App\Domain\Order', ]); @@ -145,23 +154,22 @@ public function testPassesUsingClassLayerMapWhenDependencyNotInForbiddenLayer(): $this->assertNotInstanceOf(RuleViolation::class, $mayNotDependOnRule->evaluate($classNode)); } - public function testClassLayerMapTakesPrecedenceOverToPath(): void + public function testClassNodeMapTakesPrecedenceOverToPath(): void { - // path would match 'App\Infrastructure\A', but map says it's in Domain — no violation + // Path would match 'App\Infrastructure\A', but its node is in Domain — no violation. $mayNotDependOnRule = new MayNotDependOnRule( from: 'Domain', to: 'Infrastructure', toPath: 'Infrastructure' ); - $mayNotDependOnRule->injectClassLayerMap([ - 'App\Infrastructure\A' => 'Domain', - ]); + $dependencyNode = $this->makeNode(layer: 'Domain', className: 'App\Infrastructure\A'); + $mayNotDependOnRule->injectClassNodeMap([$dependencyNode->className => $dependencyNode]); $classNode = $this->makeNode('Domain', ['App\Infrastructure\A']); $this->assertNotInstanceOf(RuleViolation::class, $mayNotDependOnRule->evaluate($classNode)); } - public function testViolatesUsingToAsPathFallbackWhenNoClassLayerMapMatch(): void + public function testViolatesUsingToAsPathFallbackWhenNoClassNodeMapMatch(): void { $mayNotDependOnRule = new MayNotDependOnRule(from: 'Domain', to: 'Infrastructure'); $classNode = $this->makeNode('Domain', [ @@ -171,14 +179,34 @@ public function testViolatesUsingToAsPathFallbackWhenNoClassLayerMapMatch(): voi $this->assertInstanceOf(RuleViolation::class, $mayNotDependOnRule->evaluate($classNode)); } - public function testViolatesWhenDependencyMatchesSecondaryLayerInClassLayerMap(): void + public function testFallsBackToPathWhenScannedDependencyHasNoLayer(): void + { + $mayNotDependOnRule = new MayNotDependOnRule( + from: 'Domain', + to: 'Infrastructure', + toPath: 'Infrastructure', + ); + $dependencyNode = $this->makeNode( + layer: null, + className: 'App\\Infrastructure\\A', + ); + $mayNotDependOnRule->injectClassNodeMap([$dependencyNode->className => $dependencyNode]); + $classNode = $this->makeNode('Domain', ['App\\Infrastructure\\A']); + + $this->assertInstanceOf(RuleViolation::class, $mayNotDependOnRule->evaluate($classNode)); + } + + public function testViolatesWhenDependencyMatchesSecondaryLayerInClassNodeMap(): void { // UserRepository is stored under primary layer 'Infrastructure' but also matches 'Repository'. // The rule forbids 'Repository'; the bug was that only the primary layer was checked. $mayNotDependOnRule = new MayNotDependOnRule(from: 'Domain', to: 'Repository'); - $mayNotDependOnRule->injectClassLayerMap([ - 'App\Infrastructure\UserRepository' => ['Infrastructure', 'Repository'], - ]); + $dependencyNode = $this->makeNode( + layer: 'Infrastructure', + className: 'App\Infrastructure\UserRepository', + layers: ['Infrastructure', 'Repository'], + ); + $mayNotDependOnRule->injectClassNodeMap([$dependencyNode->className => $dependencyNode]); $classNode = $this->makeNode('Domain', ['App\Infrastructure\UserRepository']); $violation = $mayNotDependOnRule->evaluate($classNode); @@ -190,9 +218,11 @@ public function testViolatesWhenDependencyMatchesSecondaryLayerInClassLayerMap() public function testPassesWhenDependencyOnlyMatchesPrimaryLayerNotForbidden(): void { $mayNotDependOnRule = new MayNotDependOnRule(from: 'Domain', to: 'Repository'); - $mayNotDependOnRule->injectClassLayerMap([ - 'App\Infrastructure\Cache\RedisCache' => ['Infrastructure'], - ]); + $dependencyNode = $this->makeNode( + layer: 'Infrastructure', + className: 'App\Infrastructure\Cache\RedisCache', + ); + $mayNotDependOnRule->injectClassNodeMap([$dependencyNode->className => $dependencyNode]); $classNode = $this->makeNode('Domain', ['App\Infrastructure\Cache\RedisCache']); $this->assertNotInstanceOf(RuleViolation::class, $mayNotDependOnRule->evaluate($classNode));