Skip to content

CLI: exempt commands with positional arguments from the sibling-subcommand check #19492

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
dereuromark wants to merge 4 commits into
base: 5.next
Choose a base branch
from
+89 −12
Open

CLI: exempt commands with positional arguments from the sibling-subcommand check #19492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eb74cb9
CLI: exempt commands with positional arguments from the sibling-subco…
dereuromark Jun 5, 2026
2117ce7
Drop redundant string cast in test (rector RemoveConcatAutocastRector)
dereuromark Jun 5, 2026
bf2baa8
Err on the side of running the command when arguments can't be determ…
dereuromark Jun 5, 2026
3c50687
Merge branch '5.next' into cli-subcommand-guard-positional-args
dereuromark Jun 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 43 additions & 12 deletions src/Console/CommandRunner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
use Cake\Routing\Router;
use Cake\Routing\RoutingApplicationInterface;
use Cake\Utility\Inflector;
use Throwable;

/**
* Run CLI commands for the provided application.
Expand Down Expand Up @@ -169,15 +170,28 @@ public function run(array $argv, ?ConsoleIo $io = null): int
$name = 'help';
}

try {
$name = $this->resolveName($commands, $io, $name);
} catch (MissingOptionException $e) {
$io->error($e->getFullMessage());

return CommandInterface::CODE_ERROR;
}

$command = $this->getCommand($io, $commands, $name);

// If the matched command also has sibling subcommands (e.g. `i18n` exists alongside
// `i18n init` / `i18n extract`), an unknown next token is almost always a typo for a
// subcommand. Reject it instead of letting it fall through as a positional argument.
//
// Commands that declare their own positional arguments are exempt: there the next token
// is a legitimate argument (e.g. `bake template Articles` alongside `bake template all`),
// not a mistyped subcommand, and only the command's own parser can tell the two apart.
if (
$name !== null
&& $commands->has($name)
&& isset($argv[0])
isset($argv[0])
&& !str_starts_with($argv[0], '-')
&& $this->hasCommandsWithPrefix($commands, $name)
&& $this->isArgumentlessCommand($command)
) {
$candidate = $name . ' ' . $argv[0];
if (!$commands->has($candidate)) {
Expand All @@ -187,15 +201,6 @@ public function run(array $argv, ?ConsoleIo $io = null): int
}
}

try {
$name = $this->resolveName($commands, $io, $name);
} catch (MissingOptionException $e) {
$io->error($e->getFullMessage());

return CommandInterface::CODE_ERROR;
}

$command = $this->getCommand($io, $commands, $name);
$result = $this->runCommand($command, $argv, $io);

if ($result === null) {
Expand Down Expand Up @@ -366,6 +371,32 @@ protected function hasCommandsWithPrefix(CommandCollection $commands, string $pr
return false;
}

/**
* Check whether a command is known to accept no positional arguments.
*
* Only returns true when the command's option parser can be inspected and declares zero
* arguments. When the parser cannot be determined, it errs on the side of caution and
* returns false, so a potentially valid command is run rather than rejected by the
* sibling-subcommand check as a mistyped subcommand.
*
* @param \Cake\Console\CommandInterface $command The resolved command instance to inspect.
* @return bool
*/
protected function isArgumentlessCommand(CommandInterface $command): bool
{
if (!$command instanceof BaseCommand) {
return false;
Copy link
Copy Markdown
Member

@ADmad ADmad Jun 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member Author

@dereuromark dereuromark Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}

try {
$parser = $command->getOptionParser();
} catch (Throwable) {
return false;
}

return $parser->arguments() === [];
}

/**
* Build the error message shown when a token following a command name doesn't
* match any known subcommand of that command.
Expand Down
46 changes: 46 additions & 0 deletions tests/TestCase/Console/CommandRunnerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
use Cake\Console\CommandInterface;
use Cake\Console\CommandRunner;
use Cake\Console\ConsoleIo;
use Cake\Console\ConsoleOptionParser;
use Cake\Console\TestSuite\StubConsoleOutput;
use Cake\Core\BasePlugin;
use Cake\Core\Configure;
Expand Down Expand Up @@ -210,6 +211,51 @@ public function testRunOptionAfterParentCommandIsNotASubcommand(): void
$this->assertStringNotContainsString('Unknown command', $messages);
}

/**
* Test that a command declaring its own positional arguments is not subject to the
* unknown-subcommand check, even when a sibling subcommand sharing its prefix exists.
*
* For example, given `widget` (which takes a `name` argument), `widget all` is also
* registered: `bin/cake widget Articles` must run the `widget` command with `Articles`
* as its argument, not be rejected as a mistyped `widget` subcommand.
*/
public function testRunPositionalArgumentNotTreatedAsUnknownSubcommand(): void
{
$parent = new class extends Command {
public function buildOptionParser(ConsoleOptionParser $parser): ConsoleOptionParser
{
return $parser->addArgument('name', ['help' => 'A name.', 'required' => false]);
}

public function execute(Arguments $args, ConsoleIo $io): int
{
$io->out('ran widget with ' . $args->getArgument('name'));

return static::CODE_SUCCESS;
}
};
$sibling = new class extends Command {
public function execute(Arguments $args, ConsoleIo $io): int
{
return static::CODE_SUCCESS;
}
};

$output = new StubConsoleOutput();
$app = $this->makeAppWithCommands([
'help' => HelpCommand::class,
'widget' => $parent,
'widget all' => $sibling,
]);
$runner = new CommandRunner($app);
$result = $runner->run(['cake', 'widget', 'Articles'], $this->getMockIo($output));

$this->assertSame(CommandInterface::CODE_SUCCESS, $result);
$messages = implode("\n", $output->messages());
$this->assertStringNotContainsString('Unknown command', $messages);
$this->assertStringContainsString('ran widget with Articles', $messages);
}

/**
* Test using `cake --help` invokes the help command
*/
Expand Down