[make:command] lets use attributes if possible #822
Conversation
| @@ -9,11 +9,19 @@ | |||
| use Symfony\Component\Console\Output\OutputInterface; | |||
| use Symfony\Component\Console\Style\SymfonyStyle; | |||
|
|
|||
| <?php if ($use_attributes): ?> | |||
There was a problem hiding this comment.
It's more complex than this. $use_attributes tells us if the PHP version supports attributes. But in this case, we also need to see if the version of Symfony has the ConsoleCommand class. Also, this will need a use statement on top.
I know we're in the process of fixing some test failures to due upstream changes (so it's hard to look at CI), but let's make sure that if we misconfigure this somehow (e.g. forget the use statement) that it does cause a test to fail.
jrushlow
force-pushed
the
feature/command-attributes
branch
from
e7db329 to
0f6902e
Compare
jrushlow
force-pushed
the
feature/command-attributes
branch
from
fc69bfc to
3ca3736
Compare
jrushlow
changed the title
jrushlow
force-pushed
the
feature/command-attributes
branch
from
41cfaa5 to
6d0db57
Compare
| @@ -63,6 +64,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen | |||
| [ | |||
| 'command_name' => $commandName, | |||
| 'set_description' => !class_exists(LazyCommand::class), | |||
| 'use_command_attribute' => 80000 <= \PHP_VERSION_ID && class_exists(AsCommand::class), | |||
There was a problem hiding this comment.
Under a different PR, I think we can add a helper to the PhpCompatUtil to eliminate having to check the PHP version before calling class_exists(). But the complexity required to make that happen is beyond the scope of this PR.
https://bugs.php.net/bug.php?id=80938 explains why this must be done like this.
| @@ -2,18 +2,29 @@ | |||
|
|
|||
| namespace <?= $namespace; ?>; | |||
|
|
|||
| <?php if ($use_attributes && $use_command_attribute): ?> | |||
There was a problem hiding this comment.
Do we need to check both? $use_command_attribute already checks for PHP 8.0 or higher. So it seems like it fully describes which situation we are in, right?
There was a problem hiding this comment.
I think we could drop $use_attributes here, but then we would loose the ability to determine if attributes should be used based on the platform-overrides. Then again as I'm saying this, regardless of platform overrides, they wouldn't be able to use attributes anyway if PHP was not 8+ at runtime..
Thinking we drop $use_attributes for now and when I'm able to accomplish the helper in PhpCompatUtil, we can re introduce / refactor our logic here.
There was a problem hiding this comment.
Ah, you're right about the config.platform.php thing. So then... shouldn't we keep the $use_attributes? Otherwise, it's possible that I'm using PHP 8... but my config.platform.php is set to 7.4... and suddenly we generate code that uses attributes?
jrushlow
force-pushed
the
feature/command-attributes
branch
from
67cdc66 to
95eafee
Compare
|
Thanks Jesse! |
Console command attribute class renamed from
ConsoleCommand=>AsCommandsymfony/symfony#40556closes #820