Skip to content

Commit ee3da33

Browse files
committed
[code-quality] Improve InlineArrayReturnAssignRector to include implicit variable init and nested keys
1 parent 87b1e0d commit ee3da33

File tree

13 files changed

+253
-199
lines changed

13 files changed

+253
-199
lines changed

phpstan.neon

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,3 +338,7 @@ parameters:
338338
-
339339
message: '#Offset float\|int\|string might not exist on string#'
340340
path: rules/Php70/EregToPcreTransformer.php
341+
342+
-
343+
identifier: symplify.noReference
344+
message: '#Use explicit return value over magic &reference#'
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
3+
namespace Rector\Tests\CodeQuality\Rector\ClassMethod\InlineArrayReturnAssignRector\Fixture;
4+
5+
final class IncludeNestedVariable
6+
{
7+
public function create()
8+
{
9+
$items[0]['name'] = 'John';
10+
$items[0]['surname'] = 'Doe';
11+
$items[0]['age'] = 50;
12+
return $items;
13+
}
14+
}
15+
16+
?>
17+
-----
18+
<?php
19+
20+
namespace Rector\Tests\CodeQuality\Rector\ClassMethod\InlineArrayReturnAssignRector\Fixture;
21+
22+
final class IncludeNestedVariable
23+
{
24+
public function create()
25+
{
26+
return [['name' => 'John', 'surname' => 'Doe', 'age' => 50]];
27+
}
28+
}
29+
30+
?>

rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/keep_comments.php.inc

Lines changed: 0 additions & 38 deletions
This file was deleted.
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
namespace Rector\Tests\CodeQuality\Rector\ClassMethod\InlineArrayReturnAssignRector\Fixture;
4+
5+
final class NestedArrayWithMultipleKeys
6+
{
7+
public function run()
8+
{
9+
$arr = [];
10+
$arr['foo']['bar']['baz'] = ['a' => 1, 'b' => 2];
11+
12+
return $arr;
13+
}
14+
}
15+
16+
17+
?>
18+
-----
19+
<?php
20+
21+
namespace Rector\Tests\CodeQuality\Rector\ClassMethod\InlineArrayReturnAssignRector\Fixture;
22+
23+
final class NestedArrayWithMultipleKeys
24+
{
25+
public function run()
26+
{
27+
return ['foo' => ['bar' => ['baz' => ['a' => 1, 'b' => 2]]]];
28+
}
29+
}
30+
31+
32+
?>
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
namespace Rector\Tests\CodeQuality\Rector\ClassMethod\InlineArrayReturnAssignRector\Fixture;
4+
5+
final class SkipDifferentVariable
6+
{
7+
public function create()
8+
{
9+
$item['sub_key'] = 500;
10+
11+
$items['name'] = 'John';
12+
$items['surname'] = 'Doe';
13+
$items['age'] = $item;
14+
15+
return $items;
16+
}
17+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
3+
namespace Rector\Tests\CodeQuality\Rector\ClassMethod\InlineArrayReturnAssignRector\Fixture;
4+
5+
final class SkipMultipleReturns
6+
{
7+
public function create()
8+
{
9+
$items['name'] = 'John';
10+
$items['surname'] = 'Doe';
11+
$items['age'] = 50;
12+
13+
if (mt_rand(1, 2)) {
14+
return [];
15+
}
16+
17+
return $items;
18+
}
19+
}

rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/skip_nested_array.php.inc

Lines changed: 0 additions & 15 deletions
This file was deleted.

rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/skip_reuse_variable_in_assign_expr.php.inc

Lines changed: 0 additions & 18 deletions
This file was deleted.
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
3+
namespace Rector\Tests\CodeQuality\Rector\ClassMethod\InlineArrayReturnAssignRector\Fixture;
4+
5+
class SomeClass
6+
{
7+
public function create()
8+
{
9+
$items['name'] = 'John';
10+
$items['surname'] = 'Doe';
11+
$items['age'] = 50;
12+
return $items;
13+
}
14+
}
15+
16+
?>
17+
-----
18+
<?php
19+
20+
namespace Rector\Tests\CodeQuality\Rector\ClassMethod\InlineArrayReturnAssignRector\Fixture;
21+
22+
class SomeClass
23+
{
24+
public function create()
25+
{
26+
return ['name' => 'John', 'surname' => 'Doe', 'age' => 50];
27+
}
28+
}
29+
30+
?>

rules/CodeQuality/NodeAnalyzer/VariableDimFetchAssignResolver.php

Lines changed: 43 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -4,34 +4,44 @@
44

55
namespace Rector\CodeQuality\NodeAnalyzer;
66

7-
use PhpParser\Node;
87
use PhpParser\Node\Expr;
98
use PhpParser\Node\Expr\ArrayDimFetch;
109
use PhpParser\Node\Expr\Assign;
1110
use PhpParser\Node\Expr\Variable;
1211
use PhpParser\Node\Stmt;
1312
use PhpParser\Node\Stmt\Expression;
13+
use PhpParser\Node\Stmt\Return_;
1414
use Rector\CodeQuality\ValueObject\KeyAndExpr;
15-
use Rector\PhpParser\Comparing\NodeComparator;
15+
use Rector\NodeNameResolver\NodeNameResolver;
1616
use Rector\PhpParser\Node\BetterNodeFinder;
17+
use Rector\PhpParser\Node\Value\ValueResolver;
1718

1819
final readonly class VariableDimFetchAssignResolver
1920
{
2021
public function __construct(
21-
private NodeComparator $nodeComparator,
22-
private BetterNodeFinder $betterNodeFinder
22+
private ValueResolver $valueResolver
2323
) {
2424
}
2525

2626
/**
2727
* @param Stmt[] $stmts
28-
* @return KeyAndExpr[]
28+
* @return array<mixed, KeyAndExpr[]>
2929
*/
30-
public function resolveFromStmtsAndVariable(array $stmts, Variable $variable): array
30+
public function resolveFromStmtsAndVariable(array $stmts, ?Assign $emptyArrayAssign): array
3131
{
32-
$keysAndExprs = [];
32+
$exprs = [];
33+
34+
$key = 0;
3335

3436
foreach ($stmts as $stmt) {
37+
if ($stmt instanceof Expression && $stmt->expr === $emptyArrayAssign) {
38+
continue;
39+
}
40+
41+
if ($stmt instanceof Return_) {
42+
continue;
43+
}
44+
3545
if (! $stmt instanceof Expression) {
3646
return [];
3747
}
@@ -43,66 +53,46 @@ public function resolveFromStmtsAndVariable(array $stmts, Variable $variable): a
4353

4454
$assign = $stmtExpr;
4555

46-
$keyExpr = $this->matchKeyOnArrayDimFetchOfVariable($assign, $variable);
47-
if ($assign->var instanceof ArrayDimFetch && $assign->var->var instanceof ArrayDimFetch) {
48-
return [];
49-
}
50-
51-
$keysAndExprs[] = new KeyAndExpr($keyExpr, $assign->expr, $stmt->getComments());
52-
}
56+
$dimValues = [];
5357

54-
// we can only work with same variable
55-
// and exclusively various keys or empty keys
56-
if (! $this->hasExclusivelyNullKeyOrFilledKey($keysAndExprs)) {
57-
return [];
58-
}
58+
$arrayDimFetch = $assign->var;
59+
while ($arrayDimFetch instanceof ArrayDimFetch) {
60+
$dimValues[] = $arrayDimFetch->dim instanceof Expr ? $this->valueResolver->getValue(
61+
$arrayDimFetch->dim
62+
) : $key;
5963

60-
return $keysAndExprs;
61-
}
64+
$arrayDimFetch = $arrayDimFetch->var;
65+
}
6266

63-
private function matchKeyOnArrayDimFetchOfVariable(Assign $assign, Variable $variable): ?Expr
64-
{
65-
if (! $assign->var instanceof ArrayDimFetch) {
66-
return null;
67-
}
67+
++$key;
6868

69-
$arrayDimFetch = $assign->var;
70-
if (! $this->nodeComparator->areNodesEqual($arrayDimFetch->var, $variable)) {
71-
return null;
69+
$this->setNestedKeysExpr($exprs, $dimValues, $assign->expr);
7270
}
7371

74-
$isFoundInExpr = (bool) $this->betterNodeFinder->findFirst(
75-
$assign->expr,
76-
fn (Node $subNode): bool => $this->nodeComparator->areNodesEqual($subNode, $variable)
77-
);
78-
79-
if ($isFoundInExpr) {
80-
return null;
81-
}
72+
// we can only work with same variable
73+
// and exclusively various keys or empty keys
74+
// if (! $this->hasExclusivelyNullKeyOrFilledKey($exprsByKeys)) {
75+
// return [];
76+
// }
8277

83-
return $arrayDimFetch->dim;
78+
return $exprs;
8479
}
8580

8681
/**
87-
* @param KeyAndExpr[] $keysAndExprs
82+
* @param mixed[] $exprsByKeys
83+
* @param array<string|int> $keys
8884
*/
89-
private function hasExclusivelyNullKeyOrFilledKey(array $keysAndExprs): bool
85+
private function setNestedKeysExpr(array &$exprsByKeys, array $keys, Expr $expr): void
9086
{
91-
$alwaysNullKey = true;
92-
$alwaysStringKey = true;
93-
94-
foreach ($keysAndExprs as $keyAndExpr) {
95-
if ($keyAndExpr->getKeyExpr() instanceof Expr) {
96-
$alwaysNullKey = false;
97-
} else {
98-
$alwaysStringKey = false;
99-
}
100-
}
87+
$reference = &$exprsByKeys;
88+
89+
$keys = array_reverse($keys);
10190

102-
if ($alwaysNullKey) {
103-
return true;
91+
foreach ($keys as $key) {
92+
// create intermediate arrays automatically
93+
$reference = &$reference[$key];
10494
}
10595

106-
return $alwaysStringKey;
96+
$reference = $expr;
10797
}
10898
}

0 commit comments

Comments
 (0)