From 142e034b3bfd153fb0dd3d40a27fd3920fae60f1 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 21 Dec 2025 23:57:05 +0100 Subject: [PATCH 1/3] [experiment] skip beforeTraverse() and afterTraverse() as never used --- .../CompleteMissingIfElseBracketRector.php | 5 ++++ .../ArrayDimFetch/ArrayFirstLastRector.php | 3 +++ src/PostRector/Rector/AbstractPostRector.php | 3 ++- src/Rector/AbstractRector.php | 27 +++++++++++-------- 4 files changed, 26 insertions(+), 12 deletions(-) diff --git a/rules/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector.php b/rules/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector.php index be8e73a7458..b6cd1f6d690 100644 --- a/rules/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector.php +++ b/rules/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector.php @@ -69,8 +69,13 @@ public function refactor(Node $node): ?Node return null; } +<<<<<<< HEAD $oldTokens = $this->getFile() ->getOldTokens(); +======= + $oldTokens = $this->file->getOldTokens(); + +>>>>>>> 7556383dcf ([experiment] skip beforeTraverse() and afterTraverse() as never used) if ($this->isIfConditionFollowedByOpeningCurlyBracket($node, $oldTokens)) { return null; } diff --git a/rules/Php85/Rector/ArrayDimFetch/ArrayFirstLastRector.php b/rules/Php85/Rector/ArrayDimFetch/ArrayFirstLastRector.php index 067d5463399..11251db7b56 100644 --- a/rules/Php85/Rector/ArrayDimFetch/ArrayFirstLastRector.php +++ b/rules/Php85/Rector/ArrayDimFetch/ArrayFirstLastRector.php @@ -179,6 +179,7 @@ private function shouldSkip(ArrayDimFetch $arrayDimFetch, Node $scopeNode): bool if ($scope->isInExpressionAssign($arrayDimFetch)) { return true; } +<<<<<<< HEAD if ($arrayDimFetch->getAttribute(AttributeKey::IS_BEING_ASSIGNED) === true) { return true; @@ -188,6 +189,8 @@ private function shouldSkip(ArrayDimFetch $arrayDimFetch, Node $scopeNode): bool return true; } +======= +>>>>>>> ce85ccbfcc (fixup! [experiment] skip beforeTraverse() and afterTraverse() as never used) return (bool) $arrayDimFetch->getAttribute(AttributeKey::IS_UNSET_VAR); } } diff --git a/src/PostRector/Rector/AbstractPostRector.php b/src/PostRector/Rector/AbstractPostRector.php index 341100666b9..18736f9fbb3 100644 --- a/src/PostRector/Rector/AbstractPostRector.php +++ b/src/PostRector/Rector/AbstractPostRector.php @@ -41,6 +41,7 @@ protected function addRectorClassWithLine(Node $node): void Assert::isInstanceOf($this->file, File::class); $rectorWithLineChange = new RectorWithLineChange(static::class, $node->getStartLine()); - $this->file->addRectorClassWithLine($rectorWithLineChange); + $this->getFile() + ->addRectorClassWithLine($rectorWithLineChange); } } diff --git a/src/Rector/AbstractRector.php b/src/Rector/AbstractRector.php index 63770a565f5..836662d9b6c 100644 --- a/src/Rector/AbstractRector.php +++ b/src/Rector/AbstractRector.php @@ -35,6 +35,9 @@ use Rector\Skipper\Skipper\Skipper; use Rector\ValueObject\Application\File; +/** + * @property-read File $file @deprecated Use $this->getFile() instead + */ abstract class AbstractRector extends NodeVisitorAbstract implements RectorInterface { private const string EMPTY_NODE_ARRAY_MESSAGE = <<>>>>>> 7556383dcf ([experiment] skip beforeTraverse() and afterTraverse() as never used) protected Skipper $skipper; private ChangedNodeScopeRefresher $changedNodeScopeRefresher; @@ -74,6 +80,16 @@ abstract class AbstractRector extends NodeVisitorAbstract implements RectorInter private CreatedByRuleDecorator $createdByRuleDecorator; + public function __get(string $name): mixed + { + if ($name === 'file') { + return $this->getFile(); + } + + // fallback to default behavior + return $this->{$name}; + } + public function autowire( NodeNameResolver $nodeNameResolver, NodeTypeResolver $nodeTypeResolver, @@ -100,23 +116,12 @@ public function autowire( /** * @final Avoid override to prevent unintended side-effects. Use enterNode() or @see \Rector\Contract\PhpParser\DecoratingNodeVisitorInterface instead. - * * @internal * * @return Node[]|null */ public function beforeTraverse(array $nodes): ?array { - // workaround for file around refactor() - $file = $this->currentFileProvider->getFile(); - if (! $file instanceof File) { - throw new ShouldNotHappenException( - 'File object is missing. Make sure you call $this->currentFileProvider->setFile(...) before traversing.' - ); - } - - $this->file = $file; - return null; } From 5bd619e85a756d45c85c185793fcc43121566f9b Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Thu, 5 Feb 2026 00:34:44 +0100 Subject: [PATCH 2/3] remove array item scope, as handled in array --- .../CompleteMissingIfElseBracketRector.php | 4 --- .../ArrayDimFetch/ArrayFirstLastRector.php | 3 --- src/Rector/AbstractRector.php | 27 +++++-------------- 3 files changed, 6 insertions(+), 28 deletions(-) diff --git a/rules/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector.php b/rules/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector.php index b6cd1f6d690..73892d533ae 100644 --- a/rules/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector.php +++ b/rules/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector.php @@ -69,13 +69,9 @@ public function refactor(Node $node): ?Node return null; } -<<<<<<< HEAD $oldTokens = $this->getFile() ->getOldTokens(); -======= - $oldTokens = $this->file->getOldTokens(); ->>>>>>> 7556383dcf ([experiment] skip beforeTraverse() and afterTraverse() as never used) if ($this->isIfConditionFollowedByOpeningCurlyBracket($node, $oldTokens)) { return null; } diff --git a/rules/Php85/Rector/ArrayDimFetch/ArrayFirstLastRector.php b/rules/Php85/Rector/ArrayDimFetch/ArrayFirstLastRector.php index 11251db7b56..067d5463399 100644 --- a/rules/Php85/Rector/ArrayDimFetch/ArrayFirstLastRector.php +++ b/rules/Php85/Rector/ArrayDimFetch/ArrayFirstLastRector.php @@ -179,7 +179,6 @@ private function shouldSkip(ArrayDimFetch $arrayDimFetch, Node $scopeNode): bool if ($scope->isInExpressionAssign($arrayDimFetch)) { return true; } -<<<<<<< HEAD if ($arrayDimFetch->getAttribute(AttributeKey::IS_BEING_ASSIGNED) === true) { return true; @@ -189,8 +188,6 @@ private function shouldSkip(ArrayDimFetch $arrayDimFetch, Node $scopeNode): bool return true; } -======= ->>>>>>> ce85ccbfcc (fixup! [experiment] skip beforeTraverse() and afterTraverse() as never used) return (bool) $arrayDimFetch->getAttribute(AttributeKey::IS_UNSET_VAR); } } diff --git a/src/Rector/AbstractRector.php b/src/Rector/AbstractRector.php index 836662d9b6c..177af8b6820 100644 --- a/src/Rector/AbstractRector.php +++ b/src/Rector/AbstractRector.php @@ -35,9 +35,6 @@ use Rector\Skipper\Skipper\Skipper; use Rector\ValueObject\Application\File; -/** - * @property-read File $file @deprecated Use $this->getFile() instead - */ abstract class AbstractRector extends NodeVisitorAbstract implements RectorInterface { private const string EMPTY_NODE_ARRAY_MESSAGE = <<>>>>>> 7556383dcf ([experiment] skip beforeTraverse() and afterTraverse() as never used) protected Skipper $skipper; private ChangedNodeScopeRefresher $changedNodeScopeRefresher; @@ -80,16 +74,6 @@ abstract class AbstractRector extends NodeVisitorAbstract implements RectorInter private CreatedByRuleDecorator $createdByRuleDecorator; - public function __get(string $name): mixed - { - if ($name === 'file') { - return $this->getFile(); - } - - // fallback to default behavior - return $this->{$name}; - } - public function autowire( NodeNameResolver $nodeNameResolver, NodeTypeResolver $nodeTypeResolver, @@ -130,12 +114,14 @@ public function beforeTraverse(array $nodes): ?array */ final public function enterNode(Node $node): int|Node|null|array { - if (is_a($this, HTMLAverseRectorInterface::class, true) && $this->getFile()->containsHTML()) { + // keep $this->file populated for BC; refactor() is only ever reached through here + $file = $this->file = $this->getFile(); + + if (is_a($this, HTMLAverseRectorInterface::class, true) && $file->containsHTML()) { return null; } - $filePath = $this->getFile() - ->getFilePath(); + $filePath = $file->getFilePath(); if ($this->skipper->shouldSkipCurrentNode($this, $filePath, static::class, $node)) { return null; } @@ -168,8 +154,7 @@ final public function enterNode(Node $node): int|Node|null|array // notify this rule changed code $rectorWithLineChange = new RectorWithLineChange(static::class, $originalNode->getStartLine()); - $this->getFile() - ->addRectorClassWithLine($rectorWithLineChange); + $file->addRectorClassWithLine($rectorWithLineChange); return $refactoredNodeOrState; } From 54e25b291b31e9c83e56a8aa03928f66d1e0a3be Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 16 Jun 2026 13:44:24 +0200 Subject: [PATCH 3/3] make beforeTraverse() and afterTraverse() final, to avoid hacking rector rule internals, use refactor() instead --- src/PostRector/Rector/AbstractPostRector.php | 2 +- src/Rector/AbstractRector.php | 26 +++++++++++++------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/PostRector/Rector/AbstractPostRector.php b/src/PostRector/Rector/AbstractPostRector.php index 18736f9fbb3..b12d58aeed9 100644 --- a/src/PostRector/Rector/AbstractPostRector.php +++ b/src/PostRector/Rector/AbstractPostRector.php @@ -29,7 +29,7 @@ public function setFile(File $file): void $this->file = $file; } - public function getFile(): File + protected function getFile(): File { Assert::isInstanceOf($this->file, File::class); diff --git a/src/Rector/AbstractRector.php b/src/Rector/AbstractRector.php index 177af8b6820..36ab2ae666d 100644 --- a/src/Rector/AbstractRector.php +++ b/src/Rector/AbstractRector.php @@ -99,12 +99,21 @@ public function autowire( } /** - * @final Avoid override to prevent unintended side-effects. Use enterNode() or @see \Rector\Contract\PhpParser\DecoratingNodeVisitorInterface instead. - * @internal + * @return Node[]|null * + * @internal + */ + final public function beforeTraverse(array $nodes): ?array + { + return null; + } + + /** * @return Node[]|null + * + * @internal */ - public function beforeTraverse(array $nodes): ?array + final public function afterTraverse(array $nodes) { return null; } @@ -115,13 +124,13 @@ public function beforeTraverse(array $nodes): ?array final public function enterNode(Node $node): int|Node|null|array { // keep $this->file populated for BC; refactor() is only ever reached through here - $file = $this->file = $this->getFile(); + $this->file = $this->getFile(); - if (is_a($this, HTMLAverseRectorInterface::class, true) && $file->containsHTML()) { + if (is_a($this, HTMLAverseRectorInterface::class, true) && $this->file->containsHTML()) { return null; } - $filePath = $file->getFilePath(); + $filePath = $this->file->getFilePath(); if ($this->skipper->shouldSkipCurrentNode($this, $filePath, static::class, $node)) { return null; } @@ -154,7 +163,7 @@ final public function enterNode(Node $node): int|Node|null|array // notify this rule changed code $rectorWithLineChange = new RectorWithLineChange(static::class, $originalNode->getStartLine()); - $file->addRectorClassWithLine($rectorWithLineChange); + $this->file->addRectorClassWithLine($rectorWithLineChange); return $refactoredNodeOrState; } @@ -264,8 +273,7 @@ private function postRefactorProcess( $this->createdByRuleDecorator->decorate($refactoredNode, $originalNode, static::class); $rectorWithLineChange = new RectorWithLineChange(static::class, $originalNode->getStartLine()); - $this->getFile() - ->addRectorClassWithLine($rectorWithLineChange); + $this->file->addRectorClassWithLine($rectorWithLineChange); /** @var MutatingScope|null $currentScope */ $currentScope = $node->getAttribute(AttributeKey::SCOPE);