Skip to content

Commit

Permalink
Fix reporting overriden variadics
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Oct 10, 2020
1 parent 3640fcd commit 2cd7001
Show file tree
Hide file tree
Showing 3 changed files with 153 additions and 20 deletions.
77 changes: 57 additions & 20 deletions src/Rules/Methods/OverridingMethodRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ public function processNode(Node $node, Scope $scope): array
$methodVariant = ParametersAcceptorSelector::selectSingle($method->getVariants());
$methodParameters = $methodVariant->getParameters();

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

if ($prototypeParameter->isVariadic()) {
$prototypeAfterVariadic = true;
if (!$methodParameter->isVariadic()) {
$messages[] = RuleErrorBuilder::message(sprintf(
'Parameter #%d $%s of method %s::%s() is not variadic but parameter #%d $%s of method %s::%s() is variadic.',
$i + 1,
$methodParameter->getName(),
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
$i + 1,
$prototypeParameter->getName(),
$prototype->getDeclaringClass()->getDisplayName(),
$prototype->getName()
))->nonIgnorable()->build();
continue;
if (!$methodParameter->isOptional()) {
if (count($methodParameters) !== $i + 1) {
$messages[] = RuleErrorBuilder::message(sprintf(
'Parameter #%d $%s of method %s::%s() is not optional.',
$i + 1,
$methodParameter->getName(),
$method->getDeclaringClass()->getDisplayName(),
$method->getName()
))->nonIgnorable()->build();
continue;
}

$messages[] = RuleErrorBuilder::message(sprintf(
'Parameter #%d $%s of method %s::%s() is not variadic but parameter #%d $%s of method %s::%s() is variadic.',
$i + 1,
$methodParameter->getName(),
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
$i + 1,
$prototypeParameter->getName(),
$prototype->getDeclaringClass()->getDisplayName(),
$prototype->getName()
))->nonIgnorable()->build();
continue;
} elseif (count($methodParameters) === $i + 1) {
$messages[] = RuleErrorBuilder::message(sprintf(
'Parameter #%d $%s of method %s::%s() is not variadic.',
$i + 1,
$methodParameter->getName(),
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
))->nonIgnorable()->build();
}
}
} elseif ($methodParameter->isVariadic()) {
$messages[] = RuleErrorBuilder::message(sprintf(
Expand Down Expand Up @@ -285,17 +308,31 @@ public function processNode(Node $node, Scope $scope): array
continue;
}

if ($methodParameter->isOptional()) {
if (
$j === count($methodParameters) - 1
&& $prototypeAfterVariadic
&& !$methodParameter->isVariadic()
) {
$messages[] = RuleErrorBuilder::message(sprintf(
'Parameter #%d $%s of method %s::%s() is not variadic.',
$j + 1,
$methodParameter->getName(),
$method->getDeclaringClass()->getDisplayName(),
$method->getName()
))->nonIgnorable()->build();
continue;
}

$messages[] = RuleErrorBuilder::message(sprintf(
'Parameter #%d $%s of method %s::%s() is not optional.',
$j + 1,
$methodParameter->getName(),
$method->getDeclaringClass()->getDisplayName(),
$method->getName()
))->nonIgnorable()->build();
if (!$methodParameter->isOptional()) {
$messages[] = RuleErrorBuilder::message(sprintf(
'Parameter #%d $%s of method %s::%s() is not optional.',
$j + 1,
$methodParameter->getName(),
$method->getDeclaringClass()->getDisplayName(),
$method->getName()
))->nonIgnorable()->build();
continue;
}
}

$methodReturnType = $methodVariant->getNativeReturnType();
Expand Down
27 changes: 27 additions & 0 deletions tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -355,4 +355,31 @@ public function testBug3629(): void
$this->analyse([__DIR__ . '/data/bug-3629.php'], []);
}

public function testVariadics(): void
{
if (!self::$useStaticReflectionProvider) {
$this->markTestSkipped('Test requires static reflection.');
}

$this->phpVersionId = PHP_VERSION_ID;
$this->analyse([__DIR__ . '/data/overriding-variadics.php'], [
[
'Parameter #2 $lang of method OverridingVariadics\OtherTranslator::translate() is not optional.',
34,
],
[
'Parameter #2 $lang of method OverridingVariadics\AnotherTranslator::translate() is not optional.',
44,
],
[
'Parameter #3 $parameters of method OverridingVariadics\AnotherTranslator::translate() is not variadic.',
44,
],
[
'Parameter #2 $lang of method OverridingVariadics\YetAnotherTranslator::translate() is not variadic.',
54,
],
]);
}

}
69 changes: 69 additions & 0 deletions tests/PHPStan/Rules/Methods/data/overriding-variadics.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
<?php

namespace OverridingVariadics;

interface ITranslator
{

/**
* Translates the given string.
* @param mixed $message
* @param string ...$parameters
*/
function translate($message, string ...$parameters): string;

}

class Translator implements ITranslator
{

/**
* @param string $message
* @param string ...$parameters
*/
public function translate($message, $lang = 'cs', string ...$parameters): string
{

}

}

class OtherTranslator implements ITranslator
{

public function translate($message, $lang, string ...$parameters): string
{

}

}

class AnotherTranslator implements ITranslator
{

public function translate($message, $lang = 'cs', string $parameters): string
{

}

}

class YetAnotherTranslator implements ITranslator
{

public function translate($message, $lang = 'cs'): string
{

}

}

class ReflectionClass extends \ReflectionClass
{

public function newInstance($arg = null, ...$args)
{

}

}

0 comments on commit 2cd7001

Please sign in to comment.