Skip to content

Commit

Permalink
Fix var/property usage for RemoveUnusedNonEmptyArrayBeforeForeachRect…
Browse files Browse the repository at this point in the history
…or. (#3040)
  • Loading branch information
Wohlie committed Nov 10, 2022
1 parent 10c6a30 commit baf8394
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 12 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php

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

class IfCheck
{
public function run()
{
$values = [];
if ($values) {
foreach ($values as $value) {
echo $value;
}
}
}
}

?>
-----
<?php

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

class IfCheck
{
public function run()
{
$values = [];
foreach ($values as $value) {
echo $value;
}
}
}

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

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

class SkipIfClassProperty
{
/** @var string[] */
private array $allowedValues = [];

protected function filterValues(array $values): array
{
if ($this->allowedValues) {
foreach ($values as $index => $value) {
if (!isset($this->allowedValues[$value])) {
unset($values[$index]);
}
}
}

return $values;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

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

class SkipPropertyNull
{
private $items = null;

public function run()
{
if ($this->items) {
foreach ($this->items as $value) {
echo $value;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use PhpParser\Node\Stmt\If_;
use PHPStan\Analyser\Scope;
use PHPStan\Type\ArrayType;
use Rector\Core\NodeAnalyzer\PropertyFetchAnalyzer;
use Rector\Core\NodeManipulator\IfManipulator;
use Rector\Core\Php\ReservedKeywordAnalyzer;
use Rector\Core\Rector\AbstractScopeAwareRector;
Expand All @@ -30,7 +31,8 @@ public function __construct(
private readonly CountManipulator $countManipulator,
private readonly IfManipulator $ifManipulator,
private readonly UselessIfCondBeforeForeachDetector $uselessIfCondBeforeForeachDetector,
private readonly ReservedKeywordAnalyzer $reservedKeywordAnalyzer
private readonly ReservedKeywordAnalyzer $reservedKeywordAnalyzer,
private readonly PropertyFetchAnalyzer $propertyFetchAnalyzer
) {
}

Expand Down Expand Up @@ -92,11 +94,6 @@ public function refactorWithScope(Node $node, Scope $scope): array|Node|null

/** @var Foreach_ $stmt */
$stmt = $node->stmts[0];

if ($node->cond instanceof Assign) {
return null;
}

$ifComments = $node->getAttribute(AttributeKey::COMMENTS) ?? [];
$stmtComments = $stmt->getAttribute(AttributeKey::COMMENTS) ?? [];

Expand All @@ -123,17 +120,17 @@ private function isUselessBeforeForeachCheck(If_ $if, Scope $scope): bool
}
}

if ($this->uselessIfCondBeforeForeachDetector->isMatchingNotIdenticalEmptyArray($if, $foreachExpr)) {
return true;
if (($if->cond instanceof Variable || $this->propertyFetchAnalyzer->isPropertyFetch($if->cond))
&& $this->nodeComparator->areNodesEqual($if->cond, $foreachExpr)
) {
return $scope->getType($if->cond) instanceof ArrayType;
}

if ($this->uselessIfCondBeforeForeachDetector->isMatchingNotEmpty($if, $foreachExpr, $scope)) {
if ($this->uselessIfCondBeforeForeachDetector->isMatchingNotIdenticalEmptyArray($if, $foreachExpr)) {
return true;
}

// we know it's an array
$condType = $this->getType($if->cond);
if ($condType instanceof ArrayType) {
if ($this->uselessIfCondBeforeForeachDetector->isMatchingNotEmpty($if, $foreachExpr, $scope)) {
return true;
}

Expand Down

0 comments on commit baf8394

Please sign in to comment.