Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,14 @@
namespace Rector\Tests\CodeQuality\Rector\Empty_\SimplifyEmptyCheckOnEmptyArrayRector;

use Iterator;
use PhpParser\Node\Expr\Array_;
use PhpParser\Node\Expr\BooleanNot;
use PhpParser\Node\Expr\Empty_;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Name;
use PHPUnit\Framework\Attributes\DataProvider;
use Rector\CodeQuality\Rector\Empty_\SimplifyEmptyCheckOnEmptyArrayRector;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;

final class SimplifyEmptyCheckOnEmptyArrayRectorTest extends AbstractRectorTestCase
Expand All @@ -21,6 +28,26 @@ public static function provideData(): Iterator
return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
}

#[DataProvider('provideUnsupportedNodes')]
public function testSkipsUnsupportedNodesWithoutScope(BooleanNot|Empty_ $expr): void
{
$rector = self::getContainer()->make(SimplifyEmptyCheckOnEmptyArrayRector::class);

$this->assertNull($rector->refactor($expr));
}

/**
* @return Iterator<string, array{BooleanNot|Empty_}>
*/
public static function provideUnsupportedNodes(): Iterator
{
yield 'plain negation' => [new BooleanNot(new Variable('values'))];
yield 'empty array literal' => [new Empty_(new Array_())];
yield 'negated empty function call' => [new BooleanNot(new Empty_(new FuncCall(new Name('getValues'))))];
yield 'superglobal get' => [new Empty_(new Variable('_GET'))];
yield 'negated superglobal post' => [new BooleanNot(new Empty_(new Variable('_POST')))];
}

public function provideConfigFilePath(): string
{
return __DIR__ . '/config/configured_rule.php';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
namespace Rector\CodeQuality\Rector\Empty_;

use PhpParser\Node;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\Array_;
use PhpParser\Node\Expr\BinaryOp\Identical;
use PhpParser\Node\Expr\BinaryOp\NotIdentical;
Expand Down Expand Up @@ -79,43 +78,36 @@ public function getNodeTypes(): array
*/
public function refactor(Node $node): ?Node
{
$scope = ScopeFetcher::fetch($node);
if ($node instanceof BooleanNot) {
if ($node->expr instanceof Empty_ && $this->isAllowedExpr($node->expr->expr, $scope)) {
return new NotIdentical($node->expr->expr, new Array_());
}

$empty = $node instanceof BooleanNot ? $node->expr : $node;
if (! $empty instanceof Empty_) {
return null;
}

if (! $this->isAllowedExpr($node->expr, $scope)) {
$expr = $empty->expr;
if (! $expr instanceof Variable && ! $expr instanceof PropertyFetch && ! $expr instanceof StaticPropertyFetch) {
return null;
}

return new Identical($node->expr, new Array_());
}
if ($expr instanceof Variable && is_string($expr->name) && $this->reservedKeywordAnalyzer->isNativeVariable($expr->name)) {
return null;
}

private function isAllowedVariable(Variable $variable): bool
{
if (is_string($variable->name) && $this->reservedKeywordAnalyzer->isNativeVariable($variable->name)) {
return false;
$scope = ScopeFetcher::fetch($node);
if (! $this->isAllowedExpr($expr, $scope)) {
return null;
}

return ! $this->exprAnalyzer->isNonTypedFromParam($variable);
return $node instanceof BooleanNot ? new NotIdentical($expr, new Array_()) : new Identical($expr, new Array_());
}

private function isAllowedExpr(Expr $expr, Scope $scope): bool
private function isAllowedExpr(Variable|PropertyFetch|StaticPropertyFetch $expr, Scope $scope): bool
{
if (! $expr instanceof Variable && ! $expr instanceof PropertyFetch && ! $expr instanceof StaticPropertyFetch) {
return false;
}

if (! $scope->getType($expr)->isArray()->yes()) {
return false;
}

if ($expr instanceof Variable) {
return $this->isAllowedVariable($expr);
return ! $this->exprAnalyzer->isNonTypedFromParam($expr);
}

if (! $expr->name instanceof Identifier) {
Expand Down
Loading