Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Command: Adding argument called 'command' makes other args/options unreadable #54729

Open
cavias opened this issue Apr 25, 2024 · 3 comments · May be fixed by #54795
Open

Command: Adding argument called 'command' makes other args/options unreadable #54729

cavias opened this issue Apr 25, 2024 · 3 comments · May be fixed by #54795

Comments

@cavias
Copy link

cavias commented Apr 25, 2024

Symfony version(s) affected

7.0.6

Description

Adding an argument called "command" to a custom Symfony command seems to conflict with the internal "command" value, making other parameters inaccessible.

There should probably be some filter/fix for this.

How to reproduce

Step 1: Create the following Command (Just add your own namespacing and such):

#[AsCommand(
    name: 'some:command',
    description: 'Some command doing something.',
)]
class SomeCommand extends Command
{
    protected function configure(): void
    {
        $this
            // The issue seems to be adding an argument called "command"
            ->addArgument('command', InputArgument::REQUIRED)
            ->addArgument('action', InputArgument::REQUIRED)
            ->addOption('cavia', 'c', InputOption::VALUE_REQUIRED, 'Cavialiefde');
    }

     public function run(InputInterface $input, OutputInterface $output): int
     {
         // This is fine and returns "some:command"
         $command = $input->getArgument('command');

         // This fails with InvalidArgumentException "The "action" argument does not exist."
         // Even though it is clearly registered.
         $action = $input->getArgument('action');

         // This also fails with InvalidArgumentException "The "cavia" option does not exist."
         // Even though it is clearly registered.
         $cavia = $input->getOption('cavia');

         return Command::SUCCESS;
     }
}

Step 2: Run php bin/console some:command whatever thing --cavia=cavia

Possible Solution

In my opinion I should be able to get the 'command' argument as well as the other registered arguments and options.
Or the 'command' should be illegal as custom argument name and produce an Exception for that.
Or the argument 'command' should be handled separately from the internal commandname.

Additional Context

Note: I don't have a usecase for this. I just came across this while testing.

@flkasper
Copy link

flkasper commented Apr 27, 2024

Reproducible and should concern all versions.
But it's quite easy to explain. command is (currently) reserved for the internal command name.

In my opinion, the solution should be as @cavias has described.

In the simplest case, command is treated as a reserved name and an exception is thrown when an argument is added with it.

Of course, it would be nicer if you could also use command as the argument name.
Renaming the internal command name would be a breaking change and would not completely rule out further collisions.

A separation of argument and command name handling should be possible to a limited extent by taking the position into account during tokenizing (command name is always in the first position).
This would work in ArgvInput, but not for ArrayInput (used, for example, to call other commands).
There, the name of the command/argument/option is used as the key, example['command' => 'foo:bar', 'foo' => 'bar', '--bar' => 'foobar'].

@chalasr
Copy link
Member

chalasr commented Apr 27, 2024

I would consider command a "reserved" argument name.
If we can do something to make it more explicit and ease understanding it, without breaking existing code (even if that code works by chance/luck), let's do it.

But to be honest, such edge cases exist for sure and we definitely don't want to break them, nor ask them to find another way to solve their need especially as it is likely to be more involving than just renaming command to something else in their application (thinking to OSS or proprietary tools using/extending/hacking the console, there's a lot).

That said, if you don't have a real use case I suggest not putting much effort into this and eventually close.

@cavias
Copy link
Author

cavias commented Apr 29, 2024

I would consider command a "reserved" argument name. If we can do something to make it more explicit and ease understanding it, without breaking existing code (even if that code works by chance/luck), let's do it.

But to be honest, such edge cases exist for sure and we definitely don't want to break them, nor ask them to find another way to solve their need especially as it is likely to be more involving than just renaming command to something else in their application (thinking to OSS or proprietary tools using/extending/hacking the console, there's a lot).

That said, if you don't have a real use case I suggest not putting much effort into this and eventually close.

I ran into this because I wanted to have an argument named "command" because of some custom usecase. But I just changed the argument into some other name as the name of the argument is arbitrary.

The main issue I had is that I spend an hour figuring out why my other arguments were failing to register for no obvious reason, only to find this bug.
And I imagine someone else will run into this as well in the future, which is why I recommend this is resolved somehow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants