Skip to content

Commit

Permalink
[CodeQuality] Handle throw after if on ConsecutiveNullCompareReturnsT…
Browse files Browse the repository at this point in the history
…oNullCoalesceQueueRector (#4107)

* [CodeQuality] Handle throw after if on ConsecutiveNullCompareReturnsToNullCoalesceQueueRector

* Fixed 🎉

* skip instead

* clean up

* Use throw expr

* [ci-review] Rector Rectify

* ensure has index removed before removal

* Fix phsptan

* [ci-review] Rector Rectify

---------

Co-authored-by: GitHub Action <actions@github.com>
  • Loading branch information
samsonasik and actions-user authored Jun 7, 2023
1 parent 71dc336 commit 63816c0
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\If_\ConsecutiveNullCompareReturnsToNullCoalesceQueueRector\Fixture;

final class ThrowAfterIf
{
protected ?\stdClass $prop1 = null;
protected ?\stdClass $prop2 = null;

public function getProp(): \stdClass
{
if ($this->prop1 !== null) {
return $this->prop1;
}
if ($this->prop2 !== null) {
return $this->prop2;
}
throw new \RuntimeException('No prop is set');
}
}

?>
-----
<?php

namespace Rector\Tests\CodeQuality\Rector\If_\ConsecutiveNullCompareReturnsToNullCoalesceQueueRector\Fixture;

final class ThrowAfterIf
{
protected ?\stdClass $prop1 = null;
protected ?\stdClass $prop2 = null;

public function getProp(): \stdClass
{
return ($this->prop1 ?? $this->prop2) ?? throw new \RuntimeException('No prop is set');
}
}

?>
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
use PhpParser\Node;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\BinaryOp\Coalesce;
use PhpParser\Node\Expr\Throw_ as ExprThrow_;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\If_;
use PhpParser\Node\Stmt\Return_;
use PhpParser\Node\Stmt\Throw_;
use Rector\Core\Contract\PhpParser\Node\StmtsAwareInterface;
use Rector\Core\NodeManipulator\IfManipulator;
use Rector\Core\Rector\AbstractRector;
Expand Down Expand Up @@ -103,9 +105,24 @@ public function refactor(Node $node): ?Node
}

// remove last return null
$throwExpr = null;
$hasChanged = false;
foreach ($node->stmts as $key => $stmt) {
if (in_array($key, $ifKeys, true)) {
unset($node->stmts[$key]);
$hasChanged = true;

continue;
}

if (! $hasChanged) {
continue;
}

if ($stmt instanceof Throw_) {
unset($node->stmts[$key]);
$throwExpr = new ExprThrow_($stmt->expr);

continue;
}

Expand All @@ -116,7 +133,7 @@ public function refactor(Node $node): ?Node
unset($node->stmts[$key]);
}

$node->stmts[] = $this->createCealesceReturn($coalescingExprs);
$node->stmts[] = $this->createCealesceReturn($coalescingExprs, $throwExpr);

return $node;
}
Expand All @@ -142,7 +159,7 @@ private function isReturnNull(Stmt $stmt): bool
/**
* @param Expr[] $coalescingExprs
*/
private function createCealesceReturn(array $coalescingExprs): Return_
private function createCealesceReturn(array $coalescingExprs, ?Expr $throwExpr): Return_
{
/** @var Expr $leftExpr */
$leftExpr = array_shift($coalescingExprs);
Expand All @@ -156,6 +173,10 @@ private function createCealesceReturn(array $coalescingExprs): Return_
$coalesce = new Coalesce($coalesce, $coalescingExpr);
}

if ($throwExpr instanceof Expr) {
return new Return_(new Coalesce($coalesce, $throwExpr));
}

return new Return_($coalesce);
}
}

0 comments on commit 63816c0

Please sign in to comment.