Skip to content

Commit cfab7c9

Browse files
TomasVotrubaTomasVotruba
committed
keep int keys in already known array shapes in DocblockGetterReturnArrayFromPropertyDocblockVarRector
1 parent e93e9cc commit cfab7c9

6 files changed

Lines changed: 118 additions & 48 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<\App\Support\Benchmarking\Contracts\Metric>>>
4747
*/
4848
public function getDuplicateNames(): array
4949
{

‎rules/TypeDeclarationDocblocks/NodeDocblockTypeDecorator.php‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public function decorateGenericIterableParamType(
4343

4444
$typeNode = $this->createTypeNode($type);
4545

46-
// no value iterable type
46+
// no value iterable typeOrTypeNode
4747
if ($typeNode instanceof IdentifierTypeNode) {
4848
return false;
4949
}
@@ -54,18 +54,28 @@ public function decorateGenericIterableParamType(
5454
}
5555

5656
public function decorateGenericIterableReturnType(
57-
Type $type,
58-
PhpDocInfo $classMethodPhpDocInfo,
59-
FunctionLike $functionLike
57+
Type|TypeNode $typeOrTypeNode,
58+
PhpDocInfo $classMethodPhpDocInfo,
59+
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

68-
// no value iterable type
78+
// no value iterable typeOrTypeNode
6979
if ($typeNode instanceof IdentifierTypeNode) {
7080
return false;
7181
}
@@ -84,7 +94,7 @@ public function decorateGenericIterableVarType(Type $type, PhpDocInfo $phpDocInf
8494
return false;
8595
}
8696

87-
// no value iterable type
97+
// no value iterable typeOrTypeNode
8898
if ($typeNode instanceof IdentifierTypeNode) {
8999
return false;
90100
}
@@ -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/Rector/ClassMethod/DocblockGetterReturnArrayFromPropertyDocblockVarRector.php‎

Lines changed: 80 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@
44

55
namespace Rector\TypeDeclarationDocblocks\Rector\ClassMethod;
66

7+
use PhpParser\Node\Stmt\Class_;
8+
use PhpParser\Node\Stmt\Property;
9+
use PHPStan\PhpDocParser\Ast\PhpDoc\VarTagValueNode;
10+
use PHPStan\PhpDocParser\Ast\Type\ArrayTypeNode;
11+
use PHPStan\PhpDocParser\Ast\Type\GenericTypeNode;
12+
use PHPStan\Type\Type;
713
use PhpParser\Node;
814
use PhpParser\Node\Expr\PropertyFetch;
915
use PhpParser\Node\Stmt\ClassMethod;
@@ -30,9 +36,12 @@ public function __construct(
3036
) {
3137
}
3238

39+
/**
40+
* @return array<class-string<Node>>
41+
*/
3342
public function getNodeTypes(): array
3443
{
35-
return [ClassMethod::class];
44+
return [Class_::class];
3645
}
3746

3847
public function getRuleDefinition(): RuleDefinition
@@ -76,53 +85,67 @@ public function getItems(): array
7685
}
7786

7887
/**
79-
* @param ClassMethod $node
88+
* @param Class_ $node
8089
*/
8190
public function refactor(Node $node): ?Node
8291
{
83-
if (! $node->returnType instanceof Node) {
84-
return null;
85-
}
86-
87-
if (! $this->isName($node->returnType, 'array')) {
88-
return null;
89-
}
90-
91-
$phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node);
92-
93-
if ($this->usefulArrayTagNodeAnalyzer->isUsefulArrayTag($phpDocInfo->getReturnTagValue())) {
94-
return null;
95-
}
96-
97-
$propertyFetch = $this->matchReturnLocalPropertyFetch($node);
98-
if (! $propertyFetch instanceof PropertyFetch) {
99-
return null;
100-
}
101-
102-
$propertyFetchType = $this->getType($propertyFetch);
103-
if ($propertyFetchType instanceof ArrayType
104-
&& $propertyFetchType->getKeyType() instanceof MixedType
105-
&& $propertyFetchType->getItemType() instanceof MixedType
106-
) {
92+
if ($node->isAnonymous()) {
10793
return null;
10894
}
10995

110-
if ($propertyFetchType instanceof UnionType) {
111-
return null;
96+
$hasChanged = false;
97+
foreach ($node->getMethods() as $classMethod) {
98+
if (! $classMethod->returnType instanceof Node) {
99+
continue;
100+
}
101+
102+
if (! $this->isName($classMethod->returnType, 'array')) {
103+
continue;
104+
}
105+
106+
$phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($classMethod);
107+
if ($this->usefulArrayTagNodeAnalyzer->isUsefulArrayTag($phpDocInfo->getReturnTagValue())) {
108+
continue;
109+
}
110+
111+
// @todo add promoted proeprty
112+
$property = $this->matchReturnLocalPropertyFetch($classMethod, $node);
113+
if (! $property instanceof Property) {
114+
continue;
115+
}
116+
117+
$propertyDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($property);
118+
119+
$varTagValueNode = $propertyDocInfo->getVarTagValueNode();
120+
121+
if (! $varTagValueNode instanceof VarTagValueNode) {
122+
continue;
123+
}
124+
125+
// is type useful?
126+
if (! $varTagValueNode->type instanceof GenericTypeNode && ! $varTagValueNode->type instanceof ArrayTypeNode) {
127+
continue;
128+
}
129+
130+
if (! $this->nodeDocblockTypeDecorator->decorateGenericIterableReturnType(
131+
$varTagValueNode->type,
132+
$phpDocInfo,
133+
$classMethod
134+
)) {
135+
continue;
136+
}
137+
138+
$hasChanged = true;
112139
}
113140

114-
if (! $this->nodeDocblockTypeDecorator->decorateGenericIterableReturnType(
115-
$propertyFetchType,
116-
$phpDocInfo,
117-
$node
118-
)) {
141+
if (! $hasChanged) {
119142
return null;
120143
}
121144

122145
return $node;
123146
}
124147

125-
private function matchReturnLocalPropertyFetch(ClassMethod $classMethod): ?PropertyFetch
148+
private function matchReturnLocalPropertyFetch(ClassMethod $classMethod, Class_ $class): ?Property
126149
{
127150
// we need exactly one statement of return
128151
if ($classMethod->stmts === null || count($classMethod->stmts) !== 1) {
@@ -143,6 +166,28 @@ private function matchReturnLocalPropertyFetch(ClassMethod $classMethod): ?Prope
143166
return null;
144167
}
145168

146-
return $propertyFetch;
169+
$propertyName = $this->getName($propertyFetch->name);
170+
if (! is_string($propertyName)) {
171+
return null;
172+
}
173+
174+
return $class->getProperty($propertyName);
175+
}
176+
177+
private function isUsefulType(Type $type): bool
178+
{
179+
if ($type instanceof UnionType) {
180+
return false;
181+
}
182+
183+
if (! $type instanceof ArrayType) {
184+
return true;
185+
}
186+
187+
if (! $type->getKeyType() instanceof MixedType) {
188+
return true;
189+
}
190+
191+
return ! $type->getItemType() instanceof MixedType;
147192
}
148193
}

‎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: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
use Rector\BetterPhpDocParser\ValueObject\Type\SpacingAwareArrayTypeNode;
2929
use Rector\BetterPhpDocParser\ValueObject\Type\SpacingAwareCallableTypeNode;
3030
use Rector\Comments\NodeDocBlock\DocBlockUpdater;
31+
use Rector\NodeTypeResolver\Node\AttributeKey;
3132
use Rector\NodeTypeResolver\TypeComparator\TypeComparator;
3233
use Rector\StaticTypeMapper\StaticTypeMapper;
3334
use Rector\TypeDeclaration\PhpDocParser\ParamPhpDocNodeFactory;
@@ -104,6 +105,9 @@ public function changeReturnTypeNode(
104105
): void {
105106
$existingReturnTagValueNode = $phpDocInfo->getReturnTagValue();
106107
if ($existingReturnTagValueNode instanceof ReturnTagValueNode) {
108+
// enforce reprint of copied type node
109+
$newTypeNode->setAttribute('orig_node', null);
110+
107111
$existingReturnTagValueNode->type = $newTypeNode;
108112
} else {
109113
$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)