Skip to content

Commit e2b5288

Browse files
samsonasiksamsonasik
committed
[CodeQuality] Change behaviour of OptionalParametersAfterRequiredRector to fill null default value when previous param is optional
1 parent f9b2954 commit e2b5288

12 files changed

Lines changed: 61 additions & 182 deletions

File tree

‎rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/mashup.php.inc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ namespace Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterReq
1717

1818
class Mashup
1919
{
20-
public function run($required, $yetRequired, $optional = 1, $anotherOptional = false)
20+
public function run($optional = 1, $required = null, $anotherOptional = false, $yetRequired = null)
2121
{
2222
}
2323
}

‎rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/new_the_constructor.php.inc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,13 @@ namespace Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterReq
2626

2727
final class NewTheConstructor
2828
{
29-
public function __construct($required, $optional = 1)
29+
public function __construct($optional = 1, $required = null)
3030
{
3131
}
3232

3333
public function create()
3434
{
35-
return new self(5, 1);
35+
return new self(1, 5);
3636
}
3737
}
3838

‎rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/on_function.php.inc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ foo($optional, $required);
1414

1515
namespace Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterRequiredRector\Fixture;
1616

17-
function foo($required, $optional = 1)
17+
function foo($optional = 1, $required = null)
1818
{
1919
}
2020

21-
foo($required, $optional);
21+
foo($optional, $required);
2222

2323
?>

‎rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/some_object.php.inc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ namespace Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterReq
1717

1818
class SomeObject
1919
{
20-
public function run($required, $optional = 1)
20+
public function run($optional = 1, $required = null)
2121
{
2222
}
2323
}

‎rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/update_method_call.php.inc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,13 @@ namespace Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterReq
2626

2727
final class UpdateMethodCall
2828
{
29-
public function run($required, $optional = 1)
29+
public function run($optional = 1, $required = null)
3030
{
3131
}
3232

3333
public function process()
3434
{
35-
$this->run(5, 1);
35+
$this->run(1, 5);
3636
}
3737
}
3838

‎rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/update_method_call_by_fluent.php.inc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ namespace Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterReq
3232

3333
final class UpdateMethodCallByFluent
3434
{
35-
public function run($required, $optional = 1)
35+
public function run($optional = 1, $required = null)
3636
{
3737
return $this;
3838
}
@@ -43,7 +43,7 @@ final class UpdateMethodCallByFluent
4343

4444
public function process()
4545
{
46-
$this->run(5, 1)
46+
$this->run(1, 5)
4747
->execute();
4848
}
4949
}

‎rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/update_method_call_by_fluent2.php.inc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,15 +38,15 @@ final class UpdateMethodCallByFluent2
3838
return $this;
3939
}
4040

41-
public function run($required, $optional = 1)
41+
public function run($optional = 1, $required = null)
4242
{
4343
return $this;
4444
}
4545

4646
public function process()
4747
{
4848
$this->execute()
49-
->run(5, 1);
49+
->run(1, 5);
5050
}
5151
}
5252

‎rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/update_params_order_of_static_method_call.php.inc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,12 @@ namespace Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterReq
2424

2525
final class WebUtil
2626
{
27-
public static function radioList($name, $selected, $items = [], $class = "", $groupingCount = 0, $groupingClass = "", $required = false)
27+
public static function radioList($name, $items = [], $selected = null, $class = "", $groupingCount = 0, $groupingClass = "", $required = false)
2828
{
2929

3030
}
3131
}
3232

33-
WebUtil::radioList("inPersonOrVirtual", $selected, $inPersonOrVirtualItems, "", 1, "col-sm-3 margin-bottom-10");
33+
WebUtil::radioList("inPersonOrVirtual", $inPersonOrVirtualItems, $selected, "", 1, "col-sm-3 margin-bottom-10");
3434

3535
?>

‎rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/uses_with_less_params.php.inc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,14 @@ namespace Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterReq
2727

2828
final class UsesWithLess
2929
{
30-
public function blah($required, $optional = 1, $optional_2 = 2)
30+
public function blah($optional = 1, $required = null, $optional_2 = 2)
3131
{
3232
}
3333

3434
public function process()
3535
{
36-
$this->blah(5, 1);
37-
$this->blah(5, 1, 2);
36+
$this->blah(1, 5);
37+
$this->blah(1, 5, 2);
3838
}
3939
}
4040

‎rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php‎

Lines changed: 38 additions & 160 deletions
Original file line numberDiff line numberDiff line change
@@ -5,23 +5,15 @@
55
namespace Rector\CodeQuality\Rector\ClassMethod;
66

77
use PhpParser\Node;
8-
use PhpParser\Node\Expr\FuncCall;
9-
use PhpParser\Node\Expr\MethodCall;
10-
use PhpParser\Node\Expr\New_;
11-
use PhpParser\Node\Expr\StaticCall;
8+
use PhpParser\Node\Expr;
9+
use PhpParser\Node\Expr\ConstFetch;
10+
use PhpParser\Node\Name;
11+
use PhpParser\Node\NullableType;
12+
use PhpParser\Node\Param;
1213
use PhpParser\Node\Stmt\ClassMethod;
1314
use PhpParser\Node\Stmt\Function_;
14-
use PHPStan\Analyser\Scope;
15-
use PHPStan\Reflection\FunctionReflection;
16-
use PHPStan\Reflection\MethodReflection;
17-
use PHPStan\Reflection\Native\NativeFunctionReflection;
18-
use Rector\CodingStyle\Reflection\VendorLocationDetector;
19-
use Rector\NodeTypeResolver\PHPStan\ParametersAcceptorSelectorVariantsWrapper;
20-
use Rector\Php80\NodeResolver\ArgumentSorter;
21-
use Rector\Php80\NodeResolver\RequireOptionalParamResolver;
22-
use Rector\PHPStan\ScopeFetcher;
15+
use PhpParser\Node\UnionType;
2316
use Rector\Rector\AbstractRector;
24-
use Rector\Reflection\ReflectionResolver;
2517
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
2618
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
2719

@@ -30,22 +22,9 @@
3022
*/
3123
final class OptionalParametersAfterRequiredRector extends AbstractRector
3224
{
33-
/**
34-
* @var string
35-
*/
36-
private const HAS_SWAPPED_PARAMS = 'has_swapped_params';
37-
38-
public function __construct(
39-
private readonly RequireOptionalParamResolver $requireOptionalParamResolver,
40-
private readonly ArgumentSorter $argumentSorter,
41-
private readonly ReflectionResolver $reflectionResolver,
42-
private readonly VendorLocationDetector $vendorLocationDetector
43-
) {
44-
}
45-
4625
public function getRuleDefinition(): RuleDefinition
4726
{
48-
return new RuleDefinition('Move required parameters after optional ones', [
27+
return new RuleDefinition('Add null default value when a required parameter follows an optional one', [
4928
new CodeSample(
5029
<<<'CODE_SAMPLE'
5130
class SomeObject
@@ -60,7 +39,7 @@ public function run($optional = 1, $required)
6039
<<<'CODE_SAMPLE'
6140
class SomeObject
6241
{
63-
public function run($required, $optional = 1)
42+
public function run($optional = 1, $required = null)
6443
{
6544
}
6645
}
@@ -74,155 +53,54 @@ public function run($required, $optional = 1)
7453
*/
7554
public function getNodeTypes(): array
7655
{
77-
return [
78-
ClassMethod::class,
79-
Function_::class,
80-
New_::class,
81-
MethodCall::class,
82-
StaticCall::class,
83-
FuncCall::class,
84-
];
56+
return [ClassMethod::class, Function_::class];
8557
}
8658

8759
/**
88-
* @param ClassMethod|Function_|New_|MethodCall|StaticCall|FuncCall $node
60+
* @param ClassMethod|Function_ $node
8961
*/
90-
public function refactor(Node $node): ClassMethod|Function_|null|New_|MethodCall|StaticCall|FuncCall
91-
{
92-
if ($node instanceof ClassMethod || $node instanceof Function_) {
93-
return $this->refactorClassMethodOrFunction($node);
94-
}
95-
96-
if ($node instanceof New_) {
97-
return $this->refactorNew($node);
98-
}
99-
100-
return $this->refactorMethodCallOrFuncCall($node);
101-
}
102-
103-
private function refactorClassMethodOrFunction(ClassMethod|Function_ $node): ClassMethod|Function_|null
62+
public function refactor(Node $node): ClassMethod|Function_|null
10463
{
10564
if ($node->params === []) {
10665
return null;
10766
}
10867

109-
if ($node->getAttribute(self::HAS_SWAPPED_PARAMS, false) === true) {
110-
return null;
111-
}
68+
$hasChanged = false;
69+
foreach ($node->params as $key => $param) {
70+
if ($param->default instanceof Expr) {
71+
continue;
72+
}
11273

113-
$scope = ScopeFetcher::fetch($node);
114-
if ($node instanceof ClassMethod) {
115-
$reflection = $this->reflectionResolver->resolveMethodReflectionFromClassMethod($node, $scope);
116-
} else {
117-
$reflection = $this->reflectionResolver->resolveFunctionReflectionFromFunction($node);
118-
}
74+
if ($param->variadic) {
75+
continue;
76+
}
11977

120-
if (! $reflection instanceof MethodReflection && ! $reflection instanceof FunctionReflection) {
121-
return null;
122-
}
78+
$previousParam = $node->params[$key - 1] ?? null;
79+
if ($previousParam instanceof Param && $previousParam->default instanceof Expr) {
80+
$hasChanged = false;
12381

124-
$expectedArgOrParamOrder = $this->resolveExpectedArgParamOrderIfDifferent($reflection, $node, $scope);
125-
if ($expectedArgOrParamOrder === null) {
126-
return null;
127-
}
82+
$param->default = new ConstFetch(new Name('null'));
83+
$paramType = $param->type;
12884

129-
$node->params = $this->argumentSorter->sortArgsByExpectedParamOrder(
130-
$node->params,
131-
$expectedArgOrParamOrder
132-
);
133-
134-
$node->setAttribute(self::HAS_SWAPPED_PARAMS, true);
135-
return $node;
136-
}
85+
if (! $paramType instanceof Node) {
86+
continue;
87+
}
13788

138-
private function refactorNew(New_ $new): ?New_
139-
{
140-
if ($new->args === []) {
141-
return null;
142-
}
89+
if ($paramType instanceof NullableType) {
90+
continue;
91+
}
14392

144-
if ($new->isFirstClassCallable()) {
145-
return null;
146-
}
147-
148-
$methodReflection = $this->reflectionResolver->resolveMethodReflectionFromNew($new);
149-
if (! $methodReflection instanceof MethodReflection) {
150-
return null;
151-
}
152-
153-
$scope = ScopeFetcher::fetch($new);
154-
$expectedArgOrParamOrder = $this->resolveExpectedArgParamOrderIfDifferent($methodReflection, $new, $scope);
155-
if ($expectedArgOrParamOrder === null) {
156-
return null;
157-
}
93+
if ($paramType instanceof UnionType) {
94+
$paramType->types[] = new ConstFetch(new Name('null'));
95+
$paramType->types = array_unique($paramType->types, SORT_REGULAR);
15896

159-
$new->args = $this->argumentSorter->sortArgsByExpectedParamOrder($new->getArgs(), $expectedArgOrParamOrder);
97+
continue;
98+
}
16099

161-
return $new;
162-
}
163-
164-
private function refactorMethodCallOrFuncCall(
165-
MethodCall|StaticCall|FuncCall $node
166-
): MethodCall|StaticCall|FuncCall|null {
167-
if ($node->isFirstClassCallable()) {
168-
return null;
169-
}
170-
171-
$reflection = $this->reflectionResolver->resolveFunctionLikeReflectionFromCall($node);
172-
if (! $reflection instanceof MethodReflection && ! $reflection instanceof FunctionReflection) {
173-
return null;
174-
}
175-
176-
$scope = ScopeFetcher::fetch($node);
177-
$expectedArgOrParamOrder = $this->resolveExpectedArgParamOrderIfDifferent($reflection, $node, $scope);
178-
if ($expectedArgOrParamOrder === null) {
179-
return null;
180-
}
181-
182-
$newArgs = $this->argumentSorter->sortArgsByExpectedParamOrder($node->getArgs(), $expectedArgOrParamOrder);
183-
184-
if ($node->args === $newArgs) {
185-
return null;
186-
}
187-
188-
$node->args = $newArgs;
189-
return $node;
190-
}
191-
192-
/**
193-
* @return int[]|null
194-
*/
195-
private function resolveExpectedArgParamOrderIfDifferent(
196-
MethodReflection|FunctionReflection $reflection,
197-
New_|MethodCall|ClassMethod|Function_|StaticCall|FuncCall $node,
198-
Scope $scope
199-
): ?array {
200-
if ($reflection instanceof NativeFunctionReflection) {
201-
return null;
202-
}
203-
204-
if ($reflection instanceof MethodReflection && $this->vendorLocationDetector->detectMethodReflection(
205-
$reflection
206-
)) {
207-
return null;
208-
}
209-
210-
if ($reflection instanceof FunctionReflection && $this->vendorLocationDetector->detectFunctionReflection(
211-
$reflection
212-
)) {
213-
return null;
214-
}
215-
216-
$scope = ScopeFetcher::fetch($node);
217-
$parametersAcceptor = ParametersAcceptorSelectorVariantsWrapper::select($reflection, $node, $scope);
218-
$expectedParameterReflections = $this->requireOptionalParamResolver->resolveFromParametersAcceptor(
219-
$parametersAcceptor
220-
);
221-
222-
if ($expectedParameterReflections === $parametersAcceptor->getParameters()) {
223-
return null;
100+
$paramType = new NullableType($paramType);
101+
}
224102
}
225103

226-
return array_keys($expectedParameterReflections);
104+
return $hasChanged ? $node : null;
227105
}
228106
}

0 commit comments

Comments
 (0)