Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DeadCode] Skip RemoveUnusedPrivatePropertyRector on used in Assign expr #1214

Merged
merged 9 commits into from
Nov 11, 2021
Merged
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

namespace Rector\Tests\DeadCode\Rector\Property\RemoveUnusedPrivatePropertyRector\Fixture;

abstract class SkipPropertyFetchUsedInAssignExpr
{
private string $foo;
private string $bar;

public function __construct(string $foo, string $bar)
{
$this->foo = $foo;
$this->bar = $bar;
}

protected function calculateMac(): string
{
$foobar = $this->foo;
$foobar .= $this->bar;

return md5($foobar);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

namespace Rector\Tests\DeadCode\Rector\Property\RemoveUnusedPrivatePropertyRector\Fixture;

class SkipPropertyFetchUsedInAssignExpr2
{
private int $foo;

public function __construct(int $foo)
{
$this->foo = $foo;
}

public function calculateMac(): int
{
$foobar = $this->foo++;
return $foobar;
}
}
35 changes: 32 additions & 3 deletions rules/Removing/NodeManipulator/ComplexNodeRemover.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ public function removePropertyAndUsages(Property $property, array $classMethodNa

$propertyFetches = $this->propertyFetchFinder->findPrivatePropertyFetches($property);

$assigns = [];
$propertyNames = [];
foreach ($propertyFetches as $propertyFetch) {
if ($this->shouldSkipPropertyForClassMethod($propertyFetch, $classMethodNamesToSkip)) {
$shouldKeepProperty = true;
Expand All @@ -55,11 +57,13 @@ public function removePropertyAndUsages(Property $property, array $classMethodNa
return;
}

// remove assigns
$this->assignRemover->removeAssignNode($assign);
$this->removeConstructorDependency($assign);
$propertyName = (string) $this->nodeNameResolver->getName($propertyFetch);
$propertyNames[] = $propertyName;
$assigns[$propertyName][] = $assign;
}

$this->processRemovePropertyAssigns($assigns, $propertyNames);

if ($shouldKeepProperty) {
return;
}
Expand All @@ -79,6 +83,22 @@ public function removePropertyAndUsages(Property $property, array $classMethodNa
$this->nodeRemover->removeNode($property);
}

/**
* @param array<string, Assign[]> $assigns
* @param string[] $propertyNames
*/
private function processRemovePropertyAssigns(array $assigns, array $propertyNames): void
{
$propertyNames = array_unique($propertyNames);
foreach ($propertyNames as $propertyName) {
foreach ($assigns[$propertyName] as $propertyNameAssign) {
// remove assigns
$this->assignRemover->removeAssignNode($propertyNameAssign);
$this->removeConstructorDependency($propertyNameAssign);
}
}
}

/**
* @param string[] $classMethodNamesToSkip
*/
Expand Down Expand Up @@ -107,6 +127,15 @@ private function resolveAssign(PropertyFetch | StaticPropertyFetch $expr): ?Assi
return null;
}

$isInExpr = (bool) $this->betterNodeFinder->findFirst(
$assign->expr,
fn (Node $subNode): bool => $this->nodeComparator->areNodesEqual($subNode, $expr)
);

if ($isInExpr) {
return null;
}

return $assign;
}

Expand Down