From 7fa9851ce61b007a7efe7c78d56c2976925d65b8 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Tue, 25 Nov 2025 22:38:25 +0700 Subject: [PATCH 1/8] [NodeVisitor] Drop ByRefReturnNodeVisitor, check early in only used once SimplifyUselessVariableRector --- .../SimplifyUselessVariableRector.php | 14 +++-- .../LazyContainerFactory.php | 2 - src/NodeTypeResolver/Node/AttributeKey.php | 5 -- .../NodeVisitor/ByRefReturnNodeVisitor.php | 57 ------------------- src/Rector/AbstractRector.php | 20 +++++-- 5 files changed, 22 insertions(+), 76 deletions(-) delete mode 100644 src/NodeTypeResolver/PHPStan/Scope/NodeVisitor/ByRefReturnNodeVisitor.php 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..bd604cf1dc2 100644 --- a/src/Rector/AbstractRector.php +++ b/src/Rector/AbstractRector.php @@ -4,9 +4,11 @@ namespace Rector\Rector; +use PhpParser\Builder\FunctionLike; use PhpParser\Node; use PhpParser\Node\Name; use PhpParser\Node\PropertyItem; +use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Const_; use PhpParser\Node\Stmt\InlineHTML; @@ -297,6 +299,8 @@ protected function mirrorComments(Node $newNode, Node $oldNode): void private function decorateCurrentAndChildren(Node $node): void { + $node->setAttribute(AttributeKey::SKIPPED_BY_RECTOR_RULE, static::class); + // filter only types that // 1. registered in getNodesTypes() method // 2. different with current node type, as already decorated above @@ -306,16 +310,20 @@ private function decorateCurrentAndChildren(Node $node): void static fn (string $nodeType): bool => $nodeType !== $node::class ); - if ($otherTypes === []) { - return; - } - - $this->traverseNodesWithCallable($node, static function (Node $subNode) use ($otherTypes): null { + $this->traverseNodesWithCallable($node, static function (Node $subNode) use ($otherTypes): int|Node { + // 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; + // inner class/function are out of scope + if ($subNode instanceof Class_ || $subNode instanceof FunctionLike) { + return NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN; + } + + $subNode->setAttribute(AttributeKey::SKIPPED_BY_RECTOR_RULE, static::class); + return $subNode; }); } From 139142912c9850257e6717f987466cf44f07d5b5 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Tue, 25 Nov 2025 22:50:50 +0700 Subject: [PATCH 2/8] temporary pin shipmonk/composer-dependency-analyser to 1.8.3 --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index d9d9d9baf90..6f1fe292d56 100644 --- a/composer.json +++ b/composer.json @@ -54,7 +54,7 @@ "rector/release-notes-generator": "^0.5", "rector/swiss-knife": "^2.3", "rector/type-perfect": "^2.1", - "shipmonk/composer-dependency-analyser": "^1.8", + "shipmonk/composer-dependency-analyser": "1.8.3", "symplify/phpstan-extensions": "^12.0.2", "symplify/phpstan-rules": "^14.8.5", "symplify/vendor-patches": "^11.5", From 9f3842b249e7fd32b67c79eb8fd9d5b715ed2584 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Wed, 26 Nov 2025 00:41:27 +0700 Subject: [PATCH 3/8] ignore errors unknown class on build/config/config-downgrade.php --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 6f1fe292d56..d9d9d9baf90 100644 --- a/composer.json +++ b/composer.json @@ -54,7 +54,7 @@ "rector/release-notes-generator": "^0.5", "rector/swiss-knife": "^2.3", "rector/type-perfect": "^2.1", - "shipmonk/composer-dependency-analyser": "1.8.3", + "shipmonk/composer-dependency-analyser": "^1.8", "symplify/phpstan-extensions": "^12.0.2", "symplify/phpstan-rules": "^14.8.5", "symplify/vendor-patches": "^11.5", From 84776884a3ded8462452b8fd1bdf7be6aa771ab8 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Wed, 26 Nov 2025 01:07:05 +0700 Subject: [PATCH 4/8] add outer ref fixture --- .../Fixture/return_by_ref_outer.php.inc | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 rules-tests/CodeQuality/Rector/FunctionLike/SimplifyUselessVariableRector/Fixture/return_by_ref_outer.php.inc 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..46d7a248366 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/FunctionLike/SimplifyUselessVariableRector/Fixture/return_by_ref_outer.php.inc @@ -0,0 +1,38 @@ + +----- + \ No newline at end of file From 494ffa9cff190d0552b601e7b029e81cd69011cb Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Wed, 26 Nov 2025 01:09:09 +0700 Subject: [PATCH 5/8] fix test --- src/Rector/AbstractRector.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Rector/AbstractRector.php b/src/Rector/AbstractRector.php index bd604cf1dc2..8f4f9a3f94a 100644 --- a/src/Rector/AbstractRector.php +++ b/src/Rector/AbstractRector.php @@ -4,8 +4,8 @@ namespace Rector\Rector; -use PhpParser\Builder\FunctionLike; use PhpParser\Node; +use PhpParser\Node\FunctionLike; use PhpParser\Node\Name; use PhpParser\Node\PropertyItem; use PhpParser\Node\Stmt\Class_; @@ -310,13 +310,18 @@ private function decorateCurrentAndChildren(Node $node): void static fn (string $nodeType): bool => $nodeType !== $node::class ); - $this->traverseNodesWithCallable($node, static function (Node $subNode) use ($otherTypes): int|Node { + $this->traverseNodesWithCallable($node, static function (Node $subNode) use ($node, $otherTypes): null|int|Node { // 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; } + // already set + if ($node === $subNode) { + return null; + } + // inner class/function are out of scope if ($subNode instanceof Class_ || $subNode instanceof FunctionLike) { return NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN; From cb512658a10280ae1bfd8328de0693727a7bced4 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 27 Nov 2025 12:22:48 +0700 Subject: [PATCH 6/8] fix --- src/Rector/AbstractRector.php | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/Rector/AbstractRector.php b/src/Rector/AbstractRector.php index 8f4f9a3f94a..c66902977f2 100644 --- a/src/Rector/AbstractRector.php +++ b/src/Rector/AbstractRector.php @@ -8,7 +8,6 @@ use PhpParser\Node\FunctionLike; use PhpParser\Node\Name; use PhpParser\Node\PropertyItem; -use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Const_; use PhpParser\Node\Stmt\InlineHTML; @@ -25,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; @@ -299,31 +299,34 @@ protected function mirrorComments(Node $newNode, Node $oldNode): void private function decorateCurrentAndChildren(Node $node): void { - $node->setAttribute(AttributeKey::SKIPPED_BY_RECTOR_RULE, static::class); - // filter only types that // 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 ); - $this->traverseNodesWithCallable($node, static function (Node $subNode) use ($node, $otherTypes): null|int|Node { - // early check here as included in other types defined in getNodeTypes() - if (in_array($subNode::class, $otherTypes, true)) { + $isCurrentNode = false; + $this->traverseNodesWithCallable($node, static function (Node $subNode) use ($node, $types, $otherTypes, &$isCurrentNode): int|Node { + // first visited is current node itself + if (! $isCurrentNode) { + $isCurrentNode = true; $subNode->setAttribute(AttributeKey::SKIPPED_BY_RECTOR_RULE, static::class); return $subNode; } - // already set - if ($node === $subNode) { - return 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; } - // inner class/function are out of scope - if ($subNode instanceof Class_ || $subNode instanceof FunctionLike) { + // exact StmtsAwareInterface will be visited by itself, and requires revalidated + // no need to apply skip by current rule + if ($types === [StmtsAwareInterface::class] && $node instanceof FunctionLike && $subNode instanceof FunctionLike) { return NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN; } From 397ac7d15c41cd39b8043451ddf0efc4accd78a8 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 27 Nov 2025 12:25:30 +0700 Subject: [PATCH 7/8] final touch: eol --- .../Fixture/return_by_ref_outer.php.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 46d7a248366..95f04466f55 100644 --- 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 @@ -35,4 +35,4 @@ class ReturnByRefOther } } -?> \ No newline at end of file +?> From d1b82d9085bc5868650d56a7a803b01a4cd2ff2f Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 27 Nov 2025 14:45:04 +0700 Subject: [PATCH 8/8] final touch: early verify only [StmtsAwareInterface::class] to avoid regression as possible --- src/Rector/AbstractRector.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Rector/AbstractRector.php b/src/Rector/AbstractRector.php index c66902977f2..f5439ceb6c9 100644 --- a/src/Rector/AbstractRector.php +++ b/src/Rector/AbstractRector.php @@ -310,7 +310,7 @@ private function decorateCurrentAndChildren(Node $node): void ); $isCurrentNode = false; - $this->traverseNodesWithCallable($node, static function (Node $subNode) use ($node, $types, $otherTypes, &$isCurrentNode): int|Node { + $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; @@ -324,9 +324,13 @@ private function decorateCurrentAndChildren(Node $node): void return $subNode; } + 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 ($types === [StmtsAwareInterface::class] && $node instanceof FunctionLike && $subNode instanceof FunctionLike) { + if ($node instanceof FunctionLike && $subNode instanceof FunctionLike) { return NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN; }