[PropertyAccess] Implement DateTime type conversion in PropertyAccess #47776
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
curry684
commented
Oct 3, 2022
| @@ -13,5 +13,4 @@ | |||
|
|
|||
| class ExtendedUninitializedProperty extends UninitializedProperty | |||
| { | |||
|
|
|||
There was a problem hiding this comment.
Note to reviewers - this was an unintended side effect of running CS Fixer, should happen anyway.
|
Hey! I think @PhilETaylor has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
|
Looks like I missed your PR when I submitted #50295 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In short
This PR enables implicit conversion between
\DateTimeand\DateTimeImmutableobjects in the PropertyAccess component, allowing developers to store values of one type where the other was typed.Longer story
Since PHP introduced the
\DateTimeImmutabletype, and related to that the\DateTimeInterface, and afterwards added strict typing, programmers have been banging their heads over converting between 2 datetime types which essentially implement the same thing but are incompatible without helper methods. nelmio/alice#1096 is a notable example of the frustrations this can cause, it makes no sense for a helper library like Faker to implement tons of extra methods just to support that PHP has 2 different ways of storing dates. Similar issues occur with Symfony Forms and Doctrine in certain integrations and combinations.Which got me thinking why the PropertyAccess component, at the core of a lot of these integrations, doesn't simply support automatic conversion between the types. Conversion from a DateTime to a DateTimeImmutable is by definition harmless - the copy in the target is locked. The other way around it may not be entirely safe, as a programmer could expect a modification of the target to be reflected in the source property, however this is not a logical expectation since in that case the source was Immutable. Hence I implemented the implicit conversion both ways.
Coercion was set up to be more extensible for future similar cases, and will only be attempted for method-based and public access. Coercing values for adder/remover makes no sense as it would (nearly) always break the remover functionality. Likewise coercion is specifically not attempted for union/intersection types, only for explicitly named types.