-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[TypeInfo] Introduce component #52510
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
Conversation
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
Show resolved
Hide resolved
WebMamba
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this! I know that some part of the LiveComponent can be refactor to use this component. We faced some limitation with the PropertyInfo on UnionType for exemple, and it look like this new Component can solve those limitations 😁
a909907 to
df1c8e9
Compare
|
As the original author of PropertyInfo, I agree that we need a new, more powerful, TypeInfo component. PHP evolved a lot on this topic since the original release of PropertyInfo, and PropertyInfo is now too limited. One of my regrets with When/if new features (e.g.: generics) are added to PHP directly, then TypeInfo should deprecate its own equivalent feature and use the builtin one. WDYT? |
|
Extending the PHP classes would mean that we cannot really deprecate things in our API as it would be PHP itself doing changes as well. And it might also create confusion if we create our own subclasses of |
|
@stof we can implement the methods with the names we want in our subclass and deprecate them in favor of the official ones when they will be added to PHP. |
|
@dunglas, if I understood well, you'd love to have a type class hierarchy like the following: classDiagram
ReflectionNamedType <|-- Type
ReflectionUnionType <|-- UnionType
ReflectionIntersectionType <|-- IntersectionType
Type <|-- BuiltinType
Type <|-- ObjectType
Type <|-- TemplateType
Type <|-- CollectionType
Type <|-- GenericType
Type <|-- UnionType
Type <|-- IntersectionType
ObjectType <|-- EnumType
EnumType <|-- BackedEnumType
By doing that way, Plus, the So I see two solutions here:
classDiagram
ReflectionType <|-- Type
Type <|-- BuiltinType
Type <|-- ObjectType
Type <|-- TemplateType
Type <|-- CollectionType
Type <|-- GenericType
Type <|-- UnionType
Type <|-- IntersectionType
ObjectType <|-- EnumType
EnumType <|-- BackedEnumType
But we won't be that precise, and we'll miss out if something is added into
classDiagram
Type <|-- BuiltinType
ReflectionNamedType <|-- BuiltinType
Type <|-- ObjectType
ReflectionNamedType <|-- ObjectType
Type <|-- TemplateType
ReflectionNamedType <|-- TemplateType
Type <|-- CollectionType
ReflectionNamedType <|-- CollectionType
Type <|-- GenericType
ReflectionNamedType <|-- GenericType
Type <|-- UnionType
ReflectionUnionType <|-- UnionType
Type <|-- IntersectionType
ReflectionIntersectionType <|-- IntersectionType
ObjectType <|-- EnumType
EnumType <|-- BackedEnumType
<<interface>> Type
But we have no way to enforce the WDYT? |
|
About using Reflection and make the types classes extend it I'm kinda against it since it will only make the component harder to maintain with no real DX improvement. All the stuff we added are there to make types easier to manage and current Reflection isn't that easy to use. |
df1c8e9 to
9123e93
Compare
|
@Korbeil Reflection is a low level API used only for advanced use cases. Type introspection as well. App developers (unlike framework and library authors) should not have to use them directly. The DX matters less than performance and maintainability for these use cases. And best perf and maintainability is always achieved by relying only on the standard library without needing extra code. Currently, the PHP stdlib doesn't expose all the features we need, but the history of PropertyInfo proved that features are being added, and it's an open goal for the PHP project to support generics and the like. In the future, this component will hopefully not be useful anymore and will be deprecated (same for PropertyInfo), which is good. IMHO TypeInfo and PropertyInfo should try to reduce the amount of work that maintainers will have to do to switch to the PHP stdlib when generics and similar features will be supported. |
|
@dunglas I don't think there is much work needed out of resolvers (which was the same in PropertyInfo extractor) for the maintainers. And if PHP starts to have a better Reflection for types will be welcome but we don't have that yet. Also about performance, extending Reflection would mean having to call Reflection everytime we use a Type class, this is clearly not something that should be done, we want this component to be the most performant possible. |
|
Hi! nice work there :) 2 questions:
|
78f1788 to
94589a6
Compare
81782c9 to
5da3d18
Compare
5da3d18 to
24ccbc5
Compare
Co-authored-by: Baptiste Leduc <[email protected]>
24ccbc5 to
6de7d7d
Compare
|
Thank you @mtarld. |
This PR was merged into the 7.1 branch. Discussion ---------- [PropertyInfo] Deprecate PropertyInfo Type | Q | A | ------------- | --- | Branch? | 7.1 | Bug fix? | no | New feature? | no | Deprecations? | yes | Issues | | License | MIT This PR is a follow-up of #52510. As the TypeInfo's `Type` aims to represent types in the Symfony ecosystem, the PropertyInfo's `Type` needs to be deprecated in favor of the first one. Commits ------- d32e81c [PropertyInfo] Deprecate PropertyInfo Type
This PR was merged into the 7.1 branch. Discussion ---------- [PropertyInfo] Deprecate PropertyInfo Type | Q | A | ------------- | --- | Branch? | 7.1 | Bug fix? | no | New feature? | no | Deprecations? | yes | Issues | | License | MIT This PR is a follow-up of symfony/symfony#52510. As the TypeInfo's `Type` aims to represent types in the Symfony ecosystem, the PropertyInfo's `Type` needs to be deprecated in favor of the first one. Commits ------- d32e81c816 [PropertyInfo] Deprecate PropertyInfo Type
This PR was merged into the 7.1 branch. Discussion ---------- [PropertyInfo] Deprecate PropertyInfo Type | Q | A | ------------- | --- | Branch? | 7.1 | Bug fix? | no | New feature? | no | Deprecations? | yes | Issues | | License | MIT This PR is a follow-up of symfony/symfony#52510. As the TypeInfo's `Type` aims to represent types in the Symfony ecosystem, the PropertyInfo's `Type` needs to be deprecated in favor of the first one. Commits ------- d32e81c816 [PropertyInfo] Deprecate PropertyInfo Type
This PR was merged into the 7.1 branch. Discussion ---------- [PropertyInfo] Deprecate PropertyInfo Type | Q | A | ------------- | --- | Branch? | 7.1 | Bug fix? | no | New feature? | no | Deprecations? | yes | Issues | | License | MIT This PR is a follow-up of symfony/symfony#52510. As the TypeInfo's `Type` aims to represent types in the Symfony ecosystem, the PropertyInfo's `Type` needs to be deprecated in favor of the first one. Commits ------- d32e81c816 [PropertyInfo] Deprecate PropertyInfo Type
This PR was merged into the 7.1 branch. Discussion ---------- [PropertyInfo] Deprecate PropertyInfo Type | Q | A | ------------- | --- | Branch? | 7.1 | Bug fix? | no | New feature? | no | Deprecations? | yes | Issues | | License | MIT This PR is a follow-up of symfony/symfony#52510. As the TypeInfo's `Type` aims to represent types in the Symfony ecosystem, the PropertyInfo's `Type` needs to be deprecated in favor of the first one. Commits ------- d32e81c816 [PropertyInfo] Deprecate PropertyInfo Type
This PR was merged into the 7.1 branch. Discussion ---------- [PropertyInfo] Deprecate PropertyInfo Type | Q | A | ------------- | --- | Branch? | 7.1 | Bug fix? | no | New feature? | no | Deprecations? | yes | Issues | | License | MIT This PR is a follow-up of symfony/symfony#52510. As the TypeInfo's `Type` aims to represent types in the Symfony ecosystem, the PropertyInfo's `Type` needs to be deprecated in favor of the first one. Commits ------- d32e81c816 [PropertyInfo] Deprecate PropertyInfo Type
This PR was squashed before being merged into the 7.1 branch. Discussion ---------- [TypeInfo] Add documentation | Q | A | ------------- | --- | Doc fix? | no | New docs? | yes (symfony/symfony#52510) | Applies to | 7.1 | Fixed tickets | #19497 Commits ------- b6ffad3 [TypeInfo] Add documentation
Introducing the brand new
TypeInfocomponentThis work has been done with @Korbeil
State of the art
Scope of the current
Symfony\Component\PropertyInfo\TypeclassNowadays, when we need to work with types within Symfony, we have to use the
Typeclass of thePropertyInfocomponent.But what if we want to represent the type of a function's return type?
We still can use that
Typeclass, but it won't make much sense as theSymfony\Component\PropertyInfo\Typeis closely related to a property (as the namespace suggests).Plus, when we need to extract types, we must use the
PropertyTypeExtractorInterfaceservice:Therefore, type retrieval in Symfony is limited to properties only.
Symfony\Component\PropertyInfo\Type's conceptual limitationsOn top of that, there is a clear limitation of the current
Typeclass where unions, intersections oreven generics can't be properly described.
The actual workaround is that the
PropertyTypeExtractorInterfaceis returning an array ofType, which can be interpreted as a union type.The
TypeInfocomponentAll these reasons bring us to create the
TypeInfocomponent.The aim here is to address these issues and:
Typedefinition that can handle union, intersections, and generics (and could be even more extended)TypeclassesTo ensure a powerful

Typedefinition, we defined multiple classes:The base
Typeclass is an abstract one, so you'll always need to use one of the classes that extend it.Other types of classes are kinda self-explanatory.
Type resolving
In the
TypeInfocomponent, we added aTypeResolverInterface, and several implementations which allow developers to get aTypefrom many things:ReflectionParameterTypeResolverto resolve a function/method parameter type thanks to reflectionReflectionPropertyTypeResolverto resolve a property type thanks to reflectionReflectionReturnTypeResolverto resolve a function/method return type thanks to reflectionReflectionTypeResolverto resolve aReflectionNamedTypeStringTypeResolverto resolve a type string representation. This can greatly work in combination with PHP documentation.ChainTypeResolverto iterate over resolvers and to return a type as soon as a nested resolver succeeds in resolving.Type Creation
We also improved a lot the DX for the type creation with factories:
Upgrade path
This PR only introduces the
TypeInfocomponent, but another one (which is already ready) will deprecate theSymfony\Component\PropertyInfo\Typein favor ofSymfony\Component\TypeInfo\Type.