Skip to content

Commit

Permalink
Remove PARENT_NODE from RemoveJustPropertyFetchRector (#3937)
Browse files Browse the repository at this point in the history
  • Loading branch information
@TomasVotruba
TomasVotruba committed May 23, 2023
1 parent 8a74ea7 commit d031c05
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 262 deletions.
21 changes: 0 additions & 21 deletions ...Fixture/use_in_anonymous_function.php.inc → ...re/skip_use_in_anonymous_function.php.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,3 @@ final class SkipUseInStaticFunction
fn () => $id();
}
}

?>
-----
<?php

namespace Rector\Tests\DeadCode\Rector\StmtsAwareInterface\RemoveJustPropertyFetchRector\Fixture;

final class SkipUseInStaticFunction
{
private $id;

public function run()
{
function () {
($this->id)();
};
fn () => ($this->id)();
}
}

?>
47 changes: 0 additions & 47 deletions ...StmtsAwareInterface/RemoveJustPropertyFetchRector/Fixture/with_simple_method_call.php.inc
View file

This file was deleted.

194 changes: 27 additions & 167 deletions rules/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,15 @@
namespace Rector\DeadCode\Rector\StmtsAwareInterface;

use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\AssignOp\Concat;
use PhpParser\Node\Expr\Closure;
use PhpParser\Node\Expr\ClosureUse;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Expression;
use PhpParser\Node\Stmt\While_;
use PhpParser\Node\Stmt\Return_;
use PHPStan\PhpDocParser\Ast\PhpDoc\VarTagValueNode;
use PHPStan\Type\ObjectType;
use Rector\Core\Contract\PhpParser\Node\StmtsAwareInterface;
use Rector\Core\Rector\AbstractRector;
use Rector\DeadCode\ValueObject\PropertyFetchToVariableAssign;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\ReadWrite\NodeAnalyzer\ReadExprAnalyzer;
use Rector\ReadWrite\NodeFinder\NodeUsageFinder;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

Expand All @@ -32,12 +22,6 @@
*/
final class RemoveJustPropertyFetchRector extends AbstractRector
{
public function __construct(
private readonly NodeUsageFinder $nodeUsageFinder,
private readonly ReadExprAnalyzer $readExprAnalyzer
) {
}

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('Inline property fetch assign to a variable, that has no added value', [
Expand Down Expand Up @@ -90,158 +74,50 @@ public function refactor(Node $node): ?Node
return null;
}

$variableUsages = [];
$currentStmtKey = null;

$variableToPropertyAssign = null;

foreach ($stmts as $key => $stmt) {
$variableToPropertyAssign = $this->matchVariableToPropertyAssign($stmt);
if (! $variableToPropertyAssign instanceof PropertyFetchToVariableAssign) {
if (! $stmt instanceof Return_) {
continue;
}

$assignPhpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($stmt);

// there is a @var tag on purpose, keep the assign
if ($assignPhpDocInfo->getVarTagValueNode() instanceof VarTagValueNode) {
if (! $stmt->expr instanceof Variable) {
continue;
}

$followingStmts = array_slice($stmts, $key + 1);
if ($this->isFollowingStatementStaticClosure($followingStmts)) {
// can not replace usages in anonymous static functions
$previousStmt = $stmts[$key - 1] ?? null;
if (! $previousStmt instanceof Expression) {
continue;
}

if (! $this->readExprAnalyzer->isExprRead($variableToPropertyAssign->getVariable())) {
$propertyFetch = $this->matchVariableToPropertyFetchAssign($previousStmt, $stmt->expr);
if (! $propertyFetch instanceof PropertyFetch) {
continue;
}

$variableUsages = $this->nodeUsageFinder->findVariableUsages(
$followingStmts,
$variableToPropertyAssign->getVariable()
);

$currentStmtKey = $key;

break;
}

// filter out variable usages that are part of nested property fetch, or change variable
$variableUsages = $this->filterOutReferencedVariableUsages($variableUsages);
if ($variableUsages === []) {
return null;
}

if ($this->isOverridenWithAssign($variableUsages)) {
return null;
}

if (! $variableToPropertyAssign instanceof PropertyFetchToVariableAssign) {
return null;
}

/** @var int $currentStmtKey */
return $this->replaceVariablesWithPropertyFetch(
$node,
$currentStmtKey,
$variableUsages,
$variableToPropertyAssign->getPropertyFetch()
);
}

/**
* @param Stmt[] $followingStmts
*/
private function isFollowingStatementStaticClosure(array $followingStmts): bool
{
if ($followingStmts === []) {
return false;
}

return $followingStmts[0] instanceof Expression
&& $followingStmts[0]->expr instanceof Closure
&& $followingStmts[0]->expr->static;
}

/**
* @param Variable[] $variableUsages
*/
private function replaceVariablesWithPropertyFetch(
StmtsAwareInterface $stmtsAware,
int $currentStmtsKey,
array $variableUsages,
PropertyFetch $propertyFetch
): StmtsAwareInterface {
// remove assign node
unset($stmtsAware->stmts[$currentStmtsKey]);

$this->traverseNodesWithCallable(
$stmtsAware,
function (Node $node) use ($variableUsages, $propertyFetch): ?PropertyFetch {
if (! in_array($node, $variableUsages, true)) {
return null;
}

$parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
if ($parentNode instanceof ClosureUse) {
// remove closure use which will be replaced by a property fetch
$this->removeNode($parentNode);

return null;
}

return $propertyFetch;
}
);

return $stmtsAware;
}

/**
* @param Variable[] $variableUsages
* @return Variable[]
*/
private function filterOutReferencedVariableUsages(array $variableUsages): array
{
return array_filter($variableUsages, function (Variable $variable): bool {
$variableUsageParent = $variable->getAttribute(AttributeKey::PARENT_NODE);
$assignPhpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($previousStmt);

if ($variableUsageParent instanceof Arg) {
$variableUsageParent = $variableUsageParent->getAttribute(AttributeKey::PARENT_NODE);
}

// skip nested property fetch, the assign is for purpose of named variable
if ($variableUsageParent instanceof PropertyFetch) {
return false;
// there is a @var tag on purpose, keep the assign
if ($assignPhpDocInfo->getVarTagValueNode() instanceof VarTagValueNode) {
continue;
}

// skip, as assign can be used in a loop
$parentWhile = $this->betterNodeFinder->findParentType($variable, While_::class);
if ($parentWhile instanceof While_) {
return false;
}
unset($node->stmts[$key - 1]);
$stmt->expr = $propertyFetch;

if (! $variableUsageParent instanceof FuncCall) {
return true;
}
return $node;
}

return ! $this->isName($variableUsageParent, 'array_pop');
});
return null;
}

private function matchVariableToPropertyAssign(Stmt $stmt): ?PropertyFetchToVariableAssign
{
if (! $stmt instanceof Expression) {
private function matchVariableToPropertyFetchAssign(
Expression $expression,
Variable $variable
): ?PropertyFetch {
if (! $expression->expr instanceof Assign) {
return null;
}

if (! $stmt->expr instanceof Assign) {
return null;
}

$assign = $stmt->expr;
$assign = $expression->expr;
if (! $assign->expr instanceof PropertyFetch) {
return null;
}
Expand All @@ -259,7 +135,11 @@ private function matchVariableToPropertyAssign(Stmt $stmt): ?PropertyFetchToVari
return null;
}

return new PropertyFetchToVariableAssign($assign->var, $assign->expr);
if (! $this->nodeComparator->areNodesEqual($variable, $assign->var)) {
return null;
}

return $assign->expr;
}

private function isPropertyFetchCallerNode(PropertyFetch $propertyFetch): bool
Expand All @@ -275,24 +155,4 @@ private function isPropertyFetchCallerNode(PropertyFetch $propertyFetch): bool
return $nodeObjectType->isSuperTypeOf($propertyFetchCallerType)
->yes();
}

/**
* @param Variable[] $variables
*/
private function isOverridenWithAssign(array $variables): bool
{
foreach ($variables as $variable) {
$parentNode = $variable->getAttribute(AttributeKey::PARENT_NODE);
if ($parentNode instanceof Concat && $parentNode->var === $variable) {
return true;
}

// variable is overriden
if ($parentNode instanceof Assign && $parentNode->var === $variable) {
return true;
}
}

return false;
}
}
27 changes: 0 additions & 27 deletions rules/DeadCode/ValueObject/PropertyFetchToVariableAssign.php
View file

This file was deleted.

0 comments on commit d031c05

Please sign in to comment.