skip next array dim fetch

rectorphp · Jul 3, 2022 · eb28afb · eb28afb
1 parent 137da55
commit eb28afb
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 5 deletions.
diff --git a/...r/StmtsAwareInterface/RemoveJustVariableAssignRector/Fixture/skip_array_dim_fetch.php.inc b/...r/StmtsAwareInterface/RemoveJustVariableAssignRector/Fixture/skip_array_dim_fetch.php.inc
@@ -0,0 +1,15 @@
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\StmtsAwareInterface\RemoveJustVariableAssignRector\Fixture;
+
+final class SkipArrayDimFetch
+{
+    private int $temporaryValue;
+
+    public function run()
+    {
+        $result = 100;
+
+        $this->temporaryValue[] = $result;
+    }
+}
diff --git a/rules/DeadCode/Rector/StmtsAwareInterface/RemoveJustVariableAssignRector.php b/rules/DeadCode/Rector/StmtsAwareInterface/RemoveJustVariableAssignRector.php
@@ -5,7 +5,9 @@
 namespace Rector\DeadCode\Rector\StmtsAwareInterface;
 
 use PhpParser\Node;
+use PhpParser\Node\Expr\ArrayDimFetch;
 use PhpParser\Node\Expr\Assign;
+use PhpParser\Node\Expr\BinaryOp\Concat;
 use PhpParser\Node\Expr\Ternary;
 use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\Stmt\Expression;
@@ -21,7 +23,7 @@
 final class RemoveJustVariableAssignRector extends AbstractRector
 {
     public function __construct(
-        private VariableAnalyzer $variableAnalyzer
+        private readonly VariableAnalyzer $variableAnalyzer
     ) {
     }
 
@@ -88,10 +90,6 @@ public function refactor(Node $node): ?Node
 
             $currentAssign = $stmt->expr;
 
-            // too complex to shorten
-            if ($currentAssign->expr instanceof Ternary) {
-                continue;
-            }
 
             if (! $nextStmt instanceof Expression) {
                 continue;
@@ -103,6 +101,10 @@ public function refactor(Node $node): ?Node
 
             $nextAssign = $nextStmt->expr;
 
+            if ($this->areTooComplexAssignsToShorten($currentAssign, $nextAssign)) {
+                continue;
+            }
+
             if (! $this->areTwoVariablesCrossAssign($currentAssign, $nextAssign)) {
                 continue;
             }
@@ -148,4 +150,20 @@ private function areTwoVariablesCrossAssign(Assign $currentAssign, Assign $nextA
 
         return ! $this->variableAnalyzer->isUsedByReference($nextAssign->expr);
     }
+
+    /**
+     * Shortening should not make code less readable.
+     */
+    private function areTooComplexAssignsToShorten(Assign $currentAssign, Assign $nextAssign): bool
+    {
+        if ($currentAssign->expr instanceof Ternary) {
+            return true;
+        }
+
+        if ($currentAssign->expr instanceof Concat) {
+            return true;
+        }
+
+        return $nextAssign->var instanceof ArrayDimFetch;
+    }
 }