NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.7k

[Console] Allow callable for`AsCommand`,`Argument` and`Option` attributes arguments#61677

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Open

MacDada wants to merge5 commits intosymfony:7.4

base:7.4

Choose a base branch

fromMacDada:74_command_description

Open

[Console] Allow callable for`AsCommand`,`Argument` and`Option` attributes arguments#61677

MacDada wants to merge5 commits intosymfony:7.4fromMacDada:74_command_description

Conversation

Copy link

Contributor

MacDada commentedSep 7, 2025

Q	A
Branch?	7.4
Bug fix?	no
New feature?	yes
Deprecations?	no
License	MIT

Given:

privateconstSOME_CONST ='foo';publicstaticfunctiongenerateTheArgumentDescription(){returnsprintf('The description is being generated with %s being a const.',self::SOME_CONST,    );}

With "old style" Commands I'm able to generate description of an Argument:

protectedfunctionconfigure():void{$this->addArgument('the-argument',        InputArgument::REQUIRED,self::generateTheArgumentDescription(),    );}

It's currently not possible with "Symfony Style":

publicfunction __invoke(SymfonyStyle$io,    #[Argument(// "Fatal error: Constant expression contains invalid operations"        description:self::generateTheArgumentDescription(),    )]string$theArgument,)

It becomes possible with this PR:

publicfunction __invoke(SymfonyStyle$io,    #[Argument(        description: [self::class,'generateTheArgumentDescription'],    )]string$theArgument,)

MacDada requested a review fromchalasr as acode owner

September 7, 2025 12:33

Copy link

Member

GromNaN commentedSep 7, 2025

An interesting option that allows the definition to be made more dynamic.

Then we should probably do the same for#[AsCommand]:description,help andusages.

With PHP 8.5, the named closure syntax can be used:

publicfunction __invoke(SymfonyStyle$io,    #[Argument(description:self::generateTheArgumentDescription(...))]string$theArgument,)

GromNaN reviewed

Sep 7, 2025

View reviewed changes

src/Symfony/Component/Console/Tests/Command/InvokableCommandTest.phpShow resolvedHide resolved

Copy link

ContributorAuthor

MacDada commentedSep 7, 2025•
edited
Loading

Then we should probably do the same for#[AsCommand]:description,help andusages.

Not required in my codebase, but should be simple enough -> I can volunteer a new PR or extend this one if this is the way to go :-)

GromNaN added Console Feature labels

Sep 7, 2025

yceruto reviewed

Sep 7, 2025

View reviewed changes

src/Symfony/Component/Console/Attribute/Argument.php Outdated

		publicstring$name ='',
		array\|callable$suggestedValues = [],
		) {
		$this->description =\is_callable($description) ?$description() :$description;

Copy link

Member

Choose a reason for hiding this comment

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

I would apply the same approach we used for suggestedValues, so that non-static callables are supported. This way, we can access the$this instance and its dependencies if needed, for example, to build a description dynamically based on the command's dependencies

Copy link

ContributorAuthor

MacDadaSep 7, 2025

Choose a reason for hiding this comment

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

already tried that and failed, not sure why, guessingarray|callable (working as getting "in" as just array) vsstring|callable (not working, as not getting in as array is not a string, and not an acceptable callable)

will try doing it again tomorrow

Copy link

ContributorAuthor

MacDadaSep 29, 2025

Choose a reason for hiding this comment

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

Alright, I've tried to do that, but I've noticed a problem: I don't think I can do that without breaking compatibility.

As for now,$description is apublic string field:https://github.com/symfony/symfony/blob/v7.3.4/src/Symfony/Component/Console/Attribute/Argument.php#L39

I'd have to change it topublic string|\Closure, so if someone's code relies upon$description being a string, it would break it.

Copy link

Member

nicolas-grekas commentedSep 8, 2025

What's the use case? I prefer simple static data. A static call can only return static data anyway.

Copy link

ContributorAuthor

MacDada commentedSep 8, 2025•
edited
Loading

What's the use case? I prefer simple static data. A static call can only return static data anyway.

Simple, I want the command to be more "user-friendly" by listing valid values in the argument description.

Yeah, sure, if the user passes an invalid value an error will be shown, but I don't want to encourage user to intentionally type invalid value just to discover what valid values are.

In my particular case, the valid values are from ENUM, so it is statically accessible. It just cannot be passed todescription, as PHP doesn't allow me that.

Given enum:

Right now, with "old style"configure – all is dandy:

Right now if I use "SymfonyStyle" – I'm missing allowed values list in the description:

Right now, if I try to use "SymfonyStyle" – with the allowed values list in the description – error:

If I move it to a static function – still an error:

With this PR, itcould work – and give me the desired description with allowed values listed:

To sum up: this PR would allow me to switch to "SymfonyStyle", instead of using "old style"configure, in order to display valid values of an Argument.

nicolas-grekas reviewed

Sep 8, 2025

View reviewed changes

src/Symfony/Component/Console/Tests/Command/InvokableCommandTest.php Outdated


		publicstaticfunctiongenerateEducationDescription():string
		{
		returnsprintf('Mention your favourite lectures from %s!',self::UNIVERSITY);

Copy link

Member

nicolas-grekasSep 8, 2025

Choose a reason for hiding this comment

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

let's inline this, indirections makes reviewing test cases more complex

Copy link

ContributorAuthor

MacDadaSep 29, 2025

Choose a reason for hiding this comment

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

i guess u meant to inline the const:71a08ac

nicolas-grekas added this to the7.4 milestone

Sep 16, 2025

GromNaN mentioned this pull request

Sep 16, 2025

[Console] Make descriptions in #[AsCommand] #[Argument] #[Option] more flexible (placeholders / callables)#61767

Open

Copy link

Member

chalasr commentedSep 29, 2025

@MacDada any chance you can finish this one soon? The feature freeze period is starting for 7.4.

MacDada added a commit to MacDada/symfony that referenced this pull request

Sep 29, 2025

InvokableCommandTest: inlined const

71a08ac

symfony#61677 (comment)

GromNaN reviewed

Sep 29, 2025

View reviewed changes

src/Symfony/Component/Console/Attribute/Argument.phpShow resolvedHide resolved

MacDada added a commit to MacDada/symfony that referenced this pull request

Sep 29, 2025

[Console] Fixed$description ofArgument's andOption's `Attrib…

295b2d1

…ute`s being interpreted as callablesymfony#61677 (comment)

Copy link

ContributorAuthor

MacDada commentedSep 29, 2025•
edited
Loading

any chance you can finish this one soon?

@chalasr As for my own particular needs – this PR is finished.

BUT:

~~There is a conflict on CHANGELOG – should I rebase and fix it?~~ -> EDIT: rebased
Should I squash the PR into one commit instead of it having multiple (with fixes)?
~~Should Iextend it uponAsCommand as well?~~ -> EDIT:done as much as I believe is doable
I guess I should skipmaking it more dynamic?

MacDada added a commit to MacDada/symfony that referenced this pull request

Sep 29, 2025

[Console]AsCommand attribute: allow callable for$description an…

a98e93d

…d $helpsymfony#61677 (comment)

Copy link

ContributorAuthor

MacDada commentedSep 29, 2025•
edited
Loading

Then we should probably do the same for#[AsCommand]:description,help andusages.

@GromNaN I've done it fordescription andhelp:5fd1f25

I didn't do it forusages as I guess it could be a "conflict" between normal array of strings and an array representing a callable… similar to this one:#61677 (comment) – but I don't know if there is a way to "fix" it here…

MacDada added a commit to MacDada/symfony that referenced this pull request

Sep 29, 2025

[Console]AsCommand attribute: allow callable for$description an…

6b89c2c

…d $helpsymfony#61677 (comment)

MacDada force-pushed the74_command_description branch froma98e93d to6b89c2cCompare

September 29, 2025 21:24

MacDada added4 commits

September 29, 2025 23:26

[Console] Allow callable for$description ofArgument's and `Opti…

6a3aeea

…on`'s `Attribute`s.

InvokableCommandTest: inlined const

54ddc7c

symfony#61677 (comment)

[Console] Fixed$description ofArgument's andOption's `Attrib…

88bbd26

…ute`s being interpreted as callablesymfony#61677 (comment)

CommandTest: removed unusedPhp8Command2 class

f3ad359

MacDada force-pushed the74_command_description branch from6b89c2c tof85c1adCompare

September 29, 2025 21:27

MacDada added a commit to MacDada/symfony that referenced this pull request

Sep 29, 2025

[Console]AsCommand attribute: allow callable for$description an…

f85c1ad

…d $helpsymfony#61677 (comment)

MacDada added a commit to MacDada/symfony that referenced this pull request

Sep 29, 2025

[Console]AsCommand attribute: allow callable for$description an…

8929999

…d $helpsymfony#61677 (comment)

MacDada force-pushed the74_command_description branch fromf85c1ad to8929999Compare

September 29, 2025 21:32

MacDada changed the title~~[Console] Allow callable for$description ofArgument's andOption'sAttributes~~[Console] Allow callable forAsCommand,Argument andOption attributes

Sep 29, 2025

[Console]AsCommand attribute: allow callable for$description an…

5fd1f25

…d $helpsymfony#61677 (comment)

MacDada force-pushed the74_command_description branch from8929999 to5fd1f25Compare

September 29, 2025 21:51

MacDada changed the title~~[Console] Allow callable forAsCommand,Argument andOption attributes~~[Console] Allow callable forAsCommand,Argument andOption attributes arguments

Sep 29, 2025