Skip to content
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

[make:command] lets use attributes if possible #822

Merged
merged 1 commit into from
Apr 9, 2021
Merged

[make:command] lets use attributes if possible #822

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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/Maker/MakeCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Symfony\Bundle\MakerBundle\Generator;
use Symfony\Bundle\MakerBundle\InputConfiguration;
use Symfony\Bundle\MakerBundle\Str;
use Symfony\Component\Console\Attribute\AsCommand;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Command\LazyCommand;
use Symfony\Component\Console\Input\InputArgument;
Expand Down Expand Up @@ -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),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.

]
);

Expand Down
11 changes: 11 additions & 0 deletions src/Resources/skeleton/command/Command.tpl.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,29 @@

namespace <?= $namespace; ?>;

<?php if ($use_attributes && $use_command_attribute): ?>
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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?

use Symfony\Component\Console\Attribute\AsCommand;
<?php endif; ?>
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Style\SymfonyStyle;

<?php if ($use_attributes && $use_command_attribute): ?>
#[AsCommand(
name: '<?= $command_name; ?>',
description: 'Add a short description for your command',
)]
<?php endif; ?>
class <?= $class_name; ?> extends Command
{
<?php if (!$use_attributes || !$use_command_attribute): ?>
protected static $defaultName = '<?= $command_name; ?>';
protected static $defaultDescription = 'Add a short description for your command';

<?php endif; ?>
protected function configure()
{
$this
Expand Down
21 changes: 20 additions & 1 deletion tests/Maker/MakeCommandTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

class MakeCommandTest extends MakerTestCase
{
public function getTestDetails()
public function getTestDetails(): \Generator
{
yield 'command' => [MakerTestDetails::createTest(
$this->getMakerInstance(MakeCommand::class),
Expand All @@ -37,5 +37,24 @@ public function getTestDetails()
->setFixtureFilesPath(__DIR__.'/../fixtures/MakeCommandInCustomRootNamespace')
->changeRootNamespace('Custom'),
];

yield 'command_with_attributes' => [MakerTestDetails::createTest(
$this->getMakerInstance(MakeCommand::class),
[
// command name
'app:foo',
])
->setRequiredPhpVersion(80000)
->addRequiredPackageVersion('symfony/console', '>=5.3')
->setFixtureFilesPath(__DIR__.'/../fixtures/MakeCommand')
->assert(
static function (string $output, string $directory) {
$commandFileContents = file_get_contents(sprintf('%s/src/Command/FooCommand.php', $directory));

self::assertStringContainsString('use Symfony\Component\Console\Attribute\AsCommand;', $commandFileContents);
self::assertStringContainsString('#[AsCommand(', $commandFileContents);
}
),
];
}
}