Skip to content

Commit 292f5ae

Browse files
fabpotfabpot
committed
bug #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 #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 | #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
2 parents bd21a08 + 4b465c0 commit 292f5ae

File tree

7 files changed

+122
-13
lines changed

7 files changed

+122
-13
lines changed

‎Controller/ArgumentResolver.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ public function getArguments(Request $request, $controller)
7979
$representative = get_class($representative);
8080
}
8181

82-
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()));
82+
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()));
8383
}
8484

8585
return $arguments;

‎Controller/ArgumentResolver/DefaultValueResolver.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,14 @@ final class DefaultValueResolver implements ArgumentValueResolverInterface
2727
*/
2828
public function supports(Request $request, ArgumentMetadata $argument)
2929
{
30-
return $argument->hasDefaultValue();
30+
return $argument->hasDefaultValue() || $argument->isNullable();
3131
}
3232

3333
/**
3434
* {@inheritdoc}
3535
*/
3636
public function resolve(Request $request, ArgumentMetadata $argument)
3737
{
38-
yield $argument->getDefaultValue();
38+
yield $argument->hasDefaultValue() ? $argument->getDefaultValue() : null;
3939
}
4040
}

‎ControllerMetadata/ArgumentMetadata.php‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,21 +23,24 @@ class ArgumentMetadata
2323
private $isVariadic;
2424
private $hasDefaultValue;
2525
private $defaultValue;
26+
private $isNullable;
2627

2728
/**
2829
* @param string $name
2930
* @param string $type
3031
* @param bool $isVariadic
3132
* @param bool $hasDefaultValue
3233
* @param mixed $defaultValue
34+
* @param bool $isNullable
3335
*/
34-
public function __construct($name, $type, $isVariadic, $hasDefaultValue, $defaultValue)
36+
public function __construct($name, $type, $isVariadic, $hasDefaultValue, $defaultValue, $isNullable = false)
3537
{
3638
$this->name = $name;
3739
$this->type = $type;
3840
$this->isVariadic = $isVariadic;
3941
$this->hasDefaultValue = $hasDefaultValue;
4042
$this->defaultValue = $defaultValue;
43+
$this->isNullable = (bool) $isNullable;
4144
}
4245

4346
/**
@@ -84,6 +87,16 @@ public function hasDefaultValue()
8487
return $this->hasDefaultValue;
8588
}
8689

90+
/**
91+
* Returns whether the argument is nullable in PHP 7.1 or higher.
92+
*
93+
* @return bool
94+
*/
95+
public function isNullable()
96+
{
97+
return $this->isNullable;
98+
}
99+
87100
/**
88101
* Returns the default value of the argument.
89102
*

‎ControllerMetadata/ArgumentMetadataFactory.php‎

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,30 @@
1818
*/
1919
final class ArgumentMetadataFactory implements ArgumentMetadataFactoryInterface
2020
{
21+
/**
22+
* If the ...$arg functionality is available.
23+
*
24+
* Requires at least PHP 5.6.0 or HHVM 3.9.1
25+
*
26+
* @var bool
27+
*/
28+
private $supportsVariadic;
29+
30+
/**
31+
* If the reflection supports the getType() method to resolve types.
32+
*
33+
* Requires at least PHP 7.0.0 or HHVM 3.11.0
34+
*
35+
* @var bool
36+
*/
37+
private $supportsParameterType;
38+
39+
public function __construct()
40+
{
41+
$this->supportsVariadic = method_exists('ReflectionParameter', 'isVariadic');
42+
$this->supportsParameterType = method_exists('ReflectionParameter', 'getType');
43+
}
44+
2145
/**
2246
* {@inheritdoc}
2347
*/
@@ -34,7 +58,7 @@ public function createArgumentMetadata($controller)
3458
}
3559

3660
foreach ($reflection->getParameters() as $param) {
37-
$arguments[] = new ArgumentMetadata($param->getName(), $this->getType($param), $this->isVariadic($param), $this->hasDefaultValue($param), $this->getDefaultValue($param));
61+
$arguments[] = new ArgumentMetadata($param->getName(), $this->getType($param), $this->isVariadic($param), $this->hasDefaultValue($param), $this->getDefaultValue($param), $this->isNullable($param));
3862
}
3963

4064
return $arguments;
@@ -49,7 +73,7 @@ public function createArgumentMetadata($controller)
4973
*/
5074
private function isVariadic(\ReflectionParameter $parameter)
5175
{
52-
return PHP_VERSION_ID >= 50600 && $parameter->isVariadic();
76+
return $this->supportsVariadic && $parameter->isVariadic();
5377
}
5478

5579
/**
@@ -64,6 +88,23 @@ private function hasDefaultValue(\ReflectionParameter $parameter)
6488
return $parameter->isDefaultValueAvailable();
6589
}
6690

91+
/**
92+
* Returns if the argument is allowed to be null but is still mandatory.
93+
*
94+
* @param \ReflectionParameter $parameter
95+
*
96+
* @return bool
97+
*/
98+
private function isNullable(\ReflectionParameter $parameter)
99+
{
100+
if ($this->supportsParameterType) {
101+
return null !== ($type = $parameter->getType()) && $type->allowsNull();
102+
}
103+
104+
// fallback for supported php 5.x versions
105+
return $this->hasDefaultValue($parameter) && null === $this->getDefaultValue($parameter);
106+
}
107+
67108
/**
68109
* Returns a default value if available.
69110
*
@@ -85,7 +126,7 @@ private function getDefaultValue(\ReflectionParameter $parameter)
85126
*/
86127
private function getType(\ReflectionParameter $parameter)
87128
{
88-
if (PHP_VERSION_ID >= 70000) {
129+
if ($this->supportsParameterType) {
89130
return $parameter->hasType() ? (string) $parameter->getType() : null;
90131
}
91132

‎Tests/Controller/ArgumentResolverTest.php‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use Symfony\Component\HttpKernel\Controller\ArgumentValueResolverInterface;
2020
use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadataFactory;
2121
use Symfony\Component\HttpKernel\Tests\Fixtures\Controller\ExtendingRequest;
22+
use Symfony\Component\HttpKernel\Tests\Fixtures\Controller\NullableController;
2223
use Symfony\Component\HttpKernel\Tests\Fixtures\Controller\VariadicController;
2324
use Symfony\Component\HttpFoundation\Request;
2425

@@ -202,6 +203,32 @@ public function testGetArgumentWithoutArray()
202203
$resolver->getArguments($request, $controller);
203204
}
204205

206+
/**
207+
* @requires PHP 7.1
208+
*/
209+
public function testGetNullableArguments()
210+
{
211+
$request = Request::create('/');
212+
$request->attributes->set('foo', 'foo');
213+
$request->attributes->set('bar', new \stdClass());
214+
$request->attributes->set('mandatory', 'mandatory');
215+
$controller = array(new NullableController(), 'action');
216+
217+
$this->assertEquals(array('foo', new \stdClass(), 'value', 'mandatory'), self::$resolver->getArguments($request, $controller));
218+
}
219+
220+
/**
221+
* @requires PHP 7.1
222+
*/
223+
public function testGetNullableArgumentsWithDefaults()
224+
{
225+
$request = Request::create('/');
226+
$request->attributes->set('mandatory', 'mandatory');
227+
$controller = array(new NullableController(), 'action');
228+
229+
$this->assertEquals(array(null, null, 'value', 'mandatory'), self::$resolver->getArguments($request, $controller));
230+
}
231+
205232
public function __invoke($foo, $bar = null)
206233
{
207234
}

‎Tests/ControllerMetadata/ArgumentMetadataFactoryTest.php‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,14 @@
1515
use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata;
1616
use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadataFactory;
1717
use Symfony\Component\HttpKernel\Tests\Fixtures\Controller\BasicTypesController;
18+
use Symfony\Component\HttpKernel\Tests\Fixtures\Controller\NullableController;
1819
use Symfony\Component\HttpKernel\Tests\Fixtures\Controller\VariadicController;
1920

2021
class ArgumentMetadataFactoryTest extends \PHPUnit_Framework_TestCase
2122
{
23+
/**
24+
* @var ArgumentMetadataFactory
25+
*/
2226
private $factory;
2327

2428
protected function setUp()
@@ -42,9 +46,9 @@ public function testSignature2()
4246
$arguments = $this->factory->createArgumentMetadata(array($this, 'signature2'));
4347

4448
$this->assertEquals(array(
45-
new ArgumentMetadata('foo', self::class, false, true, null),
46-
new ArgumentMetadata('bar', __NAMESPACE__.'\FakeClassThatDoesNotExist', false, true, null),
47-
new ArgumentMetadata('baz', 'Fake\ImportedAndFake', false, true, null),
49+
new ArgumentMetadata('foo', self::class, false, true, null, true),
50+
new ArgumentMetadata('bar', __NAMESPACE__.'\FakeClassThatDoesNotExist', false, true, null, true),
51+
new ArgumentMetadata('baz', 'Fake\ImportedAndFake', false, true, null, true),
4852
), $arguments);
4953
}
5054

@@ -74,7 +78,7 @@ public function testSignature5()
7478
$arguments = $this->factory->createArgumentMetadata(array($this, 'signature5'));
7579

7680
$this->assertEquals(array(
77-
new ArgumentMetadata('foo', 'array', false, true, null),
81+
new ArgumentMetadata('foo', 'array', false, true, null, true),
7882
new ArgumentMetadata('bar', null, false, false, null),
7983
), $arguments);
8084
}
@@ -106,6 +110,21 @@ public function testBasicTypesSignature()
106110
), $arguments);
107111
}
108112

113+
/**
114+
* @requires PHP 7.1
115+
*/
116+
public function testNullableTypesSignature()
117+
{
118+
$arguments = $this->factory->createArgumentMetadata(array(new NullableController(), 'action'));
119+
120+
$this->assertEquals(array(
121+
new ArgumentMetadata('foo', 'string', false, false, null, true),
122+
new ArgumentMetadata('bar', \stdClass::class, false, false, null, true),
123+
new ArgumentMetadata('baz', 'string', false, true, 'value', true),
124+
new ArgumentMetadata('mandatory', null, false, false, null),
125+
), $arguments);
126+
}
127+
109128
private function signature1(ArgumentMetadataFactoryTest $foo, array $bar, callable $baz)
110129
{
111130
}

‎Tests/ControllerMetadata/ArgumentMetadataTest.php‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,18 @@
1515

1616
class ArgumentMetadataTest extends \PHPUnit_Framework_TestCase
1717
{
18-
public function testDefaultValueAvailable()
18+
public function testWithBcLayerWithDefault()
1919
{
2020
$argument = new ArgumentMetadata('foo', 'string', false, true, 'default value');
2121

22+
$this->assertFalse($argument->isNullable());
23+
}
24+
25+
public function testDefaultValueAvailable()
26+
{
27+
$argument = new ArgumentMetadata('foo', 'string', false, true, 'default value', true);
28+
29+
$this->assertTrue($argument->isNullable());
2230
$this->assertTrue($argument->hasDefaultValue());
2331
$this->assertSame('default value', $argument->getDefaultValue());
2432
}
@@ -28,8 +36,9 @@ public function testDefaultValueAvailable()
2836
*/
2937
public function testDefaultValueUnavailable()
3038
{
31-
$argument = new ArgumentMetadata('foo', 'string', false, false, null);
39+
$argument = new ArgumentMetadata('foo', 'string', false, false, null, false);
3240

41+
$this->assertFalse($argument->isNullable());
3342
$this->assertFalse($argument->hasDefaultValue());
3443
$argument->getDefaultValue();
3544
}

0 commit comments

Comments
 (0)