Skip to content

Commit

Permalink
[Core] Refactor RectifiedAnalyzer: remove exclude _Class (#1754)
Browse files Browse the repository at this point in the history
* [Core] Refactor RectifiedAnalyzer: remove exclude _Class

* [ci-review] Rector Rectify

* phpstan

* [ci-review] Rector Rectify

* clean up

* clean up: the infiniteLoopValidator already use originalNode for created by rule check, next apply to Node

* clean up: do not clone on early skip to avoid performance drop

* [ci-review] Rector Rectify

Co-authored-by: GitHub Action <action@github.com>
  • Loading branch information
samsonasik and actions-user committed Feb 1, 2022
1 parent a51b81c commit 3545abf
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 19 deletions.
34 changes: 22 additions & 12 deletions src/ProcessAnalyzer/RectifiedAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,16 @@
namespace Rector\Core\ProcessAnalyzer;

use PhpParser\Node;
use PhpParser\Node\Stmt\Class_;
use Rector\Core\Contract\Rector\RectorInterface;
use Rector\Core\ValueObject\Application\File;
use Rector\Core\ValueObject\RectifiedNode;
use Rector\NodeNameResolver\NodeNameResolver;
use Rector\NodeTypeResolver\Node\AttributeKey;

/**
* This service verify if the Node already rectified with same Rector rule before current Rector rule with condition
*
* Same Rector Rule <-> Same Node <-> Same File
*
* Limitation:
*
* It only check against Node which not a Class_
*
* which possibly changed by other process.
*/
final class RectifiedAnalyzer
{
Expand All @@ -28,12 +23,13 @@ final class RectifiedAnalyzer
*/
private array $previousFileWithNodes = [];

public function verify(RectorInterface $rector, Node $node, File $currentFile): ?RectifiedNode
{
if ($node instanceof Class_) {
return null;
}
public function __construct(
private readonly NodeNameResolver $nodeNameResolver
) {
}

public function verify(RectorInterface $rector, Node $node, ?Node $originalNode, File $currentFile): ?RectifiedNode
{
$smartFileInfo = $currentFile->getSmartFileInfo();
$realPath = $smartFileInfo->getRealPath();

Expand All @@ -52,6 +48,20 @@ public function verify(RectorInterface $rector, Node $node, File $currentFile):
return null;
}

$createdByRule = $node->getAttribute(AttributeKey::CREATED_BY_RULE) ?? [];
$nodeName = $this->nodeNameResolver->getName($node);
$originalNodeName = $originalNode instanceof Node
? $this->nodeNameResolver->getName($originalNode)
: null;

if (is_string($nodeName) && $nodeName === $originalNodeName && $createdByRule !== [] && ! in_array(
$rector::class,
$createdByRule,
true
)) {
return null;
}

// re-set to refill next
$this->previousFileWithNodes[$realPath] = null;
return $rectifiedNode;
Expand Down
31 changes: 24 additions & 7 deletions src/Rector/AbstractRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@ final public function enterNode(Node $node)
return null;
}

if ($this->shouldSkipCurrentNode($node)) {
$originalNode = $node->getAttribute(AttributeKey::ORIGINAL_NODE);
if ($this->shouldSkipCurrentNode($node, $originalNode)) {
return null;
}

Expand All @@ -229,7 +230,7 @@ final public function enterNode(Node $node)
$this->printDebugApplying();

$originalAttributes = $node->getAttributes();
$originalNode = $node->getAttribute(AttributeKey::ORIGINAL_NODE) ?? clone $node;
$originalNode ??= clone $node;

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

Expand All @@ -239,7 +240,7 @@ final public function enterNode(Node $node)
}

if (is_array($node)) {
$this->createdByRuleDecorator->decorate($originalNode, static::class);
$this->createdByRule($node, $originalNode);

$originalNodeHash = spl_object_hash($originalNode);
$this->nodesToReturn[$originalNodeHash] = $node;
Expand All @@ -258,8 +259,6 @@ final public function enterNode(Node $node)
return $node;
}

$this->createdByRuleDecorator->decorate($node, static::class);

$rectorWithLineChange = new RectorWithLineChange($this::class, $originalNode->getLine());
$this->file->addRectorClassWithLine($rectorWithLineChange);

Expand All @@ -269,11 +268,13 @@ final public function enterNode(Node $node)

// is equals node type? return node early
if ($originalNode::class === $node::class) {
$this->createdByRule($node, $originalNode);
return $node;
}

// is different node type? do not traverse children to avoid looping
$this->infiniteLoopValidator->process($node, $originalNode, static::class);
$this->createdByRuleDecorator->decorate($node, static::class);

// search "infinite recursion" in https://github.com/nikic/PHP-Parser/blob/master/doc/component/Walking_the_AST.markdown
$originalNodeHash = spl_object_hash($originalNode);
Expand Down Expand Up @@ -429,6 +430,22 @@ protected function removeNodes(array $nodes): void
$this->nodeRemover->removeNodes($nodes);
}

/**
* @param array<Node>|Node $node
*/
private function createdByRule(array | Node $node, Node $originalNode): void
{
if ($node instanceof Node) {
$node = [$node];
}

foreach ($node as $singleNode) {
$this->createdByRuleDecorator->decorate($singleNode, static::class);
}

$this->createdByRuleDecorator->decorate($originalNode, static::class);
}

/**
* @param class-string<Node> $nodeClass
*/
Expand All @@ -443,7 +460,7 @@ private function isMatchingNodeType(string $nodeClass): bool
return false;
}

private function shouldSkipCurrentNode(Node $node): bool
private function shouldSkipCurrentNode(Node $node, ?Node $originalNode): bool
{
if ($this->nodesToRemoveCollector->isNodeRemoved($node)) {
return true;
Expand All @@ -458,7 +475,7 @@ private function shouldSkipCurrentNode(Node $node): bool
return true;
}

$rectifiedNode = $this->rectifiedAnalyzer->verify($this, $node, $this->file);
$rectifiedNode = $this->rectifiedAnalyzer->verify($this, $node, $originalNode, $this->file);
return $rectifiedNode instanceof RectifiedNode;
}

Expand Down

0 comments on commit 3545abf

Please sign in to comment.