diff --git a/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/DocblockGetterReturnArrayFromPropertyDocblockVarRector/Fixture/add_prefix_backslash.php.inc b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/DocblockGetterReturnArrayFromPropertyDocblockVarRector/Fixture/add_prefix_backslash.php.inc index 76a5ed639c9..9df08812ee7 100644 --- a/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/DocblockGetterReturnArrayFromPropertyDocblockVarRector/Fixture/add_prefix_backslash.php.inc +++ b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/DocblockGetterReturnArrayFromPropertyDocblockVarRector/Fixture/add_prefix_backslash.php.inc @@ -43,7 +43,7 @@ class AddPrefixBackslash extends LogicException } /** - * @return array[]> + * @return array>> */ public function getDuplicateNames(): array { diff --git a/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/DocblockGetterReturnArrayFromPropertyDocblockVarRector/Fixture/respect_int_string_key.php.inc b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/DocblockGetterReturnArrayFromPropertyDocblockVarRector/Fixture/respect_int_string_key.php.inc new file mode 100644 index 00000000000..f647df2a06c --- /dev/null +++ b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/DocblockGetterReturnArrayFromPropertyDocblockVarRector/Fixture/respect_int_string_key.php.inc @@ -0,0 +1,43 @@ + + */ + private array $names = []; + + /** + * @return array + */ + public function getNames(): array + { + return $this->names; + } +} + +?> +----- + + */ + private array $names = []; + + /** + * @return array + */ + public function getNames(): array + { + return $this->names; + } +} + +?> diff --git a/rules/TypeDeclarationDocblocks/NodeDocblockTypeDecorator.php b/rules/TypeDeclarationDocblocks/NodeDocblockTypeDecorator.php index 6e552bf8ba7..1f626242910 100644 --- a/rules/TypeDeclarationDocblocks/NodeDocblockTypeDecorator.php +++ b/rules/TypeDeclarationDocblocks/NodeDocblockTypeDecorator.php @@ -54,16 +54,26 @@ public function decorateGenericIterableParamType( } public function decorateGenericIterableReturnType( - Type $type, + Type|TypeNode $typeOrTypeNode, PhpDocInfo $classMethodPhpDocInfo, FunctionLike $functionLike ): bool { + if ($typeOrTypeNode instanceof TypeNode) { + $type = $this->staticTypeMapper->mapPHPStanPhpDocTypeNodeToPHPStanType($typeOrTypeNode, $functionLike); + } else { + $type = $typeOrTypeNode; + } + if ($this->isBareMixedType($type)) { // no value return false; } - $typeNode = $this->createTypeNode($type); + if ($typeOrTypeNode instanceof TypeNode) { + $typeNode = $typeOrTypeNode; + } else { + $typeNode = $this->createTypeNode($typeOrTypeNode); + } // no value iterable type if ($typeNode instanceof IdentifierTypeNode) { @@ -96,11 +106,10 @@ public function decorateGenericIterableVarType(Type $type, PhpDocInfo $phpDocInf private function createTypeNode(Type $type): TypeNode { - $generalizedReturnType = $this->typeNormalizer->generalizeConstantTypes($type); - - // turn into rather generic short return type, to keep it open to extension later and readable to human - $typeNode = $this->staticTypeMapper->mapPHPStanTypeToPHPStanPhpDocTypeNode($generalizedReturnType); + $generalizedType = $this->typeNormalizer->generalizeConstantTypes($type); + // turn into rather generic short return typeOrTypeNode, to keep it open to extension later and readable to human + $typeNode = $this->staticTypeMapper->mapPHPStanTypeToPHPStanPhpDocTypeNode($generalizedType); if ($typeNode instanceof IdentifierTypeNode && $typeNode->name === 'mixed') { return new ArrayTypeNode($typeNode); } diff --git a/rules/TypeDeclarationDocblocks/NodeFinder/GetterClassMethodPropertyFinder.php b/rules/TypeDeclarationDocblocks/NodeFinder/GetterClassMethodPropertyFinder.php new file mode 100644 index 00000000000..c1ddfefe86a --- /dev/null +++ b/rules/TypeDeclarationDocblocks/NodeFinder/GetterClassMethodPropertyFinder.php @@ -0,0 +1,74 @@ +stmts === null || count($classMethod->stmts) !== 1) { + return null; + } + + $onlyStmt = $classMethod->stmts[0]; + if (! $onlyStmt instanceof Return_) { + return null; + } + + if (! $onlyStmt->expr instanceof PropertyFetch) { + return null; + } + + $propertyFetch = $onlyStmt->expr; + if (! $this->nodeNameResolver->isName($propertyFetch->var, 'this')) { + return null; + } + + $propertyName = $this->nodeNameResolver->getName($propertyFetch->name); + if (! is_string($propertyName)) { + return null; + } + + $property = $class->getProperty($propertyName); + if ($property instanceof Property) { + return $property; + } + + // try also promoted property in constructor + $constructClassMethod = $class->getMethod(MethodName::CONSTRUCT); + if (! $constructClassMethod instanceof ClassMethod) { + return null; + } + + foreach ($constructClassMethod->getParams() as $param) { + if (! $param->isPromoted()) { + continue; + } + + if (! $this->nodeNameResolver->isName($param, $propertyName)) { + continue; + } + + return $param; + } + + return null; + } +} diff --git a/rules/TypeDeclarationDocblocks/Rector/ClassMethod/DocblockGetterReturnArrayFromPropertyDocblockVarRector.php b/rules/TypeDeclarationDocblocks/Rector/ClassMethod/DocblockGetterReturnArrayFromPropertyDocblockVarRector.php index 195b15ab5c7..fc2a1dd214e 100644 --- a/rules/TypeDeclarationDocblocks/Rector/ClassMethod/DocblockGetterReturnArrayFromPropertyDocblockVarRector.php +++ b/rules/TypeDeclarationDocblocks/Rector/ClassMethod/DocblockGetterReturnArrayFromPropertyDocblockVarRector.php @@ -5,15 +5,14 @@ namespace Rector\TypeDeclarationDocblocks\Rector\ClassMethod; use PhpParser\Node; -use PhpParser\Node\Expr\PropertyFetch; -use PhpParser\Node\Stmt\ClassMethod; -use PhpParser\Node\Stmt\Return_; -use PHPStan\Type\ArrayType; -use PHPStan\Type\MixedType; -use PHPStan\Type\UnionType; +use PhpParser\Node\Stmt\Class_; +use PHPStan\PhpDocParser\Ast\PhpDoc\VarTagValueNode; +use PHPStan\PhpDocParser\Ast\Type\ArrayTypeNode; +use PHPStan\PhpDocParser\Ast\Type\GenericTypeNode; use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory; use Rector\Rector\AbstractRector; use Rector\TypeDeclarationDocblocks\NodeDocblockTypeDecorator; +use Rector\TypeDeclarationDocblocks\NodeFinder\GetterClassMethodPropertyFinder; use Rector\TypeDeclarationDocblocks\TagNodeAnalyzer\UsefulArrayTagNodeAnalyzer; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -26,13 +25,17 @@ final class DocblockGetterReturnArrayFromPropertyDocblockVarRector extends Abstr public function __construct( private readonly PhpDocInfoFactory $phpDocInfoFactory, private readonly UsefulArrayTagNodeAnalyzer $usefulArrayTagNodeAnalyzer, - private readonly NodeDocblockTypeDecorator $nodeDocblockTypeDecorator + private readonly NodeDocblockTypeDecorator $nodeDocblockTypeDecorator, + private readonly GetterClassMethodPropertyFinder $getterClassMethodPropertyFinder, ) { } + /** + * @return array> + */ public function getNodeTypes(): array { - return [ClassMethod::class]; + return [Class_::class]; } public function getRuleDefinition(): RuleDefinition @@ -76,73 +79,62 @@ public function getItems(): array } /** - * @param ClassMethod $node + * @param Class_ $node */ public function refactor(Node $node): ?Node { - if (! $node->returnType instanceof Node) { - return null; - } - - if (! $this->isName($node->returnType, 'array')) { + if ($node->isAnonymous()) { return null; } - $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node); + $hasChanged = false; + foreach ($node->getMethods() as $classMethod) { + if (! $classMethod->returnType instanceof Node) { + continue; + } - if ($this->usefulArrayTagNodeAnalyzer->isUsefulArrayTag($phpDocInfo->getReturnTagValue())) { - return null; - } + if (! $this->isName($classMethod->returnType, 'array')) { + continue; + } - $propertyFetch = $this->matchReturnLocalPropertyFetch($node); - if (! $propertyFetch instanceof PropertyFetch) { - return null; - } + $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($classMethod); + if ($this->usefulArrayTagNodeAnalyzer->isUsefulArrayTag($phpDocInfo->getReturnTagValue())) { + continue; + } - $propertyFetchType = $this->getType($propertyFetch); - if ($propertyFetchType instanceof ArrayType - && $propertyFetchType->getKeyType() instanceof MixedType - && $propertyFetchType->getItemType() instanceof MixedType - ) { - return null; - } + $propertyOrParam = $this->getterClassMethodPropertyFinder->find($classMethod, $node); + if (! $propertyOrParam instanceof Node) { + continue; + } - if ($propertyFetchType instanceof UnionType) { - return null; - } + $propertyDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($propertyOrParam); - if (! $this->nodeDocblockTypeDecorator->decorateGenericIterableReturnType( - $propertyFetchType, - $phpDocInfo, - $node - )) { - return null; - } + $varTagValueNode = $propertyDocInfo->getVarTagValueNode(); - return $node; - } + if (! $varTagValueNode instanceof VarTagValueNode) { + continue; + } - private function matchReturnLocalPropertyFetch(ClassMethod $classMethod): ?PropertyFetch - { - // we need exactly one statement of return - if ($classMethod->stmts === null || count($classMethod->stmts) !== 1) { - return null; - } + // is type useful? + if (! $varTagValueNode->type instanceof GenericTypeNode && ! $varTagValueNode->type instanceof ArrayTypeNode) { + continue; + } - $onlyStmt = $classMethod->stmts[0]; - if (! $onlyStmt instanceof Return_) { - return null; - } + if (! $this->nodeDocblockTypeDecorator->decorateGenericIterableReturnType( + $varTagValueNode->type, + $phpDocInfo, + $classMethod + )) { + continue; + } - if (! $onlyStmt->expr instanceof PropertyFetch) { - return null; + $hasChanged = true; } - $propertyFetch = $onlyStmt->expr; - if (! $this->isName($propertyFetch->var, 'this')) { + if (! $hasChanged) { return null; } - return $propertyFetch; + return $node; } } diff --git a/rules/TypeDeclarationDocblocks/Rector/Class_/ClassMethodArrayDocblockParamFromLocalCallsRector.php b/rules/TypeDeclarationDocblocks/Rector/Class_/ClassMethodArrayDocblockParamFromLocalCallsRector.php index 3e1749681e2..5757210532b 100644 --- a/rules/TypeDeclarationDocblocks/Rector/Class_/ClassMethodArrayDocblockParamFromLocalCallsRector.php +++ b/rules/TypeDeclarationDocblocks/Rector/Class_/ClassMethodArrayDocblockParamFromLocalCallsRector.php @@ -115,6 +115,7 @@ public function refactor(Node $node): ?Node } $resolvedParameterType = $classMethodParameterTypes[$parameterPosition] ?? $classMethodParameterTypes[$parameterName] ?? null; + if (! $resolvedParameterType instanceof Type) { continue; } diff --git a/src/BetterPhpDocParser/PhpDocManipulator/PhpDocTypeChanger.php b/src/BetterPhpDocParser/PhpDocManipulator/PhpDocTypeChanger.php index 4a4df6812cf..f5ec4ac1410 100644 --- a/src/BetterPhpDocParser/PhpDocManipulator/PhpDocTypeChanger.php +++ b/src/BetterPhpDocParser/PhpDocManipulator/PhpDocTypeChanger.php @@ -104,6 +104,9 @@ public function changeReturnTypeNode( ): void { $existingReturnTagValueNode = $phpDocInfo->getReturnTagValue(); if ($existingReturnTagValueNode instanceof ReturnTagValueNode) { + // enforce reprint of copied type node + $newTypeNode->setAttribute('orig_node', null); + $existingReturnTagValueNode->type = $newTypeNode; } else { $returnTagValueNode = new ReturnTagValueNode($newTypeNode, ''); diff --git a/src/PHPStanStaticTypeMapper/TypeMapper/ArrayTypeMapper.php b/src/PHPStanStaticTypeMapper/TypeMapper/ArrayTypeMapper.php index 38349e2e2bf..4b1d91f49a3 100644 --- a/src/PHPStanStaticTypeMapper/TypeMapper/ArrayTypeMapper.php +++ b/src/PHPStanStaticTypeMapper/TypeMapper/ArrayTypeMapper.php @@ -13,6 +13,7 @@ use PHPStan\Type\Constant\ConstantArrayType; use PHPStan\Type\Constant\ConstantIntegerType; use PHPStan\Type\Generic\GenericClassStringType; +use PHPStan\Type\IntegerType; use PHPStan\Type\MixedType; use PHPStan\Type\NeverType; use PHPStan\Type\Type; @@ -75,7 +76,6 @@ public function mapToPHPStanPhpDocTypeNode(Type $type): TypeNode } if ($itemType instanceof ArrayType && $this->isGenericArrayCandidate($itemType)) { - return $this->createGenericArrayType($type, true); } @@ -83,6 +83,17 @@ public function mapToPHPStanPhpDocTypeNode(Type $type): TypeNode return $this->createGenericArrayType($type, true); } + // keep "int" key in arary + if ($type->getKeyType() instanceof IntegerType) { + $keyTypeNode = $this->phpStanStaticTypeMapper->mapToPHPStanPhpDocTypeNode($type->getKeyType()); + + if (! $type->isList()->maybe()) { + $nestedTypeNode = $this->phpStanStaticTypeMapper->mapToPHPStanPhpDocTypeNode($type->getItemType()); + + return new GenericTypeNode(new IdentifierTypeNode('array'), [$keyTypeNode, $nestedTypeNode]); + } + } + $itemTypeNode = $this->phpStanStaticTypeMapper->mapToPHPStanPhpDocTypeNode($itemType); return new SpacingAwareArrayTypeNode($itemTypeNode); }