[HttpKernel] Fix ArgumentValueResolver for arguments with default value null

chalasr · chalasr · commit 50b4bc7d87b9 · 2017-01-18T14:26:59.000+01:00
diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver.php
@@ -52,7 +52,12 @@ public function getArguments(Request $request, $controller)
         $arguments = array();
 
         foreach ($this->argumentMetadataFactory->createArgumentMetadata($controller) as $metadata) {
+            $resolvedValue = array();
             foreach ($this->argumentValueResolvers as $resolver) {
+                if (array() !== $resolvedValue) {
+                    break;
+                }
+
                 if (!$resolver->supports($request, $metadata)) {
                     continue;
                 }
@@ -64,11 +69,8 @@ public function getArguments(Request $request, $controller)
                 }
 
                 foreach ($resolved as $append) {
-                    $arguments[] = $append;
+                    $resolvedValue[] = $append;
                 }
-
-                // continue to the next controller argument
-                continue 2;
             }
 
             $representative = $controller;
@@ -79,7 +81,11 @@ 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. 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()));
+            if (array() === $resolvedValue && !$metadata->isNullable()) {
+                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()));
+            }
+
+            $arguments = array_merge($arguments, $resolvedValue);
         }
 
         return $arguments;
diff --git a/src/Symfony/Component/HttpKernel/ControllerMetadata/ArgumentMetadata.php b/src/Symfony/Component/HttpKernel/ControllerMetadata/ArgumentMetadata.php
@@ -40,7 +40,7 @@ public function __construct($name, $type, $isVariadic, $hasDefaultValue, $defaul
         $this->isVariadic = $isVariadic;
         $this->hasDefaultValue = $hasDefaultValue;
         $this->defaultValue = $defaultValue;
-        $this->isNullable = $isNullable || null === $type || ($hasDefaultValue && null === $defaultValue);
+        $this->isNullable = $isNullable;
     }
 
     /**
diff --git a/src/Symfony/Component/HttpKernel/ControllerMetadata/ArgumentMetadataFactory.php b/src/Symfony/Component/HttpKernel/ControllerMetadata/ArgumentMetadataFactory.php
@@ -58,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), $param->allowsNull());
+            $arguments[] = new ArgumentMetadata($param->getName(), $this->getType($param), $this->isVariadic($param), $this->hasDefaultValue($param), $this->getDefaultValue($param), $this->isDefaultNull($param));
         }
 
         return $arguments;
@@ -126,4 +126,13 @@ private function getType(\ReflectionParameter $parameter)
             return $info[1];
         }
     }
+
+    private function isDefaultNull(\ReflectionParameter $parameter)
+    {
+        if ($this->supportsParameterType && $this->getType($parameter)) {
+            return $parameter->allowsNull();
+        }
+
+        return $this->hasDefaultValue($parameter) && null === $parameter->getDefaultValue();
+    }
 }
diff --git a/src/Symfony/Component/HttpKernel/Tests/ControllerMetadata/ArgumentMetadataFactoryTest.php b/src/Symfony/Component/HttpKernel/Tests/ControllerMetadata/ArgumentMetadataFactoryTest.php
@@ -121,7 +121,7 @@ public function testNullableTypesSignature()
             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, true),
+            new ArgumentMetadata('mandatory', null, false, false, null, false),
         ), $arguments);
     }
 

Original file line number	Diff line number	Diff line change
`@@ -52,7 +52,12 @@ public function getArguments(Request $request, $controller)`
`52`	`52`	`$arguments = array();`
`53`	`53`
`54`	`54`	`foreach ($this->argumentMetadataFactory->createArgumentMetadata($controller) as $metadata) {`
	`55`	`+ $resolvedValue = array();`
`55`	`56`	`foreach ($this->argumentValueResolvers as $resolver) {`
	`57`	`+ if (array() !== $resolvedValue) {`
	`58`	`+ break;`
	`59`	`+ }`
	`60`	`+`
`56`	`61`	`if (!$resolver->supports($request, $metadata)) {`
`57`	`62`	`continue;`
`58`	`63`	`}`
`@@ -64,11 +69,8 @@ public function getArguments(Request $request, $controller)`
`64`	`69`	`}`
`65`	`70`
`66`	`71`	`foreach ($resolved as $append) {`
`67`		`- $arguments[] = $append;`
	`72`	`+ $resolvedValue[] = $append;`
`68`	`73`	`}`
`69`		`-`
`70`		`- // continue to the next controller argument`
`71`		`- continue 2;`
`72`	`74`	`}`
`73`	`75`
`74`	`76`	`$representative = $controller;`
`@@ -79,7 +81,11 @@ public function getArguments(Request $request, $controller)`
`79`	`81`	`$representative = get_class($representative);`
`80`	`82`	`}`
`81`	`83`
`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()));`
	`84`	`+ if (array() === $resolvedValue && !$metadata->isNullable()) {`
	`85`	`+ 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()));`
	`86`	`+ }`
	`87`	`+`
	`88`	`+ $arguments = array_merge($arguments, $resolvedValue);`
`83`	`89`	`}`
`84`	`90`
`85`	`91`	`return $arguments;`
Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,7 @@ public function __construct($name, $type, $isVariadic, $hasDefaultValue, $defaul`
`40`	`40`	`$this->isVariadic = $isVariadic;`
`41`	`41`	`$this->hasDefaultValue = $hasDefaultValue;`
`42`	`42`	`$this->defaultValue = $defaultValue;`
`43`		`- $this->isNullable = $isNullable \|\| null === $type \|\| ($hasDefaultValue && null === $defaultValue);`
	`43`	`+ $this->isNullable = $isNullable;`
`44`	`44`	`}`
`45`	`45`
`46`	`46`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -58,7 +58,7 @@ public function createArgumentMetadata($controller)`
`58`	`58`	`}`
`59`	`59`
`60`	`60`	`foreach ($reflection->getParameters() as $param) {`
`61`		`- $arguments[] = new ArgumentMetadata($param->getName(), $this->getType($param), $this->isVariadic($param), $this->hasDefaultValue($param), $this->getDefaultValue($param), $param->allowsNull());`
	`61`	`+ $arguments[] = new ArgumentMetadata($param->getName(), $this->getType($param), $this->isVariadic($param), $this->hasDefaultValue($param), $this->getDefaultValue($param), $this->isDefaultNull($param));`
`62`	`62`	`}`
`63`	`63`
`64`	`64`	`return $arguments;`
`@@ -126,4 +126,13 @@ private function getType(\ReflectionParameter $parameter)`
`126`	`126`	`return $info[1];`
`127`	`127`	`}`
`128`	`128`	`}`
	`129`	`+`
	`130`	`+ private function isDefaultNull(\ReflectionParameter $parameter)`
	`131`	`+ {`
	`132`	`+ if ($this->supportsParameterType && $this->getType($parameter)) {`
	`133`	`+ return $parameter->allowsNull();`
	`134`	`+ }`
	`135`	`+`
	`136`	`+ return $this->hasDefaultValue($parameter) && null === $parameter->getDefaultValue();`
	`137`	`+ }`
`129`	`138`	`}`
Original file line number	Diff line number	Diff line change
`@@ -121,7 +121,7 @@ public function testNullableTypesSignature()`
`121`	`121`	`new ArgumentMetadata('foo', 'string', false, false, null, true),`
`122`	`122`	`new ArgumentMetadata('bar', \stdClass::class, false, false, null, true),`
`123`	`123`	`new ArgumentMetadata('baz', 'string', false, true, 'value', true),`
`124`		`- new ArgumentMetadata('mandatory', null, false, false, null, true),`
	`124`	`+ new ArgumentMetadata('mandatory', null, false, false, null, false),`
`125`	`125`	`), $arguments);`
`126`	`126`	`}`
`127`	`127`