From e2b52886e2a0a88811bb3fdd74b5bce437ac5277 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 6 Sep 2025 17:17:02 +0700 Subject: [PATCH 1/6] [CodeQuality] Change behaviour of OptionalParametersAfterRequiredRector to fill null default value when previous param is optional --- .../Fixture/mashup.php.inc | 2 +- .../Fixture/new_the_constructor.php.inc | 4 +- .../Fixture/on_function.php.inc | 4 +- .../Fixture/some_object.php.inc | 2 +- .../Fixture/update_method_call.php.inc | 4 +- .../update_method_call_by_fluent.php.inc | 4 +- .../update_method_call_by_fluent2.php.inc | 4 +- ...params_order_of_static_method_call.php.inc | 4 +- .../Fixture/uses_with_less_params.php.inc | 6 +- .../OptionalParametersAfterRequiredRector.php | 198 ++++-------------- .../Privatization/Guard/LaravelModelGuard.php | 7 +- .../Fixture/fixture.php.inc | 4 +- 12 files changed, 61 insertions(+), 182 deletions(-) diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/mashup.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/mashup.php.inc index 73eabd61cfd..24c4e75b9ce 100644 --- a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/mashup.php.inc +++ b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/mashup.php.inc @@ -17,7 +17,7 @@ namespace Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterReq class Mashup { - public function run($required, $yetRequired, $optional = 1, $anotherOptional = false) + public function run($optional = 1, $required = null, $anotherOptional = false, $yetRequired = null) { } } diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/new_the_constructor.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/new_the_constructor.php.inc index 7dbe5313c1a..860c55635a1 100644 --- a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/new_the_constructor.php.inc +++ b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/new_the_constructor.php.inc @@ -26,13 +26,13 @@ namespace Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterReq final class NewTheConstructor { - public function __construct($required, $optional = 1) + public function __construct($optional = 1, $required = null) { } public function create() { - return new self(5, 1); + return new self(1, 5); } } diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/on_function.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/on_function.php.inc index f1f4a248b76..768f593ffb1 100644 --- a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/on_function.php.inc +++ b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/on_function.php.inc @@ -14,10 +14,10 @@ foo($optional, $required); namespace Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterRequiredRector\Fixture; -function foo($required, $optional = 1) +function foo($optional = 1, $required = null) { } -foo($required, $optional); +foo($optional, $required); ?> diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/some_object.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/some_object.php.inc index 5261514d783..1e50fc489a1 100644 --- a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/some_object.php.inc +++ b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/some_object.php.inc @@ -17,7 +17,7 @@ namespace Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterReq class SomeObject { - public function run($required, $optional = 1) + public function run($optional = 1, $required = null) { } } diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/update_method_call.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/update_method_call.php.inc index 42bbd1b4f16..7d0dd962d2c 100644 --- a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/update_method_call.php.inc +++ b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/update_method_call.php.inc @@ -26,13 +26,13 @@ namespace Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterReq final class UpdateMethodCall { - public function run($required, $optional = 1) + public function run($optional = 1, $required = null) { } public function process() { - $this->run(5, 1); + $this->run(1, 5); } } diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/update_method_call_by_fluent.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/update_method_call_by_fluent.php.inc index 040bb324dfb..8e81d249666 100644 --- a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/update_method_call_by_fluent.php.inc +++ b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/update_method_call_by_fluent.php.inc @@ -32,7 +32,7 @@ namespace Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterReq final class UpdateMethodCallByFluent { - public function run($required, $optional = 1) + public function run($optional = 1, $required = null) { return $this; } @@ -43,7 +43,7 @@ final class UpdateMethodCallByFluent public function process() { - $this->run(5, 1) + $this->run(1, 5) ->execute(); } } diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/update_method_call_by_fluent2.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/update_method_call_by_fluent2.php.inc index e8dad329bb3..39e8adae837 100644 --- a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/update_method_call_by_fluent2.php.inc +++ b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/update_method_call_by_fluent2.php.inc @@ -38,7 +38,7 @@ final class UpdateMethodCallByFluent2 return $this; } - public function run($required, $optional = 1) + public function run($optional = 1, $required = null) { return $this; } @@ -46,7 +46,7 @@ final class UpdateMethodCallByFluent2 public function process() { $this->execute() - ->run(5, 1); + ->run(1, 5); } } diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/update_params_order_of_static_method_call.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/update_params_order_of_static_method_call.php.inc index e17cdc76ef5..07cac15f239 100644 --- a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/update_params_order_of_static_method_call.php.inc +++ b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/update_params_order_of_static_method_call.php.inc @@ -24,12 +24,12 @@ namespace Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterReq final class WebUtil { - public static function radioList($name, $selected, $items = [], $class = "", $groupingCount = 0, $groupingClass = "", $required = false) + public static function radioList($name, $items = [], $selected = null, $class = "", $groupingCount = 0, $groupingClass = "", $required = false) { } } -WebUtil::radioList("inPersonOrVirtual", $selected, $inPersonOrVirtualItems, "", 1, "col-sm-3 margin-bottom-10"); +WebUtil::radioList("inPersonOrVirtual", $inPersonOrVirtualItems, $selected, "", 1, "col-sm-3 margin-bottom-10"); ?> diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/uses_with_less_params.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/uses_with_less_params.php.inc index 30acdf2cb7b..28fe322e1f8 100644 --- a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/uses_with_less_params.php.inc +++ b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/uses_with_less_params.php.inc @@ -27,14 +27,14 @@ namespace Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterReq final class UsesWithLess { - public function blah($required, $optional = 1, $optional_2 = 2) + public function blah($optional = 1, $required = null, $optional_2 = 2) { } public function process() { - $this->blah(5, 1); - $this->blah(5, 1, 2); + $this->blah(1, 5); + $this->blah(1, 5, 2); } } diff --git a/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php b/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php index f1d65d1a2ac..0cdc6d692f2 100644 --- a/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php +++ b/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php @@ -5,23 +5,15 @@ namespace Rector\CodeQuality\Rector\ClassMethod; use PhpParser\Node; -use PhpParser\Node\Expr\FuncCall; -use PhpParser\Node\Expr\MethodCall; -use PhpParser\Node\Expr\New_; -use PhpParser\Node\Expr\StaticCall; +use PhpParser\Node\Expr; +use PhpParser\Node\Expr\ConstFetch; +use PhpParser\Node\Name; +use PhpParser\Node\NullableType; +use PhpParser\Node\Param; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Function_; -use PHPStan\Analyser\Scope; -use PHPStan\Reflection\FunctionReflection; -use PHPStan\Reflection\MethodReflection; -use PHPStan\Reflection\Native\NativeFunctionReflection; -use Rector\CodingStyle\Reflection\VendorLocationDetector; -use Rector\NodeTypeResolver\PHPStan\ParametersAcceptorSelectorVariantsWrapper; -use Rector\Php80\NodeResolver\ArgumentSorter; -use Rector\Php80\NodeResolver\RequireOptionalParamResolver; -use Rector\PHPStan\ScopeFetcher; +use PhpParser\Node\UnionType; use Rector\Rector\AbstractRector; -use Rector\Reflection\ReflectionResolver; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -30,22 +22,9 @@ */ final class OptionalParametersAfterRequiredRector extends AbstractRector { - /** - * @var string - */ - private const HAS_SWAPPED_PARAMS = 'has_swapped_params'; - - public function __construct( - private readonly RequireOptionalParamResolver $requireOptionalParamResolver, - private readonly ArgumentSorter $argumentSorter, - private readonly ReflectionResolver $reflectionResolver, - private readonly VendorLocationDetector $vendorLocationDetector - ) { - } - public function getRuleDefinition(): RuleDefinition { - return new RuleDefinition('Move required parameters after optional ones', [ + return new RuleDefinition('Add null default value when a required parameter follows an optional one', [ new CodeSample( <<<'CODE_SAMPLE' class SomeObject @@ -60,7 +39,7 @@ public function run($optional = 1, $required) <<<'CODE_SAMPLE' class SomeObject { - public function run($required, $optional = 1) + public function run($optional = 1, $required = null) { } } @@ -74,155 +53,54 @@ public function run($required, $optional = 1) */ public function getNodeTypes(): array { - return [ - ClassMethod::class, - Function_::class, - New_::class, - MethodCall::class, - StaticCall::class, - FuncCall::class, - ]; + return [ClassMethod::class, Function_::class]; } /** - * @param ClassMethod|Function_|New_|MethodCall|StaticCall|FuncCall $node + * @param ClassMethod|Function_ $node */ - public function refactor(Node $node): ClassMethod|Function_|null|New_|MethodCall|StaticCall|FuncCall - { - if ($node instanceof ClassMethod || $node instanceof Function_) { - return $this->refactorClassMethodOrFunction($node); - } - - if ($node instanceof New_) { - return $this->refactorNew($node); - } - - return $this->refactorMethodCallOrFuncCall($node); - } - - private function refactorClassMethodOrFunction(ClassMethod|Function_ $node): ClassMethod|Function_|null + public function refactor(Node $node): ClassMethod|Function_|null { if ($node->params === []) { return null; } - if ($node->getAttribute(self::HAS_SWAPPED_PARAMS, false) === true) { - return null; - } + $hasChanged = false; + foreach ($node->params as $key => $param) { + if ($param->default instanceof Expr) { + continue; + } - $scope = ScopeFetcher::fetch($node); - if ($node instanceof ClassMethod) { - $reflection = $this->reflectionResolver->resolveMethodReflectionFromClassMethod($node, $scope); - } else { - $reflection = $this->reflectionResolver->resolveFunctionReflectionFromFunction($node); - } + if ($param->variadic) { + continue; + } - if (! $reflection instanceof MethodReflection && ! $reflection instanceof FunctionReflection) { - return null; - } + $previousParam = $node->params[$key - 1] ?? null; + if ($previousParam instanceof Param && $previousParam->default instanceof Expr) { + $hasChanged = false; - $expectedArgOrParamOrder = $this->resolveExpectedArgParamOrderIfDifferent($reflection, $node, $scope); - if ($expectedArgOrParamOrder === null) { - return null; - } + $param->default = new ConstFetch(new Name('null')); + $paramType = $param->type; - $node->params = $this->argumentSorter->sortArgsByExpectedParamOrder( - $node->params, - $expectedArgOrParamOrder - ); - - $node->setAttribute(self::HAS_SWAPPED_PARAMS, true); - return $node; - } + if (! $paramType instanceof Node) { + continue; + } - private function refactorNew(New_ $new): ?New_ - { - if ($new->args === []) { - return null; - } + if ($paramType instanceof NullableType) { + continue; + } - if ($new->isFirstClassCallable()) { - return null; - } - - $methodReflection = $this->reflectionResolver->resolveMethodReflectionFromNew($new); - if (! $methodReflection instanceof MethodReflection) { - return null; - } - - $scope = ScopeFetcher::fetch($new); - $expectedArgOrParamOrder = $this->resolveExpectedArgParamOrderIfDifferent($methodReflection, $new, $scope); - if ($expectedArgOrParamOrder === null) { - return null; - } + if ($paramType instanceof UnionType) { + $paramType->types[] = new ConstFetch(new Name('null')); + $paramType->types = array_unique($paramType->types, SORT_REGULAR); - $new->args = $this->argumentSorter->sortArgsByExpectedParamOrder($new->getArgs(), $expectedArgOrParamOrder); + continue; + } - return $new; - } - - private function refactorMethodCallOrFuncCall( - MethodCall|StaticCall|FuncCall $node - ): MethodCall|StaticCall|FuncCall|null { - if ($node->isFirstClassCallable()) { - return null; - } - - $reflection = $this->reflectionResolver->resolveFunctionLikeReflectionFromCall($node); - if (! $reflection instanceof MethodReflection && ! $reflection instanceof FunctionReflection) { - return null; - } - - $scope = ScopeFetcher::fetch($node); - $expectedArgOrParamOrder = $this->resolveExpectedArgParamOrderIfDifferent($reflection, $node, $scope); - if ($expectedArgOrParamOrder === null) { - return null; - } - - $newArgs = $this->argumentSorter->sortArgsByExpectedParamOrder($node->getArgs(), $expectedArgOrParamOrder); - - if ($node->args === $newArgs) { - return null; - } - - $node->args = $newArgs; - return $node; - } - - /** - * @return int[]|null - */ - private function resolveExpectedArgParamOrderIfDifferent( - MethodReflection|FunctionReflection $reflection, - New_|MethodCall|ClassMethod|Function_|StaticCall|FuncCall $node, - Scope $scope - ): ?array { - if ($reflection instanceof NativeFunctionReflection) { - return null; - } - - if ($reflection instanceof MethodReflection && $this->vendorLocationDetector->detectMethodReflection( - $reflection - )) { - return null; - } - - if ($reflection instanceof FunctionReflection && $this->vendorLocationDetector->detectFunctionReflection( - $reflection - )) { - return null; - } - - $scope = ScopeFetcher::fetch($node); - $parametersAcceptor = ParametersAcceptorSelectorVariantsWrapper::select($reflection, $node, $scope); - $expectedParameterReflections = $this->requireOptionalParamResolver->resolveFromParametersAcceptor( - $parametersAcceptor - ); - - if ($expectedParameterReflections === $parametersAcceptor->getParameters()) { - return null; + $paramType = new NullableType($paramType); + } } - return array_keys($expectedParameterReflections); + return $hasChanged ? $node : null; } } diff --git a/rules/Privatization/Guard/LaravelModelGuard.php b/rules/Privatization/Guard/LaravelModelGuard.php index e9e0a6b8895..97a543a82ce 100644 --- a/rules/Privatization/Guard/LaravelModelGuard.php +++ b/rules/Privatization/Guard/LaravelModelGuard.php @@ -44,9 +44,10 @@ public function isProtectedMethod(ClassReflection $classReflection, ClassMethod } $name = (string) $this->nodeNameResolver->getName($classMethod->name); - - return $this->isAttributeMethod($name, $classMethod) - || $this->isScopeMethod($name, $classMethod); + if ($this->isAttributeMethod($name, $classMethod)) { + return true; + } + return $this->isScopeMethod($name, $classMethod); } private function isAttributeMethod(string $name, ClassMethod $classMethod): bool diff --git a/tests/Issues/InfiniteSwapParams/Fixture/fixture.php.inc b/tests/Issues/InfiniteSwapParams/Fixture/fixture.php.inc index 05e625a7213..ab69a721d80 100644 --- a/tests/Issues/InfiniteSwapParams/Fixture/fixture.php.inc +++ b/tests/Issues/InfiniteSwapParams/Fixture/fixture.php.inc @@ -20,8 +20,8 @@ namespace Rector\Tests\Issues\InfiniteSwapParams\Fixture; class Fixture { - function foo($b, $a = 1) { return 3; } - function bar() { return self::foo(2, 1); } + function foo($a = 1, $b = null) { return 3; } + function bar() { return self::foo(1, 2); } } ?> From 590a5220fd09c877dc22b8ae00d11c6b0c642ac8 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 6 Sep 2025 17:20:33 +0700 Subject: [PATCH 2/6] remove unused classes --- .../Reflection/VendorLocationDetector.php | 43 ------------------- rules/Php80/NodeResolver/ArgumentSorter.php | 35 --------------- .../RequireOptionalParamResolver.php | 31 ------------- 3 files changed, 109 deletions(-) delete mode 100644 rules/CodingStyle/Reflection/VendorLocationDetector.php delete mode 100644 rules/Php80/NodeResolver/ArgumentSorter.php delete mode 100644 rules/Php80/NodeResolver/RequireOptionalParamResolver.php diff --git a/rules/CodingStyle/Reflection/VendorLocationDetector.php b/rules/CodingStyle/Reflection/VendorLocationDetector.php deleted file mode 100644 index cc0b665cadb..00000000000 --- a/rules/CodingStyle/Reflection/VendorLocationDetector.php +++ /dev/null @@ -1,43 +0,0 @@ -getDeclaringClass(); - $fileName = $declaringClassReflection->getFileName(); - - return $this->detect($fileName); - } - - public function detectFunctionReflection(FunctionReflection $functionReflection): bool - { - $fileName = $functionReflection->getFileName(); - - return $this->detect($fileName); - } - - private function detect(?string $fileName = null): bool - { - // probably internal - if ($fileName === null) { - return false; - } - - $normalizedFileName = $this->filePathHelper->normalizePathAndSchema($fileName); - return str_contains($normalizedFileName, '/vendor/'); - } -} diff --git a/rules/Php80/NodeResolver/ArgumentSorter.php b/rules/Php80/NodeResolver/ArgumentSorter.php deleted file mode 100644 index 05e7dbd41cb..00000000000 --- a/rules/Php80/NodeResolver/ArgumentSorter.php +++ /dev/null @@ -1,35 +0,0 @@ - $oldToNewPositions - * @param T[] $argOrParams - * @return T[] - */ - public function sortArgsByExpectedParamOrder(array $argOrParams, array $oldToNewPositions): array - { - $newArgsOrParams = []; - - foreach (array_keys($argOrParams) as $position) { - assert(is_int($position)); - - $newPosition = $oldToNewPositions[$position] ?? null; - if ($newPosition === null) { - continue; - } - - $newArgsOrParams[$position] = $argOrParams[$newPosition]; - } - - return $newArgsOrParams; - } -} diff --git a/rules/Php80/NodeResolver/RequireOptionalParamResolver.php b/rules/Php80/NodeResolver/RequireOptionalParamResolver.php deleted file mode 100644 index 93ea907d4c3..00000000000 --- a/rules/Php80/NodeResolver/RequireOptionalParamResolver.php +++ /dev/null @@ -1,31 +0,0 @@ -getParameters() as $position => $parameterReflection) { - if (! $parameterReflection->getDefaultValue() instanceof Type && ! $parameterReflection->isVariadic()) { - $requireParams[$position] = $parameterReflection; - } else { - $optionalParams[$position] = $parameterReflection; - } - } - - return $requireParams + $optionalParams; - } -} From 772833c077ceec77660034532e39630c72d670ca Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Sat, 6 Sep 2025 10:20:58 +0000 Subject: [PATCH 3/6] [ci-review] Rector Rectify --- rules/Privatization/Guard/LaravelModelGuard.php | 1 + 1 file changed, 1 insertion(+) diff --git a/rules/Privatization/Guard/LaravelModelGuard.php b/rules/Privatization/Guard/LaravelModelGuard.php index 97a543a82ce..e51610dc48a 100644 --- a/rules/Privatization/Guard/LaravelModelGuard.php +++ b/rules/Privatization/Guard/LaravelModelGuard.php @@ -47,6 +47,7 @@ public function isProtectedMethod(ClassReflection $classReflection, ClassMethod if ($this->isAttributeMethod($name, $classMethod)) { return true; } + return $this->isScopeMethod($name, $classMethod); } From 1346ece5a80319811eee0f6b1b959804ca376fbe Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 6 Sep 2025 18:27:14 +0700 Subject: [PATCH 4/6] fix phpstan --- .../OptionalParametersAfterRequiredRector.php | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php b/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php index 0cdc6d692f2..95bb2f061d3 100644 --- a/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php +++ b/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php @@ -5,8 +5,11 @@ namespace Rector\CodeQuality\Rector\ClassMethod; use PhpParser\Node; +use PhpParser\Node\ComplexType; use PhpParser\Node\Expr; use PhpParser\Node\Expr\ConstFetch; +use PhpParser\Node\Identifier; +use PhpParser\Node\IntersectionType; use PhpParser\Node\Name; use PhpParser\Node\NullableType; use PhpParser\Node\Param; @@ -77,7 +80,7 @@ public function refactor(Node $node): ClassMethod|Function_|null $previousParam = $node->params[$key - 1] ?? null; if ($previousParam instanceof Param && $previousParam->default instanceof Expr) { - $hasChanged = false; + $hasChanged = true; $param->default = new ConstFetch(new Name('null')); $paramType = $param->type; @@ -90,10 +93,18 @@ public function refactor(Node $node): ClassMethod|Function_|null continue; } - if ($paramType instanceof UnionType) { - $paramType->types[] = new ConstFetch(new Name('null')); - $paramType->types = array_unique($paramType->types, SORT_REGULAR); + if ($paramType instanceof UnionType || $paramType instanceof IntersectionType) { + foreach ($paramType->types as $unionedType) { + if ($unionedType instanceof Identifier && $this->isName($unionedType, 'null')) { + continue 2; + } + } + $paramType->types[] = new Identifier('null'); + continue; + } + + if ($paramType instanceof ComplexType) { continue; } From dd8cca8e6d05571b0aa66060775b410be134ee03 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 6 Sep 2025 18:33:24 +0700 Subject: [PATCH 5/6] add param typed --- .../Fixture/typed_params.php.inc | 41 +++++++++++++++++++ .../OptionalParametersAfterRequiredRector.php | 2 +- 2 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/typed_params.php.inc diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/typed_params.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/typed_params.php.inc new file mode 100644 index 00000000000..c8f5ecaf3cd --- /dev/null +++ b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/typed_params.php.inc @@ -0,0 +1,41 @@ + +----- + diff --git a/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php b/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php index 95bb2f061d3..245310646aa 100644 --- a/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php +++ b/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php @@ -108,7 +108,7 @@ public function refactor(Node $node): ClassMethod|Function_|null continue; } - $paramType = new NullableType($paramType); + $param->type = new NullableType($paramType); } } From 7eb8a4e90d8116c89c5681126240b6ae39646f0d Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 6 Sep 2025 18:36:41 +0700 Subject: [PATCH 6/6] add min php version nullable type for use of it --- .../OptionalParametersAfterRequiredRector.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php b/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php index 245310646aa..74706884090 100644 --- a/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php +++ b/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php @@ -17,13 +17,15 @@ use PhpParser\Node\Stmt\Function_; use PhpParser\Node\UnionType; use Rector\Rector\AbstractRector; +use Rector\ValueObject\PhpVersionFeature; +use Rector\VersionBonding\Contract\MinPhpVersionInterface; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; /** * @see \Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterRequiredRector\OptionalParametersAfterRequiredRectorTest */ -final class OptionalParametersAfterRequiredRector extends AbstractRector +final class OptionalParametersAfterRequiredRector extends AbstractRector implements MinPhpVersionInterface { public function getRuleDefinition(): RuleDefinition { @@ -114,4 +116,9 @@ public function refactor(Node $node): ClassMethod|Function_|null return $hasChanged ? $node : null; } + + public function provideMinPhpVersion(): int + { + return PhpVersionFeature::NULLABLE_TYPE; + } }