From 2003ed18cd7fb2e9c272cb42fed017a4b75b40bf Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 28 Oct 2025 20:04:15 +0100 Subject: [PATCH 1/2] cleanup --- .../Fixture/float_var_string.php.inc | 30 ++++ ...tyFromJMSSerializerAttributeTypeRector.php | 129 +++++++++++------- 2 files changed, 107 insertions(+), 52 deletions(-) create mode 100644 rules-tests/TypeDeclaration/Rector/Class_/TypedPropertyFromJMSSerializerAttributeTypeRector/Fixture/float_var_string.php.inc diff --git a/rules-tests/TypeDeclaration/Rector/Class_/TypedPropertyFromJMSSerializerAttributeTypeRector/Fixture/float_var_string.php.inc b/rules-tests/TypeDeclaration/Rector/Class_/TypedPropertyFromJMSSerializerAttributeTypeRector/Fixture/float_var_string.php.inc new file mode 100644 index 00000000000..6d5af71701e --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/Class_/TypedPropertyFromJMSSerializerAttributeTypeRector/Fixture/float_var_string.php.inc @@ -0,0 +1,30 @@ + +----- + diff --git a/rules/TypeDeclaration/Rector/Class_/TypedPropertyFromJMSSerializerAttributeTypeRector.php b/rules/TypeDeclaration/Rector/Class_/TypedPropertyFromJMSSerializerAttributeTypeRector.php index e8afb98afeb..34b2a0c02f8 100644 --- a/rules/TypeDeclaration/Rector/Class_/TypedPropertyFromJMSSerializerAttributeTypeRector.php +++ b/rules/TypeDeclaration/Rector/Class_/TypedPropertyFromJMSSerializerAttributeTypeRector.php @@ -5,6 +5,7 @@ namespace Rector\TypeDeclaration\Rector\Class_; use PhpParser\Node; +use PhpParser\Node\Attribute; use PhpParser\Node\Expr; use PhpParser\Node\Expr\ConstFetch; use PhpParser\Node\Identifier; @@ -16,7 +17,9 @@ use PHPStan\Reflection\ClassReflection; use PHPStan\Type\MixedType; use PHPStan\Type\ObjectType; +use PHPStan\Type\Type; use Rector\Doctrine\CodeQuality\Enum\CollectionMapping; +use Rector\Doctrine\NodeAnalyzer\AttributeFinder; use Rector\Enum\ClassName; use Rector\Php74\Guard\MakePropertyTypedGuard; use Rector\Php80\NodeAnalyzer\PhpAttributeAnalyzer; @@ -43,7 +46,8 @@ public function __construct( private readonly ValueResolver $valueResolver, private readonly PhpAttributeAnalyzer $phpAttributeAnalyzer, private readonly ScalarStringToTypeMapper $scalarStringToTypeMapper, - private readonly StaticTypeMapper $staticTypeMapper + private readonly StaticTypeMapper $staticTypeMapper, + private readonly AttributeFinder $attributeFinder, ) { } @@ -90,34 +94,18 @@ public function provideMinPhpVersion(): int public function refactor(Node $node): ?Node { $hasChanged = false; - $classReflection = null; - foreach ($node->getProperties() as $property) { - if ($property->type instanceof Node || $property->props[0]->default instanceof Expr) { - continue; - } - - if (! $this->phpAttributeAnalyzer->hasPhpAttribute($property, ClassName::JMS_TYPE)) { - continue; - } - - // this will be most likely collection, not single type - if ($this->phpAttributeAnalyzer->hasPhpAttributes( - $property, - array_merge(CollectionMapping::TO_MANY_CLASSES, CollectionMapping::TO_ONE_CLASSES) - )) { - continue; - } - - if (! $classReflection instanceof ClassReflection) { - $classReflection = $this->reflectionResolver->resolveClassReflection($node); - } + if (! $this->hasAtLeastOneUntypedPropertyUsingJmsAttribute($node)) { + return null; + } - if (! $classReflection instanceof ClassReflection) { - return null; - } + $classReflection = $this->reflectionResolver->resolveClassReflection($node); + if (! $classReflection instanceof ClassReflection) { + return null; + } - if (! $this->makePropertyTypedGuard->isLegal($property, $classReflection, true)) { + foreach ($node->getProperties() as $property) { + if ($this->shouldSkipProperty($property, $classReflection)) { continue; } @@ -131,20 +119,15 @@ public function refactor(Node $node): ?Node continue; } - $type = $this->scalarStringToTypeMapper->mapScalarStringToType($typeValue); - if ($type instanceof MixedType) { - // fallback to object type - $type = new ObjectType($typeValue); - } + $type = $this->createType($typeValue); $propertyType = $this->staticTypeMapper->mapPHPStanTypeToPhpParserNode($type, TypeKind::PROPERTY); - if (! $propertyType instanceof Identifier && ! $propertyType instanceof FullyQualified) { - return null; + continue; } $property->type = new NullableType($propertyType); - $property->props[0]->default = new ConstFetch(new Name('null')); + $property->props[0]->default = $this->nodeFactory->createNull(); $hasChanged = true; } @@ -158,26 +141,68 @@ public function refactor(Node $node): ?Node private function resolveAttributeType(Property $property): ?string { - foreach ($property->attrGroups as $attrGroup) { - foreach ($attrGroup->attrs as $attr) { - if (! $this->isName($attr->name, ClassName::JMS_TYPE)) { - continue; - } - - $typeValue = $this->valueResolver->getValue($attr->args[0]->value); - if (! is_string($typeValue)) { - return null; - } - - if (StringUtils::isMatch($typeValue, '#DateTime\<(.*?)\>#')) { - // special case for DateTime, which is not a scalar type - return 'DateTime'; - } - - return $typeValue; + $jmsTypeAttribute = $this->attributeFinder->findAttributeByClass($property, ClassName::JMS_TYPE); + if (! $jmsTypeAttribute instanceof Attribute) { + return null; + } + + $typeValue = $this->valueResolver->getValue($jmsTypeAttribute->args[0]->value); + if (! is_string($typeValue)) { + return null; + } + + if (StringUtils::isMatch($typeValue, '#DateTime\<(.*?)\>#')) { + // special case for DateTime, which is not a scalar type + return 'DateTime'; + } + + return $typeValue; + } + + private function hasAtLeastOneUntypedPropertyUsingJmsAttribute(Class_ $class): bool + { + foreach ($class->getProperties() as $property) { + if ($property->type instanceof Node) { + continue; + } + + if ($this->attributeFinder->hasAttributeByClasses($property, [ClassName::JMS_TYPE])) { + return true; } } - return null; + return false; + } + + private function createType(string $typeValue): Type + { + $type = $this->scalarStringToTypeMapper->mapScalarStringToType($typeValue); + if (! $type instanceof MixedType) { + return $type; + } + + // fallback to object type + return new ObjectType($typeValue); + } + + private function shouldSkipProperty(Property $property, ClassReflection $classReflection): bool + { + if ($property->type instanceof Node || $property->props[0]->default instanceof Expr) { + return true; + } + + if (!$this->phpAttributeAnalyzer->hasPhpAttribute($property, ClassName::JMS_TYPE)) { + return true; + } + + // this will be most likely collection, not single type + if ($this->phpAttributeAnalyzer->hasPhpAttributes( + $property, + array_merge(CollectionMapping::TO_MANY_CLASSES, CollectionMapping::TO_ONE_CLASSES) + )) { + return true; + } + + return ! $this->makePropertyTypedGuard->isLegal($property, $classReflection); } } From af9b2aeaa33aea34bffe31e14efc24f182f6199f Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 28 Oct 2025 20:09:22 +0100 Subject: [PATCH 2/2] prefer string over float --- .../Fixture/float_var_string.php.inc | 2 +- ...tyFromJMSSerializerAttributeTypeRector.php | 40 +++++++++++++------ 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/rules-tests/TypeDeclaration/Rector/Class_/TypedPropertyFromJMSSerializerAttributeTypeRector/Fixture/float_var_string.php.inc b/rules-tests/TypeDeclaration/Rector/Class_/TypedPropertyFromJMSSerializerAttributeTypeRector/Fixture/float_var_string.php.inc index 6d5af71701e..c2ec4e75038 100644 --- a/rules-tests/TypeDeclaration/Rector/Class_/TypedPropertyFromJMSSerializerAttributeTypeRector/Fixture/float_var_string.php.inc +++ b/rules-tests/TypeDeclaration/Rector/Class_/TypedPropertyFromJMSSerializerAttributeTypeRector/Fixture/float_var_string.php.inc @@ -21,7 +21,7 @@ namespace Rector\Tests\TypeDeclaration\Rector\Class_\TypedPropertyFromJMSSeriali use JMS\Serializer\Annotation\Type; -final class ExactClass +final class FloatVarString { #[Type('float')] private ?string $price = null; diff --git a/rules/TypeDeclaration/Rector/Class_/TypedPropertyFromJMSSerializerAttributeTypeRector.php b/rules/TypeDeclaration/Rector/Class_/TypedPropertyFromJMSSerializerAttributeTypeRector.php index 34b2a0c02f8..8bb276b09d8 100644 --- a/rules/TypeDeclaration/Rector/Class_/TypedPropertyFromJMSSerializerAttributeTypeRector.php +++ b/rules/TypeDeclaration/Rector/Class_/TypedPropertyFromJMSSerializerAttributeTypeRector.php @@ -7,9 +7,7 @@ use PhpParser\Node; use PhpParser\Node\Attribute; use PhpParser\Node\Expr; -use PhpParser\Node\Expr\ConstFetch; use PhpParser\Node\Identifier; -use PhpParser\Node\Name; use PhpParser\Node\Name\FullyQualified; use PhpParser\Node\NullableType; use PhpParser\Node\Stmt\Class_; @@ -17,7 +15,11 @@ use PHPStan\Reflection\ClassReflection; use PHPStan\Type\MixedType; use PHPStan\Type\ObjectType; +use PHPStan\Type\StringType; use PHPStan\Type\Type; +use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo; +use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory; +use Rector\DeadCode\PhpDoc\TagRemover\VarTagRemover; use Rector\Doctrine\CodeQuality\Enum\CollectionMapping; use Rector\Doctrine\NodeAnalyzer\AttributeFinder; use Rector\Enum\ClassName; @@ -48,6 +50,8 @@ public function __construct( private readonly ScalarStringToTypeMapper $scalarStringToTypeMapper, private readonly StaticTypeMapper $staticTypeMapper, private readonly AttributeFinder $attributeFinder, + private readonly PhpDocInfoFactory $phpDocInfoFactory, + private readonly VarTagRemover $varTagRemover ) { } @@ -119,14 +123,12 @@ public function refactor(Node $node): ?Node continue; } - $type = $this->createType($typeValue); - - $propertyType = $this->staticTypeMapper->mapPHPStanTypeToPhpParserNode($type, TypeKind::PROPERTY); - if (! $propertyType instanceof Identifier && ! $propertyType instanceof FullyQualified) { + $propertyTypeNode = $this->createTypeNode($typeValue, $property); + if (! $propertyTypeNode instanceof Identifier && ! $propertyTypeNode instanceof FullyQualified) { continue; } - $property->type = new NullableType($propertyType); + $property->type = new NullableType($propertyTypeNode); $property->props[0]->default = $this->nodeFactory->createNull(); $hasChanged = true; @@ -174,15 +176,27 @@ private function hasAtLeastOneUntypedPropertyUsingJmsAttribute(Class_ $class): b return false; } - private function createType(string $typeValue): Type + private function createTypeNode(string $typeValue, Property $property): ?Node { + if ($typeValue === 'float') { + $propertyPhpDocInfo = $this->phpDocInfoFactory->createFromNode($property); + if ($propertyPhpDocInfo instanceof PhpDocInfo) { + // fallback to string, as most likely string representation of float + if ($propertyPhpDocInfo->getVarType() instanceof StringType) { + $this->varTagRemover->removeVarTag($property); + + return new Identifier('string'); + } + } + } + $type = $this->scalarStringToTypeMapper->mapScalarStringToType($typeValue); - if (! $type instanceof MixedType) { - return $type; + if ($type instanceof MixedType) { + // fallback to object type + $type = new ObjectType($typeValue); } - // fallback to object type - return new ObjectType($typeValue); + return $this->staticTypeMapper->mapPHPStanTypeToPhpParserNode($type, TypeKind::PROPERTY); } private function shouldSkipProperty(Property $property, ClassReflection $classReflection): bool @@ -191,7 +205,7 @@ private function shouldSkipProperty(Property $property, ClassReflection $classRe return true; } - if (!$this->phpAttributeAnalyzer->hasPhpAttribute($property, ClassName::JMS_TYPE)) { + if (! $this->phpAttributeAnalyzer->hasPhpAttribute($property, ClassName::JMS_TYPE)) { return true; }