Skip to content

Commit

Permalink
Ensure reindex in deep stmts before refactor() (#6614)
Browse files Browse the repository at this point in the history
* Ensure reindex in deep stmts

* fix

* fix

* [ci-review] Rector Rectify

---------

Co-authored-by: GitHub Action <[email protected]>
  • Loading branch information
samsonasik and actions-user authored Dec 19, 2024
1 parent 0c2185f commit 89441c4
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 18 deletions.
2 changes: 2 additions & 0 deletions src/DependencyInjection/LazyContainerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@
use Rector\PhpParser\Comparing\NodeComparator;
use Rector\PhpParser\Node\NodeFactory;
use Rector\PhpParser\NodeTraverser\RectorNodeTraverser;
use Rector\PhpParser\NodeTraverser\ReIndexNodeAttributesTraverser;
use Rector\PHPStanStaticTypeMapper\Contract\TypeMapperInterface;
use Rector\PHPStanStaticTypeMapper\PHPStanStaticTypeMapper;
use Rector\PHPStanStaticTypeMapper\TypeMapper\AccessoryLiteralStringTypeMapper;
Expand Down Expand Up @@ -549,6 +550,7 @@ static function (AbstractRector $rector, Container $container): void {
$container->get(CurrentFileProvider::class),
$container->get(CreatedByRuleDecorator::class),
$container->get(ChangedNodeScopeRefresher::class),
$container->get(ReIndexNodeAttributesTraverser::class)
);
}
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\NodeTypeResolver\PHPStan\Scope\Contract\NodeVisitor\ScopeResolverNodeVisitorInterface;
use Rector\PhpParser\Node\CustomNode\FileWithoutNamespace;
use Rector\PHPStan\NodeVisitor\ReIndexNodeAttributeVisitor;
use Rector\PhpParser\NodeTraverser\ReIndexNodeAttributesTraverser;
use Rector\PHPStan\NodeVisitor\UnreachableStatementNodeVisitor;
use Rector\PHPStan\NodeVisitor\WrappedNodeRestoringNodeVisitor;
use Rector\Util\Reflection\PrivatesAccessor;
Expand Down Expand Up @@ -134,7 +134,8 @@ public function __construct(
private ScopeFactory $scopeFactory,
private PrivatesAccessor $privatesAccessor,
private NodeNameResolver $nodeNameResolver,
private ClassAnalyzer $classAnalyzer
private ClassAnalyzer $classAnalyzer,
private ReIndexNodeAttributesTraverser $reIndexNodeAttributesTraverser
) {
$this->nodeTraverser = new NodeTraverser();

Expand Down Expand Up @@ -163,8 +164,7 @@ public function processNodes(
// due to PHPStan doing printing internally on process nodes
// using reindex via NodeVisitor on purpose to ensure reindex happen even on deep node changed
if ($formerMutatingScope instanceof MutatingScope) {
$nodeTraverser = new NodeTraverser(new ReIndexNodeAttributeVisitor());
$stmts = $nodeTraverser->traverse($stmts);
$stmts = $this->reIndexNodeAttributesTraverser->traverse($stmts);
}

$this->nodeTraverser->traverse($stmts);
Expand Down
16 changes: 16 additions & 0 deletions src/PhpParser/NodeTraverser/ReIndexNodeAttributesTraverser.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

declare(strict_types=1);

namespace Rector\PhpParser\NodeTraverser;

use PhpParser\NodeTraverser;
use Rector\PHPStan\NodeVisitor\ReIndexNodeAttributeVisitor;

final class ReIndexNodeAttributesTraverser extends NodeTraverser
{
public function __construct()
{
parent::__construct(new ReIndexNodeAttributeVisitor());
}
}
8 changes: 6 additions & 2 deletions src/Rector/AbstractRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
use PHPStan\Type\ObjectType;
use PHPStan\Type\Type;
use Rector\Application\ChangedNodeScopeRefresher;
use Rector\Application\NodeAttributeReIndexer;
use Rector\Application\Provider\CurrentFileProvider;
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo;
use Rector\ChangesReporting\ValueObject\RectorWithLineChange;
Expand All @@ -34,6 +33,7 @@
use Rector\PhpDocParser\NodeTraverser\SimpleCallableNodeTraverser;
use Rector\PhpParser\Comparing\NodeComparator;
use Rector\PhpParser\Node\NodeFactory;
use Rector\PhpParser\NodeTraverser\ReIndexNodeAttributesTraverser;
use Rector\Skipper\Skipper\Skipper;
use Rector\ValueObject\Application\File;

Expand Down Expand Up @@ -79,6 +79,8 @@ abstract class AbstractRector extends NodeVisitorAbstract implements RectorInter

private CreatedByRuleDecorator $createdByRuleDecorator;

private ReIndexNodeAttributesTraverser $reIndexNodeAttributesTraverser;

private ?int $toBeRemovedNodeId = null;

public function autowire(
Expand All @@ -91,6 +93,7 @@ public function autowire(
CurrentFileProvider $currentFileProvider,
CreatedByRuleDecorator $createdByRuleDecorator,
ChangedNodeScopeRefresher $changedNodeScopeRefresher,
ReIndexNodeAttributesTraverser $reIndexNodeAttributesTraverser
): void {
$this->nodeNameResolver = $nodeNameResolver;
$this->nodeTypeResolver = $nodeTypeResolver;
Expand All @@ -101,6 +104,7 @@ public function autowire(
$this->currentFileProvider = $currentFileProvider;
$this->createdByRuleDecorator = $createdByRuleDecorator;
$this->changedNodeScopeRefresher = $changedNodeScopeRefresher;
$this->reIndexNodeAttributesTraverser = $reIndexNodeAttributesTraverser;
}

/**
Expand Down Expand Up @@ -139,7 +143,7 @@ final public function enterNode(Node $node): int|Node|null
// ensure origNode pulled before refactor to avoid changed during refactor, ref https://3v4l.org/YMEGN
$originalNode = $node->getAttribute(AttributeKey::ORIGINAL_NODE) ?? $node;

NodeAttributeReIndexer::reIndexNodeAttributes($node);
$this->reIndexNodeAttributesTraverser->traverse([$node]);

$refactoredNode = $this->refactor($node);

Expand Down
14 changes: 9 additions & 5 deletions tests/Issues/IndexedStmt/Fixture/fixture.php.inc
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ final class Fixture
{
public function run()
{
'with index 0';
'with index 1';
'with index 2';
if (rand(0, 1)) {
'with index 0';
'with index 1';
'with index 2';
}
}
}

Expand All @@ -22,8 +24,10 @@ final class Fixture
{
public function run()
{
'with index 0';
'final index';
if (rand(0, 1)) {
'with index 0';
'final index';
}
}
}

Expand Down
18 changes: 11 additions & 7 deletions tests/Issues/IndexedStmt/Source/ChangeLastIndex1Rector.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@

use PhpParser\Node;
use PhpParser\Node\Scalar\String_;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Expression;
use PhpParser\NodeVisitor;
use Rector\Contract\PhpParser\Node\StmtsAwareInterface;
use PhpParser\Node\Stmt\If_;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
Expand All @@ -22,11 +22,11 @@ public function getRuleDefinition(): RuleDefinition

public function getNodeTypes(): array
{
return [StmtsAwareInterface::class];
return [ClassMethod::class];
}

/**
* @param StmtsAwareInterface $node
* @param ClassMethod $node
*/
public function refactor(Node $node)
{
Expand All @@ -35,9 +35,13 @@ public function refactor(Node $node)
}

foreach ($node->stmts as $stmt) {
if ($stmt->getAttribute(AttributeKey::STMT_KEY) === 1 && $stmt instanceof Expression && $stmt->expr instanceof String_ && $stmt->expr->value === 'with index 2') {
$stmt->expr->value = 'final index';
return $node;
if ($stmt instanceof If_) {
foreach ($stmt->stmts as $childStmt) {
if ($childStmt->getAttribute(AttributeKey::STMT_KEY) === 1 && $childStmt instanceof Expression && $childStmt->expr instanceof String_ && $childStmt->expr->value === 'with index 2') {
$childStmt->expr->value = 'final index';
return $node;
}
}
}
}

Expand Down

0 comments on commit 89441c4

Please sign in to comment.