Skip to content

Commit ac0cfd0

Browse files
ondrejmirtesondrejmirtes
committed
Overriding method rule - check parameter type compatibility
1 parent f7210dc commit ac0cfd0

File tree

10 files changed

+693
-4
lines changed

10 files changed

+693
-4
lines changed

‎src/Php/PhpVersion.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,9 @@ public function supportsNullCoalesceAssign(): bool
1717
return $this->versionId >= 70400;
1818
}
1919

20+
public function supportsParameterContravariance(): bool
21+
{
22+
return $this->versionId >= 70400;
23+
}
24+
2025
}

‎src/Reflection/MethodPrototypeReflection.php‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,28 @@ class MethodPrototypeReflection implements ClassMemberReflection
1919

2020
private bool $isFinal;
2121

22+
/** @var ParametersAcceptor[] */
23+
private array $variants;
24+
25+
/**
26+
* @param string $name
27+
* @param ClassReflection $declaringClass
28+
* @param bool $isStatic
29+
* @param bool $isPrivate
30+
* @param bool $isPublic
31+
* @param bool $isAbstract
32+
* @param bool $isFinal
33+
* @param ParametersAcceptor[] $variants
34+
*/
2235
public function __construct(
2336
string $name,
2437
ClassReflection $declaringClass,
2538
bool $isStatic,
2639
bool $isPrivate,
2740
bool $isPublic,
2841
bool $isAbstract,
29-
bool $isFinal
42+
bool $isFinal,
43+
array $variants
3044
)
3145
{
3246
$this->name = $name;
@@ -36,6 +50,7 @@ public function __construct(
3650
$this->isPublic = $isPublic;
3751
$this->isAbstract = $isAbstract;
3852
$this->isFinal = $isFinal;
53+
$this->variants = $variants;
3954
}
4055

4156
public function getName(): string
@@ -78,4 +93,12 @@ public function getDocComment(): ?string
7893
return null;
7994
}
8095

96+
/**
97+
* @return ParametersAcceptor[]
98+
*/
99+
public function getVariants(): array
100+
{
101+
return $this->variants;
102+
}
103+
81104
}

‎src/Reflection/Native/NativeMethodReflection.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,8 @@ public function getPrototype(): ClassMemberReflection
9090
$prototypeMethod->isPrivate(),
9191
$prototypeMethod->isPublic(),
9292
$prototypeMethod->isAbstract(),
93-
$prototypeMethod->isFinal()
93+
$prototypeMethod->isFinal(),
94+
$prototypeDeclaringClass->getNativeMethod($prototypeMethod->getName())->getVariants()
9495
);
9596
} catch (\ReflectionException $e) {
9697
return $this;

‎src/Reflection/Php/PhpMethodReflection.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,8 @@ public function getPrototype(): ClassMemberReflection
164164
$prototypeMethod->isPrivate(),
165165
$prototypeMethod->isPublic(),
166166
$prototypeMethod->isAbstract(),
167-
$prototypeMethod->isFinal()
167+
$prototypeMethod->isFinal(),
168+
$prototypeDeclaringClass->getNativeMethod($prototypeMethod->getName())->getVariants()
168169
);
169170
} catch (\ReflectionException $e) {
170171
return $this;

‎src/Rules/Methods/OverridingMethodRule.php‎

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,34 @@
55
use PhpParser\Node;
66
use PHPStan\Analyser\Scope;
77
use PHPStan\Node\InClassMethodNode;
8+
use PHPStan\Php\PhpVersion;
89
use PHPStan\Reflection\MethodPrototypeReflection;
10+
use PHPStan\Reflection\Native\NativeParameterReflection;
11+
use PHPStan\Reflection\ParametersAcceptorSelector;
912
use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection;
13+
use PHPStan\Reflection\Php\PhpParameterReflection;
1014
use PHPStan\Rules\Rule;
1115
use PHPStan\Rules\RuleErrorBuilder;
16+
use PHPStan\Type\ArrayType;
17+
use PHPStan\Type\IterableType;
18+
use PHPStan\Type\MixedType;
19+
use PHPStan\Type\ObjectType;
20+
use PHPStan\Type\TypeCombinator;
21+
use PHPStan\Type\VerbosityLevel;
1222

1323
/**
1424
* @implements Rule<InClassMethodNode>
1525
*/
1626
class OverridingMethodRule implements Rule
1727
{
1828

29+
private PhpVersion $phpVersion;
30+
31+
public function __construct(PhpVersion $phpVersion)
32+
{
33+
$this->phpVersion = $phpVersion;
34+
}
35+
1936
public function getNodeType(): string
2037
{
2138
return InClassMethodNode::class;
@@ -88,6 +105,194 @@ public function processNode(Node $node, Scope $scope): array
88105
))->nonIgnorable()->build();
89106
}
90107

108+
if (strtolower($method->getName()) === '__construct') {
109+
if (!$prototype->isAbstract()) {
110+
return $messages;
111+
}
112+
}
113+
114+
$prototypeVariants = $prototype->getVariants();
115+
if (count($prototypeVariants) !== 1) {
116+
return $messages;
117+
}
118+
119+
$prototypeVariant = $prototypeVariants[0];
120+
121+
$methodVariant = ParametersAcceptorSelector::selectSingle($method->getVariants());
122+
$methodParameters = $methodVariant->getParameters();
123+
124+
foreach ($prototypeVariant->getParameters() as $i => $prototypeParameter) {
125+
if (!array_key_exists($i, $methodParameters)) {
126+
$messages[] = RuleErrorBuilder::message(sprintf(
127+
'Method %s::%s() overrides method %s::%s() but misses parameter #%d $%s.',
128+
$method->getDeclaringClass()->getName(),
129+
$method->getName(),
130+
$prototype->getDeclaringClass()->getName(),
131+
$prototype->getName(),
132+
$i + 1,
133+
$prototypeParameter->getName()
134+
))->nonIgnorable()->build();
135+
continue;
136+
}
137+
138+
$methodParameter = $methodParameters[$i];
139+
if ($prototypeParameter->passedByReference()->no()) {
140+
if (!$methodParameter->passedByReference()->no()) {
141+
$messages[] = RuleErrorBuilder::message(sprintf(
142+
'Parameter #%d $%s of method %s::%s() is passed by reference but parameter #%d $%s of method %s::%s() is not passed by reference.',
143+
$i + 1,
144+
$methodParameter->getName(),
145+
$method->getDeclaringClass()->getName(),
146+
$method->getName(),
147+
$i + 1,
148+
$prototypeParameter->getName(),
149+
$prototype->getDeclaringClass()->getName(),
150+
$prototype->getName()
151+
))->nonIgnorable()->build();
152+
}
153+
} elseif ($methodParameter->passedByReference()->no()) {
154+
$messages[] = RuleErrorBuilder::message(sprintf(
155+
'Parameter #%d $%s of method %s::%s() is not passed by reference but parameter #%d $%s of method %s::%s() is passed by reference.',
156+
$i + 1,
157+
$methodParameter->getName(),
158+
$method->getDeclaringClass()->getName(),
159+
$method->getName(),
160+
$i + 1,
161+
$prototypeParameter->getName(),
162+
$prototype->getDeclaringClass()->getName(),
163+
$prototype->getName()
164+
))->nonIgnorable()->build();
165+
}
166+
167+
if ($prototypeParameter->isVariadic()) {
168+
if (!$methodParameter->isVariadic()) {
169+
$messages[] = RuleErrorBuilder::message(sprintf(
170+
'Parameter #%d $%s of method %s::%s() is not variadic but parameter #%d $%s of method %s::%s() is variadic.',
171+
$i + 1,
172+
$methodParameter->getName(),
173+
$method->getDeclaringClass()->getName(),
174+
$method->getName(),
175+
$i + 1,
176+
$prototypeParameter->getName(),
177+
$prototype->getDeclaringClass()->getName(),
178+
$prototype->getName()
179+
))->nonIgnorable()->build();
180+
continue;
181+
}
182+
} elseif ($methodParameter->isVariadic()) {
183+
$messages[] = RuleErrorBuilder::message(sprintf(
184+
'Parameter #%d $%s of method %s::%s() is variadic but parameter #%d $%s of method %s::%s() is not variadic.',
185+
$i + 1,
186+
$methodParameter->getName(),
187+
$method->getDeclaringClass()->getName(),
188+
$method->getName(),
189+
$i + 1,
190+
$prototypeParameter->getName(),
191+
$prototype->getDeclaringClass()->getName(),
192+
$prototype->getName()
193+
))->nonIgnorable()->build();
194+
continue;
195+
}
196+
197+
if ($prototypeParameter->isOptional() && !$methodParameter->isOptional()) {
198+
$messages[] = RuleErrorBuilder::message(sprintf(
199+
'Parameter #%d $%s of method %s::%s() is required but parameter #%d $%s of method %s::%s() is optional.',
200+
$i + 1,
201+
$methodParameter->getName(),
202+
$method->getDeclaringClass()->getName(),
203+
$method->getName(),
204+
$i + 1,
205+
$prototypeParameter->getName(),
206+
$prototype->getDeclaringClass()->getName(),
207+
$prototype->getName()
208+
))->nonIgnorable()->build();
209+
}
210+
211+
$methodParameterType = $methodParameter->getNativeType();
212+
if ($methodParameterType instanceof MixedType) {
213+
continue;
214+
}
215+
216+
if ($prototypeParameter instanceof PhpParameterReflection) {
217+
$prototypeParameterType = $prototypeParameter->getNativeType();
218+
} elseif ($prototypeParameter instanceof NativeParameterReflection) {
219+
$prototypeParameterType = $prototypeParameter->getType();
220+
} else {
221+
continue;
222+
}
223+
224+
if (!$this->phpVersion->supportsParameterContravariance()) {
225+
$originalPrototypeParameterType = $prototypeParameterType;
226+
if (TypeCombinator::containsNull($methodParameterType)) {
227+
$prototypeParameterType = TypeCombinator::removeNull($prototypeParameterType);
228+
}
229+
$originalMethodParameterType = $methodParameterType;
230+
$methodParameterType = TypeCombinator::removeNull($methodParameterType);
231+
if ($methodParameterType->equals($prototypeParameterType)) {
232+
continue;
233+
}
234+
235+
if ($methodParameterType instanceof IterableType) {
236+
if ($prototypeParameterType instanceof ArrayType) {
237+
continue;
238+
}
239+
if ($prototypeParameterType instanceof ObjectType && $prototypeParameterType->getClassName() === \Traversable::class) {
240+
continue;
241+
}
242+
}
243+
244+
$messages[] = RuleErrorBuilder::message(sprintf(
245+
'Parameter #%d $%s (%s) of method %s::%s() is not compatible with parameter #%d $%s (%s) of method %s::%s().',
246+
$i + 1,
247+
$methodParameter->getName(),
248+
$originalMethodParameterType->describe(VerbosityLevel::typeOnly()),
249+
$method->getDeclaringClass()->getName(),
250+
$method->getName(),
251+
$i + 1,
252+
$prototypeParameter->getName(),
253+
$originalPrototypeParameterType->describe(VerbosityLevel::typeOnly()),
254+
$prototype->getDeclaringClass()->getName(),
255+
$prototype->getName()
256+
))->nonIgnorable()->build();
257+
} elseif (!$methodParameterType->isSuperTypeOf($prototypeParameterType)->yes()) {
258+
$messages[] = RuleErrorBuilder::message(sprintf(
259+
'Parameter #%d $%s (%s) of method %s::%s() is not contravariant with parameter #%d $%s (%s) of method %s::%s().',
260+
$i + 1,
261+
$methodParameter->getName(),
262+
$methodParameterType->describe(VerbosityLevel::typeOnly()),
263+
$method->getDeclaringClass()->getName(),
264+
$method->getName(),
265+
$i + 1,
266+
$prototypeParameter->getName(),
267+
$prototypeParameterType->describe(VerbosityLevel::typeOnly()),
268+
$prototype->getDeclaringClass()->getName(),
269+
$prototype->getName()
270+
))->nonIgnorable()->build();
271+
}
272+
}
273+
274+
if (!isset($i)) {
275+
$i = -1;
276+
}
277+
278+
foreach ($methodParameters as $j => $methodParameter) {
279+
if ($j <= $i) {
280+
continue;
281+
}
282+
283+
if ($methodParameter->isOptional()) {
284+
continue;
285+
}
286+
287+
$messages[] = RuleErrorBuilder::message(sprintf(
288+
'Parameter #%d $%s of method %s::%s() is not optional.',
289+
$j + 1,
290+
$methodParameter->getName(),
291+
$method->getDeclaringClass()->getName(),
292+
$method->getName()
293+
))->nonIgnorable()->build();
294+
}
295+
91296
return $messages;
92297
}
93298

0 commit comments

Comments
 (0)