From 54c7629b7243027b5331dd4d6138cc0bf3c21120 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Thu, 12 Jun 2025 14:33:17 +0200 Subject: [PATCH 1/2] add failing fixture --- .../Fixture/property_with_other_doc.php.inc | 51 +++++++++++++++++++ .../remove_from_property_docblock.php.inc | 2 +- ...emove_union_from_property_docblock.php.inc | 2 +- ...veNullFromNullableCollectionTypeRector.php | 35 +++++++------ 4 files changed, 70 insertions(+), 20 deletions(-) create mode 100644 rules-tests/TypedCollections/Rector/ClassMethod/RemoveNullFromNullableCollectionTypeRector/Fixture/property_with_other_doc.php.inc diff --git a/rules-tests/TypedCollections/Rector/ClassMethod/RemoveNullFromNullableCollectionTypeRector/Fixture/property_with_other_doc.php.inc b/rules-tests/TypedCollections/Rector/ClassMethod/RemoveNullFromNullableCollectionTypeRector/Fixture/property_with_other_doc.php.inc new file mode 100644 index 00000000..31dc5ef2 --- /dev/null +++ b/rules-tests/TypedCollections/Rector/ClassMethod/RemoveNullFromNullableCollectionTypeRector/Fixture/property_with_other_doc.php.inc @@ -0,0 +1,51 @@ +|null + * @Assert\Valid() + */ + private Collection $collection; + + public function __construct() + { + $this->collection = new ArrayCollection([]); + } +} + +?> +----- + + * @Assert\Valid() + */ + private Collection $collection; + + public function __construct() + { + $this->collection = new ArrayCollection([]); + } +} + +?> diff --git a/rules-tests/TypedCollections/Rector/ClassMethod/RemoveNullFromNullableCollectionTypeRector/Fixture/remove_from_property_docblock.php.inc b/rules-tests/TypedCollections/Rector/ClassMethod/RemoveNullFromNullableCollectionTypeRector/Fixture/remove_from_property_docblock.php.inc index dcd84c5e..21da50be 100644 --- a/rules-tests/TypedCollections/Rector/ClassMethod/RemoveNullFromNullableCollectionTypeRector/Fixture/remove_from_property_docblock.php.inc +++ b/rules-tests/TypedCollections/Rector/ClassMethod/RemoveNullFromNullableCollectionTypeRector/Fixture/remove_from_property_docblock.php.inc @@ -30,7 +30,7 @@ use Doctrine\Common\Collections\Collection; final class RemoveFromPropertyDocblock { /** - * @var Collection + * @var \Doctrine\Common\Collections\Collection */ private Collection $collection; diff --git a/rules-tests/TypedCollections/Rector/ClassMethod/RemoveNullFromNullableCollectionTypeRector/Fixture/remove_union_from_property_docblock.php.inc b/rules-tests/TypedCollections/Rector/ClassMethod/RemoveNullFromNullableCollectionTypeRector/Fixture/remove_union_from_property_docblock.php.inc index ef0f957d..77dbad98 100644 --- a/rules-tests/TypedCollections/Rector/ClassMethod/RemoveNullFromNullableCollectionTypeRector/Fixture/remove_union_from_property_docblock.php.inc +++ b/rules-tests/TypedCollections/Rector/ClassMethod/RemoveNullFromNullableCollectionTypeRector/Fixture/remove_union_from_property_docblock.php.inc @@ -30,7 +30,7 @@ use Doctrine\Common\Collections\Collection; final class RemoveUnionFromPropertyDocblock { /** - * @var Collection + * @var \Doctrine\Common\Collections\Collection */ private Collection $collection; diff --git a/rules/TypedCollections/Rector/ClassMethod/RemoveNullFromNullableCollectionTypeRector.php b/rules/TypedCollections/Rector/ClassMethod/RemoveNullFromNullableCollectionTypeRector.php index b06cf1e2..a3fa4e83 100644 --- a/rules/TypedCollections/Rector/ClassMethod/RemoveNullFromNullableCollectionTypeRector.php +++ b/rules/TypedCollections/Rector/ClassMethod/RemoveNullFromNullableCollectionTypeRector.php @@ -15,10 +15,11 @@ use PHPStan\PhpDocParser\Ast\Type\UnionTypeNode; use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo; use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory; -use Rector\Comments\NodeDocBlock\DocBlockUpdater; +use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTypeChanger; use Rector\Doctrine\Enum\DoctrineClass; use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer; use Rector\Rector\AbstractRector; +use Rector\StaticTypeMapper\StaticTypeMapper; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -30,7 +31,8 @@ final class RemoveNullFromNullableCollectionTypeRector extends AbstractRector public function __construct( private readonly TestsNodeAnalyzer $testsNodeAnalyzer, private readonly PhpDocInfoFactory $phpDocInfoFactory, - private readonly DocBlockUpdater $docBlockUpdater, + private readonly PhpDocTypeChanger $phpDocTypeChanger, + private readonly StaticTypeMapper $staticTypeMapper ) { } @@ -96,6 +98,7 @@ private function refactorClassMethod(ClassMethod $classMethod): null|ClassMethod return null; } + // nullable might be on purpose, e.g. via data provider if ($this->testsNodeAnalyzer->isInTestClass($classMethod)) { return null; } @@ -155,10 +158,13 @@ private function refactorProperty(Property $property): ?Property // only one type left, lets use it directly if (count($unionTypeNode->types) === 1) { $onlyType = array_pop($unionTypeNode->types); - $varTagValueNode->type = $onlyType; + $finalType = $onlyType; + } else { + $finalType = $unionTypeNode; } - $this->updateVarTagValueNode($phpDocInfo, $varTagValueNode, $property); + $finalType = $this->staticTypeMapper->mapPHPStanPhpDocTypeNodeToPHPStanType($finalType, $property); + $this->phpDocTypeChanger->changeVarType($property, $phpDocInfo, $finalType); return $property; } @@ -170,8 +176,13 @@ private function refactorProperty(Property $property): ?Property } // unwrap nullable type - $varTagValueNode->type = $varTagValueNode->type->type; - $this->updateVarTagValueNode($phpDocInfo, $varTagValueNode, $property); + // $varTagValueNode->type = $varTagValueNode->type->type; + + $finalType = $this->staticTypeMapper->mapPHPStanPhpDocTypeNodeToPHPStanType( + $varTagValueNode->type->type, + $property + ); + $this->phpDocTypeChanger->changeVarType($property, $phpDocInfo, $finalType); return $property; } @@ -184,16 +195,4 @@ private function hasNativeCollectionType(Property $property): bool return $this->isName($property->type, DoctrineClass::COLLECTION); } - - private function updateVarTagValueNode( - PhpDocInfo $phpDocInfo, - VarTagValueNode $varTagValueNode, - Property $property - ): void { - - $phpDocInfo->removeByType(VarTagValueNode::class); - $phpDocInfo->addTagValueNode($varTagValueNode); - - $this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($property); - } } From c9bd4cd99d0ae261682a7b77b429a77644925a99 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Thu, 12 Jun 2025 14:44:20 +0200 Subject: [PATCH 2/2] fixup! add failing fixture --- .../ClassMethod/RemoveNullFromNullableCollectionTypeRector.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rules/TypedCollections/Rector/ClassMethod/RemoveNullFromNullableCollectionTypeRector.php b/rules/TypedCollections/Rector/ClassMethod/RemoveNullFromNullableCollectionTypeRector.php index a3fa4e83..f653776b 100644 --- a/rules/TypedCollections/Rector/ClassMethod/RemoveNullFromNullableCollectionTypeRector.php +++ b/rules/TypedCollections/Rector/ClassMethod/RemoveNullFromNullableCollectionTypeRector.php @@ -176,12 +176,11 @@ private function refactorProperty(Property $property): ?Property } // unwrap nullable type - // $varTagValueNode->type = $varTagValueNode->type->type; - $finalType = $this->staticTypeMapper->mapPHPStanPhpDocTypeNodeToPHPStanType( $varTagValueNode->type->type, $property ); + $this->phpDocTypeChanger->changeVarType($property, $phpDocInfo, $finalType); return $property;