[Console] SymonfyStyle - Check value isset to avoid PHP notice

[leevigraham]

Q	A
Branch?	4.3
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets	Fix #34093
License	MIT
Doc PR	n/a

This PR addresses the issue when a default value is not a valid choice. Currently this would throw a notice which outputs to the console.

This fix is a similar implementation to the QuestionHelper: https://github.com/symfony/symfony/blob/4.4/src/Symfony/Component/Console/Helper/QuestionHelper.php#L63

Example console command and output can be found in the issue: #34093

[chalasr]

Can you please rebase against 3.4 and add a test to prevent regressions?

[leevigraham]

@chalasr Yep… I thought I would throw in a quick PR to get some feedback.

[xabbuh]

I am not really sure how this could happen. Could you please add a test that illustrates why this is necessary?

[Nek-]

@xabbuh did you see #34093 ?

[xabbuh]

I think we need to clarify that the $default argument must be one of the choices. I am not sure that returning it as is when the choice does not exist is a good approach, but IMO we should throw an exception instead.

[chalasr]

I thought this was about supporting using the choice key instead of the value as default, read too fast.
I agree with @xabbuh here, the default must be a valid choice. IIRC it's already enforced by the ChoiceQuestion's default validator, let's prevent the notice with a proper exception.

[leevigraham]

Thanks for the feedback @xabbuh

I am not really sure how this could happen. Could you please add a test that illustrates why this is necessary?

I ran into this issue when I was trying to figure out how to confirm option inputs during the interact function see: #34093

In the original example the flow should probably be that if the user enters an option skip the question but if I did that when I use the value I would need to throw an InvalidArgument Exception in the execute method.

I think we need to clarify that the $default argument must be one of the choices.

That doesn't seem to be enforced in the QuestionHelper https://github.com/symfony/symfony/blob/4.4/src/Symfony/Component/Console/Helper/QuestionHelper.php#L63

I am not sure that returning it as is when the choice does not exist is a good approach, but IMO we should throw an exception instead.

If the user is prevented an invalid default and quickly presses enter the console shows an Invalid value error and prompts the user to select a valid value.

[nicolas-grekas]

I think we need to clarify that the $default argument must be one of the choices.

It looks like nothing requires it in QuestionHelper, so I don't see the need for this enforcement.
Anyway, this would need a test case. @leevigraham can you add one please?

[leevigraham]

It looks like nothing requires it in QuestionHelper

Yep that was my thinking. The implementation mirrors the QuestionHelper. I'll add test cases today or over the weekend.

[nicolas-grekas]

friendly ping @leevigraham

[fabpot]

Thank you @leevigraham.

[leevigraham]

@fabpot @nicolas-grekas Thanks for merging… I was overseas on holidays for a few months.