[Console] Additional validation of mode in InputArgument and InputOption
#52055
Conversation
Also automatically set OPTIONAL whenever REQUIRED is not specified.
|
Tests failing due to issues fixed in #52058; I'll finish this after that is merged. |
…Option Also automatically set NONE whenever REQUIRED is not specified.
…tArgument and InputOption
b1e241c to
251b242
Compare
| { | ||
| $this->expectDeprecation('Since symfony/console 6.4: InputOption mode should be either none, required or optional.'); | ||
|
|
||
| new InputOption('foo', 'f', InputOption::VALUE_NONE | InputOption::VALUE_REQUIRED); |
There was a problem hiding this comment.
Have you considered using a data provider to reduce repetition?
| throw new InvalidArgumentException(sprintf('Option mode "%s" is not valid.', $mode)); | ||
| } | ||
|
|
||
| $requirementSpecifier = ($mode & (self::VALUE_NONE | self::VALUE_REQUIRED | self::VALUE_OPTIONAL)); | ||
| if (!(self::VALUE_NONE === $requirementSpecifier || self::VALUE_REQUIRED === $requirementSpecifier || self::VALUE_OPTIONAL === $requirementSpecifier)) { | ||
| trigger_deprecation('symfony/console', '6.4', 'InputOption mode should be either none, required or optional.'); |
There was a problem hiding this comment.
We can add the option name to the deprecation message. That would help to find where the change need to be done when our tools don't provide the backtrace of the trigger.
Co-authored-by: Jérôme Tamarelle <jerome@tamarelle.net>
InputArgument and InputOption
|
What is the status of this PR? If you still want to work on it, please rebase on top of |
Awaiting the results of #52058, which addresses the test failing here. Once a decision is reached there, I'll be rebasing and adjusting this further. |
Currently it is allowed to to combine various combinations of input value requirement specifiers, which are logically exclusive behavior-wise. To prevent mistakes or possibly ambiguous behavior, this PR adds additional validation to ensure there's always exactly one specifier part of the mode after finishing the constructor.
This is done by adding a deprecation message for these cases, which can be replaced with an
InvalidArgumentExceptionin a future major version. The tests added are currently checking for the deprecation message and could be updated to check for this exception instead.