Skip to content

[Console] Add support for method-based commands#62567

Merged
chalasr merged 1 commit intosymfony:8.1from
yceruto:method_based_command
Dec 25, 2025
Merged

[Console] Add support for method-based commands#62567
chalasr merged 1 commit intosymfony:8.1from
yceruto:method_based_command

Conversation

@yceruto
Copy link
Copy Markdown
Member

@yceruto yceruto commented Nov 29, 2025 •
edited
Loading

Q A
Branch? 8.1
Bug fix? no
New feature? yes
Deprecations? no
Issues -
License MIT

Same as with Controllers and Messenger Handlers, this would allow defining multiple commands in the same class:

class MethodBasedCommand
{
    public function __construct(
        // common dependencies ...
    ) {
    }

    #[AsCommand('app:cmd1')]
    public function cmd1(): int
    {
        // ...
    }

    #[AsCommand('app:cmd2')]
    public function cmd2(): int
    {
        // ...
    }
}

Asked by @kbond in #59340 (review)

Component standalone usage:

$instance = new MethodBasedCommand();

$application = new Application();
$application->addCommand($instance->cmd1(...)); // or [$instance, 'cmd1']
$application->run($input, $output);

$application = new Application();
$application->addCommand($instance->cmd2(...));
$application->run($input, $output);

Testing:

$instance = new MethodBasedCommand();

$tester = new CommandTester($instance->cmd1(...)); // or [$instance, 'cmd1']
$tester->execute([]);
$tester->assertCommandIsSuccessful();

// etc.

@yceruto yceruto requested a review from chalasr as a code owner November 29, 2025 11:33
@carsonbot carsonbot added this to the 8.1 milestone Nov 29, 2025
@yceruto yceruto force-pushed the method_based_command branch 2 times, most recently from 9eaaeca to 8e4f9a0 Compare November 30, 2025 23:43
@yceruto
Copy link
Copy Markdown
Member Author

yceruto commented Nov 30, 2025

better review hiding whitespace https://github.com/symfony/symfony/pull/62567/files?w=1

@yceruto yceruto force-pushed the method_based_command branch from 8e4f9a0 to 74eac25 Compare December 1, 2025 16:37
@stof
Copy link
Copy Markdown
Member

stof commented Dec 1, 2025

how does this interact with the Prompt attribute added in version 7.4 ?

@stof
Copy link
Copy Markdown
Member

stof commented Dec 1, 2025

To me, the component standalone usage should show how to register those in an Application actually (which will probably show that those are much more cumbersome to use than the invokable commands by having to do the Command wrapping in userland instead of keeping it internal, unless I missed something)

@yceruto
Copy link
Copy Markdown
Member Author

yceruto commented Dec 1, 2025

how does this interact with the Prompt attribute added in version 7.4 ?

when the #[Interact] attribute is used, it will need to check $input->getArgument('command') to distinguish the executed command. we can create as many #[Interact] as needed. though this new approach shouldn't be used for complex commands

stof
stof reviewed Dec 1, 2025
View reviewed changes
src/Symfony/Component/Console/DependencyInjection/AddConsoleCommandPass.php
private function registerCommand(ContainerBuilder $container, \ReflectionClass $reflection, string $id, string $class, array $tags, Definition $definition, array &$serviceIds, array &$lazyCommandMap, array &$lazyCommandRefs): void
{
if (!$reflection->isSubclassOf(Command::class)) {
$method = $tags[0]['method'] ?? '__invoke';
Copy link
Copy Markdown
Member

@stof stof Dec 1, 2025

Choose a reason for hiding this comment

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

reading $tags[0] is broken when processing the registration for other tags. It will not validate the right method (and won't register the right one either)

Copy link
Copy Markdown
Member Author

@yceruto yceruto Dec 7, 2025

Choose a reason for hiding this comment

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

I don't see the broken escenario. other tags for the same command method will be processed later and interpreted as aliases.

Copy link
Copy Markdown
Member Author

@yceruto yceruto Dec 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

btw, tags are now split into methods, so there is no way to process other tags with a different method in this same iteration

@stof
Copy link
Copy Markdown
Member

stof commented Dec 1, 2025

it will need to check $input->getArgument('command') to distinguish the executed command.

This is a bad advice because the Application class supports resolving commands using any non-ambiguous shortened name passed as the command argument. So you could be running the foobar command with $input->getArgument('command') === 'f' if this is non ambiguous.

@yceruto yceruto force-pushed the method_based_command branch 2 times, most recently from b84cd82 to e123736 Compare December 7, 2025 22:32
@yceruto
Copy link
Copy Markdown
Member Author

yceruto commented Dec 7, 2025 •
edited
Loading

it will need to check $input->getArgument('command') to distinguish the executed command.

This is a bad advice because the Application class supports resolving commands using any non-ambiguous shortened name passed as the command argument. So you could be running the foobar command with $input->getArgument('command') === 'f' if this is non ambiguous.

yes, that's unfortunately not perfect, so I've added support for injecting the Command instance. This way people can check getName() to distinguish them:

#[Interact]
public function prompt(Command $cmd, /* ... */): void
{
    // $cmd->getName()
}

GromNaN
GromNaN reviewed Dec 7, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@GromNaN GromNaN left a comment •
edited
Loading

Choose a reason for hiding this comment

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

how does this interact with the Prompt attribute added in version 7.4 ?

when the #[Interact] attribute is used, it will need to check $input->getArgument('command') to distinguish the executed command. we can create as many #[Interact] as needed. though this new approach shouldn't be used for complex commands

Calling all the #[Interact] methods of the class for every commands is the most logical behavior. This feature will only be usable when all the command methods have the same interaction needs.

@yceruto yceruto force-pushed the method_based_command branch from e123736 to cb1d853 Compare December 8, 2025 10:39
GromNaN
GromNaN approved these changes Dec 10, 2025
View reviewed changes
nicolas-grekas
nicolas-grekas approved these changes Dec 23, 2025
View reviewed changes
@yceruto yceruto force-pushed the method_based_command branch from cb1d853 to 8bfc423 Compare December 23, 2025 11:52
@yceruto yceruto force-pushed the method_based_command branch from 8bfc423 to 6b3ebfb Compare December 23, 2025 11:59
@chalasr
Copy link
Copy Markdown
Member

chalasr commented Dec 25, 2025

Thank you @yceruto.

@chalasr chalasr merged commit 5832ee9 into symfony:8.1 Dec 25, 2025
9 of 11 checks passed
@chalasr chalasr mentioned this pull request Dec 25, 2025
Closed
@yceruto yceruto deleted the method_based_command branch December 25, 2025 11:10
This was referenced Jan 27, 2026
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stof stof stof left review comments

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@GromNaN GromNaN GromNaN approved these changes

@chalasr chalasr chalasr approved these changes

Assignees

No one assigned

Labels

Console Feature Status: Reviewed

Projects

None yet

Milestone

8.1

Development

Successfully merging this pull request may close these issues.

6 participants

@yceruto @stof @chalasr @nicolas-grekas @GromNaN @carsonbot