Skip to content

[HttpKernel] Fixed the nullable support for php 7.1 and below #19784

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}
*/
Expand All @@ -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;
Expand All @@ -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();
}

/**
Expand All @@ -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.
*
Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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)
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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);
}

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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)
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand All @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

namespace Symfony\Component\HttpKernel\Tests\Fixtures\Controller;

class NullableController
{
public function action(?string $foo, ?\stdClass $bar, ?string $baz = 'value', $mandatory)
{
}
}