RemoveUnusedNonEmptyArrayBeforeForeachRector: skip array dim fetch (#…

…5166) * RemoveUnusedNonEmptyArrayBeforeForeachRector: skip array dim fetch * fix with known offset * test phpdoc array shape * refactor * simplify test
rectorphp · Oct 14, 2023 · da2dd66 · da2dd66
1 parent c639ef8
commit da2dd66
Show file tree

Hide file tree

Showing 4 changed files with 134 additions and 10 deletions.
diff --git a/.../If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/offset_exists_dim_fetch.php.inc b/.../If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/offset_exists_dim_fetch.php.inc
@@ -0,0 +1,59 @@
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\If_\RemoveUnusedNonEmptyArrayBeforeForeachRector\Fixture;
+
+class KnownOffset
+{
+    public const DEFAULT_GROUP = 'default';
+
+    /**
+     * @var array<string, list<self>>
+     */
+    private static $groups = [];
+
+    public static function knownOffset(): array
+    {
+        $group = 'default';
+
+        self::$groups[$group] = ["foo"];
+
+        if (! empty(self::$groups[$group])) {
+            foreach (self::$groups[$group] as $group) {
+                echo "hello";
+            }
+        }
+
+        return [];
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\If_\RemoveUnusedNonEmptyArrayBeforeForeachRector\Fixture;
+
+class KnownOffset
+{
+    public const DEFAULT_GROUP = 'default';
+
+    /**
+     * @var array<string, list<self>>
+     */
+    private static $groups = [];
+
+    public static function knownOffset(): array
+    {
+        $group = 'default';
+
+        self::$groups[$group] = ["foo"];
+
+        foreach (self::$groups[$group] as $group) {
+            echo "hello";
+        }
+
+        return [];
+    }
+}
+
+?>
diff --git a/...RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/skip_phpdoc_offset_dim_fetch.php.inc b/...RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/skip_phpdoc_offset_dim_fetch.php.inc
@@ -0,0 +1,24 @@
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\If_\RemoveUnusedNonEmptyArrayBeforeForeachRector\Fixture;
+
+class SkipPhpdocOffset
+{
+    /**
+     * @param array{default: foo} $array
+     */
+    public static function knownOffset($array): array
+    {
+        $group = 'default';
+
+        if (! empty($array[$group])) {
+            foreach ($array[$group] as $groupValue) {
+                echo "hello";
+            }
+        }
+
+        return [];
+    }
+}
+
+?>
diff --git a/.../If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/skip_property_dim_fetch.php.inc b/.../If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/skip_property_dim_fetch.php.inc
@@ -0,0 +1,24 @@
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\If_\RemoveUnusedNonEmptyArrayBeforeForeachRector\Fixture;
+
+class SkipPropertyDimFetch
+{
+    public const DEFAULT_GROUP = 'default';
+
+    /**
+     * @var array<string, list<self>>
+     */
+    private static $groups = [];
+
+    public static function getFootnotes($group = self::DEFAULT_GROUP): array
+    {
+        if (! empty(self::$groups[$group])) {
+            foreach (self::$groups[$group] as $note) {
+                echo "hello";
+            }
+        }
+
+        return [];
+    }
+}
diff --git a/rules/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector.php b/rules/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector.php
@@ -106,16 +106,8 @@ private function isUselessBeforeForeachCheck(If_ $if, Scope $scope): bool
         $foreach = $if->stmts[0];
         $foreachExpr = $foreach->expr;
 
-        if ($foreachExpr instanceof Variable) {
-            $variableName = $this->nodeNameResolver->getName($foreachExpr);
-            if (is_string($variableName) && $this->reservedKeywordAnalyzer->isNativeVariable($variableName)) {
-                return false;
-            }
-
-            $ifType = $scope->getNativeType($foreachExpr);
-            if (!$ifType->isArray()->yes()) {
-                return false;
-            }
+        if ($this->shouldSkipForeachExpr($foreachExpr, $scope)) {
+            return false;
         }
 
         $ifCond = $if->cond;
@@ -208,4 +200,29 @@ private function refactorIf(If_ $if, Scope $scope): ?Foreach_
 
         return $stmt;
     }
+
+    private function shouldSkipForeachExpr(Expr $foreachExpr, Scope $scope): bool
+    {
+        if ($foreachExpr instanceof Expr\ArrayDimFetch && $foreachExpr->dim !== null) {
+            $exprType = $this->nodeTypeResolver->getNativeType($foreachExpr->var);
+            $dimType = $this->nodeTypeResolver->getNativeType($foreachExpr->dim);
+            if (!$exprType->hasOffsetValueType($dimType)->yes()) {
+                return true;
+            }
+        }
+
+        if ($foreachExpr instanceof Variable) {
+            $variableName = $this->nodeNameResolver->getName($foreachExpr);
+            if (is_string($variableName) && $this->reservedKeywordAnalyzer->isNativeVariable($variableName)) {
+                return true;
+            }
+
+            $ifType = $scope->getNativeType($foreachExpr);
+            if (!$ifType->isArray()->yes()) {
+                return true;
+            }
+        }
+
+        return false;
+    }
 }