[DeadCode] Remove RemoveUnusedAssignVariableRector in favor of RemoveUnusedVariableAssignRector (#1988)

This commit is contained in:
Abdul Malik Ikhsan 2022-04-01 14:13:23 +07:00 committed by GitHub
parent 24d20f3f09
commit aedfc65a48
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
34 changed files with 2 additions and 1059 deletions

View File

@ -1,4 +1,4 @@
# 507 Rules Overview
# 506 Rules Overview
<br>
@ -14,7 +14,7 @@
- [Composer](#composer) (6)
- [DeadCode](#deadcode) (50)
- [DeadCode](#deadcode) (49)
- [DependencyInjection](#dependencyinjection) (2)
@ -3354,31 +3354,6 @@ Remove unreachable statements
<br>
### RemoveUnusedAssignVariableRector
Remove assigned unused variable
- class: [`Rector\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector`](../rules/DeadCode/Rector/Assign/RemoveUnusedAssignVariableRector.php)
```diff
class SomeClass
{
public function run()
{
- $value = $this->process();
+ $this->process();
}
public function process()
{
// something going on
return 5;
}
}
```
<br>
### RemoveUnusedConstructorParamRector
Remove unused parameter in constructor

View File

@ -1,30 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class AssignConcat
{
public function run()
{
$removeMe = 'a' . 5 . 'b';
}
}
?>
-----
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class AssignConcat
{
public function run()
{
}
}
?>

View File

@ -1,32 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class AssignDouble
{
public function run()
{
$days = [1 => 'one', 'two'];
$days = ['something_else'];
}
}
?>
-----
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class AssignDouble
{
public function run()
{
}
}
?>

View File

@ -1,30 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class AssignFuncCallInternal
{
public function run()
{
$days = substr('lololo', 5);
}
}
?>
-----
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class AssignFuncCallInternal
{
public function run()
{
}
}
?>

View File

@ -1,37 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
final class AssignMultiFirstToGo
{
public function run()
{
$peValId = $values = 0;
if ($values) {
return true;
}
}
}
?>
-----
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
final class AssignMultiFirstToGo
{
public function run()
{
$values = 0;
if ($values) {
return true;
}
}
}
?>

View File

@ -1,49 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
final class AssignMultiSameLine
{
public function run()
{
$me = $this->createMe();
$me = $this->createMe();
$me = $this->createMe();
return $me;
}
public function createMe()
{
return new self();
}
}
?>
-----
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
final class AssignMultiSameLine
{
public function run()
{
$this->createMe();
$this->createMe();
$me = $this->createMe();
return $me;
}
public function createMe()
{
return new self();
}
}
?>

View File

@ -1,30 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class AssignNewInstance
{
public function run()
{
$days = new \stdClass();
}
}
?>
-----
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class AssignNewInstance
{
public function run()
{
}
}
?>

View File

@ -1,34 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class AssignProperty
{
private $days = '123';
public function run()
{
$days = $this->days;
}
}
?>
-----
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class AssignProperty
{
private $days = '123';
public function run()
{
}
}
?>

View File

@ -1,30 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class AssignScalar
{
public function run()
{
$days = [1 => 'one', 'two'];
}
}
?>
-----
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class AssignScalar
{
public function run()
{
}
}
?>

View File

@ -1,30 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class AssignSession
{
public function run()
{
$days = $_SESSION['ID'];
}
}
?>
-----
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class AssignSession
{
public function run()
{
}
}
?>

View File

@ -1,32 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class AssignStringLong
{
public function run()
{
$removeMe = <<<EOF
Value: {$embed}<br />
EOF;
}
}
?>
-----
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class AssignStringLong
{
public function run()
{
}
}
?>

View File

@ -1,43 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class Fixture
{
public function run()
{
$value = $this->process();
}
public function process()
{
// something going on
return 5;
}
}
?>
-----
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class Fixture
{
public function run()
{
$this->process();
}
public function process()
{
// something going on
return 5;
}
}
?>

View File

@ -1,23 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
final class SkipAssignEmbedHtml
{
public function content()
{
for ($c = 0; $c <= \pg_numrows($result); $c++) {
if ($a !== $b) {
if ($row === 0) {
StaticCaller::callMe($suspect);
}
$suspect = 0;
}
if ($row[5] === 1) {
$suspect = $row[6];
}
}
}
}

View File

@ -1,15 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
final class SkipAssignExpressionClassUseVariable
{
public function run()
{
$datetime = time();
$datetime = new \DateTime($datetime);
return $datetime;
}
}

View File

@ -1,17 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
final class SkipAssignExpressionClosureUseVariable
{
public function run()
{
$content = [1, 2, 3];
$filtered = array_filter([1, 2, 3], function ($v) use ($content) {
return $content !== $v;
});
return $filtered;
}
}

View File

@ -1,15 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
final class SkipAssignExpressionUseVariable
{
public function run()
{
$content = 1;
$content = $content + 1;
return $content;
}
}

View File

@ -1,15 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
final class SkipAssignExpressionUseVariableAsFunctionParam
{
public function run()
{
$content = [1, 2, 3];
$content = array_push($content, 1);
return $content;
}
}

View File

@ -1,14 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class SkipFor
{
public function run($nesting = 100)
{
for ($j = 2; $j < $nesting; $j++) {
}
}
}

View File

@ -1,15 +0,0 @@
<?php
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class SkipIfAssignNestedUse
{
public function run($token)
{
if (($id = $this->get($token))) {
pg_query_params('SELECT ... =$1', [
$id,
]);
}
}
}

View File

@ -1,12 +0,0 @@
<?php
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class SkipInCompact
{
public function run()
{
$value = 'foobar';
return compact('value');
}
}

View File

@ -1,12 +0,0 @@
<?php
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class SkipInCompactArray
{
public function run()
{
$value = 'foobar';
return compact(['value']);
}
}

View File

@ -1,19 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class SkipMultiAssign
{
public function run()
{
$values = $peValId = $peColId = $auto = 0;
if ($values) {
return true;
}
return false;
}
}

View File

@ -1,23 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
final class SkipProperty
{
/**
* @var string[]
*/
private $days;
public function run()
{
$this->days = [1 => 'one', 'two'];
}
public function useMe()
{
return $this->days;
}
}

View File

@ -1,17 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class SkipUseAnd
{
public function run()
{
if (($format = $this->getImageFormat()) && $format !== false) {
return 'png';
}
return 'non_png';
}
}

View File

@ -1,21 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
class SkipUsedAfterIfElse {
function test() {
if ( true ) {
$test1 = '123';
$test2 = '123';
} else {
$test1 = 'abc';
$test2 = 'abc';
}
echo $test1 . $test2;
}
}
?>

View File

@ -1,15 +0,0 @@
<?php
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\Fixture;
final class SkipVarOverrideAndReUse
{
public function run()
{
$sep = '';
while ($row = \pg_fetch_assoc($query)) {
$text .= "{$sep}" . ($row['b'] ?: $row['a']);
$sep = ', ';
}
}
}

View File

@ -1,33 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector;
use Iterator;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
use Symplify\SmartFileSystem\SmartFileInfo;
final class RemoveUnusedAssignVariableRectorTest extends AbstractRectorTestCase
{
/**
* @dataProvider provideData()
*/
public function test(SmartFileInfo $fileInfo): void
{
$this->doTestFileInfo($fileInfo);
}
/**
* @return Iterator<SmartFileInfo>
*/
public function provideData(): Iterator
{
return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture');
}
public function provideConfigFilePath(): string
{
return __DIR__ . '/config/configured_rule.php';
}
}

View File

@ -1,11 +0,0 @@
<?php
declare(strict_types=1);
use Rector\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector;
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
return static function (ContainerConfigurator $containerConfigurator): void {
$services = $containerConfigurator->services();
$services->set(RemoveUnusedAssignVariableRector::class);
};

View File

@ -1,78 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\DeadCode\NodeFinder;
use PhpParser\Node;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\Variable;
use PhpParser\NodeTraverser;
use Rector\Core\PhpParser\Comparing\NodeComparator;
use Rector\Core\PhpParser\Node\BetterNodeFinder;
use Rector\NodeNameResolver\NodeNameResolver;
use Rector\NodeNestingScope\ParentScopeFinder;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Symplify\Astral\NodeTraverser\SimpleCallableNodeTraverser;
final class NextVariableUsageNodeFinder
{
public function __construct(
private readonly BetterNodeFinder $betterNodeFinder,
private readonly SimpleCallableNodeTraverser $simpleCallableNodeTraverser,
private readonly NodeNameResolver $nodeNameResolver,
private readonly ParentScopeFinder $parentScopeFinder,
private readonly NodeComparator $nodeComparator
) {
}
public function find(Assign $assign): ?Node
{
$scopeNode = $this->parentScopeFinder->find($assign);
if (! $scopeNode instanceof Node) {
return null;
}
/** @var Variable $expr */
$expr = $assign->var;
$this->simpleCallableNodeTraverser->traverseNodesWithCallable((array) $scopeNode->stmts, function (
Node $currentNode
) use ($expr, &$nextUsageOfVariable): ?int {
// used above the assign
if ($currentNode->getStartTokenPos() < $expr->getStartTokenPos()) {
return null;
}
// skip self
if ($this->nodeComparator->areSameNode($currentNode, $expr)) {
return null;
}
if (! $this->nodeComparator->areNodesEqual($currentNode, $expr)) {
return null;
}
$currentNodeParent = $currentNode->getAttribute(AttributeKey::PARENT_NODE);
if ($currentNodeParent instanceof Assign && ! $this->hasInParentExpression($currentNodeParent, $expr)) {
return NodeTraverser::STOP_TRAVERSAL;
}
$nextUsageOfVariable = $currentNode;
return NodeTraverser::STOP_TRAVERSAL;
});
return $nextUsageOfVariable;
}
private function hasInParentExpression(Assign $assign, Variable $variable): bool
{
$name = $this->nodeNameResolver->getName($variable);
if ($name === null) {
return false;
}
return $this->betterNodeFinder->hasVariableOfName($assign->expr, $name);
}
}

View File

@ -1,47 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\DeadCode\NodeFinder;
use PhpParser\Node;
use PhpParser\Node\Expr\Assign;
use Rector\Core\PhpParser\Comparing\NodeComparator;
use Rector\Core\PhpParser\Node\BetterNodeFinder;
use Rector\NodeNameResolver\NodeNameResolver;
final class PreviousVariableAssignNodeFinder
{
public function __construct(
private readonly BetterNodeFinder $betterNodeFinder,
private readonly NodeNameResolver $nodeNameResolver,
private readonly NodeComparator $nodeComparator
) {
}
public function find(Assign $assign): ?Node
{
$currentAssign = $assign;
$variableName = $this->nodeNameResolver->getName($assign->var);
if ($variableName === null) {
return null;
}
return $this->betterNodeFinder->findFirstPrevious($assign, function (Node $node) use (
$variableName,
$currentAssign
): bool {
if (! $node instanceof Assign) {
return false;
}
// skip self
if ($this->nodeComparator->areSameNode($node, $currentAssign)) {
return false;
}
return $this->nodeNameResolver->isName($node->var, $variableName);
});
}
}

View File

@ -1,180 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\DeadCode\Rector\Assign;
use PhpParser\Node;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Stmt\Else_;
use PhpParser\Node\Stmt\If_;
use PHPStan\Analyser\Scope;
use Rector\Core\Rector\AbstractRector;
use Rector\DeadCode\NodeAnalyzer\ExprUsedInNextNodeAnalyzer;
use Rector\DeadCode\NodeFinder\NextVariableUsageNodeFinder;
use Rector\DeadCode\NodeFinder\PreviousVariableAssignNodeFinder;
use Rector\DeadCode\SideEffect\SideEffectNodeDetector;
use Rector\NodeNestingScope\ScopeNestingComparator;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
/**
* @see \Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector\RemoveUnusedAssignVariableRectorTest
*/
final class RemoveUnusedAssignVariableRector extends AbstractRector
{
public function __construct(
private readonly NextVariableUsageNodeFinder $nextVariableUsageNodeFinder,
private readonly PreviousVariableAssignNodeFinder $previousVariableAssignNodeFinder,
private readonly ScopeNestingComparator $scopeNestingComparator,
private readonly SideEffectNodeDetector $sideEffectNodeDetector,
private readonly ExprUsedInNextNodeAnalyzer $exprUsedInNextNodeAnalyzer
) {
}
/**
* @return array<class-string<Node>>
*/
public function getNodeTypes(): array
{
return [Assign::class];
}
public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('Remove assigned unused variable', [
new CodeSample(
<<<'CODE_SAMPLE'
class SomeClass
{
public function run()
{
$value = $this->process();
}
public function process()
{
// something going on
return 5;
}
}
CODE_SAMPLE
,
<<<'CODE_SAMPLE'
class SomeClass
{
public function run()
{
$this->process();
}
public function process()
{
// something going on
return 5;
}
}
CODE_SAMPLE
),
]);
}
/**
* @param Assign $node
*/
public function refactor(Node $node): ?Node
{
if ($this->shouldSkipAssign($node)) {
return null;
}
if (! $this->isPreviousVariablePartOfOverridingAssign($node) && ($this->isVariableTypeInScope(
$node
) || $this->exprUsedInNextNodeAnalyzer->isUsed($node->var))) {
return null;
}
// @see https://github.com/rectorphp/rector/issues/6655
// verify current statement is a Node before removing or use its Assign Expr
$currentStatement = $node->getAttribute(AttributeKey::CURRENT_STATEMENT);
if (! $currentStatement instanceof Node) {
return null;
}
// is scalar assign? remove whole
if (! $this->sideEffectNodeDetector->detect($node->expr)) {
$this->removeNode($node);
return null;
}
return $node->expr;
}
private function shouldSkipAssign(Assign $assign): bool
{
$variable = $assign->var;
if (! $variable instanceof Variable) {
return true;
}
// unable to resolve name
$variableName = $this->getName($variable);
if ($variableName === null) {
return true;
}
if ($this->isNestedAssign($assign)) {
return true;
}
$parentIf = $this->betterNodeFinder->findParentType($assign, If_::class);
if (! $parentIf instanceof If_) {
return (bool) $this->nextVariableUsageNodeFinder->find($assign);
}
if (! $parentIf->else instanceof Else_) {
return (bool) $this->nextVariableUsageNodeFinder->find($assign);
}
return (bool) $this->betterNodeFinder->findFirstNext(
$parentIf,
fn (Node $node): bool => $this->nodeComparator->areNodesEqual($node, $variable)
);
}
private function isVariableTypeInScope(Assign $assign): bool
{
$scope = $assign->getAttribute(AttributeKey::SCOPE);
if (! $scope instanceof Scope) {
return false;
}
/** @var string $variableName */
$variableName = $this->getName($assign->var);
return ! $scope->hasVariableType($variableName)
->no();
}
private function isPreviousVariablePartOfOverridingAssign(Assign $assign): bool
{
// is previous variable node as part of assign?
$previousVariableAssign = $this->previousVariableAssignNodeFinder->find($assign);
if (! $previousVariableAssign instanceof Node) {
return false;
}
return $this->scopeNestingComparator->areScopeNestingEqual($assign, $previousVariableAssign);
}
/**
* Nested assign, e.g "$oldValues = <$values> = 5;"
*/
private function isNestedAssign(Assign $assign): bool
{
$parent = $assign->getAttribute(AttributeKey::PARENT_NODE);
return $parent instanceof Assign;
}
}

View File

@ -1,23 +0,0 @@
<?php
function getRes(): string
{
if ('a' !== 'b') {
$res = "a";
} else {
$res = "b";
}
return $res;
}
?>
-----
<?php
function getRes(): string
{
$res = 'a' !== 'b' ? "a" : "b";
return $res;
}
?>

View File

@ -1,36 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Core\Tests\Issues\Issue6655;
use Iterator;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
use Symplify\SmartFileSystem\SmartFileInfo;
/**
* @see https://github.com/rectorphp/rector/issues/6655
*/
final class SimplifyIfElseAndRemoveUnusedAssignVariableTest extends AbstractRectorTestCase
{
/**
* @dataProvider provideData()
*/
public function test(SmartFileInfo $fileInfo): void
{
$this->doTestFileInfo($fileInfo);
}
/**
* @return Iterator<SmartFileInfo>
*/
public function provideData(): Iterator
{
return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture');
}
public function provideConfigFilePath(): string
{
return __DIR__ . '/config/configured_rule.php';
}
}

View File

@ -1,14 +0,0 @@
<?php
declare(strict_types=1);
use Rector\CodeQuality\Rector\If_\SimplifyIfElseToTernaryRector;
use Rector\DeadCode\Rector\Assign\RemoveUnusedAssignVariableRector;
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
return static function (ContainerConfigurator $containerConfigurator): void {
$services = $containerConfigurator->services();
$services->set(SimplifyIfElseToTernaryRector::class);
$services->set(RemoveUnusedAssignVariableRector::class);
};