Skip to content

Commit

Permalink
[Php80] Skip Assign on its var not directly used in next return on Ch…
Browse files Browse the repository at this point in the history
…angeSwitchToMatchRector (#2535)

* [Php80] Skip Assign on ArrayDimFetch used next return on ChangeSwitchToMatchRector

* Fixed 🎉

* final touch: compare next return expr

* final touch: clean up

* really final touch: rename fixture

* really final touch: rename fixture

* really final touch: rename method and vars
  • Loading branch information
samsonasik authored Jun 20, 2022
1 parent 4eeadae commit f4a9b50
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

declare(strict_types=1);

namespace Rector\Tests\Php80\Rector\Switch_\ChangeSwitchToMatchRector\Fixture;

final class SkipAssignOnItsVarNotDirectlyUsedInNextReturn
{
public const STATUS_A = 'a';
public const STATUS_B = 'b';
public const STATUS_C = 'c';
public const STATUS_D = 'd';

public function run(): \Closure
{
return static function (self $item) {
$class = [];
switch ($item->status) {
case self::STATUS_A:
$class[] = 'green';
break;

case self::STATUS_B:
case self::STATUS_C:
case self::STATUS_D:
$class[] = 'red';
break;
}

return execute($class);
};
}
}
27 changes: 18 additions & 9 deletions rules/Php80/Rector/Switch_/ChangeSwitchToMatchRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,9 @@ public function refactor(Node $node): ?Node
return $this->processReturn($match);
}

$assignExpr = $this->resolveAssignExpr($condAndExprs);
if ($assignExpr instanceof Expr) {
return $this->changeToAssign($node, $match, $assignExpr);
$assignVar = $this->resolveAssignVar($condAndExprs);
if ($assignVar instanceof Expr) {
return $this->changeToAssign($node, $match, $assignVar);
}

return $match;
Expand All @@ -142,17 +142,26 @@ private function processReturn(Match_ $match): ?Return_
return new Return_($match);
}

private function changeToAssign(Switch_ $switch, Match_ $match, Expr $assignExpr): Assign
private function changeToAssign(Switch_ $switch, Match_ $match, Expr $expr): ?Assign
{
$nextReturn = $switch->getAttribute(AttributeKey::NEXT_NODE);

if ($nextReturn instanceof Return_ && $nextReturn->expr instanceof Expr && ! $this->nodeComparator->areNodesEqual(
$expr,
$nextReturn->expr
)) {
return null;
}

$prevInitializedAssign = $this->betterNodeFinder->findFirstInlinedPrevious(
$switch,
fn (Node $node): bool => $node instanceof Assign && $this->nodeComparator->areNodesEqual(
$node->var,
$assignExpr
$expr
)
);

$assign = new Assign($assignExpr, $match);
$assign = new Assign($expr, $match);
if (! $prevInitializedAssign instanceof Assign) {
return $assign;
}
Expand All @@ -177,7 +186,7 @@ private function changeToAssign(Switch_ $switch, Match_ $match, Expr $assignExpr
/**
* @param CondAndExpr[] $condAndExprs
*/
private function resolveAssignExpr(array $condAndExprs): ?Expr
private function resolveAssignVar(array $condAndExprs): ?Expr
{
foreach ($condAndExprs as $condAndExpr) {
$expr = $condAndExpr->getExpr();
Expand Down Expand Up @@ -210,9 +219,9 @@ private function processImplicitReturnAfterSwitch(Switch_ $switch, Match_ $match
return $match;
}

$assignExpr = $this->resolveAssignExpr($condAndExprs);
$assignVar = $this->resolveAssignVar($condAndExprs);

if (! $assignExpr instanceof Expr) {
if (! $assignVar instanceof Expr) {
$this->removeNode($nextNode);
}

Expand Down

0 comments on commit f4a9b50

Please sign in to comment.