[DX] Deprecate parent node attribute, allow return of NodeTraverser::…

…* in refactor() method (#3922) Co-authored-by: GitHub Action <actions@github.com>
rectorphp · May 22, 2023 · 6c3f2cd · 6c3f2cd
1 parent 71f00d9
commit 6c3f2cd
Show file tree

Hide file tree

Showing 9 changed files with 97 additions and 34 deletions.
diff --git a/packages/NodeTypeResolver/Node/AttributeKey.php b/packages/NodeTypeResolver/Node/AttributeKey.php
@@ -73,6 +73,9 @@ final class AttributeKey
     public const RESOLVED_NAME = 'resolvedName';
 
     /**
+     * @deprecated Refactor to a custom visitors/parent node instead,
+     * @see https://phpstan.org/blog/preprocessing-ast-for-custom-rules
+     *
      * @internal of php-parser, do not change
      * @see https://github.com/nikic/PHP-Parser/pull/681/files
      * @var string

diff --git a/phpstan.neon b/phpstan.neon
@@ -767,3 +767,6 @@ parameters:
 
         - '#Cognitive complexity for "Rector\\TypeDeclaration\\Rector\\ClassMethod\\BoolReturnTypeFromStrictScalarReturnsRector\:\:hasOnlyBoolScalarReturnExprs\(\)" is 11, keep it under 10#'
         - '#Access to an undefined property PhpParser\\Node\\Stmt\\ClassLike\|PhpParser\\Node\\Stmt\\Declare_\|Rector\\Core\\Contract\\PhpParser\\Node\\StmtsAwareInterface\:\:\$stmts#'
+
+        # WIP
+        - '#Fetching deprecated class constant PARENT_NODE#'
diff --git a/rules-tests/CodeQuality/Rector/FuncCall/SetTypeToCastRector/Fixture/skip_args.php.inc b/rules-tests/CodeQuality/Rector/FuncCall/SetTypeToCastRector/Fixture/skip_args.php.inc
@@ -0,0 +1,13 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\FuncCall\SetTypeToCastRector\Fixture;
+
+final class SkipArgs
+{
+    public function run($foo)
+    {
+        is_bool(settype($foo, 'string'));
+
+        settype($foo, settype($foo, 'string'));
+    }
+}
diff --git a/rules-tests/CodeQuality/Rector/FuncCall/SetTypeToCastRector/Fixture/skip_array_items.php.inc b/rules-tests/CodeQuality/Rector/FuncCall/SetTypeToCastRector/Fixture/skip_array_items.php.inc
@@ -0,0 +1,12 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\FuncCall\SetTypeToCastRector\Fixture;
+
+class SkipArrayItems
+{
+    public function run($foo)
+    {
+        $result = [settype($foo, 'string')];
+        $result = [0 => settype($foo, 'string')];
+    }
+}
diff --git a/rules-tests/CodeQuality/Rector/FuncCall/SetTypeToCastRector/Fixture/skip_assign.php.inc b/rules-tests/CodeQuality/Rector/FuncCall/SetTypeToCastRector/Fixture/skip_assign.php.inc
@@ -7,13 +7,5 @@ class SkipAssign
     public function run($foo)
     {
         $result = settype($foo, 'string');
-
-        is_bool(settype($foo, 'string'));
-
-        $result = [settype($foo, 'string')];
-
-        $result = [0 => settype($foo, 'string')];
-
-        settype($foo, settype($foo, 'string'));
     }
 }
diff --git a/rules/CodeQuality/Rector/FuncCall/ChangeArrayPushToArrayAssignRector.php b/rules/CodeQuality/Rector/FuncCall/ChangeArrayPushToArrayAssignRector.php
@@ -8,6 +8,7 @@
 use PhpParser\Node\Expr\ArrayDimFetch;
 use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\FuncCall;
+use PhpParser\Node\Stmt;
 use PhpParser\Node\Stmt\Expression;
 use Rector\Core\Rector\AbstractRector;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
@@ -49,8 +50,8 @@ public function getNodeTypes(): array
     }
 
     /**
-     * @param Expression[] $node
-     * @param Expression[]|null $node
+     * @param Expression $node
+     * @return Stmt[]|null
      */
     public function refactor(Node $node): ?array
     {

diff --git a/rules/CodeQuality/Rector/FuncCall/SetTypeToCastRector.php b/rules/CodeQuality/Rector/FuncCall/SetTypeToCastRector.php
@@ -4,8 +4,9 @@
 
 namespace Rector\CodeQuality\Rector\FuncCall;
 
-use PhpParser\Node;
+use PhpParser\Node\Expr\ArrayItem;
 use PhpParser\Node\Arg;
+use PhpParser\Node;
 use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\Cast;
@@ -17,8 +18,8 @@
 use PhpParser\Node\Expr\Cast\String_;
 use PhpParser\Node\Expr\FuncCall;
 use PhpParser\Node\Stmt\Expression;
+use PhpParser\NodeTraverser;
 use Rector\Core\Rector\AbstractRector;
-use Rector\NodeTypeResolver\Node\AttributeKey;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
@@ -80,54 +81,87 @@ public function run($foo)
      */
     public function getNodeTypes(): array
     {
-        return [FuncCall::class];
+        return [FuncCall::class, Expression::class, Assign::class, ArrayItem::class, Arg::class];
     }
 
     /**
-     * @param FuncCall $node
+     * @param FuncCall|Expression|Assign|Expr\ArrayItem|Node\Arg $node
      */
-    public function refactor(Node $node): ?Node
+    public function refactor(Node $node)
+    {
+        if ($node instanceof Arg || $node instanceof ArrayItem) {
+            if ($this->isSetTypeFuncCall($node->value)) {
+                return NodeTraverser::STOP_TRAVERSAL;
+            }
+
+            return null;
+        }
+
+        if ($node instanceof Assign) {
+            if (! $this->isSetTypeFuncCall($node->expr)) {
+                return null;
+            }
+
+            return NodeTraverser::DONT_TRAVERSE_CHILDREN;
+        }
+
+        if ($node instanceof Expression) {
+            if (! $node->expr instanceof FuncCall) {
+                return null;
+            }
+
+            return $this->refactorFuncCall($node->expr, true);
+        }
+
+        return $this->refactorFuncCall($node, false);
+    }
+
+    private function refactorFuncCall(FuncCall $funcCall, bool $isStandaloneExpression): Assign|null|Cast
     {
-        if (! $this->isName($node, 'settype')) {
+        if (! $this->isSetTypeFuncCall($funcCall)) {
             return null;
         }
 
-        if ($node->isFirstClassCallable()) {
+        if ($funcCall->isFirstClassCallable()) {
             return null;
         }
 
-        $typeValue = $this->valueResolver->getValue($node->getArgs()[1]->value);
+        $typeValue = $this->valueResolver->getValue($funcCall->getArgs()[1]->value);
         if (! is_string($typeValue)) {
             return null;
         }
 
         $typeValue = strtolower($typeValue);
 
-        $variable = $node->getArgs()[0]
-->value;
-        $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
-
-        // result of function or probably used
-        if ($parentNode instanceof Expr || $parentNode instanceof Arg) {
-            return null;
-        }
+        $variable = $funcCall->getArgs()[0]
+            ->value;
 
         if (isset(self::TYPE_TO_CAST[$typeValue])) {
             $castClass = self::TYPE_TO_CAST[$typeValue];
             $castNode = new $castClass($variable);
 
-            if ($parentNode instanceof Expression) {
-                // bare expression? → assign
-                return new Assign($variable, $castNode);
+            if (! $isStandaloneExpression) {
+                return $castNode;
             }
 
-            return $castNode;
+            // bare expression? → assign
+            return new Assign($variable, $castNode);
         }
 
         if ($typeValue === 'null') {
             return new Assign($variable, $this->nodeFactory->createNull());
         }
 
-        return $node;
+        return null;
+    }
+
+    private function isSetTypeFuncCall(Expr $expr): bool
+    {
+        // skip assign of settype() calls
+        if (! $expr instanceof FuncCall) {
+            return false;
+        }
+
+        return $this->isName($expr, 'settype');
     }
 }
diff --git a/src/Contract/Rector/PhpRectorInterface.php b/src/Contract/Rector/PhpRectorInterface.php
@@ -5,6 +5,7 @@
 namespace Rector\Core\Contract\Rector;
 
 use PhpParser\Node;
+use PhpParser\NodeTraverser;
 use PhpParser\NodeVisitor;
 
 interface PhpRectorInterface extends NodeVisitor, RectorInterface
@@ -19,7 +20,7 @@ public function getNodeTypes(): array;
 
     /**
      * Process Node of matched type
-     * @return Node|Node[]|null
+     * @return Node|Node[]|null|NodeTraverser::*
      */
     public function refactor(Node $node);
 }
diff --git a/src/Rector/AbstractRector.php b/src/Rector/AbstractRector.php
@@ -184,7 +184,7 @@ public function beforeTraverse(array $nodes): ?array
         return parent::beforeTraverse($nodes);
     }
 
-    final public function enterNode(Node $node): ?Node
+    final public function enterNode(Node $node)
     {
         $nodeClass = $node::class;
         if (! $this->isMatchingNodeType($nodeClass)) {
@@ -216,11 +216,15 @@ final public function enterNode(Node $node): ?Node
         }
 
         $refactoredNode = $this->refactor($node);
-
         if ($isDebug) {
             $this->printConsumptions($startTime, $previousMemory);
         }
 
+        // @see NodeTravser::* codes, e.g. removal of node of stopping the traversing
+        if (is_int($refactoredNode)) {
+            return $refactoredNode;
+        }
+
         // nothing to change or just removed via removeNode() → continue
         if ($refactoredNode === null) {
             return null;