Skip to content

Commit 1e6c462

Browse files
VincentLangletVincentLanglet
committed
Differenciate signaturePrototype and inheritancePrototype
1 parent d9b9af7 commit 1e6c462

File tree

4 files changed

+113
-30
lines changed

4 files changed

+113
-30
lines changed

‎src/Rules/Methods/MethodPrototypeFinder.php‎

Lines changed: 41 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,19 @@ public function __construct(
2525
}
2626

2727
/**
28-
* @return array{ExtendedMethodReflection, ClassReflection, bool}|null
28+
* Finds the prototype method that a class method should be validated against.
29+
* Returns two prototypes with different purposes:
30+
* - Signature prototype: Used for validating method signature (parameters, return type, ...).
31+
* - Inheritance prototype: Used for validating inheritance rules (final keyword, override attribute, ...).
32+
*
33+
* @return array{ExtendedMethodReflection, ClassReflection, bool, ExtendedMethodReflection, ClassReflection}|null
2934
*/
3035
public function findPrototype(ClassReflection $classReflection, string $methodName): ?array
3136
{
3237
foreach ($classReflection->getImmediateInterfaces() as $immediateInterface) {
3338
if ($immediateInterface->hasNativeMethod($methodName)) {
3439
$method = $immediateInterface->getNativeMethod($methodName);
35-
return [$method, $method->getDeclaringClass(), true];
40+
return [$method, $method->getDeclaringClass(), true, $method, $method->getDeclaringClass()];
3641
}
3742
}
3843

@@ -47,15 +52,19 @@ public function findPrototype(ClassReflection $classReflection, string $methodNa
4752
$isAbstract = $methodReflection->isAbstract();
4853
if ($isAbstract) {
4954
$declaringTrait = $trait->getNativeMethod($methodName)->getDeclaringClass();
55+
$prototype = $this->phpClassReflectionExtension->createUserlandMethodReflection(
56+
$trait,
57+
$classReflection,
58+
$methodReflection,
59+
$declaringTrait->getName(),
60+
);
61+
5062
return [
51-
$this->phpClassReflectionExtension->createUserlandMethodReflection(
52-
$trait,
53-
$classReflection,
54-
$methodReflection,
55-
$declaringTrait->getName(),
56-
),
63+
$prototype,
5764
$declaringTrait,
5865
false,
66+
$prototype,
67+
$declaringTrait,
5968
];
6069
}
6170
}
@@ -76,25 +85,36 @@ public function findPrototype(ClassReflection $classReflection, string $methodNa
7685
}
7786

7887
$declaringClass = $method->getDeclaringClass();
79-
if ($declaringClass->hasConstructor()) {
80-
if ($method->getName() === $declaringClass->getConstructor()->getName()) {
81-
$prototype = $method->getPrototype();
82-
if ($prototype instanceof PhpMethodReflection || $prototype instanceof MethodPrototypeReflection || $prototype instanceof NativeMethodReflection) {
83-
$abstract = $prototype->isAbstract();
84-
if (is_bool($abstract)) {
85-
if (!$abstract) {
86-
return null;
87-
}
88-
} elseif (!$abstract->yes()) {
88+
if ($declaringClass->hasConstructor() && $method->getName() === $declaringClass->getConstructor()->getName()) {
89+
$prototype = $method->getPrototype();
90+
if ($prototype instanceof PhpMethodReflection || $prototype instanceof MethodPrototypeReflection || $prototype instanceof NativeMethodReflection) {
91+
$abstract = $prototype->isAbstract();
92+
if (is_bool($abstract)) {
93+
if (!$abstract) {
8994
return null;
9095
}
96+
} elseif (!$abstract->yes()) {
97+
return null;
98+
}
99+
}
100+
}
101+
102+
$prototype = $method;
103+
if (strtolower($method->getName()) === '__construct') {
104+
foreach ($parentClass->getInterfaces() as $interface) {
105+
if ($interface->hasNativeMethod($method->getName())) {
106+
$prototype = $interface->getNativeMethod($method->getName());
91107
}
92-
} elseif (strtolower($methodName) === '__construct') {
93-
return null;
94108
}
95109
}
96110

97-
return [$method, $method->getDeclaringClass(), true];
111+
return [
112+
$prototype,
113+
$prototype->getDeclaringClass(),
114+
true,
115+
$method,
116+
$declaringClass,
117+
];
98118
}
99119

100120
}

‎src/Rules/Methods/OverridingMethodRule.php‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,13 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker $scope): array
107107
return [];
108108
}
109109

110-
[$prototype, $prototypeDeclaringClass, $checkVisibility] = $prototypeData;
110+
[
111+
$prototype,
112+
$prototypeDeclaringClass,
113+
$checkVisibility,
114+
$inheritancePrototype,
115+
$inheritancePrototypeDeclaringClass,
116+
] = $prototypeData;
111117

112118
$messages = [];
113119
if (
@@ -119,8 +125,8 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker $scope): array
119125
'Method %s::%s() overrides method %s::%s() but is missing the #[\Override] attribute.',
120126
$method->getDeclaringClass()->getDisplayName(),
121127
$method->getName(),
122-
$prototypeDeclaringClass->getDisplayName(true),
123-
$prototype->getName(),
128+
$inheritancePrototypeDeclaringClass->getDisplayName(true),
129+
$inheritancePrototype->getName(),
124130
))
125131
->identifier('method.missingOverride')
126132
->fixNode($node->getOriginalNode(), static function (Node\Stmt\ClassMethod $method) {
@@ -132,24 +138,24 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker $scope): array
132138
})
133139
->build();
134140
}
135-
if ($prototype->isFinalByKeyword()->yes()) {
141+
if ($inheritancePrototype->isFinalByKeyword()->yes()) {
136142
$messages[] = RuleErrorBuilder::message(sprintf(
137143
'Method %s::%s() overrides final method %s::%s().',
138144
$method->getDeclaringClass()->getDisplayName(),
139145
$method->getName(),
140-
$prototypeDeclaringClass->getDisplayName(true),
141-
$prototype->getName(),
146+
$inheritancePrototypeDeclaringClass->getDisplayName(true),
147+
$inheritancePrototype->getName(),
142148
))
143149
->nonIgnorable()
144150
->identifier('method.parentMethodFinal')
145151
->build();
146-
} elseif ($prototype->isFinal()->yes()) {
152+
} elseif ($inheritancePrototype->isFinal()->yes()) {
147153
$messages[] = RuleErrorBuilder::message(sprintf(
148154
'Method %s::%s() overrides @final method %s::%s().',
149155
$method->getDeclaringClass()->getDisplayName(),
150156
$method->getName(),
151-
$prototypeDeclaringClass->getDisplayName(true),
152-
$prototype->getName(),
157+
$inheritancePrototypeDeclaringClass->getDisplayName(true),
158+
$inheritancePrototype->getName(),
153159
))->identifier('method.parentMethodFinalByPhpDoc')
154160
->build();
155161
}

‎tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -803,6 +803,17 @@ public function testBug10165(): void
803803
$this->analyse([__DIR__ . '/data/bug-10165.php'], []);
804804
}
805805

806+
public function testBug11067(): void
807+
{
808+
$this->phpVersionId = PHP_VERSION_ID;
809+
$this->analyse([__DIR__ . '/data/bug-11067.php'], [
810+
[
811+
'Method Bug11067\BooleanBuilder2::__construct() overrides final method Bug11067\BaseBuilder2::__construct().',
812+
41,
813+
],
814+
]);
815+
}
816+
806817
public function testBug9524(): void
807818
{
808819
$this->phpVersionId = PHP_VERSION_ID;

‎tests/PHPStan/Rules/Methods/data/bug-11067.php‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug11067;
4+
5+
interface BuilderInterface
6+
{
7+
public function __construct(string $field);
8+
}
9+
10+
class BaseBuilder implements BuilderInterface
11+
{
12+
public function __construct(
13+
protected string $field,
14+
bool $checkType = true,
15+
) {
16+
var_dump($field, $checkType);
17+
}
18+
}
19+
20+
class BooleanBuilder extends BaseBuilder
21+
{
22+
public function __construct(string $field)
23+
{
24+
parent::__construct($field, false);
25+
26+
}
27+
}
28+
29+
class BaseBuilder2 implements BuilderInterface
30+
{
31+
final public function __construct(
32+
protected string $field,
33+
bool $checkType = true,
34+
) {
35+
var_dump($field, $checkType);
36+
}
37+
}
38+
39+
class BooleanBuilder2 extends BaseBuilder2
40+
{
41+
public function __construct(string $field)
42+
{
43+
parent::__construct($field, false);
44+
45+
}
46+
}

0 commit comments

Comments
 (0)