diff --git a/phpunit.xml b/phpunit.xml index e927ad4806a..37c4eb12d7f 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -20,7 +20,5 @@ - - diff --git a/src/Application/ChangedNodeScopeRefresher.php b/src/Application/ChangedNodeScopeRefresher.php index 2fff52d0abb..954edadc8e4 100644 --- a/src/Application/ChangedNodeScopeRefresher.php +++ b/src/Application/ChangedNodeScopeRefresher.php @@ -15,14 +15,19 @@ use PhpParser\Node\Expr; use PhpParser\Node\Expr\Array_; use PhpParser\Node\Expr\Closure; +use PhpParser\Node\Expr\ConstFetch; use PhpParser\Node\Expr\New_; +use PhpParser\Node\Name; use PhpParser\Node\Param; use PhpParser\Node\PropertyItem; use PhpParser\Node\StaticVar; use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\Declare_; +use PhpParser\Node\Stmt\Else_; +use PhpParser\Node\Stmt\ElseIf_; use PhpParser\Node\Stmt\Expression; +use PhpParser\Node\Stmt\If_; use PhpParser\Node\Stmt\Property; use PhpParser\Node\Stmt\Static_; use PhpParser\Node\Stmt\Use_; @@ -80,6 +85,22 @@ public function refresh(Node $node, string $filePath, ?MutatingScope $mutatingSc */ private function resolveStmts(Node $node): array { + // Else_/ElseIf_ are not processed standalone by PHPStan, only as part of an If_, so their body + // stays unscoped; wrap into an If_ so processNodes() scopes the real statements they hold + if ($node instanceof Else_) { + return [ + new If_(new ConstFetch(new Name('true')), [ + 'stmts' => $node->stmts, + ])]; + } + + if ($node instanceof ElseIf_) { + return [ + new If_($node->cond, [ + 'stmts' => $node->stmts, + ])]; + } + if ($node instanceof Stmt) { return [$node]; } diff --git a/src/Application/FileProcessor.php b/src/Application/FileProcessor.php index 8d047b05414..91f2ec44a1c 100644 --- a/src/Application/FileProcessor.php +++ b/src/Application/FileProcessor.php @@ -22,7 +22,6 @@ use Rector\PhpParser\Parser\RectorParser; use Rector\PhpParser\Printer\BetterStandardPrinter; use Rector\PostRector\Application\PostFileProcessor; -use Rector\PostRector\Contract\Rector\PostRectorInterface; use Rector\Testing\PHPUnit\StaticPHPUnitEnvironment; use Rector\ValueObject\Application\File; use Rector\ValueObject\Configuration; @@ -60,9 +59,7 @@ public function processFile(File $file, Configuration $configuration): FileProce $fileHasChanged = false; $filePath = $file->getFilePath(); - $previousPassRectorClasses = []; do { - $ruleChangesBefore = count($file->getRectorWithLineChanges()); $file->changeHasChanged(false); // 1. change nodes with Rector Rules @@ -78,17 +75,6 @@ public function processFile(File $file, Configuration $configuration): FileProce // important to detect if file has changed $this->printFile($file, $configuration, $filePath); - // in rector-src tests, fail a rule that re-applies to its own previous-pass output; - // such a rule is not finished in a single pass and must handle the nested/chained case itself. - // gated by env so sibling packages pulling rector-src as dev-main are not affected - if (StaticPHPUnitEnvironment::isPHPUnitRun() && getenv('RECTOR_ENFORCE_SINGLE_PASS') === '1') { - $previousPassRectorClasses = $this->ensureRectorsAreIdempotent( - $file, - $ruleChangesBefore, - $previousPassRectorClasses - ); - } - // no change in current iteration, stop if (! $file->hasChanged()) { break; @@ -122,42 +108,6 @@ public function processFile(File $file, Configuration $configuration): FileProce return new FileProcessResult([], $file->getFileDiff(), $file->hasChanged()); } - /** - * @param array $previousPassRectorClasses rule classes that changed the file in the previous pass - * @return array rule classes that changed the file in the current pass - */ - private function ensureRectorsAreIdempotent(File $file, int $ruleChangesBefore, array $previousPassRectorClasses): array - { - $currentPassRectorClasses = []; - $rectorWithLineChanges = $file->getRectorWithLineChanges(); - - foreach ($rectorWithLineChanges as $key => $rectorWithLineChange) { - if ($key < $ruleChangesBefore) { - continue; - } - - $rectorClass = $rectorWithLineChange->getRectorClass(); - - // post rectors (imports) legitimately converge over several passes - if (is_a($rectorClass, PostRectorInterface::class, true)) { - continue; - } - - $currentPassRectorClasses[$rectorClass] = true; - - if (isset($previousPassRectorClasses[$rectorClass])) { - throw new ShouldNotHappenException(sprintf( - 'The "%s" rule changed "%s" again after it already changed it in the previous pass. ' . - 'A rule must reach its final result in a single pass; handle the nested or chained case inside refactor() instead of relying on the file being re-run.', - $rectorClass, - $file->getFilePath() - )); - } - } - - return $currentPassRectorClasses; - } - private function parseFileAndDecorateNodes(File $file): ?SystemError { try { diff --git a/src/NodeDecorator/CreatedByRuleDecorator.php b/src/NodeDecorator/CreatedByRuleDecorator.php deleted file mode 100644 index 0843e006e6e..00000000000 --- a/src/NodeDecorator/CreatedByRuleDecorator.php +++ /dev/null @@ -1,63 +0,0 @@ -|Node $node - * @param class-string $rectorClass - */ - public function decorate(array|Node $node, Node $originalNode, string $rectorClass): void - { - if ($node instanceof Node && $node === $originalNode) { - $this->createByRule($node, $rectorClass); - return; - } - - if ($node instanceof Node) { - $node = [$node]; - } - - foreach ($node as $singleNode) { - if ($singleNode::class === $originalNode::class) { - $this->createByRule($singleNode, $rectorClass); - } - } - - $this->createByRule($originalNode, $rectorClass); - } - - /** - * @param class-string $rectorClass - */ - private function createByRule(Node $node, string $rectorClass): void - { - /** @var class-string[] $createdByRule */ - $createdByRule = $node->getAttribute(AttributeKey::CREATED_BY_RULE) ?? []; - - // empty array, insert - if ($createdByRule === []) { - $node->setAttribute(AttributeKey::CREATED_BY_RULE, [$rectorClass]); - return; - } - - // consecutive, no need to refill - if (end($createdByRule) === $rectorClass) { - return; - } - - // filter out when exists, then append - $createdByRule = array_filter( - $createdByRule, - static fn (string $rectorRule): bool => $rectorRule !== $rectorClass - ); - $node->setAttribute(AttributeKey::CREATED_BY_RULE, [...$createdByRule, $rectorClass]); - } -} diff --git a/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php b/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php index 780e7c2e0ee..ef8994833c7 100644 --- a/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php +++ b/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php @@ -91,6 +91,7 @@ use PHPStan\Node\VirtualNode; use PHPStan\Parser\ParserErrorsException; use PHPStan\PhpDocParser\Parser\ParserException; +use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\ReflectionProvider; use PHPStan\ShouldNotHappenException; use PHPStan\Type\ObjectType; @@ -636,6 +637,13 @@ private function resolveClassOrInterfaceScope( } catch (ShouldNotHappenException) { } + // enterClass() throws when the scope is already inside a class, which happens when a changed + // class node is re-processed on a later pass; keep the in-class scope instead of dropping it + $currentClassReflection = $mutatingScope->getClassReflection(); + if ($currentClassReflection instanceof ClassReflection && $currentClassReflection->getName() === $classReflection->getName()) { + return $mutatingScope; + } + $context = $this->privatesAccessor->getPrivateProperty($mutatingScope, 'context'); $this->privatesAccessor->setPrivateProperty($context, 'classReflection', null); diff --git a/src/Rector/RectorRunner.php b/src/Rector/RectorRunner.php index 5f0e8224aa3..ff17126bbfe 100644 --- a/src/Rector/RectorRunner.php +++ b/src/Rector/RectorRunner.php @@ -12,9 +12,7 @@ use Rector\ChangesReporting\ValueObject\RectorWithLineChange; use Rector\Contract\Rector\RectorInterface; use Rector\Exception\ShouldNotHappenException; -use Rector\NodeDecorator\CreatedByRuleDecorator; use Rector\NodeTypeResolver\Node\AttributeKey; -use Rector\Skipper\Skipper\Skipper; use Rector\ValueObject\Application\File; /** @@ -38,8 +36,6 @@ CODE_SAMPLE; public function __construct( - private Skipper $skipper, - private CreatedByRuleDecorator $createdByRuleDecorator, private ChangedNodeScopeRefresher $changedNodeScopeRefresher, private CurrentFileProvider $currentFileProvider, ) { @@ -53,11 +49,6 @@ public function run(RectorInterface $rector, Node $node): int|Node|null|array $file = $this->getFile(); $filePath = $file->getFilePath(); - // node already changed by this rule in a previous pass → hard skip - if ($this->skipper->shouldSkipCurrentNode($rector::class, $node)) { - return null; - } - // ensure origNode pulled before refactor to avoid changed during refactor, ref https://3v4l.org/YMEGN $originalNode = $node->getAttribute(AttributeKey::ORIGINAL_NODE) ?? $node; @@ -74,8 +65,6 @@ public function run(RectorInterface $rector, Node $node): int|Node|null|array } if (is_int($refactoredNodeOrState)) { - $this->createdByRuleDecorator->decorate($node, $originalNode, $rector::class); - // only remove node is supported if ($refactoredNodeOrState !== NodeVisitor::REMOVE_NODE) { throw new ShouldNotHappenException(sprintf('Unsupported state "%d" returned from "%s".', $refactoredNodeOrState, $rector::class)); @@ -116,8 +105,6 @@ private function postRefactorProcess( string $filePath ): Node|array { /** @var non-empty-array|Node $refactoredNode */ - $this->createdByRuleDecorator->decorate($refactoredNode, $originalNode, $rector::class); - $rectorWithLineChange = new RectorWithLineChange($rector::class, $originalNode->getStartLine()); $file->addRectorClassWithLine($rectorWithLineChange);