Skip to content

[PropertyInfo] ConstructorExtractor can read from promoted properties #62385

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
xavierleune wants to merge 3 commits into
base: 7.3
Choose a base branch
from
+237 −14
Open

[PropertyInfo] ConstructorExtractor can read from promoted properties #62385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
22de9db
[PropertyInfo] ConstructorExtractor now fallbacks on promoted properties
xavierleune Nov 13, 2025
18c682c
[PropertyInfo] ConstructorExtractor gives priority to @var (consisten…
xavierleune Nov 14, 2025
cbfd37a
[PropertyInfo] ConstructorExtractor gives priority to @var (consisten…
xavierleune Nov 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 46 additions & 5 deletions src/Symfony/Bundle/FrameworkBundle/Tests/Functional/SerializerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,26 @@
*/
class SerializerTest extends AbstractWebTestCase
{
public function testDeserializeArrayOfObject()
/**
* @dataProvider provideDeserializeArrayOfObjectData
*/
public function testDeserializeArrayOfObject(string $expectedClass, bool $usePromotedProperties, bool $withConstructorExtractor)
Copy link
Member

@HypeMC HypeMC Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lowest-deps build is failing because it's using PropertyInfo v6.4.0. Even though the with_constructor_extractor option was added in v7.3, the actual constructor extractor logic was added in v5.2.

It might make sense to split the PR and apply the PropertyInfo part of the fix to 6.4, but I'm not completely sure about that.

{
static::bootKernel(['test_case' => 'Serializer']);
static::bootKernel(['test_case' => 'Serializer', 'root_config' => $withConstructorExtractor ? 'config.yml' : 'config_without_constructor_extractor.yml']);

$result = static::getContainer()->get('serializer.alias')->deserialize('{"bars": [{"id": 1}, {"id": 2}]}', Foo::class, 'json');
$result = static::getContainer()->get('serializer.alias')->deserialize('{"bars": [{"id": 1}, {"id": 2}]}', $expectedClass, 'json');

$bar1 = new Bar();
$bar1->id = 1;
$bar2 = new Bar();
$bar2->id = 2;

$expected = new Foo();
$expected->bars = [$bar1, $bar2];
if ($usePromotedProperties) {
$expected = new ($expectedClass)([$bar1, $bar2]);
} else {
$expected = new $expectedClass();
$expected->bars = [$bar1, $bar2];
}

$this->assertEquals($expected, $result);
}
Expand Down Expand Up @@ -64,6 +71,18 @@ protected static function getKernelClass(): string
{
return SerializerKernel::class;
}

public function provideDeserializeArrayOfObjectData(): array
{
return [
Copy link
Member

@HypeMC HypeMC Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not yield here as well?

['expectedClass' => Foo::class, 'usePromotedProperties' => false, 'withConstructorExtractor' => false],
['expectedClass' => FooVar::class, 'usePromotedProperties' => true, 'withConstructorExtractor' => false],
['expectedClass' => FooParam::class, 'usePromotedProperties' => true, 'withConstructorExtractor' => false],
['expectedClass' => Foo::class, 'usePromotedProperties' => false, 'withConstructorExtractor' => true],
['expectedClass' => FooVar::class, 'usePromotedProperties' => true, 'withConstructorExtractor' => true],
['expectedClass' => FooParam::class, 'usePromotedProperties' => true, 'withConstructorExtractor' => true],
];
}
}

class SerializerKernel extends AppKernel implements CompilerPassInterface
Expand Down Expand Up @@ -117,3 +136,25 @@ class Bar
*/
public $id;
}

class FooVar
{
public function __construct(
/**
* @var Bar[]
*/
public array $bars,
) {
}
}

class FooParam
{
/**
* @param Bar[] $bars
*/
public function __construct(
public array $bars,
) {
}
}
24 changes: 24 additions & 0 deletions .../FrameworkBundle/Tests/Functional/app/Serializer/config_without_constructor_extractor.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
imports:
- { resource: ../config/default.yml }

framework:
http_method_override: false
translator: true
serializer:
enabled: true
circular_reference_handler: Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Serializer\CircularReferenceHandler
max_depth_handler: Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Serializer\MaxDepthHandler
default_context:
enable_max_depth: true
property_info:
enabled: true
with_constructor_extractor: false

services:
serializer.alias:
alias: serializer
public: true

Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Serializer\CircularReferenceHandler: ~

Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Serializer\MaxDepthHandler: ~
45 changes: 38 additions & 7 deletions src/Symfony/Component/PropertyInfo/Extractor/PhpDocExtractor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,15 +183,20 @@ public function getTypesFromConstructor(string $class, string $property): ?array
{
trigger_deprecation('symfony/property-info', '7.3', 'The "%s()" method is deprecated, use "%s::getTypeFromConstructor()" instead.', __METHOD__, self::class);

$docBlock = $this->getDocBlockFromConstructor($class, $property);
// Give priority to @var declarations, fallback on @param on constructor if not found
if (!$docBlock = $this->getDocBlockFromPromotedProperty($class, $property)) {
if (!$docBlock = $this->getDocBlockFromConstructor($class, $property)) {
return null;
}

if (!$docBlock) {
return null;
$tags = $docBlock->getTagsByName('param');
} else {
$tags = $docBlock->getTagsByName('var');
}
Comment on lines +187 to 195
Copy link
Member

@HypeMC HypeMC Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this would be a bit more readable:

if ($docBlock = $this->getDocBlockFromPromotedProperty($class, $property)) {
    $tags = $docBlock->getTagsByName('var');
} elseif ($docBlock = $this->getDocBlockFromConstructor($class, $property)) {
    $tags = $docBlock->getTagsByName('param');
} else {
    return null;
}

Copy link
Contributor

@webda2l webda2l Dec 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'll go for:

    $tags = $this->getDocBlockFromPromotedProperty($class, $property)?->getTagsByName('var')
        ?? $this->getDocBlockFromConstructor($class, $property)?->getTagsByName('param');
        
   if (null === $tags) {
     return null;
  }


$types = [];
/** @var DocBlock\Tags\Var_|DocBlock\Tags\Return_|DocBlock\Tags\Param $tag */
foreach ($docBlock->getTagsByName('param') as $tag) {
foreach ($tags as $tag) {
if ($tag && null !== $tag->getType()) {
$types[] = $this->phpDocTypeHelper->getTypes($tag->getType());
}
Expand Down Expand Up @@ -265,13 +270,20 @@ public function getType(string $class, string $property, array $context = []): ?

public function getTypeFromConstructor(string $class, string $property): ?Type
{
if (!$docBlock = $this->getDocBlockFromConstructor($class, $property)) {
return null;
// Give priority to @var declarations, fallback on @param if not found
if (!$docBlock = $this->getDocBlockFromPromotedProperty($class, $property)) {
if (!$docBlock = $this->getDocBlockFromConstructor($class, $property)) {
return null;
}

$tags = $docBlock->getTagsByName('param');
} else {
$tags = $docBlock->getTagsByName('var');
}

$types = [];
/** @var DocBlock\Tags\Var_|DocBlock\Tags\Return_|DocBlock\Tags\Param $tag */
foreach ($docBlock->getTagsByName('param') as $tag) {
foreach ($tags as $tag) {
if ($tag instanceof InvalidTag || !$tagType = $tag->getType()) {
continue;
}
Expand Down Expand Up @@ -310,6 +322,25 @@ private function getDocBlockFromConstructor(string $class, string $property): ?D
}
}

private function getDocBlockFromPromotedProperty(string $class, string $property): ?DocBlock
{
try {
$reflectionProperty = new \ReflectionProperty($class, $property);
} catch (\ReflectionException) {
return null;
}

if (!$reflectionProperty->isPromoted()) {
return null;
}

try {
return $this->docBlockFactory->create($reflectionProperty, $this->createFromReflector($reflectionProperty->getDeclaringClass()));
} catch (\InvalidArgumentException|\RuntimeException) {
return null;
}
}

private function filterDocBlockParams(DocBlock $docBlock, string $allowedParam): DocBlock
{
$tags = array_values(array_filter($docBlock->getTagsByName('param'), fn ($tag) => $tag instanceof DocBlock\Tags\Param && $allowedParam === $tag->getVariableName()));
Expand Down
35 changes: 33 additions & 2 deletions src/Symfony/Component/PropertyInfo/Extractor/PhpStanExtractor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,8 @@ public function getTypesFromConstructor(string $class, string $property): ?array
{
trigger_deprecation('symfony/property-info', '7.3', 'The "%s()" method is deprecated, use "%s::getTypeFromConstructor()" instead.', __METHOD__, self::class);

if (null === $tagDocNode = $this->getDocBlockFromConstructor($class, $property)) {
// Give priority to @var declarations, fallback on @param if not found
if (!$tagDocNode = $this->getDocBlockFromPromotedProperty($class, $property) ?? $this->getDocBlockFromConstructor($class, $property)) {
return null;
}

Expand Down Expand Up @@ -246,7 +247,9 @@ public function getType(string $class, string $property, array $context = []): ?
public function getTypeFromConstructor(string $class, string $property): ?Type
{
$declaringClass = $class;
if (!$tagDocNode = $this->getDocBlockFromConstructor($declaringClass, $property)) {

// Give priority to @var declarations, fallback on @param if not found
if (!$tagDocNode = $this->getDocBlockFromPromotedProperty($declaringClass, $property) ?? $this->getDocBlockFromConstructor($declaringClass, $property)) {
return null;
}

Expand Down Expand Up @@ -408,6 +411,34 @@ private function filterDocBlockParams(PhpDocNode $docNode, string $allowedParam)
return $tags[0]->value;
}

private function getDocBlockFromPromotedProperty(string &$class, string $property): ?VarTagValueNode
{
try {
$reflectionProperty = new \ReflectionProperty($class, $property);
} catch (\ReflectionException) {
return null;
}

if (!$reflectionProperty->isPromoted()) {
return null;
}

if (!$this->canAccessMemberBasedOnItsVisibility($reflectionProperty)) {
return null;
}

if (!$rawDocNode = $reflectionProperty->getDocComment()) {
return null;
}

$phpDocNode = $this->getPhpDocNode($rawDocNode);
$class = $reflectionProperty->class;

$tags = array_values(array_filter($phpDocNode->getTagsByName('@var'), static fn ($tagNode) => $tagNode->value instanceof VarTagValueNode));

return $tags ? $tags[0]->value : null;
}

/**
* @return array{PhpDocNode|null, int|null, string|null, string|null}
*/
Expand Down
19 changes: 19 additions & 0 deletions src/Symfony/Component/PropertyInfo/Tests/Extractor/PhpDocExtractorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
use Symfony\Component\PropertyInfo\Tests\Fixtures\InvalidDummy;
use Symfony\Component\PropertyInfo\Tests\Fixtures\ParentDummy;
use Symfony\Component\PropertyInfo\Tests\Fixtures\Php80Dummy;
use Symfony\Component\PropertyInfo\Tests\Fixtures\Php80PromotedDummyWithDocblock;
use Symfony\Component\PropertyInfo\Tests\Fixtures\PseudoTypeDummy;
use Symfony\Component\PropertyInfo\Tests\Fixtures\PseudoTypesDummy;
use Symfony\Component\PropertyInfo\Tests\Fixtures\TraitUsage\DummyUsedInTrait;
Expand Down Expand Up @@ -887,6 +888,24 @@ public static function constructorTypesProvider(): iterable
yield ['mixed', Type::mixed()];
}

/**
* @dataProvider constructorPromotedPropertyWithDocblockProvider
*/
public function testExtractConstructorPromotedPropertyWithDocblock(string $property, ?Type $type)
{
$this->assertEquals($type, $this->extractor->getTypeFromConstructor(Php80PromotedDummyWithDocblock::class, $property));
}

/**
* @return iterable<array{0: string, 1: ?Type}>
*/
public static function constructorPromotedPropertyWithDocblockProvider(): iterable
{
// Test that promoted properties with @var docblock are recognized
yield ['dates', Type::list(Type::object(\DateTimeImmutable::class))];
yield ['datesWithIncoherentDocBlock', Type::list(Type::object(\DateTimeImmutable::class))];
}

/**
* @dataProvider pseudoTypeProvider
*/
Expand Down
19 changes: 19 additions & 0 deletions src/Symfony/Component/PropertyInfo/Tests/Extractor/PhpStanExtractorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
use Symfony\Component\PropertyInfo\Tests\Fixtures\ParentDummy;
use Symfony\Component\PropertyInfo\Tests\Fixtures\Php80Dummy;
use Symfony\Component\PropertyInfo\Tests\Fixtures\Php80PromotedDummy;
use Symfony\Component\PropertyInfo\Tests\Fixtures\Php80PromotedDummyWithDocblock;
use Symfony\Component\PropertyInfo\Tests\Fixtures\PhpStanPseudoTypesDummy;
use Symfony\Component\PropertyInfo\Tests\Fixtures\RootDummy\RootDummyItem;
use Symfony\Component\PropertyInfo\Tests\Fixtures\TraitUsage\AnotherNamespace\DummyInAnotherNamespace;
Expand Down Expand Up @@ -947,6 +948,24 @@ public static function constructorTypesProvider(): iterable
yield ['ddd', null];
}

/**
* @dataProvider constructorPromotedPropertyWithDocblockProvider
*/
public function testExtractConstructorPromotedPropertyWithDocblock(string $property, ?Type $type)
{
$this->assertEquals($type, $this->extractor->getTypeFromConstructor(Php80PromotedDummyWithDocblock::class, $property));
}

/**
* @return iterable<array{0: string, 1: ?Type}>
*/
public static function constructorPromotedPropertyWithDocblockProvider(): iterable
{
// Test that promoted properties with @var docblock are recognized
yield ['dates', Type::list(Type::object(\DateTimeImmutable::class))];
yield ['datesWithIncoherentDocBlock', Type::list(Type::object(\DateTimeImmutable::class))];
}

/**
* @dataProvider constructorTypesOfParentClassProvider
*/
Expand Down
18 changes: 18 additions & 0 deletions src/Symfony/Component/PropertyInfo/Tests/Extractor/ReflectionExtractorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
use Symfony\Component\PropertyInfo\Tests\Fixtures\Php7Dummy;
use Symfony\Component\PropertyInfo\Tests\Fixtures\Php7ParentDummy;
use Symfony\Component\PropertyInfo\Tests\Fixtures\Php80Dummy;
use Symfony\Component\PropertyInfo\Tests\Fixtures\Php80PromotedDummyWithDocblock;
use Symfony\Component\PropertyInfo\Tests\Fixtures\Php81Dummy;
use Symfony\Component\PropertyInfo\Tests\Fixtures\Php82Dummy;
use Symfony\Component\PropertyInfo\Tests\Fixtures\SnakeCaseDummy;
Expand Down Expand Up @@ -1064,6 +1065,23 @@ public static function extractConstructorTypesProvider(): iterable
yield ['ddd', null];
}

/**
* @dataProvider constructorPromotedPropertyWithDocblockProvider
*/
public function testExtractConstructorPromotedPropertyWithDocblock(string $property, ?Type $type)
{
$this->assertEquals($type, $this->extractor->getTypeFromConstructor(Php80PromotedDummyWithDocblock::class, $property));
}

/**
* @return iterable<array{0: string, 1: ?Type}>
*/
public static function constructorPromotedPropertyWithDocblockProvider(): iterable
{
// ReflectionExtractor can only extract native PHP types, not generic types from docblocks
yield ['dates', Type::array()];
Copy link
Member

@HypeMC HypeMC Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for the data provider if there's only one case.

}

/**
* @dataProvider camelizeProvider
*/
Expand Down
40 changes: 40 additions & 0 deletions src/Symfony/Component/PropertyInfo/Tests/Fixtures/Php80PromotedDummyWithDocblock.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\PropertyInfo\Tests\Fixtures;

class Php80PromotedDummyWithDocblock
{
/**
* @param \DateTime[] $datesWithIncoherentDocBlock
Copy link
Contributor

@mtarld mtarld Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you are testing with Type::list, you might want to write list<\DateTime> instead. Otherwise, tests will fail after #62388 being merged.

*/
public function __construct(
/**
* @var \DateTimeImmutable[]
*/
private array $dates,
/**
* @var \DateTimeImmutable[] $datesWithIncoherentDocBlock
*/
private array $datesWithIncoherentDocBlock,
) {
}

public function getDates(): array
{
return $this->dates;
}

public function getDatesWithIncoherentDocBlock(): array
{
return $this->datesWithIncoherentDocBlock;
}
}
Loading