Skip to content

Commit 2cd7001

Browse files
ondrejmirtesondrejmirtes
committed
Fix reporting overriden variadics
1 parent 3640fcd commit 2cd7001

File tree

3 files changed

+153
-20
lines changed

3 files changed

+153
-20
lines changed

‎src/Rules/Methods/OverridingMethodRule.php‎

Lines changed: 57 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ public function processNode(Node $node, Scope $scope): array
146146
$methodVariant = ParametersAcceptorSelector::selectSingle($method->getVariants());
147147
$methodParameters = $methodVariant->getParameters();
148148

149+
$prototypeAfterVariadic = false;
149150
foreach ($prototypeVariant->getParameters() as $i => $prototypeParameter) {
150151
if (!array_key_exists($i, $methodParameters)) {
151152
$messages[] = RuleErrorBuilder::message(sprintf(
@@ -190,19 +191,41 @@ public function processNode(Node $node, Scope $scope): array
190191
}
191192

192193
if ($prototypeParameter->isVariadic()) {
194+
$prototypeAfterVariadic = true;
193195
if (!$methodParameter->isVariadic()) {
194-
$messages[] = RuleErrorBuilder::message(sprintf(
195-
'Parameter #%d $%s of method %s::%s() is not variadic but parameter #%d $%s of method %s::%s() is variadic.',
196-
$i + 1,
197-
$methodParameter->getName(),
198-
$method->getDeclaringClass()->getDisplayName(),
199-
$method->getName(),
200-
$i + 1,
201-
$prototypeParameter->getName(),
202-
$prototype->getDeclaringClass()->getDisplayName(),
203-
$prototype->getName()
204-
))->nonIgnorable()->build();
205-
continue;
196+
if (!$methodParameter->isOptional()) {
197+
if (count($methodParameters) !== $i + 1) {
198+
$messages[] = RuleErrorBuilder::message(sprintf(
199+
'Parameter #%d $%s of method %s::%s() is not optional.',
200+
$i + 1,
201+
$methodParameter->getName(),
202+
$method->getDeclaringClass()->getDisplayName(),
203+
$method->getName()
204+
))->nonIgnorable()->build();
205+
continue;
206+
}
207+
208+
$messages[] = RuleErrorBuilder::message(sprintf(
209+
'Parameter #%d $%s of method %s::%s() is not variadic but parameter #%d $%s of method %s::%s() is variadic.',
210+
$i + 1,
211+
$methodParameter->getName(),
212+
$method->getDeclaringClass()->getDisplayName(),
213+
$method->getName(),
214+
$i + 1,
215+
$prototypeParameter->getName(),
216+
$prototype->getDeclaringClass()->getDisplayName(),
217+
$prototype->getName()
218+
))->nonIgnorable()->build();
219+
continue;
220+
} elseif (count($methodParameters) === $i + 1) {
221+
$messages[] = RuleErrorBuilder::message(sprintf(
222+
'Parameter #%d $%s of method %s::%s() is not variadic.',
223+
$i + 1,
224+
$methodParameter->getName(),
225+
$method->getDeclaringClass()->getDisplayName(),
226+
$method->getName(),
227+
))->nonIgnorable()->build();
228+
}
206229
}
207230
} elseif ($methodParameter->isVariadic()) {
208231
$messages[] = RuleErrorBuilder::message(sprintf(
@@ -285,17 +308,31 @@ public function processNode(Node $node, Scope $scope): array
285308
continue;
286309
}
287310

288-
if ($methodParameter->isOptional()) {
311+
if (
312+
$j === count($methodParameters) - 1
313+
&& $prototypeAfterVariadic
314+
&& !$methodParameter->isVariadic()
315+
) {
316+
$messages[] = RuleErrorBuilder::message(sprintf(
317+
'Parameter #%d $%s of method %s::%s() is not variadic.',
318+
$j + 1,
319+
$methodParameter->getName(),
320+
$method->getDeclaringClass()->getDisplayName(),
321+
$method->getName()
322+
))->nonIgnorable()->build();
289323
continue;
290324
}
291325

292-
$messages[] = RuleErrorBuilder::message(sprintf(
293-
'Parameter #%d $%s of method %s::%s() is not optional.',
294-
$j + 1,
295-
$methodParameter->getName(),
296-
$method->getDeclaringClass()->getDisplayName(),
297-
$method->getName()
298-
))->nonIgnorable()->build();
326+
if (!$methodParameter->isOptional()) {
327+
$messages[] = RuleErrorBuilder::message(sprintf(
328+
'Parameter #%d $%s of method %s::%s() is not optional.',
329+
$j + 1,
330+
$methodParameter->getName(),
331+
$method->getDeclaringClass()->getDisplayName(),
332+
$method->getName()
333+
))->nonIgnorable()->build();
334+
continue;
335+
}
299336
}
300337

301338
$methodReturnType = $methodVariant->getNativeReturnType();

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,4 +355,31 @@ public function testBug3629(): void
355355
$this->analyse([__DIR__ . '/data/bug-3629.php'], []);
356356
}
357357

358+
public function testVariadics(): void
359+
{
360+
if (!self::$useStaticReflectionProvider) {
361+
$this->markTestSkipped('Test requires static reflection.');
362+
}
363+
364+
$this->phpVersionId = PHP_VERSION_ID;
365+
$this->analyse([__DIR__ . '/data/overriding-variadics.php'], [
366+
[
367+
'Parameter #2 $lang of method OverridingVariadics\OtherTranslator::translate() is not optional.',
368+
34,
369+
],
370+
[
371+
'Parameter #2 $lang of method OverridingVariadics\AnotherTranslator::translate() is not optional.',
372+
44,
373+
],
374+
[
375+
'Parameter #3 $parameters of method OverridingVariadics\AnotherTranslator::translate() is not variadic.',
376+
44,
377+
],
378+
[
379+
'Parameter #2 $lang of method OverridingVariadics\YetAnotherTranslator::translate() is not variadic.',
380+
54,
381+
],
382+
]);
383+
}
384+
358385
}

‎tests/PHPStan/Rules/Methods/data/overriding-variadics.php‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
<?php
2+
3+
namespace OverridingVariadics;
4+
5+
interface ITranslator
6+
{
7+
8+
/**
9+
* Translates the given string.
10+
* @param mixed $message
11+
* @param string ...$parameters
12+
*/
13+
function translate($message, string ...$parameters): string;
14+
15+
}
16+
17+
class Translator implements ITranslator
18+
{
19+
20+
/**
21+
* @param string $message
22+
* @param string ...$parameters
23+
*/
24+
public function translate($message, $lang = 'cs', string ...$parameters): string
25+
{
26+
27+
}
28+
29+
}
30+
31+
class OtherTranslator implements ITranslator
32+
{
33+
34+
public function translate($message, $lang, string ...$parameters): string
35+
{
36+
37+
}
38+
39+
}
40+
41+
class AnotherTranslator implements ITranslator
42+
{
43+
44+
public function translate($message, $lang = 'cs', string $parameters): string
45+
{
46+
47+
}
48+
49+
}
50+
51+
class YetAnotherTranslator implements ITranslator
52+
{
53+
54+
public function translate($message, $lang = 'cs'): string
55+
{
56+
57+
}
58+
59+
}
60+
61+
class ReflectionClass extends \ReflectionClass
62+
{
63+
64+
public function newInstance($arg = null, ...$args)
65+
{
66+
67+
}
68+
69+
}

0 commit comments

Comments
 (0)