Skip to content

Commit 35fbc87

Browse files
ondrejmirtesondrejmirtes
committed
Internal PHPStan rule - use named arguments instead of passing default parameter values
1 parent 682b2b5 commit 35fbc87

File tree

4 files changed

+282
-0
lines changed

4 files changed

+282
-0
lines changed

‎build/PHPStan/Build/NamedArgumentsRule.php

Lines changed: 216 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,216 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Build;
4+
5+
use PhpParser\Node;
6+
use PHPStan\Analyser\ArgumentsNormalizer;
7+
use PHPStan\Analyser\Scope;
8+
use PHPStan\Reflection\ExtendedParametersAcceptor;
9+
use PHPStan\Reflection\ReflectionProvider;
10+
use PHPStan\Rules\IdentifierRuleError;
11+
use PHPStan\Rules\Rule;
12+
use PHPStan\Rules\RuleErrorBuilder;
13+
use PHPStan\ShouldNotHappenException;
14+
use PHPStan\Type\VerbosityLevel;
15+
use function count;
16+
use function implode;
17+
use function sprintf;
18+
19+
/**
20+
* @implements Rule<Node\Expr\CallLike>
21+
*/
22+
final class NamedArgumentsRule implements Rule
23+
{
24+
25+
public function __construct(private ReflectionProvider $reflectionProvider)
26+
{
27+
}
28+
29+
public function getNodeType(): string
30+
{
31+
return Node\Expr\CallLike::class;
32+
}
33+
34+
public function processNode(Node $node, Scope $scope): array
35+
{
36+
if ($node->isFirstClassCallable()) {
37+
return [];
38+
}
39+
40+
if ($node instanceof Node\Expr\FuncCall && $node->name instanceof Node\Name) {
41+
if ($this->reflectionProvider->hasFunction($node->name, $scope)) {
42+
$function = $this->reflectionProvider->getFunction($node->name, $scope);
43+
$variants = $function->getVariants();
44+
if (count($variants) !== 1) {
45+
return [];
46+
}
47+
48+
return $this->processArgs($variants[0], $scope, $node);
49+
}
50+
}
51+
52+
if ($node instanceof Node\Expr\New_ && $node->class instanceof Node\Name) {
53+
if ($this->reflectionProvider->hasClass($node->class->toString())) {
54+
$class = $this->reflectionProvider->getClass($node->class->toString());
55+
if ($class->hasConstructor()) {
56+
$constructor = $class->getConstructor();
57+
$variants = $constructor->getVariants();
58+
if (count($variants) !== 1) {
59+
return [];
60+
}
61+
62+
return $this->processArgs($variants[0], $scope, $node);
63+
}
64+
}
65+
}
66+
67+
if ($node instanceof Node\Expr\StaticCall && $node->class instanceof Node\Name && $node->name instanceof Node\Identifier) {
68+
$className = $scope->resolveName($node->class);
69+
if ($this->reflectionProvider->hasClass($className)) {
70+
$class = $this->reflectionProvider->getClass($className);
71+
if ($class->hasNativeMethod($node->name->toString())) {
72+
$method = $class->getNativeMethod($node->name->toString());
73+
$variants = $method->getVariants();
74+
if (count($variants) !== 1) {
75+
return [];
76+
}
77+
78+
return $this->processArgs($variants[0], $scope, $node);
79+
}
80+
}
81+
}
82+
83+
return [];
84+
}
85+
86+
/**
87+
* @param Node\Expr\FuncCall|Node\Expr\New_|Node\Expr\StaticCall $node
88+
* @return list<IdentifierRuleError>
89+
*/
90+
private function processArgs(ExtendedParametersAcceptor $acceptor, Scope $scope, Node\Expr\CallLike $node): array
91+
{
92+
if ($acceptor->isVariadic()) {
93+
return [];
94+
}
95+
$normalizedArgs = ArgumentsNormalizer::reorderArgs($acceptor, $node->getArgs());
96+
if ($normalizedArgs === null) {
97+
return [];
98+
}
99+
100+
$hasNamedArgument = false;
101+
foreach ($node->getArgs() as $arg) {
102+
if ($arg->name === null) {
103+
continue;
104+
}
105+
106+
$hasNamedArgument = true;
107+
break;
108+
}
109+
110+
$errorBuilders = [];
111+
$parameters = $acceptor->getParameters();
112+
$defaultValueWasPassed = [];
113+
foreach ($normalizedArgs as $i => $normalizedArg) {
114+
if ($normalizedArg->unpack) {
115+
return [];
116+
}
117+
$parameter = $parameters[$i];
118+
if ($parameter->getDefaultValue() === null) {
119+
continue;
120+
}
121+
$argValue = $scope->getType($normalizedArg->value);
122+
if ($normalizedArg->name !== null) {
123+
continue;
124+
}
125+
126+
/** @var Node\Arg|null $originalArg */
127+
$originalArg = $normalizedArg->getAttribute(ArgumentsNormalizer::ORIGINAL_ARG_ATTRIBUTE);
128+
if ($originalArg === null) {
129+
if ($hasNamedArgument) {
130+
// this is an optional parameter not passed by the user, but filled in by ArgumentsNormalizer
131+
continue;
132+
}
133+
}
134+
135+
if (!$argValue->equals($parameter->getDefaultValue())) {
136+
if (count($defaultValueWasPassed) > 0) {
137+
$errorBuilders[] = RuleErrorBuilder::message(sprintf(
138+
'You\'re passing a non-default value %s to parameter $%s but previous %s (%s). You can skip %s and use named argument for $%s instead.',
139+
$argValue->describe(VerbosityLevel::precise()),
140+
$parameter->getName(),
141+
count($defaultValueWasPassed) === 1 ? 'argument is passing default value to its parameter' : 'arguments are passing default values to their parameters',
142+
implode(', ', $defaultValueWasPassed),
143+
count($defaultValueWasPassed) === 1 ? 'it' : 'them',
144+
$parameter->getName(),
145+
))
146+
->identifier('phpstan.namedArgument')
147+
->line($normalizedArg->getStartLine())
148+
->nonIgnorable();
149+
}
150+
continue;
151+
} else {
152+
if ($originalArg !== null && $originalArg->name !== null) {
153+
$errorBuilders[] = RuleErrorBuilder::message(sprintf('Named argument $%s can be omitted, type %s is the same as the default value.', $originalArg->name, $argValue->describe(VerbosityLevel::precise())))
154+
->identifier('phpstan.namedArgumentWithDefaultValue')
155+
->nonIgnorable();
156+
continue;
157+
}
158+
}
159+
160+
$defaultValueWasPassed[] = '$' . $parameter->getName();
161+
}
162+
163+
if (count($errorBuilders) > 0) {
164+
$errorBuilders[0]->fixNode(static function (Node $node) use ($acceptor, $hasNamedArgument, $parameters, $scope) {
165+
/** @var Node\Expr\FuncCall|Node\Expr\New_|Node\Expr\StaticCall $node */
166+
$normalizedArgs = ArgumentsNormalizer::reorderArgs($acceptor, $node->getArgs());
167+
if ($normalizedArgs === null) {
168+
return $node;
169+
}
170+
171+
$newArgs = [];
172+
$skippedOptional = false;
173+
foreach ($normalizedArgs as $i => $normalizedArg) {
174+
/** @var Node\Arg|null $originalArg */
175+
$originalArg = $normalizedArg->getAttribute(ArgumentsNormalizer::ORIGINAL_ARG_ATTRIBUTE);
176+
if ($originalArg === null) {
177+
if ($hasNamedArgument) {
178+
// this is an optional parameter not passed by the user, but filled in by ArgumentsNormalizer
179+
continue;
180+
}
181+
182+
$originalArg = $normalizedArg;
183+
}
184+
$parameter = $parameters[$i];
185+
if ($parameter->getDefaultValue() === null) {
186+
$newArgs[] = $originalArg;
187+
continue;
188+
}
189+
$argValue = $scope->getType($normalizedArg->value);
190+
if ($argValue->equals($parameter->getDefaultValue())) {
191+
$skippedOptional = true;
192+
continue;
193+
}
194+
195+
if ($skippedOptional) {
196+
if ($parameter->getName() === '') {
197+
throw new ShouldNotHappenException();
198+
}
199+
200+
$newArgs[] = new Node\Arg($originalArg->value, $originalArg->byRef, $originalArg->unpack, $originalArg->getAttributes(), new Node\Identifier($parameter->getName()));
201+
continue;
202+
}
203+
204+
$newArgs[] = $originalArg;
205+
}
206+
207+
$node->args = $newArgs;
208+
209+
return $node;
210+
});
211+
}
212+
213+
return array_map(static fn ($builder) => $builder->build(), $errorBuilders);
214+
}
215+
216+
}

‎build/phpstan.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ parameters:
118118
rules:
119119
- PHPStan\Build\FinalClassRule
120120
- PHPStan\Build\AttributeNamedArgumentsRule
121+
- PHPStan\Build\NamedArgumentsRule
121122

122123
services:
123124
-

‎tests/PHPStan/Build/NamedArgumentsRuleTest.php

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Build;
4+
5+
use PHPStan\Rules\Rule;
6+
use PHPStan\Testing\RuleTestCase;
7+
use const PHP_VERSION_ID;
8+
9+
/**
10+
* @extends RuleTestCase<NamedArgumentsRule>
11+
*/
12+
class NamedArgumentsRuleTest extends RuleTestCase
13+
{
14+
15+
protected function getRule(): Rule
16+
{
17+
return new NamedArgumentsRule($this->createReflectionProvider());
18+
}
19+
20+
public function testRule(): void
21+
{
22+
if (PHP_VERSION_ID < 80000) {
23+
$this->markTestSkipped('Test requires PHP 8.0.');
24+
}
25+
26+
$this->analyse([__DIR__ . '/data/named-arguments.php'], [
27+
[
28+
'You\'re passing a non-default value Exception to parameter $previous but previous argument is passing default value to its parameter ($code). You can skip it and use named argument for $previous instead.',
29+
14,
30+
],
31+
[
32+
'Named argument $code can be omitted, type 0 is the same as the default value.',
33+
16,
34+
],
35+
[
36+
'You\'re passing a non-default value Exception to parameter $previous but previous arguments are passing default values to their parameters ($message, $code). You can skip them and use named argument for $previous instead.',
37+
20,
38+
],
39+
]);
40+
}
41+
42+
}

‎tests/PHPStan/Build/data/named-arguments.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php // lint >= 8.0
2+
3+
namespace NamedArgumentRule;
4+
5+
use Exception;
6+
7+
class Foo
8+
{
9+
10+
public function doFoo(): void
11+
{
12+
new Exception('foo', 0);
13+
new Exception('foo', 0, null);
14+
new Exception('foo', 0, new Exception('previous'));
15+
new Exception('foo', previous: new Exception('previous'));
16+
new Exception('foo', code: 0, previous: new Exception('previous'));
17+
new Exception('foo', code: 1, previous: new Exception('previous'));
18+
new Exception('foo', 1, new Exception('previous'));
19+
new Exception('foo', 1);
20+
new Exception('', 0, new Exception('previous'));
21+
}
22+
23+
}

0 commit comments

Comments
 (0)