Skip to content

Commit

Permalink
[Core] Apply CREATED_BY_RULE attribute as array collection of applied…
Browse files Browse the repository at this point in the history
… Rector rule (#1600)

* [Core] Apply CREATED_BY_RULE attribute as array collection of applied Rector rule

* phpstan

* early return

* [ci-review] Rector Rectify

* update comment

* comment

Co-authored-by: GitHub Action <action@github.com>
  • Loading branch information
samsonasik and actions-user committed Dec 31, 2021
1 parent 311ffc6 commit 4399f28
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 37 deletions.
4 changes: 2 additions & 2 deletions rules/DeadCode/NodeAnalyzer/ExprUsedInNextNodeAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ function (Node $node) use ($expr): bool {

private function hasIfChangedByRemoveAlwaysElseRector(If_ $if): bool
{
$createdByRule = $if->getAttribute(AttributeKey::CREATED_BY_RULE);
return $createdByRule === RemoveAlwaysElseRector::class;
$createdByRule = $if->getAttribute(AttributeKey::CREATED_BY_RULE) ?? [];
return in_array(RemoveAlwaysElseRector::class, $createdByRule, true);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ public function refactor(Node $node): FuncCall|Ternary|null
return null;
}

$createdByRule = $node->getAttribute(AttributeKey::CREATED_BY_RULE);
if ($createdByRule === self::class) {
$createdByRule = $node->getAttribute(AttributeKey::CREATED_BY_RULE) ?? [];
if (in_array(self::class, $createdByRule, true)) {
return null;
}

Expand All @@ -106,7 +106,6 @@ public function refactor(Node $node): FuncCall|Ternary|null
$args = [$args[0]];
$node->args = $args;

$node->setAttribute(AttributeKey::CREATED_BY_RULE, self::class);
return $node;
}

Expand All @@ -117,7 +116,6 @@ public function refactor(Node $node): FuncCall|Ternary|null
$node->args[1] = new Arg($this->createNewArgFirstTernary($args));
$node->args[2] = new Arg($this->createNewArgSecondTernary($args));

$node->setAttribute(AttributeKey::CREATED_BY_RULE, self::class);
return $node;
}

Expand Down
3 changes: 0 additions & 3 deletions rules/EarlyReturn/Rector/If_/RemoveAlwaysElseRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
use PhpParser\Node\Stmt\Return_;
use PhpParser\Node\Stmt\Throw_;
use Rector\Core\Rector\AbstractRector;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

Expand Down Expand Up @@ -82,7 +81,6 @@ public function refactor(Node $node): ?Node
$originalNode = clone $node;
$if = new If_($node->cond);
$if->stmts = $node->stmts;
$node->setAttribute(AttributeKey::CREATED_BY_RULE, self::class);

$this->nodesToAddCollector->addNodeBeforeNode($if, $node);
$this->mirrorComments($if, $node);
Expand Down Expand Up @@ -110,7 +108,6 @@ public function refactor(Node $node): ?Node
$this->nodesToAddCollector->addNodesAfterNode($node->else->stmts, $node);
$node->else = null;

$node->setAttribute(AttributeKey::CREATED_BY_RULE, self::class);
return $node;
}

Expand Down
19 changes: 19 additions & 0 deletions src/NodeDecorator/CreatedByRuleDecorator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

declare(strict_types=1);

namespace Rector\Core\NodeDecorator;

use PhpParser\Node;
use Rector\NodeTypeResolver\Node\AttributeKey;

final class CreatedByRuleDecorator
{
public function decorate(Node $node, string $rectorClass): void
{
$mergeCreatedByRule = array_merge($node->getAttribute(AttributeKey::CREATED_BY_RULE) ?? [], [$rectorClass]);
$mergeCreatedByRule = array_unique($mergeCreatedByRule);

$node->setAttribute(AttributeKey::CREATED_BY_RULE, $mergeCreatedByRule);
}
}
5 changes: 3 additions & 2 deletions src/PhpParser/NodeVisitor/CreatedByRuleNodeVisitor.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,19 @@

use PhpParser\Node;
use PhpParser\NodeVisitorAbstract;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\Core\NodeDecorator\CreatedByRuleDecorator;

final class CreatedByRuleNodeVisitor extends NodeVisitorAbstract
{
public function __construct(
private readonly CreatedByRuleDecorator $createdByRuleDecorator,
private readonly string $rectorClass
) {
}

public function enterNode(Node $node)
{
$node->setAttribute(AttributeKey::CREATED_BY_RULE, $this->rectorClass);
$this->createdByRuleDecorator->decorate($node, $this->rectorClass);
return $node;
}
}
56 changes: 34 additions & 22 deletions src/Rector/AbstractRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
use Rector\Core\Exclusion\ExclusionManager;
use Rector\Core\Logging\CurrentRectorProvider;
use Rector\Core\NodeAnalyzer\ChangedNodeAnalyzer;
use Rector\Core\NodeDecorator\CreatedByRuleDecorator;
use Rector\Core\Php\PhpVersionProvider;
use Rector\Core\PhpParser\Comparing\NodeComparator;
use Rector\Core\PhpParser\Node\BetterNodeFinder;
Expand Down Expand Up @@ -127,6 +128,8 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn

private RectifiedAnalyzer $rectifiedAnalyzer;

private CreatedByRuleDecorator $createdByRuleDecorator;

#[Required]
public function autowire(
NodesToRemoveCollector $nodesToRemoveCollector,
Expand All @@ -153,7 +156,8 @@ public function autowire(
CurrentFileProvider $currentFileProvider,
ChangedNodeAnalyzer $changedNodeAnalyzer,
InfiniteLoopValidator $infiniteLoopValidator,
RectifiedAnalyzer $rectifiedAnalyzer
RectifiedAnalyzer $rectifiedAnalyzer,
CreatedByRuleDecorator $createdByRuleDecorator
): void {
$this->nodesToRemoveCollector = $nodesToRemoveCollector;
$this->nodesToAddCollector = $nodesToAddCollector;
Expand All @@ -180,6 +184,7 @@ public function autowire(
$this->changedNodeAnalyzer = $changedNodeAnalyzer;
$this->infiniteLoopValidator = $infiniteLoopValidator;
$this->rectifiedAnalyzer = $rectifiedAnalyzer;
$this->createdByRuleDecorator = $createdByRuleDecorator;
}

/**
Expand Down Expand Up @@ -228,7 +233,14 @@ final public function enterNode(Node $node)

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

// nothing to change → continue
if ($node === null) {
return null;
}

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

$originalNodeHash = spl_object_hash($originalNode);
$this->nodesToReturn[$originalNodeHash] = $node;

Expand All @@ -241,37 +253,37 @@ final public function enterNode(Node $node)
return $originalNode;
}

// nothing to change → continue
if (! $node instanceof Node) {
return null;
// not changed, return node early
if (! $this->changedNodeAnalyzer->hasNodeChanged($originalNode, $node)) {
return $node;
}

// changed!
if ($this->changedNodeAnalyzer->hasNodeChanged($originalNode, $node)) {
$rectorWithLineChange = new RectorWithLineChange($this::class, $originalNode->getLine());
$this->file->addRectorClassWithLine($rectorWithLineChange);
$this->createdByRuleDecorator->decorate($node, static::class);

// update parents relations - must run before connectParentNodes()
$this->mirrorAttributes($originalAttributes, $node);
$this->connectParentNodes($node);
$rectorWithLineChange = new RectorWithLineChange($this::class, $originalNode->getLine());
$this->file->addRectorClassWithLine($rectorWithLineChange);

// is different node type? do not traverse children to avoid looping
if ($originalNode::class !== $node::class) {
$this->infiniteLoopValidator->process($node, $originalNode, static::class);
// update parents relations - must run before connectParentNodes()
$this->mirrorAttributes($originalAttributes, $node);
$this->connectParentNodes($node);

// search "infinite recursion" in https://github.com/nikic/PHP-Parser/blob/master/doc/component/Walking_the_AST.markdown
$originalNodeHash = spl_object_hash($originalNode);
// is equals node type? return node early
if ($originalNode::class === $node::class) {
return $node;
}

if ($originalNode instanceof Stmt && $node instanceof Expr) {
$node = new Expression($node);
}
// is different node type? do not traverse children to avoid looping
$this->infiniteLoopValidator->process($node, $originalNode, static::class);

$this->nodesToReturn[$originalNodeHash] = $node;
// search "infinite recursion" in https://github.com/nikic/PHP-Parser/blob/master/doc/component/Walking_the_AST.markdown
$originalNodeHash = spl_object_hash($originalNode);

return $node;
}
if ($originalNode instanceof Stmt && $node instanceof Expr) {
$node = new Expression($node);
}

$this->nodesToReturn[$originalNodeHash] = $node;

return $node;
}

Expand Down
10 changes: 6 additions & 4 deletions src/Validation/InfiniteLoopValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use PhpParser\NodeTraverser;
use Rector\Core\Contract\Rector\RectorInterface;
use Rector\Core\Exception\NodeTraverser\InfiniteLoopTraversingException;
use Rector\Core\NodeDecorator\CreatedByRuleDecorator;
use Rector\Core\PhpParser\Node\BetterNodeFinder;
use Rector\Core\PhpParser\NodeVisitor\CreatedByRuleNodeVisitor;
use Rector\DowngradePhp74\Rector\ArrowFunction\ArrowFunctionToAnonymousFunctionRector;
Expand All @@ -27,7 +28,8 @@ final class InfiniteLoopValidator
];

public function __construct(
private readonly BetterNodeFinder $betterNodeFinder
private readonly BetterNodeFinder $betterNodeFinder,
private readonly CreatedByRuleDecorator $createdByRuleDecorator
) {
}

Expand All @@ -40,10 +42,10 @@ public function process(Node $node, Node $originalNode, string $rectorClass): vo
return;
}

$createdByRule = $originalNode->getAttribute(AttributeKey::CREATED_BY_RULE);
$createdByRule = $originalNode->getAttribute(AttributeKey::CREATED_BY_RULE) ?? [];

// special case
if ($createdByRule === $rectorClass) {
if (in_array($rectorClass, $createdByRule, true)) {
// does it contain the same node type as input?
$originalNodeClass = $originalNode::class;

Expand All @@ -63,7 +65,7 @@ private function decorateNode(Node $node, string $rectorClass): void
{
$nodeTraverser = new NodeTraverser();

$createdByRuleNodeVisitor = new CreatedByRuleNodeVisitor($rectorClass);
$createdByRuleNodeVisitor = new CreatedByRuleNodeVisitor($this->createdByRuleDecorator, $rectorClass);
$nodeTraverser->addVisitor($createdByRuleNodeVisitor);

$nodeTraverser->traverse([$node]);
Expand Down

0 comments on commit 4399f28

Please sign in to comment.