diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector/Fixture/also_update_generic_collection_docblock.php.inc b/rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector/Fixture/also_update_generic_collection_docblock.php.inc deleted file mode 100644 index bbdf8a63297..00000000000 --- a/rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector/Fixture/also_update_generic_collection_docblock.php.inc +++ /dev/null @@ -1,41 +0,0 @@ - - */ - protected function build(): Collection - { - return new ArrayCollection([new stdClass()]); - } -} - -?> ------ - - */ - protected function build(): \Doctrine\Common\Collections\ArrayCollection - { - return new ArrayCollection([new stdClass()]); - } -} - -?> diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector/Fixture/do_not_change_valid_generic_collection_docblock.php.inc b/rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector/Fixture/do_not_change_valid_generic_collection_docblock.php.inc deleted file mode 100644 index b640a49ff97..00000000000 --- a/rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector/Fixture/do_not_change_valid_generic_collection_docblock.php.inc +++ /dev/null @@ -1,41 +0,0 @@ - - */ - protected function build(): Collection - { - return new ArrayCollection([new stdClass()]); - } -} - -?> ------ - - */ - protected function build(): \Doctrine\Common\Collections\ArrayCollection - { - return new ArrayCollection([new stdClass()]); - } -} - -?> diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector/Fixture/narrow_from_interface.php.inc b/rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector/Fixture/narrow_from_interface.php.inc deleted file mode 100644 index 81b02dd94b4..00000000000 --- a/rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector/Fixture/narrow_from_interface.php.inc +++ /dev/null @@ -1,33 +0,0 @@ - ------ - \ No newline at end of file diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector/Fixture/narrow_interface_inheritance.php.inc b/rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector/Fixture/narrow_interface_inheritance.php.inc deleted file mode 100644 index eca3ef10ad1..00000000000 --- a/rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector/Fixture/narrow_interface_inheritance.php.inc +++ /dev/null @@ -1,45 +0,0 @@ -createProduct(); - } - - private function createProduct(): ConcreteProduct - { - return new ConcreteProduct(); - } -} - -?> ------ -createProduct(); - } - - private function createProduct(): ConcreteProduct - { - return new ConcreteProduct(); - } -} - -?> \ No newline at end of file diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector/Fixture/narrow_interface_via_method_call.php.inc b/rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector/Fixture/narrow_interface_via_method_call.php.inc deleted file mode 100644 index d5b5ce80bd2..00000000000 --- a/rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector/Fixture/narrow_interface_via_method_call.php.inc +++ /dev/null @@ -1,43 +0,0 @@ -createStripePayment(); - } - - private function createStripePayment(): StripePayment - { - return new StripePayment(); - } -} - -?> ------ -createStripePayment(); - } - - private function createStripePayment(): StripePayment - { - return new StripePayment(); - } -} - -?> \ No newline at end of file diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector/Fixture/skip_interface_as_on_purpose.php.inc b/rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector/Fixture/skip_interface_as_on_purpose.php.inc new file mode 100644 index 00000000000..5f946401491 --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector/Fixture/skip_interface_as_on_purpose.php.inc @@ -0,0 +1,14 @@ + \ No newline at end of file diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector/Fixture/skip_non_final_class.php.inc b/rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector/Fixture/skip_non_final_class.php.inc index 28f40654959..5e65d6dc4c4 100644 --- a/rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector/Fixture/skip_non_final_class.php.inc +++ b/rules-tests/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector/Fixture/skip_non_final_class.php.inc @@ -11,5 +11,3 @@ class NonFinalFactory return new ConferenceTalk(); } } - -?> \ No newline at end of file diff --git a/rules/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector.php b/rules/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector.php index f19253e10a0..bafc1688487 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/NarrowObjectReturnTypeRector.php @@ -9,9 +9,9 @@ use PhpParser\Node\Name\FullyQualified; use PhpParser\Node\Stmt\ClassMethod; use PHPStan\PhpDocParser\Ast\PhpDoc\ReturnTagValueNode; -use PHPStan\PhpDocParser\Ast\Type\GenericTypeNode; use PHPStan\PhpDocParser\Ast\Type\IdentifierTypeNode; use PHPStan\Reflection\ClassReflection; +use PHPStan\Reflection\ReflectionProvider; use PHPStan\Type\Generic\GenericObjectType; use PHPStan\Type\ObjectType; use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo; @@ -41,14 +41,15 @@ public function __construct( private readonly StaticTypeMapper $staticTypeMapper, private readonly TypeComparator $typeComparator, private readonly PhpDocInfoFactory $phpDocInfoFactory, - private readonly DocBlockUpdater $docBlockUpdater + private readonly DocBlockUpdater $docBlockUpdater, + private readonly ReflectionProvider $reflectionProvider ) { } public function getRuleDefinition(): RuleDefinition { return new RuleDefinition( - 'Narrows return type from generic object or parent class to specific class in final classes/methods', + 'Narrows return type from generic `object` or parent class to specific class in final classes/methods', [ new CodeSample( <<<'CODE_SAMPLE' @@ -110,7 +111,6 @@ public function getNodeTypes(): array public function refactor(Node $node): ?Node { $returnType = $node->returnType; - if (! $returnType instanceof Identifier && ! $returnType instanceof FullyQualified) { return null; } @@ -124,24 +124,35 @@ public function refactor(Node $node): ?Node return null; } - $actualReturnClass = $this->getActualReturnClass($node); - + $actualReturnClass = $this->getActualReturnedClass($node); if ($actualReturnClass === null) { return null; } $declaredType = $returnType->toString(); + // already most narrow type if ($declaredType === $actualReturnClass) { return null; } - if ($this->isDeclaredTypeFinal($declaredType)) { - return null; - } + // non-existing class + if ($declaredType !== 'object') { + if (! $this->reflectionProvider->hasClass($declaredType)) { + return null; + } - if ($this->isActualTypeAnonymous($actualReturnClass)) { - return null; + $declaredTypeClassReflection = $this->reflectionProvider->getClass($declaredType); + + // already last final object + if ($declaredTypeClassReflection->isFinalByKeyword()) { + return null; + } + + // this rule narrows only object or class types, not interfaces + if (! $declaredTypeClassReflection->isClass()) { + return null; + } } if (! $this->isNarrowingValid($node, $declaredType, $actualReturnClass)) { @@ -153,7 +164,6 @@ public function refactor(Node $node): ?Node } $node->returnType = new FullyQualified($actualReturnClass); - $this->updateDocblock($node, $actualReturnClass); return $node; @@ -176,11 +186,6 @@ private function updateDocblock(ClassMethod $classMethod, string $actualReturnCl $returnTagValueNode->type, $classMethod ); - } elseif ($returnTagValueNode->type instanceof GenericTypeNode) { - $oldType = $this->staticTypeMapper->mapPHPStanPhpDocTypeNodeToPHPStanType( - $returnTagValueNode->type->type, - $classMethod - ); } else { return; } @@ -192,43 +197,10 @@ private function updateDocblock(ClassMethod $classMethod, string $actualReturnCl } } - if ($returnTagValueNode->type instanceof IdentifierTypeNode) { - $returnTagValueNode->type = new FullyQualifiedIdentifierTypeNode($actualReturnClass); - } else { - $returnTagValueNode->type->type = new FullyQualifiedIdentifierTypeNode($actualReturnClass); - } - + $returnTagValueNode->type = new FullyQualifiedIdentifierTypeNode($actualReturnClass); $this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($classMethod); } - private function isDeclaredTypeFinal(string $declaredType): bool - { - if ($declaredType === 'object') { - return false; - } - - $declaredObjectType = new ObjectType($declaredType); - $classReflection = $declaredObjectType->getClassReflection(); - - if (! $classReflection instanceof ClassReflection) { - return false; - } - - return $classReflection->isFinalByKeyword(); - } - - private function isActualTypeAnonymous(string $actualType): bool - { - $actualObjectType = new ObjectType($actualType); - $classReflection = $actualObjectType->getClassReflection(); - - if (! $classReflection instanceof ClassReflection) { - return false; - } - - return $classReflection->isAnonymous(); - } - private function isNarrowingValid(ClassMethod $classMethod, string $declaredType, string $actualType): bool { if ($declaredType === 'object') { @@ -259,7 +231,6 @@ private function isNarrowingValidFromParent(ClassMethod $classMethod, string $ac } $classReflection = $this->reflectionResolver->resolveClassReflection($classMethod); - if (! $classReflection instanceof ClassReflection) { return true; } @@ -281,13 +252,11 @@ private function isNarrowingValidFromParent(ClassMethod $classMethod, string $ac } $parentClassMethod = $this->astResolver->resolveClassMethod($ancestor->getName(), $methodName); - if (! $parentClassMethod instanceof ClassMethod) { continue; } $parentReturnType = $parentClassMethod->returnType; - if (! $parentReturnType instanceof Identifier && ! $parentReturnType instanceof FullyQualified) { continue; } @@ -302,10 +271,9 @@ private function isNarrowingValidFromParent(ClassMethod $classMethod, string $ac return true; } - private function getActualReturnClass(ClassMethod $classMethod): ?string + private function getActualReturnedClass(ClassMethod $classMethod): ?string { $returnStatements = $this->betterNodeFinder->findReturnsScoped($classMethod); - if ($returnStatements === []) { return null; }