From 8ab2f32ba2cd60121e55d2162f85afd59ecbf7ed Mon Sep 17 00:00:00 2001 From: Yonel Ceruto Date: Mon, 13 Jan 2025 10:35:32 -0500 Subject: [PATCH] [Console] Invokable command adjustments --- .../FrameworkExtension.php | 2 +- .../Component/Console/Attribute/Argument.php | 22 ++- .../Component/Console/Attribute/Option.php | 52 ++++--- .../Component/Console/Command/Command.php | 23 +-- .../Console/Command/InvokableCommand.php | 32 ++++- .../Tests/Command/InvokableCommandTest.php | 132 ++++++++++++++++-- 6 files changed, 198 insertions(+), 65 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 38d37c7fc1990..deb3d6957e2f0 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -610,7 +610,7 @@ public function load(array $configs, ContainerBuilder $container): void $container->registerForAutoconfiguration(AssetCompilerInterface::class) ->addTag('asset_mapper.compiler'); $container->registerAttributeForAutoconfiguration(AsCommand::class, static function (ChildDefinition $definition, AsCommand $attribute, \ReflectionClass $reflector): void { - $definition->addTag('console.command', ['command' => $attribute->name, 'description' => $attribute->description ?? $reflector->getName()]); + $definition->addTag('console.command', ['command' => $attribute->name, 'description' => $attribute->description]); }); $container->registerForAutoconfiguration(Command::class) ->addTag('console.command'); diff --git a/src/Symfony/Component/Console/Attribute/Argument.php b/src/Symfony/Component/Console/Attribute/Argument.php index c32d45c19aaa5..099d49676e033 100644 --- a/src/Symfony/Component/Console/Attribute/Argument.php +++ b/src/Symfony/Component/Console/Attribute/Argument.php @@ -22,25 +22,23 @@ class Argument { private const ALLOWED_TYPES = ['string', 'bool', 'int', 'float', 'array']; + private string|bool|int|float|array|null $default = null; + private array|\Closure $suggestedValues; private ?int $mode = null; /** * Represents a console command definition. * - * If unset, the `name` and `default` values will be inferred from the parameter definition. + * If unset, the `name` value will be inferred from the parameter definition. * - * @param string|bool|int|float|array|null $default The default value (for InputArgument::OPTIONAL mode only) - * @param array|callable-string(CompletionInput):list $suggestedValues The values used for input completion + * @param array|callable(CompletionInput):list $suggestedValues The values used for input completion */ public function __construct( public string $name = '', public string $description = '', - public string|bool|int|float|array|null $default = null, - public array|string $suggestedValues = [], + array|callable $suggestedValues = [], ) { - if (\is_string($suggestedValues) && !\is_callable($suggestedValues)) { - throw new \TypeError(\sprintf('Argument 4 passed to "%s()" must be either an array or a callable-string.', __METHOD__)); - } + $this->suggestedValues = \is_callable($suggestedValues) ? $suggestedValues(...) : $suggestedValues; } /** @@ -70,13 +68,13 @@ public static function tryFrom(\ReflectionParameter $parameter): ?self $self->name = $name; } - $self->mode = null !== $self->default || $parameter->isDefaultValueAvailable() ? InputArgument::OPTIONAL : InputArgument::REQUIRED; + $self->default = $parameter->isDefaultValueAvailable() ? $parameter->getDefaultValue() : null; + + $self->mode = $parameter->isDefaultValueAvailable() || $parameter->allowsNull() ? InputArgument::OPTIONAL : InputArgument::REQUIRED; if ('array' === $parameterTypeName) { $self->mode |= InputArgument::IS_ARRAY; } - $self->default ??= $parameter->isDefaultValueAvailable() ? $parameter->getDefaultValue() : null; - if (\is_array($self->suggestedValues) && !\is_callable($self->suggestedValues) && 2 === \count($self->suggestedValues) && ($instance = $parameter->getDeclaringFunction()->getClosureThis()) && $instance::class === $self->suggestedValues[0] && \is_callable([$instance, $self->suggestedValues[1]])) { $self->suggestedValues = [$instance, $self->suggestedValues[1]]; } @@ -99,6 +97,6 @@ public function toInputArgument(): InputArgument */ public function resolveValue(InputInterface $input): mixed { - return $input->hasArgument($this->name) ? $input->getArgument($this->name) : null; + return $input->getArgument($this->name); } } diff --git a/src/Symfony/Component/Console/Attribute/Option.php b/src/Symfony/Component/Console/Attribute/Option.php index 98d074b9dd48f..02002a5ad1256 100644 --- a/src/Symfony/Component/Console/Attribute/Option.php +++ b/src/Symfony/Component/Console/Attribute/Option.php @@ -22,28 +22,27 @@ class Option { private const ALLOWED_TYPES = ['string', 'bool', 'int', 'float', 'array']; + private string|bool|int|float|array|null $default = null; + private array|\Closure $suggestedValues; private ?int $mode = null; private string $typeName = ''; + private bool $allowNull = false; /** * Represents a console command --option definition. * - * If unset, the `name` and `default` values will be inferred from the parameter definition. + * If unset, the `name` value will be inferred from the parameter definition. * - * @param array|string|null $shortcut The shortcuts, can be null, a string of shortcuts delimited by | or an array of shortcuts - * @param scalar|array|null $default The default value (must be null for self::VALUE_NONE) - * @param array|callable-string(CompletionInput):list $suggestedValues The values used for input completion + * @param array|string|null $shortcut The shortcuts, can be null, a string of shortcuts delimited by | or an array of shortcuts + * @param array|callable(CompletionInput):list $suggestedValues The values used for input completion */ public function __construct( public string $name = '', public array|string|null $shortcut = null, public string $description = '', - public string|bool|int|float|array|null $default = null, - public array|string $suggestedValues = [], + array|callable $suggestedValues = [], ) { - if (\is_string($suggestedValues) && !\is_callable($suggestedValues)) { - throw new \TypeError(\sprintf('Argument 5 passed to "%s()" must be either an array or a callable-string.', __METHOD__)); - } + $this->suggestedValues = \is_callable($suggestedValues) ? $suggestedValues(...) : $suggestedValues; } /** @@ -69,25 +68,29 @@ public static function tryFrom(\ReflectionParameter $parameter): ?self throw new LogicException(\sprintf('The type "%s" of parameter "$%s" is not supported as a command option. Only "%s" types are allowed.', $self->typeName, $name, implode('", "', self::ALLOWED_TYPES))); } + if (!$parameter->isDefaultValueAvailable()) { + throw new LogicException(\sprintf('The option parameter "$%s" must declare a default value.', $name)); + } + if (!$self->name) { $self->name = $name; } + $self->default = $parameter->getDefaultValue(); + $self->allowNull = $parameter->allowsNull(); + if ('bool' === $self->typeName) { - $self->mode = InputOption::VALUE_NONE | InputOption::VALUE_NEGATABLE; + $self->mode = InputOption::VALUE_NONE; + if (false !== $self->default) { + $self->mode |= InputOption::VALUE_NEGATABLE; + } } else { - $self->mode = null !== $self->default || $parameter->isDefaultValueAvailable() ? InputOption::VALUE_OPTIONAL : InputOption::VALUE_REQUIRED; + $self->mode = $self->allowNull ? InputOption::VALUE_OPTIONAL : InputOption::VALUE_REQUIRED; if ('array' === $self->typeName) { $self->mode |= InputOption::VALUE_IS_ARRAY; } } - if (InputOption::VALUE_NONE === (InputOption::VALUE_NONE & $self->mode)) { - $self->default = null; - } else { - $self->default ??= $parameter->isDefaultValueAvailable() ? $parameter->getDefaultValue() : null; - } - if (\is_array($self->suggestedValues) && !\is_callable($self->suggestedValues) && 2 === \count($self->suggestedValues) && ($instance = $parameter->getDeclaringFunction()->getClosureThis()) && $instance::class === $self->suggestedValues[0] && \is_callable([$instance, $self->suggestedValues[1]])) { $self->suggestedValues = [$instance, $self->suggestedValues[1]]; } @@ -100,9 +103,10 @@ public static function tryFrom(\ReflectionParameter $parameter): ?self */ public function toInputOption(): InputOption { + $default = InputOption::VALUE_NONE === (InputOption::VALUE_NONE & $this->mode) ? null : $this->default; $suggestedValues = \is_callable($this->suggestedValues) ? ($this->suggestedValues)(...) : $this->suggestedValues; - return new InputOption($this->name, $this->shortcut, $this->mode, $this->description, $this->default, $suggestedValues); + return new InputOption($this->name, $this->shortcut, $this->mode, $this->description, $default, $suggestedValues); } /** @@ -110,10 +114,16 @@ public function toInputOption(): InputOption */ public function resolveValue(InputInterface $input): mixed { - if ('bool' === $this->typeName) { - return $input->hasOption($this->name) && null !== $input->getOption($this->name) ? $input->getOption($this->name) : ($this->default ?? false); + $value = $input->getOption($this->name); + + if ('bool' !== $this->typeName) { + return $value; + } + + if ($this->allowNull && null === $value) { + return null; } - return $input->hasOption($this->name) ? $input->getOption($this->name) : null; + return $value ?? $this->default; } } diff --git a/src/Symfony/Component/Console/Command/Command.php b/src/Symfony/Component/Console/Command/Command.php index b9664371abdc4..6c85825d10de5 100644 --- a/src/Symfony/Component/Console/Command/Command.php +++ b/src/Symfony/Component/Console/Command/Command.php @@ -100,6 +100,10 @@ public function __construct(?string $name = null) $this->setDescription(static::getDefaultDescription() ?? ''); } + if (\is_callable($this)) { + $this->code = new InvokableCommand($this, $this(...)); + } + $this->configure(); } @@ -164,9 +168,6 @@ public function isEnabled(): bool */ protected function configure() { - if (!$this->code && \is_callable($this)) { - $this->code = new InvokableCommand($this, $this(...)); - } } /** @@ -312,22 +313,6 @@ public function complete(CompletionInput $input, CompletionSuggestions $suggesti */ public function setCode(callable $code): static { - if ($code instanceof \Closure) { - $r = new \ReflectionFunction($code); - if (null === $r->getClosureThis()) { - set_error_handler(static function () {}); - try { - if ($c = \Closure::bind($code, $this)) { - $code = $c; - } - } finally { - restore_error_handler(); - } - } - } else { - $code = $code(...); - } - $this->code = new InvokableCommand($this, $code, triggerDeprecations: true); return $this; diff --git a/src/Symfony/Component/Console/Command/InvokableCommand.php b/src/Symfony/Component/Console/Command/InvokableCommand.php index ccdbd057985ec..2b3c41501111f 100644 --- a/src/Symfony/Component/Console/Command/InvokableCommand.php +++ b/src/Symfony/Component/Console/Command/InvokableCommand.php @@ -30,14 +30,16 @@ */ class InvokableCommand { + private readonly \Closure $code; private readonly \ReflectionFunction $reflection; public function __construct( private readonly Command $command, - private readonly \Closure $code, + callable $code, private readonly bool $triggerDeprecations = false, ) { - $this->reflection = new \ReflectionFunction($code); + $this->code = $this->getClosure($code); + $this->reflection = new \ReflectionFunction($this->code); } /** @@ -49,7 +51,7 @@ public function __invoke(InputInterface $input, OutputInterface $output): int if (null !== $statusCode && !\is_int($statusCode)) { if ($this->triggerDeprecations) { - trigger_deprecation('symfony/console', '7.3', \sprintf('Returning a non-integer value from the command "%s" is deprecated and will throw an exception in PHP 8.0.', $this->command->getName())); + trigger_deprecation('symfony/console', '7.3', \sprintf('Returning a non-integer value from the command "%s" is deprecated and will throw an exception in Symfony 8.0.', $this->command->getName())); return 0; } @@ -77,6 +79,28 @@ public function configure(InputDefinition $definition): void } } + private function getClosure(callable $code): \Closure + { + if (!$code instanceof \Closure) { + return $code(...); + } + + if (null !== (new \ReflectionFunction($code))->getClosureThis()) { + return $code; + } + + set_error_handler(static function () {}); + try { + if ($c = \Closure::bind($code, $this->command)) { + $code = $c; + } + } finally { + restore_error_handler(); + } + + return $code; + } + private function getParameters(InputInterface $input, OutputInterface $output): array { $parameters = []; @@ -97,7 +121,7 @@ private function getParameters(InputInterface $input, OutputInterface $output): if (!$type instanceof \ReflectionNamedType) { if ($this->triggerDeprecations) { - trigger_deprecation('symfony/console', '7.3', \sprintf('Omitting the type declaration for the parameter "$%s" is deprecated and will throw an exception in PHP 8.0.', $parameter->getName())); + trigger_deprecation('symfony/console', '7.3', \sprintf('Omitting the type declaration for the parameter "$%s" is deprecated and will throw an exception in Symfony 8.0.', $parameter->getName())); continue; } diff --git a/src/Symfony/Component/Console/Tests/Command/InvokableCommandTest.php b/src/Symfony/Component/Console/Tests/Command/InvokableCommandTest.php index e6292c60d8476..3633c865971d5 100644 --- a/src/Symfony/Component/Console/Tests/Command/InvokableCommandTest.php +++ b/src/Symfony/Component/Console/Tests/Command/InvokableCommandTest.php @@ -18,7 +18,10 @@ use Symfony\Component\Console\Completion\CompletionInput; use Symfony\Component\Console\Completion\CompletionSuggestions; use Symfony\Component\Console\Completion\Suggestion; +use Symfony\Component\Console\Exception\InvalidOptionException; use Symfony\Component\Console\Exception\LogicException; +use Symfony\Component\Console\Input\ArrayInput; +use Symfony\Component\Console\Output\NullOutput; class InvokableCommandTest extends TestCase { @@ -27,7 +30,8 @@ public function testCommandInputArgumentDefinition() $command = new Command('foo'); $command->setCode(function ( #[Argument(name: 'first-name')] string $name, - #[Argument(default: '')] string $lastName, + #[Argument] ?string $firstName, + #[Argument] string $lastName = '', #[Argument(description: 'Short argument description')] string $bio = '', #[Argument(suggestedValues: [self::class, 'getSuggestedRoles'])] array $roles = ['ROLE_USER'], ) {}); @@ -36,6 +40,11 @@ public function testCommandInputArgumentDefinition() self::assertSame('first-name', $nameInputArgument->getName()); self::assertTrue($nameInputArgument->isRequired()); + $lastNameInputArgument = $command->getDefinition()->getArgument('firstName'); + self::assertSame('firstName', $lastNameInputArgument->getName()); + self::assertFalse($lastNameInputArgument->isRequired()); + self::assertNull($lastNameInputArgument->getDefault()); + $lastNameInputArgument = $command->getDefinition()->getArgument('lastName'); self::assertSame('lastName', $lastNameInputArgument->getName()); self::assertFalse($lastNameInputArgument->isRequired()); @@ -61,8 +70,8 @@ public function testCommandInputOptionDefinition() { $command = new Command('foo'); $command->setCode(function ( - #[Option(name: 'idle')] int $timeout, - #[Option(default: 'USER_TYPE')] string $type, + #[Option(name: 'idle')] ?int $timeout = null, + #[Option] string $type = 'USER_TYPE', #[Option(shortcut: 'v')] bool $verbose = false, #[Option(description: 'User groups')] array $groups = [], #[Option(suggestedValues: [self::class, 'getSuggestedRoles'])] array $roles = ['ROLE_USER'], @@ -71,30 +80,35 @@ public function testCommandInputOptionDefinition() $timeoutInputOption = $command->getDefinition()->getOption('idle'); self::assertSame('idle', $timeoutInputOption->getName()); self::assertNull($timeoutInputOption->getShortcut()); - self::assertTrue($timeoutInputOption->isValueRequired()); + self::assertTrue($timeoutInputOption->isValueOptional()); + self::assertFalse($timeoutInputOption->isNegatable()); self::assertNull($timeoutInputOption->getDefault()); $typeInputOption = $command->getDefinition()->getOption('type'); self::assertSame('type', $typeInputOption->getName()); - self::assertFalse($typeInputOption->isValueRequired()); + self::assertTrue($typeInputOption->isValueRequired()); + self::assertFalse($typeInputOption->isNegatable()); self::assertSame('USER_TYPE', $typeInputOption->getDefault()); $verboseInputOption = $command->getDefinition()->getOption('verbose'); self::assertSame('verbose', $verboseInputOption->getName()); self::assertSame('v', $verboseInputOption->getShortcut()); self::assertFalse($verboseInputOption->isValueRequired()); - self::assertTrue($verboseInputOption->isNegatable()); - self::assertNull($verboseInputOption->getDefault()); + self::assertFalse($verboseInputOption->isValueOptional()); + self::assertFalse($verboseInputOption->isNegatable()); + self::assertFalse($verboseInputOption->getDefault()); $groupsInputOption = $command->getDefinition()->getOption('groups'); self::assertSame('groups', $groupsInputOption->getName()); self::assertTrue($groupsInputOption->isArray()); self::assertSame('User groups', $groupsInputOption->getDescription()); + self::assertFalse($groupsInputOption->isNegatable()); self::assertSame([], $groupsInputOption->getDefault()); $rolesInputOption = $command->getDefinition()->getOption('roles'); self::assertSame('roles', $rolesInputOption->getName()); - self::assertFalse($rolesInputOption->isValueRequired()); + self::assertTrue($rolesInputOption->isValueRequired()); + self::assertFalse($rolesInputOption->isNegatable()); self::assertTrue($rolesInputOption->isArray()); self::assertSame(['ROLE_USER'], $rolesInputOption->getDefault()); self::assertTrue($rolesInputOption->hasCompletion()); @@ -124,6 +138,108 @@ public function testInvalidOptionType() $command->getDefinition(); } + /** + * @dataProvider provideInputArguments + */ + public function testInputArguments(array $parameters, array $expected) + { + $command = new Command('foo'); + $command->setCode(function ( + #[Argument] string $a, + #[Argument] ?string $b, + #[Argument] string $c = '', + #[Argument] array $d = [], + ) use ($expected) { + $this->assertSame($expected[0], $a); + $this->assertSame($expected[1], $b); + $this->assertSame($expected[2], $c); + $this->assertSame($expected[3], $d); + }); + + $command->run(new ArrayInput($parameters), new NullOutput()); + } + + public static function provideInputArguments(): \Generator + { + yield 'required & defaults' => [['a' => 'x'], ['x', null, '', []]]; + yield 'required & with-value' => [['a' => 'x', 'b' => 'y', 'c' => 'z', 'd' => ['d']], ['x', 'y', 'z', ['d']]]; + yield 'required & without-value' => [['a' => 'x', 'b' => null, 'c' => null, 'd' => null], ['x', null, '', []]]; + } + + /** + * @dataProvider provideBinaryInputOptions + */ + public function testBinaryInputOptions(array $parameters, array $expected) + { + $command = new Command('foo'); + $command->setCode(function ( + #[Option] bool $a = true, + #[Option] bool $b = false, + #[Option] ?bool $c = null, + ) use ($expected) { + $this->assertSame($expected[0], $a); + $this->assertSame($expected[1], $b); + $this->assertSame($expected[2], $c); + }); + + $command->run(new ArrayInput($parameters), new NullOutput()); + } + + public static function provideBinaryInputOptions(): \Generator + { + yield 'defaults' => [[], [true, false, null]]; + yield 'positive' => [['--a' => null, '--b' => null, '--c' => null], [true, true, true]]; + yield 'negative' => [['--no-a' => null, '--no-c' => null], [false, false, false]]; + } + + /** + * @dataProvider provideNonBinaryInputOptions + */ + public function testNonBinaryInputOptions(array $parameters, array $expected) + { + $command = new Command('foo'); + $command->setCode(function ( + #[Option] ?string $a = null, + #[Option] ?string $b = 'b', + #[Option] ?array $c = [], + ) use ($expected) { + $this->assertSame($expected[0], $a); + $this->assertSame($expected[1], $b); + $this->assertSame($expected[2], $c); + }); + + $command->run(new ArrayInput($parameters), new NullOutput()); + } + + public static function provideNonBinaryInputOptions(): \Generator + { + yield 'defaults' => [[], [null, 'b', []]]; + yield 'with-value' => [['--a' => 'x', '--b' => 'y', '--c' => ['z']], ['x', 'y', ['z']]]; + yield 'without-value' => [['--a' => null, '--b' => null, '--c' => null], [null, null, null]]; + } + + public function testInvalidOptionDefinition() + { + $command = new Command('foo'); + $command->setCode(function (#[Option] string $a) {}); + + $this->expectException(LogicException::class); + $this->expectExceptionMessage('The option parameter "$a" must declare a default value.'); + + $command->getDefinition(); + } + + public function testInvalidRequiredValueOptionEvenWithDefault() + { + $command = new Command('foo'); + $command->setCode(function (#[Option] string $a = 'a') {}); + + $this->expectException(InvalidOptionException::class); + $this->expectExceptionMessage('The "--a" option requires a value.'); + + $command->run(new ArrayInput(['--a' => null]), new NullOutput()); + } + public function getSuggestedRoles(CompletionInput $input): array { return ['ROLE_ADMIN', 'ROLE_USER'];