Skip to content
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.

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

[TypeInfo] Handle custom collection objects properly #54661

Merged
merged 1 commit into from Apr 30, 2024

Conversation

mtarld
Copy link
Contributor

@mtarld mtarld commented Apr 18, 2024

Q A
Branch? 7.1
Bug fix? no
New feature? no
Deprecations? no
Issues
License MIT

Understand custom collection objects.

@mtarld mtarld force-pushed the fix/handle-doctrine-collections branch from 106fec9 to b2a7627 Compare April 18, 2024 13:18
@@ -164,7 +162,7 @@ private function getTypeFromNode(TypeNode $node, ?TypeContext $typeContext): Typ
default => $this->resolveCustomIdentifier($node->name, $typeContext),
};

if ($type instanceof ObjectType && \in_array($type->getClassName(), self::COLLECTION_CLASS_NAMES, true)) {
if ($type instanceof ObjectType && (is_a($type->getClassName(), \Traversable::class, true) || is_a($type->getClassName(), \ArrayAccess::class, true))) {
Copy link
Member

Choose a reason for hiding this comment

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

is_subclass_of?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When using is_subclass_of, only subclasses will return true. And I think that it'd be great to be able to understand Traversable as a collection as well. WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Not sure what you mean. is_subclass_of works with interfaces too: https://3v4l.org/bP6JN

Copy link
Member

Choose a reason for hiding this comment

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

I guess what @mtarld means is that is_subclass_of(\Traversable::class, \Traversable::class); returns false: https://3v4l.org/1CbGV

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly, thanks @xabbuh.

Copy link
Contributor

@Korbeil Korbeil left a comment

Choose a reason for hiding this comment

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

👍

@xabbuh xabbuh added the ❄️ Feature Freeze Important Pull Requests to finish before the next Symfony "feature freeze" label Apr 30, 2024
@chalasr
Copy link
Member

chalasr commented Apr 30, 2024

Thank you @mtarld.

@chalasr chalasr merged commit 95e8168 into symfony:7.1 Apr 30, 2024
8 of 10 checks passed
@mtarld mtarld deleted the fix/handle-doctrine-collections branch April 30, 2024 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
❄️ Feature Freeze Important Pull Requests to finish before the next Symfony "feature freeze" Status: Reviewed TypeInfo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants