Skip to content

Commit 9d8d716

Browse files
VincentLangletVincentLanglet
authored
Differenciate signaturePrototype and inheritancePrototype
1 parent d7ba1e3 commit 9d8d716

File tree

6 files changed

+187
-18
lines changed

6 files changed

+187
-18
lines changed

‎src/Rules/Methods/MethodPrototypeFinder.php‎

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,20 @@ 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+
* Also, return a bool to precise if the visibility of the prototype needs to be respected.
33+
*
34+
* @return array{ExtendedMethodReflection, ClassReflection, bool, ExtendedMethodReflection, ClassReflection}|null
2935
*/
3036
public function findPrototype(ClassReflection $classReflection, string $methodName): ?array
3137
{
3238
foreach ($classReflection->getImmediateInterfaces() as $immediateInterface) {
3339
if ($immediateInterface->hasNativeMethod($methodName)) {
3440
$method = $immediateInterface->getNativeMethod($methodName);
35-
return [$method, $method->getDeclaringClass(), true];
41+
return [$method, $method->getDeclaringClass(), true, $method, $method->getDeclaringClass()];
3642
}
3743
}
3844

@@ -47,15 +53,19 @@ public function findPrototype(ClassReflection $classReflection, string $methodNa
4753
$isAbstract = $methodReflection->isAbstract();
4854
if ($isAbstract) {
4955
$declaringTrait = $trait->getNativeMethod($methodName)->getDeclaringClass();
56+
$prototype = $this->phpClassReflectionExtension->createUserlandMethodReflection(
57+
$trait,
58+
$classReflection,
59+
$methodReflection,
60+
$declaringTrait->getName(),
61+
);
62+
5063
return [
51-
$this->phpClassReflectionExtension->createUserlandMethodReflection(
52-
$trait,
53-
$classReflection,
54-
$methodReflection,
55-
$declaringTrait->getName(),
56-
),
64+
$prototype,
5765
$declaringTrait,
5866
false,
67+
$prototype,
68+
$declaringTrait,
5969
];
6070
}
6171
}
@@ -94,7 +104,23 @@ public function findPrototype(ClassReflection $classReflection, string $methodNa
94104
}
95105
}
96106

97-
return [$method, $method->getDeclaringClass(), true];
107+
$prototype = $method;
108+
if (strtolower($method->getName()) === '__construct') {
109+
foreach ($parentClass->getInterfaces() as $interface) {
110+
if ($interface->hasNativeMethod($method->getName())) {
111+
$prototype = $interface->getNativeMethod($method->getName());
112+
break;
113+
}
114+
}
115+
}
116+
117+
return [
118+
$prototype,
119+
$prototype->getDeclaringClass(),
120+
true,
121+
$method,
122+
$declaringClass,
123+
];
98124
}
99125

100126
}

‎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: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -803,6 +803,29 @@ 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+
817+
public function testBug12272(): void
818+
{
819+
$this->phpVersionId = PHP_VERSION_ID;
820+
$this->analyse([__DIR__ . '/data/bug-12272.php'], []);
821+
}
822+
823+
public function testBug12830(): void
824+
{
825+
$this->phpVersionId = PHP_VERSION_ID;
826+
$this->analyse([__DIR__ . '/data/bug-12830.php'], []);
827+
}
828+
806829
public function testBug9524(): void
807830
{
808831
$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+
}

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
namespace Bug12272;
4+
5+
interface ExceptionContract
6+
{
7+
public function __construct(string $message);
8+
}
9+
10+
class BaseException extends Exception implements ExceptionContract
11+
{
12+
public function __construct(string $message, ?Throwable $previous = null)
13+
{
14+
parent::__construct($message, 0, $previous);
15+
}
16+
}
17+
18+
class SomeException extends BaseException
19+
{
20+
}
21+
22+
class SpecificException extends SomeException
23+
{
24+
public function __construct(string $message, int $code = 0)
25+
{
26+
if ($code) {
27+
echo 1;
28+
}
29+
30+
parent::__construct($message);
31+
}
32+
}

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace Bug12830;
4+
5+
interface I
6+
{
7+
public function __construct(string $mustBeString);
8+
}
9+
10+
class A implements I
11+
{
12+
public string $MustBeString;
13+
public int $CanBeInt;
14+
15+
public function __construct(string $mustBeString, int $canBeInt = -1)
16+
{
17+
$this->MustBeString = $mustBeString;
18+
$this->CanBeInt = $canBeInt;
19+
}
20+
}
21+
22+
class B extends A
23+
{
24+
public bool $CanBeBool;
25+
26+
public function __construct(string $mustBeString, bool $canBeBool = false)
27+
{
28+
$this->MustBeString = $mustBeString;
29+
$this->CanBeBool = $canBeBool;
30+
}
31+
}
32+
33+
var_dump([
34+
new A('A', 1),
35+
new B('B', true),
36+
]);

0 commit comments

Comments
 (0)