diff --git a/docs/available-rules.md b/docs/available-rules.md index cc7cfd4b..91dfcd9f 100644 --- a/docs/available-rules.md +++ b/docs/available-rules.md @@ -169,6 +169,7 @@ Namespace: `Boundwize\StructArmed\Rule\Rules\Usage`. |---|---|---| | `MayNotCallFunctionRule` | `new MayNotCallFunctionRule(layer: 'Domain', function: 'header')` | Classes in a layer do not call a forbidden function. | | `MayNotUseClassRule` | `new MayNotUseClassRule(layer: 'Domain', forbiddenClass: DateTime::class)` | Classes in a layer do not depend on a forbidden class. | +| `MayNotUseConstantRule` | `new MayNotUseConstantRule(layer: 'Domain', constant: 'PHP_EOL')` | Classes in a layer do not use a forbidden constant. | | `MayNotUseLanguageConstructRule` | `new MayNotUseLanguageConstructRule(layer: 'Domain', construct: 'echo')` | Classes in a layer do not use a forbidden language construct. | | `MayNotUseNamespaceRule` | `new MayNotUseNamespaceRule(layer: 'Domain', forbiddenNamespace: 'Doctrine\\ORM\\')` | Classes in a layer do not depend on a forbidden namespace. | | `MayNotUseSuperglobalsRule` | `new MayNotUseSuperglobalsRule(layer: 'Controller')` | Classes in a layer do not access superglobals directly. | @@ -176,4 +177,6 @@ Namespace: `Boundwize\StructArmed\Rule\Rules\Usage`. `MayNotUseClassRule` and `MayNotUseNamespaceRule` also accept `classNamePattern` when only matching classes should be checked. +`MayNotUseConstantRule` takes a global or namespaced constant name, such as `'PHP_EOL'` or `'Vendor\\Config\\DEBUG'`. An unqualified constant inside a namespace counts as both the constant of that namespace and the global constant of that name, as PHP only resolves it at runtime. + `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/AnalysisNodeCollector.php b/src/Analyser/AnalysisNodeCollector.php index e58af247..3bd78816 100644 --- a/src/Analyser/AnalysisNodeCollector.php +++ b/src/Analyser/AnalysisNodeCollector.php @@ -1176,8 +1176,26 @@ private function collectNodeAnalysis(Node $node): void // Entered before its name, so the FullyQualified branch above sees // the mark. if ($node instanceof ConstFetch) { - $node->name->setAttribute(self::NON_CLASS_NAME_ATTRIBUTE, true); - $this->collectKeywordConstant($node->name); + $name = $node->name; + $name->setAttribute(self::NON_CLASS_NAME_ATTRIBUTE, true); + $this->collectKeywordConstant($name); + + if ($this->activeClassLikeAnalyses !== [] && ! isset(self::KEYWORD_CONSTANTS[$name->toLowerString()])) { + // An unqualified fetch in a namespace is not a FullyQualified + // node: PHP fetches the namespaced constant when it exists and + // the global one otherwise, which is not known here, so both + // candidates are recorded. + $namespacedName = $name->getAttribute('namespacedName'); + $constant = $name->toString(); + + foreach ($this->activeClassLikeAnalyses as $activeClassLikeAnalysis) { + if ($namespacedName instanceof Name) { + $activeClassLikeAnalysis->constantFetches[$namespacedName->toString()] = true; + } + + $activeClassLikeAnalysis->constantFetches[$constant] = true; + } + } return; } @@ -1617,6 +1635,7 @@ enumBackingType: $classLike instanceof Enum_ && $classLike->scalarType instan ? $classLike->scalarType->toLowerString() : null, nonClassDependencies: $analysis['nonClassDependencies'], + constantFetches: $analysis['constantFetches'], ); } @@ -1760,6 +1779,7 @@ private function resolveClassName(ClassLike $classLike): string * @return array{ * dependencies: list, * nonClassDependencies: list, + * constantFetches: list, * functionCalls: string[], * superglobals: string[], * languageConstructs: string[], @@ -1787,6 +1807,7 @@ private function collectClassLikeAnalysis(ClassLikeAnalysis $classLikeAnalysis): strcasecmp(...) ) ), + 'constantFetches' => array_keys($classLikeAnalysis->constantFetches), 'functionCalls' => array_values(array_unique($functionCalls)), 'superglobals' => array_keys($classLikeAnalysis->superglobals), 'languageConstructs' => array_keys($classLikeAnalysis->languageConstructs), diff --git a/src/Analyser/ClassLikeAnalysis.php b/src/Analyser/ClassLikeAnalysis.php index 22f658a0..dc061afe 100644 --- a/src/Analyser/ClassLikeAnalysis.php +++ b/src/Analyser/ClassLikeAnalysis.php @@ -25,6 +25,14 @@ final class ClassLikeAnalysis */ public array $classDependencies = []; + /** + * The constants fetched by name, kept apart from the dependencies: a + * class-like or function of the same name is not a constant fetch. + * + * @var array + */ + public array $constantFetches = []; + /** @var list */ public array $functionCallNames = []; diff --git a/src/Analyser/ClassNode.php b/src/Analyser/ClassNode.php index a4f6b8d7..26e0851c 100644 --- a/src/Analyser/ClassNode.php +++ b/src/Analyser/ClassNode.php @@ -5,8 +5,10 @@ namespace Boundwize\StructArmed\Analyser; use function array_filter; +use function in_array; use function preg_match; use function strcasecmp; +use function strncasecmp; use function strrpos; use function substr; @@ -37,6 +39,7 @@ final class ClassNode * @param EnumCaseNode[] $enumCases Cases of this enum * @param string|null $enumBackingType Backing type for a backed enum, null otherwise * @param list $nonClassDependencies Dependencies only ever used as a function or constant name + * @param list $constantFetches Global and namespaced constants fetched within this class */ public function __construct( public readonly string $className, @@ -70,6 +73,7 @@ public function __construct( public readonly array $enumCases = [], public readonly ?string $enumBackingType = null, public readonly array $nonClassDependencies = [], + public readonly array $constantFetches = [], ) { $this->layers = $layers ?: array_filter([$this->layer]); } @@ -93,6 +97,36 @@ public function usesClass(string $class): bool return true; } + /** + * Whether the class fetches the constant $constant: a class-like or + * function of the same name does not count. + */ + public function usesConstant(string $constant): bool + { + $separatorPosition = strrpos($constant, '\\'); + + // a constant name is case-sensitive + if ($separatorPosition === false) { + return in_array($constant, $this->constantFetches, true); + } + + $namespaceLength = $separatorPosition + 1; + $name = substr($constant, $namespaceLength); + + // namespace names are case-insensitive; only the namespace is + // compared that way, the constant name after it stays case-sensitive + foreach ($this->constantFetches as $constantFetch) { + if ( + strncasecmp($constantFetch, $constant, $namespaceLength) === 0 + && substr($constantFetch, $namespaceLength) === $name + ) { + return true; + } + } + + return false; + } + public function isBackedEnum(): bool { return $this->isEnum && $this->enumBackingType !== null; diff --git a/src/Cache/AnalysisResultCache.php b/src/Cache/AnalysisResultCache.php index 1776884e..238960da 100644 --- a/src/Cache/AnalysisResultCache.php +++ b/src/Cache/AnalysisResultCache.php @@ -65,7 +65,7 @@ final class AnalysisResultCache * their shape or naming changes: it is recorded in the metadata marker, * so a cache written by an older format is cleared on its next use. */ - public const FORMAT_VERSION = 13; + public const FORMAT_VERSION = 14; private readonly string $cacheDirectory; @@ -970,6 +970,7 @@ private function classNodeToArray(ClassNode $classNode): array $lists = [ 'dependencies' => $classNode->dependencies, 'nonClassDependencies' => $classNode->nonClassDependencies, + 'constantFetches' => $classNode->constantFetches, 'implements' => array_values($classNode->implements), 'interfaceExtends' => array_values($classNode->interfaceExtends), 'parentClasses' => $classNode->parentClasses, @@ -1011,6 +1012,7 @@ private function classNodeFromArray(array $node, string $file): ?ClassNode $isReadonly = $node['isReadonly'] ?? null; $dependencies = $node['dependencies'] ?? []; $nonClassDependencies = $node['nonClassDependencies'] ?? []; + $constantFetches = $node['constantFetches'] ?? []; $implements = $node['implements'] ?? []; $interfaceExtends = $node['interfaceExtends'] ?? []; $parentClasses = $node['parentClasses'] ?? []; @@ -1035,6 +1037,7 @@ private function classNodeFromArray(array $node, string $file): ?ClassNode || ! is_bool($isReadonly) || ! $this->isStringArray($dependencies) || ! $this->isStringArray($nonClassDependencies) + || ! $this->isStringArray($constantFetches) || ! $this->isStringArray($implements) || ! $this->isStringArray($interfaceExtends) || ! $this->isStringArray($parentClasses) @@ -1091,6 +1094,7 @@ interfaceExtends: array_values($interfaceExtends), enumCases: $enumCases, enumBackingType: $enumBackingType, nonClassDependencies: array_values($nonClassDependencies), + constantFetches: array_values($constantFetches), ); } diff --git a/src/Preset/Presets/DddPreset.php b/src/Preset/Presets/DddPreset.php index 2ba49ded..78d8411b 100644 --- a/src/Preset/Presets/DddPreset.php +++ b/src/Preset/Presets/DddPreset.php @@ -17,6 +17,7 @@ use Boundwize\StructArmed\Rule\Rules\Method\MustHaveReturnTypeRule; use Boundwize\StructArmed\Rule\Rules\Usage\MayNotCallFunctionRule; use Boundwize\StructArmed\Rule\Rules\Usage\MayNotUseClassRule; +use Boundwize\StructArmed\Rule\Rules\Usage\MayNotUseConstantRule; use Boundwize\StructArmed\Rule\Rules\Usage\MayNotUseLanguageConstructRule; use DateTime; use Exception; @@ -270,6 +271,13 @@ private function applySafetyRules(Architecture $architecture): self new MayNotImplementInterfaceRule(layer: 'Domain', interface: JsonSerializable::class) ); + foreach (['STDIN', 'STDOUT', 'STDERR'] as $constant) { + $architecture->rule( + sprintf('ddd.safety.domain_no_%s', strtolower($constant)), + new MayNotUseConstantRule(layer: 'Domain', constant: $constant) + ); + } + foreach (['Domain', 'Application'] as $layer) { $architecture->rule( sprintf('ddd.safety.%s_max_complexity', strtolower($layer)), diff --git a/src/Rule/Rules/Usage/MayNotUseConstantRule.php b/src/Rule/Rules/Usage/MayNotUseConstantRule.php new file mode 100644 index 00000000..ea6cba13 --- /dev/null +++ b/src/Rule/Rules/Usage/MayNotUseConstantRule.php @@ -0,0 +1,45 @@ +isInLayer($this->layer); + } + + public function evaluate(ClassNode $classNode): ?RuleViolation + { + if (! $classNode->usesConstant($this->constant)) { + return null; + } + + return new RuleViolation( + message: sprintf( + '%s [%s] must not use constant [%s]', + $classNode->getType(), + $classNode->className, + $this->constant + ), + file: $classNode->file, + line: $classNode->line, + className: $classNode->className, + layer: $classNode->layer, + ); + } +} diff --git a/tests/Analyser/AnalysisNodeCollectorTest.php b/tests/Analyser/AnalysisNodeCollectorTest.php index f0e9a57e..027d6da2 100644 --- a/tests/Analyser/AnalysisNodeCollectorTest.php +++ b/tests/Analyser/AnalysisNodeCollectorTest.php @@ -2349,6 +2349,66 @@ public function isEnabled(): bool $this->assertContains('App\Infrastructure\Config\FEATURE_ENABLED', $classNode->dependencies); } + public function testCollectsConstantFetchesApartFromDependencies(): void + { + $classNode = $this->collect( + <<<'PHP_WRAP' + assertSame( + ['App\Domain\PHP_EOL', 'PHP_EOL', 'PHP_INT_MAX', 'App\Infrastructure\Config\FEATURE_ENABLED'], + $classNode->constantFetches + ); + $this->assertNotContains('PHP_EOL', $classNode->dependencies); + } + + public function testCollectsUnqualifiedConstantFetchOfSameNamespaceConstant(): void + { + $classNode = $this->collect( + <<<'PHP' + assertTrue($classNode->usesConstant('Vendor\Config\DEBUG')); + $this->assertTrue($classNode->usesConstant('DEBUG')); + } + public function testCollectsFullyQualifiedDependencies(): void { $classNode = $this->collect('assertTrue($classNode->dependsOn('Vendor\\helper')); } + public function testUsesConstantMatchesNamespaceCaseInsensitivelyAndNameCaseSensitively(): 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, + dependencies: ['STDOUT'], + constantFetches: ['STDIN', 'Vendor\\Config\\DEBUG'], + ); + + $this->assertTrue($classNode->usesConstant('STDIN')); + $this->assertFalse($classNode->usesConstant('stdin')); + $this->assertTrue($classNode->usesConstant('vendor\\config\\DEBUG')); + $this->assertFalse($classNode->usesConstant('Vendor\\Config\\debug')); + $this->assertFalse($classNode->usesConstant('Vendor\\DEBUG')); + $this->assertFalse($classNode->usesConstant('DEBUG')); + // a dependency of that name is a class-like or function, not a fetch + $this->assertFalse($classNode->usesConstant('STDOUT')); + } + public function testDependsOnDoesNotMatchNamespacePrefix(): void { $classNode = new ClassNode( diff --git a/tests/Cache/AnalysisResultCacheTest.php b/tests/Cache/AnalysisResultCacheTest.php index 2c0dbca8..8b04594f 100644 --- a/tests/Cache/AnalysisResultCacheTest.php +++ b/tests/Cache/AnalysisResultCacheTest.php @@ -2945,6 +2945,7 @@ className: Foo::class, ], functionCalls: ['sprintf'], superglobals: ['_SERVER'], + constantFetches: ['PHP_EOL'], ); } diff --git a/tests/Preset/PresetTest.php b/tests/Preset/PresetTest.php index 06e1b7a8..0040c224 100644 --- a/tests/Preset/PresetTest.php +++ b/tests/Preset/PresetTest.php @@ -323,6 +323,7 @@ public function testDddPresetRegistersAllDefaultRules(): void ); $this->assertArrayHasKey('ddd.safety.domain_no_dd', $rules); $this->assertArrayHasKey('ddd.safety.application_no_exit', $rules); + $this->assertArrayHasKey('ddd.safety.domain_no_stderr', $rules); } public function testDddPresetCanSkipOptionalFinalRules(): void diff --git a/tests/Rule/RuleViolationTest.php b/tests/Rule/RuleViolationTest.php index 279e959f..9aeb406f 100644 --- a/tests/Rule/RuleViolationTest.php +++ b/tests/Rule/RuleViolationTest.php @@ -162,6 +162,7 @@ public function testCollectionFiltersAndSerializesViolations(): void $this->assertTrue($collection->hasViolations()); $this->assertCount(2, $collection); $this->assertSame([$app], $collection->forRule('app.rule')); + $this->assertSame([$ruleViolation->toArray(), $app->toArray()], $collection->toArray()); $this->assertSame([$ruleViolation, $app], iterator_to_array($collection)); } diff --git a/tests/Rule/Usage/MayNotUseConstantRuleTest.php b/tests/Rule/Usage/MayNotUseConstantRuleTest.php new file mode 100644 index 00000000..52b56164 --- /dev/null +++ b/tests/Rule/Usage/MayNotUseConstantRuleTest.php @@ -0,0 +1,138 @@ + $constantFetches */ + private function makeNode( + array $constantFetches, + string $layer = 'Domain', + bool $isTrait = false, + bool $isEnum = false, + ): ClassNode { + return new ClassNode( + className: 'App\\Domain\\OrderService', + file: '/fake.php', + line: 1, + layer: $layer, + extends: null, + isAbstract: false, + isFinal: true, + isInterface: false, + isReadonly: false, + isTrait: $isTrait, + isEnum: $isEnum, + constantFetches: $constantFetches, + ); + } + + public function testPassesWhenForbiddenConstantNotUsed(): void + { + $mayNotUseConstantRule = new MayNotUseConstantRule(layer: 'Domain', constant: 'PHP_EOL'); + $classNode = $this->makeNode(['PHP_INT_MAX']); + + $this->assertNotInstanceOf(RuleViolation::class, $mayNotUseConstantRule->evaluate($classNode)); + } + + public function testViolatesWhenForbiddenConstantIsUsed(): void + { + $mayNotUseConstantRule = new MayNotUseConstantRule(layer: 'Domain', constant: 'PHP_EOL'); + $classNode = $this->makeNode(['PHP_EOL']); + + $violation = $mayNotUseConstantRule->evaluate($classNode); + + $this->assertInstanceOf(RuleViolation::class, $violation); + $this->assertSame('Class [App\\Domain\\OrderService] must not use constant [PHP_EOL]', $violation->message); + } + + #[DataProvider('traitAndEnumKindProvider')] + public function testViolationMessageNamesTheClassLikeKind(string $expectedKind, bool $isTrait, bool $isEnum): void + { + $mayNotUseConstantRule = new MayNotUseConstantRule(layer: 'Domain', constant: 'PHP_EOL'); + $classNode = $this->makeNode(['PHP_EOL'], isTrait: $isTrait, isEnum: $isEnum); + + $violation = $mayNotUseConstantRule->evaluate($classNode); + + $this->assertInstanceOf(RuleViolation::class, $violation); + $this->assertSame( + $expectedKind . ' [App\\Domain\\OrderService] must not use constant [PHP_EOL]', + $violation->message + ); + } + + /** @return iterable */ + public static function traitAndEnumKindProvider(): iterable + { + yield 'trait' => ['Trait', true, false]; + yield 'enum' => ['Enum', false, true]; + } + + public function testConstantComparisonIsCaseSensitive(): void + { + $mayNotUseConstantRule = new MayNotUseConstantRule(layer: 'Domain', constant: 'PHP_EOL'); + $classNode = $this->makeNode(['php_eol']); + + $this->assertNotInstanceOf(RuleViolation::class, $mayNotUseConstantRule->evaluate($classNode)); + } + + public function testNamespaceComparisonIsCaseInsensitive(): void + { + $mayNotUseConstantRule = new MayNotUseConstantRule(layer: 'Domain', constant: 'Vendor\\Config\\DEBUG'); + + $this->assertInstanceOf( + RuleViolation::class, + $mayNotUseConstantRule->evaluate($this->makeNode(['vendor\\config\\DEBUG'])) + ); + $this->assertNotInstanceOf( + RuleViolation::class, + $mayNotUseConstantRule->evaluate($this->makeNode(['Vendor\\Config\\debug'])) + ); + } + + public function testPassesWhenOnlyAClassOrFunctionOfThatNameIsUsed(): void + { + $mayNotUseConstantRule = new MayNotUseConstantRule(layer: 'Domain', constant: 'STDIN'); + $classNode = new ClassNode( + className: 'App\\Domain\\OrderService', + file: '/fake.php', + line: 1, + layer: 'Domain', + extends: null, + isAbstract: false, + isFinal: true, + isInterface: false, + isReadonly: false, + dependencies: ['STDIN'], + functionCalls: ['STDIN'], + ); + + $this->assertNotInstanceOf(RuleViolation::class, $mayNotUseConstantRule->evaluate($classNode)); + } + + public function testDoesNotApplyToWrongLayer(): void + { + $mayNotUseConstantRule = new MayNotUseConstantRule(layer: 'Domain', constant: 'PHP_EOL'); + $classNode = $this->makeNode(['PHP_EOL'], layer: 'Infrastructure'); + + $this->assertFalse($mayNotUseConstantRule->appliesTo($classNode)); + } + + public function testAppliesToCorrectLayer(): void + { + $mayNotUseConstantRule = new MayNotUseConstantRule(layer: 'Domain', constant: 'PHP_EOL'); + $classNode = $this->makeNode([]); + + $this->assertTrue($mayNotUseConstantRule->appliesTo($classNode)); + } +}