mirror of
https://github.com/rectorphp/rector.git
synced 2024-06-07 11:50:51 +00:00
[Core] Refactor InfiniteLoopValidator (#1771)
* [Core] Refactor InfiniteLoopValidator * pass rector class * check found * verify rector class * remove InfiniteLoopTraversingException * remove manual check on rules * test possibly null already transformed for DowngradeJsonDecodeNullAssociativeArgRector * [ci-review] Rector Rectify * [ci-review] Rector Rectify * [ci-review] Rector Rectify * test for skip some method call infinity already transformed * finally, created by rule check can be cleaned up * clean up ALREADY_CHANGED_ON_COUNT attribute check on CountOnNullRector Co-authored-by: GitHub Action <action@github.com>
This commit is contained in:
parent
aa4f45484a
commit
ca5b89baad
|
@ -72,7 +72,6 @@ return static function (ContainerConfigurator $containerConfigurator): void {
|
|||
__DIR__ . '/../src/PhpParser/ValueObject',
|
||||
__DIR__ . '/../src/functions',
|
||||
__DIR__ . '/../src/constants.php',
|
||||
__DIR__ . '/../src/PhpParser/NodeVisitor/CreatedByRuleNodeVisitor.php',
|
||||
]);
|
||||
|
||||
$services->alias(Application::class, ConsoleApplication::class);
|
||||
|
|
|
@ -0,0 +1,18 @@
|
|||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace Rector\Tests\DowngradePhp72\Rector\FuncCall\DowngradeJsonDecodeNullAssociativeArgRector\Fixture;
|
||||
|
||||
class SkipPossiblyNullAlreadyTransformed
|
||||
{
|
||||
function run(string $json, ?bool $associative)
|
||||
{
|
||||
if ($associative === null) {
|
||||
$associative = true;
|
||||
}
|
||||
$value = json_decode($json, $associative);
|
||||
}
|
||||
}
|
||||
|
||||
?>
|
|
@ -70,11 +70,6 @@ CODE_SAMPLE
|
|||
return null;
|
||||
}
|
||||
|
||||
$createdByRule = $node->getAttribute(AttributeKey::CREATED_BY_RULE) ?? [];
|
||||
if (in_array(self::class, $createdByRule, true)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// Ensure the refactoring is idempotent.
|
||||
if ($this->isAlreadyTransformed($node)) {
|
||||
return null;
|
||||
|
|
|
@ -89,11 +89,6 @@ CODE_SAMPLE
|
|||
return null;
|
||||
}
|
||||
|
||||
$createdByRule = $node->getAttribute(AttributeKey::CREATED_BY_RULE) ?? [];
|
||||
if (in_array(self::class, $createdByRule, true)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$args = $node->getArgs();
|
||||
if ($this->argsAnalyzer->hasNamedArg($args)) {
|
||||
return null;
|
||||
|
|
|
@ -27,7 +27,6 @@ use PHPStan\Type\Constant\ConstantStringType;
|
|||
use PHPStan\Type\StringType;
|
||||
use Rector\Core\NodeAnalyzer\ArgsAnalyzer;
|
||||
use Rector\Core\Rector\AbstractRector;
|
||||
use Rector\NodeTypeResolver\Node\AttributeKey;
|
||||
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
|
||||
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
|
||||
|
||||
|
@ -101,11 +100,6 @@ CODE_SAMPLE
|
|||
return null;
|
||||
}
|
||||
|
||||
$createdByRule = $node->getAttribute(AttributeKey::CREATED_BY_RULE) ?? [];
|
||||
if (in_array(self::class, $createdByRule, true)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// direct null check ConstFetch
|
||||
if ($args[1]->value instanceof ConstFetch && $this->valueResolver->isNull($args[1]->value)) {
|
||||
$args = [$args[0]];
|
||||
|
|
|
@ -40,11 +40,6 @@ use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
|
|||
*/
|
||||
final class CountOnNullRector extends AbstractRector implements MinPhpVersionInterface
|
||||
{
|
||||
/**
|
||||
* @var string
|
||||
*/
|
||||
private const ALREADY_CHANGED_ON_COUNT = 'already_changed_on_count';
|
||||
|
||||
public function __construct(
|
||||
private readonly CountableTypeAnalyzer $countableTypeAnalyzer,
|
||||
private readonly CountableAnalyzer $countableAnalyzer,
|
||||
|
@ -121,10 +116,8 @@ CODE_SAMPLE
|
|||
|
||||
if ($this->nodeTypeResolver->isNullableType($countedNode) || $countedType instanceof NullType) {
|
||||
$identical = new Identical($countedNode, $this->nodeFactory->createNull());
|
||||
$ternary = new Ternary($identical, new LNumber(0), $node);
|
||||
// prevent infinity loop re-resolution
|
||||
$node->setAttribute(self::ALREADY_CHANGED_ON_COUNT, true);
|
||||
return $ternary;
|
||||
|
||||
return new Ternary($identical, new LNumber(0), $node);
|
||||
}
|
||||
|
||||
if ($this->phpVersionProvider->isAtLeastPhpVersion(PhpVersionFeature::IS_COUNTABLE)) {
|
||||
|
@ -137,9 +130,6 @@ CODE_SAMPLE
|
|||
), $instanceof);
|
||||
}
|
||||
|
||||
// prevent infinity loop re-resolution
|
||||
$node->setAttribute(self::ALREADY_CHANGED_ON_COUNT, true);
|
||||
|
||||
return new Ternary($conditionNode, $node, new LNumber(0));
|
||||
}
|
||||
|
||||
|
@ -182,13 +172,6 @@ CODE_SAMPLE
|
|||
return true;
|
||||
}
|
||||
|
||||
$alreadyChangedOnCount = (bool) $funcCall->getAttribute(self::ALREADY_CHANGED_ON_COUNT, false);
|
||||
|
||||
// check if it has some condition before already, if so, probably it's already handled
|
||||
if ($alreadyChangedOnCount) {
|
||||
return true;
|
||||
}
|
||||
|
||||
$parentNode = $funcCall->getAttribute(AttributeKey::PARENT_NODE);
|
||||
if ($parentNode instanceof Ternary) {
|
||||
return true;
|
||||
|
|
|
@ -1,11 +0,0 @@
|
|||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace Rector\Core\Exception\NodeTraverser;
|
||||
|
||||
use Exception;
|
||||
|
||||
final class InfiniteLoopTraversingException extends Exception
|
||||
{
|
||||
}
|
|
@ -9,7 +9,23 @@ use Rector\NodeTypeResolver\Node\AttributeKey;
|
|||
|
||||
final class CreatedByRuleDecorator
|
||||
{
|
||||
public function decorate(Node $node, string $rectorClass): void
|
||||
/**
|
||||
* @param array<Node>|Node $node
|
||||
*/
|
||||
public function decorate(array | Node $node, Node $originalNode, string $rectorClass): void
|
||||
{
|
||||
if ($node instanceof Node) {
|
||||
$node = [$node];
|
||||
}
|
||||
|
||||
foreach ($node as $singleNode) {
|
||||
$this->createByRule($singleNode, $rectorClass);
|
||||
}
|
||||
|
||||
$this->createByRule($originalNode, $rectorClass);
|
||||
}
|
||||
|
||||
private function createByRule(Node $node, string $rectorClass): void
|
||||
{
|
||||
$mergeCreatedByRule = array_merge($node->getAttribute(AttributeKey::CREATED_BY_RULE) ?? [], [$rectorClass]);
|
||||
$mergeCreatedByRule = array_unique($mergeCreatedByRule);
|
||||
|
|
|
@ -1,24 +0,0 @@
|
|||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace Rector\Core\PhpParser\NodeVisitor;
|
||||
|
||||
use PhpParser\Node;
|
||||
use PhpParser\NodeVisitorAbstract;
|
||||
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)
|
||||
{
|
||||
$this->createdByRuleDecorator->decorate($node, $this->rectorClass);
|
||||
return $node;
|
||||
}
|
||||
}
|
|
@ -233,24 +233,31 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn
|
|||
|
||||
$originalNode ??= clone $node;
|
||||
|
||||
if (! $this->infiniteLoopValidator->isValid($node, $originalNode, static::class)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$node = $this->refactor($node);
|
||||
|
||||
// nothing to change → continue
|
||||
if ($node === null) {
|
||||
if ($this->isNothingToChange($node)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
/** @var Node|array<Node> $node */
|
||||
if (! $this->infiniteLoopValidator->isValid($node, $originalNode, static::class)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
/** @var Node $originalNode */
|
||||
if (is_array($node)) {
|
||||
$this->createdByRule($node, $originalNode);
|
||||
$this->createdByRuleDecorator->decorate($node, $originalNode, static::class);
|
||||
|
||||
$originalNodeHash = spl_object_hash($originalNode);
|
||||
$this->nodesToReturn[$originalNodeHash] = $node;
|
||||
|
||||
if ($node !== []) {
|
||||
$firstNodeKey = array_key_first($node);
|
||||
$this->mirrorComments($node[$firstNodeKey], $originalNode);
|
||||
}
|
||||
$firstNodeKey = array_key_first($node);
|
||||
$this->mirrorComments($node[$firstNodeKey], $originalNode);
|
||||
|
||||
// will be replaced in leaveNode() the original node must be passed
|
||||
return $originalNode;
|
||||
|
@ -268,16 +275,13 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn
|
|||
$this->mirrorAttributes($originalAttributes, $node);
|
||||
$this->connectParentNodes($node);
|
||||
|
||||
$this->createdByRuleDecorator->decorate($node, $originalNode, static::class);
|
||||
|
||||
// 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);
|
||||
|
||||
|
@ -433,19 +437,15 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn
|
|||
}
|
||||
|
||||
/**
|
||||
* @param array<Node>|Node $node
|
||||
* @param Node|array<Node>|null $node
|
||||
*/
|
||||
private function createdByRule(array | Node $node, Node $originalNode): void
|
||||
private function isNothingToChange(array|Node|null $node): bool
|
||||
{
|
||||
if ($node instanceof Node) {
|
||||
$node = [$node];
|
||||
if ($node === null) {
|
||||
return true;
|
||||
}
|
||||
|
||||
foreach ($node as $singleNode) {
|
||||
$this->createdByRuleDecorator->decorate($singleNode, static::class);
|
||||
}
|
||||
|
||||
$this->createdByRuleDecorator->decorate($originalNode, static::class);
|
||||
return $node === [];
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -5,69 +5,41 @@ declare(strict_types=1);
|
|||
namespace Rector\Core\Validation;
|
||||
|
||||
use PhpParser\Node;
|
||||
use PhpParser\NodeTraverser;
|
||||
use Rector\Core\Contract\Rector\RectorInterface;
|
||||
use Rector\Core\Exception\NodeTraverser\InfiniteLoopTraversingException;
|
||||
use Rector\Core\NodeDecorator\CreatedByRuleDecorator;
|
||||
use Rector\Core\PhpParser\Comparing\NodeComparator;
|
||||
use Rector\Core\PhpParser\Node\BetterNodeFinder;
|
||||
use Rector\Core\PhpParser\NodeVisitor\CreatedByRuleNodeVisitor;
|
||||
use Rector\DowngradePhp74\Rector\ArrowFunction\ArrowFunctionToAnonymousFunctionRector;
|
||||
use Rector\DowngradePhp80\Rector\NullsafeMethodCall\DowngradeNullsafeToTernaryOperatorRector;
|
||||
use Rector\NodeTypeResolver\Node\AttributeKey;
|
||||
use Rector\Php74\Rector\Closure\ClosureToArrowFunctionRector;
|
||||
|
||||
final class InfiniteLoopValidator
|
||||
{
|
||||
/**
|
||||
* @var array<class-string<RectorInterface>>
|
||||
*/
|
||||
private const ALLOWED_INFINITE_RECTOR_CLASSES = [
|
||||
DowngradeNullsafeToTernaryOperatorRector::class,
|
||||
ArrowFunctionToAnonymousFunctionRector::class,
|
||||
ClosureToArrowFunctionRector::class,
|
||||
];
|
||||
|
||||
public function __construct(
|
||||
private readonly BetterNodeFinder $betterNodeFinder,
|
||||
private readonly CreatedByRuleDecorator $createdByRuleDecorator
|
||||
private readonly NodeComparator $nodeComparator,
|
||||
private readonly BetterNodeFinder $betterNodeFinder
|
||||
) {
|
||||
}
|
||||
|
||||
/**
|
||||
* @param class-string<RectorInterface> $rectorClass
|
||||
* @param Node|array<Node> $node
|
||||
*/
|
||||
public function process(Node $node, Node $originalNode, string $rectorClass): void
|
||||
public function isValid(Node|array $node, Node $originalNode, string $rectorClass): bool
|
||||
{
|
||||
if (in_array($rectorClass, self::ALLOWED_INFINITE_RECTOR_CLASSES, true)) {
|
||||
return;
|
||||
if ($this->nodeComparator->areNodesEqual($node, $originalNode)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
$isFound = (bool) $this->betterNodeFinder->findFirst(
|
||||
$node,
|
||||
fn (Node $subNode): bool => $this->nodeComparator->areNodesEqual($node, $subNode)
|
||||
);
|
||||
|
||||
if (! $isFound) {
|
||||
return true;
|
||||
}
|
||||
|
||||
$createdByRule = $originalNode->getAttribute(AttributeKey::CREATED_BY_RULE) ?? [];
|
||||
|
||||
// special case
|
||||
if (in_array($rectorClass, $createdByRule, true)) {
|
||||
// does it contain the same node type as input?
|
||||
$originalNodeClass = $originalNode::class;
|
||||
|
||||
$hasNestedOriginalNodeType = $this->betterNodeFinder->findInstanceOf($node, $originalNodeClass);
|
||||
if ($hasNestedOriginalNodeType !== []) {
|
||||
throw new InfiniteLoopTraversingException($rectorClass);
|
||||
}
|
||||
if ($createdByRule === []) {
|
||||
return true;
|
||||
}
|
||||
|
||||
$this->decorateNode($originalNode, $rectorClass);
|
||||
}
|
||||
|
||||
/**
|
||||
* @param class-string<RectorInterface> $rectorClass
|
||||
*/
|
||||
private function decorateNode(Node $node, string $rectorClass): void
|
||||
{
|
||||
$nodeTraverser = new NodeTraverser();
|
||||
|
||||
$createdByRuleNodeVisitor = new CreatedByRuleNodeVisitor($this->createdByRuleDecorator, $rectorClass);
|
||||
$nodeTraverser->addVisitor($createdByRuleNodeVisitor);
|
||||
|
||||
$nodeTraverser->traverse([$node]);
|
||||
return ! in_array($rectorClass, $createdByRule, true);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -0,0 +1,4 @@
|
|||
<?php
|
||||
|
||||
$dateTime = new DateTime();
|
||||
$dateTime = $dateTime->modify('+1');
|
|
@ -4,7 +4,6 @@ declare(strict_types=1);
|
|||
|
||||
namespace Rector\Core\Tests\Issues\InfiniteLoop;
|
||||
|
||||
use Rector\Core\Exception\NodeTraverser\InfiniteLoopTraversingException;
|
||||
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
|
||||
use Symplify\SmartFileSystem\SmartFileInfo;
|
||||
|
||||
|
@ -12,8 +11,6 @@ final class InfiniteLoopTest extends AbstractRectorTestCase
|
|||
{
|
||||
public function testException(): void
|
||||
{
|
||||
$this->expectException(InfiniteLoopTraversingException::class);
|
||||
|
||||
$fixtureFileInfo = new SmartFileInfo(__DIR__ . '/Fixture/some_method_call_infinity.php.inc');
|
||||
$this->doTestFileInfo($fixtureFileInfo);
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue
Block a user