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

Jump to bottom

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

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

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 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

@GromNaN GromNaN Nov 26, 2022

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 Nov 26, 2022
@GromNaN GromNaN requested a review from stof Nov 28, 2022
stof
stof approved these changes Nov 28, 2022
View changes
@nicolas-grekas
Copy link
Member

nicolas-grekas commented Dec 4, 2022

Thank you @GromNaN.

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

@stof stof

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.

None yet
4 participants
@GromNaN @nicolas-grekas @stof @carsonbot