Skip to content

Commit 9548641

Browse files
TomasVotrubaactions-user
TomasVotruba
and
actions-user
authored
[tdd] Add AddVarArrayDocblockFromDimFetchAssignRector (#7801)
* [tdd] Add AddVarArrayDocblockFromDimFetchAssignRector * [ci-review] Rector Rectify --------- Co-authored-by: GitHub Action <actions@github.com>
1 parent 2681751 commit 9548641

9 files changed

Lines changed: 331 additions & 1 deletion

File tree

‎rules-tests/TypeDeclarationDocblocks/Rector/Class_/AddVarArrayDocblockFromDimFetchAssignRector/AddVarArrayDocblockFromDimFetchAssignRectorTest.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Tests\TypeDeclarationDocblocks\Rector\Class_\AddVarArrayDocblockFromDimFetchAssignRector;
6+
7+
use Iterator;
8+
use PHPUnit\Framework\Attributes\DataProvider;
9+
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
10+
11+
final class AddVarArrayDocblockFromDimFetchAssignRectorTest extends AbstractRectorTestCase
12+
{
13+
#[DataProvider('provideData')]
14+
public function test(string $filePath): void
15+
{
16+
$this->doTestFile($filePath);
17+
}
18+
19+
public static function provideData(): Iterator
20+
{
21+
return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
22+
}
23+
24+
public function provideConfigFilePath(): string
25+
{
26+
return __DIR__ . '/config/configured_rule.php';
27+
}
28+
}

‎rules-tests/TypeDeclarationDocblocks/Rector/Class_/AddVarArrayDocblockFromDimFetchAssignRector/Fixture/multiple_assigns.php.inc‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?php
2+
3+
namespace Rector\Tests\TypeDeclarationDocblocks\Rector\Class_\AddVarArrayDocblockFromDimFetchAssignRector\Fixture;
4+
5+
final class MultipleAssigns
6+
{
7+
private array $items = [];
8+
9+
public function go()
10+
{
11+
$this->items[] = 'item1';
12+
}
13+
14+
public function run()
15+
{
16+
$this->items[] = 'item2';
17+
}
18+
}
19+
20+
?>
21+
-----
22+
<?php
23+
24+
namespace Rector\Tests\TypeDeclarationDocblocks\Rector\Class_\AddVarArrayDocblockFromDimFetchAssignRector\Fixture;
25+
26+
final class MultipleAssigns
27+
{
28+
/**
29+
* @var string[]
30+
*/
31+
private array $items = [];
32+
33+
public function go()
34+
{
35+
$this->items[] = 'item1';
36+
}
37+
38+
public function run()
39+
{
40+
$this->items[] = 'item2';
41+
}
42+
}
43+
44+
?>

‎rules-tests/TypeDeclarationDocblocks/Rector/Class_/AddVarArrayDocblockFromDimFetchAssignRector/Fixture/skp_already_known.php.inc‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
namespace Rector\Tests\TypeDeclarationDocblocks\Rector\Class_\AddVarArrayDocblockFromDimFetchAssignRector\Fixture;
4+
5+
final class SkipAlreadyKnown
6+
{
7+
/**
8+
* @var list<string>
9+
*/
10+
private array $items = [];
11+
12+
public function go()
13+
{
14+
$this->items[] = 'item1';
15+
}
16+
}

‎rules-tests/TypeDeclarationDocblocks/Rector/Class_/AddVarArrayDocblockFromDimFetchAssignRector/Fixture/some_class.php.inc‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php
2+
3+
namespace Rector\Tests\TypeDeclarationDocblocks\Rector\Class_\AddVarArrayDocblockFromDimFetchAssignRector\Fixture;
4+
5+
final class SomeClass
6+
{
7+
private array $items = [];
8+
9+
public function go()
10+
{
11+
$this->items[] = 'item1';
12+
}
13+
}
14+
15+
?>
16+
-----
17+
<?php
18+
19+
namespace Rector\Tests\TypeDeclarationDocblocks\Rector\Class_\AddVarArrayDocblockFromDimFetchAssignRector\Fixture;
20+
21+
final class SomeClass
22+
{
23+
/**
24+
* @var string[]
25+
*/
26+
private array $items = [];
27+
28+
public function go()
29+
{
30+
$this->items[] = 'item1';
31+
}
32+
}
33+
34+
?>

‎rules-tests/TypeDeclarationDocblocks/Rector/Class_/AddVarArrayDocblockFromDimFetchAssignRector/config/configured_rule.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
use Rector\Config\RectorConfig;
6+
use Rector\TypeDeclarationDocblocks\Rector\Class_\AddVarArrayDocblockFromDimFetchAssignRector;
7+
8+
return static function (RectorConfig $rectorConfig): void {
9+
$rectorConfig->rule(AddVarArrayDocblockFromDimFetchAssignRector::class);
10+
};

‎rules/TypeDeclarationDocblocks/NodeFinder/ArrayDimFetchFinder.php‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@
88
use PhpParser\Node\Expr;
99
use PhpParser\Node\Expr\ArrayDimFetch;
1010
use PhpParser\Node\Expr\Assign;
11+
use PhpParser\Node\Expr\PropertyFetch;
1112
use PhpParser\Node\Expr\Variable;
13+
use PhpParser\Node\Stmt\Class_;
1214
use PhpParser\Node\Stmt\ClassMethod;
1315
use Rector\NodeNameResolver\NodeNameResolver;
1416
use Rector\PhpParser\Node\BetterNodeFinder;
@@ -49,6 +51,45 @@ public function findDimFetchAssignToVariableName(ClassMethod $classMethod, strin
4951
return $exprs;
5052
}
5153

54+
/**
55+
* Look for bare assigns, $this->someProperty[] = ...
56+
* @return Expr[]
57+
*/
58+
public function findDimFetchAssignToPropertyName(Class_ $class, string $variableName): array
59+
{
60+
$assigns = $this->betterNodeFinder->findInstancesOfScoped($class->getMethods(), Assign::class);
61+
62+
$exprs = [];
63+
foreach ($assigns as $assign) {
64+
if (! $assign->var instanceof ArrayDimFetch) {
65+
continue;
66+
}
67+
68+
$arrayDimFetch = $assign->var;
69+
if ($arrayDimFetch->dim instanceof Expr) {
70+
continue;
71+
}
72+
73+
if (! $arrayDimFetch->var instanceof PropertyFetch) {
74+
continue;
75+
}
76+
77+
$propertyFetch = $arrayDimFetch->var;
78+
79+
if (! $this->nodeNameResolver->isName($propertyFetch->var, 'this')) {
80+
continue;
81+
}
82+
83+
if (! $this->nodeNameResolver->isName($propertyFetch->name, $variableName)) {
84+
continue;
85+
}
86+
87+
$exprs[] = $assign->expr;
88+
}
89+
90+
return $exprs;
91+
}
92+
5293
/**
5394
* @return ArrayDimFetch[]
5495
*/

‎rules/TypeDeclarationDocblocks/Rector/Class_/AddVarArrayDocblockFromDimFetchAssignRector.php‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\TypeDeclarationDocblocks\Rector\Class_;
6+
7+
use PhpParser\Node;
8+
use PhpParser\Node\Identifier;
9+
use PhpParser\Node\Stmt\Class_;
10+
use PhpParser\Node\Stmt\Property;
11+
use PHPStan\Type\ArrayType;
12+
use PHPStan\Type\MixedType;
13+
use PHPStan\Type\UnionType;
14+
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
15+
use Rector\NodeTypeResolver\PHPStan\Type\TypeFactory;
16+
use Rector\Rector\AbstractRector;
17+
use Rector\TypeDeclarationDocblocks\NodeDocblockTypeDecorator;
18+
use Rector\TypeDeclarationDocblocks\NodeFinder\ArrayDimFetchFinder;
19+
use Rector\TypeDeclarationDocblocks\TagNodeAnalyzer\UsefulArrayTagNodeAnalyzer;
20+
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
21+
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
22+
23+
/**
24+
* @see \Rector\Tests\TypeDeclarationDocblocks\Rector\Class_\AddVarArrayDocblockFromDimFetchAssignRector\AddVarArrayDocblockFromDimFetchAssignRectorTest
25+
*/
26+
final class AddVarArrayDocblockFromDimFetchAssignRector extends AbstractRector
27+
{
28+
public function __construct(
29+
private readonly TypeFactory $typeFactory,
30+
private readonly PhpDocInfoFactory $phpDocInfoFactory,
31+
private readonly UsefulArrayTagNodeAnalyzer $usefulArrayTagNodeAnalyzer,
32+
private readonly NodeDocblockTypeDecorator $nodeDocblockTypeDecorator,
33+
private readonly ArrayDimFetchFinder $arrayDimFetchFinder,
34+
) {
35+
36+
}
37+
38+
public function getRuleDefinition(): RuleDefinition
39+
{
40+
return new RuleDefinition('Add @param array docblock if array_map is used on the parameter', [
41+
new CodeSample(
42+
<<<'CODE_SAMPLE'
43+
final class SomeClass
44+
{
45+
private array $items = [];
46+
47+
public function run()
48+
{
49+
$this->items[] = [
50+
'name' => 'John',
51+
];
52+
}
53+
}
54+
CODE_SAMPLE
55+
,
56+
<<<'CODE_SAMPLE'
57+
final class SomeClass
58+
{
59+
/**
60+
* @var array<array<string, string>>
61+
*/
62+
private array $items = [];
63+
64+
public function run()
65+
{
66+
$this->items[] = [
67+
'name' => 'John',
68+
];
69+
}
70+
}
71+
CODE_SAMPLE
72+
),
73+
]);
74+
}
75+
76+
/**
77+
* @return array<class-string<Node>>
78+
*/
79+
public function getNodeTypes(): array
80+
{
81+
return [Class_::class];
82+
}
83+
84+
/**
85+
* @param Class_ $node
86+
*/
87+
public function refactor(Node $node): ?Node
88+
{
89+
$hasChanged = false;
90+
91+
foreach ($node->getProperties() as $property) {
92+
if (! $this->isPropertyTypeArray($property)) {
93+
continue;
94+
}
95+
96+
$propertyPhpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($property);
97+
98+
if ($this->usefulArrayTagNodeAnalyzer->isUsefulArrayTag($propertyPhpDocInfo->getVarTagValueNode())) {
99+
continue;
100+
}
101+
102+
$propertyName = $this->getName($property);
103+
104+
$assignedExprs = $this->arrayDimFetchFinder->findDimFetchAssignToPropertyName($node, $propertyName);
105+
106+
$assignedExprTypes = [];
107+
foreach ($assignedExprs as $assignedExpr) {
108+
$assignedExprTypes[] = $this->getType($assignedExpr);
109+
}
110+
111+
// nothing to add
112+
if ($assignedExprTypes === []) {
113+
continue;
114+
}
115+
116+
$uniqueGeneralizedUnionTypes = $this->typeFactory->uniquateTypes($assignedExprTypes);
117+
118+
if (count($uniqueGeneralizedUnionTypes) > 1) {
119+
$generalizedUnionedTypes = new UnionType($uniqueGeneralizedUnionTypes);
120+
} else {
121+
$generalizedUnionedTypes = $uniqueGeneralizedUnionTypes[0];
122+
}
123+
124+
$arrayReturnType = new ArrayType(new MixedType(), $generalizedUnionedTypes);
125+
126+
$hasPropertyChanged = $this->nodeDocblockTypeDecorator->decorateGenericIterableVarType(
127+
$arrayReturnType,
128+
$propertyPhpDocInfo,
129+
$property
130+
);
131+
132+
if ($hasPropertyChanged === false) {
133+
continue;
134+
}
135+
136+
$hasChanged = true;
137+
}
138+
139+
if (! $hasChanged) {
140+
return null;
141+
}
142+
143+
return $node;
144+
}
145+
146+
private function isPropertyTypeArray(Property $property): bool
147+
{
148+
if (! $property->type instanceof Identifier) {
149+
return false;
150+
}
151+
152+
return $this->isName($property->type, 'array');
153+
}
154+
}

‎src/Config/Level/TypeDeclarationDocblocksLevel.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use Rector\TypeDeclaration\Rector\ClassMethod\AddReturnArrayDocblockBasedOnArrayMapRector;
1010
use Rector\TypeDeclaration\Rector\ClassMethod\AddReturnDocblockForScalarArrayFromAssignsRector;
1111
use Rector\TypeDeclarationDocblocks\Rector\Class_\AddReturnDocblockDataProviderRector;
12+
use Rector\TypeDeclarationDocblocks\Rector\Class_\AddVarArrayDocblockFromDimFetchAssignRector;
1213
use Rector\TypeDeclarationDocblocks\Rector\Class_\ClassMethodArrayDocblockParamFromLocalCallsRector;
1314
use Rector\TypeDeclarationDocblocks\Rector\Class_\DocblockVarArrayFromGetterReturnRector;
1415
use Rector\TypeDeclarationDocblocks\Rector\Class_\DocblockVarArrayFromPropertyDefaultsRector;
@@ -57,6 +58,7 @@ final class TypeDeclarationDocblocksLevel
5758
// property var
5859
DocblockVarFromParamDocblockInConstructorRector::class,
5960
DocblockVarArrayFromGetterReturnRector::class,
61+
AddVarArrayDocblockFromDimFetchAssignRector::class,
6062

6163
// return
6264
DocblockGetterReturnArrayFromPropertyDocblockVarRector::class,

‎src/PhpParser/Node/BetterNodeFinder.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace Rector\PhpParser\Node;
66

7+
use PhpParser\Node\Stmt\ClassMethod;
78
use PhpParser\Node;
89
use PhpParser\Node\Expr\Variable;
910
use PhpParser\Node\Expr\Yield_;
@@ -239,7 +240,7 @@ public function findInstancesOfScoped(array $nodes, string|array $types): array
239240
$this->simpleCallableNodeTraverser->traverseNodesWithCallable(
240241
$nodes,
241242
static function (Node $subNode) use ($types, &$foundNodes): ?int {
242-
if ($subNode instanceof Class_ || $subNode instanceof FunctionLike) {
243+
if ($subNode instanceof Class_ || ($subNode instanceof FunctionLike && ! $subNode instanceof ClassMethod)) {
243244
return NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN;
244245
}
245246

0 commit comments

Comments
 (0)