From 0b3604d12fffd23fd180c14ebf36dc4f0675978a Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 8 Jun 2023 16:47:20 +0700 Subject: [PATCH 1/6] [CodeQuality] Handle return new object and no return on ConsecutiveNullCompareReturnsToNullCoalesceQueueRector --- .../Fixture/return_new_object.php.inc | 39 +++++++++++++++++++ .../Fixture/skip_no_return.php.inc | 20 ++++++++++ 2 files changed, 59 insertions(+) create mode 100644 rules-tests/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector/Fixture/return_new_object.php.inc create mode 100644 rules-tests/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector/Fixture/skip_no_return.php.inc diff --git a/rules-tests/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector/Fixture/return_new_object.php.inc b/rules-tests/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector/Fixture/return_new_object.php.inc new file mode 100644 index 00000000000..70750c257c8 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector/Fixture/return_new_object.php.inc @@ -0,0 +1,39 @@ +prop1 !== null) { + return $this->prop1; + } + if ($this->prop2 !== null) { + return $this->prop2; + } + return new \stdClass(); + } +} + +?> +----- +prop1 ?? $this->prop2) ?? throw new \stdClass(); + } +} + +?> diff --git a/rules-tests/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector/Fixture/skip_no_return.php.inc b/rules-tests/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector/Fixture/skip_no_return.php.inc new file mode 100644 index 00000000000..81d4d4a0dcf --- /dev/null +++ b/rules-tests/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector/Fixture/skip_no_return.php.inc @@ -0,0 +1,20 @@ +prop1 !== null) { + return $this->prop1; + } + if ($this->prop2 !== null) { + return $this->prop2; + } + echo 'hi'; + } +} From d94a54d336651ba926ea6ecc29e6c8226dfef6a4 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 8 Jun 2023 16:47:46 +0700 Subject: [PATCH 2/6] Fixed :tada: --- ...ompareReturnsToNullCoalesceQueueRector.php | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/rules/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector.php b/rules/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector.php index 9c279cc2859..481d10b9c90 100644 --- a/rules/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector.php +++ b/rules/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector.php @@ -105,8 +105,9 @@ public function refactor(Node $node): ?Node } // remove last return null - $throwExpr = null; + $appendExpr = null; $hasChanged = false; + $originalStmts = $node->stmts; foreach ($node->stmts as $key => $stmt) { if (in_array($key, $ifKeys, true)) { unset($node->stmts[$key]); @@ -121,19 +122,27 @@ public function refactor(Node $node): ?Node if ($stmt instanceof Throw_) { unset($node->stmts[$key]); - $throwExpr = new ExprThrow_($stmt->expr); + $appendExpr = new ExprThrow_($stmt->expr); continue; } if (! $this->isReturnNull($stmt)) { - continue; + if ($stmt instanceof Return_ && $stmt->expr instanceof Expr) { + unset($node->stmts[$key]); + $appendExpr = new ExprThrow_($stmt->expr); + + continue; + } + + $node->stmts = $originalStmts; + return $node; } unset($node->stmts[$key]); } - $node->stmts[] = $this->createCealesceReturn($coalescingExprs, $throwExpr); + $node->stmts[] = $this->createCealesceReturn($coalescingExprs, $appendExpr); return $node; } @@ -159,7 +168,7 @@ private function isReturnNull(Stmt $stmt): bool /** * @param Expr[] $coalescingExprs */ - private function createCealesceReturn(array $coalescingExprs, ?Expr $throwExpr): Return_ + private function createCealesceReturn(array $coalescingExprs, ?Expr $appendExpr): Return_ { /** @var Expr $leftExpr */ $leftExpr = array_shift($coalescingExprs); @@ -173,8 +182,8 @@ private function createCealesceReturn(array $coalescingExprs, ?Expr $throwExpr): $coalesce = new Coalesce($coalesce, $coalescingExpr); } - if ($throwExpr instanceof Expr) { - return new Return_(new Coalesce($coalesce, $throwExpr)); + if ($appendExpr instanceof Expr) { + return new Return_(new Coalesce($coalesce, $appendExpr)); } return new Return_($coalesce); From 5d96442cdeb24f1a9746a7ba1f82bef87f7bd54a Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 8 Jun 2023 16:48:37 +0700 Subject: [PATCH 3/6] fixture update --- .../Fixture/return_new_object.php.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules-tests/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector/Fixture/return_new_object.php.inc b/rules-tests/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector/Fixture/return_new_object.php.inc index 70750c257c8..9376eea4cda 100644 --- a/rules-tests/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector/Fixture/return_new_object.php.inc +++ b/rules-tests/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector/Fixture/return_new_object.php.inc @@ -32,7 +32,7 @@ class ReturnNewObject public function getProp(): \stdClass { - return ($this->prop1 ?? $this->prop2) ?? throw new \stdClass(); + return ($this->prop1 ?? $this->prop2) ?? new \stdClass(); } } From f3f67d8e3b6ef0e41c111a31bcfee98047ca5465 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 8 Jun 2023 16:49:03 +0700 Subject: [PATCH 4/6] fix --- .../ConsecutiveNullCompareReturnsToNullCoalesceQueueRector.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector.php b/rules/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector.php index 481d10b9c90..d89ef9b7ace 100644 --- a/rules/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector.php +++ b/rules/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector.php @@ -130,7 +130,7 @@ public function refactor(Node $node): ?Node if (! $this->isReturnNull($stmt)) { if ($stmt instanceof Return_ && $stmt->expr instanceof Expr) { unset($node->stmts[$key]); - $appendExpr = new ExprThrow_($stmt->expr); + $appendExpr = $stmt->expr; continue; } From fe02d7d9e62102cd193d0d29edb2fac8ffbcb821 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 8 Jun 2023 16:57:10 +0700 Subject: [PATCH 5/6] Final touch: no return after if handling --- .../Fixture/skip_no_return2.php.inc | 19 +++++++++++++++++++ ...ompareReturnsToNullCoalesceQueueRector.php | 4 ++++ 2 files changed, 23 insertions(+) create mode 100644 rules-tests/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector/Fixture/skip_no_return2.php.inc diff --git a/rules-tests/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector/Fixture/skip_no_return2.php.inc b/rules-tests/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector/Fixture/skip_no_return2.php.inc new file mode 100644 index 00000000000..ed132bdd5c8 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector/Fixture/skip_no_return2.php.inc @@ -0,0 +1,19 @@ +prop1 !== null) { + return $this->prop1; + } + if ($this->prop2 !== null) { + return $this->prop2; + } + } +} diff --git a/rules/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector.php b/rules/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector.php index d89ef9b7ace..61ab25c3bfb 100644 --- a/rules/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector.php +++ b/rules/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector.php @@ -95,6 +95,10 @@ public function refactor(Node $node): ?Node continue; } + if (! isset($node->stmts[$key + 1])) { + return null; + } + $coalescingExprs[] = $comparedExpr; $ifKeys[] = $key; } From 956b55e439df62a6b6a7bae47569a72c38872c24 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 8 Jun 2023 17:01:03 +0700 Subject: [PATCH 6/6] final touch: add fixture for indirect return --- .../Fixture/skip_indirect_return.php.inc | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 rules-tests/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector/Fixture/skip_indirect_return.php.inc diff --git a/rules-tests/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector/Fixture/skip_indirect_return.php.inc b/rules-tests/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector/Fixture/skip_indirect_return.php.inc new file mode 100644 index 00000000000..07e47b4cb05 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/If_/ConsecutiveNullCompareReturnsToNullCoalesceQueueRector/Fixture/skip_indirect_return.php.inc @@ -0,0 +1,21 @@ +prop1 !== null) { + return $this->prop1; + } + if ($this->prop2 !== null) { + return $this->prop2; + } + echo 'hi'; + return null; + } +}