Skip to content

[HttpKernel] In DateTimeValueResolver, convert previously defined date attribute to the expected class #48346

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 4, 2022
Merged

[HttpKernel] In DateTimeValueResolver, convert previously defined date attribute to the expected class #48346

merged 1 commit into from
Dec 4, 2022

Conversation

GromNaN
Copy link
Member

@GromNaN GromNaN commented Nov 26, 2022 •
edited
Loading

Q A
Branch? 6.1
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix https://github.com/symfony/symfony/pull/48098/files#r1013997729
License MIT

Convert an instance of DateTimeInterface to the expected class if the value was predefined in the request attributes.

# in a request listener
$request->attributes->set('date', new \DateTimeImmutable());
class MyController
{
    public function index(\DateTime $date)
    {
        // Use the $date
    }
}
Uncaught TypeError: MyController::index(): Argument #1 ($date) must be of type DateTime, DateTimeImmutable given

@carsonbot carsonbot added this to the 6.1 milestone Nov 26, 2022
GromNaN
GromNaN commented Nov 26, 2022
View reviewed changes
...Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/DateTimeValueResolverTest.php
@@ -20,7 +20,7 @@

class DateTimeValueResolverTest extends TestCase
{
private $defaultTimezone;
private readonly string $defaultTimezone;
Copy link
Member Author

Choose a reason for hiding this comment

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

This class was introduced in 6.1, I took this PR as an opportunity to improve type definition and make data providers static.

@GromNaN GromNaN force-pushed the 6.1-fix-datetime-class branch from 1bbb521 to 22a1567 Compare November 26, 2022 20:48
@GromNaN GromNaN requested a review from stof November 28, 2022 13:03
@nicolas-grekas
Copy link
Member

Thank you @GromNaN.

@nicolas-grekas nicolas-grekas merged commit 972926c into symfony:6.1 Dec 4, 2022
@GromNaN GromNaN deleted the 6.1-fix-datetime-class branch December 4, 2022 16:55
@GromNaN GromNaN mentioned this pull request Dec 4, 2022
Merged
@fabpot fabpot mentioned this pull request Dec 6, 2022
Merged
@fabpot fabpot mentioned this pull request Dec 28, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stof stof stof approved these changes

Assignees
No one assigned
Labels
Bug HttpKernel Status: Reviewed
Projects
None yet
Milestone
6.1
Development

Successfully merging this pull request may close these issues.
4 participants
@GromNaN @nicolas-grekas @stof @carsonbot