Skip to content

Commit 17f8d5c

Browse files
TomasVotrubaTomasVotruba
authored
[internal] Remove duplicated enterNode() type check already handled in node traverser (#7717)
* avoid double checking node type * init test * add simple test to allow node change type
1 parent e1fdfaa commit 17f8d5c

File tree

9 files changed

+155
-18
lines changed

9 files changed

+155
-18
lines changed

‎composer.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
"require": {
1515
"php": "^8.2",
1616
"clue/ndjson-react": "^1.3",
17-
"composer/pcre": "^3.3.0",
17+
"composer/pcre": "^3.3.2",
1818
"composer/semver": "^3.4",
1919
"composer/xdebug-handler": "^3.0.5",
2020
"doctrine/inflector": "^2.1",
@@ -50,7 +50,7 @@
5050
"phpstan/phpstan-phpunit": "^2.0",
5151
"phpstan/phpstan-webmozart-assert": "^2.0",
5252
"phpunit/phpunit": "^11.5",
53-
"rector/jack": "^0.4.0",
53+
"rector/jack": "^0.4",
5454
"rector/release-notes-generator": "^0.5",
5555
"rector/swiss-knife": "^2.3",
5656
"rector/type-perfect": "^2.1",

‎phpstan.neon‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,11 @@ parameters:
433433
- rules/Php55/Rector/String_/StringClassNameToClassConstantRector.php
434434
- rules/Php81/Enum/AttributeName.php
435435

436+
-
437+
identifier: symplify.seeAnnotationToTest
438+
paths:
439+
- tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/Class_
440+
436441
# deprecated rule
437442
- '#Rule Rector\\Php81\\Rector\\Array_\\FirstClassCallableRector must implements Rector\\VersionBonding\\Contract\\MinPhpVersionInterface#'
438443
- '#Register "Rector\\Php81\\Rector\\Array_\\FirstClassCallableRector" service to "php81\.php" config set#'

‎src/PhpParser/NodeTraverser/AbstractImmutableNodeTraverser.php‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,13 +101,22 @@ protected function traverseNode(Node $node): void
101101
$traverseChildren = true;
102102
$visitorIndex = -1;
103103
$currentNodeVisitors = $this->getVisitorsForNode($subNode);
104+
104105
foreach ($currentNodeVisitors as $visitorIndex => $visitor) {
105106
$return = $visitor->enterNode($subNode);
106107
if ($return !== null) {
107108
if ($return instanceof Node) {
109+
$originalSubNodeClass = $subNode::class;
110+
108111
$this->ensureReplacementReasonable($subNode, $return);
109112
$subNode = $return;
110113
$node->{$name} = $return;
114+
115+
if ($originalSubNodeClass !== $subNode::class) {
116+
// stop traversing as node type changed and visitors won't work
117+
return;
118+
}
119+
111120
} elseif ($return === NodeVisitor::DONT_TRAVERSE_CHILDREN) {
112121
$traverseChildren = false;
113122
} elseif ($return === NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN) {
@@ -177,12 +186,19 @@ protected function traverseArray(array $nodes): array
177186
$traverseChildren = true;
178187
$visitorIndex = -1;
179188
$currentNodeVisitors = $this->getVisitorsForNode($node);
189+
180190
foreach ($currentNodeVisitors as $visitorIndex => $visitor) {
181191
$return = $visitor->enterNode($node);
182192
if ($return !== null) {
183193
if ($return instanceof Node) {
194+
$originalNodeNodeClass = $node::class;
184195
$this->ensureReplacementReasonable($node, $return);
185196
$nodes[$i] = $node = $return;
197+
198+
if ($originalNodeNodeClass !== $return::class) {
199+
// stop traversing as node type changed and visitors won't work
200+
return $nodes;
201+
}
186202
} elseif (\is_array($return)) {
187203
$doNodes[] = [$i, $return];
188204
continue 2;

‎src/PhpParser/NodeTraverser/RectorNodeTraverser.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ public function getVisitorsForNode(Node $node): array
6767

6868
if (! isset($this->visitorsPerNodeClass[$nodeClass])) {
6969
$this->visitorsPerNodeClass[$nodeClass] = [];
70+
7071
/** @var RectorInterface $visitor */
7172
foreach ($this->visitors as $visitor) {
7273
foreach ($visitor->getNodeTypes() as $nodeType) {
@@ -94,6 +95,7 @@ private function prepareNodeVisitors(): void
9495

9596
// filer out by version
9697
$this->visitors = $this->phpVersionedFilter->filter($this->rectors);
98+
9799
// filter by configuration
98100
$this->visitors = $this->configurationRuleFilter->filter($this->visitors);
99101

‎src/Rector/AbstractRector.php‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -132,10 +132,6 @@ public function beforeTraverse(array $nodes): ?array
132132
*/
133133
final public function enterNode(Node $node): int|Node|null
134134
{
135-
if (! $this->isMatchingNodeType($node)) {
136-
return null;
137-
}
138-
139135
if (is_a($this, HTMLAverseRectorInterface::class, true) && $this->file->containsHTML()) {
140136
return null;
141137
}
@@ -317,16 +313,4 @@ private function refreshScopeNodes(array | Node $node, string $filePath, ?Mutati
317313
$this->changedNodeScopeRefresher->refresh($node, $filePath, $mutatingScope);
318314
}
319315
}
320-
321-
private function isMatchingNodeType(Node $node): bool
322-
{
323-
$nodeClass = $node::class;
324-
foreach ($this->getNodeTypes() as $nodeType) {
325-
if (is_a($nodeClass, $nodeType, true)) {
326-
return true;
327-
}
328-
}
329-
330-
return false;
331-
}
332316
}

‎tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/Class_/RuleChangingClassToTraitRector.php‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Tests\PhpParser\NodeTraverser\StopTraverseOnTypeChange\Class_;
6+
7+
use PhpParser\Node;
8+
use PhpParser\Node\Name;
9+
use PhpParser\Node\Stmt\Class_;
10+
use PhpParser\Node\Stmt\Trait_;
11+
use Rector\Rector\AbstractRector;
12+
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
13+
14+
final class RuleChangingClassToTraitRector extends AbstractRector
15+
{
16+
public function getRuleDefinition(): RuleDefinition
17+
{
18+
return new RuleDefinition('Change node from class to trait', []);
19+
}
20+
21+
public function getNodeTypes(): array
22+
{
23+
return [Class_::class];
24+
}
25+
26+
/**
27+
* @param Class_ $node
28+
*/
29+
public function refactor(Node $node): Trait_
30+
{
31+
$trait = new Trait_('SomeTrait');
32+
$trait->namespacedName = new Name('SomeNamespace\SomeTrait');
33+
34+
return $trait;
35+
}
36+
}

‎tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/Class_/RuleCheckingClassRector.php‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Tests\PhpParser\NodeTraverser\StopTraverseOnTypeChange\Class_;
6+
7+
use PhpParser\Node;
8+
use PhpParser\Node\Stmt\Class_;
9+
use Rector\Rector\AbstractRector;
10+
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
11+
use Webmozart\Assert\Assert;
12+
13+
final class RuleCheckingClassRector extends AbstractRector
14+
{
15+
public function getRuleDefinition(): RuleDefinition
16+
{
17+
return new RuleDefinition('Make sure input is class', []);
18+
}
19+
20+
public function getNodeTypes(): array
21+
{
22+
return [Class_::class];
23+
}
24+
25+
/**
26+
* @param Class_ $node
27+
* @return Class_
28+
*/
29+
public function refactor(Node $node): Node
30+
{
31+
Assert::isInstanceOf($node, Class_::class);
32+
33+
return $node;
34+
}
35+
}

‎tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/Fixture/SimpleClass.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Tests\PhpParser\NodeTraverser\StopTraverseOnTypeChange\Fixture;
6+
7+
final class SimpleClass
8+
{
9+
}

‎tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/StopTraverseOnTypeChangeTest.php‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Tests\PhpParser\NodeTraverser\StopTraverseOnTypeChange;
6+
7+
use PhpParser\Node\Stmt\Class_;
8+
use PhpParser\Node\Stmt\Trait_;
9+
use PhpParser\NodeFinder;
10+
use Rector\PhpParser\NodeTraverser\RectorNodeTraverser;
11+
use Rector\Testing\PHPUnit\AbstractLazyTestCase;
12+
use Rector\Testing\TestingParser\TestingParser;
13+
use Rector\Tests\PhpParser\NodeTraverser\StopTraverseOnTypeChange\Class_\RuleChangingClassToTraitRector;
14+
use Rector\Tests\PhpParser\NodeTraverser\StopTraverseOnTypeChange\Class_\RuleCheckingClassRector;
15+
16+
final class StopTraverseOnTypeChangeTest extends AbstractLazyTestCase
17+
{
18+
private RectorNodeTraverser $rectorNodeTraverser;
19+
20+
private TestingParser $testingParser;
21+
22+
protected function setUp(): void
23+
{
24+
parent::setUp();
25+
26+
$this->rectorNodeTraverser = $this->make(RectorNodeTraverser::class);
27+
28+
$this->rectorNodeTraverser->refreshPhpRectors([
29+
$this->make(RuleChangingClassToTraitRector::class),
30+
$this->make(RuleCheckingClassRector::class),
31+
]);
32+
33+
$this->testingParser = $this->make(TestingParser::class);
34+
}
35+
36+
public function testGetVisitorsForNodeWhenNoVisitorsAvailable(): void
37+
{
38+
// must be cloned + Scope set to allow node replacement
39+
$nodes = $this->testingParser->parseFileToDecoratedNodes(__DIR__ . '/Fixture/SimpleClass.php');
40+
41+
$changedNodes = $this->rectorNodeTraverser->traverse($nodes);
42+
43+
$nodeFinder = new NodeFinder();
44+
$classes = $nodeFinder->findInstanceOf($changedNodes, Class_::class);
45+
$this->assertCount(0, $classes);
46+
47+
$traits = $nodeFinder->findInstanceOf($changedNodes, Trait_::class);
48+
$this->assertCount(1, $traits);
49+
}
50+
}

0 commit comments

Comments
 (0)