Skip to content

Commit

Permalink
[DeadCode] Handle anonymous and arrow function uses in RemoveJustProp…
Browse files Browse the repository at this point in the history
…ertyFetchRector (#2592)

* Add failing test cases for rectorphp/rector#7263

* Fix phpstan crash rectorphp/rector#7263

* Fix testcase

* Skip usages in anonymous static functions

* Remove no longer needed closure uses

* Add arrow functions to test cases

* Add arrow functions to test cases

* Don't throw exceptions

* Fix phpstan and code style

* Import used classes

* Remove work-around

* Remove phpstan error ignore

* Remove unused import
  • Loading branch information
kick-the-bucket committed Jun 30, 2022
1 parent 09df9d0 commit 82b2467
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

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

final class SkipUseInStaticFunction
{
private $id;

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

static function () use ($id) {
$id();
};
static fn () => $id();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<?php

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

final class SkipUseInStaticFunction
{
private $id;

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

function () use ($id) {
$id();
};
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)();
}
}

?>
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr\Assign;
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;
Expand Down Expand Up @@ -104,14 +106,18 @@ public function refactor(Node $node): ?Node
}

$followingStmts = array_slice($stmts, $key + 1);
if ($this->isFollowingStatementStaticClosure($followingStmts)) {
// can not replace usages in anonymous static functions
continue;
}

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

$currentStmtKey = $key;

// @todo validate the variable is not used in some place where property fetch cannot be used
break;
}

Expand All @@ -135,6 +141,17 @@ public function refactor(Node $node): ?Node
);
}

/**
* @param Stmt[] $followingStmts
*/
public function isFollowingStatementStaticClosure(array $followingStmts): bool
{
return count($followingStmts) > 0
&& $followingStmts[0] instanceof Expression
&& $followingStmts[0]->expr instanceof Closure
&& $followingStmts[0]->expr->static;
}

/**
* @param Variable[] $variableUsages
*/
Expand All @@ -149,11 +166,19 @@ private function replaceVariablesWithPropertyFetch(

$this->traverseNodesWithCallable(
$stmtsAware,
static function (Node $node) use ($variableUsages, $propertyFetch): ?PropertyFetch {
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->nodesToRemoveCollector->addNodeToRemove($parentNode);

return null;
}

return $propertyFetch;
}
);
Expand Down

0 comments on commit 82b2467

Please sign in to comment.