Skip iterator_to_array()/iterator_count() already guarded by is_array() on DowngradeIteratorCountToArrayRector

phpstan.neon

@@ -117,6 +117,7 @@ parameters:
                  - rules/DowngradePhp74/Rector/MethodCall/DowngradeReflectionGetTypeRector.php
                  - rules/DowngradePhp80/Rector/MethodCall/DowngradeReflectionGetAttributesRector.php
                  - rules/DowngradePhp81/Rector/MethodCall/DowngradeIsEnumRector.php
+                 - rules/DowngradePhp82/Rector/FuncCall/DowngradeIteratorCountToArrayRector.php
                  - rules/DowngradePhp73/Rector/String_/DowngradeFlexibleHeredocSyntaxRector.php
                  - rules/DowngradePhp80/Rector/Expression/DowngradeMatchToSwitchRector.php
 

rules-tests/DowngradePhp82/Rector/FuncCall/DowngradeIteratorCountToArrayRector/Fixture/is_array_guard_on_other_expr.php.inc

@@ -0,0 +1,37 @@
+<?php
+
+namespace Rector\Tests\DowngradePhp82\Rector\FuncCall\DowngradeIteratorCountToArrayRector\Fixture;
+
+final class IsArrayGuardOnOtherExpr
+{
+    public function run($data, $other)
+    {
+        return is_array($other) ? $other : iterator_to_array($data);
+    }
+
+    public function run2($data)
+    {
+        return is_array($data) ? iterator_to_array($data) : [];
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\DowngradePhp82\Rector\FuncCall\DowngradeIteratorCountToArrayRector\Fixture;
+
+final class IsArrayGuardOnOtherExpr
+{
+    public function run($data, $other)
+    {
+        return is_array($other) ? $other : iterator_to_array(is_array($data) ? new \ArrayIterator($data) : $data);
+    }
+
+    public function run2($data)
+    {
+        return is_array($data) ? iterator_to_array(is_array($data) ? new \ArrayIterator($data) : $data) : [];
+    }
+}
+
+?>

rules-tests/DowngradePhp82/Rector/FuncCall/DowngradeIteratorCountToArrayRector/Fixture/skip_is_array_guarded.php.inc

@@ -0,0 +1,38 @@
+<?php
+
+namespace Rector\Tests\DowngradePhp82\Rector\FuncCall\DowngradeIteratorCountToArrayRector\Fixture;
+
+final class SkipIsArrayGuarded
+{
+    private $data;
+
+    public function run($data)
+    {
+        return is_array($data) ? $data : iterator_to_array($data);
+    }
+
+    public function run2($data)
+    {
+        return ! is_array($data) ? iterator_count($data) : count($data);
+    }
+
+    public function run3()
+    {
+        return array_merge(is_array($this->data) ? $this->data : iterator_to_array($this->data), []);
+    }
+
+    public function run4(SomeUnknownClass $object)
+    {
+        return array_merge(is_array($object->getItems()) ? $object->getItems() : iterator_to_array($object->getItems()), []);
+    }
+
+    public function run5()
+    {
+        return is_array($this->getItems()) ? $this->getItems() : iterator_to_array($this->getItems());
+    }
+
+    private function getItems()
+    {
+        return [];
+    }
+}

rules/DowngradePhp82/Rector/FuncCall/DowngradeIteratorCountToArrayRector.php

@@ -7,6 +7,7 @@
 use PhpParser\Node;
 use PhpParser\Node\Arg;
 use PhpParser\Node\Expr;
+use PhpParser\Node\Expr\BooleanNot;
 use PhpParser\Node\Expr\FuncCall;
 use PhpParser\Node\Expr\New_;
 use PhpParser\Node\Expr\Ternary;
@@ -27,6 +28,8 @@
  */
 final class DowngradeIteratorCountToArrayRector extends AbstractRector
 {
+    private const string SKIP_NODE = 'skip_node';
+
     public function __construct(
         private readonly ArgsAnalyzer $argsAnalyzer,
     ) {
@@ -65,10 +68,19 @@ function test(array|Traversable $data) {
     }
 
     /**
-     * @param FuncCall $node
+     * @param Ternary|FuncCall $node
      */
     public function refactor(Node $node): null|FuncCall
     {
+        if ($node instanceof Ternary) {
+            $this->markIsArrayGuardedCall($node);
+            return null;
+        }
+
+        if ($node->getAttribute(self::SKIP_NODE) === true) {
+            return null;
+        }
+
         if (! $this->isNames($node, ['iterator_count', 'iterator_to_array'])) {
             return null;
         }
@@ -107,6 +119,43 @@ public function refactor(Node $node): null|FuncCall
         return $node;
     }
 
+    /**
+     * Skip the not-array branch of an is_array() check on the same expr, e.g.
+     * is_array($data) ? $data : iterator_to_array($data)
+     */
+    private function markIsArrayGuardedCall(Ternary $ternary): void
+    {
+        $cond = $ternary->cond;
+        $notArrayBranch = $ternary->else;
+
+        if ($cond instanceof BooleanNot) {
+            $cond = $cond->expr;
+            $notArrayBranch = $ternary->if;
+        }
+
+        if (! $cond instanceof FuncCall || ! $this->isName($cond, 'is_array') || $cond->isFirstClassCallable()) {
+            return;
+        }
+
+        if (! $notArrayBranch instanceof FuncCall || $notArrayBranch->isFirstClassCallable()) {
+            return;
+        }
+
+        if (! $this->isNames($notArrayBranch, ['iterator_count', 'iterator_to_array'])) {
+            return;
+        }
+
+        $checkedArg = $cond->getArg('value', 0);
+        $iteratorArg = $notArrayBranch->getArg('iterator', 0);
+        if (! $checkedArg instanceof Arg || ! $iteratorArg instanceof Arg) {
+            return;
+        }
+
+        if ($this->nodeComparator->areNodesEqual($checkedArg->value, $iteratorArg->value)) {
+            $notArrayBranch->setAttribute(self::SKIP_NODE, true);
+        }
+    }
+
     private function isArrayIteratorWrapped(Expr $expr): bool
     {
         if (! $expr instanceof Ternary) {
@@ -130,6 +179,12 @@ private function shouldSkip(Type $type): bool
             return false;
         }
 
+        // never an array, e.g. narrowed by is_array($data) ? $data : iterator_to_array($data),
+        // so the is_array() check would always be false
+        if ($type->isArray()->no()) {
+            return true;
+        }
+
         $type = TypeCombinator::removeNull($type);
         return $type->isObject()
             ->yes();