Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[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.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:3.1fromnicolas-grekas:php71-fix
Oct 4, 2016

Conversation

@nicolas-grekas
Copy link
Member

@nicolas-grekasnicolas-grekas commentedOct 4, 2016
edited
Loading

QA
Branch?3.1
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#19784
LicenseMIT
Doc PR-

@nicolas-grekasnicolas-grekas changed the title[HttpKernel][PropertyInfo] Fix HHVM/PHP7.1 compat[HttpKernel][PropertyInfo] Fix nullable types handlingOct 4, 2016
@nicolas-grekas
Copy link
MemberAuthor

nicolas-grekas commentedOct 4, 2016
edited
Loading

The current code builds on top of a non existing semantical difference betweenisNullable andallowsNull. The engine has only one single state for these concepts. As such, we can't build something reliable on side effects of the new notation for nullable types as introduced in PHP 7.1. See e.g.this comment on the topic.
This PR makes both concepts collapse into a single one.

@nicolas-grekas
Copy link
MemberAuthor

Fixes the current failures on 3.1 for HHVM.
Ping@iltar FYI

publicfunctionsupports(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 error:  Variadic parameter cannot have a default value in php shell code on line 1

Copy link
MemberAuthor

@nicolas-grekasnicolas-grekasOct 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

@nicolas-grekasnicolas-grekas changed the title[HttpKernel][PropertyInfo] Fix nullable types handling[HttpKernel] Fix nullable types handlingOct 4, 2016
@fabpot
Copy link
Member

Thank you@nicolas-grekas.

@fabpotfabpot merged commit0884518 intosymfony:3.1Oct 4, 2016
fabpot added a commit that referenced this pull requestOct 4, 2016
This PR was merged into the 3.1 branch.Discussion----------[HttpKernel] Fix nullable types handling| Q             | A| ------------- | ---| Branch?       | 3.1| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#19784| License       | MIT| Doc PR        | -Commits-------0884518 [HttpKernel] Fix nullable types handling
@nicolas-grekasnicolas-grekas deleted the php71-fix branchOctober 5, 2016 08:16
@fabpotfabpot mentioned this pull requestOct 27, 2016
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

1 more reviewer

@linaorilinaorilinaori approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@nicolas-grekas@fabpot@linaori@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp