diff --git a/phpstan.neon b/phpstan.neon index 0d6e44bf..f740bf03 100644 --- a/phpstan.neon +++ b/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 diff --git a/rules-tests/DowngradePhp82/Rector/FuncCall/DowngradeIteratorCountToArrayRector/Fixture/is_array_guard_on_other_expr.php.inc b/rules-tests/DowngradePhp82/Rector/FuncCall/DowngradeIteratorCountToArrayRector/Fixture/is_array_guard_on_other_expr.php.inc new file mode 100644 index 00000000..52fdac31 --- /dev/null +++ b/rules-tests/DowngradePhp82/Rector/FuncCall/DowngradeIteratorCountToArrayRector/Fixture/is_array_guard_on_other_expr.php.inc @@ -0,0 +1,37 @@ + +----- + diff --git a/rules-tests/DowngradePhp82/Rector/FuncCall/DowngradeIteratorCountToArrayRector/Fixture/skip_is_array_guarded.php.inc b/rules-tests/DowngradePhp82/Rector/FuncCall/DowngradeIteratorCountToArrayRector/Fixture/skip_is_array_guarded.php.inc new file mode 100644 index 00000000..de169da3 --- /dev/null +++ b/rules-tests/DowngradePhp82/Rector/FuncCall/DowngradeIteratorCountToArrayRector/Fixture/skip_is_array_guarded.php.inc @@ -0,0 +1,38 @@ +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 []; + } +} diff --git a/rules/DowngradePhp82/Rector/FuncCall/DowngradeIteratorCountToArrayRector.php b/rules/DowngradePhp82/Rector/FuncCall/DowngradeIteratorCountToArrayRector.php index ccabb63d..0364c423 100644 --- a/rules/DowngradePhp82/Rector/FuncCall/DowngradeIteratorCountToArrayRector.php +++ b/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();