Skip to content

Commit

Permalink
[DeadCode] Skip property fetch on cond as may be overridden on Remove…
Browse files Browse the repository at this point in the history
…AlwaysTrueIfConditionRector (#5796)

* [DeadCode] Skip property fetch overriden by next method call on RemoveAlwaysTrueIfConditionRector

* [ci-review] Rector Rectify

* clean up

* skip property fetch

* skip

* skip also ArrayDimFetch

* also ArrayDimfetch

---------

Co-authored-by: GitHub Action <actions@github.com>
  • Loading branch information
samsonasik and actions-user committed Apr 3, 2024
1 parent e503792 commit 91ed251
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 30 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php

namespace Rector\Tests\DeadCode\Rector\If_\RemoveAlwaysTrueIfConditionRector\Fixture;

$changes = [];
foreach (toAdd() as $add) {
$changes[$add['id']]['add'][] = 1;
}
foreach (toRem() as $del) {
$changes[$del['id']]['del'][] = 2;
}
foreach ($changes as $changeSet) {
if (isset($changeSet['del'])) {
echo 1;
}
if (isset($changeSet['add'])) {
echo 2;
}
}

/**
* @return array{array{id: int}}|array{}
*/
function toAdd(): array
{
if (rand(0,1)) {
return [];
}

return [
0 => [
'id' => 1,
],
];
}

/**
* @return array{array{id: int}}|array{}
*/
function toRem(): array
{
if (rand(0,1)) {
return [];
}

return [
0 => [
'id' => 1,
]
];
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

namespace Rector\Tests\DeadCode\Rector\If_\RemoveAlwaysTrueIfConditionRector\Fixture;

class SkipPropertyFetchOverriddenByNextMethodCall
{
private bool $executed;

public function run(): void
{
$this->executed = false;

$this->execute();

if (!$this->executed) {
echo 'not run';
}
}

protected function execute()
{
if (rand(0, 1)) {
$this->executed = true;
}
}
}
35 changes: 5 additions & 30 deletions rules/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use PhpParser\Node;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\ArrayDimFetch;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
use PhpParser\Node\Expr\PropertyFetch;
Expand All @@ -15,13 +16,11 @@
use PhpParser\Node\Stmt\Else_;
use PhpParser\Node\Stmt\If_;
use PhpParser\NodeTraverser;
use PHPStan\Reflection\ClassReflection;
use PHPStan\Type\Constant\ConstantBooleanType;
use Rector\DeadCode\NodeAnalyzer\SafeLeftTypeBooleanAndOrAnalyzer;
use Rector\NodeAnalyzer\ExprAnalyzer;
use Rector\PhpParser\Node\BetterNodeFinder;
use Rector\Rector\AbstractRector;
use Rector\Reflection\ReflectionResolver;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

Expand All @@ -31,7 +30,6 @@
final class RemoveAlwaysTrueIfConditionRector extends AbstractRector
{
public function __construct(
private readonly ReflectionResolver $reflectionResolver,
private readonly ExprAnalyzer $exprAnalyzer,
private readonly BetterNodeFinder $betterNodeFinder,
private readonly SafeLeftTypeBooleanAndOrAnalyzer $safeLeftTypeBooleanAndOrAnalyzer
Expand Down Expand Up @@ -107,7 +105,7 @@ public function refactor(Node $node): int|null|array|If_
return null;
}

if ($this->shouldSkipPropertyFetch($node->cond)) {
if ($this->shouldSkipExpr($node->cond)) {
return null;
}

Expand Down Expand Up @@ -141,35 +139,12 @@ private function shouldSkipFromParam(Expr $expr): bool
return false;
}

private function shouldSkipPropertyFetch(Expr $expr): bool
private function shouldSkipExpr(Expr $expr): bool
{
/** @var PropertyFetch[]|StaticPropertyFetch[] $propertyFetches */
$propertyFetches = $this->betterNodeFinder->findInstancesOf(
return (bool) $this->betterNodeFinder->findInstancesOf(
$expr,
[PropertyFetch::class, StaticPropertyFetch::class]
[PropertyFetch::class, StaticPropertyFetch::class, ArrayDimFetch::class]
);

foreach ($propertyFetches as $propertyFetch) {
$classReflection = $this->reflectionResolver->resolveClassReflectionSourceObject($propertyFetch);

if (! $classReflection instanceof ClassReflection) {
// cannot get parent Trait_ from Property Fetch
return true;
}

$propertyName = (string) $this->nodeNameResolver->getName($propertyFetch);

if (! $classReflection->hasNativeProperty($propertyName)) {
continue;
}

$nativeProperty = $classReflection->getNativeProperty($propertyName);
if (! $nativeProperty->hasNativeType()) {
return true;
}
}

return false;
}

private function refactorIfWithBooleanAnd(If_ $if): ?If_
Expand Down

0 comments on commit 91ed251

Please sign in to comment.