Make use of StmtsAwareInterface (#3781)

rectorphp · May 10, 2023 · 89bc4d8 · 89bc4d8
1 parent 376f6cb
commit 89bc4d8
Show file tree

Hide file tree

Showing 11 changed files with 233 additions and 165 deletions.
diff --git a/packages/NodeNameResolver/NodeNameResolver/FunctionNameResolver.php b/packages/NodeNameResolver/NodeNameResolver/FunctionNameResolver.php
@@ -8,7 +8,6 @@
 use PhpParser\Node\Stmt\Function_;
 use PHPStan\Analyser\Scope;
 use Rector\NodeNameResolver\Contract\NodeNameResolverInterface;
-use Rector\NodeTypeResolver\Node\AttributeKey;
 
 /**
  * @implements NodeNameResolverInterface<Function_>

diff --git a/packages/NodeTypeResolver/Node/AttributeKey.php b/packages/NodeTypeResolver/Node/AttributeKey.php
@@ -78,13 +78,17 @@ final class AttributeKey
 
     /**
      * @internal of php-parser, do not change
+     * @deprecated Use StmtsAwareInterface instead
+     *
      * @see https://github.com/nikic/PHP-Parser/pull/681/files
      * @var string
      */
     public const PREVIOUS_NODE = 'previous';
 
     /**
      * @internal of php-parser, do not change
+     * @deprecated Use StmtsAwareInterface instead
+     *
      * @see https://github.com/nikic/PHP-Parser/pull/681/files
      * @var string
      */

diff --git a/phpstan.neon b/phpstan.neon
@@ -789,3 +789,12 @@ parameters:
         # next to resolve
         - '#Cognitive complexity for "Rector\\CodeQuality\\Rector\\Isset_\\IssetOnPropertyObjectToPropertyExistsRector\:\:refactor\(\)" is 13, keep it under 10#'
         - '#Cognitive complexity for "Rector\\EarlyReturn\\Rector\\Return_\\PreparedValueToEarlyReturnRector\:\:refactor\(\)" is 11, keep it under 10#'
+        - '#Cognitive complexity for "Rector\\CodeQuality\\Rector\\Foreach_\\SimplifyForeachToCoalescingRector\:\:refactor\(\)" is 16, keep it under 10#'
+        - '#Cognitive complexity for "Rector\\CodeQuality\\Rector\\Foreach_\\ForeachToInArrayRector\:\:refactor\(\)" is \d+, keep it under 10#'
+
+        # resolve continually
+        - '#(Matching|Fetching) deprecated class constant (NEXT_NODE|PREVIOUS_NODE) of class Rector\\NodeTypeResolver\\Node\\AttributeKey#'
+        - '#Property Rector\\Core\\Contract\\PhpParser\\Node\\StmtsAwareInterface\:\:\$stmts \(array<PhpParser\\Node\\Stmt>\|null\) does not accept array<PhpParser\\Node\\Stmt\|null>#'
+
+        # make autoload of php-parser in rector first
+        - '#PHPDoc tag @return contains generic type PhpParser\\Node\\Stmt\\Expression<PhpParser\\Node\\Expr\\Assign> but class PhpParser\\Node\\Stmt\\Expression is not generic#'
diff --git a/...odeQuality/Rector/Foreach_/ForeachToInArrayRector/Fixture/skip_more_inner_foreach.php.inc b/...odeQuality/Rector/Foreach_/ForeachToInArrayRector/Fixture/skip_more_inner_foreach.php.inc
@@ -0,0 +1,21 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\CodeQuality\Rector\Foreach_\ForeachToInArrayRector\Fixture;
+
+final class SkipMoreInnerForeach
+{
+    public function run($value)
+    {
+        foreach ($this->items as $item) {
+            $value = 100;
+
+            if (100 === $item) {
+                return true;
+            }
+        }
+
+        return $value;
+    }
+}
diff --git a/...uality/Rector/Foreach_/ForeachToInArrayRector/Fixture/skip_return_value_in_return.php.inc b/...uality/Rector/Foreach_/ForeachToInArrayRector/Fixture/skip_return_value_in_return.php.inc
@@ -0,0 +1,17 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\Foreach_\ForeachToInArrayRector\Fixture;
+
+final class SkipReturnValueInReturn
+{
+    public function run($value)
+    {
+        foreach ($this->items as $item) {
+            if (100 === $item) {
+                return true;
+            }
+        }
+
+        return $value;
+    }
+}
diff --git a/rules-tests/CodeQuality/Rector/Foreach_/ForeachToInArrayRector/Fixture/some_class.php.inc b/rules-tests/CodeQuality/Rector/Foreach_/ForeachToInArrayRector/Fixture/some_class.php.inc
@@ -27,7 +27,6 @@ class SomeClass
 {
     public function foreachWithElseNullable($items)
     {
-        // some comment
         return in_array('something', $items);
     }
 }

diff --git a/...CoalescingRector/Fixture/fixture3.php.inc → ...Rector/Fixture/multiple_foreaches.php.inc b/...CoalescingRector/Fixture/fixture3.php.inc → ...Rector/Fixture/multiple_foreaches.php.inc
@@ -2,11 +2,13 @@
 
 namespace Rector\Tests\CodeQuality\Rector\Foreach_\SimplifyForeachToCoalescingRector\Fixture;
 
-class Fixture3
+final class MultipleForeaches
 {
     public function run()
     {
         $newValue = null;
+        $nextValue = null;
+
         $values = [];
         $input = '123';
 
@@ -18,7 +20,7 @@ class Fixture3
 
         foreach ($values as $key => $value) {
             if ($input === $key) {
-                $newValue = $value;
+                $nextValue = $value;
             }
         }
     }
@@ -30,17 +32,19 @@ class Fixture3
 
 namespace Rector\Tests\CodeQuality\Rector\Foreach_\SimplifyForeachToCoalescingRector\Fixture;
 
-class Fixture3
+final class MultipleForeaches
 {
     public function run()
     {
         $newValue = null;
+        $nextValue = null;
+
         $values = [];
         $input = '123';
 
         $newValue = $values[$input] ?? $newValue;
 
-        $newValue = $values[$input] ?? $newValue;
+        $nextValue = $values[$input] ?? $nextValue;
     }
 }
 

diff --git a/...espace_/ImportFullyQualifiedNamesRector/FixtureNonNamespaced/simple_with_strict_types.php b/...espace_/ImportFullyQualifiedNamesRector/FixtureNonNamespaced/simple_with_strict_types.php
@@ -0,0 +1,8 @@
+<?php
+
+declare(strict_types=1);
+
+function simpleWithStrictTypes()
+{
+    $baz = new \Foo\Bar();
+}
diff --git a/rules/CodeQuality/Rector/Foreach_/ForeachToInArrayRector.php b/rules/CodeQuality/Rector/Foreach_/ForeachToInArrayRector.php
@@ -15,10 +15,9 @@
 use PhpParser\Node\Stmt\If_;
 use PhpParser\Node\Stmt\Return_;
 use PHPStan\Type\ObjectType;
-use Rector\BetterPhpDocParser\Comment\CommentsMerger;
+use Rector\Core\Contract\PhpParser\Node\StmtsAwareInterface;
 use Rector\Core\NodeManipulator\BinaryOpManipulator;
 use Rector\Core\Rector\AbstractRector;
-use Rector\NodeTypeResolver\Node\AttributeKey;
 use Rector\Php71\ValueObject\TwoNodeMatch;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
@@ -30,7 +29,6 @@ final class ForeachToInArrayRector extends AbstractRector
 {
     public function __construct(
         private readonly BinaryOpManipulator $binaryOpManipulator,
-        private readonly CommentsMerger $commentsMerger
     ) {
     }
 
@@ -63,70 +61,88 @@ public function getRuleDefinition(): RuleDefinition
      */
     public function getNodeTypes(): array
     {
-        return [Foreach_::class];
+        return [StmtsAwareInterface::class];
     }
 
     /**
-     * @param Foreach_ $node
+     * @param StmtsAwareInterface $node
      */
     public function refactor(Node $node): ?Node
     {
-        if ($this->shouldSkipForeach($node)) {
+        if ($node->stmts === null) {
             return null;
         }
 
-        /** @var If_ $firstNodeInsideForeach */
-        $firstNodeInsideForeach = $node->stmts[0];
-        if ($this->shouldSkipIf($firstNodeInsideForeach)) {
-            return null;
-        }
+        foreach ($node->stmts as $key => $stmt) {
+            if (! $stmt instanceof Return_) {
+                continue;
+            }
 
-        /** @var Identical|Equal $ifCondition */
-        $ifCondition = $firstNodeInsideForeach->cond;
-        $foreachValueVar = $node->valueVar;
+            $prevStmt = $node->stmts[$key - 1] ?? null;
+            if (! $prevStmt instanceof Foreach_) {
+                continue;
+            }
 
-        $twoNodeMatch = $this->matchNodes($ifCondition, $foreachValueVar);
-        if (! $twoNodeMatch instanceof TwoNodeMatch) {
-            return null;
-        }
+            $return = $stmt;
+            $foreach = $prevStmt;
 
-        $comparedExpr = $twoNodeMatch->getSecondExpr();
-        if (! $this->isIfBodyABoolReturnNode($firstNodeInsideForeach)) {
-            return null;
-        }
+            if ($this->shouldSkipForeach($foreach)) {
+                return null;
+            }
 
-        $funcCall = $this->createInArrayFunction($comparedExpr, $ifCondition, $node);
+            /** @var If_ $firstNodeInsideForeach */
+            $firstNodeInsideForeach = $foreach->stmts[0];
+            if ($this->shouldSkipIf($firstNodeInsideForeach)) {
+                return null;
+            }
 
-        /** @var Return_ $returnToRemove */
-        $returnToRemove = $node->getAttribute(AttributeKey::NEXT_NODE);
+            /** @var Identical|Equal $ifCondition */
+            $ifCondition = $firstNodeInsideForeach->cond;
 
-        /** @var Return_ $return */
-        $return = $firstNodeInsideForeach->stmts[0];
-        if (! $returnToRemove->expr instanceof Expr) {
-            return null;
-        }
+            $twoNodeMatch = $this->matchNodes($ifCondition, $foreach->valueVar);
+            if (! $twoNodeMatch instanceof TwoNodeMatch) {
+                return null;
+            }
 
-        if (! $this->valueResolver->isTrueOrFalse($returnToRemove->expr)) {
-            return null;
-        }
+            $comparedExpr = $twoNodeMatch->getSecondExpr();
+            if (! $this->isIfBodyABoolReturnNode($firstNodeInsideForeach)) {
+                return null;
+            }
 
-        $returnedExpr = $return->expr;
-        if (! $returnedExpr instanceof Expr) {
-            return null;
-        }
+            $foreachReturn = $firstNodeInsideForeach->stmts[0];
+            if (! $foreachReturn instanceof Return_) {
+                return null;
+            }
 
-        // cannot be "return true;" + "return true;"
-        if ($this->nodeComparator->areNodesEqual($return, $returnToRemove)) {
-            return null;
-        }
+            if (! $return->expr instanceof Expr) {
+                return null;
+            }
+
+            if (! $this->valueResolver->isTrueOrFalse($return->expr)) {
+                return null;
+            }
+
+            if (! $foreachReturn->expr instanceof Expr) {
+                return null;
+            }
 
-        $this->removeNode($returnToRemove);
+            // cannot be "return true;" + "return true;"
+            if ($this->nodeComparator->areNodesEqual($return, $foreachReturn)) {
+                return null;
+            }
 
-        $return = $this->createReturn($returnedExpr, $funcCall);
+            // 1. remove foreach
+            unset($node->stmts[$key - 1]);
 
-        $this->commentsMerger->keepChildren($return, $node);
+            // 2. make return of in_array()
+            $funcCall = $this->createInArrayFunction($comparedExpr, $ifCondition, $foreach);
+            $return = $this->createReturn($foreachReturn->expr, $funcCall);
+            $node->stmts[$key] = $return;
 
-        return $return;
+            return $node;
+        }
+
+        return null;
     }
 
     private function shouldSkipForeach(Foreach_ $foreach): bool
@@ -143,26 +159,7 @@ private function shouldSkipForeach(Foreach_ $foreach): bool
             return true;
         }
 
-        $nextNode = $foreach->getAttribute(AttributeKey::NEXT_NODE);
-        if (! $nextNode instanceof Node) {
-            return true;
-        }
-
-        if (! $nextNode instanceof Return_) {
-            return true;
-        }
-
-        $returnExpression = $nextNode->expr;
-        if (! $returnExpression instanceof Expr) {
-            return true;
-        }
-
-        if (! $this->valueResolver->isTrueOrFalse($returnExpression)) {
-            return true;
-        }
-
         $foreachValueStaticType = $this->getType($foreach->expr);
-
         return $foreachValueStaticType instanceof ObjectType;
     }