Skip to content

[make:*] Use a PHP-CS-Fixer shim rather than an external PHAR #1575

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

Merged
merged 7 commits into from
Sep 25, 2024
Merged

[make:*] Use a PHP-CS-Fixer shim rather than an external PHAR #1575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
755c184
build: Use a PHP-CS-Fixer shim rather than an external PHAR
theofidry Jun 25, 2024
4f3ae39
minor fixes
jrushlow Sep 13, 2024
e9c4af6
use sprintf
jrushlow Sep 13, 2024
431cd14
fix config file path
jrushlow Sep 25, 2024
45c871c
use file manager to get the root project path
jrushlow Sep 25, 2024
2dd07dd
php-cs-fixer expectations for makers w/ generated comparison
jrushlow Sep 25, 2024
3456389
fix missing fileManager instance in test
jrushlow Sep 25, 2024
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
1 change: 1 addition & 0 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
"php": ">=8.1",
"doctrine/inflector": "^2.0",
"nikic/php-parser": "^4.18|^5.0",
"php-cs-fixer/shim": "^v3.64",
Copy link

Choose a reason for hiding this comment

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

This should be in require-dev

Copy link
Contributor

@mmarton mmarton Jan 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

yes, and it has a recipe and created/update a bunch of files in my project

Copy link
Member

Choose a reason for hiding this comment

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

#1648

Copy link

Choose a reason for hiding this comment

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

#1644

Copy link

Choose a reason for hiding this comment

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

It should not be in require-dev as php-cs-fixer is used at runtime to format the generated files. (See the changes in \Symfony\Bundle\MakerBundle\Util\TemplateLinter) I think symfony/maker-bundle should instead require friendsofphp/php-cs-fixer, so the decision to use the shim package can be made by the user.

pr #1651 contains this change.

"symfony/config": "^6.4|^7.0",
"symfony/console": "^6.4|^7.0",
"symfony/dependency-injection": "^6.4|^7.0",
Expand Down
Binary file removed src/Resources/bin/php-cs-fixer-v3.49.0.phar
View file
Open in desktop
Binary file not shown.
1 change: 1 addition & 0 deletions src/Resources/config/services.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
</service>

<service id="maker.template_linter" class="Symfony\Bundle\MakerBundle\Util\TemplateLinter">
<argument type="service" id="maker.file_manager" />
<argument>%env(default::string:MAKER_PHP_CS_FIXER_BINARY_PATH)%</argument>
<argument>%env(default::string:MAKER_PHP_CS_FIXER_CONFIG_PATH)%</argument>
</service>
Expand Down
18 changes: 12 additions & 6 deletions src/Util/TemplateLinter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Bundle\MakerBundle\Util;

use Symfony\Bundle\MakerBundle\Exception\RuntimeCommandException;
use Symfony\Bundle\MakerBundle\FileManager;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Process\ExecutableFinder;
use Symfony\Component\Process\Process;
Expand All @@ -25,14 +26,12 @@
*/
final class TemplateLinter
{
// Version must match bundled version file name. e.g. php-cs-fixer-v3.49.9.phar
public const BUNDLED_PHP_CS_FIXER_VERSION = '3.49.0';

private bool $usingBundledPhpCsFixer = true;
private bool $usingBundledPhpCsFixerConfig = true;
private bool $needsPhpCmdPrefix = true;

public function __construct(
private FileManager $fileManager,
private ?string $phpCsFixerBinaryPath = null,
private ?string $phpCsFixerConfigPath = null,
) {
Expand Down Expand Up @@ -98,9 +97,15 @@ public function writeLinterMessage(OutputInterface $output): void

private function setBinary(): void
{
// Use Bundled PHP-CS-Fixer
// Use Bundled (shim) PHP-CS-Fixer
if (null === $this->phpCsFixerBinaryPath) {
$this->phpCsFixerBinaryPath = \sprintf('%s/Resources/bin/php-cs-fixer-v%s.phar', \dirname(__DIR__), self::BUNDLED_PHP_CS_FIXER_VERSION);
$shimLocation = \sprintf('%s/vendor/bin/php-cs-fixer', \dirname(__DIR__, 2));

if (is_file($shimLocation)) {
$this->phpCsFixerBinaryPath = $shimLocation;

return;
}

return;
}
Expand Down Expand Up @@ -129,7 +134,8 @@ private function setBinary(): void
private function setConfig(): void
{
// No config provided, but there is a dist config file in the project dir
if (null === $this->phpCsFixerConfigPath && file_exists($defaultConfigPath = '.php-cs-fixer.dist.php')) {
$defaultConfigPath = \sprintf('%s/.php-cs-fixer.dist.php', $this->fileManager->getRootDirectory());
if (null === $this->phpCsFixerConfigPath && file_exists($defaultConfigPath)) {
$this->phpCsFixerConfigPath = $defaultConfigPath;

$this->usingBundledPhpCsFixerConfig = false;
Expand Down
4 changes: 2 additions & 2 deletions tests/Command/MakerCommandTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public function testExceptionOnMissingDependencies(): void

$fileManager = $this->createMock(FileManager::class);

$command = new MakerCommand($maker, $fileManager, new Generator($fileManager, 'App'), new TemplateLinter());
$command = new MakerCommand($maker, $fileManager, new Generator($fileManager, 'App'), new TemplateLinter($fileManager));
// needed because it's normally set by the Application
$command->setName('make:foo');
$tester = new CommandTester($command);
Expand All @@ -53,7 +53,7 @@ public function testExceptionOnUnknownRootNamespace(): void

$fileManager = $this->createMock(FileManager::class);

$command = new MakerCommand($maker, $fileManager, new Generator($fileManager, 'Unknown'), new TemplateLinter());
$command = new MakerCommand($maker, $fileManager, new Generator($fileManager, 'Unknown'), new TemplateLinter($fileManager));
// needed because it's normally set by the Application
$command->setName('make:foo');
$tester = new CommandTester($command);
Expand Down
7 changes: 3 additions & 4 deletions tests/Maker/TemplateLinterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ protected function getMakerClass(): string
public function getTestDetails(): \Generator
{
yield 'lints_templates_with_custom_php_cs_fixer_and_config' => [$this->createMakerTest()
->addExtraDependencies('php-cs-fixer/shim')
->run(function (MakerTestRunner $runner) {
$runner->copy('template-linter/php-cs-fixer.test.php', 'php-cs-fixer.test.php');

Expand All @@ -53,13 +52,12 @@ public function getTestDetails(): \Generator

self::assertStringContainsString('Linted by custom php-cs-config', $generatedTemplate);

$expectedOutput = 'System PHP-CS-Fixer (bin/php-cs-fixer) & System PHP-CS-Fixer Configuration (php-cs-fixer.test.php)';
$expectedOutput = 'System PHP-CS-Fixer (bin/php-cs-fixer) & System PHP-CS-Fixer Configuration';
self::assertStringContainsString($expectedOutput, $output);
}),
];

yield 'lints_templates_with_flex_generated_config_file' => [$this->createMakerTest()
->addExtraDependencies('php-cs-fixer/shim')
->run(function (MakerTestRunner $runner) {
$runner->replaceInFile(
'.php-cs-fixer.dist.php',
Expand All @@ -79,13 +77,14 @@ public function getTestDetails(): \Generator

self::assertStringContainsString('Linted with stock php-cs-config', $generatedTemplate);

$expectedOutput = 'Bundled PHP-CS-Fixer & System PHP-CS-Fixer Configuration (.php-cs-fixer.dist.php)';
$expectedOutput = 'Bundled PHP-CS-Fixer & System PHP-CS-Fixer Configuration';
self::assertStringContainsString($expectedOutput, $output);
}),
];

yield 'lints_templates_with_bundled_php_cs_fixer' => [$this->createMakerTest()
->run(function (MakerTestRunner $runner) {
$runner->deleteFile('.php-cs-fixer.dist.php');
// Voter class name
$output = $runner->runMaker(['FooBar']);

Expand Down
13 changes: 9 additions & 4 deletions tests/Util/TemplateLinterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@

namespace Symfony\Bundle\MakerBundle\Tests\Util;

use Composer\InstalledVersions;
use PHPUnit\Framework\TestCase;
use Symfony\Bundle\MakerBundle\FileManager;
use Symfony\Bundle\MakerBundle\Util\TemplateLinter;
use Symfony\Component\Process\Process;

Expand All @@ -28,27 +30,30 @@ public function testExceptionBinaryPathDoesntExist(): void
{
$this->expectExceptionMessage('The MAKER_PHP_CS_FIXER_BINARY_PATH provided: /some/bad/path does not exist.');

new TemplateLinter(phpCsFixerBinaryPath: '/some/bad/path');
new TemplateLinter($this->createMock(FileManager::class), phpCsFixerBinaryPath: '/some/bad/path');
}

public function testExceptionThrownIfConfigPathDoesntExist(): void
{
$this->expectExceptionMessage('The MAKER_PHP_CS_FIXER_CONFIG_PATH provided: /bad/config/path does not exist.');

new TemplateLinter(phpCsFixerConfigPath: '/bad/config/path');
new TemplateLinter($this->createMock(FileManager::class), phpCsFixerConfigPath: '/bad/config/path');
}

public function testPhpCsFixerVersion(): void
{
$this->markTestSkippedOnWindows();

$fixerPath = \sprintf('%s/src/Resources/bin/php-cs-fixer-v%s.phar', \dirname(__DIR__, 2), TemplateLinter::BUNDLED_PHP_CS_FIXER_VERSION);
$fixerPath = \sprintf('%s/vendor/php-cs-fixer/shim/php-cs-fixer', \dirname(__DIR__, 2));

// Get the installed version and remove the preceding "v"
$expectedVersion = ltrim(InstalledVersions::getPrettyVersion('php-cs-fixer/shim'), 'v');

$process = Process::fromShellCommandline(\sprintf('%s -V', $fixerPath));

$process->run();

self::assertStringContainsString(TemplateLinter::BUNDLED_PHP_CS_FIXER_VERSION, $process->getOutput());
self::assertStringContainsString($expectedVersion, $process->getOutput());
}

private function markTestSkippedOnWindows(): void
Expand Down
2 changes: 1 addition & 1 deletion tests/fixtures/make-serializer-normalizer/EntityFixtureNormalizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class EntityFixtureNormalizer implements NormalizerInterface
{
public function __construct(
#[Autowire(service: 'serializer.normalizer.object')]
private NormalizerInterface $normalizer
private NormalizerInterface $normalizer,
) {
}

Expand Down
2 changes: 1 addition & 1 deletion tests/fixtures/make-serializer-normalizer/FooBarNormalizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class FooBarNormalizer implements NormalizerInterface
{
public function __construct(
#[Autowire(service: 'serializer.normalizer.object')]
private NormalizerInterface $normalizer
private NormalizerInterface $normalizer,
) {
}

Expand Down
2 changes: 1 addition & 1 deletion tests/fixtures/make-validator/expected/FooBar.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ final class FooBar extends Constraint
public function __construct(
public string $mode = 'strict',
?array $groups = null,
mixed $payload = null
mixed $payload = null,
) {
parent::__construct([], $groups, $payload);
}
Expand Down
2 changes: 0 additions & 2 deletions tests/fixtures/make-voter/expected/FooBarVoter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ protected function supports(string $attribute, mixed $subject): bool
protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token): bool
{
$user = $token->getUser();

// if the user is anonymous, do not grant access
if (!$user instanceof UserInterface) {
return false;
Expand All @@ -34,7 +33,6 @@ protected function voteOnAttribute(string $attribute, mixed $subject, TokenInter
// logic to determine if the user can EDIT
// return true or false
break;

case self::VIEW:
// logic to determine if the user can VIEW
// return true or false
Expand Down
2 changes: 0 additions & 2 deletions tests/fixtures/make-voter/expected/not_final_FooBarVoter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ protected function supports(string $attribute, mixed $subject): bool
protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token): bool
{
$user = $token->getUser();

// if the user is anonymous, do not grant access
if (!$user instanceof UserInterface) {
return false;
Expand All @@ -34,7 +33,6 @@ protected function voteOnAttribute(string $attribute, mixed $subject, TokenInter
// logic to determine if the user can EDIT
// return true or false
break;

case self::VIEW:
// logic to determine if the user can VIEW
// return true or false
Expand Down
Loading