diff --git a/README.md b/README.md index e56aa34..2b8172d 100644 --- a/README.md +++ b/README.md @@ -27,6 +27,7 @@ | | Contravariance for parameter types and covariance for return types in inherited methods (also known as Liskov substitution principle - LSP).| | | Check LSP even for static methods. | | `requireParentConstructorCall` | Require calling parent constructor. | +| `noRedundantTraitUse` | Disallow redundant trait usage when a trait is already included via another trait. | | `disallowedBacktick` | Disallow usage of backtick operator (`` $ls = `ls -la` ``). | | `closureUsesThis` | Closure should use `$this` directly instead of using `$this` variable indirectly. | @@ -80,6 +81,7 @@ parameters: noVariableVariables: false strictArrayFilter: false illegalConstructorMethodCall: false + noRedundantTraitUse: false ``` Aside from introducing new custom rules, phpstan-strict-rules also [change the default values of some configuration parameters](./rules.neon#L1) that are present in PHPStan itself. These parameters are [documented on phpstan.org](https://phpstan.org/config-reference#stricter-analysis). diff --git a/rules.neon b/rules.neon index 7a63c4e..fb1c7be 100644 --- a/rules.neon +++ b/rules.neon @@ -32,6 +32,7 @@ parameters: noVariableVariables: %strictRules.allRules% strictArrayFilter: %strictRules.allRules% illegalConstructorMethodCall: %strictRules.allRules% + noRedundantTraitUse: %strictRules.allRules% parametersSchema: strictRules: structure([ @@ -55,6 +56,7 @@ parametersSchema: noVariableVariables: anyOf(bool(), arrayOf(bool())) strictArrayFilter: anyOf(bool(), arrayOf(bool())) illegalConstructorMethodCall: anyOf(bool(), arrayOf(bool())) + noRedundantTraitUse: anyOf(bool(), arrayOf(bool())) ]) conditionalTags: @@ -148,6 +150,8 @@ conditionalTags: phpstan.rules.rule: %strictRules.illegalConstructorMethodCall% PHPStan\Rules\Methods\IllegalConstructorStaticCallRule: phpstan.rules.rule: %strictRules.illegalConstructorMethodCall% + PHPStan\Rules\Classes\NoRedundantTraitUseRule: + phpstan.rules.rule: %strictRules.noRedundantTraitUse% services: - @@ -197,6 +201,9 @@ services: - class: PHPStan\Rules\Classes\RequireParentConstructCallRule + - + class: PHPStan\Rules\Classes\NoRedundantTraitUseRule + - class: PHPStan\Rules\DisallowedConstructs\DisallowedBacktickRule diff --git a/src/Rules/Classes/NoRedundantTraitUseRule.php b/src/Rules/Classes/NoRedundantTraitUseRule.php new file mode 100644 index 0000000..935b5d4 --- /dev/null +++ b/src/Rules/Classes/NoRedundantTraitUseRule.php @@ -0,0 +1,156 @@ + + */ +class NoRedundantTraitUseRule implements Rule +{ + + private const ERROR_MESSAGE = 'Class uses trait "%s" redundantly as it is already included via trait "%s".'; + + private ReflectionProvider $reflectionProvider; + + public function __construct(ReflectionProvider $reflectionProvider) + { + $this->reflectionProvider = $reflectionProvider; + } + + public function getNodeType(): string + { + return Class_::class; + } + + public function processNode(Node $node, Scope $scope): array + { + $errors = []; + + // Get all trait use statements from the class. + $traitUseNodes = array_filter($node->stmts, static fn ($stmt): bool => $stmt instanceof TraitUse); + + if (count($traitUseNodes) < 2) { + // Need at least 2 traits to have redundancy. + return []; + } + + // Collect all directly used trait names with their resolved names. + $directlyUsedTraits = []; + foreach ($traitUseNodes as $traitUseNode) { + foreach ($traitUseNode->traits as $trait) { + $traitName = $scope->resolveName($trait); + $directlyUsedTraits[] = $traitName; + } + } + + // Build a map of trait -> [traits it uses] with full resolution. + $traitDependencies = []; + foreach ($directlyUsedTraits as $traitName) { + try { + if ($this->reflectionProvider->hasClass($traitName)) { + $traitReflection = $this->reflectionProvider->getClass($traitName); + if ($traitReflection->isTrait()) { + $traitDependencies[$traitName] = $this->getAllTraitsUsedByTrait($traitName, []); + } + } + } catch (Throwable $e) { + // Skip traits that can't be reflected. + continue; + } + } + + // Check for redundancies. + foreach ($directlyUsedTraits as $traitA) { + foreach ($directlyUsedTraits as $traitB) { + if ($traitA === $traitB) { + continue; + } + + // Check if traitA uses traitB (directly or transitively). + if (isset($traitDependencies[$traitA]) && in_array($traitB, $traitDependencies[$traitA], true)) { + $shortNameA = basename(str_replace('\\', '/', $traitA)); + $shortNameB = basename(str_replace('\\', '/', $traitB)); + + $errors[] = RuleErrorBuilder::message(sprintf(self::ERROR_MESSAGE, $shortNameB, $shortNameA)) + ->line($node->getStartLine()) + ->identifier('traits.redundantTraitUse') + ->build(); + + // Only report each redundant trait once + break; + } + } + } + + return $errors; + } + + /** + * Get all traits used by a given trait recursively. + * + * @param string $traitName The fully qualified trait name. + * @param array $visited Array to track visited traits (for cycle detection). + * + * @return array Array of all trait names used by the given trait (directly and transitively). + */ + private function getAllTraitsUsedByTrait(string $traitName, array $visited = []): array + { + // Prevent infinite loops. + if (in_array($traitName, $visited, true)) { + return []; + } + + $visited[] = $traitName; + + try { + if (!$this->reflectionProvider->hasClass($traitName)) { + return []; + } + + $traitReflection = $this->reflectionProvider->getClass($traitName); + if (!$traitReflection->isTrait()) { + return []; + } + + $allTraits = []; + + // Get direct traits used by this trait. + foreach ($traitReflection->getTraits() as $trait) { + $usedTraitName = $trait->getName(); + $allTraits[] = $usedTraitName; + + // Recursively get traits used by the used trait. + $nestedTraits = $this->getAllTraitsUsedByTrait($usedTraitName, $visited); + $allTraits = array_merge($allTraits, $nestedTraits); + } + + return array_unique($allTraits); + } catch (Throwable $e) { + return []; + } + } + +} diff --git a/tests/Rules/Classes/NoRedundantTraitUseRuleTest.php b/tests/Rules/Classes/NoRedundantTraitUseRuleTest.php new file mode 100644 index 0000000..0b328e1 --- /dev/null +++ b/tests/Rules/Classes/NoRedundantTraitUseRuleTest.php @@ -0,0 +1,49 @@ + + */ +class NoRedundantTraitUseRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new NoRedundantTraitUseRule($this->createReflectionProvider()); + } + + public function testNoRedundantTraitUse(): void + { + $this->analyse([__DIR__ . '/data/no-redundant-trait-use.php'], [ + [ + 'Class uses trait "BarTrait" redundantly as it is already included via trait "FooTrait".', + 24, + ], + ]); + } + + public function testValidTraitUse(): void + { + $this->analyse([__DIR__ . '/data/no-redundant-trait-use-valid.php'], []); + } + + public function testSingleTraitUse(): void + { + $this->analyse([__DIR__ . '/data/no-redundant-trait-use-single-trait.php'], []); + } + + public function testTransitiveRedundantTraitUse(): void + { + $this->analyse([__DIR__ . '/data/no-redundant-trait-use-transitive.php'], [ + [ + 'Class uses trait "BaseTrait" redundantly as it is already included via trait "WrapperTrait".', + 24, + ], + ]); + } + +} diff --git a/tests/Rules/Classes/data/no-redundant-trait-use-single-trait.php b/tests/Rules/Classes/data/no-redundant-trait-use-single-trait.php new file mode 100644 index 0000000..5a74036 --- /dev/null +++ b/tests/Rules/Classes/data/no-redundant-trait-use-single-trait.php @@ -0,0 +1,18 @@ +