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

Missing handling of array-type in ArrayInput::getFirstArgument #52580

Open
niklaswolf opened this issue Nov 14, 2023 · 11 comments
Open

Missing handling of array-type in ArrayInput::getFirstArgument #52580

niklaswolf opened this issue Nov 14, 2023 · 11 comments

Comments

@niklaswolf
Copy link

Symfony version(s) affected

7.0

Description

In contrast to other methods in this class, in ArrayInput::getFirstArgument there is no handling of the case that the value of the first parameter is of type Array.

if ($param && \is_string($param) && '-' === $param[0]) {

If this is the case, there is a type error:

Symfony\Component\Console\Input\ArrayInput::getFirstArgument(): Return value must be of type ?string, array returned

How to reproduce

Initialize ArrayInput with an array-value as the first argument, e.g.

$input = new ArrayInput(['foo' => ['value']]);
$value = $input->getFirstArgument()

Possible Solution

  • change the return type to mixed or allow array as a return type
  • handle arrays another way

Additional Context

No response

@hlallahom
Copy link

hlallahom commented Nov 14, 2023

@niklaswolf it seems that's not a bug
Usage: $input = new ArrayInput(['command' => 'foo:bar', 'foo' => 'bar', '--bar' => 'foobar']);

but It possible to add a check if it's an array or not (\is_array($param))

@niklaswolf
Copy link
Author

If that is not the intended usage, then it is strange why there is handling for the value being an array in every other method of this class, see for example here:

$params[] = \is_array($val) ? implode(' ', array_map($this->escapeToken(...), $val)) : $this->escapeToken($val);

@hlallahom
Copy link

hlallahom commented Nov 14, 2023

The first argument getFirstArgument() allocated for the command name, so it's normal that accept string only
Maybe if you put a real usage that you need, it'll be more clear to understand

@hlallahom
Copy link

hlallahom commented Nov 15, 2023

It's more clear now, Thanks
If i see, maybe problem come from here
https://github.com/shopware/shopware/blob/540d526e61e32c7065d1eae6d74a81f5e1728cb9/src/Core/TestBootstrapper.php#L390C13-L390C13

I think the issue can be resolved in the shopware core

$args = [
                'command' => 'plugin:refresh',
                '--activate' => true,
                '--reinstall' => true,
                'plugins' => [$activePlugin],
            ];

@niklaswolf
Copy link
Author

No this is not the solution, I tried it and got the following error:

Symfony\Component\Console\Exception\InvalidArgumentException:
The "command" argument does not exist.

@hlallahom
Copy link

hlallahom commented Nov 16, 2023

I didn't use shopware before (what i wrote, it was a suggestion that can help you to resolve your problem)
The main idea that you can resolve the issue in shopware core

@hlallahom
Copy link

hlallahom commented Nov 16, 2023

I mean something like that:

protected function configure(): void
    {
        $this->configureCommand(self::LIFECYCLE_METHOD);
        $this->addArgument('plugins', InputArgument::OPTIONAL | InputArgument::IS_ARRAY, 'The plugins list')
            ->addOption('activate', 'a', InputOption::VALUE_NONE, 'Activate plugins after installation.')
            ->addOption('reinstall', null, InputOption::VALUE_NONE, 'Reinstall the plugins');
    }

@maxbeckers
Copy link
Contributor

The docs for that were optimized in https://github.com/symfony/symfony-docs/pull/18741/files, hope this helps.

@niklaswolf
Copy link
Author

Okay I created an issue at Shopware for that.

Nevertheless I stand by my point that it is strange that there is handling for the value of the first argument beeing an array in other methods of this class, but not in getFirstArgument.

But if there is no intention to adjust this, this issue can be closed I think.

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

No branches or pull requests

5 participants