bug symfony#19784 [HttpKernel] Fixed the nullable support for php 7.1…

… and below (iltar) This PR was squashed before being merged into the 3.1 branch (closes symfony#19784). Discussion ---------- [HttpKernel] Fixed the nullable support for php 7.1 and below | Q | A | ------------- | --- | Branch? | 3.1 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#19771 | License | MIT | Doc PR | ~ This PR gives support for for the new php 7.1 and will only work in beta3 or higher. I've had to backport the support to 3.1 because I consider this a bug that it won't work, even though 3.1 won't be supported for much longer. ~~The deprecation I've added in the `ArgumentMetadata` should not be triggered as all framework cases create it with the argument. Just for developers who for some reason implemented this manually, I've added the deprecation.~~ ~~*If needed, I can re-open this against 3.2 and leave 3.1 "broken"*~~ On 7.1 lower than beta3 this will happen but shouldn't affect any higher versions (I hope). ``` There was 1 failure: 1) Symfony\Component\HttpKernel\Tests\ControllerMetadata\ArgumentMetadataFactoryTest::testNullableTypesSignature Failed asserting that two arrays are equal. --- Expected +++ Actual @@ @@ Array ( 0 => Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata Object (...) 1 => Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata Object ( 'name' => 'bar' - 'type' => 'stdClass' + 'type' => 'Symfony\Component\HttpKernel\Tests\Fixtures\Controller\stdClass' 'isVariadic' => false 'hasDefaultValue' => false 'defaultValue' => null 'isNullable' => true ) 2 => Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata Object (...) ) /home/ivanderberg/projects/symfony/src/Symfony/Component/HttpKernel/Tests/ControllerMetadata/ArgumentMetadataFactoryTest.php:123 ``` Commits ------- 4a1ab6d [HttpKernel] Fixed the nullable support for php 7.1 and below
eriksencosta · Sep 14, 2016 · 3a4eaf9 · 3a4eaf9
2 parents 0d5dbb7 + 4a1ab6d
commit 3a4eaf9
Show file tree

Hide file tree

Showing 7 changed files with 122 additions and 13 deletions.
diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver.php
@@ -79,7 +79,7 @@ public function getArguments(Request $request, $controller)
                 $representative = get_class($representative);
             }
 
-            throw new \RuntimeException(sprintf('Controller "%s" requires that you provide a value for the "$%s" argument (because there is no default value or because there is a non optional argument after this one).', $representative, $metadata->getName()));
+            throw new \RuntimeException(sprintf('Controller "%s" requires that you provide a value for the "$%s" argument. Either the argument is nullable and no null value has been provided, no default value has been provided or because there is a non optional argument after this one.', $representative, $metadata->getName()));
         }
 
         return $arguments;

diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/DefaultValueResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/DefaultValueResolver.php
@@ -27,14 +27,14 @@ final class DefaultValueResolver implements ArgumentValueResolverInterface
      */
     public function supports(Request $request, ArgumentMetadata $argument)
     {
-        return $argument->hasDefaultValue();
+        return $argument->hasDefaultValue() || $argument->isNullable();
     }
 
     /**
      * {@inheritdoc}
      */
     public function resolve(Request $request, ArgumentMetadata $argument)
     {
-        yield $argument->getDefaultValue();
+        yield $argument->hasDefaultValue() ? $argument->getDefaultValue() : null;
     }
 }
diff --git a/src/Symfony/Component/HttpKernel/ControllerMetadata/ArgumentMetadata.php b/src/Symfony/Component/HttpKernel/ControllerMetadata/ArgumentMetadata.php
@@ -23,21 +23,24 @@ class ArgumentMetadata
     private $isVariadic;
     private $hasDefaultValue;
     private $defaultValue;
+    private $isNullable;
 
     /**
      * @param string $name
      * @param string $type
      * @param bool   $isVariadic
      * @param bool   $hasDefaultValue
      * @param mixed  $defaultValue
+     * @param bool   $isNullable
      */
-    public function __construct($name, $type, $isVariadic, $hasDefaultValue, $defaultValue)
+    public function __construct($name, $type, $isVariadic, $hasDefaultValue, $defaultValue, $isNullable = false)
     {
         $this->name = $name;
         $this->type = $type;
         $this->isVariadic = $isVariadic;
         $this->hasDefaultValue = $hasDefaultValue;
         $this->defaultValue = $defaultValue;
+        $this->isNullable = (bool) $isNullable;
     }
 
     /**
@@ -84,6 +87,16 @@ public function hasDefaultValue()
         return $this->hasDefaultValue;
     }
 
+    /**
+     * Returns whether the argument is nullable in PHP 7.1 or higher.
+     *
+     * @return bool
+     */
+    public function isNullable()
+    {
+        return $this->isNullable;
+    }
+
     /**
      * Returns the default value of the argument.
      *

diff --git a/src/Symfony/Component/HttpKernel/ControllerMetadata/ArgumentMetadataFactory.php b/src/Symfony/Component/HttpKernel/ControllerMetadata/ArgumentMetadataFactory.php
@@ -18,6 +18,30 @@
  */
 final class ArgumentMetadataFactory implements ArgumentMetadataFactoryInterface
 {
+    /**
+     * If the ...$arg functionality is available.
+     *
+     * Requires at least PHP 5.6.0 or HHVM 3.9.1
+     *
+     * @var bool
+     */
+    private $supportsVariadic;
+
+    /**
+     * If the reflection supports the getType() method to resolve types.
+     *
+     * Requires at least PHP 7.0.0 or HHVM 3.11.0
+     *
+     * @var bool
+     */
+    private $supportsParameterType;
+
+    public function __construct()
+    {
+        $this->supportsVariadic = method_exists('ReflectionParameter', 'isVariadic');
+        $this->supportsParameterType = method_exists('ReflectionParameter', 'getType');
+    }
+
     /**
      * {@inheritdoc}
      */
@@ -34,7 +58,7 @@ public function createArgumentMetadata($controller)
         }
 
         foreach ($reflection->getParameters() as $param) {
-            $arguments[] = new ArgumentMetadata($param->getName(), $this->getType($param), $this->isVariadic($param), $this->hasDefaultValue($param), $this->getDefaultValue($param));
+            $arguments[] = new ArgumentMetadata($param->getName(), $this->getType($param), $this->isVariadic($param), $this->hasDefaultValue($param), $this->getDefaultValue($param), $this->isNullable($param));
         }
 
         return $arguments;
@@ -49,7 +73,7 @@ public function createArgumentMetadata($controller)
      */
     private function isVariadic(\ReflectionParameter $parameter)
     {
-        return PHP_VERSION_ID >= 50600 && $parameter->isVariadic();
+        return $this->supportsVariadic && $parameter->isVariadic();
     }
 
     /**
@@ -64,6 +88,23 @@ private function hasDefaultValue(\ReflectionParameter $parameter)
         return $parameter->isDefaultValueAvailable();
     }
 
+    /**
+     * Returns if the argument is allowed to be null but is still mandatory.
+     *
+     * @param \ReflectionParameter $parameter
+     *
+     * @return bool
+     */
+    private function isNullable(\ReflectionParameter $parameter)
+    {
+        if ($this->supportsParameterType) {
+            return null !== ($type = $parameter->getType()) && $type->allowsNull();
+        }
+
+        // fallback for supported php 5.x versions
+        return $this->hasDefaultValue($parameter) && null === $this->getDefaultValue($parameter);
+    }
+
     /**
      * Returns a default value if available.
      *
@@ -85,7 +126,7 @@ private function getDefaultValue(\ReflectionParameter $parameter)
      */
     private function getType(\ReflectionParameter $parameter)
     {
-        if (PHP_VERSION_ID >= 70000) {
+        if ($this->supportsParameterType) {
             return $parameter->hasType() ? (string) $parameter->getType() : null;
         }
 

diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverTest.php
@@ -19,6 +19,7 @@
 use Symfony\Component\HttpKernel\Controller\ArgumentValueResolverInterface;
 use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadataFactory;
 use Symfony\Component\HttpKernel\Tests\Fixtures\Controller\ExtendingRequest;
+use Symfony\Component\HttpKernel\Tests\Fixtures\Controller\NullableController;
 use Symfony\Component\HttpKernel\Tests\Fixtures\Controller\VariadicController;
 use Symfony\Component\HttpFoundation\Request;
 
@@ -202,6 +203,32 @@ public function testGetArgumentWithoutArray()
         $resolver->getArguments($request, $controller);
     }
 
+    /**
+     * @requires PHP 7.1
+     */
+    public function testGetNullableArguments()
+    {
+        $request = Request::create('/');
+        $request->attributes->set('foo', 'foo');
+        $request->attributes->set('bar', new \stdClass());
+        $request->attributes->set('mandatory', 'mandatory');
+        $controller = array(new NullableController(), 'action');
+
+        $this->assertEquals(array('foo', new \stdClass(), 'value', 'mandatory'), self::$resolver->getArguments($request, $controller));
+    }
+
+    /**
+     * @requires PHP 7.1
+     */
+    public function testGetNullableArgumentsWithDefaults()
+    {
+        $request = Request::create('/');
+        $request->attributes->set('mandatory', 'mandatory');
+        $controller = array(new NullableController(), 'action');
+
+        $this->assertEquals(array(null, null, 'value', 'mandatory'), self::$resolver->getArguments($request, $controller));
+    }
+
     public function __invoke($foo, $bar = null)
     {
     }

diff --git a/src/Symfony/Component/HttpKernel/Tests/ControllerMetadata/ArgumentMetadataFactoryTest.php b/src/Symfony/Component/HttpKernel/Tests/ControllerMetadata/ArgumentMetadataFactoryTest.php
@@ -15,10 +15,14 @@
 use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata;
 use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadataFactory;
 use Symfony\Component\HttpKernel\Tests\Fixtures\Controller\BasicTypesController;
+use Symfony\Component\HttpKernel\Tests\Fixtures\Controller\NullableController;
 use Symfony\Component\HttpKernel\Tests\Fixtures\Controller\VariadicController;
 
 class ArgumentMetadataFactoryTest extends \PHPUnit_Framework_TestCase
 {
+    /**
+     * @var ArgumentMetadataFactory
+     */
     private $factory;
 
     protected function setUp()
@@ -42,9 +46,9 @@ public function testSignature2()
         $arguments = $this->factory->createArgumentMetadata(array($this, 'signature2'));
 
         $this->assertEquals(array(
-            new ArgumentMetadata('foo', self::class, false, true, null),
-            new ArgumentMetadata('bar', __NAMESPACE__.'\FakeClassThatDoesNotExist', false, true, null),
-            new ArgumentMetadata('baz', 'Fake\ImportedAndFake', false, true, null),
+            new ArgumentMetadata('foo', self::class, false, true, null, true),
+            new ArgumentMetadata('bar', __NAMESPACE__.'\FakeClassThatDoesNotExist', false, true, null, true),
+            new ArgumentMetadata('baz', 'Fake\ImportedAndFake', false, true, null, true),
         ), $arguments);
     }
 
@@ -74,7 +78,7 @@ public function testSignature5()
         $arguments = $this->factory->createArgumentMetadata(array($this, 'signature5'));
 
         $this->assertEquals(array(
-            new ArgumentMetadata('foo', 'array', false, true, null),
+            new ArgumentMetadata('foo', 'array', false, true, null, true),
             new ArgumentMetadata('bar', null, false, false, null),
         ), $arguments);
     }
@@ -106,6 +110,21 @@ public function testBasicTypesSignature()
         ), $arguments);
     }
 
+    /**
+     * @requires PHP 7.1
+     */
+    public function testNullableTypesSignature()
+    {
+        $arguments = $this->factory->createArgumentMetadata(array(new NullableController(), 'action'));
+
+        $this->assertEquals(array(
+            new ArgumentMetadata('foo', 'string', false, false, null, true),
+            new ArgumentMetadata('bar', \stdClass::class, false, false, null, true),
+            new ArgumentMetadata('baz', 'string', false, true, 'value', true),
+            new ArgumentMetadata('mandatory', null, false, false, null),
+        ), $arguments);
+    }
+
     private function signature1(ArgumentMetadataFactoryTest $foo, array $bar, callable $baz)
     {
     }

diff --git a/src/Symfony/Component/HttpKernel/Tests/ControllerMetadata/ArgumentMetadataTest.php b/src/Symfony/Component/HttpKernel/Tests/ControllerMetadata/ArgumentMetadataTest.php
@@ -15,10 +15,18 @@
 
 class ArgumentMetadataTest extends \PHPUnit_Framework_TestCase
 {
-    public function testDefaultValueAvailable()
+    public function testWithBcLayerWithDefault()
     {
         $argument = new ArgumentMetadata('foo', 'string', false, true, 'default value');
 
+        $this->assertFalse($argument->isNullable());
+    }
+
+    public function testDefaultValueAvailable()
+    {
+        $argument = new ArgumentMetadata('foo', 'string', false, true, 'default value', true);
+
+        $this->assertTrue($argument->isNullable());
         $this->assertTrue($argument->hasDefaultValue());
         $this->assertSame('default value', $argument->getDefaultValue());
     }
@@ -28,8 +36,9 @@ public function testDefaultValueAvailable()
      */
     public function testDefaultValueUnavailable()
     {
-        $argument = new ArgumentMetadata('foo', 'string', false, false, null);
+        $argument = new ArgumentMetadata('foo', 'string', false, false, null, false);
 
+        $this->assertFalse($argument->isNullable());
         $this->assertFalse($argument->hasDefaultValue());
         $argument->getDefaultValue();
     }