Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Form] fix Catchable Fatal Error if choices is not an array#17163
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
xabbuh commentedDec 29, 2015
The choice list normalizer does this: // Harden against NULL values (like in EntityType and ModelType)$choices =null !==$options['choices'] ?$options['choices'] :array(); What about using the same here? |
Gladhon commentedDec 29, 2015
If the choicesNormalizer have to make an array in every case, you're right. |
Gladhon commentedDec 29, 2015
@webmozart@nicolas-grekas Depends on: |
nicolas-grekas commentedDec 29, 2015
👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm not sure but I thinkassertEquals doesn't make a strict comparison. An assertion will be considered valid if you receivefalse whereas you expectnull (and vice-versa). I suggest to use the proper assertion methods instead for both readability and strict comparison.
$this->assertNull($form->getConfig()->getOption('choices'));$this->assertFalse($form->getConfig()->getOption('multiple'));$this->assertFalse($form->getConfig()->getOption('expanded'));There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I will change this as you wish.
GrahamCampbell commentedDec 29, 2015
Surely checking if is null is more correct? If something else is passed that's not null or an array, we want the type error to happen, right? |
Tobion commentedDec 29, 2015
nicolas-grekas commentedDec 30, 2015
Gladhon commentedDec 30, 2015
@nicolas-grekas done |
nicolas-grekas commentedDec 30, 2015
@Gladhon it looks like the phpdoc block wasn't formatted correctly (see fabbot's patch) can you please fix it and also squash all the commits in one? |
nicolas-grekas commentedDec 30, 2015
Thank you@Gladhon. |
…y (Gladhon, nicolas-grekas)This PR was merged into the 2.7 branch.Discussion----------[Form] fix Catchable Fatal Error if choices is not an array| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR |Since 2.7.8 I got a BC-Break ErrorCatchable Fatal Error: Argument 1 passed to Symfony\Component\Form\Extension\Core\Type\ChoiceType::normalizeLegacyChoices() must be of the type array, null givennormalizeLegacyChoices work only with array, so if choices not an array, just don't try to normlize.Commits-------f3c2a9b [Form] fix Catchable Fatal Error if choices is not an array
Since 2.7.8 I got a BC-Break Error
Catchable Fatal Error: Argument 1 passed to Symfony\Component\Form\Extension\Core\Type\ChoiceType::normalizeLegacyChoices() must be of the type array, null given
normalizeLegacyChoices work only with array, so if choices not an array, just don't try to normlize.