mirror_php/rector
1
0
Fork 0
mirror of https://github.com/rectorphp/rector.git synced 2025-04-21 16:02:23 +02:00
Code Issues Projects Releases Wiki Activity

[DeadCode] Fix binary different nesting in RemoveOverriddenValuesRector (#4422)

Browse Source 
* decopule NodeByTypeAndPositionCollector

* decouple VariableUseFinder
This commit is contained in:
Tomas Votruba 2020-10-15 22:23:25 +02:00 committed by GitHub
parent 09dc6cef9a
commit 16031da4b7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 260 additions and 120 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
packages/node-nesting-scope/src/FlowOfControlLocator.php
View File

@ -5,6 +5,7 @@ declare(strict_types=1);
namespace Rector\NodeNestingScope;
use PhpParser\Node;
use PhpParser\Node\Expr\BinaryOp;
use PhpParser\Node\FunctionLike;
use PhpParser\Node\Stmt\Expression;
use Rector\NodeTypeResolver\Node\AttributeKey;
@ -13,20 +14,49 @@ final class FlowOfControlLocator
{
public function resolveNestingHashFromFunctionLike(FunctionLike $functionLike, Node $checkedNode): string
{
$nestingHash = '_';
$nestingHash = spl_object_hash($functionLike) . '__';
$parentNode = $checkedNode;
while ($parentNode = $parentNode->getAttribute(AttributeKey::PARENT_NODE)) {
if ($parentNode instanceof Expression) {
$currentNode = $checkedNode;
$previous = $currentNode;
while ($currentNode = $currentNode->getAttribute(AttributeKey::PARENT_NODE)) {
if ($currentNode instanceof Expression) {
continue;
}
$nestingHash .= spl_object_hash($parentNode);
if ($functionLike === $parentNode) {
return $nestingHash;
if (! $currentNode instanceof Node) {
continue;
}
if ($functionLike === $currentNode) {
// to high
break;
}
$nestingHash .= $this->resolveBinaryOpNestingHash($currentNode, $previous);
$nestingHash .= spl_object_hash($currentNode);
$previous = $currentNode;
}
return $nestingHash;
}
private function resolveBinaryOpNestingHash(Node $currentNode, Node $previous): string
{
if (! $currentNode instanceof BinaryOp) {
return '';
}
// left && right have differnt nesting
if ($currentNode->left === $previous) {
return 'binary_left__';
}
if ($currentNode->right === $previous) {
return 'binary_right__';
}
return '';
}
}

67
rules/dead-code/src/FlowControl/VariableUseFinder.php Normal file
View File

@ -0,0 +1,67 @@
<?php
declare(strict_types=1);
namespace Rector\DeadCode\FlowControl;
use PhpParser\Node;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\Variable;
use Rector\Core\PhpParser\Node\BetterNodeFinder;
use Rector\Core\PhpParser\Printer\BetterStandardPrinter;
use Rector\NodeNameResolver\NodeNameResolver;
use Rector\NodeTypeResolver\Node\AttributeKey;
final class VariableUseFinder
{
/**
* @var BetterNodeFinder
*/
private $betterNodeFinder;
/**
* @var NodeNameResolver
*/
private $nodeNameResolver;
/**
* @var BetterStandardPrinter
*/
private $betterStandardPrinter;
public function __construct(
BetterNodeFinder $betterNodeFinder,
NodeNameResolver $nodeNameResolver,
BetterStandardPrinter $betterStandardPrinter
) {
$this->betterNodeFinder = $betterNodeFinder;
$this->nodeNameResolver = $nodeNameResolver;
$this->betterStandardPrinter = $betterStandardPrinter;
}
/**
* @param Variable[] $assignedVariables
* @return Variable[]
*/
public function resolveUsedVariables(Node $node, array $assignedVariables): array
{
return $this->betterNodeFinder->find($node, function (Node $node) use ($assignedVariables): bool {
if (! $node instanceof Variable) {
return false;
}
$parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
// is the left assign - not use of one
if ($parentNode instanceof Assign && ($parentNode->var instanceof Variable && $parentNode->var === $node)) {
return false;
}
// simple variable only
if ($this->nodeNameResolver->getName($node) === null) {
return false;
}
return $this->betterStandardPrinter->isNodeEqual($node, $assignedVariables);
});
}
}

99
rules/dead-code/src/NodeCollector/NodeByTypeAndPositionCollector.php Normal file
View File

@ -0,0 +1,99 @@
<?php
declare(strict_types=1);
namespace Rector\DeadCode\NodeCollector;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\FunctionLike;
use Rector\DeadCode\ValueObject\VariableNodeUse;
use Rector\NodeNameResolver\NodeNameResolver;
use Rector\NodeNestingScope\FlowOfControlLocator;
use Rector\NodeTypeResolver\Node\AttributeKey;
final class NodeByTypeAndPositionCollector
{
/**
* @var FlowOfControlLocator
*/
private $flowOfControlLocator;
/**
* @var NodeNameResolver
*/
private $nodeNameResolver;
public function __construct(FlowOfControlLocator $flowOfControlLocator, NodeNameResolver $nodeNameResolver)
{
$this->flowOfControlLocator = $flowOfControlLocator;
$this->nodeNameResolver = $nodeNameResolver;
}
/**
* @param Variable[] $assignedVariables
* @param Variable[] $assignedVariablesUse
* @return VariableNodeUse[]
*/
public function collectNodesByTypeAndPosition(
array $assignedVariables,
array $assignedVariablesUse,
FunctionLike $functionLike
): array {
$nodesByTypeAndPosition = [];
foreach ($assignedVariables as $assignedVariable) {
/** @var int $startTokenPos */
$startTokenPos = $assignedVariable->getAttribute(AttributeKey::START_TOKEN_POSITION);
// not in different scope, than previous one - e.g. if/while/else...
// get nesting level to $classMethodNode
/** @var Assign $assign */
$assign = $assignedVariable->getAttribute(AttributeKey::PARENT_NODE);
$nestingHash = $this->flowOfControlLocator->resolveNestingHashFromFunctionLike($functionLike, $assign);
/** @var string $variableName */
$variableName = $this->nodeNameResolver->getName($assignedVariable);
$nodesByTypeAndPosition[] = new VariableNodeUse(
$startTokenPos,
$variableName,
VariableNodeUse::TYPE_ASSIGN,
$assignedVariable,
$nestingHash
);
}
foreach ($assignedVariablesUse as $assignedVariableUse) {
/** @var int $startTokenPos */
$startTokenPos = $assignedVariableUse->getAttribute(AttributeKey::START_TOKEN_POSITION);
/** @var string $variableName */
$variableName = $this->nodeNameResolver->getName($assignedVariableUse);
$nodesByTypeAndPosition[] = new VariableNodeUse(
$startTokenPos,
$variableName,
VariableNodeUse::TYPE_USE,
$assignedVariableUse
);
}
return $this->sortByStart($nodesByTypeAndPosition);
}
/**
* @param VariableNodeUse[] $nodesByTypeAndPosition
* @return VariableNodeUse[]
*/
private function sortByStart(array $nodesByTypeAndPosition): array
{
usort(
$nodesByTypeAndPosition,
function (VariableNodeUse $firstVariableNodeUse, VariableNodeUse $secondVariableNodeUse): int {
return $firstVariableNodeUse->getStartTokenPosition() <=> $secondVariableNodeUse->getStartTokenPosition();
}
);
return $nodesByTypeAndPosition;
}
}

120
rules/dead-code/src/Rector/FunctionLike/RemoveOverriddenValuesRector.php
View File

@ -12,8 +12,9 @@ use Rector\Core\Context\ContextAnalyzer;
use Rector\Core\Rector\AbstractRector;
use Rector\Core\RectorDefinition\CodeSample;
use Rector\Core\RectorDefinition\RectorDefinition;
use Rector\DeadCode\FlowControl\VariableUseFinder;
use Rector\DeadCode\NodeCollector\NodeByTypeAndPositionCollector;
use Rector\DeadCode\ValueObject\VariableNodeUse;
use Rector\NodeNestingScope\FlowOfControlLocator;
use Rector\NodeTypeResolver\Node\AttributeKey;
/**
@ -27,14 +28,23 @@ final class RemoveOverriddenValuesRector extends AbstractRector
private $contextAnalyzer;
/**
* @var FlowOfControlLocator
* @var NodeByTypeAndPositionCollector
*/
private $flowOfControlLocator;
private $nodeByTypeAndPositionCollector;
public function __construct(ContextAnalyzer $contextAnalyzer, FlowOfControlLocator $flowOfControlLocator)
{
/**
* @var VariableUseFinder
*/
private $variableUseFinder;
public function __construct(
ContextAnalyzer $contextAnalyzer,
NodeByTypeAndPositionCollector $nodeByTypeAndPositionCollector,
VariableUseFinder $variableUseFinder
) {
$this->contextAnalyzer = $contextAnalyzer;
$this->flowOfControlLocator = $flowOfControlLocator;
$this->nodeByTypeAndPositionCollector = $nodeByTypeAndPositionCollector;
$this->variableUseFinder = $variableUseFinder;
}
public function getDefinition(): RectorDefinition
@ -85,9 +95,9 @@ CODE_SAMPLE
$assignedVariableNames = $this->getNodeNames($assignedVariables);
// 2. collect use of those variables
$assignedVariablesUse = $this->resolveUsedVariables($node, $assignedVariables);
$assignedVariablesUse = $this->variableUseFinder->resolveUsedVariables($node, $assignedVariables);
$nodesByTypeAndPosition = $this->collectNodesByTypeAndPosition(
$nodesByTypeAndPosition = $this->nodeByTypeAndPositionCollector->collectNodesByTypeAndPosition(
$assignedVariables,
$assignedVariablesUse,
$node
@ -151,92 +161,6 @@ CODE_SAMPLE
return array_unique($nodeNames);
}
/**
* @param Variable[] $assignedVariables
* @return Variable[]
*/
private function resolveUsedVariables(Node $node, array $assignedVariables): array
{
return $this->betterNodeFinder->find($node, function (Node $node) use ($assignedVariables): bool {
if (! $node instanceof Variable) {
return false;
}
$parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
// is the left assign - not use of one
if ($parentNode instanceof Assign && ($parentNode->var instanceof Variable && $parentNode->var === $node)) {
return false;
}
// simple variable only
if ($this->getName($node) === null) {
return false;
}
return $this->isNodeEqual($node, $assignedVariables);
});
}
/**
* @param Variable[] $assignedVariables
* @param Variable[] $assignedVariablesUse
* @return VariableNodeUse[]
*/
private function collectNodesByTypeAndPosition(
array $assignedVariables,
array $assignedVariablesUse,
FunctionLike $functionLike
): array {
$nodesByTypeAndPosition = [];
foreach ($assignedVariables as $assignedVariable) {
/** @var int $startTokenPos */
$startTokenPos = $assignedVariable->getAttribute(AttributeKey::START_TOKEN_POSITION);
// not in different scope, than previous one - e.g. if/while/else...
// get nesting level to $classMethodNode
/** @var Assign $assignNode */
$assignNode = $assignedVariable->getAttribute(AttributeKey::PARENT_NODE);
$nestingHash = $this->flowOfControlLocator->resolveNestingHashFromFunctionLike($functionLike, $assignNode);
/** @var string $variableName */
$variableName = $this->getName($assignedVariable);
$nodesByTypeAndPosition[] = new VariableNodeUse(
$startTokenPos,
$variableName,
VariableNodeUse::TYPE_ASSIGN,
$assignedVariable,
$nestingHash
);
}
foreach ($assignedVariablesUse as $assignedVariableUse) {
/** @var int $startTokenPos */
$startTokenPos = $assignedVariableUse->getAttribute(AttributeKey::START_TOKEN_POSITION);
/** @var string $variableName */
$variableName = $this->getName($assignedVariableUse);
$nodesByTypeAndPosition[] = new VariableNodeUse(
$startTokenPos,
$variableName,
VariableNodeUse::TYPE_USE,
$assignedVariableUse
);
}
// sort
usort(
$nodesByTypeAndPosition,
function (VariableNodeUse $firstVariableNodeUse, VariableNodeUse $secondVariableNodeUse): int {
return $firstVariableNodeUse->getStartTokenPosition() <=> $secondVariableNodeUse->getStartTokenPosition();
}
);
return $nodesByTypeAndPosition;
}
/**
* @param string[] $assignedVariableNames
* @param VariableNodeUse[] $nodesByTypeAndPosition
@ -295,9 +219,11 @@ CODE_SAMPLE
return false;
}
if (! $previousNode->isType(VariableNodeUse::TYPE_ASSIGN) || ! $nodeByTypeAndPosition->isType(
VariableNodeUse::TYPE_ASSIGN
)) {
if (! $previousNode->isType(VariableNodeUse::TYPE_ASSIGN)) {
return false;
}
if (! $nodeByTypeAndPosition->isType(VariableNodeUse::TYPE_ASSIGN)) {
return false;
}

11
rules/dead-code/tests/Rector/FunctionLike/RemoveOverriddenValuesRector/Fixture/skip_set_or_set.php.inc Normal file
View File

@ -0,0 +1,11 @@
<?php
namespace Rector\DeadCode\Tests\Rector\FunctionLike\RemoveOverriddenValuesRector\Fixture;
final class SkipSetOrSet
{
public function run()
{
$isSet = (($one = 10) || ($one = 200));
}
}

22
src/PhpParser/Printer/BetterStandardPrinter.php
View File

@ -188,6 +188,28 @@ final class BetterStandardPrinter extends Standard
return ltrim($content);
}
/**
* @param Node[] $availableNodes
*/
public function isNodeEqual(Node $singleNode, array $availableNodes): bool
{
// remove comments, only content is relevant
$singleNode = clone $singleNode;
$singleNode->setAttribute(AttributeKey::COMMENTS, null);
foreach ($availableNodes as $availableNode) {
// remove comments, only content is relevant
$availableNode = clone $availableNode;
$availableNode->setAttribute(AttributeKey::COMMENTS, null);
if ($this->areNodesEqual($singleNode, $availableNode)) {
return true;
}
}
return false;
}
/**
* This allows to use both spaces and tabs vs. original space-only
*/

17
src/Rector/AbstractRector/BetterStandardPrinterTrait.php
View File

@ -7,7 +7,6 @@ namespace Rector\Core\Rector\AbstractRector;
use PhpParser\Node;
use Rector\Core\PhpParser\Node\BetterNodeFinder;
use Rector\Core\PhpParser\Printer\BetterStandardPrinter;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Symplify\SmartFileSystem\SmartFileSystem;
/**
@ -85,21 +84,7 @@ trait BetterStandardPrinterTrait
*/
protected function isNodeEqual(Node $singleNode, array $availableNodes): bool
{
// remove comments, only content is relevant
$singleNode = clone $singleNode;
$singleNode->setAttribute(AttributeKey::COMMENTS, null);
foreach ($availableNodes as $availableNode) {
// remove comments, only content is relevant
$availableNode = clone $availableNode;
$availableNode->setAttribute(AttributeKey::COMMENTS, null);
if ($this->areNodesEqual($singleNode, $availableNode)) {
return true;
}
}
return false;
return $this->betterStandardPrinter->isNodeEqual($singleNode, $availableNodes);
}
/**