diff --git a/docs/assets/no-violation.svg b/docs/assets/no-violation.svg index a29441d..5f8c367 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.14.20 — Architecture Enforcement + StructArmed 0.15.0 — Architecture Enforcement =============================================== diff --git a/docs/assets/structarmed-showoff.svg b/docs/assets/structarmed-showoff.svg index 57154c0..fefd68d 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.14.20 — Architecture Enforcement + StructArmed 0.15.0 — Architecture Enforcement =============================================== diff --git a/docs/available-rules.md b/docs/available-rules.md index 49611b6..88bddfc 100644 --- a/docs/available-rules.md +++ b/docs/available-rules.md @@ -80,7 +80,7 @@ Namespace: `Boundwize\StructArmed\Rule\Rules\Class_`. | `ClassNameMustNotHavePrefixRule` | `new ClassNameMustNotHavePrefixRule(layer: 'Model', prefix: 'Model')` | Classes in a layer do not use a forbidden prefix. | | `MaxDependencyCountRule` | `new MaxDependencyCountRule(layer: 'Controller', maxCount: 5)` | Constructor dependency count stays below the configured limit. | | `MayNotImplementInterfaceRule` | `new MayNotImplementInterfaceRule(layer: 'Domain', interface: JsonSerializable::class)` | Classes in a layer do not implement a forbidden interface. | -| `MustBeFinalRule` | `new MustBeFinalRule(layer: 'Domain', classNamePattern: '/Entity$/')` | Matching classes in a layer are declared `final`. Supports `--fix`. | +| `MustBeFinalRule` | `new MustBeFinalRule(layer: 'Domain', classNamePattern: '/Entity$/')` | Matching classes in a layer are declared `final`. Classes extended by another scanned class are skipped (making them `final` would break the child). Supports `--fix`. | | `MustBeInterfaceRule` | `new MustBeInterfaceRule(layer: 'Contract', classNamePattern: '/Interface$/')` | Matching declarations in a layer are interfaces. | | `MustDeclareConstantVisibilityRule` | `new MustDeclareConstantVisibilityRule(layer: 'Source')` | Class constants declare `public`, `protected`, or `private`. Supports `--fix`. | | `MustDeclareMethodVisibilityRule` | `new MustDeclareMethodVisibilityRule(layer: 'Source')` | Methods declare `public`, `protected`, or `private`. Supports `--fix`. | diff --git a/src/Analyser/Analyser.php b/src/Analyser/Analyser.php index db61b1a..5175c27 100644 --- a/src/Analyser/Analyser.php +++ b/src/Analyser/Analyser.php @@ -12,6 +12,7 @@ use Boundwize\StructArmed\LayerResolver\ChainLayerResolver; use Boundwize\StructArmed\Progress\ProgressHandlerInterface; use Boundwize\StructArmed\Rule\ComposerJsonRuleInterface; +use Boundwize\StructArmed\Rule\ExtendedClassAwareRuleInterface; use Boundwize\StructArmed\Rule\FileAnalysisRuleInterface; use Boundwize\StructArmed\Rule\FixableInterface; use Boundwize\StructArmed\Rule\LayerAwareRuleInterface; @@ -85,9 +86,10 @@ public function analyse( $classRules = $this->classRules($rules, $skippedRuleKeys); $ruleSkipMatchers = $this->ruleSkipMatchers($classRules, $globalSkipPaths, $ruleSkipPaths); - $projectRuleViolations = []; - $fileAnalysisRules = []; - $layerAwareRules = []; + $projectRuleViolations = []; + $fileAnalysisRules = []; + $layerAwareRules = []; + $hasExtendedClassAwareRule = false; foreach ($rules as $key => $rule) { if (array_key_exists($key, $skippedRuleKeys)) { @@ -98,6 +100,10 @@ public function analyse( $layerAwareRules[] = $rule; } + if ($rule instanceof ExtendedClassAwareRuleInterface) { + $hasExtendedClassAwareRule = true; + } + if (! $rule instanceof ProjectRuleInterface) { continue; } @@ -141,6 +147,10 @@ public function analyse( $classNodes = $extractionResult->classNodes; $classNodes = $this->withRecursiveParents($classNodes); + if ($hasExtendedClassAwareRule) { + $this->markExtendedClasses($classNodes); + } + $fileAnalysisProvider = new FileAnalysisProvider($extractionResult->fileAnalyses); foreach ($fileAnalysisRules as $key => $rule) { @@ -646,6 +656,32 @@ private function dependenciesForInheritanceDependency( return $resolvedDependencies; } + /** + * Flag every class that another scanned class extends, using the recursive + * parent chain resolved by {@see withRecursiveParents()}. A class name found + * among any node's parent classes is extended within the scanned paths. + * + * @param list $classNodes + */ + private function markExtendedClasses(array $classNodes): void + { + $extended = []; + + foreach ($classNodes as $classNode) { + foreach ($classNode->parentClasses as $parentClass) { + $extended[$parentClass] = true; + } + } + + foreach ($classNodes as $classNode) { + // Only classes appear in parentClasses; traits, interfaces, and enums + // never do, so they are left with the default (not extended). + if (isset($extended[$classNode->className])) { + $classNode->setExtended(true); + } + } + } + /** * @param list $classNodes * @return list diff --git a/src/Analyser/ClassNode.php b/src/Analyser/ClassNode.php index 5479b33..3d85f0f 100644 --- a/src/Analyser/ClassNode.php +++ b/src/Analyser/ClassNode.php @@ -58,6 +58,7 @@ public function __construct( public readonly array $interfaceExtends = [], public array $parentClasses = [], public array $parentInterfaces = [], + public bool $isExtended = false, ) { $this->layers = $layers ?: array_filter([$this->layer]); } @@ -72,6 +73,15 @@ public function setRecursiveParents(array $parentClasses, array $parentInterface $this->parentInterfaces = $parentInterfaces; } + /** + * Whether another scanned class extends this class. Computed by the analyser + * for rules implementing ExtendedClassAwareRuleInterface; false otherwise. + */ + public function setExtended(bool $isExtended): void + { + $this->isExtended = $isExtended; + } + public function shortName(): string { $parts = explode('\\', $this->className); diff --git a/src/Rule/ExtendedClassAwareRuleInterface.php b/src/Rule/ExtendedClassAwareRuleInterface.php new file mode 100644 index 0000000..055e188 --- /dev/null +++ b/src/Rule/ExtendedClassAwareRuleInterface.php @@ -0,0 +1,18 @@ +isExtended. + * + * Trade-off: only classes extended within the scanned paths are known. A class + * extended solely by a consumer outside the scan is reported as if not extended. + */ +interface ExtendedClassAwareRuleInterface extends RuleInterface +{ +} diff --git a/src/Rule/Rules/Class_/MustBeFinalRule.php b/src/Rule/Rules/Class_/MustBeFinalRule.php index 66b8237..06329e1 100644 --- a/src/Rule/Rules/Class_/MustBeFinalRule.php +++ b/src/Rule/Rules/Class_/MustBeFinalRule.php @@ -5,14 +5,14 @@ namespace Boundwize\StructArmed\Rule\Rules\Class_; use Boundwize\StructArmed\Analyser\ClassNode; +use Boundwize\StructArmed\Rule\ExtendedClassAwareRuleInterface; use Boundwize\StructArmed\Rule\Fixer\PhpParser\AbstractPhpParserFixableRule; use Boundwize\StructArmed\Rule\Fixer\PhpParser\Class_\AddFinalClassVisitor; -use Boundwize\StructArmed\Rule\RuleInterface; use Boundwize\StructArmed\Rule\RuleViolation; use function sprintf; -final readonly class MustBeFinalRule extends AbstractPhpParserFixableRule implements RuleInterface +final readonly class MustBeFinalRule extends AbstractPhpParserFixableRule implements ExtendedClassAwareRuleInterface { public function __construct( private string $layer, @@ -43,6 +43,12 @@ public function evaluate(ClassNode $classNode): ?RuleViolation return null; } + // A class another scanned class extends cannot be made final; forcing it + // would break the child, so treat it as legitimately non-final. + if ($classNode->isExtended) { + return null; + } + return new RuleViolation( message: sprintf( 'Class [%s] must be declared final', diff --git a/tests/Analyser/AnalyserTest.php b/tests/Analyser/AnalyserTest.php index 73398da..81a2d33 100644 --- a/tests/Analyser/AnalyserTest.php +++ b/tests/Analyser/AnalyserTest.php @@ -95,6 +95,52 @@ public function testAnalyserCollectsClassNodesWithSequentialRunner(): void $this->assertCount(2, $ruleViolationCollection->forRule('source.must_be_final')); } + public function testMustBeFinalRuleDoesNotFlagClassExtendedByAnotherScannedClass(): void + { + $basePath = $this->makeTempProject([ + 'src/BaseHandler.php' => ' ' 'layer('Source', 'src/') + ->rule('source.must_be_final', new MustBeFinalRule('Source')); + + $violations = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()) + ->forRule('source.must_be_final'); + + // BaseHandler is extended (must stay non-final); PaymentHandler is the + // only genuinely non-final leaf class. + $this->assertCount(1, $violations); + $this->assertSame('App\PaymentHandler', $violations[0]->className); + } + + public function testMustBeFinalRuleFlagsExtendedClassWhenChildIsOutsideScannedPaths(): void + { + $order = 'makeTempProject([ + 'src/BaseHandler.php' => ' $order, + ]); + + $architecture = Architecture::define() + ->layer('Source', 'src/') + ->rule('source.must_be_final', new MustBeFinalRule('Source')); + + $violations = (new Analyser($basePath)) + ->analyse($architecture, ['src/'], null, AnalyserOptions::sequential()) + ->forRule('source.must_be_final'); + + // The extending class lives outside the scanned paths, so BaseHandler is + // reported as if not extended — a false positive on purpose. + $this->assertCount(1, $violations); + $this->assertSame('App\BaseHandler', $violations[0]->className); + } + public function testAnalyserMarksFixableRuleViolations(): void { $basePath = $this->makeTempProject([ diff --git a/tests/Analyser/ClassNodeTest.php b/tests/Analyser/ClassNodeTest.php index ca0f742..07bd620 100644 --- a/tests/Analyser/ClassNodeTest.php +++ b/tests/Analyser/ClassNodeTest.php @@ -182,6 +182,29 @@ className: 'App\\Domain\\OrderService', $this->assertSame(['App\\Contracts\\OrderService'], $classNode->parentInterfaces); } + public function testSetExtendedTogglesIsExtendedFlag(): void + { + $classNode = new ClassNode( + className: 'App\\Domain\\OrderService', + file: '/src/OrderService.php', + line: 5, + layer: 'Domain', + extends: null, + isAbstract: false, + isFinal: false, + isInterface: false, + isReadonly: false, + ); + + $this->assertFalse($classNode->isExtended); + + $classNode->setExtended(true); + $this->assertTrue($classNode->isExtended); + + $classNode->setExtended(false); + $this->assertFalse($classNode->isExtended); + } + public function testDependsOnMatchesExistingClassesExactly(): void { $classNode = new ClassNode( diff --git a/tests/Rule/Class_/MustBeFinalRuleTest.php b/tests/Rule/Class_/MustBeFinalRuleTest.php index 2acc1ed..97419c1 100644 --- a/tests/Rule/Class_/MustBeFinalRuleTest.php +++ b/tests/Rule/Class_/MustBeFinalRuleTest.php @@ -6,6 +6,7 @@ use Boundwize\StructArmed\Analyser\ClassNode; use Boundwize\StructArmed\Preset\Preset; +use Boundwize\StructArmed\Rule\ExtendedClassAwareRuleInterface; use Boundwize\StructArmed\Rule\FixableInterface; use Boundwize\StructArmed\Rule\Fixer\PhpParser\Class_\AddFinalClassVisitor; use Boundwize\StructArmed\Rule\Rules\Class_\MustBeFinalRule; @@ -25,6 +26,7 @@ private function makeNode( bool $isInterface = false, bool $isTrait = false, bool $isEnum = false, + bool $isExtended = false, ): ClassNode { return new ClassNode( className: $className, @@ -38,6 +40,7 @@ className: $className, isReadonly: false, isTrait: $isTrait, isEnum: $isEnum, + isExtended: $isExtended, ); } @@ -60,6 +63,22 @@ public function testViolatesWhenClassIsNotFinal(): void $this->assertStringContainsString('final', $violation->message); } + public function testPassesWhenClassIsExtendedByAnotherClass(): void + { + $mustBeFinalRule = new MustBeFinalRule(layer: 'Domain'); + $classNode = $this->makeNode(isFinal: false, isExtended: true); + + $this->assertNotInstanceOf(RuleViolation::class, $mustBeFinalRule->evaluate($classNode)); + } + + public function testIsExtendedClassAware(): void + { + $this->assertInstanceOf( + ExtendedClassAwareRuleInterface::class, + new MustBeFinalRule(layer: 'Domain') + ); + } + public function testIsFixable(): void { $this->assertInstanceOf(FixableInterface::class, new MustBeFinalRule(layer: 'Domain'));