Skip to content
Closed
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
@@ -0,0 +1,36 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\StmtsAwareInterface\InitVariableDefaultBeforeNullCoalesceReturnRector\Fixture;

class ArrayDimFill
{
public function run(array $keys): array
{
foreach ($keys as $key) {
$result[$key] = $key;
}

return $result ?? [];
}
}

?>
-----
<?php

namespace Rector\Tests\CodeQuality\Rector\StmtsAwareInterface\InitVariableDefaultBeforeNullCoalesceReturnRector\Fixture;

class ArrayDimFill
{
public function run(array $keys): array
{
$result = [];
foreach ($keys as $key) {
$result[$key] = $key;
}

return $result;
}
}

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

namespace Rector\Tests\CodeQuality\Rector\StmtsAwareInterface\InitVariableDefaultBeforeNullCoalesceReturnRector\Fixture;

class SkipAlreadyInitialized
{
public function run(array $keys): array
{
$result = [];
foreach ($keys as $key) {
$result[$key] = $key;
}

return $result ?? [];
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\StmtsAwareInterface\InitVariableDefaultBeforeNullCoalesceReturnRector\Fixture;

class SkipNoArrayDimWrite
{
public function run(array $config): array
{
return $config['items'] ?? [];
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\StmtsAwareInterface\InitVariableDefaultBeforeNullCoalesceReturnRector\Fixture;

class StringDefaultInIfElse
{
public function run(array $keys, array $values): array
{
foreach ($keys as $key) {
if (isset($values[$key])) {
$row[$key] = $values[$key];
} else {
$row[$key] = '';
}
}

return $row ?? [];
}
}

?>
-----
<?php

namespace Rector\Tests\CodeQuality\Rector\StmtsAwareInterface\InitVariableDefaultBeforeNullCoalesceReturnRector\Fixture;

class StringDefaultInIfElse
{
public function run(array $keys, array $values): array
{
$row = [];
foreach ($keys as $key) {
if (isset($values[$key])) {
$row[$key] = $values[$key];
} else {
$row[$key] = '';
}
}

return $row;
}
}

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

declare(strict_types=1);

namespace Rector\Tests\CodeQuality\Rector\StmtsAwareInterface\InitVariableDefaultBeforeNullCoalesceReturnRector;

use Iterator;
use PHPUnit\Framework\Attributes\DataProvider;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;

final class InitVariableDefaultBeforeNullCoalesceReturnRectorTest extends AbstractRectorTestCase
{
#[DataProvider('provideData')]
public function test(string $filePath): void
{
$this->doTestFile($filePath);
}

public static function provideData(): Iterator
{
return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
}

public function provideConfigFilePath(): string
{
return __DIR__ . '/config/configured_rule.php';
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

declare(strict_types=1);

use Rector\CodeQuality\Rector\StmtsAwareInterface\InitVariableDefaultBeforeNullCoalesceReturnRector;
use Rector\Config\RectorConfig;

return static function (RectorConfig $rectorConfig): void {
$rectorConfig->rule(InitVariableDefaultBeforeNullCoalesceReturnRector::class);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
<?php

declare(strict_types=1);

namespace Rector\CodeQuality\Rector\StmtsAwareInterface;

use PhpParser\Node;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\Array_;
use PhpParser\Node\Expr\ArrayDimFetch;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\BinaryOp\Coalesce;
use PhpParser\Node\Expr\ConstFetch;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Scalar;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Expression;
use PhpParser\Node\Stmt\Return_;
use Rector\PhpParser\Enum\NodeGroup;
use Rector\PhpParser\Node\BetterNodeFinder;
use Rector\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

/**
* @see \Rector\Tests\CodeQuality\Rector\StmtsAwareInterface\InitVariableDefaultBeforeNullCoalesceReturnRector\InitVariableDefaultBeforeNullCoalesceReturnRectorTest
*/
final class InitVariableDefaultBeforeNullCoalesceReturnRector extends AbstractRector
{
public function __construct(
private readonly BetterNodeFinder $betterNodeFinder
) {
}

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition(
'Initialize a variable with its default value before it is filled, instead of null coalescing a possibly undefined variable on return',
[
new CodeSample(
<<<'CODE_SAMPLE'
function run(array $keys): array
{
foreach ($keys as $key) {
$result[$key] = $key;
}

return $result ?? [];
}
CODE_SAMPLE
,
<<<'CODE_SAMPLE'
function run(array $keys): array
{
$result = [];
foreach ($keys as $key) {
$result[$key] = $key;
}

return $result;
}
CODE_SAMPLE
),
]
);
}

/**
* @return array<class-string<Node>>
*/
public function getNodeTypes(): array
{
return NodeGroup::STMTS_AWARE;
}

/**
* @param StmtsAware $node
*/
public function refactor(Node $node): ?Node
{
if ($node->stmts === null) {
return null;
}

$stmts = $node->stmts;

foreach ($stmts as $returnKey => $stmt) {
if (! $stmt instanceof Return_) {
continue;
}

if (! $stmt->expr instanceof Coalesce) {
continue;
}

$coalesce = $stmt->expr;
if (! $coalesce->left instanceof Variable) {
continue;
}

if (! $this->isLiteralDefault($coalesce->right)) {
continue;
}

$variableName = $this->getName($coalesce->left);
if ($variableName === null) {
continue;
}

if (! $this->isFilledOnlyAsArrayDim($stmts, $variableName)) {
continue;
}

$insertKey = $this->resolveFirstArrayDimWriteKey($stmts, $returnKey, $variableName);
if ($insertKey === null) {
continue;
}

$initExpression = new Expression(new Assign(new Variable($variableName), $coalesce->right));
array_splice($stmts, $insertKey, 0, [$initExpression]);

$stmt->expr = $coalesce->left;

$node->stmts = $stmts;

return $node;
}

return null;
}

private function isLiteralDefault(Expr $expr): bool
{
return $expr instanceof Array_ || $expr instanceof Scalar || $expr instanceof ConstFetch;
}

/**
* The variable must be written at least once as an array dimension and never assigned directly,
* otherwise the null coalesce guards a real value, not just an undefined variable.
*
* @param Stmt[] $stmts
*/
private function isFilledOnlyAsArrayDim(array $stmts, string $variableName): bool
{
$hasArrayDimAssign = false;

$assigns = $this->betterNodeFinder->findInstancesOf($stmts, [Assign::class]);
foreach ($assigns as $assign) {
if ($assign->var instanceof Variable && $this->isName($assign->var, $variableName)) {
return false;
}

if (! $assign->var instanceof ArrayDimFetch) {
continue;
}

$rootVariable = $this->resolveArrayDimRootVariable($assign->var);
if ($rootVariable instanceof Variable && $this->isName($rootVariable, $variableName)) {
$hasArrayDimAssign = true;
}
}

return $hasArrayDimAssign;
}

/**
* @param Stmt[] $stmts
*/
private function resolveFirstArrayDimWriteKey(array $stmts, int $returnKey, string $variableName): ?int
{
foreach ($stmts as $key => $stmt) {
if ($key >= $returnKey) {
return null;
}

$foundArrayDimAssign = $this->betterNodeFinder->findFirst($stmt, function (Node $subNode) use (
$variableName
): bool {
if (! $subNode instanceof Assign) {
return false;
}

if (! $subNode->var instanceof ArrayDimFetch) {
return false;
}

$expr = $this->resolveArrayDimRootVariable($subNode->var);
return $expr instanceof Variable && $this->isName($expr, $variableName);
});

if ($foundArrayDimAssign instanceof Node) {
return $key;
}
}

return null;
}

private function resolveArrayDimRootVariable(ArrayDimFetch $arrayDimFetch): Expr
{
$currentVariable = $arrayDimFetch->var;
while ($currentVariable instanceof ArrayDimFetch) {
$currentVariable = $currentVariable->var;
}

return $currentVariable;
}
}
2 changes: 2 additions & 0 deletions src/Config/Level/CodeQualityLevel.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
use Rector\CodeQuality\Rector\NotEqual\CommonNotEqualRector;
use Rector\CodeQuality\Rector\NullsafeMethodCall\CleanupUnneededNullsafeOperatorRector;
use Rector\CodeQuality\Rector\Property\FixClassCaseSensitivityVarDocblockRector;
use Rector\CodeQuality\Rector\StmtsAwareInterface\InitVariableDefaultBeforeNullCoalesceReturnRector;
use Rector\CodeQuality\Rector\StmtsAwareInterface\MoveInnerFunctionToTopLevelRector;
use Rector\CodeQuality\Rector\Switch_\SingularSwitchToIfRector;
use Rector\CodeQuality\Rector\Switch_\SwitchTrueToIfRector;
Expand Down Expand Up @@ -121,6 +122,7 @@ final class CodeQualityLevel
RepeatedOrEqualToInArrayRector::class,
RepeatedAndNotEqualToNotInArrayRector::class,
MoveInnerFunctionToTopLevelRector::class,
InitVariableDefaultBeforeNullCoalesceReturnRector::class,
InnerFunctionToPrivateMethodRector::class,
SimplifyForeachToCoalescingRector::class,
SimplifyFuncGetArgsCountRector::class,
Expand Down
Loading