Skip to content

Commit

Permalink
[DeadCode] Skip isset() from property fetch from docblock on RemoveAl…
Browse files Browse the repository at this point in the history
…waysTrueIfConditionRector (#5754)

* [DeadCode] Skip isset() from property fetch from docblock on RemoveAlwaysTrueIfConditionRector

* Fix

* skip on array dim fetch as well

* skip on array dim fetch as well

* skip on array dim fetch as well

* fix

* Add SafeLeftTypeBooleanAndOrAnalyzer
  • Loading branch information
samsonasik committed Mar 21, 2024
1 parent dc69b1a commit b4eb883
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 12 deletions.
@@ -0,0 +1,22 @@
<?php

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

class SkipPropertyFetchDoc
{
/**
* @var string
*/
public $bodyFormat = '';

public function run()
{
if (isset($this->bodyFormat) && $this->bodyFormat !== '') {
}
}

public function reset()
{
$this->bodyFormat = null;
}
}
@@ -0,0 +1,17 @@
<?php

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

class SkipPropertyFetchDoc2
{
/**
* @var array<int, int>
*/
public $data = [];

public function run($key)
{
if (is_int($this->data[$key]) && $this->data[$key] > 0) {
}
}
}
42 changes: 42 additions & 0 deletions rules/DeadCode/NodeAnalyzer/SafeLeftTypeBooleanAndOrAnalyzer.php
@@ -0,0 +1,42 @@
<?php

declare(strict_types=1);

namespace Rector\DeadCode\NodeAnalyzer;

use PhpParser\Node;
use PhpParser\Node\Expr\ArrayDimFetch;
use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
use PhpParser\Node\Expr\BinaryOp\BooleanOr;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\StaticPropertyFetch;
use PhpParser\Node\Expr\Variable;
use Rector\NodeAnalyzer\ExprAnalyzer;
use Rector\PhpParser\Node\BetterNodeFinder;

final readonly class SafeLeftTypeBooleanAndOrAnalyzer
{
public function __construct(
private readonly BetterNodeFinder $betterNodeFinder,
private readonly ExprAnalyzer $exprAnalyzer
) {
}

public function isSafe(BooleanAnd|BooleanOr $booleanAnd): bool
{
$hasNonTypedFromParam = (bool) $this->betterNodeFinder->findFirst(
$booleanAnd->left,
fn (Node $node): bool => $node instanceof Variable && $this->exprAnalyzer->isNonTypedFromParam($node)
);

if ($hasNonTypedFromParam) {
return false;
}

// get type from Property and ArrayDimFetch is unreliable
return ! (bool) $this->betterNodeFinder->findFirst(
$booleanAnd->left,
static fn (Node $node): bool => $node instanceof PropertyFetch || $node instanceof StaticPropertyFetch || $node instanceof ArrayDimFetch
);
}
}
14 changes: 3 additions & 11 deletions rules/DeadCode/Rector/If_/ReduceAlwaysFalseIfOrRector.php
Expand Up @@ -6,11 +6,9 @@

use PhpParser\Node;
use PhpParser\Node\Expr\BinaryOp\BooleanOr;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Stmt\If_;
use PHPStan\Type\Constant\ConstantBooleanType;
use Rector\NodeAnalyzer\ExprAnalyzer;
use Rector\PhpParser\Node\BetterNodeFinder;
use Rector\DeadCode\NodeAnalyzer\SafeLeftTypeBooleanAndOrAnalyzer;
use Rector\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
Expand All @@ -21,8 +19,7 @@
final class ReduceAlwaysFalseIfOrRector extends AbstractRector
{
public function __construct(
private readonly BetterNodeFinder $betterNodeFinder,
private readonly ExprAnalyzer $exprAnalyzer
private readonly SafeLeftTypeBooleanAndOrAnalyzer $safeLeftTypeBooleanAndOrAnalyzer
) {
}

Expand Down Expand Up @@ -90,12 +87,7 @@ public function refactor(Node $node): ?Node
return null;
}

$hasNonTypedFromParam = $this->betterNodeFinder->findFirst(
$booleanOr->left,
fn (Node $node): bool => $node instanceof Variable && $this->exprAnalyzer->isNonTypedFromParam($node)
);

if ($hasNonTypedFromParam instanceof Node) {
if (! $this->safeLeftTypeBooleanAndOrAnalyzer->isSafe($booleanOr)) {
return null;
}

Expand Down
Expand Up @@ -17,6 +17,7 @@
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;
Expand All @@ -32,7 +33,8 @@ final class RemoveAlwaysTrueIfConditionRector extends AbstractRector
public function __construct(
private readonly ReflectionResolver $reflectionResolver,
private readonly ExprAnalyzer $exprAnalyzer,
private readonly BetterNodeFinder $betterNodeFinder
private readonly BetterNodeFinder $betterNodeFinder,
private readonly SafeLeftTypeBooleanAndOrAnalyzer $safeLeftTypeBooleanAndOrAnalyzer
) {
}

Expand Down Expand Up @@ -177,6 +179,7 @@ private function refactorIfWithBooleanAnd(If_ $if): ?If_
}

$booleanAnd = $if->cond;

$leftType = $this->getType($booleanAnd->left);
if (! $leftType instanceof ConstantBooleanType) {
return null;
Expand All @@ -186,6 +189,10 @@ private function refactorIfWithBooleanAnd(If_ $if): ?If_
return null;
}

if (! $this->safeLeftTypeBooleanAndOrAnalyzer->isSafe($booleanAnd)) {
return null;
}

$if->cond = $booleanAnd->right;
return $if;
}
Expand Down

0 comments on commit b4eb883

Please sign in to comment.