Add rule to detect redundant trait usage

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).

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
 

src/Rules/Classes/NoRedundantTraitUseRule.php

@@ -0,0 +1,156 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Classes;
+
+use PhpParser\Node;
+use PhpParser\Node\Stmt\Class_;
+use PhpParser\Node\Stmt\TraitUse;
+use PHPStan\Analyser\Scope;
+use PHPStan\Reflection\ReflectionProvider;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use Throwable;
+use function array_filter;
+use function array_merge;
+use function array_unique;
+use function basename;
+use function count;
+use function in_array;
+use function sprintf;
+use function str_replace;
+
+/**
+ * Disallows redundant trait usage when a trait is already included via another trait.
+ *
+ * This rule checks classes that use multiple traits and ensures that
+ * they don't use a trait that is already being used by another trait.
+ * For example, if trait A uses trait B, then a class shouldn't use both A and B.
+ *
+ * @implements Rule<Class_>
+ */
+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<string> $visited Array to track visited traits (for cycle detection).
+	 *
+	 * @return array<string> 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 [];
+		}
+	}
+
+}

tests/Rules/Classes/NoRedundantTraitUseRuleTest.php

@@ -0,0 +1,49 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Classes;
+
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+
+/**
+ * @extends RuleTestCase<NoRedundantTraitUseRule>
+ */
+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,
+			],
+		]);
+	}
+
+}

tests/Rules/Classes/data/no-redundant-trait-use-single-trait.php

@@ -0,0 +1,18 @@
+<?php declare(strict_types = 1);
+
+trait SingleTrait
+{
+
+	public function singleMethod(): void
+	{
+	}
+
+}
+
+// This class uses only one trait - this is valid (no redundancy possible)
+class SingleTraitUseClass
+{
+
+	use SingleTrait;
+
+}

tests/Rules/Classes/data/no-redundant-trait-use-transitive.php

@@ -0,0 +1,30 @@
+<?php declare(strict_types = 1);
+
+trait BaseTrait
+{
+
+	public function baseMethod(): void
+	{
+	}
+
+}
+
+trait WrapperTrait
+{
+
+	use BaseTrait;
+
+	public function wrapperMethod(): void
+	{
+	}
+
+}
+
+// This class uses WrapperTrait and BaseTrait, but BaseTrait is already included via WrapperTrait
+class TransitiveRedundantTraitUseClass
+{
+
+	use WrapperTrait;
+	use BaseTrait; // This should trigger an error (transitive redundancy)
+
+}

tests/Rules/Classes/data/no-redundant-trait-use-valid.php

@@ -0,0 +1,28 @@
+<?php declare(strict_types = 1);
+
+trait IndependentTrait1
+{
+
+	public function method1(): void
+	{
+	}
+
+}
+
+trait IndependentTrait2
+{
+
+	public function method2(): void
+	{
+	}
+
+}
+
+// This class uses two independent traits - this is valid
+class ValidTraitUseClass
+{
+
+	use IndependentTrait1;
+	use IndependentTrait2;
+
+}

tests/Rules/Classes/data/no-redundant-trait-use.php

@@ -0,0 +1,30 @@
+<?php declare(strict_types = 1);
+
+trait BarTrait
+{
+
+	public function barMethod(): void
+	{
+	}
+
+}
+
+trait FooTrait
+{
+
+	use BarTrait;
+
+	public function fooMethod(): void
+	{
+	}
+
+}
+
+// This class uses both FooTrait and BarTrait, but BarTrait is already included via FooTrait
+class RedundantTraitUseClass
+{
+
+	use FooTrait;
+	use BarTrait; // This should trigger an error
+
+}