8000 bug #52699 [Serializer] [PropertyAccessor] Ignore non-collection inte… · symfony/symfony@f4b0c27 · GitHub
[go: up one dir, main page]
Skip to content

Commit f4b0c27

Browse files
fabpotfabpot
committed
bug #52699 [Serializer] [PropertyAccessor] Ignore non-collection interface generics (mtarld)
This PR was merged into the 5.4 branch. Discussion ---------- [Serializer] [PropertyAccessor] Ignore non-collection interface generics | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix #41996 #49924 | License | MIT `PhpDocExtractor` and `PhpStanDocExtractor` are following an old version of PSR-5 with used to define collections as the following: ``` generic = collection-type "<" [type-expression "," *SP] type-expression ">" ``` But, it does conflict with non-collection generics. This issue will automatically be solved if the `TypeInfo` is merged in Symfony. But for older versions (<7.1 at least), it needs a fix. ~I was not able to find a proper bug fix without introducing a BC break, so this PR proposes to opt-on the "non-collection generics" parsing, such as `stcClass<int>` for example.~ ~To opt-out from parsing these generics, it'll be required to set `ignore_docblock_generics` in the context. And this key/value will become obsolete as soon as the `TypeInfo` is introduced.~ ~I'm not sure how to proceed though, should we only merge it in 5.4, and 6.3 supposing that the `TypeInfo` might be merged in 7.x? Should we document it only in these branches?~ EDIT: I finally ignored PHPDoc generics that aren't well known collection generic types so that the process will fall back to other resolvers (such as reflection resolver for example) Commits ------- e31aeeb [PropertyAccessor] Fix unexpected collection when generics
2 parents 2eb46eb + e31aeeb commit f4b0c27

File tree

8 files changed

+39
-7
lines changed

8 files changed

+39
-7
lines changed

‎src/Symfony/Component/PropertyInfo/Tests/Extractor/PhpDocExtractorTest.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,11 @@ public function testUnknownPseudoType()
428428
$this->assertEquals([new Type(Type::BUILTIN_TYPE_OBJECT, false, 'scalar')], $this->extractor->getTypes(PseudoTypeDummy::class, 'unknownPseudoType'));
429429
}
430430

431+
public function testGenericInterface()
432+
{
433+
$this->assertNull($this->extractor->getTypes(Dummy::class, 'genericInterface'));
434+
}
435+
431436
protected static function isPhpDocumentorV5()
432437
{
433438
if (class_exists(InvalidTag::class)) {

‎src/Symfony/Component/PropertyInfo/Tests/Extractor/PhpStanExtractorTest.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ public static function unionTypesProvider(): array
390390
['b', [new Type(Type::BUILTIN_TYPE_ARRAY, false, null, true, [new Type(Type::BUILTIN_TYPE_INT)], [new Type(Type::BUILTIN_TYPE_STRING), new Type(Type::BUILTIN_TYPE_INT)])]],
391391
['c', [new Type(Type::BUILTIN_TYPE_ARRAY, false, null, true, [], [new Type(Type::BUILTIN_TYPE_STRING), new Type(Type::BUILTIN_TYPE_INT)])]],
392392
['d', [new Type(Type::BUILTIN_TYPE_ARRAY, false, null, true, [new Type(Type::BUILTIN_TYPE_STRING), new Type(Type::BUILTIN_TYPE_INT)], [new Type(Type::BUILTIN_TYPE_ARRAY, false, null, true, [], [new Type(Type::BUILTIN_TYPE_STRING)])])]],
393-
['e', [new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class, true, [new Type(Type::BUILTIN_TYPE_ARRAY, false, null, true, [], [new Type(Type::BUILTIN_TYPE_STRING)])], [new Type(Type::BUILTIN_TYPE_INT), new Type(Type::BUILTIN_TYPE_ARRAY, false, null, true, [new Type(Type::BUILTIN_TYPE_INT)], [new Type(Type::BUILTIN_TYPE_STRING, false, null, true, [], [new Type(Type::BUILTIN_TYPE_OBJECT, false, DefaultValue::class)])])]), new Type(Type::BUILTIN_TYPE_OBJECT, false, ParentDummy::class)]],
393+
['e', [new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class, false, [new Type(Type::BUILTIN_TYPE_ARRAY, false, null, true, [], [new Type(Type::BUILTIN_TYPE_STRING)])], [new Type(Type::BUILTIN_TYPE_INT), new Type(Type::BUILTIN_TYPE_ARRAY, false, null, true, [new Type(Type::BUILTIN_TYPE_INT)], [new Type(Type::BUILTIN_TYPE_OBJECT, false, \Traversable::class, true, [], [new Type(Type::BUILTIN_TYPE_OBJECT, false, DefaultValue::class)])])]), new Type(Type::BUILTIN_TYPE_OBJECT, false, ParentDummy::class)]],
394394
['f', null],
395395
['g', [new Type(Type::BUILTIN_TYPE_ARRAY, false, null, true, [], [new Type(Type::BUILTIN_TYPE_STRING), new Type(Type::BUILTIN_TYPE_INT)])]],
396396
];
@@ -429,6 +429,11 @@ public static function intRangeTypeProvider(): array
429429
['c', [new Type(Type::BUILTIN_TYPE_INT)]],
430430
];
431431
}
432+
433+
public function testGenericInterface()
434+
{
435+
$this->assertNull($this->extractor->getTypes(Dummy::class, 'genericInterface'));
436+
}
432437
}
433438

434439
class PhpStanOmittedParamTagTypeDocBlock

‎src/Symfony/Component/PropertyInfo/Tests/Extractor/ReflectionExtractorTest.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ public function testGetProperties()
7373
'arrayOfMixed',
7474
'listOfStrings',
7575
'parentAnnotation',
76+
'genericInterface',
7677
'foo',
7778
'foo2',
7879
'foo3',
@@ -137,6 +138,7 @@ public function testGetPropertiesWithCustomPrefixes()
137138
'arrayOfMixed',
138139
'listOfStrings',
139140
'parentAnnotation',
141+
'genericInterface',
140142
'foo',
141143
'foo2',
142144
'foo3',
@@ -190,6 +192,7 @@ public function testGetPropertiesWithNoPrefixes()
190192
'arrayOfMixed',
191193
'listOfStrings',
192194
'parentAnnotation',
195+
'genericInterface',
193196
'foo',
194197
'foo2',
195198
'foo3',
< 57AE /code>

‎src/Symfony/Component/PropertyInfo/Tests/Fixtures/Dummy.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,11 @@ class Dummy extends ParentDummy
165165
*/
166166
public $parentAnnotation;
167167

168+
/**
169+
* @var \BackedEnum<string>
170+
*/
171+
public $genericInterface;
172+
168173
public static function getStatic()
169174
{
170175
}

‎src/Symfony/Component/PropertyInfo/Tests/Fixtures/DummyUnionType.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ class DummyUnionType
4040
public $d;
4141

4242
/**
43-
* @var (Dummy<array<mixed, string>, (int | (string<DefaultValue>)[])> | ParentDummy | null)
43+
* @var (Dummy<array<mixed, string>, (int | (\Traversable<DefaultValue>)[])> | ParentDummy | null)
4444
*/
4545
public $e;
4646

‎src/Symfony/Component/PropertyInfo/Util/PhpDocTypeHelper.php

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,9 @@ public function getTypes(DocType $varType): array
102102
/**
103103
* Creates a {@see Type} from a PHPDoc type.
104104
*/
105-
private function createType(DocType $type, bool $nullable, ?string $docType = null): ?Type
105+
private function createType(DocType $type, bool $nullable): ?Type
106106
{
107-
$docType = $docType ?? (string) $type;
107+
$docType = (string) $type;
108108

109109
if ($type instanceof Collection) {
110110
$fqsen = $type->getFqsen();
@@ -115,10 +115,17 @@ private function createType(DocType $type, bool $nullable, ?string $docType = nu
115115

116116
[$phpType, $class] = $this->getPhpTypeAndClass((string) $fqsen);
117117

118+
$collection = \is_a($class, \Traversable::class, true) || \is_a($class, \ArrayAccess::class, true);
119+
120+
// it's safer to fall back to other extractors if the generic type is too abstract
121+
if (!$collection && !class_exists($class)) {
122+
return null;
123+
}
124+
118125
$keys = $this->getTypes($type->getKeyType());
119126
$values = $this->getTypes($type->getValueType());
120127

121-
return new Type($phpType, $nullable, $class, true, $keys, $values);
128+
return new Type($phpType, $nullable, $class, $collection, $keys, $values);
122129
}
123130

124131
// Cannot guess

‎src/Symfony/Component/PropertyInfo/Util/PhpStanTypeHelper.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,13 @@ private function extractTypes(TypeNode $node, NameScope $nameScope): array
121121
return [$mainType];
122122
}
123123

124+
$collection = $mainType->isCollection() || \in_array($mainType->getClassName(), [\Traversable::class, \Iterator::class, \IteratorAggregate::class, \ArrayAccess::class, \Generator::class], true);
125+
126+
// it's safer to fall back to other extractors if the generic type is too abstract
127+
if (!$collection && !class_exists($mainType->getClassName())) {
128+
return [];
129+
}
130+
124131
$collectionKeyTypes = $mainType->getCollectionKeyTypes();
125132
$collectionKeyValues = [];
126133
if (1 === \count($node->genericTypes)) {
@@ -136,7 +143,7 @@ private function extractTypes(TypeNode $node, NameScope $nameScope): array
136143
}
137144
}
138145

139-
return [new Type($mainType->getBuiltinType(), $mainType->isNullable(), $mainType->getClassName(), true, $collectionKeyTypes, $collectionKeyValues)];
146+
return [new Type($mainType->getBuiltinType(), $mainType->isNullable(), $mainType->getClassName(), $collection, $collectionKeyTypes, $collectionKeyValues)];
140147
}
141148
if ($node instanceof ArrayShapeNode) {
142149
return [new Type(Type::BUILTIN_TYPE_ARRAY, false, null, true)];

‎src/Symfony/Component/Serializer/Tests/DeserializeNestedArrayOfObjectsTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ class ZooWithKeyTypes
156156
public $animalsString = [];
157157
/** @var array<int|string, Animal> */
158158
public $animalsUnion = [];
159-
/** @var \stdClass<Animal> */
159+
/** @var \Traversable<Animal> */
160160
public $animalsGenerics = [];
161161
}
162162

0 commit comments

Comments
 (0)
0