Skip to content

Commit 36833c0

Browse files
phpstan-botphpstan-bot
committed
Detect conflicting trait methods in class compositions
- New ConflictingTraitMethodsRule reports when a class uses multiple traits that define the same method without resolving the conflict via insteadof - Handles abstract trait methods correctly (abstract + concrete is allowed) - Conflicts are also resolved if the class defines the method itself - Registered at level 0 as this is a fatal error in PHP - New regression tests in tests/PHPStan/Rules/Classes/data/bug-14332.php
1 parent 594b7bd commit 36833c0

4 files changed

Lines changed: 326 additions & 0 deletions

File tree

‎src/Rules/Classes/ConflictingTraitMethodsRule.php‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Classes;
4+
5+
use PhpParser\Node;
6+
use PhpParser\Node\Stmt\TraitUseAdaptation\Precedence;
7+
use PHPStan\Analyser\Scope;
8+
use PHPStan\DependencyInjection\RegisteredRule;
9+
use PHPStan\Node\InClassNode;
10+
use PHPStan\Reflection\ReflectionProvider;
11+
use PHPStan\Rules\IdentifierRuleError;
12+
use PHPStan\Rules\Rule;
13+
use PHPStan\Rules\RuleErrorBuilder;
14+
use function array_key_exists;
15+
use function array_keys;
16+
use function count;
17+
use function reset;
18+
use function sprintf;
19+
use function strtolower;
20+
21+
/**
22+
* @implements Rule<InClassNode>
23+
*/
24+
#[RegisteredRule(level: 0)]
25+
final class ConflictingTraitMethodsRule implements Rule
26+
{
27+
28+
public function __construct(private ReflectionProvider $reflectionProvider)
29+
{
30+
}
31+
32+
public function getNodeType(): string
33+
{
34+
return InClassNode::class;
35+
}
36+
37+
/**
38+
* @return list<IdentifierRuleError>
39+
*/
40+
public function processNode(Node $node, Scope $scope): array
41+
{
42+
$classReflection = $node->getClassReflection();
43+
$classLike = $node->getOriginalNode();
44+
$traitUses = $classLike->getTraitUses();
45+
46+
if ($traitUses === []) {
47+
return [];
48+
}
49+
50+
// Collect methods defined directly on the class (not from traits)
51+
$classOwnMethods = [];
52+
foreach ($classLike->getMethods() as $method) {
53+
$classOwnMethods[strtolower($method->name->name)] = true;
54+
}
55+
56+
// Collect all insteadof adaptations
57+
// Key: "traitName::methodName" that is being overridden
58+
$insteadofResolutions = [];
59+
foreach ($traitUses as $traitUse) {
60+
foreach ($traitUse->adaptations as $adaptation) {
61+
if (!$adaptation instanceof Precedence) {
62+
continue;
63+
}
64+
$methodName = strtolower($adaptation->method->name);
65+
foreach ($adaptation->insteadof as $insteadofTrait) {
66+
$insteadofResolutions[strtolower((string) $insteadofTrait) . '::' . $methodName] = true;
67+
}
68+
}
69+
}
70+
71+
// Collect methods from each trait
72+
// Map: lowercased method name => [traitName => true]
73+
$methodTraitMap = [];
74+
foreach ($traitUses as $traitUse) {
75+
foreach ($traitUse->traits as $traitName) {
76+
$traitNameStr = (string) $traitName;
77+
if (!$this->reflectionProvider->hasClass($traitNameStr)) {
78+
continue;
79+
}
80+
$traitReflection = $this->reflectionProvider->getClass($traitNameStr);
81+
if (!$traitReflection->isTrait()) {
82+
continue;
83+
}
84+
85+
foreach ($traitReflection->getNativeReflection()->getMethods() as $method) {
86+
$lowerMethodName = strtolower($method->getName());
87+
$methodTraitMap[$lowerMethodName][$traitReflection->getName()] = [
88+
'name' => $method->getName(),
89+
'abstract' => $method->isAbstract(),
90+
];
91+
}
92+
}
93+
}
94+
95+
$errors = [];
96+
foreach ($methodTraitMap as $lowerMethodName => $traits) {
97+
if (count($traits) <= 1) {
98+
continue;
99+
}
100+
101+
// If the class defines the method itself, no conflict
102+
if (array_key_exists($lowerMethodName, $classOwnMethods)) {
103+
continue;
104+
}
105+
106+
// Filter out abstract methods - PHP allows abstract + concrete without conflict
107+
$concreteTraits = [];
108+
foreach ($traits as $traitName => $methodInfo) {
109+
if ($methodInfo['abstract']) {
110+
continue;
111+
}
112+
113+
$concreteTraits[$traitName] = $methodInfo;
114+
}
115+
116+
if (count($concreteTraits) <= 1) {
117+
continue;
118+
}
119+
120+
// Check which traits still have unresolved conflicts
121+
$unresolvedTraits = [];
122+
foreach ($concreteTraits as $traitName => $methodInfo) {
123+
$key = strtolower($traitName) . '::' . $lowerMethodName;
124+
if (array_key_exists($key, $insteadofResolutions)) {
125+
continue;
126+
}
127+
128+
$unresolvedTraits[$traitName] = $methodInfo['name'];
129+
}
130+
131+
if (count($unresolvedTraits) <= 1) {
132+
continue;
133+
}
134+
135+
$traitNames = array_keys($unresolvedTraits);
136+
$methodName = reset($unresolvedTraits);
137+
138+
$errors[] = RuleErrorBuilder::message(sprintf(
139+
'Trait method %s::%s() has not been applied as %s::%s(), because of collision with %s::%s().',
140+
$traitNames[1],
141+
$methodName,
142+
$classReflection->getDisplayName(),
143+
$methodName,
144+
$traitNames[0],
145+
$methodName,
146+
))
147+
->identifier('class.traitMethodCollision')
148+
->nonIgnorable()
149+
->build();
150+
}
151+
152+
return $errors;
153+
}
154+
155+
}

‎tests/PHPStan/Rules/Classes/ConflictingTraitMethodsRuleTest.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\Reflection\ReflectionProvider;
6+
use PHPStan\Rules\Rule;
7+
use PHPStan\Testing\RuleTestCase;
8+
9+
/**
10+
* @extends RuleTestCase<ConflictingTraitMethodsRule>
11+
*/
12+
class ConflictingTraitMethodsRuleTest extends RuleTestCase
13+
{
14+
15+
protected function getRule(): Rule
16+
{
17+
return new ConflictingTraitMethodsRule(
18+
self::getContainer()->getByType(ReflectionProvider::class),
19+
);
20+
}
21+
22+
public function testBug14332(): void
23+
{
24+
$this->analyse([__DIR__ . '/data/bug-14332.php'], [
25+
[
26+
'Trait method Bug14332\MyTrait2::doSomething() has not been applied as Bug14332\FooWithMultipleConflictingTraits::doSomething(), because of collision with Bug14332\MyTrait1::doSomething().',
27+
20,
28+
],
29+
[
30+
'Trait method Bug14332\MyTrait4::doSomething() has not been applied as Bug14332\FooWithMultipleConflicts::doSomething(), because of collision with Bug14332\MyTrait1::doSomething().',
31+
75,
32+
],
33+
[
34+
'Trait method Bug14332\MyTrait5::anotherMethod() has not been applied as Bug14332\FooWithMultipleConflicts::anotherMethod(), because of collision with Bug14332\MyTrait4::anotherMethod().',
35+
75,
36+
],
37+
[
38+
'Trait method Bug14332\MyTrait5::anotherMethod() has not been applied as Bug14332\FooWithPartialResolution::anotherMethod(), because of collision with Bug14332\MyTrait4::anotherMethod().',
39+
81,
40+
],
41+
]);
42+
}
43+
44+
public function testBug14332Abstract(): void
45+
{
46+
$this->analyse([__DIR__ . '/data/bug-14332-abstract.php'], []);
47+
}
48+
49+
}

‎tests/PHPStan/Rules/Classes/data/bug-14332-abstract.php‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug14332Abstract;
4+
5+
trait ConcreteTrait
6+
{
7+
public function doSomething(): void
8+
{
9+
}
10+
}
11+
12+
trait AbstractTrait
13+
{
14+
abstract public function doSomething(): void;
15+
}
16+
17+
// ok - abstract + concrete is allowed
18+
class FooWithAbstractAndConcrete
19+
{
20+
use ConcreteTrait, AbstractTrait;
21+
}

‎tests/PHPStan/Rules/Classes/data/bug-14332.php‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug14332;
4+
5+
trait MyTrait1
6+
{
7+
public function doSomething(): void
8+
{
9+
}
10+
}
11+
12+
trait MyTrait2
13+
{
14+
public function doSomething(): void
15+
{
16+
}
17+
}
18+
19+
// error - conflicting methods without resolution
20+
class FooWithMultipleConflictingTraits
21+
{
22+
use MyTrait1, MyTrait2;
23+
}
24+
25+
// ok - resolved with insteadof
26+
class FooWithInsteadof
27+
{
28+
use MyTrait1, MyTrait2 {
29+
MyTrait1::doSomething insteadof MyTrait2;
30+
}
31+
}
32+
33+
// ok - class defines the method itself
34+
class FooWithOwnMethod
35+
{
36+
use MyTrait1, MyTrait2;
37+
38+
public function doSomething(): void
39+
{
40+
}
41+
}
42+
43+
trait MyTrait3
44+
{
45+
public function otherMethod(): void
46+
{
47+
}
48+
}
49+
50+
// ok - no conflicting methods
51+
class FooWithNoConflict
52+
{
53+
use MyTrait1, MyTrait3;
54+
}
55+
56+
trait MyTrait4
57+
{
58+
public function doSomething(): void
59+
{
60+
}
61+
62+
public function anotherMethod(): void
63+
{
64+
}
65+
}
66+
67+
trait MyTrait5
68+
{
69+
public function anotherMethod(): void
70+
{
71+
}
72+
}
73+
74+
// error - two conflicting methods from different pairs of traits
75+
class FooWithMultipleConflicts
76+
{
77+
use MyTrait1, MyTrait4, MyTrait5;
78+
}
79+
80+
// error - partially resolved (only one conflict resolved)
81+
class FooWithPartialResolution
82+
{
83+
use MyTrait1, MyTrait4, MyTrait5 {
84+
MyTrait1::doSomething insteadof MyTrait4;
85+
}
86+
}
87+
88+
// ok - all conflicts resolved
89+
class FooWithFullResolution
90+
{
91+
use MyTrait1, MyTrait4, MyTrait5 {
92+
MyTrait1::doSomething insteadof MyTrait4;
93+
MyTrait4::anotherMethod insteadof MyTrait5;
94+
}
95+
}
96+
97+
// ok - single trait, no conflict
98+
class FooWithSingleTrait
99+
{
100+
use MyTrait1;
101+
}

0 commit comments

Comments
 (0)