8000 [Console] Expose the original input arguments and options and to unparse options by theofidry · Pull Request #57598 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Console] Expose the original input arguments and options and to unparse options #57598

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
wants to merge 6 commits into
base: 7.4
Choose a base branch
from
Open

[Console] Expose the original input arguments and options and to unparse options #57598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a944a32
[Console] Expose the original input arguments and options
theofidry Jun 26, 2024
378f1c6
feedback
theofidry Dec 6, 2024
0a834b9
fix cs
theofidry Dec 6, 2024
d36058e
doc
theofidry Dec 6, 2024
9338ba8
slightly adjust the API
theofidry Apr 18, 2025
b01d0ef
inline the private method
theofidry Apr 19, 2025
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
2 changes: 2 additions & 0 deletions src/Symfony/Component/Console/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ CHANGELOG
* Add support for `LockableTrait` in invokable commands
* Deprecate returning a non-integer value from a `\Closure` function set via `Command::setCode()`
* Mark `#[AsCommand]` attribute as `@final`
* Add `InputInterface::getRawArguments()`, `InputInterface::getRawOptions()` and `InputInterface::unparse()` methods. All are
implemented in the child abstract class `Input`.

7.2
---
Expand Down
58 changes: 58 additions & 0 deletions src/Symfony/Component/Console/Input/Input.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@

use Symfony\Component\Console\Exception\InvalidArgumentException;
use Symfony\Component\Console\Exception\RuntimeException;
use function array_map;
use function sprintf;
use const true;

/**
* Input is the base class for all concrete Input classes.
Expand Down Expand Up @@ -85,6 +88,16 @@
return array_merge($this->definition->getArgumentDefaults(), $this->arguments);
}

/**
* Returns all the given arguments NOT merged with the default values.
*
* @return array<string|bool|int|float|array<string|bool|int|float|null>|null>
*/
public function getRawArguments(): array
{
return $this->arguments;
8000 }

public function getArgument(string $name): mixed
{
if (!$this->definition->hasArgument($name)) {
Expand Down Expand Up @@ -113,6 +126,16 @@
return array_merge($this->definition->getOptionDefaults(), $this->options);
}

/**
* Returns all the given options NOT merged with the default values.
*
* @return array<string|bool|int|float|array<string|bool|int|float|null>|null>
*/
public function getRawOptions(): array
{
return $this->options;
}

public function getOption(string $name): mixed
{
if ($this->definition->hasNegation($name)) {
Expand Down Expand Up @@ -171,4 +194,39 @@
{
return $this->stream;
}

/**
* Returns a stringified representation of the options passed to the command.
* The options must NOT be escaped as otherwise passing them to a `Process` would result in them being escaped twice.
*
* @param string[] $optionNames Names of the options returned. If null, all options are returned. Requested options
* that either do not exist or were not passed (even if the option has a default value)
* will not be part of the method output.
*
* @return list<string>
*/
public function unparse(?array $optionNames = null): array
Comment on lines +198 to +208
Copy link
Member
@GromNaN GromNaN Apr 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This function should be extracted to a dedicated class (SRP) so that it's easier to maintain: we can change it more easily. I don't know for the class name and namespace (we need to think about it). The method signature would be:

function format(RawInputInterface $input, ?array $optionNames = null): array { /* ... */ }

Note: webmozarts/console-parallelization has a class dedicated to serialization, and that's great.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree on principle, and more generally Input and its children are doing too much making them harder to maintain (although thankfully they don't change often, so that alleviates that).

I am not sure it is best to move this now though, because:

  • in essence it is the counter-part of ::parse() which is part of Input, hence you would put two very closely related pieces in different places
  • ::parse() is abstract, hence every children implement its own. So in theory you could have different implementations of ::unparse() to specific to an input implementation.

Copy link
Member
@GromNaN GromNaN Apr 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

According to how you would use this method in webmozarts/console-parallelization, you have a list of option names to exclude. This parameter should be $excludedOptions?

Copy link
Contributor Author
@theofidry theofidry Apr 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

In WebmozartsConsoleParallelization we know the options we do NOT want, hence to get the ones we need to forward we need to juggle with the command definition. It is a specific use case for that feature, but not the only one, for instance you could want those values for specific options.

That's why I thought it would be better in Symfony to have a more re-usable piece of code (which is also simpler) even thought that means a tiny bit more work on my side.

{
$rawOptions = $this->getRawOptions();

$filteredRawOptions = null === $optionNames
? $rawOptions
: array_intersect_key($rawOptions, array_fill_keys($optionNames, ''));

$unparsedOptions = [];

foreach ($filteredRawOptions as $optionName => $parsedOption) {
$option = $this->definition->getOption($optionName);

$unparsedOptions[] = match (true) {
$option->isNegatable() => [sprintf('--%s%s', $parsedOption ? '' : 'no-', $optionName)],
!$option->acceptValue() => [sprintf('--%s', $optionName,)],
$option->isArray() => array_map(static fn($item,) => sprintf('--%s=%s', $optionName, $item), $parsedOption),
default => [sprintf('--%s=%s', $optionName, $parsedOption)],

Check failure on line 225 in src/Symfony/Component/Console/Input/Input.php

View workflow job for this annotation

GitHub Actions / Psalm

InvalidArgument 

src/Symfony/Component/Console/Input/Input.php:225:61: InvalidArgument: Argument 3 of sprintf expects float|int|object{__tostring()}|string, but array<array-key, null|scalar>|null|scalar provided (see https://psalm.dev/004)

Check failure on line 225 in src/Symfony/Component/Console/Input/Input.php

View workflow job for this annotation

GitHub Actions / Psalm

InvalidArgument 

src/Symfony/Component/Console/Input/Input.php:225:61: InvalidArgument: Argument 3 of sprintf expects float|int|object{__tostring()}|string, but array<array-key, null|scalar>|null|scalar provided (see https://psalm.dev/004)
};
}

return array_merge(...$unparsedOptions);
}

}
4 changes: 4 additions & 0 deletions src/Symfony/Component/Console/Input/InputInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
* InputInterface is the interface implemented by all input classes.
*
* @author Fabien Potencier <fabien@symfony.com>
*
* @method getRawArguments(bool $strip = false): array<string|bool|int|float|null|array<string|bool|int|float|null>> Returns all the given arguments NOT merged with the default values.
* @method getRawOptions(): array<string|bool|int|float|array<string|bool|int|float|null>|null> Returns all the given options NOT merged with the default values.
* @method unparse(): list<string> Returns a stringified representation of the options passed to the command.
*/
interface InputInterface
Comment on lines +22 to 26
Copy link
Member
@GromNaN GromNaN Apr 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

To avoid any BC break, we can add a new interface for these new capabilities:

interface RawInputInterface
{
    public function getRawArguments(): array:
    public function getRawOptions(): array;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would that really help? In the end what you receive, because that is what is typed everywhere, is InputInterface. Having a separate interface would bring back something similar to this code:

    public static function getRawOptions(InputInterface $input): array
    {
        return $input instanceof RawInputInterface
            ? $input->getRawOptions()
            : [];
    }

Which... err yes in practice if we make Input implement it it will work 99% of the cases but type-wise leaves it to be desired.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to clarify: the only affected users are users implementing InputInterface without extending Input, which AFAIK (but I can be wrong) is extremely rare, unless you're aware of more cases/usages in the wild?

{
Expand Down
272 changes: 272 additions & 0 deletions src/Symfony/Component/Console/Tests/Input/ArgvInputTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -594,4 +594,276 @@ public static function provideGetRawTokensTrueTests(): iterable
yield [['app/console', '--no-ansi', 'foo:bar', 'foo:bar'], ['foo:bar']];
yield [['app/console', '--no-ansi', 'foo:bar', '--', 'argument'], ['--', 'argument']];
}

/**
* @dataProvider unparseProvider
*/
public function testUnparse(?InputDefinition $inputDefinition, ArgvInput $input, ?array $parsedOptions, array $expected)
{
if (null !== $inputDefinition) {
$input->bind($inputDefinition);
}

$actual = null === $parsedOptions ? $input->unparse() : $input->unparse($parsedOptions);

self::assertSame($expected, $actual);
}

public static function unparseProvider(): iterable
{
yield 'empty input and empty definition' => [
new InputDefinition(),
new ArgvInput([]),
null,
[],
];

yield 'empty input and definition with default values: ignore default values' => [
new InputDefinition([
new InputArgument(
'argWithDefaultValue',
InputArgument::OPTIONAL,
'Argument with a default value',
'arg1DefaultValue',
),
new InputOption(
'optWithDefaultValue',
null,
InputOption::VALUE_REQUIRED,
'Option with a default value',
'opt1DefaultValue',
),
]),
new ArgvInput([]),
null,
[],
];

$completeInputDefinition = new InputDefinition([
new InputArgument(
'requiredArgWithoutDefaultValue',
InputArgument::REQUIRED,
'Argument without a default value',
),
new InputArgument(
'optionalArgWithDefaultValue',
InputArgument::OPTIONAL,
'Argument with a default value',
'argDefaultValue',
),
new InputOption(
'optWithoutDefaultValue',
null,
InputOption::VALUE_REQUIRED,
'Option without a default value',
),
new InputOption(
'optWithDefaultValue',
null,
InputOption::VALUE_REQUIRED,
'Option with a default value',
'optDefaultValue',
),
]);

yield 'arguments & options: returns all passed options but ignore default values' => [
$completeInputDefinition,
new ArgvInput(['argValue', '--optWithoutDefaultValue=optValue']),
null,
['--optWithoutDefaultValue=optValue'],
];

yield 'arguments & options; explicitly pass the default values: the default values are returned' => [
$completeInputDefinition,
new ArgvInput(['argValue', 'argDefaultValue', '--optWithoutDefaultValue=optValue', '--optWithDefaultValue=optDefaultValue']),
null,
[
'--optWithoutDefaultValue=optValue',
'--optWithDefaultValue=optDefaultValue',
],
];

yield 'arguments & options; no input definition: nothing returned' => [
null,
new ArgvInput(['argValue', 'argDefaultValue', '--optWithoutDefaultValue=optValue', '--optWithDefaultValue=optDefaultValue']),
null,
[],
];

yield 'arguments & options; parsing an argument name instead of an option name: that option is ignored' => [
$completeInputDefinition,
new ArgvInput(['argValue']),
['requiredArgWithoutDefaultValue'],
[],
];

yield 'arguments & options; non passed option: it is ignored' => [
$completeInputDefinition,
new ArgvInput(['argValue']),
['optWithDefaultValue'],
[],
];

yield 'arguments & options; requesting a specific option' => [
$completeInputDefinition,
new ArgvInput([
'--optWithoutDefaultValue=optValue1',
'--optWithDefaultValue=optValue2',
]),
['optWithDefaultValue'],
['--optWithDefaultValue=optValue2'],
];

yield 'arguments & options; requesting no options' => [
$completeInputDefinition,
new ArgvInput([
'--optWithoutDefaultValue=optValue1',
'--optWithDefaultValue=optValue2',
]),
[],
[],
];

$createSingleOptionScenario = static fn (
InputOption $option,
array $input,
array $expected,
) => [
new InputDefinition([$option]),
new ArgvInput(['appName', ...$input]),
null,
$expected,
];

yield 'option without value' => $createSingleOptionScenario(
new InputOption(
'opt',
null,
InputOption::VALUE_NONE,
),
['--opt'],
['--opt'],
);

yield 'option without value by shortcut' => $createSingleOptionScenario(
new InputOption(
'opt',
'o',
InputOption::VALUE_NONE,
),
['-o'],
['--opt'],
);

yield 'option with value required' => $createSingleOptionScenario(
new InputOption(
'opt',
null,
InputOption::VALUE_REQUIRED,
),
['--opt=foo'],
['--opt=foo'],
);

yield 'option with non string value (bool)' => $createSingleOptionScenario(
new InputOption(
'opt',
null,
InputOption::VALUE_REQUIRED,
),
['--opt=1'],
['--opt=1'],
);

yield 'option with non string value (int)' => $createSingleOptionScenario(
new InputOption(
'opt',
null,
InputOption::VALUE_REQUIRED,
),
['--opt=20'],
['--opt=20'],
);

yield 'option with non string value (float)' => $createSingleOptionScenario(
new InputOption(
'opt',
null,
InputOption::VALUE_REQUIRED,
),
['--opt=5.3'],
['--opt=5.3'],
);

yield 'option with non string value (array of strings)' => $createSingleOptionScenario(
new InputOption(
'opt',
null,
InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY,
),
['--opt=v1', '--opt=v2', '--opt=v3 --opt=v4'],
['--opt=v1', '--opt=v2', '--opt=v3 --opt=v4'],
);

yield 'negatable option (positive)' => $createSingleOptionScenario(
new InputOption(
'opt',
null,
InputOption::VALUE_NEGATABLE,
),
['--opt'],
['--opt'],
);

yield 'negatable option (negative)' => $createSingleOptionScenario(
new InputOption(
'opt',
null,
InputOption::VALUE_NEGATABLE,
),
['--no-opt'],
['--no-opt'],
);

$createEscapeOptionTokenScenario = static fn (
string $optionValue,
?string $expected,
) => [
new InputDefinition([
new InputOption(
'opt',
null,
InputOption::VALUE_REQUIRED,
),
]),
new ArgvInput(['appName', '--opt='.$optionValue]),
null,
['--opt='.$expected],
];

yield 'escape token; string token' => $createEscapeOptionTokenScenario(
'foo',
'foo',
);

yield 'escape token; escaped string token' => $createEscapeOptionTokenScenario(
'"foo"',
'"foo"',
);

yield 'escape token; escaped string token with both types of quotes' => $createEscapeOptionTokenScenario(
'"o_id in(\'20\')"',
'"o_id in(\'20\')"',
);

yield 'escape token; string token with spaces' => $createEscapeOptionTokenScenario(
'a b c d',
'a b c d',
);

yield 'escape token; string token with line return' => $createEscapeOptionTokenScenario(
"A\nB'C",
"A\nB'C",
);
}
}
Loading
Loading
0