diff --git a/docs/assets/no-violation.svg b/docs/assets/no-violation.svg index 9a4d42ab..a29441d9 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.19 — Architecture Enforcement + StructArmed 0.14.20 — Architecture Enforcement =============================================== diff --git a/docs/assets/structarmed-showoff.svg b/docs/assets/structarmed-showoff.svg index 32a86beb..57154c07 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.19 — Architecture Enforcement + StructArmed 0.14.20 — Architecture Enforcement =============================================== diff --git a/docs/available-rules.md b/docs/available-rules.md index d5f57472..49611b68 100644 --- a/docs/available-rules.md +++ b/docs/available-rules.md @@ -130,3 +130,5 @@ Namespace: `Boundwize\StructArmed\Rule\Rules\Usage`. {: .rule-table } `MayNotUseClassRule` and `MayNotUseNamespaceRule` also accept `classNamePattern` when only matching classes should be checked. + +`MayNotUseLanguageConstructRule` accepts one of the following `construct` names: `echo`, `print`, `eval`, `isset`, `empty`, `unset`, `list`, `exit`, `die`, `include`, `include_once`, `require`, `require_once`. `die` is a pure alias of `exit`, so banning either spelling catches both. The `include` / `include_once` / `require` / `require_once` constructs are distinct and are matched exactly. diff --git a/src/Analyser/ClassNode.php b/src/Analyser/ClassNode.php index 85f6d5fd..5479b338 100644 --- a/src/Analyser/ClassNode.php +++ b/src/Analyser/ClassNode.php @@ -147,7 +147,16 @@ public function callsFunction(string $function): bool public function usesLanguageConstruct(string $construct): bool { - return in_array($construct, $this->languageConstructs, true); + if (in_array($construct, $this->languageConstructs, true)) { + return true; + } + + // `die` is a pure alias of `exit`, so banning either spelling catches both. + return match ($construct) { + 'exit' => in_array('die', $this->languageConstructs, true), + 'die' => in_array('exit', $this->languageConstructs, true), + default => false, + }; } public function accessesSuperglobals(): bool diff --git a/tests/Analyser/ClassCollectorTest.php b/tests/Analyser/ClassCollectorTest.php index ae346d95..5d9039ed 100644 --- a/tests/Analyser/ClassCollectorTest.php +++ b/tests/Analyser/ClassCollectorTest.php @@ -509,6 +509,20 @@ public function testDeduplicatesLanguageConstructs(): void $this->assertSame(['exit'], $classNode->languageConstructs); } + public function testKeepsIncludeFamilyConstructsDistinct(): void + { + $classNode = $this->collect( + 'assertSame( + ['include', 'include_once', 'require', 'require_once'], + $classNode->languageConstructs + ); + } + public function testCollectsEchoAsLanguageConstruct(): void { $classNode = $this->collect('assertFalse($classNode->usesLanguageConstruct('eval')); } + public function testUsesLanguageConstructResolvesExitDieAliases(): void + { + $usesExit = new ClassNode( + className: 'App\\Domain\\ExitService', + file: '/src/ExitService.php', + line: 1, + layer: 'Domain', + extends: null, + isAbstract: false, + isFinal: true, + isInterface: false, + isReadonly: false, + languageConstructs: ['exit'], + ); + + $usesDie = new ClassNode( + className: 'App\\Domain\\DieService', + file: '/src/DieService.php', + line: 1, + layer: 'Domain', + extends: null, + isAbstract: false, + isFinal: true, + isInterface: false, + isReadonly: false, + languageConstructs: ['die'], + ); + + // `die` is a pure alias of `exit`, so either query matches either spelling. + $this->assertTrue($usesExit->usesLanguageConstruct('exit')); + $this->assertTrue($usesExit->usesLanguageConstruct('die')); + $this->assertTrue($usesDie->usesLanguageConstruct('die')); + $this->assertTrue($usesDie->usesLanguageConstruct('exit')); + + // Distinct constructs are not over-aliased. + $this->assertFalse($usesExit->usesLanguageConstruct('echo')); + $this->assertFalse($usesExit->usesLanguageConstruct('include')); + } + public function testSetRecursiveParents(): void { $classNode = new ClassNode( diff --git a/tests/Rule/Usage/MayNotUseLanguageConstructRuleTest.php b/tests/Rule/Usage/MayNotUseLanguageConstructRuleTest.php index 707d4dbf..b2d54d09 100644 --- a/tests/Rule/Usage/MayNotUseLanguageConstructRuleTest.php +++ b/tests/Rule/Usage/MayNotUseLanguageConstructRuleTest.php @@ -8,6 +8,7 @@ use Boundwize\StructArmed\Rule\Rules\Usage\MayNotUseLanguageConstructRule; use Boundwize\StructArmed\Rule\RuleViolation; use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; #[CoversClass(MayNotUseLanguageConstructRule::class)] @@ -61,6 +62,57 @@ public function testViolatesWhenDieIsUsed(): void $this->assertStringContainsString('die', $violation->message); } + /** + * `die` is a pure alias of `exit`, so a rule banning either spelling must + * fire whichever spelling the code actually used. `exit;`/`exit(1);` are + * recorded as `exit`; `die;`/`die(1);` as `die` — the two recorded spellings + * cover the four code forms. + * + * @param array $recordedConstructs + */ + #[DataProvider('exitDieAliasProvider')] + public function testExitAndDieAreSymmetricAliases(string $configured, array $recordedConstructs): void + { + $mayNotUseLanguageConstructRule = new MayNotUseLanguageConstructRule(layer: 'Domain', construct: $configured); + $classNode = $this->makeNode($recordedConstructs); + + $violation = $mayNotUseLanguageConstructRule->evaluate($classNode); + + $this->assertInstanceOf(RuleViolation::class, $violation); + $this->assertStringContainsString($configured, $violation->message); + } + + /** @return iterable}> */ + public static function exitDieAliasProvider(): iterable + { + yield 'ban exit, code used exit' => ['exit', ['exit']]; + yield 'ban exit, code used die' => ['exit', ['die']]; + yield 'ban die, code used die' => ['die', ['die']]; + yield 'ban die, code used exit' => ['die', ['exit']]; + } + + /** + * The include family are genuinely distinct constructs and must not be + * aliased to one another. + */ + #[DataProvider('includeFamilyProvider')] + public function testIncludeFamilyRemainsDistinct(string $configured, string $recorded): void + { + $mayNotUseLanguageConstructRule = new MayNotUseLanguageConstructRule(layer: 'Domain', construct: $configured); + $classNode = $this->makeNode([$recorded]); + + $this->assertNotInstanceOf(RuleViolation::class, $mayNotUseLanguageConstructRule->evaluate($classNode)); + } + + /** @return iterable */ + public static function includeFamilyProvider(): iterable + { + yield 'include vs include_once' => ['include', 'include_once']; + yield 'include vs require' => ['include', 'require']; + yield 'require vs require_once' => ['require', 'require_once']; + yield 'require_once vs include' => ['require_once', 'include']; + } + public function testDoesNotApplyToWrongLayer(): void { $mayNotUseLanguageConstructRule = new MayNotUseLanguageConstructRule(layer: 'Domain', construct: 'exit');