From 76a15e03c5dab3178e88d352f8856c4a07c7d001 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sun, 7 Sep 2025 14:23:57 +0700 Subject: [PATCH 1/4] [CodeQuality] Fill reasonable default value on OptionalParametersAfterRequiredRector --- .../Fixture/typed_params.php.inc | 12 +- .../OptionalParametersAfterRequiredRector.php | 116 +++++++++++++----- 2 files changed, 98 insertions(+), 30 deletions(-) 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 index 244ec16a5a9..88712903515 100644 --- a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/typed_params.php.inc +++ b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/typed_params.php.inc @@ -19,6 +19,10 @@ class TypedParams function run4($optional = 1, A&B $required) { } + + function run5($optional = 1, array $required) + { + } } ?> @@ -29,11 +33,11 @@ namespace Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterReq class TypedParams { - function run1($optional = 1, ?int $required = null) + function run1($optional = 1, int $required = 0) { } - function run2($optional = 1, string|int|null $required = null) + function run2($optional = 1, string|int $required = '') { } @@ -44,6 +48,10 @@ class TypedParams function run4($optional = 1, (A&B)|null $required = null) { } + + function run5($optional = 1, array $required = []) + { + } } ?> diff --git a/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php b/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php index 5dc2fb3624e..f20654fef30 100644 --- a/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php +++ b/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php @@ -14,9 +14,13 @@ use PhpParser\Node\Name; use PhpParser\Node\NullableType; use PhpParser\Node\Param; +use PhpParser\Node\Scalar\Float_; +use PhpParser\Node\Scalar\Int_; +use PhpParser\Node\Scalar\String_; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Function_; use PhpParser\Node\UnionType; +use Rector\PhpParser\Node\Value\ValueResolver; use Rector\Rector\AbstractRector; use Rector\ValueObject\PhpVersionFeature; use Rector\VersionBonding\Contract\MinPhpVersionInterface; @@ -28,6 +32,11 @@ */ final class OptionalParametersAfterRequiredRector extends AbstractRector implements MinPhpVersionInterface { + public function __construct( + private readonly ValueResolver $valueResolver, + ) { + } + public function getRuleDefinition(): RuleDefinition { return new RuleDefinition('Add null default value when a required parameter follows an optional one', [ @@ -84,47 +93,98 @@ public function refactor(Node $node): ClassMethod|Function_|Closure|null $previousParam = $node->params[$key - 1] ?? null; if ($previousParam instanceof Param && $previousParam->default instanceof Expr) { $hasChanged = true; + $this->processParam($param); + } + } - $param->default = new ConstFetch(new Name('null')); + return $hasChanged ? $node : null; + } - if (! $param->type instanceof Node) { - continue; - } + public function provideMinPhpVersion(): int + { + return PhpVersionFeature::NULLABLE_TYPE; + } - if ($param->type instanceof NullableType) { - continue; - } + /** + * Look first found type reasonable value + * + * @param Node[] $types + */ + private function mapReasonableParamValue(array $types): Expr + { + foreach ($types as $type) { + if ($this->isName($type, 'string')) { + return new String_(''); + } - if ($param->type instanceof UnionType) { - foreach ($param->type->types as $unionedType) { - if ($unionedType instanceof Identifier && $this->isName($unionedType, 'null')) { - continue 2; - } - } + if ($this->isName($type, 'int')) { + return new Int_(0); + } - $param->type->types[] = new Identifier('null'); - continue; - } + if ($this->isName($type, 'float')) { + return new Float_(0.0); + } - if ($param->type instanceof IntersectionType) { - $param->type = new UnionType([$param->type, new Identifier('null')]); + if ($this->isName($type, 'bool')) { + return $this->nodeFactory->createFalse(); + } - continue; - } + if ($this->isName($type, 'array')) { + return $this->nodeFactory->createArray([]); + } + } + + return new ConstFetch(new Name('null')); + } + + private function processParam(Param $param): void + { + if (! $param->type instanceof Node) { + $param->default = new ConstFetch(new Name('null')); + return; + } + + if ($param->type instanceof NullableType) { + $param->default = new ConstFetch(new Name('null')); + return; + } + + if ($param->type instanceof IntersectionType) { + $param->default = new ConstFetch(new Name('null')); + $param->type = new UnionType([$param->type, new Identifier('null')]); + return; + } - if ($param->type instanceof ComplexType) { - continue; + if ($param->type instanceof UnionType) { + foreach ($param->type->types as $unionedType) { + if ($unionedType instanceof Identifier && $this->isName($unionedType, 'null')) { + $param->default = new ConstFetch(new Name('null')); + return; } + } - $param->type = new NullableType($param->type); + $reasonableValue = $this->mapReasonableParamValue($param->type->types); + if ($this->valueResolver->isNull($reasonableValue)) { + $param->default = new ConstFetch(new Name('null')); + $param->type->types[] = new Identifier('null'); + return; } + + $param->default = $reasonableValue; + return; } - return $hasChanged ? $node : null; - } + if ($param->type instanceof ComplexType) { + return; + } - public function provideMinPhpVersion(): int - { - return PhpVersionFeature::NULLABLE_TYPE; + $reasonableValue = $this->mapReasonableParamValue([$param->type]); + if ($this->valueResolver->isNull($reasonableValue)) { + $param->type = new NullableType($param->type); + $param->default = new ConstFetch(new Name('null')); + return; + } + + $param->default = $reasonableValue; } } From d835c9c8eb6c5def2ec10d28e38c2676c8a852c8 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sun, 7 Sep 2025 14:24:47 +0700 Subject: [PATCH 2/4] [CodeQuality] Fill reasonable default value on OptionalParametersAfterRequiredRector --- .../ClassMethod/OptionalParametersAfterRequiredRector.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php b/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php index f20654fef30..8ae61312362 100644 --- a/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php +++ b/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php @@ -39,12 +39,12 @@ public function __construct( public function getRuleDefinition(): RuleDefinition { - return new RuleDefinition('Add null default value when a required parameter follows an optional one', [ + return new RuleDefinition('Add reasonable default value when a required parameter follows an optional one', [ new CodeSample( <<<'CODE_SAMPLE' class SomeObject { - public function run($optional = 1, $required) + public function run($optional = 1, int $required) { } } @@ -54,7 +54,7 @@ public function run($optional = 1, $required) <<<'CODE_SAMPLE' class SomeObject { - public function run($optional = 1, $required = null) + public function run($optional = 1, int $required = 0) { } } From 307779ede46041184a05bd107df4e5c7c642bcce Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sun, 7 Sep 2025 14:34:48 +0700 Subject: [PATCH 3/4] Fix independent true, false, null param --- .../Fixture/typed_params.php.inc | 24 +++++++++++++++++++ .../OptionalParametersAfterRequiredRector.php | 13 +++++++++- 2 files changed, 36 insertions(+), 1 deletion(-) 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 index 88712903515..fd9cc17c604 100644 --- a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/typed_params.php.inc +++ b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/typed_params.php.inc @@ -23,6 +23,18 @@ class TypedParams function run5($optional = 1, array $required) { } + + function run6($optional = 1, true $required) + { + } + + function run7($optional = 1, false $required) + { + } + + function run8($optional = 1, null $required) + { + } } ?> @@ -52,6 +64,18 @@ class TypedParams function run5($optional = 1, array $required = []) { } + + function run6($optional = 1, true $required = true) + { + } + + function run7($optional = 1, false $required = false) + { + } + + function run8($optional = 1, null $required = null) + { + } } ?> diff --git a/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php b/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php index 8ae61312362..7c969be2526 100644 --- a/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php +++ b/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php @@ -132,6 +132,14 @@ private function mapReasonableParamValue(array $types): Expr if ($this->isName($type, 'array')) { return $this->nodeFactory->createArray([]); } + + if ($this->isName($type, 'true')) { + return $this->nodeFactory->createTrue(); + } + + if ($this->isName($type, 'false')) { + return $this->nodeFactory->createFalse(); + } } return new ConstFetch(new Name('null')); @@ -180,7 +188,10 @@ private function processParam(Param $param): void $reasonableValue = $this->mapReasonableParamValue([$param->type]); if ($this->valueResolver->isNull($reasonableValue)) { - $param->type = new NullableType($param->type); + if (! $param->type instanceof Identifier || ! $this->isName($param->type, 'null')) { + $param->type = new NullableType($param->type); + } + $param->default = new ConstFetch(new Name('null')); return; } From 8c576796a75457b844a24717941fc02ee55b30bf Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sun, 7 Sep 2025 14:38:10 +0700 Subject: [PATCH 4/4] Fix mixed nullable --- .../Fixture/typed_params.php.inc | 8 ++++++++ .../ClassMethod/OptionalParametersAfterRequiredRector.php | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) 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 index fd9cc17c604..719bb40be34 100644 --- a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/typed_params.php.inc +++ b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/typed_params.php.inc @@ -35,6 +35,10 @@ class TypedParams function run8($optional = 1, null $required) { } + + function run9($optional = 1, mixed $required) + { + } } ?> @@ -76,6 +80,10 @@ class TypedParams function run8($optional = 1, null $required = null) { } + + function run9($optional = 1, mixed $required = null) + { + } } ?> diff --git a/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php b/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php index 7c969be2526..b9be5cbff33 100644 --- a/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php +++ b/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php @@ -188,7 +188,7 @@ private function processParam(Param $param): void $reasonableValue = $this->mapReasonableParamValue([$param->type]); if ($this->valueResolver->isNull($reasonableValue)) { - if (! $param->type instanceof Identifier || ! $this->isName($param->type, 'null')) { + if (! $param->type instanceof Identifier || ! $this->isNames($param->type, ['null', 'mixed'])) { $param->type = new NullableType($param->type); }