8000 [HttpKernel] Fix nullable types handling by nicolas-grekas · Pull Request #20152 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[HttpKernel] Fix nullable types handling #20152

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 4, 2016
Merged

[HttpKernel] Fix nullable types handling #20152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/DefaultValueResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ final class DefaultValueResolver implements ArgumentValueResolverInterface
*/
public function supports(Request $request, ArgumentMetadata $argument)
{
return $argument->hasDefaultValue() || $argument->isNullable();
return $argument->hasDefaultValue() || ($argument->isNullable() && !$argument->isVariadic());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think variadic works here either way (php 7.0 and 7.1):

php > function (...$foo = []) {};
PHP Fatal 
10000
error:  Variadic parameter cannot have a default value in php shell code on line 1

Copy link
Member Author
@nicolas-grekas nicolas-grekas Oct 4, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but variadics allow null: function foo(...$bar) {}; => foo(null) is allowed, that's the point here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah the previous case wouldn't even reach here, fair enough

}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/HttpKernel/ControllerMetadata/ArgumentMetadata.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public function __construct($name, $type, $isVariadic, $hasDefaultValue, $defaul
$this->isVariadic = $isVariadic;
$this->hasDefaultValue = $hasDefaultValue;
$this->defaultValue = $defaultValue;
$this->isNullable = (bool) $isNullable;
$this->isNullable = $isNullable || null === $type || ($hasDefaultValue && null === $defaultValue);
}

/**
Expand Down Expand Up @@ -88,7 +88,7 @@ public function hasDefaultValue()
}

/**
* Returns whether the argument is nullable in PHP 7.1 or higher.
* Returns whether the argument accepts null values.
*
* @return bool
*/
Expand Down
30 changes: 11 additions & 19 deletions src/Symfony/Component/HttpKernel/ControllerMetadata/ArgumentMetadataFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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), $this->isNullable($param));
$arguments[] = new ArgumentMetadata($param->getName(), $this->getType($param), $this->isVariadic($param), $this->hasDefaultValue($param), $this->getDefaultValue($param), $param->allowsNull());
}

return $arguments;
Expand Down Expand Up @@ -88,23 +88,6 @@ 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 @@ -127,7 +110,16 @@ private function getDefaultValue(\ReflectionParameter $parameter)
private function getType(\ReflectionParameter $parameter)
{
if ($this->supportsParameterType) {
return $parameter->hasType() ? (string) $parameter->getType() : null;
if (!$type = $parameter->getType()) {
return;
}
$typeName = $type instanceof \ReflectionNamedType ? $type->getName() : $type->__toString();
if ('array' === $typeName && !$type->isBuiltin()) {
// Special case for HHVM with variadics
return;
}

return $typeName;
}

if ($parameter->isArray()) {
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/HttpKernel/Tests/ControllerMetadata/ArgumentMetadataFactoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
new ArgumentMetadata('mandatory', null, false, false, null, true),
), $arguments);
}

Expand Down
0