Fixed the nullable support for php 7.1 and below (2.7, 2.8, 3.0) #19811
Conversation
| * @param callable $controller | ||
| * @param \ReflectionParameter[] $parameters | ||
| * | ||
| * @return mixed[] |
There was a problem hiding this comment.
@return array The values to use when calling the controller
There was a problem hiding this comment.
Sounds good to me, is there any reason array is preferred over mixed[]?
edit: The arguments to use instead of The values to use, WDYT?
|
@dunglas https://3v4l.org/3Vb4g if this is how you would expect it to work, I'd say it's nullable as default as long as it doesn't have a default value. |
|
@iltar looks good: https://3v4l.org/rn5gR |
|
@dunglas the problem is that it's not really nullable anymore. For the old 2.7, 2.8 and 3.0 it's fine, but for the new variant (argument value resolver) this is not a good solution imo. If I ask if something is nullable, I refer to the |
|
@iltar but it will never happen because of https://github.com/symfony/symfony/pull/19811/files#diff-8518ab757bc9acd6d5a874a2e2737502R133 or am I missing something? |
|
@dunglas no, for this case it's fine, I'm referring to https://github.com/symfony/symfony/pull/19784/files#diff-d298199ac90f557ade8fffe47ab3f755R98 While not directly related to this PR, it's a small detail difference. |
| $controller = array(new NullableController(), 'action'); | ||
| $this->assertEquals(array('foo', new \stdClass(), 'value', 'mandatory'), $resolver->getArguments($request, $controller)); | ||
| } | ||
| /** |
There was a problem hiding this comment.
missing blank line before
There was a problem hiding this comment.
is fabbot.io skipping test files?
|
👍 |
|
Thank you @iltar. |
…, 3.0) (iltar) This PR was merged into the 2.7 branch. Discussion ---------- Fixed the nullable support for php 7.1 and below (2.7, 2.8, 3.0) | Q | A | ------------- | --- | Branch? | 2.7, 2.8, 3.0 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #19784 (comment) | License | MIT | Doc PR | ~ Fixes the nullable support for 2.7, 2.8 and 3.0, can probably be partially merged into 3.1 but not 100% sure. /ping @fabpot Commits ------- 9c48756 Fixed the nullable support for php 7.1 and below
Fixes the nullable support for 2.7, 2.8 and 3.0, can probably be partially merged into 3.1 but not 100% sure.
/ping @fabpot