CLI: exempt commands with positional arguments from the sibling-subcommand check#19492
Open
dereuromark wants to merge 4 commits into
Open
CLI: exempt commands with positional arguments from the sibling-subcommand check#19492dereuromark wants to merge 4 commits into
dereuromark wants to merge 4 commits into
Conversation
…mmand check
The sibling-subcommand guard added for 5.4 rejects an unknown token after a
command that has registered subcommands, treating it as a mistyped subcommand.
That assumption is wrong for commands which declare their own positional
arguments: there the trailing token is a legitimate argument, not a subcommand.
This broke every Bake command of that shape, e.g. `bake template Articles` is
rejected because `bake template all` is also registered, even though
`TemplateCommand` accepts a `name` argument. Only the command's own option
parser can tell a positional argument apart from a subcommand name.
Resolve the command first, then skip the guard when its parser declares
positional arguments. This restores the behavior the original change already
promised ("commands with declared positional arguments are unaffected") while
keeping the typo protection for parent commands that take no arguments.
ADmad
reviewed
Jun 5, 2026
| protected function commandHasArguments(CommandInterface $command): bool | ||
| { | ||
| if (!$command instanceof BaseCommand) { | ||
| return false; |
Member
There was a problem hiding this comment.
Shouldn't we err on the side of caution and assume the command does have positional arguments? Ensuring a valid command runs is more important than providing a meaningful error for an invalid usage.
Member
Author
There was a problem hiding this comment.
Good call. Inverted the helper to isArgumentlessCommand(): it now only returns true when the parser can be inspected and declares zero arguments. If the parser can't be determined (not a BaseCommand, or getOptionParser() throws) it returns false, so the guard is skipped and the command runs. That also stops a command whose parser throws (e.g. the Bake *AllCommand uninitialized-property case) from being masked by a bogus unknown-subcommand error. Pushed in bf2baa8.
…ined When the option parser cannot be inspected, assume the command may take positional arguments and skip the sibling-subcommand check, so a valid command runs instead of being rejected with an unknown-subcommand error.
ADmad
approved these changes
Jun 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the core half of #19491 (Cluster A).
Problem
The sibling-subcommand guard added in #19425 rejects an unknown token after a command that has registered subcommands, on the assumption that the token is a mistyped subcommand:
That assumption holds for parent commands that take no arguments (
i18n,cache), but it is wrong for commands that declare their own positional arguments. There the trailing token is a legitimate argument, not a subcommand.Bake is the concrete victim:
BakePluginregisters bothbake templateandbake template all(and the same forcontroller,model,fixture).TemplateCommandaccepts anameargument, sobake template Articlesis a valid invocation, yet the guard refuses it becausebake template allshares the prefix. Only--flag-first invocations slipped through, which is why some tests stayed green.The original change already documented the intended exemption ("Commands without siblings, commands with declared positional arguments, and option-like tokens are unaffected") - the positional-argument part was just never implemented.
Fix
A token can only be told apart from a subcommand by the matched command's own option parser. So resolve and instantiate the command first, then skip the guard when its parser declares positional arguments:
commandHasArguments()reads the resolved command's option parser and returns whether it defines any positional arguments. The command is now instantiated once and reused for both the check and execution.Typo protection for argument-less parent commands (
cake i18n nonsense) is preserved.Notes
*AllCommand::buildOptionParser()) is a Bake bug and is fixed separately in the bake plugin.