Skip to content

Commit 1c557a8

Browse files
AronNovakAronNovak
committed
Add NoRedundantTraitUseRule to detect redundant trait usage
1 parent 9f76a17 commit 1c557a8

8 files changed

+320
-0
lines changed

‎README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
| | Contravariance for parameter types and covariance for return types in inherited methods (also known as Liskov substitution principle - LSP).|
2828
| | Check LSP even for static methods. |
2929
| `requireParentConstructorCall` | Require calling parent constructor. |
30+
| `noRedundantTraitUse` | Disallow redundant trait usage when a trait is already included via another trait. |
3031
| `disallowedBacktick` | Disallow usage of backtick operator (`` $ls = `ls -la` ``). |
3132
| `closureUsesThis` | Closure should use `$this` directly instead of using `$this` variable indirectly. |
3233

@@ -80,6 +81,7 @@ parameters:
8081
noVariableVariables: false
8182
strictArrayFilter: false
8283
illegalConstructorMethodCall: false
84+
noRedundantTraitUse: false
8385
```
8486

8587
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‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ parameters:
3232
noVariableVariables: %strictRules.allRules%
3333
strictArrayFilter: %strictRules.allRules%
3434
illegalConstructorMethodCall: %strictRules.allRules%
35+
noRedundantTraitUse: %strictRules.allRules%
3536

3637
parametersSchema:
3738
strictRules: structure([
@@ -55,6 +56,7 @@ parametersSchema:
5556
noVariableVariables: anyOf(bool(), arrayOf(bool()))
5657
strictArrayFilter: anyOf(bool(), arrayOf(bool()))
5758
illegalConstructorMethodCall: anyOf(bool(), arrayOf(bool()))
59+
noRedundantTraitUse: anyOf(bool(), arrayOf(bool()))
5860
])
5961

6062
conditionalTags:
@@ -148,6 +150,8 @@ conditionalTags:
148150
phpstan.rules.rule: %strictRules.illegalConstructorMethodCall%
149151
PHPStan\Rules\Methods\IllegalConstructorStaticCallRule:
150152
phpstan.rules.rule: %strictRules.illegalConstructorMethodCall%
153+
PHPStan\Rules\Classes\NoRedundantTraitUseRule:
154+
phpstan.rules.rule: %strictRules.noRedundantTraitUse%
151155

152156
services:
153157
-
@@ -197,6 +201,9 @@ services:
197201
-
198202
class: PHPStan\Rules\Classes\RequireParentConstructCallRule
199203

204+
-
205+
class: PHPStan\Rules\Classes\NoRedundantTraitUseRule
206+
200207
-
201208
class: PHPStan\Rules\DisallowedConstructs\DisallowedBacktickRule
202209

‎src/Rules/Classes/NoRedundantTraitUseRule.php‎

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Classes;
4+
5+
use PhpParser\Node;
6+
use PhpParser\Node\Stmt\Class_;
7+
use PhpParser\Node\Stmt\TraitUse;
8+
use PHPStan\Analyser\Scope;
9+
use PHPStan\Reflection\ReflectionProvider;
10+
use PHPStan\Rules\Rule;
11+
use PHPStan\Rules\RuleErrorBuilder;
12+
use Throwable;
13+
use function array_filter;
14+
use function array_merge;
15+
use function array_unique;
16+
use function basename;
17+
use function count;
18+
use function in_array;
19+
use function sprintf;
20+
use function str_replace;
21+
22+
/**
23+
* Disallows redundant trait usage when a trait is already included via another trait.
24+
*
25+
* This rule checks classes that use multiple traits and ensures that
26+
* they don't use a trait that is already being used by another trait.
27+
* For example, if trait A uses trait B, then a class shouldn't use both A and B.
28+
*
29+
* @implements Rule<Class_>
30+
*/
31+
class NoRedundantTraitUseRule implements Rule
32+
{
33+
34+
private const ERROR_MESSAGE = 'Class uses trait "%s" redundantly as it is already included via trait "%s".';
35+
36+
private ReflectionProvider $reflectionProvider;
37+
38+
public function __construct(ReflectionProvider $reflectionProvider)
39+
{
40+
$this->reflectionProvider = $reflectionProvider;
41+
}
42+
43+
public function getNodeType(): string
44+
{
45+
return Class_::class;
46+
}
47+
48+
public function processNode(Node $node, Scope $scope): array
49+
{
50+
$errors = [];
51+
52+
// Get all trait use statements from the class.
53+
$traitUseNodes = array_filter($node->stmts, static fn ($stmt): bool => $stmt instanceof TraitUse);
54+
55+
if (count($traitUseNodes) < 2) {
56+
// Need at least 2 traits to have redundancy.
57+
return [];
58+
}
59+
60+
// Collect all directly used trait names with their resolved names.
61+
$directlyUsedTraits = [];
62+
foreach ($traitUseNodes as $traitUseNode) {
63+
foreach ($traitUseNode->traits as $trait) {
64+
$traitName = $scope->resolveName($trait);
65+
$directlyUsedTraits[] = $traitName;
66+
}
67+
}
68+
69+
// Build a map of trait -> [traits it uses] with full resolution.
70+
$traitDependencies = [];
71+
foreach ($directlyUsedTraits as $traitName) {
72+
try {
73+
if ($this->reflectionProvider->hasClass($traitName)) {
74+
$traitReflection = $this->reflectionProvider->getClass($traitName);
75+
if ($traitReflection->isTrait()) {
76+
$traitDependencies[$traitName] = $this->getAllTraitsUsedByTrait($traitName, []);
77+
}
78+
}
79+
} catch (Throwable $e) {
80+
// Skip traits that can't be reflected.
81+
continue;
82+
}
83+
}
84+
85+
// Check for redundancies.
86+
foreach ($directlyUsedTraits as $traitA) {
87+
foreach ($directlyUsedTraits as $traitB) {
88+
if ($traitA === $traitB) {
89+
continue;
90+
}
91+
92+
// Check if traitA uses traitB (directly or transitively).
93+
if (isset($traitDependencies[$traitA]) && in_array($traitB, $traitDependencies[$traitA], true)) {
94+
$shortNameA = basename(str_replace('\\', '/', $traitA));
95+
$shortNameB = basename(str_replace('\\', '/', $traitB));
96+
97+
$errors[] = RuleErrorBuilder::message(sprintf(self::ERROR_MESSAGE, $shortNameB, $shortNameA))
98+
->line($node->getStartLine())
99+
->identifier('traits.redundantTraitUse')
100+
->build();
101+
102+
// Only report each redundant trait once
103+
break;
104+
}
105+
}
106+
}
107+
108+
return $errors;
109+
}
110+
111+
/**
112+
* Get all traits used by a given trait recursively.
113+
*
114+
* @param string $traitName The fully qualified trait name.
115+
* @param array<string> $visited Array to track visited traits (for cycle detection).
116+
*
117+
* @return array<string> Array of all trait names used by the given trait (directly and transitively).
118+
*/
119+
private function getAllTraitsUsedByTrait(string $traitName, array $visited = []): array
120+
{
121+
// Prevent infinite loops.
122+
if (in_array($traitName, $visited, true)) {
123+
return [];
124+
}
125+
126+
$visited[] = $traitName;
127+
128+
try {
129+
if (!$this->reflectionProvider->hasClass($traitName)) {
130+
return [];
131+
}
132+
133+
$traitReflection = $this->reflectionProvider->getClass($traitName);
134+
if (!$traitReflection->isTrait()) {
135+
return [];
136+
}
137+
138+
$allTraits = [];
139+
140+
// Get direct traits used by this trait.
141+
foreach ($traitReflection->getTraits() as $trait) {
142+
$usedTraitName = $trait->getName();
143+
$allTraits[] = $usedTraitName;
144+
145+
// Recursively get traits used by the used trait.
146+
$nestedTraits = $this->getAllTraitsUsedByTrait($usedTraitName, $visited);
147+
$allTraits = array_merge($allTraits, $nestedTraits);
148+
}
149+
150+
return array_unique($allTraits);
151+
} catch (Throwable $e) {
152+
return [];
153+
}
154+
}
155+
156+
}

‎tests/Rules/Classes/NoRedundantTraitUseRuleTest.php‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Classes;
4+
5+
use PHPStan\Rules\Rule;
6+
use PHPStan\Testing\RuleTestCase;
7+
8+
/**
9+
* @extends RuleTestCase<NoRedundantTraitUseRule>
10+
*/
11+
class NoRedundantTraitUseRuleTest extends RuleTestCase
12+
{
13+
14+
protected function getRule(): Rule
15+
{
16+
return new NoRedundantTraitUseRule($this->createReflectionProvider());
17+
}
18+
19+
public function testNoRedundantTraitUse(): void
20+
{
21+
$this->analyse([__DIR__ . '/data/no-redundant-trait-use.php'], [
22+
[
23+
'Class uses trait "BarTrait" redundantly as it is already included via trait "FooTrait".',
24+
24,
25+
],
26+
]);
27+
}
28+
29+
public function testValidTraitUse(): void
30+
{
31+
$this->analyse([__DIR__ . '/data/no-redundant-trait-use-valid.php'], []);
32+
}
33+
34+
public function testSingleTraitUse(): void
35+
{
36+
$this->analyse([__DIR__ . '/data/no-redundant-trait-use-single-trait.php'], []);
37+
}
38+
39+
public function testTransitiveRedundantTraitUse(): void
40+
{
41+
$this->analyse([__DIR__ . '/data/no-redundant-trait-use-transitive.php'], [
42+
[
43+
'Class uses trait "BaseTrait" redundantly as it is already included via trait "WrapperTrait".',
44+
24,
45+
],
46+
]);
47+
}
48+
49+
}

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php declare(strict_types = 1);
2+
3+
trait SingleTrait
4+
{
5+
6+
public function singleMethod(): void
7+
{
8+
}
9+
10+
}
11+
12+
// This class uses only one trait - this is valid (no redundancy possible)
13+
class SingleTraitUseClass
14+
{
15+
16+
use SingleTrait;
17+
18+
}

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php declare(strict_types = 1);
2+
3+
trait BaseTrait
4+
{
5+
6+
public function baseMethod(): void
7+
{
8+
}
9+
10+
}
11+
12+
trait WrapperTrait
13+
{
14+
15+
use BaseTrait;
16+
17+
public function wrapperMethod(): void
18+
{
19+
}
20+
21+
}
22+
23+
// This class uses WrapperTrait and BaseTrait, but BaseTrait is already included via WrapperTrait
24+
class TransitiveRedundantTraitUseClass
25+
{
26+
27+
use WrapperTrait;
28+
use BaseTrait; // This should trigger an error (transitive redundancy)
29+
30+
}

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php declare(strict_types = 1);
2+
3+
trait IndependentTrait1
4+
{
5+
6+
public function method1(): void
7+
{
8+
}
9+
10+
}
11+
12+
trait IndependentTrait2
13+
{
14+
15+
public function method2(): void
16+
{
17+
}
18+
19+
}
20+
21+
// This class uses two independent traits - this is valid
22+
class ValidTraitUseClass
23+
{
24+
25+
use IndependentTrait1;
26+
use IndependentTrait2;
27+
28+
}

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php declare(strict_types = 1);
2+
3+
trait BarTrait
4+
{
5+
6+
public function barMethod(): void
7+
{
8+
}
9+
10+
}
11+
12+
trait FooTrait
13+
{
14+
15+
use BarTrait;
16+
17+
public function fooMethod(): void
18+
{
19+
}
20+
21+
}
22+
23+
// This class uses both FooTrait and BarTrait, but BarTrait is already included via FooTrait
24+
class RedundantTraitUseClass
25+
{
26+
27+
use FooTrait;
28+
use BarTrait; // This should trigger an error
29+
30+
}

0 commit comments

Comments
 (0)