From 4b40665cf97997631926f4e7c8b2977202fa1394 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Wed, 28 May 2025 06:57:01 +0700 Subject: [PATCH 1/4] [DowngradePhp80] Handle multiple coalesce before throw on DowngradeThrowExprRector --- .../Fixture/multiple_coalesce.php.inc | 30 +++++++++++++++++++ .../Expression/DowngradeThrowExprRector.php | 21 +++++++++++++ src/NodeAnalyzer/CoalesceAnalyzer.php | 4 +++ 3 files changed, 55 insertions(+) create mode 100644 rules-tests/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector/Fixture/multiple_coalesce.php.inc diff --git a/rules-tests/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector/Fixture/multiple_coalesce.php.inc b/rules-tests/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector/Fixture/multiple_coalesce.php.inc new file mode 100644 index 00000000..fe517fb1 --- /dev/null +++ b/rules-tests/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector/Fixture/multiple_coalesce.php.inc @@ -0,0 +1,30 @@ +getInternalId() ?? $content->getId() ?? throw new \LogicException('content id is required for ackVersion'); + } +} + +?> +----- +getInternalId() ?? $content->getId()) === null) { + throw new \LogicException('content id is required for ackVersion'); + } + $contentId = $content->getInternalId() ?? $content->getId(); + } +} + +?> diff --git a/rules/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector.php b/rules/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector.php index 57aad9e9..da2183cf 100644 --- a/rules/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector.php +++ b/rules/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector.php @@ -180,9 +180,30 @@ private function refactorTernary(Ternary $ternary, ?Assign $assign): If_|null|ar private function refactorCoalesce(Coalesce $coalesce, ?Assign $assign): If_|null|array { if (! $coalesce->right instanceof Throw_) { + $rightCoalesce = $coalesce->right; + $leftCoalesce = $coalesce->left; + + while ($rightCoalesce instanceof Coalesce) { + $leftCoalesce = new Coalesce($leftCoalesce, $rightCoalesce->left); + $rightCoalesce = $rightCoalesce->right; + } + + if ($rightCoalesce instanceof Throw_) { + $coalesce = new Coalesce($leftCoalesce, $rightCoalesce); + return $this->processCoalesce($coalesce, $assign); + } + return null; } + return $this->processCoalesce($coalesce, $assign); + } + + /** + * @return If_|Stmt[]|null + */ + private function processCoalesce(Coalesce $coalesce, ?Assign $assign): If_|null|array + { if (! $this->coalesceAnalyzer->hasIssetableLeft($coalesce)) { return null; } diff --git a/src/NodeAnalyzer/CoalesceAnalyzer.php b/src/NodeAnalyzer/CoalesceAnalyzer.php index 1e5d1eb2..f29b8982 100644 --- a/src/NodeAnalyzer/CoalesceAnalyzer.php +++ b/src/NodeAnalyzer/CoalesceAnalyzer.php @@ -25,6 +25,10 @@ final class CoalesceAnalyzer public function hasIssetableLeft(Coalesce $coalesce): bool { + if ($coalesce->left instanceof Coalesce) { + return true; + } + $leftClass = $coalesce->left::class; return in_array($leftClass, self::ISSETABLE_EXPR, true); } From 408b83916ccc5816afafb9d4b95d7866e59083a5 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Wed, 28 May 2025 07:00:35 +0700 Subject: [PATCH 2/4] direct assign --- .../Fixture/multiple_coalesce.php.inc | 3 +-- .../Rector/Expression/DowngradeThrowExprRector.php | 9 +++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/rules-tests/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector/Fixture/multiple_coalesce.php.inc b/rules-tests/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector/Fixture/multiple_coalesce.php.inc index fe517fb1..5082e2e1 100644 --- a/rules-tests/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector/Fixture/multiple_coalesce.php.inc +++ b/rules-tests/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector/Fixture/multiple_coalesce.php.inc @@ -20,10 +20,9 @@ class MultipleCoalesce { public function run($content) { - if (($content->getInternalId() ?? $content->getId()) === null) { + if (($contentId = $content->getInternalId() ?? $content->getId()) === null) { throw new \LogicException('content id is required for ackVersion'); } - $contentId = $content->getInternalId() ?? $content->getId(); } } diff --git a/rules/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector.php b/rules/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector.php index da2183cf..97b53804 100644 --- a/rules/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector.php +++ b/rules/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector.php @@ -190,7 +190,7 @@ private function refactorCoalesce(Coalesce $coalesce, ?Assign $assign): If_|null if ($rightCoalesce instanceof Throw_) { $coalesce = new Coalesce($leftCoalesce, $rightCoalesce); - return $this->processCoalesce($coalesce, $assign); + return $this->processCoalesce($coalesce, $assign, true); } return null; @@ -202,7 +202,7 @@ private function refactorCoalesce(Coalesce $coalesce, ?Assign $assign): If_|null /** * @return If_|Stmt[]|null */ - private function processCoalesce(Coalesce $coalesce, ?Assign $assign): If_|null|array + private function processCoalesce(Coalesce $coalesce, ?Assign $assign, bool $innerAssign = false): If_|null|array { if (! $this->coalesceAnalyzer->hasIssetableLeft($coalesce)) { return null; @@ -219,6 +219,11 @@ private function processCoalesce(Coalesce $coalesce, ?Assign $assign): If_|null| $assign->expr = $coalesce->left; + if ($innerAssign && $if->cond instanceof Identical) { + $if->cond->left = new Assign($assign->var, $if->cond->left); + return $if; + } + return [$if, new Expression($assign)]; } From 1e523a6c4a4481078271cd8a70eee36d75e4e155 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Wed, 28 May 2025 07:04:30 +0700 Subject: [PATCH 3/4] Fix --- .../Fixture/multiple_coalesce.php.inc | 3 ++- .../Rector/Expression/DowngradeThrowExprRector.php | 10 ++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/rules-tests/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector/Fixture/multiple_coalesce.php.inc b/rules-tests/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector/Fixture/multiple_coalesce.php.inc index 5082e2e1..6a7bfdd5 100644 --- a/rules-tests/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector/Fixture/multiple_coalesce.php.inc +++ b/rules-tests/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector/Fixture/multiple_coalesce.php.inc @@ -20,7 +20,8 @@ class MultipleCoalesce { public function run($content) { - if (($contentId = $content->getInternalId() ?? $content->getId()) === null) { + $contentId = $content->getInternalId() ?? $content->getId(); + if ($contentId === null) { throw new \LogicException('content id is required for ackVersion'); } } diff --git a/rules/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector.php b/rules/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector.php index 97b53804..7719a15b 100644 --- a/rules/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector.php +++ b/rules/DowngradePhp80/Rector/Expression/DowngradeThrowExprRector.php @@ -202,7 +202,7 @@ private function refactorCoalesce(Coalesce $coalesce, ?Assign $assign): If_|null /** * @return If_|Stmt[]|null */ - private function processCoalesce(Coalesce $coalesce, ?Assign $assign, bool $innerAssign = false): If_|null|array + private function processCoalesce(Coalesce $coalesce, ?Assign $assign, bool $assignEarly = false): If_|null|array { if (! $this->coalesceAnalyzer->hasIssetableLeft($coalesce)) { return null; @@ -219,9 +219,11 @@ private function processCoalesce(Coalesce $coalesce, ?Assign $assign, bool $inne $assign->expr = $coalesce->left; - if ($innerAssign && $if->cond instanceof Identical) { - $if->cond->left = new Assign($assign->var, $if->cond->left); - return $if; + if ($assignEarly && $if->cond instanceof Identical) { + $expression = new Expression(new Assign($assign->var, $if->cond->left)); + $if->cond->left = $assign->var; + + return [$expression, $if]; } return [$if, new Expression($assign)]; From 305713b852b7ea75f14a864425f94fe99ba9edd3 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Wed, 28 May 2025 07:10:12 +0700 Subject: [PATCH 4/4] fix to use composer/install v2 --- .github/workflows/tests.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 268fafe4..1a0b76f5 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -25,6 +25,6 @@ jobs: coverage: none ini-values: zend.assertions=1 - - uses: "ramsey/composer-install@v1" + - uses: "ramsey/composer-install@v2" - run: vendor/bin/phpunit