Skip to content

Commit d8b44f4

Browse files
committed
Abstract trait method check - do not check visibility
1 parent 9bf44e2 commit d8b44f4

File tree

3 files changed

+68
-20
lines changed

3 files changed

+68
-20
lines changed

src/Rules/Methods/OverridingMethodRule.php

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ public function getNodeType(): string
4949
public function processNode(Node $node, Scope $scope): array
5050
{
5151
$method = $node->getMethodReflection();
52-
$prototype = $this->findPrototype($node->getClassReflection(), $method->getName());
53-
if ($prototype === null) {
52+
$prototypeData = $this->findPrototype($node->getClassReflection(), $method->getName());
53+
if ($prototypeData === null) {
5454
if (strtolower($method->getName()) === '__construct') {
5555
$parent = $method->getDeclaringClass()->getParentClass();
5656
if ($parent !== null && $parent->hasConstructor()) {
@@ -93,6 +93,8 @@ public function processNode(Node $node, Scope $scope): array
9393
return [];
9494
}
9595

96+
[$prototype, $checkVisibility] = $prototypeData;
97+
9698
$messages = [];
9799
if ($prototype->isFinalByKeyword()->yes()) {
98100
$messages[] = RuleErrorBuilder::message(sprintf(
@@ -132,19 +134,19 @@ public function processNode(Node $node, Scope $scope): array
132134
))->nonIgnorable()->build();
133135
}
134136

135-
if ($prototype->isPublic()) {
136-
if (!$method->isPublic()) {
137-
$messages[] = RuleErrorBuilder::message(sprintf(
138-
'%s method %s::%s() overriding public method %s::%s() should also be public.',
139-
$method->isPrivate() ? 'Private' : 'Protected',
140-
$method->getDeclaringClass()->getDisplayName(),
141-
$method->getName(),
142-
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
143-
$prototype->getName(),
144-
))->nonIgnorable()->build();
145-
}
146-
} elseif ($method->isPrivate()) {
147-
if (!$prototype->isPrivate()) {
137+
if ($checkVisibility) {
138+
if ($prototype->isPublic()) {
139+
if (!$method->isPublic()) {
140+
$messages[] = RuleErrorBuilder::message(sprintf(
141+
'%s method %s::%s() overriding public method %s::%s() should also be public.',
142+
$method->isPrivate() ? 'Private' : 'Protected',
143+
$method->getDeclaringClass()->getDisplayName(),
144+
$method->getName(),
145+
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
146+
$prototype->getName(),
147+
))->nonIgnorable()->build();
148+
}
149+
} elseif ($method->isPrivate()) {
148150
$messages[] = RuleErrorBuilder::message(sprintf(
149151
'Private method %s::%s() overriding protected method %s::%s() should be protected or public.',
150152
$method->getDeclaringClass()->getDisplayName(),
@@ -293,11 +295,14 @@ private function hasOverrideAttribute(Node\Stmt\ClassMethod $method): bool
293295
return false;
294296
}
295297

296-
private function findPrototype(ClassReflection $classReflection, string $methodName): ?ExtendedMethodReflection
298+
/**
299+
* @return array{ExtendedMethodReflection, bool}|null
300+
*/
301+
private function findPrototype(ClassReflection $classReflection, string $methodName): ?array
297302
{
298303
foreach ($classReflection->getImmediateInterfaces() as $immediateInterface) {
299304
if ($immediateInterface->hasNativeMethod($methodName)) {
300-
return $immediateInterface->getNativeMethod($methodName);
305+
return [$immediateInterface->getNativeMethod($methodName), true];
301306
}
302307
}
303308

@@ -311,10 +316,10 @@ private function findPrototype(ClassReflection $classReflection, string $methodN
311316
$isAbstract = $method->isAbstract();
312317
if (is_bool($isAbstract)) {
313318
if ($isAbstract) {
314-
return $method;
319+
return [$method, false];
315320
}
316321
} elseif ($isAbstract->yes()) {
317-
return $method;
322+
return [$method, false];
318323
}
319324
}
320325
}
@@ -350,7 +355,7 @@ private function findPrototype(ClassReflection $classReflection, string $methodN
350355
}
351356
}
352357

353-
return $method;
358+
return [$method, true];
354359
}
355360

356361
}

tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -722,6 +722,14 @@ public function testTraits(): void
722722
'Parameter #1 $i (int) of method OverridingTraitMethods\Baz::doBar() is not contravariant with parameter #1 $i (string) of method OverridingTraitMethods\FooPrivate::doBar().',
723723
45,
724724
],
725+
[
726+
'Static method OverridingTraitMethods\Ipsum::doBar() overrides non-static method OverridingTraitMethods\Foo::doBar().',
727+
65,
728+
],
729+
[
730+
'Non-static method OverridingTraitMethods\Dolor::doBar() overrides static method OverridingTraitMethods\FooStatic::doBar().',
731+
80,
732+
],
725733
];
726734
}
727735
$this->phpVersionId = PHP_VERSION_ID;

tests/PHPStan/Rules/Methods/data/overriding-trait-methods.php

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,38 @@ private function doBar(int $i): int
4747

4848
}
4949
}
50+
51+
class Lorem
52+
{
53+
use Foo;
54+
55+
protected function doBar(string $i): int
56+
{
57+
58+
}
59+
}
60+
61+
class Ipsum
62+
{
63+
use Foo;
64+
65+
public static function doBar(string $i): int
66+
{
67+
68+
}
69+
}
70+
71+
trait FooStatic
72+
{
73+
abstract public static function doBar(string $i): int;
74+
}
75+
76+
class Dolor
77+
{
78+
use FooStatic;
79+
80+
public function doBar(string $i): int
81+
{
82+
83+
}
84+
}

0 commit comments

Comments
 (0)