add array dim fetch property unpacking

This commit is contained in:
Tomas Votruba 2019-11-02 18:42:23 +01:00
parent bf16bfe341
commit 564e9d418a
8 changed files with 159 additions and 25 deletions

View File

@ -97,7 +97,6 @@ PHP
return null; return null;
} }
if ($this->isSelfReferencing($node)) { if ($this->isSelfReferencing($node)) {
return null; return null;
} }
@ -160,7 +159,7 @@ PHP
private function isSelfReferencing(Assign $assign): bool private function isSelfReferencing(Assign $assign): bool
{ {
return (bool) $this->betterNodeFinder->findFirst($assign->expr, function ($subNode) use ($assign) : bool { return (bool) $this->betterNodeFinder->findFirst($assign->expr, function ($subNode) use ($assign): bool {
return $this->areNodesEqual($assign->var, $subNode); return $this->areNodesEqual($assign->var, $subNode);
}); });
} }

View File

@ -13,6 +13,7 @@ use PhpParser\Node\Stmt\Property;
use Rector\DeadCode\Analyzer\SetterOnlyMethodAnalyzer; use Rector\DeadCode\Analyzer\SetterOnlyMethodAnalyzer;
use Rector\NodeTypeResolver\Node\AttributeKey; use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\PhpParser\Node\Manipulator\AssignManipulator; use Rector\PhpParser\Node\Manipulator\AssignManipulator;
use Rector\PhpParser\Node\Manipulator\PropertyFetchManipulator;
use Rector\Rector\AbstractRector; use Rector\Rector\AbstractRector;
use Rector\RectorDefinition\CodeSample; use Rector\RectorDefinition\CodeSample;
use Rector\RectorDefinition\RectorDefinition; use Rector\RectorDefinition\RectorDefinition;
@ -34,12 +35,19 @@ final class RemoveSetterOnlyPropertyAndMethodCallRector extends AbstractRector
*/ */
private $assignManipulator; private $assignManipulator;
/**
* @var PropertyFetchManipulator
*/
private $propertyFetchManipulator;
public function __construct( public function __construct(
SetterOnlyMethodAnalyzer $setterOnlyMethodAnalyzer, SetterOnlyMethodAnalyzer $setterOnlyMethodAnalyzer,
AssignManipulator $assignManipulator AssignManipulator $assignManipulator,
PropertyFetchManipulator $propertyFetchManipulator
) { ) {
$this->setterOnlyMethodAnalyzer = $setterOnlyMethodAnalyzer; $this->setterOnlyMethodAnalyzer = $setterOnlyMethodAnalyzer;
$this->assignManipulator = $assignManipulator; $this->assignManipulator = $assignManipulator;
$this->propertyFetchManipulator = $propertyFetchManipulator;
} }
public function getDefinition(): RectorDefinition public function getDefinition(): RectorDefinition
@ -179,7 +187,11 @@ PHP
} }
/** @var Assign $node */ /** @var Assign $node */
$propertyFetch = $node->var; $propertyFetch = $this->propertyFetchManipulator->matchPropertyFetch($node->var);
if ($propertyFetch === null) {
return;
}
/** @var PropertyFetch $propertyFetch */ /** @var PropertyFetch $propertyFetch */
if ($this->isNames($propertyFetch->name, $propertyNames)) { if ($this->isNames($propertyFetch->name, $propertyNames)) {
$this->removeNode($node); $this->removeNode($node);

View File

@ -0,0 +1,25 @@
<?php
namespace Rector\DeadCode\Tests\Rector\Class_\RemoveSetterOnlyPropertyAndMethodCallRector\Fixture;
class RemoveDimFetch
{
private $name;
public function setName($name)
{
$this->name[] = $name;
}
}
?>
-----
<?php
namespace Rector\DeadCode\Tests\Rector\Class_\RemoveSetterOnlyPropertyAndMethodCallRector\Fixture;
class RemoveDimFetch
{
}
?>

View File

@ -0,0 +1,25 @@
<?php
namespace Rector\DeadCode\Tests\Rector\Class_\RemoveSetterOnlyPropertyAndMethodCallRector\Fixture;
class RemoveMultipleDimFetch
{
private $name;
public function setName($name)
{
$this->name[][][][][][][] = $name;
}
}
?>
-----
<?php
namespace Rector\DeadCode\Tests\Rector\Class_\RemoveSetterOnlyPropertyAndMethodCallRector\Fixture;
class RemoveMultipleDimFetch
{
}
?>

View File

@ -22,6 +22,10 @@ final class RemoveSetterOnlyPropertyAndMethodCallRectorTest extends AbstractRect
{ {
yield [__DIR__ . '/Fixture/fixture.php.inc']; yield [__DIR__ . '/Fixture/fixture.php.inc'];
yield [__DIR__ . '/Fixture/in_constructor.php.inc']; yield [__DIR__ . '/Fixture/in_constructor.php.inc'];
yield [__DIR__ . '/Fixture/remove_dim_fetch.php.inc'];
yield [__DIR__ . '/Fixture/remove_multiple_dim_fetch.php.inc'];
yield [__DIR__ . '/Fixture/keep_many_to_one.php.inc']; yield [__DIR__ . '/Fixture/keep_many_to_one.php.inc'];
yield [__DIR__ . '/Fixture/keep_static_property.php.inc']; yield [__DIR__ . '/Fixture/keep_static_property.php.inc'];
yield [__DIR__ . '/Fixture/keep_public_property.php.inc']; yield [__DIR__ . '/Fixture/keep_public_property.php.inc'];

View File

@ -6,12 +6,9 @@ namespace Rector\PhpParser\Node\Manipulator;
use PhpParser\Node; use PhpParser\Node;
use PhpParser\Node\Expr; use PhpParser\Node\Expr;
use PhpParser\Node\Expr\ArrayDimFetch;
use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\List_; use PhpParser\Node\Expr\List_;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\StaticPropertyFetch;
use Rector\PhpParser\Node\Resolver\NameResolver; use Rector\PhpParser\Node\Resolver\NameResolver;
final class AssignManipulator final class AssignManipulator
@ -21,9 +18,15 @@ final class AssignManipulator
*/ */
private $nameResolver; private $nameResolver;
public function __construct(NameResolver $nameResolver) /**
* @var PropertyFetchManipulator
*/
private $propertyFetchManipulator;
public function __construct(NameResolver $nameResolver, PropertyFetchManipulator $propertyFetchManipulator)
{ {
$this->nameResolver = $nameResolver; $this->nameResolver = $nameResolver;
$this->propertyFetchManipulator = $propertyFetchManipulator;
} }
/** /**
@ -37,9 +40,7 @@ final class AssignManipulator
return false; return false;
} }
$potentialPropertyFetch = $node->var instanceof ArrayDimFetch ? $node->var->var : $node->var; return (bool) $this->propertyFetchManipulator->matchPropertyFetch($node->var);
return $potentialPropertyFetch instanceof PropertyFetch || $potentialPropertyFetch instanceof StaticPropertyFetch;
} }
/** /**
@ -53,7 +54,11 @@ final class AssignManipulator
return false; return false;
} }
$propertyFetch = $node->var instanceof ArrayDimFetch ? $node->var->var : $node->var; /** @var Assign $node */
$propertyFetch = $this->propertyFetchManipulator->matchPropertyFetch($node->var);
if ($propertyFetch === null) {
return false;
}
return $this->nameResolver->isNames($propertyFetch, $propertyNames); return $this->nameResolver->isNames($propertyFetch, $propertyNames);
} }

View File

@ -5,6 +5,7 @@ declare(strict_types=1);
namespace Rector\PhpParser\Node\Manipulator; namespace Rector\PhpParser\Node\Manipulator;
use PhpParser\Node; use PhpParser\Node;
use PhpParser\Node\Expr\ArrayDimFetch;
use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\StaticCall; use PhpParser\Node\Expr\StaticCall;
@ -69,6 +70,11 @@ final class ClassManipulator
*/ */
private $betterStandardPrinter; private $betterStandardPrinter;
/**
* @var PropertyFetchManipulator
*/
private $propertyFetchManipulator;
public function __construct( public function __construct(
NameResolver $nameResolver, NameResolver $nameResolver,
NodeFactory $nodeFactory, NodeFactory $nodeFactory,
@ -76,7 +82,8 @@ final class ClassManipulator
CallableNodeTraverser $callableNodeTraverser, CallableNodeTraverser $callableNodeTraverser,
NodeRemovingCommander $nodeRemovingCommander, NodeRemovingCommander $nodeRemovingCommander,
DocBlockManipulator $docBlockManipulator, DocBlockManipulator $docBlockManipulator,
BetterStandardPrinter $betterStandardPrinter BetterStandardPrinter $betterStandardPrinter,
PropertyFetchManipulator $propertyFetchManipulator
) { ) {
$this->nodeFactory = $nodeFactory; $this->nodeFactory = $nodeFactory;
$this->nameResolver = $nameResolver; $this->nameResolver = $nameResolver;
@ -85,6 +92,7 @@ final class ClassManipulator
$this->nodeRemovingCommander = $nodeRemovingCommander; $this->nodeRemovingCommander = $nodeRemovingCommander;
$this->docBlockManipulator = $docBlockManipulator; $this->docBlockManipulator = $docBlockManipulator;
$this->betterStandardPrinter = $betterStandardPrinter; $this->betterStandardPrinter = $betterStandardPrinter;
$this->propertyFetchManipulator = $propertyFetchManipulator;
} }
public function addConstructorDependency(Class_ $classNode, string $name, Type $type): void public function addConstructorDependency(Class_ $classNode, string $name, Type $type): void
@ -298,7 +306,6 @@ final class ClassManipulator
/** @var string $methodName */ /** @var string $methodName */
$methodName = $this->nameResolver->getName($method); $methodName = $this->nameResolver->getName($method);
$publicMethodNames[] = $methodName; $publicMethodNames[] = $methodName;
} }
@ -317,15 +324,16 @@ final class ClassManipulator
$this->callableNodeTraverser->traverseNodesWithCallable([$node], function (Node $node) use ( $this->callableNodeTraverser->traverseNodesWithCallable([$node], function (Node $node) use (
&$propertyNonAssignNames &$propertyNonAssignNames
): void { ): void {
if (! $node instanceof PropertyFetch && ! $node instanceof StaticPropertyFetch) { $propertyFetch = $this->propertyFetchManipulator->matchPropertyFetch($node);
if ($propertyFetch === null) {
return; return;
} }
if (! $this->isNonAssignPropertyFetch($node)) { if (! $this->isNonAssignPropertyFetch($propertyFetch)) {
return; return;
} }
$propertyNonAssignNames[] = $this->nameResolver->getName($node); $propertyNonAssignNames[] = $this->nameResolver->getName($propertyFetch);
}); });
// skip serializable properties, because they are probably used in serialization even though assign only // skip serializable properties, because they are probably used in serialization even though assign only
@ -473,11 +481,7 @@ final class ClassManipulator
private function isNonAssignPropertyFetch(Node $node): bool private function isNonAssignPropertyFetch(Node $node): bool
{ {
if ($node instanceof PropertyFetch) { if ($node instanceof PropertyFetch) {
if (! $node->var instanceof Variable) { if (! $this->isLocalPropertyFetch($node)) {
return false;
}
if (! $this->nameResolver->isName($node->var, 'this')) {
return false; return false;
} }
@ -571,7 +575,32 @@ final class ClassManipulator
private function isNodeLeftPartOfAssign(Node $node): bool private function isNodeLeftPartOfAssign(Node $node): bool
{ {
$parentNode = $node->getAttribute(AttributeKey::PARENT_NODE); $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
if ($parentNode instanceof Assign && $parentNode->var === $node) {
return true;
}
return $parentNode instanceof Assign && $parentNode->var === $node; // traverse up to array dim fetches
if ($parentNode instanceof ArrayDimFetch) {
$previousParentNode = $parentNode;
while ($parentNode instanceof ArrayDimFetch) {
$previousParentNode = $parentNode;
$parentNode = $parentNode->getAttribute(AttributeKey::PARENT_NODE);
}
if ($parentNode instanceof Assign) {
return $parentNode->var === $previousParentNode;
}
}
return false;
}
private function isLocalPropertyFetch(PropertyFetch $propertyFetch): bool
{
if (! $propertyFetch->var instanceof Variable) {
return false;
}
return $this->nameResolver->isName($propertyFetch->var, 'this');
} }
} }

View File

@ -6,8 +6,10 @@ namespace Rector\PhpParser\Node\Manipulator;
use PhpParser\Node; use PhpParser\Node;
use PhpParser\Node\Expr; use PhpParser\Node\Expr;
use PhpParser\Node\Expr\ArrayDimFetch;
use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\StaticPropertyFetch;
use PhpParser\Node\Expr\Variable; use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Param; use PhpParser\Node\Param;
use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\Class_;
@ -60,13 +62,19 @@ final class PropertyFetchManipulator
NodeTypeResolver $nodeTypeResolver, NodeTypeResolver $nodeTypeResolver,
Broker $broker, Broker $broker,
NameResolver $nameResolver, NameResolver $nameResolver,
CallableNodeTraverser $callableNodeTraverser, CallableNodeTraverser $callableNodeTraverser
AssignManipulator $assignManipulator
) { ) {
$this->nodeTypeResolver = $nodeTypeResolver; $this->nodeTypeResolver = $nodeTypeResolver;
$this->broker = $broker; $this->broker = $broker;
$this->nameResolver = $nameResolver; $this->nameResolver = $nameResolver;
$this->callableNodeTraverser = $callableNodeTraverser; $this->callableNodeTraverser = $callableNodeTraverser;
}
/**
* @required
*/
public function autowirePropertyFetchManipulator(AssignManipulator $assignManipulator): void
{
$this->assignManipulator = $assignManipulator; $this->assignManipulator = $assignManipulator;
} }
@ -296,6 +304,33 @@ final class PropertyFetchManipulator
return null; return null;
} }
/**
* @param Node $node
* @return PropertyFetch|StaticPropertyFetch|null
*/
public function matchPropertyFetch(Node $node): ?Node
{
if ($node instanceof PropertyFetch) {
return $node;
}
if ($node instanceof StaticPropertyFetch) {
return $node;
}
if ($node instanceof ArrayDimFetch) {
$nestedNode = $node->var;
while ($nestedNode instanceof ArrayDimFetch) {
$nestedNode = $nestedNode->var;
}
return $this->matchPropertyFetch($nestedNode);
}
return null;
}
private function hasPublicProperty(PropertyFetch $propertyFetch, string $propertyName): bool private function hasPublicProperty(PropertyFetch $propertyFetch, string $propertyName): bool
{ {
$nodeScope = $propertyFetch->getAttribute(AttributeKey::SCOPE); $nodeScope = $propertyFetch->getAttribute(AttributeKey::SCOPE);