Skip to content

Commit 7371495

Browse files
TomasVotrubaTomasVotruba
authored
Keep int keys in already known array shapes in DocblockGetterReturnArrayFromPropertyDocblockVarRector (#7495)
* add return fixture to mimic var int key * keep int keys in already known array shapes in DocblockGetterReturnArrayFromPropertyDocblockVarRector * fix promoted property
1 parent ff5603d commit 7371495

8 files changed

Lines changed: 197 additions & 64 deletions

File tree

‎rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/DocblockGetterReturnArrayFromPropertyDocblockVarRector/Fixture/add_prefix_backslash.php.inc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ class AddPrefixBackslash extends LogicException
4343
}
4444

4545
/**
46-
* @return array<string, class-string<\App\Support\Benchmarking\Contracts\Metric>[]>
46+
* @return array<string, array<int, class-string<Metric>>>
4747
*/
4848
public function getDuplicateNames(): array
4949
{

‎rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/DocblockGetterReturnArrayFromPropertyDocblockVarRector/Fixture/respect_int_string_key.php.inc‎

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

‎rules/TypeDeclarationDocblocks/NodeDocblockTypeDecorator.php‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,16 +54,26 @@ public function decorateGenericIterableParamType(
5454
}
5555

5656
public function decorateGenericIterableReturnType(
57-
Type $type,
57+
Type|TypeNode $typeOrTypeNode,
5858
PhpDocInfo $classMethodPhpDocInfo,
5959
FunctionLike $functionLike
6060
): bool {
61+
if ($typeOrTypeNode instanceof TypeNode) {
62+
$type = $this->staticTypeMapper->mapPHPStanPhpDocTypeNodeToPHPStanType($typeOrTypeNode, $functionLike);
63+
} else {
64+
$type = $typeOrTypeNode;
65+
}
66+
6167
if ($this->isBareMixedType($type)) {
6268
// no value
6369
return false;
6470
}
6571

66-
$typeNode = $this->createTypeNode($type);
72+
if ($typeOrTypeNode instanceof TypeNode) {
73+
$typeNode = $typeOrTypeNode;
74+
} else {
75+
$typeNode = $this->createTypeNode($typeOrTypeNode);
76+
}
6777

6878
// no value iterable type
6979
if ($typeNode instanceof IdentifierTypeNode) {
@@ -96,11 +106,10 @@ public function decorateGenericIterableVarType(Type $type, PhpDocInfo $phpDocInf
96106

97107
private function createTypeNode(Type $type): TypeNode
98108
{
99-
$generalizedReturnType = $this->typeNormalizer->generalizeConstantTypes($type);
100-
101-
// turn into rather generic short return type, to keep it open to extension later and readable to human
102-
$typeNode = $this->staticTypeMapper->mapPHPStanTypeToPHPStanPhpDocTypeNode($generalizedReturnType);
109+
$generalizedType = $this->typeNormalizer->generalizeConstantTypes($type);
103110

111+
// turn into rather generic short return typeOrTypeNode, to keep it open to extension later and readable to human
112+
$typeNode = $this->staticTypeMapper->mapPHPStanTypeToPHPStanPhpDocTypeNode($generalizedType);
104113
if ($typeNode instanceof IdentifierTypeNode && $typeNode->name === 'mixed') {
105114
return new ArrayTypeNode($typeNode);
106115
}

‎rules/TypeDeclarationDocblocks/NodeFinder/GetterClassMethodPropertyFinder.php‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\TypeDeclarationDocblocks\NodeFinder;
6+
7+
use PhpParser\Node\Expr\PropertyFetch;
8+
use PhpParser\Node\Param;
9+
use PhpParser\Node\Stmt\Class_;
10+
use PhpParser\Node\Stmt\ClassMethod;
11+
use PhpParser\Node\Stmt\Property;
12+
use PhpParser\Node\Stmt\Return_;
13+
use Rector\NodeNameResolver\NodeNameResolver;
14+
use Rector\ValueObject\MethodName;
15+
16+
final readonly class GetterClassMethodPropertyFinder
17+
{
18+
public function __construct(
19+
private NodeNameResolver $nodeNameResolver,
20+
) {
21+
}
22+
23+
public function find(ClassMethod $classMethod, Class_ $class): Property|Param|null
24+
{
25+
// we need exactly one statement of return
26+
if ($classMethod->stmts === null || count($classMethod->stmts) !== 1) {
27+
return null;
28+
}
29+
30+
$onlyStmt = $classMethod->stmts[0];
31+
if (! $onlyStmt instanceof Return_) {
32+
return null;
33+
}
34+
35+
if (! $onlyStmt->expr instanceof PropertyFetch) {
36+
return null;
37+
}
38+
39+
$propertyFetch = $onlyStmt->expr;
40+
if (! $this->nodeNameResolver->isName($propertyFetch->var, 'this')) {
41+
return null;
42+
}
43+
44+
$propertyName = $this->nodeNameResolver->getName($propertyFetch->name);
45+
if (! is_string($propertyName)) {
46+
return null;
47+
}
48+
49+
$property = $class->getProperty($propertyName);
50+
if ($property instanceof Property) {
51+
return $property;
52+
}
53+
54+
// try also promoted property in constructor
55+
$constructClassMethod = $class->getMethod(MethodName::CONSTRUCT);
56+
if (! $constructClassMethod instanceof ClassMethod) {
57+
return null;
58+
}
59+
60+
foreach ($constructClassMethod->getParams() as $param) {
61+
if (! $param->isPromoted()) {
62+
continue;
63+
}
64+
65+
if (! $this->nodeNameResolver->isName($param, $propertyName)) {
66+
continue;
67+
}
68+
69+
return $param;
70+
}
71+
72+
return null;
73+
}
74+
}

‎rules/TypeDeclarationDocblocks/Rector/ClassMethod/DocblockGetterReturnArrayFromPropertyDocblockVarRector.php‎

Lines changed: 48 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,14 @@
55
namespace Rector\TypeDeclarationDocblocks\Rector\ClassMethod;
66

77
use PhpParser\Node;
8-
use PhpParser\Node\Expr\PropertyFetch;
9-
use PhpParser\Node\Stmt\ClassMethod;
10-
use PhpParser\Node\Stmt\Return_;
11-
use PHPStan\Type\ArrayType;
12-
use PHPStan\Type\MixedType;
13-
use PHPStan\Type\UnionType;
8+
use PhpParser\Node\Stmt\Class_;
9+
use PHPStan\PhpDocParser\Ast\PhpDoc\VarTagValueNode;
10+
use PHPStan\PhpDocParser\Ast\Type\ArrayTypeNode;
11+
use PHPStan\PhpDocParser\Ast\Type\GenericTypeNode;
1412
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
1513
use Rector\Rector\AbstractRector;
1614
use Rector\TypeDeclarationDocblocks\NodeDocblockTypeDecorator;
15+
use Rector\TypeDeclarationDocblocks\NodeFinder\GetterClassMethodPropertyFinder;
1716
use Rector\TypeDeclarationDocblocks\TagNodeAnalyzer\UsefulArrayTagNodeAnalyzer;
1817
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
1918
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
@@ -26,13 +25,17 @@ final class DocblockGetterReturnArrayFromPropertyDocblockVarRector extends Abstr
2625
public function __construct(
2726
private readonly PhpDocInfoFactory $phpDocInfoFactory,
2827
private readonly UsefulArrayTagNodeAnalyzer $usefulArrayTagNodeAnalyzer,
29-
private readonly NodeDocblockTypeDecorator $nodeDocblockTypeDecorator
28+
private readonly NodeDocblockTypeDecorator $nodeDocblockTypeDecorator,
29+
private readonly GetterClassMethodPropertyFinder $getterClassMethodPropertyFinder,
3030
) {
3131
}
3232

33+
/**
34+
* @return array<class-string<Node>>
35+
*/
3336
public function getNodeTypes(): array
3437
{
35-
return [ClassMethod::class];
38+
return [Class_::class];
3639
}
3740

3841
public function getRuleDefinition(): RuleDefinition
@@ -76,73 +79,62 @@ public function getItems(): array
7679
}
7780

7881
/**
79-
* @param ClassMethod $node
82+
* @param Class_ $node
8083
*/
8184
public function refactor(Node $node): ?Node
8285
{
83-
if (! $node->returnType instanceof Node) {
84-
return null;
85-
}
86-
87-
if (! $this->isName($node->returnType, 'array')) {
86+
if ($node->isAnonymous()) {
8887
return null;
8988
}
9089

91-
$phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node);
90+
$hasChanged = false;
91+
foreach ($node->getMethods() as $classMethod) {
92+
if (! $classMethod->returnType instanceof Node) {
93+
continue;
94+
}
9295

93-
if ($this->usefulArrayTagNodeAnalyzer->isUsefulArrayTag($phpDocInfo->getReturnTagValue())) {
94-
return null;
95-
}
96+
if (! $this->isName($classMethod->returnType, 'array')) {
97+
continue;
98+
}
9699

97-
$propertyFetch = $this->matchReturnLocalPropertyFetch($node);
98-
if (! $propertyFetch instanceof PropertyFetch) {
99-
return null;
100-
}
100+
$phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($classMethod);
101+
if ($this->usefulArrayTagNodeAnalyzer->isUsefulArrayTag($phpDocInfo->getReturnTagValue())) {
102+
continue;
103+
}
101104

102-
$propertyFetchType = $this->getType($propertyFetch);
103-
if ($propertyFetchType instanceof ArrayType
104-
&& $propertyFetchType->getKeyType() instanceof MixedType
105-
&& $propertyFetchType->getItemType() instanceof MixedType
106-
) {
107-
return null;
108-
}
105+
$propertyOrParam = $this->getterClassMethodPropertyFinder->find($classMethod, $node);
106+
if (! $propertyOrParam instanceof Node) {
107+
continue;
108+
}
109109

110-
if ($propertyFetchType instanceof UnionType) {
111-
return null;
112-
}
110+
$propertyDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($propertyOrParam);
113111

114-
if (! $this->nodeDocblockTypeDecorator->decorateGenericIterableReturnType(
115-
$propertyFetchType,
116-
$phpDocInfo,
117-
$node
118-
)) {
119-
return null;
120-
}
112+
$varTagValueNode = $propertyDocInfo->getVarTagValueNode();
121113

122-
return $node;
123-
}
114+
if (! $varTagValueNode instanceof VarTagValueNode) {
115+
continue;
116+
}
124117

125-
private function matchReturnLocalPropertyFetch(ClassMethod $classMethod): ?PropertyFetch
126-
{
127-
// we need exactly one statement of return
128-
if ($classMethod->stmts === null || count($classMethod->stmts) !== 1) {
129-
return null;
130-
}
118+
// is type useful?
119+
if (! $varTagValueNode->type instanceof GenericTypeNode && ! $varTagValueNode->type instanceof ArrayTypeNode) {
120+
continue;
121+
}
131122

132-
$onlyStmt = $classMethod->stmts[0];
133-
if (! $onlyStmt instanceof Return_) {
134-
return null;
135-
}
123+
if (! $this->nodeDocblockTypeDecorator->decorateGenericIterableReturnType(
124+
$varTagValueNode->type,
125+
$phpDocInfo,
126+
$classMethod
127+
)) {
128+
continue;
129+
}
136130

137-
if (! $onlyStmt->expr instanceof PropertyFetch) {
138-
return null;
131+
$hasChanged = true;
139132
}
140133

141-
$propertyFetch = $onlyStmt->expr;
142-
if (! $this->isName($propertyFetch->var, 'this')) {
134+
if (! $hasChanged) {
143135
return null;
144136
}
145137

146-
return $propertyFetch;
138+
return $node;
147139
}
148140
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ public function refactor(Node $node): ?Node
115115
}
116116

117117
$resolvedParameterType = $classMethodParameterTypes[$parameterPosition] ?? $classMethodParameterTypes[$parameterName] ?? null;
118+
118119
if (! $resolvedParameterType instanceof Type) {
119120
continue;
120121
}

‎src/BetterPhpDocParser/PhpDocManipulator/PhpDocTypeChanger.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,9 @@ public function changeReturnTypeNode(
104104
): void {
105105
$existingReturnTagValueNode = $phpDocInfo->getReturnTagValue();
106106
if ($existingReturnTagValueNode instanceof ReturnTagValueNode) {
107+
// enforce reprint of copied type node
108+
$newTypeNode->setAttribute('orig_node', null);
109+
107110
$existingReturnTagValueNode->type = $newTypeNode;
108111
} else {
109112
$returnTagValueNode = new ReturnTagValueNode($newTypeNode, '');

‎src/PHPStanStaticTypeMapper/TypeMapper/ArrayTypeMapper.php‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use PHPStan\Type\Constant\ConstantArrayType;
1414
use PHPStan\Type\Constant\ConstantIntegerType;
1515
use PHPStan\Type\Generic\GenericClassStringType;
16+
use PHPStan\Type\IntegerType;
1617
use PHPStan\Type\MixedType;
1718
use PHPStan\Type\NeverType;
1819
use PHPStan\Type\Type;
@@ -75,14 +76,24 @@ public function mapToPHPStanPhpDocTypeNode(Type $type): TypeNode
7576
}
7677

7778
if ($itemType instanceof ArrayType && $this->isGenericArrayCandidate($itemType)) {
78-
7979
return $this->createGenericArrayType($type, true);
8080
}
8181

8282
if ($isGenericArray) {
8383
return $this->createGenericArrayType($type, true);
8484
}
8585

86+
// keep "int" key in arary<int, mixed>
87+
if ($type->getKeyType() instanceof IntegerType) {
88+
$keyTypeNode = $this->phpStanStaticTypeMapper->mapToPHPStanPhpDocTypeNode($type->getKeyType());
89+
90+
if (! $type->isList()->maybe()) {
91+
$nestedTypeNode = $this->phpStanStaticTypeMapper->mapToPHPStanPhpDocTypeNode($type->getItemType());
92+
93+
return new GenericTypeNode(new IdentifierTypeNode('array'), [$keyTypeNode, $nestedTypeNode]);
94+
}
95+
}
96+
8697
$itemTypeNode = $this->phpStanStaticTypeMapper->mapToPHPStanPhpDocTypeNode($itemType);
8798
return new SpacingAwareArrayTypeNode($itemTypeNode);
8899
}

0 commit comments

Comments
 (0)