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

nicolas-grekas · nicolas-grekas · commit 972926c32c77 · 2022-12-04T17:18:43.000+01:00
This PR was merged into the 6.1 branch. Discussion ---------- [HttpKernel] In DateTimeValueResolver, convert previously defined date attribute to the expected class | 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. ```php # in a request listener $request->attributes->set('date', new \DateTimeImmutable()); ``` ```php class MyController { public function index(\DateTime $date) { // Use the $date } } ``` ``` Uncaught TypeError: MyController::index(): Argument #1 ($date) must be of type DateTime, DateTimeImmutable given ``` Commits ------- 22a1567 Convert previously defined date attribute to the expected class
diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/DateTimeValueResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/DateTimeValueResolver.php
@@ -39,9 +39,10 @@ public function supports(Request $request, ArgumentMetadata $argument): bool
     public function resolve(Request $request, ArgumentMetadata $argument): iterable
     {
         $value = $request->attributes->get($argument->getName());
+        $class = \DateTimeInterface::class === $argument->getType() ? \DateTimeImmutable::class : $argument->getType();
 
         if ($value instanceof \DateTimeInterface) {
-            yield $value;
+            yield $value instanceof $class ? $value : $class::createFromInterface($value);
 
             return;
         }
@@ -52,7 +53,6 @@ public function resolve(Request $request, ArgumentMetadata $argument): iterable
             return;
         }
 
-        $class = \DateTimeInterface::class === $argument->getType() ? \DateTimeImmutable::class : $argument->getType();
         $format = null;
 
         if ($attributes = $argument->getAttributes(MapDateTime::class, ArgumentMetadata::IS_INSTANCEOF)) {
diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/DateTimeValueResolverTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/DateTimeValueResolverTest.php
@@ -20,7 +20,7 @@
 
 class DateTimeValueResolverTest extends TestCase
 {
-    private $defaultTimezone;
+    private readonly string $defaultTimezone;
 
     protected function setUp(): void
     {
@@ -32,13 +32,20 @@ protected function tearDown(): void
         date_default_timezone_set($this->defaultTimezone);
     }
 
-    public function getTimeZones()
+    public static function getTimeZones()
     {
         yield ['UTC'];
         yield ['Etc/GMT+9'];
         yield ['Etc/GMT-14'];
     }
 
+    public static function getClasses()
+    {
+        yield [\DateTimeInterface::class];
+        yield [\DateTime::class];
+        yield [FooDateTime::class];
+    }
+
     public function testSupports()
     {
         $resolver = new DateTimeValueResolver();
@@ -133,19 +140,25 @@ public function testNow(string $timezone)
         $this->assertSame($timezone, $results[0]->getTimezone()->getName(), 'Default timezone');
     }
 
-    public function testPreviouslyConvertedAttribute()
+    /**
+     * @param class-string<\DateTimeInterface> $class
+     *
+     * @dataProvider getClasses
+     */
+    public function testPreviouslyConvertedAttribute(string $class)
     {
         $resolver = new DateTimeValueResolver();
 
-        $argument = new ArgumentMetadata('dummy', \DateTime::class, false, false, null, true);
+        $argument = new ArgumentMetadata('dummy', $class, false, false, null, true);
         $request = self::requestWithAttributes(['dummy' => $datetime = new \DateTime()]);
 
         /** @var \Generator $results */
         $results = $resolver->resolve($request, $argument);
         $results = iterator_to_array($results);
 
         $this->assertCount(1, $results);
-        $this->assertSame($datetime, $results[0]);
+        $this->assertEquals($datetime, $results[0], 'The value is the same, but the class can be modified.');
+        $this->assertInstanceOf($class, $results[0]);
     }
 
     public function testCustomClass()
@@ -209,7 +222,7 @@ public function testWithFormat(string $timezone)
         $this->assertEquals('2016-09-08 12:34:56', $results[0]->format('Y-m-d H:i:s'));
     }
 
-    public function provideInvalidDates()
+    public static function provideInvalidDates()
     {
         return [
             'invalid date' => [

Original file line number	Diff line number	Diff line change
`@@ -39,9 +39,10 @@ public function supports(Request $request, ArgumentMetadata $argument): bool`
`39`	`39`	`public function resolve(Request $request, ArgumentMetadata $argument): iterable`
`40`	`40`	`{`
`41`	`41`	`$value = $request->attributes->get($argument->getName());`
	`42`	`+ $class = \DateTimeInterface::class === $argument->getType() ? \DateTimeImmutable::class : $argument->getType();`
`42`	`43`
`43`	`44`	`if ($value instanceof \DateTimeInterface) {`
`44`		`- yield $value;`
	`45`	`+ yield $value instanceof $class ? $value : $class::createFromInterface($value);`
`45`	`46`
`46`	`47`	`return;`
`47`	`48`	`}`
`@@ -52,7 +53,6 @@ public function resolve(Request $request, ArgumentMetadata $argument): iterable`
`52`	`53`	`return;`
`53`	`54`	`}`
`54`	`55`
`55`		`- $class = \DateTimeInterface::class === $argument->getType() ? \DateTimeImmutable::class : $argument->getType();`
`56`	`56`	`$format = null;`
`57`	`57`
`58`	`58`	`if ($attributes = $argument->getAttributes(MapDateTime::class, ArgumentMetadata::IS_INSTANCEOF)) {`
Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@`
`20`	`20`
`21`	`21`	`class DateTimeValueResolverTest extends TestCase`
`22`	`22`	`{`
`23`		`- private $defaultTimezone;`
	`23`	`+ private readonly string $defaultTimezone;`
`24`	`24`
`25`	`25`	`protected function setUp(): void`
`26`	`26`	`{`
`@@ -32,13 +32,20 @@ protected function tearDown(): void`
`32`	`32`	`date_default_timezone_set($this->defaultTimezone);`
`33`	`33`	`}`
`34`	`34`
`35`		`- public function getTimeZones()`
	`35`	`+ public static function getTimeZones()`
`36`	`36`	`{`
`37`	`37`	`yield ['UTC'];`
`38`	`38`	`yield ['Etc/GMT+9'];`
`39`	`39`	`yield ['Etc/GMT-14'];`
`40`	`40`	`}`
`41`	`41`
	`42`	`+ public static function getClasses()`
	`43`	`+ {`
	`44`	`+ yield [\DateTimeInterface::class];`
	`45`	`+ yield [\DateTime::class];`
	`46`	`+ yield [FooDateTime::class];`
	`47`	`+ }`
	`48`	`+`
`42`	`49`	`public function testSupports()`
`43`	`50`	`{`
`44`	`51`	`$resolver = new DateTimeValueResolver();`
`@@ -133,19 +140,25 @@ public function testNow(string $timezone)`
`133`	`140`	`$this->assertSame($timezone, $results[0]->getTimezone()->getName(), 'Default timezone');`
`134`	`141`	`}`
`135`	`142`
`136`		`- public function testPreviouslyConvertedAttribute()`
	`143`	`+ /**`
	`144`	`+ * @param class-string<\DateTimeInterface> $class`
	`145`	`+ *`
	`146`	`+ * @dataProvider getClasses`
	`147`	`+ */`
	`148`	`+ public function testPreviouslyConvertedAttribute(string $class)`
`137`	`149`	`{`
`138`	`150`	`$resolver = new DateTimeValueResolver();`
`139`	`151`
`140`		`- $argument = new ArgumentMetadata('dummy', \DateTime::class, false, false, null, true);`
	`152`	`+ $argument = new ArgumentMetadata('dummy', $class, false, false, null, true);`
`141`	`153`	`$request = self::requestWithAttributes(['dummy' => $datetime = new \DateTime()]);`
`142`	`154`
`143`	`155`	`/** @var \Generator $results */`
`144`	`156`	`$results = $resolver->resolve($request, $argument);`
`145`	`157`	`$results = iterator_to_array($results);`
`146`	`158`
`147`	`159`	`$this->assertCount(1, $results);`
`148`		`- $this->assertSame($datetime, $results[0]);`
	`160`	`+ $this->assertEquals($datetime, $results[0], 'The value is the same, but the class can be modified.');`
	`161`	`+ $this->assertInstanceOf($class, $results[0]);`
`149`	`162`	`}`
`150`	`163`
`151`	`164`	`public function testCustomClass()`
`@@ -209,7 +222,7 @@ public function testWithFormat(string $timezone)`
`209`	`222`	`$this->assertEquals('2016-09-08 12:34:56', $results[0]->format('Y-m-d H:i:s'));`
`210`	`223`	`}`
`211`	`224`
`212`		`- public function provideInvalidDates()`
	`225`	`+ public static function provideInvalidDates()`
`213`	`226`	`{`
`214`	`227`	`return [`
`215`	`228`	`'invalid date' => [`