[NodeVisitor] Drop ByRefReturnNodeVisitor, check early in only used once SimplifyUselessVariableRector

rules-tests/CodeQuality/Rector/FunctionLike/SimplifyUselessVariableRector/Fixture/return_by_ref_outer.php.inc

@@ -0,0 +1,38 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\FunctionLike\SimplifyUselessVariableRector\Fixture;
+
+class ReturnByRefOther
+{
+    function & test()
+    {
+        function foo() {
+            $var = 9000;
+            return $var;
+        }
+
+        $var = 9000;
+        return $var;
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\FunctionLike\SimplifyUselessVariableRector\Fixture;
+
+class ReturnByRefOther
+{
+    function & test()
+    {
+        function foo() {
+            return 9000;
+        }
+
+        $var = 9000;
+        return $var;
+    }
+}
+
+?>

rules/CodeQuality/Rector/FunctionLike/SimplifyUselessVariableRector.php

@@ -10,16 +10,17 @@
 use PhpParser\Node\Expr\AssignOp;
 use PhpParser\Node\Expr\Ternary;
 use PhpParser\Node\Expr\Variable;
+use PhpParser\Node\FunctionLike;
 use PhpParser\Node\Stmt;
 use PhpParser\Node\Stmt\Expression;
 use PhpParser\Node\Stmt\Return_;
+use PhpParser\NodeVisitor;
 use PHPStan\Type\MixedType;
 use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
 use Rector\Contract\PhpParser\Node\StmtsAwareInterface;
 use Rector\Contract\Rector\ConfigurableRectorInterface;
 use Rector\NodeAnalyzer\CallAnalyzer;
 use Rector\NodeAnalyzer\VariableAnalyzer;
-use Rector\NodeTypeResolver\Node\AttributeKey;
 use Rector\PhpParser\Node\AssignAndBinaryMap;
 use Rector\Rector\AbstractRector;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\ConfiguredCodeSample;
@@ -111,14 +112,19 @@ public function getNodeTypes(): array
 
     /**
      * @param StmtsAwareInterface $node
+     * @return NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN|null|StmtsAwareInterface
      */
-    public function refactor(Node $node): ?Node
+    public function refactor(Node $node): int|null|Node
     {
         $stmts = $node->stmts;
         if ($stmts === null) {
             return null;
         }
 
+        if ($node instanceof FunctionLike && $node->returnsByRef()) {
+            return NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN;
+        }
+
         foreach ($stmts as $key => $stmt) {
             // has previous node?
             if (! isset($stmts[$key - 1])) {
@@ -178,10 +184,6 @@ private function shouldSkipStmt(Return_ $return, Stmt $previousStmt): bool
             return true;
         }
 
-        if ($return->getAttribute(AttributeKey::IS_BYREF_RETURN) === true) {
-            return true;
-        }
-
         if (! $previousStmt instanceof Expression) {
             return true;
         }

src/DependencyInjection/LazyContainerFactory.php

@@ -97,7 +97,6 @@
 use Rector\NodeTypeResolver\NodeTypeResolver\TraitTypeResolver;
 use Rector\NodeTypeResolver\PHPStan\Scope\NodeVisitor\ArgNodeVisitor;
 use Rector\NodeTypeResolver\PHPStan\Scope\NodeVisitor\AssignedToNodeVisitor;
-use Rector\NodeTypeResolver\PHPStan\Scope\NodeVisitor\ByRefReturnNodeVisitor;
 use Rector\NodeTypeResolver\PHPStan\Scope\NodeVisitor\ByRefVariableNodeVisitor;
 use Rector\NodeTypeResolver\PHPStan\Scope\NodeVisitor\ContextNodeVisitor;
 use Rector\NodeTypeResolver\PHPStan\Scope\NodeVisitor\GlobalVariableNodeVisitor;
@@ -236,7 +235,6 @@ final class LazyContainerFactory
     private const DECORATING_NODE_VISITOR_CLASSES = [
         ArgNodeVisitor::class,
         AssignedToNodeVisitor::class,
-        ByRefReturnNodeVisitor::class,
         ByRefVariableNodeVisitor::class,
         ContextNodeVisitor::class,
         GlobalVariableNodeVisitor::class,

src/NodeTypeResolver/Node/AttributeKey.php

@@ -150,11 +150,6 @@ final class AttributeKey
      */
     public const IS_BYREF_VAR = 'is_byref_var';
 
-    /**
-     * @var string
-     */
-    public const IS_BYREF_RETURN = 'is_byref_return';
-
     /**
      * @deprecated This value can change, as based on default input keys. Use existing array keys instead.
      *

src/Rector/AbstractRector.php

@@ -5,6 +5,7 @@
 namespace Rector\Rector;
 
 use PhpParser\Node;
+use PhpParser\Node\FunctionLike;
 use PhpParser\Node\Name;
 use PhpParser\Node\PropertyItem;
 use PhpParser\Node\Stmt\ClassMethod;
@@ -23,6 +24,7 @@
 use Rector\Application\Provider\CurrentFileProvider;
 use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo;
 use Rector\ChangesReporting\ValueObject\RectorWithLineChange;
+use Rector\Contract\PhpParser\Node\StmtsAwareInterface;
 use Rector\Contract\Rector\HTMLAverseRectorInterface;
 use Rector\Contract\Rector\RectorInterface;
 use Rector\Exception\ShouldNotHappenException;
@@ -301,21 +303,39 @@ private function decorateCurrentAndChildren(Node $node): void
         //    1. registered in getNodesTypes() method
         //    2. different with current node type, as already decorated above
         //
+        $types = $this->getNodeTypes();
         $otherTypes = array_filter(
-            $this->getNodeTypes(),
+            $types,
             static fn (string $nodeType): bool => $nodeType !== $node::class
         );
 
-        if ($otherTypes === []) {
-            return;
-        }
+        $isCurrentNode = false;
+        $this->traverseNodesWithCallable($node, static function (Node $subNode) use ($node, $types, $otherTypes, &$isCurrentNode): null|int|Node {
+            // first visited is current node itself
+            if (! $isCurrentNode) {
+                $isCurrentNode = true;
+                $subNode->setAttribute(AttributeKey::SKIPPED_BY_RECTOR_RULE, static::class);
+                return $subNode;
+            }
 
-        $this->traverseNodesWithCallable($node, static function (Node $subNode) use ($otherTypes): null {
+            // early check here as included in other types defined in getNodeTypes()
             if (in_array($subNode::class, $otherTypes, true)) {
                 $subNode->setAttribute(AttributeKey::SKIPPED_BY_RECTOR_RULE, static::class);
+                return $subNode;
             }
 
-            return null;
+            if ($types !== [StmtsAwareInterface::class]) {
+                return null;
+            }
+
+            // exact StmtsAwareInterface will be visited by itself, and requires revalidated
+            // no need to apply skip by current rule
+            if ($node instanceof FunctionLike && $subNode instanceof FunctionLike) {
+                return NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN;
+            }
+
+            $subNode->setAttribute(AttributeKey::SKIPPED_BY_RECTOR_RULE, static::class);
+            return $subNode;
         });
     }