diff --git a/rules.neon b/rules.neon index 34f4b3b6..85fa027f 100644 --- a/rules.neon +++ b/rules.neon @@ -12,6 +12,7 @@ rules: - mglaman\PHPStanDrupal\Rules\Drupal\LoadIncludes - mglaman\PHPStanDrupal\Rules\Drupal\EntityQuery\EntityQueryHasAccessCheckRule - mglaman\PHPStanDrupal\Rules\Drupal\TestClassesProtectedPropertyModulesRule + - mglaman\PHPStanDrupal\Rules\Drupal\NoRedundantTraitUseRule conditionalTags: mglaman\PHPStanDrupal\Rules\Drupal\Tests\TestClassSuffixNameRule: diff --git a/src/Rules/Drupal/NoRedundantTraitUseRule.php b/src/Rules/Drupal/NoRedundantTraitUseRule.php new file mode 100644 index 00000000..13bba410 --- /dev/null +++ b/src/Rules/Drupal/NoRedundantTraitUseRule.php @@ -0,0 +1,143 @@ + + */ +class NoRedundantTraitUseRule implements Rule +{ + private const ERROR_MESSAGE = 'Class uses trait "%s" redundantly as it is already included via trait "%s".'; + + public function __construct( + private 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, fn($stmt) => $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 (Exception $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 (Exception $e) { + return []; + } + } +} diff --git a/tests/src/Rules/NoRedundantTraitUseRuleTest.php b/tests/src/Rules/NoRedundantTraitUseRuleTest.php new file mode 100644 index 00000000..493b529a --- /dev/null +++ b/tests/src/Rules/NoRedundantTraitUseRuleTest.php @@ -0,0 +1,58 @@ +createReflectionProvider()); + } + + /** + * @dataProvider resultData + * + * @param list $errorMessages + */ + public function testRule(string $path, array $errorMessages): void + { + $this->analyse([$path], $errorMessages); + } + + public static function resultData(): \Generator + { + yield [ + __DIR__ . '/data/no-redundant-trait-use.php', + [ + [ + 'Class uses trait "BarTrait" redundantly as it is already included via trait "FooTrait".', + 20 + ], + ] + ]; + + yield [ + __DIR__ . '/data/no-redundant-trait-use-valid.php', + [] + ]; + + yield [ + __DIR__ . '/data/no-redundant-trait-use-single-trait.php', + [] + ]; + + yield [ + __DIR__ . '/data/no-redundant-trait-use-transitive.php', + [ + [ + 'Class uses trait "BaseTrait" redundantly as it is already included via trait "WrapperTrait".', + 20 + ], + ] + ]; + } +} \ No newline at end of file diff --git a/tests/src/Rules/data/no-redundant-trait-use-single-trait.php b/tests/src/Rules/data/no-redundant-trait-use-single-trait.php new file mode 100644 index 00000000..63a125d3 --- /dev/null +++ b/tests/src/Rules/data/no-redundant-trait-use-single-trait.php @@ -0,0 +1,14 @@ +