Use DateTimeImmutable if type is DateTimeInterface#98
Conversation
oddevan
force-pushed
the
deserialize-datetimeinterface
branch
from
777baad to
0fd18b7
Compare
tests/SerdeTestCases.php
Outdated
| } | ||
|
|
||
| #[Test] | ||
| public function datetime_field_can_be_datetimeinterface(): void |
There was a problem hiding this comment.
I think this test would work better as a case in round_trip_examples(). Most tests are using concrete classes rather than anonymous, as... I'm kinda surprised it even works this way. 😄
There was a problem hiding this comment.
Honestly, if I hadn't spun my wheels for so long doing my own (de)serializing code, I wouldn't have known either 😅 I'll get this change in shortly.
There was a problem hiding this comment.
Change is in; LMK if I should squash the commits
|
One comment on the test structure, but otherwise looks good, and nicely polished. Thanks! The PHPStan checks appear unrelated to this issue. They're all in PHP 8.5, so I presume PHPStan is doing some extra checks there or something? Dunno, but not this PR's job to fix. I'll try to look into that when I am able. |
|
I just pushed a fix for the PHPStan issue, as well as a bunch of other style and pedantry cleanups. If you rebase your branch it should be all green now. (Though do still please update the test.) |
oddevan
force-pushed
the
deserialize-datetimeinterface
branch
from
0fd18b7 to
2358f71
Compare
|
Code looks good, and the bots agree. Thanks! |
2 participants
Description
For objects with a type hint of
DateTimeInterface, create aDateTimeImmutableobject on deserialization.Motivation and context
An object with a
DateTimeInterfaceproperty will serialize without trouble. Deserializing that object, however, will cause an error as Serde attempts to instantiate aDateTimeInterfaceobject. Since theDateTimeExporterclass advertises that it can import aDateTimeInterface, this is assumed to be a bug and not intended behavior.How has this been tested?
Added a new test to
SerdeTestCasesthat failed before the fix and passes after the fix. Also tested the fix in a project using Serde and confirmed it there.Types of changes
What types of changes does your code introduce? Put an
xin all the boxes that apply:Checklist:
Go over all the following points, and put an
xin all the boxes that apply.Please, please, please, don't send your pull request until all of the boxes are ticked. Once your pull request is created, it will trigger a build on our continuous integration server to make sure your tests and code style pass.
If you're unsure about any of these, don't hesitate to ask. We're here to help!