Skip to content

Commit

Permalink
early return
Browse files Browse the repository at this point in the history
  • Loading branch information
TomasVotruba committed Apr 17, 2021
1 parent ab39200 commit 733be85
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 47 deletions.
46 changes: 46 additions & 0 deletions rules/DeadCode/NodeAnalyzer/UsedVariableNameAnalyzer.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<?php

declare(strict_types=1);

namespace Rector\DeadCode\NodeAnalyzer;

use PhpParser\Node;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\Variable;
use Rector\NodeNameResolver\NodeNameResolver;

final class UsedVariableNameAnalyzer
{
/**
* @var NodeNameResolver
*/
private $nodeNameResolver;

public function __construct(NodeNameResolver $nodeNameResolver)
{
$this->nodeNameResolver = $nodeNameResolver;
}

public function isVariableNamed(Node $node, Variable $variable): bool
{
$variableName = $this->nodeNameResolver->getName($variable);
if ($variableName === null) {
return false;
}

if ($node instanceof MethodCall && $node->name instanceof Variable) {
return $this->nodeNameResolver->isName($node->name, $variableName);
}

if ($node instanceof PropertyFetch && $node->name instanceof Variable) {
return $this->nodeNameResolver->isName($node->name, $variableName);
}

if (! $node instanceof Variable) {
return false;
}

return $this->nodeNameResolver->isName($node, $variableName);
}
}
71 changes: 35 additions & 36 deletions rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\New_;
use PhpParser\Node\Expr\NullsafeMethodCall;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\StaticCall;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\FunctionLike;
Expand All @@ -22,6 +21,7 @@
use Rector\Core\Php\ReservedKeywordAnalyzer;
use Rector\Core\PhpParser\Comparing\ConditionSearcher;
use Rector\Core\Rector\AbstractRector;
use Rector\DeadCode\NodeAnalyzer\UsedVariableNameAnalyzer;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
Expand All @@ -46,13 +46,20 @@ final class RemoveUnusedVariableAssignRector extends AbstractRector
*/
private $conditionSearcher;

/**
* @var UsedVariableNameAnalyzer
*/
private $usedVariableNameAnalyzer;

public function __construct(
ReservedKeywordAnalyzer $reservedKeywordAnalyzer,
CompactFuncCallAnalyzer $compactFuncCallAnalyzer
CompactFuncCallAnalyzer $compactFuncCallAnalyzer,
UsedVariableNameAnalyzer $usedVariableNameAnalyzer
) {
$this->reservedKeywordAnalyzer = $reservedKeywordAnalyzer;
$this->compactFuncCallAnalyzer = $compactFuncCallAnalyzer;
$this->conditionSearcher = new ConditionSearcher();
$this->usedVariableNameAnalyzer = $usedVariableNameAnalyzer;
}

public function getRuleDefinition(): RuleDefinition
Expand Down Expand Up @@ -98,31 +105,19 @@ public function refactor(Node $node): ?Node
return null;
}

/** @var Variable $variable */
$variable = $node->var;
if (! $variable instanceof Variable || (is_string(
$variable->name
) && $this->reservedKeywordAnalyzer->isNativeVariable($variable->name))) {
if (! $variable instanceof Variable) {
return null;
}

$variableName = $this->getName($variable);
if ($variableName !== null && $this->reservedKeywordAnalyzer->isNativeVariable($variableName)) {
return null;
}

// variable is used
if ($this->isUsed($node, $variable)) {
$parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
$ifNode = $parentNode->getAttribute(AttributeKey::NEXT_NODE);

// check if next node is if
if (! $ifNode instanceof If_) {
return null;
}

$nodeFound = $this->conditionSearcher->searchIfAndElseForVariableRedeclaration($node, $ifNode);
if ($nodeFound) {
$this->removeNode($node);
return $node;
}

return null;
return $this->refactorUsedVariable($node);
}

$parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
Expand Down Expand Up @@ -151,7 +146,7 @@ private function shouldSkip(Assign $assign): bool
return true;
}

return $variable->name instanceof Variable && (bool) $this->betterNodeFinder->findFirstNext(
return $variable->name instanceof Variable && $this->betterNodeFinder->findFirstNext(
$assign,
function (Node $node): bool {
return $node instanceof Variable;
Expand All @@ -164,7 +159,7 @@ private function isUsed(Assign $assign, Variable $variable): bool
$isUsedPrev = (bool) $this->betterNodeFinder->findFirstPreviousOfNode($variable, function (Node $node) use (
$variable
): bool {
return $this->isVariableNamed($node, $variable);
return $this->usedVariableNameAnalyzer->isVariableNamed($node, $variable);
});

if ($isUsedPrev) {
Expand All @@ -189,7 +184,7 @@ private function isUsedNext(Variable $variable): bool
return (bool) $this->betterNodeFinder->findFirstNext($variable, function (Node $node) use (
$variable
): bool {
if ($this->isVariableNamed($node, $variable)) {
if ($this->usedVariableNameAnalyzer->isVariableNamed($node, $variable)) {
return true;
}

Expand All @@ -206,8 +201,7 @@ private function isUsedNext(Variable $variable): bool
*/
private function isUsedInAssignExpr(Expr $expr, Assign $assign): bool
{
$args = $expr->args;
foreach ($args as $arg) {
foreach ($expr->args as $arg) {
$variable = $arg->value;
if (! $variable instanceof Variable) {
continue;
Expand All @@ -216,8 +210,12 @@ private function isUsedInAssignExpr(Expr $expr, Assign $assign): bool
$previousAssign = $this->betterNodeFinder->findFirstPreviousOfNode($assign, function (Node $node) use (
$variable
): bool {
return $node instanceof Assign && $this->isVariableNamed($node->var, $variable);
return $node instanceof Assign && $this->usedVariableNameAnalyzer->isVariableNamed(
$node->var,
$variable
);
});

if ($previousAssign instanceof Assign) {
return $this->isUsed($assign, $variable);
}
Expand All @@ -231,20 +229,21 @@ private function isCall(Expr $expr): bool
return $expr instanceof FuncCall || $expr instanceof MethodCall || $expr instanceof New_ || $expr instanceof NullsafeMethodCall || $expr instanceof StaticCall;
}

private function isVariableNamed(Node $node, Variable $variable): bool
private function refactorUsedVariable(Assign $assign): ?Assign
{
if ($node instanceof MethodCall && $node->name instanceof Variable && is_string($node->name->name)) {
return $this->isName($variable, $node->name->name);
}
$parentNode = $assign->getAttribute(AttributeKey::PARENT_NODE);
$ifNode = $parentNode->getAttribute(AttributeKey::NEXT_NODE);

if ($node instanceof PropertyFetch && $node->name instanceof Variable && is_string($node->name->name)) {
return $this->isName($variable, $node->name->name);
// check if next node is if
if (! $ifNode instanceof If_) {
return null;
}

if (! $node instanceof Variable) {
return false;
if ($this->conditionSearcher->searchIfAndElseForVariableRedeclaration($assign, $ifNode)) {
$this->removeNode($assign);
return $assign;
}

return $this->isName($variable, (string) $this->getName($node));
return null;
}
}
27 changes: 18 additions & 9 deletions rules/PSR4/NodeManipulator/FullyQualifyStmtsAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,18 +72,27 @@ public function process(array $nodes): void
return null;
}

$parent = $node->getAttribute(AttributeKey::PARENT_NODE);
if ($parent instanceof ConstFetch) {
$scope = $node->getAttribute(AttributeKey::SCOPE);
if ($this->reflectionProvider->hasConstant($node, $scope)) {
$constantReflection = $this->reflectionProvider->getConstant($node, $scope);
if ($constantReflection instanceof RuntimeConstantReflection) {
return null;
}
}
if ($this->isNativeConstant($node)) {
return null;
}

return new FullyQualified($fullyQualifiedName);
});
}

private function isNativeConstant(Name $name): bool
{
$parent = $name->getAttribute(AttributeKey::PARENT_NODE);
if (! $parent instanceof ConstFetch) {
return false;
}

$scope = $name->getAttribute(AttributeKey::SCOPE);
if (! $this->reflectionProvider->hasConstant($name, $scope)) {
return false;
}

$constantReflection = $this->reflectionProvider->getConstant($name, $scope);
return $constantReflection instanceof RuntimeConstantReflection;
}
}
3 changes: 1 addition & 2 deletions src/Bootstrap/ExtensionConfigResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@ private function resolveIncludeFilePath(
array $extensionConfig,
string $generatedConfigDirectory,
string $includedFile
): ?string
{
): ?string {
if (! isset($extensionConfig['relative_install_path'])) {
return null;
}
Expand Down

0 comments on commit 733be85

Please sign in to comment.