Skip to content

Commit

Permalink
[Core] Handle Scope not available on DowngradeArrayIsListRector+Downg…
Browse files Browse the repository at this point in the history
…radePregUnmatchedAsNullConstantRector (#2556)

* [Core] Handle Scope not avaiable on DowngradeArrayIsListRector+DowngradePregUnmatchedAsNullConstantRector

* Fixed 🎉

* final touch: clean up set scope on addNodesBeforeNode as already in addNodeBeforeNode, also avoid repetitive call getNode node on loop on addNodesAfterNode

* really final touch: clean up addNodesAfterNode like in addNodesBeforeNode

* final touch: use else for not pass SmartFileInfo

* really final touch: make consistent to always use ChangedNodeScopeRefresher

* final touch: remove unnecessary pass SmartFileInfo

* really final touch: make consistent for getAttributes() check

* [ci-review] Rector Rectify

* [ci-review] Rector Rectify

* final touch: restructure parameter on refresh so make smartFileInfo optional to be pulled from CurrentFileProvider when null

Co-authored-by: GitHub Action <action@github.com>
  • Loading branch information
samsonasik and actions-user committed Jun 25, 2022
1 parent 237f603 commit dcad222
Show file tree
Hide file tree
Showing 25 changed files with 119 additions and 71 deletions.
22 changes: 11 additions & 11 deletions packages/PostRector/Collector/NodesToAddCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
use Rector\Core\PhpParser\Node\BetterNodeFinder;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\PostRector\Contract\Collector\NodeCollectorInterface;
use Symplify\SmartFileSystem\SmartFileInfo;

final class NodesToAddCollector implements NodeCollectorInterface
{
Expand Down Expand Up @@ -46,19 +45,14 @@ public function isActive(): bool
/**
* @deprecated Return created nodes right in refactor() method to keep context instead.
*/
public function addNodeBeforeNode(Node $addedNode, Node $positionNode, ?SmartFileInfo $smartFileInfo = null): void
public function addNodeBeforeNode(Node $addedNode, Node $positionNode): void
{
if ($positionNode->getAttributes() === []) {
$message = sprintf('Switch arguments in "%s()" method', __METHOD__);
throw new ShouldNotHappenException($message);
}

// @todo the node must be returned here, so traverser can refresh it
// this is nasty hack to verify it works
if ($smartFileInfo instanceof SmartFileInfo) {
$currentScope = $positionNode->getAttribute(AttributeKey::SCOPE);
$this->changedNodeScopeRefresher->refresh($addedNode, $smartFileInfo, $currentScope);
}
$this->changedNodeScopeRefresher->refresh($addedNode, $positionNode->getAttribute(AttributeKey::SCOPE));

$position = $this->resolveNearestStmtPosition($positionNode);
$this->nodesToAddBefore[$position][] = $this->wrapToExpression($addedNode);
Expand All @@ -72,11 +66,10 @@ public function addNodeBeforeNode(Node $addedNode, Node $positionNode, ?SmartFil
*/
public function addNodesAfterNode(array $addedNodes, Node $positionNode): void
{
$position = $this->resolveNearestStmtPosition($positionNode);
foreach ($addedNodes as $addedNode) {
// prevent fluent method weird indent
$addedNode->setAttribute(AttributeKey::ORIGINAL_NODE, null);
$this->nodesToAddAfter[$position][] = $this->wrapToExpression($addedNode);
$this->addNodeAfterNode($addedNode, $positionNode);
}

$this->rectorChangeCollector->notifyNodeFileInfo($positionNode);
Expand All @@ -88,6 +81,13 @@ public function addNodesAfterNode(array $addedNodes, Node $positionNode): void
*/
public function addNodeAfterNode(Node $addedNode, Node $positionNode): void
{
if ($positionNode->getAttributes() === []) {
$message = sprintf('Switch arguments in "%s()" method', __METHOD__);
throw new ShouldNotHappenException($message);
}

$this->changedNodeScopeRefresher->refresh($addedNode, $positionNode->getAttribute(AttributeKey::SCOPE));

$position = $this->resolveNearestStmtPosition($positionNode);
$this->nodesToAddAfter[$position][] = $this->wrapToExpression($addedNode);

Expand Down Expand Up @@ -131,7 +131,7 @@ public function clearNodesToAddBefore(Node $node): void
public function addNodesBeforeNode(array $newNodes, Node $positionNode): void
{
foreach ($newNodes as $newNode) {
$this->addNodeBeforeNode($newNode, $positionNode, null);
$this->addNodeBeforeNode($newNode, $positionNode);
}

$this->rectorChangeCollector->notifyNodeFileInfo($positionNode);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ private function refactorAssignedArray(Assign $assign, FuncCall $funcCall, Expr
$assignVariable = $firstArg->value;
$preAssign = new Assign($assignVariable, $array);

$this->nodesToAddCollector->addNodeBeforeNode($preAssign, $currentStmt, $this->file->getSmartFileInfo());
$this->nodesToAddCollector->addNodeBeforeNode($preAssign, $currentStmt);

return $expr;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public function refactor(Node $node): ?Node

$expression = new Expression(new Assign($selfVariable, new Variable('this')));

$this->nodesToAddCollector->addNodeBeforeNode($expression, $node, $this->file->getSmartFileInfo());
$this->nodesToAddCollector->addNodeBeforeNode($expression, $node);

$this->traverseNodesWithCallable($node, static function (Node $subNode) use ($selfVariable): ?Closure {
if (! $subNode instanceof Closure) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public function refactor(Node $node): ?Node
$variable = $this->namedVariableFactory->createVariable($node, 'object');
$expression = new Expression(new Assign($variable, $node->var));

$this->nodesToAddCollector->addNodeBeforeNode($expression, $node, $this->file->getSmartFileInfo());
$this->nodesToAddCollector->addNodeBeforeNode($expression, $node);
$node->var = $variable;
// necessary to remove useless parentheses
$node->setAttribute(AttributeKey::ORIGINAL_NODE, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,7 @@ private function processClosure(FuncCall $funcCall, array $args): ?Variable

$this->nodesToAddCollector->addNodeBeforeNode(
new Expression(new Assign($variable, new Array_([]))),
$funcCall,
$this->file->getSmartFileInfo()
$funcCall
);

/** @var ConstFetch $constant */
Expand All @@ -121,7 +120,7 @@ private function processClosure(FuncCall $funcCall, array $args): ?Variable
? $this->applyArrayFilterUseKey($args, $closure, $variable)
: $this->applyArrayFilterUseBoth($args, $closure, $variable);

$this->nodesToAddCollector->addNodeBeforeNode($foreach, $funcCall, $this->file->getSmartFileInfo());
$this->nodesToAddCollector->addNodeBeforeNode($foreach, $funcCall);

return $variable;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ private function refactorForVariableLevels(FuncCall $funcCall): FuncCall
$closure = $this->createClosure();
$exprAssignClosure = $this->createExprAssign($funcVariable, $closure);

$this->nodesToAddCollector->addNodeBeforeNode($exprAssignClosure, $funcCall, $this->file->getSmartFileInfo());
$this->nodesToAddCollector->addNodeBeforeNode($exprAssignClosure, $funcCall);

$funcCall->name = $funcVariable;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,7 @@ public function refactor(Node $node): ?Node
$funcName = new Name('ini_set');
$iniSet = new FuncCall($funcName, [new Arg($sessionKey), new Arg($option->value)]);

$this->nodesToAddCollector->addNodeBeforeNode(
new Expression($iniSet),
$node,
$this->file->getSmartFileInfo()
);
$this->nodesToAddCollector->addNodeBeforeNode(new Expression($iniSet), $node);
}

unset($node->args[0]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public function refactor(Node $node): ?MethodCall
$assign = new Assign($variable, $node->var);
}

$this->nodesToAddCollector->addNodeBeforeNode(new Expression($assign), $node, $this->file->getSmartFileInfo());
$this->nodesToAddCollector->addNodeBeforeNode(new Expression($assign), $node);
$node->var = $variable;
$node->setAttribute(AttributeKey::ORIGINAL_NODE, null);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public function refactor(Node $node): FuncCall
$assignVariable = $this->namedVariableFactory->createVariable($node, 'battleShipcompare');

$assignExpression = $this->getAssignExpression($anonymousFunction, $assignVariable);
$this->nodesToAddCollector->addNodeBeforeNode($assignExpression, $node, $this->file->getSmartFileInfo());
$this->nodesToAddCollector->addNodeBeforeNode($assignExpression, $node);

return new FuncCall($assignVariable, [new Arg($node->left), new Arg($node->right)]);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public function refactor(Node $node): ?Node
$tempVariable = $this->namedVariableFactory->createVariable($node, 'callable');
$expression = new Expression(new Assign($tempVariable, $node->args[0]->value));

$this->nodesToAddCollector->addNodeBeforeNode($expression, $node, $this->file->getSmartFileInfo());
$this->nodesToAddCollector->addNodeBeforeNode($expression, $node);

$closure = new Closure();
$closure->uses[] = new ClosureUse($tempVariable);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ private function handleNotInIdenticalAndBooleanNot(If_ $if, FuncCall $funcCall):
{
if ($if->stmts !== []) {
$firstStmt = $if->stmts[0];
$this->nodesToAddCollector->addNodeBeforeNode($funcCall, $firstStmt, $this->file->getSmartFileInfo());
$this->nodesToAddCollector->addNodeBeforeNode($funcCall, $firstStmt);

return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node
$variable = new Variable($this->variableNaming->createCountedValueName('streamIsatty', $scope));
$assign = new Assign($variable, $function);

$this->nodesToAddCollector->addNodeBeforeNode($assign, $node, $this->file->getSmartFileInfo());
$this->nodesToAddCollector->addNodeBeforeNode($assign, $node);

return new FuncCall($variable, $node->args);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ private function refactorArrayKeyFirst(FuncCall $funcCall): ?FuncCall
}

$resetFuncCall = $this->nodeFactory->createFuncCall('reset', [$array]);
$this->nodesToAddCollector->addNodeBeforeNode($resetFuncCall, $funcCall, $this->file->getSmartFileInfo());
$this->nodesToAddCollector->addNodeBeforeNode($resetFuncCall, $funcCall);

$funcCall->name = new Name('key');
if ($originalArray !== $array) {
Expand Down Expand Up @@ -130,7 +130,7 @@ private function refactorArrayKeyLast(FuncCall $funcCall): ?FuncCall
}

$resetFuncCall = $this->nodeFactory->createFuncCall('end', [$array]);
$this->nodesToAddCollector->addNodeBeforeNode($resetFuncCall, $funcCall, $this->file->getSmartFileInfo());
$this->nodesToAddCollector->addNodeBeforeNode($resetFuncCall, $funcCall);

$funcCall->name = new Name('key');
if ($originalArray !== $array) {
Expand All @@ -142,11 +142,7 @@ private function refactorArrayKeyLast(FuncCall $funcCall): ?FuncCall

private function addAssignNewVariable(FuncCall $funcCall, Expr $expr, Expr|Variable $variable): void
{
$this->nodesToAddCollector->addNodeBeforeNode(
new Expression(new Assign($variable, $expr)),
$funcCall,
$this->file->getSmartFileInfo()
);
$this->nodesToAddCollector->addNodeBeforeNode(new Expression(new Assign($variable, $expr)), $funcCall);
}

private function resolveCastedArray(Expr $expr): Expr|Variable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,7 @@ public function refactor(Node $node): ?Node
);
// Assign the value to the variable
$newVariable = new Variable($variableName);
$this->nodesToAddCollector->addNodeBeforeNode(
new Assign($newVariable, $allowableTagsParam),
$node,
$this->file->getSmartFileInfo()
);
$this->nodesToAddCollector->addNodeBeforeNode(new Assign($newVariable, $allowableTagsParam), $node);

// Apply refactor on the variable
$newExpr = $this->createIsArrayTernaryFromExpression($newVariable);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,20 +135,12 @@ private function processClassConstFetches(MethodCall $methodCall, array $classCo
$variableReflectionClassConstants,
new MethodCall($methodCall->var, 'getReflectionConstants')
);
$this->nodesToAddCollector->addNodeBeforeNode(
new Expression($assign),
$methodCall,
$this->file->getSmartFileInfo()
);
$this->nodesToAddCollector->addNodeBeforeNode(new Expression($assign), $methodCall);

$result = $this->variableNaming->createCountedValueName('result', $scope);
$variableResult = new Variable($result);
$assignVariableResult = new Assign($variableResult, new Array_());
$this->nodesToAddCollector->addNodeBeforeNode(
new Expression($assignVariableResult),
$methodCall,
$this->file->getSmartFileInfo()
);
$this->nodesToAddCollector->addNodeBeforeNode(new Expression($assignVariableResult), $methodCall);

$ifs = [];
$valueVariable = new Variable('value');
Expand Down Expand Up @@ -176,11 +168,7 @@ private function processClassConstFetches(MethodCall $methodCall, array $classCo
'array_walk',
[$variableReflectionClassConstants, $closure]
);
$this->nodesToAddCollector->addNodeBeforeNode(
new Expression($funcCall),
$methodCall,
$this->file->getSmartFileInfo()
);
$this->nodesToAddCollector->addNodeBeforeNode(new Expression($funcCall), $methodCall);

return $variableResult;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public function refactor(Node $node): ?Node
$assign = new Assign($variable, $node->class);
}

$this->nodesToAddCollector->addNodeBeforeNode($assign, $node, $this->file->getSmartFileInfo());
$this->nodesToAddCollector->addNodeBeforeNode($assign, $node);
$node->class = $variable;
return $node;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ private function createVariableFromNonVariable(
$newVariable = new Variable($variableName);

$newVariableAssign = new Assign($newVariable, $arrayItem->value);
$this->nodesToAddCollector->addNodeBeforeNode($newVariableAssign, $array, $file->getSmartFileInfo());
$this->nodesToAddCollector->addNodeBeforeNode($newVariableAssign, $array);

return $newVariable;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public function refactorWithScope(Node $node, Scope $scope): ?FuncCall
$function = $this->createClosure();
$expression = new Expression(new Assign($variable, $function));

$this->nodesToAddCollector->addNodeBeforeNode($expression, $node, $this->file->getSmartFileInfo());
$this->nodesToAddCollector->addNodeBeforeNode($expression, $node);

return new FuncCall($variable, $node->args);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,7 @@ private function processEarlyReturn(
Foreach_ $foreach
): Foreach_ {
$this->removeNode($expression);
$this->nodesToAddCollector->addNodeBeforeNode(
new Return_($assign->expr),
$breaks[0],
$this->file->getSmartFileInfo()
);
$this->nodesToAddCollector->addNodeBeforeNode(new Return_($assign->expr), $breaks[0]);
$this->removeNode($breaks[0]);

$return->expr = $assignPreviousVariable->expr;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,7 @@ public function refactor(Node $node): ?Node
continue;
}

$this->nodesToAddCollector->addNodeBeforeNode(
$replacements->getAssign(),
$currentStmt,
$this->file->getSmartFileInfo()
);
$this->nodesToAddCollector->addNodeBeforeNode($replacements->getAssign(), $currentStmt);

$node->args[$key]->value = $replacements->getVariable();

Expand Down
13 changes: 11 additions & 2 deletions src/Application/ChangedNodeScopeRefresher.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
use Rector\Core\NodeAnalyzer\UnreachableStmtAnalyzer;
use Rector\Core\PhpParser\Node\BetterNodeFinder;
use Rector\Core\PhpParser\Node\CustomNode\FileWithoutNamespace;
use Rector\Core\Provider\CurrentFileProvider;
use Rector\Core\ValueObject\Application\File;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\NodeTypeResolver\PHPStan\Scope\PHPStanNodeScopeResolver;
use Symplify\SmartFileSystem\SmartFileInfo;
Expand All @@ -45,11 +47,12 @@ public function __construct(
private readonly PHPStanNodeScopeResolver $phpStanNodeScopeResolver,
private readonly ScopeAnalyzer $scopeAnalyzer,
private readonly UnreachableStmtAnalyzer $unreachableStmtAnalyzer,
private readonly BetterNodeFinder $betterNodeFinder
private readonly BetterNodeFinder $betterNodeFinder,
private readonly CurrentFileProvider $currentFileProvider
) {
}

public function refresh(Node $node, SmartFileInfo $smartFileInfo, ?MutatingScope $mutatingScope): void
public function refresh(Node $node, ?MutatingScope $mutatingScope, ?SmartFileInfo $smartFileInfo = null): void
{
// nothing to refresh
if (! $this->scopeAnalyzer->hasScope($node)) {
Expand All @@ -76,6 +79,12 @@ public function refresh(Node $node, SmartFileInfo $smartFileInfo, ?MutatingScope
}
}

if (! $smartFileInfo instanceof SmartFileInfo) {
/** @var File $file */
$file = $this->currentFileProvider->getFile();
$smartFileInfo = $file->getSmartFileInfo();
}

// note from flight: when we traverse ClassMethod, the scope must be already in Class_, otherwise it crashes
// so we need to somehow get a parent scope that is already in the same place the $node is

Expand Down
2 changes: 1 addition & 1 deletion src/Rector/AbstractRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ final public function enterNode(Node $node)
$this->mirrorAttributes($originalAttributes, $node);

$currentScope = $originalNode->getAttribute(AttributeKey::SCOPE);
$this->changedNodeScopeRefresher->refresh($node, $this->file->getSmartFileInfo(), $currentScope);
$this->changedNodeScopeRefresher->refresh($node, $currentScope, $this->file->getSmartFileInfo());

$this->connectParentNodes($node);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

namespace Rector\Core\Tests\Issues\ScopeNotAvailable\Fixture;

function test()
{
$match = ((\PREG_PATTERN_ORDER | \PREG_SET_ORDER) & $flags) ? 'preg_match_all' : 'preg_match';
$match($regexp.'u', $this->string, $matches, $flags | \PREG_UNMATCHED_AS_NULL, $offset);
}

?>
-----
<?php

namespace Rector\Core\Tests\Issues\ScopeNotAvailable\Fixture;

function test()
{
$match = ((\PREG_PATTERN_ORDER | \PREG_SET_ORDER) & $flags) ? 'preg_match_all' : 'preg_match';
$match($regexp.'u', $this->string, $matches, $flags, $offset);
array_walk_recursive($matches, function (&$value) {
if ($value === '') {
$value = null;
}
});
}

?>

0 comments on commit dcad222

Please sign in to comment.