diff --git a/rules-tests/CodeQuality/Rector/FunctionLike/SimplifyUselessVariableRector/Fixture/return_by_ref_outer.php.inc b/rules-tests/CodeQuality/Rector/FunctionLike/SimplifyUselessVariableRector/Fixture/return_by_ref_outer.php.inc new file mode 100644 index 00000000000..95f04466f55 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/FunctionLike/SimplifyUselessVariableRector/Fixture/return_by_ref_outer.php.inc @@ -0,0 +1,38 @@ + +----- + diff --git a/rules/CodeQuality/Rector/FunctionLike/SimplifyUselessVariableRector.php b/rules/CodeQuality/Rector/FunctionLike/SimplifyUselessVariableRector.php index 0f9714edbac..b76de90b4a6 100644 --- a/rules/CodeQuality/Rector/FunctionLike/SimplifyUselessVariableRector.php +++ b/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; } diff --git a/src/DependencyInjection/LazyContainerFactory.php b/src/DependencyInjection/LazyContainerFactory.php index 89a743cf712..cc757811cfc 100644 --- a/src/DependencyInjection/LazyContainerFactory.php +++ b/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, diff --git a/src/NodeTypeResolver/Node/AttributeKey.php b/src/NodeTypeResolver/Node/AttributeKey.php index a685d2bbc9c..2a65d9010de 100644 --- a/src/NodeTypeResolver/Node/AttributeKey.php +++ b/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. * diff --git a/src/NodeTypeResolver/PHPStan/Scope/NodeVisitor/ByRefReturnNodeVisitor.php b/src/NodeTypeResolver/PHPStan/Scope/NodeVisitor/ByRefReturnNodeVisitor.php deleted file mode 100644 index 6950ee01d2f..00000000000 --- a/src/NodeTypeResolver/PHPStan/Scope/NodeVisitor/ByRefReturnNodeVisitor.php +++ /dev/null @@ -1,57 +0,0 @@ -returnsByRef()) { - return null; - } - - $stmts = $node->getStmts(); - if ($stmts === null) { - return null; - } - - $this->simpleCallableNodeTraverser->traverseNodesWithCallable( - $stmts, - static function (Node $node): int|null|Node { - if ($node instanceof Class_ || $node instanceof FunctionLike) { - return NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN; - } - - if (! $node instanceof Return_) { - return null; - } - - $node->setAttribute(AttributeKey::IS_BYREF_RETURN, true); - return $node; - } - ); - - return null; - } -} diff --git a/src/Rector/AbstractRector.php b/src/Rector/AbstractRector.php index 789560ff739..f5439ceb6c9 100644 --- a/src/Rector/AbstractRector.php +++ b/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; }); }