diff --git a/rules-tests/TypedCollections/Rector/ClassMethod/NarrowReturnUnionToCollectionRector/Fixture/array_and_collection.php.inc b/rules-tests/TypedCollections/Rector/ClassMethod/NarrowReturnUnionToCollectionRector/Fixture/array_and_collection.php.inc index 5216211b..28d67c59 100644 --- a/rules-tests/TypedCollections/Rector/ClassMethod/NarrowReturnUnionToCollectionRector/Fixture/array_and_collection.php.inc +++ b/rules-tests/TypedCollections/Rector/ClassMethod/NarrowReturnUnionToCollectionRector/Fixture/array_and_collection.php.inc @@ -11,7 +11,6 @@ final class ArrayAndCollection */ public function someMethod() { - } } @@ -30,7 +29,6 @@ final class ArrayAndCollection */ public function someMethod() { - } } diff --git a/rules-tests/TypedCollections/Rector/ClassMethod/NarrowReturnUnionToCollectionRector/Fixture/nullable_union_collection.php.inc b/rules-tests/TypedCollections/Rector/ClassMethod/NarrowReturnUnionToCollectionRector/Fixture/nullable_union_collection.php.inc new file mode 100644 index 00000000..51910661 --- /dev/null +++ b/rules-tests/TypedCollections/Rector/ClassMethod/NarrowReturnUnionToCollectionRector/Fixture/nullable_union_collection.php.inc @@ -0,0 +1,37 @@ + + */ + public function someMethod(): Collection|null + { + } +} + +?> +----- + + */ + public function someMethod(): Collection + { + } +} + +?> diff --git a/rules-tests/TypedCollections/Rector/ClassMethod/NarrowReturnUnionToCollectionRector/Fixture/return_non_nullable_collection.php.inc b/rules-tests/TypedCollections/Rector/ClassMethod/NarrowReturnUnionToCollectionRector/Fixture/return_non_nullable_collection.php.inc new file mode 100644 index 00000000..4c6cbce0 --- /dev/null +++ b/rules-tests/TypedCollections/Rector/ClassMethod/NarrowReturnUnionToCollectionRector/Fixture/return_non_nullable_collection.php.inc @@ -0,0 +1,37 @@ + + */ + public function someMethod(): ?Collection + { + } +} + +?> +----- + + */ + public function someMethod(): Collection + { + } +} + +?> diff --git a/rules-tests/TypedCollections/Rector/FuncCall/ArrayMapOnCollectionToArrayRector/Fixture/array_filter_to_array.php.inc b/rules-tests/TypedCollections/Rector/FuncCall/ArrayMapOnCollectionToArrayRector/Fixture/array_filter_to_array.php.inc new file mode 100644 index 00000000..1c44236e --- /dev/null +++ b/rules-tests/TypedCollections/Rector/FuncCall/ArrayMapOnCollectionToArrayRector/Fixture/array_filter_to_array.php.inc @@ -0,0 +1,49 @@ + + */ + public $items; + + public function merge() + { + $items = $this->items; + + return array_filter($items, fn ($item) => $item); + } +} + +?> +----- + + */ + public $items; + + public function merge() + { + $items = $this->items; + + return array_filter($items->toArray(), fn ($item) => $item); + } +} + +?> diff --git a/rules-tests/TypedCollections/Rector/FuncCall/ArrayMapOnCollectionToArrayRector/Fixture/skip_non_collection.php.inc b/rules-tests/TypedCollections/Rector/FuncCall/ArrayMapOnCollectionToArrayRector/Fixture/skip_non_collection.php.inc new file mode 100644 index 00000000..355b89ff --- /dev/null +++ b/rules-tests/TypedCollections/Rector/FuncCall/ArrayMapOnCollectionToArrayRector/Fixture/skip_non_collection.php.inc @@ -0,0 +1,20 @@ + + */ + public $items; + + public function merge() + { + $items = $this->items; + + return array_filter($items, fn ($item) => $item); + } +} diff --git a/rules/TypedCollections/Rector/ClassMethod/NarrowReturnUnionToCollectionRector.php b/rules/TypedCollections/Rector/ClassMethod/NarrowReturnUnionToCollectionRector.php index 71878680..0633b3b7 100644 --- a/rules/TypedCollections/Rector/ClassMethod/NarrowReturnUnionToCollectionRector.php +++ b/rules/TypedCollections/Rector/ClassMethod/NarrowReturnUnionToCollectionRector.php @@ -5,8 +5,11 @@ namespace Rector\Doctrine\TypedCollections\Rector\ClassMethod; use PhpParser\Node; +use PhpParser\Node\NullableType; use PhpParser\Node\Stmt\ClassMethod; +use PhpParser\Node\UnionType; use PHPStan\PhpDocParser\Ast\PhpDoc\ReturnTagValueNode; +use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo; use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory; use Rector\Comments\NodeDocBlock\DocBlockUpdater; use Rector\Doctrine\Enum\DoctrineClass; @@ -74,26 +77,97 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?ClassMethod { - $classMethodPhpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node); - $returnTagValueNode = $classMethodPhpDocInfo->getReturnTagValue(); + $hasChanged = false; + if ($this->refactorReturnDocblockTag($node)) { + $hasChanged = true; + } - if (! $returnTagValueNode instanceof ReturnTagValueNode) { + if ($this->refactorNativeReturn($node)) { + $hasChanged = true; + } + + if (! $hasChanged) { return null; } - if ($node->returnType !== null && $this->isName($node->returnType, DoctrineClass::COLLECTION)) { - $hasNativeCollectionType = true; - } else { - $hasNativeCollectionType = false; + return $node; + } + + private function refactorReturnDocblockTag(ClassMethod $classMethod): bool + { + $phpDocInfo = $this->phpDocInfoFactory->createFromNode($classMethod); + if (! $phpDocInfo instanceof PhpDocInfo) { + return false; } + $returnTagValueNode = $phpDocInfo->getReturnTagValue(); + if (! $returnTagValueNode instanceof ReturnTagValueNode) { + return false; + } + + $hasNativeCollectionType = $this->hasNativeCollectionType($classMethod); + $hasChanged = $this->unionCollectionTagValueNodeNarrower->narrow($returnTagValueNode, $hasNativeCollectionType); if ($hasChanged === false) { - return null; + return false; } - $this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($node); + $this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($classMethod); - return $node; + return true; + } + + private function hasNativeCollectionType(ClassMethod $classMethod): bool + { + if (! $classMethod->returnType instanceof Node) { + return false; + } + + return $this->isName($classMethod->returnType, DoctrineClass::COLLECTION); + } + + private function refactorNativeReturn(ClassMethod $classMethod): bool + { + if (! $classMethod->returnType instanceof Node) { + return false; + } + + if ($classMethod->returnType instanceof NullableType && $this->isName( + $classMethod->returnType->type, + DoctrineClass::COLLECTION + )) { + // unwrap nullable type + $classMethod->returnType = $classMethod->returnType->type; + return true; + } + + if (! $classMethod->returnType instanceof UnionType) { + return false; + } + + $unionType = $classMethod->returnType; + if (! $this->hasNativeReturnCollectionType($unionType)) { + return false; + } + + // remove null from union type + foreach ($unionType->types as $key => $unionedType) { + if ($this->isName($unionedType, 'null')) { + unset($unionType->types[$key]); + } + } + + return true; + } + + private function hasNativeReturnCollectionType(UnionType $unionType): bool + { + foreach ($unionType->types as $unionedType) { + if ($this->isName($unionedType, DoctrineClass::COLLECTION)) { + return true; + } + } + + return false; } } diff --git a/rules/TypedCollections/Rector/FuncCall/ArrayMapOnCollectionToArrayRector.php b/rules/TypedCollections/Rector/FuncCall/ArrayMapOnCollectionToArrayRector.php index f3a1cda8..ae07e51a 100644 --- a/rules/TypedCollections/Rector/FuncCall/ArrayMapOnCollectionToArrayRector.php +++ b/rules/TypedCollections/Rector/FuncCall/ArrayMapOnCollectionToArrayRector.php @@ -25,7 +25,7 @@ public function __construct( public function getRuleDefinition(): RuleDefinition { return new RuleDefinition( - 'Change array_map on Collection typed property to ->toArray() call, to always provide an array', + 'Change array_map() and array_filter() on Collection typed property to ->toArray() call, to always provide an array', [ new CodeSample( <<<'CODE_SAMPLE' @@ -84,18 +84,38 @@ public function refactor(Node $node): ?FuncCall return null; } - if (! $this->isName($node->name, 'array_map')) { - return null; + if ($this->isName($node->name, 'array_map')) { + return $this->refactorArrayMap($node); + } + + if ($this->isName($node->name, 'array_filter')) { + $this->refactorArrayFilter($node); } - $secondArg = $node->getArgs()[1]; + return null; + } + private function refactorArrayMap(FuncCall $funcCall): null|FuncCall + { + $secondArg = $funcCall->getArgs()[1]; if (! $this->collectionTypeDetector->isCollectionType($secondArg->value)) { return null; } $secondArg->value = new MethodCall($secondArg->value, 'toArray'); - return $node; + return $funcCall; + } + + private function refactorArrayFilter(FuncCall $funcCall): ?FuncCall + { + $firstArg = $funcCall->getArgs()[0]; + if (! $this->collectionTypeDetector->isCollectionType($firstArg->value)) { + return null; + } + + $firstArg->value = new MethodCall($firstArg->value, 'toArray'); + + return $funcCall; } }