bug #1654 Revert #1575 (Use a PHP-CS-Fixer shim rather than an extern…

…al PHAR) (kbond) This PR was merged into the 1.x-dev branch. Discussion ---------- Revert #1575 (Use a PHP-CS-Fixer shim rather than an external PHAR) This reverts #1575. This PR has caused a lot of trouble as it made php-cs-fixer a required dep which installs a recipe. See #1644, #1653, #1651, #1648. We can't have php-cs-fixer (or the shim) as a required dependency. I think we need to have this bundle only use php-cs-fixer if available/configured (or drop it entirely). I'm not sure why it is used at all - feels like a lot of added complexity... Commits ------- 41744d7 Revert "feature #1575 [make:*] Use a PHP-CS-Fixer shim rather than an external PHAR"
symfony · Jan 15, 2025 · 468ff27 · 468ff27
2 parents 495b7d6 + 41744d7
commit 468ff27
Show file tree

Hide file tree

Showing 12 changed files with 23 additions and 31 deletions.
diff --git a/composer.json b/composer.json
@@ -16,7 +16,6 @@
         "php": ">=8.1",
         "doctrine/inflector": "^2.0",
         "nikic/php-parser": "^4.18|^5.0",
-        "php-cs-fixer/shim": "^v3.64",
         "symfony/config": "^6.4|^7.0",
         "symfony/console": "^6.4|^7.0",
         "symfony/dependency-injection": "^6.4|^7.0",

diff --git a/config/services.xml b/config/services.xml
@@ -41,7 +41,6 @@
             </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>

diff --git a/src/Resources/bin/php-cs-fixer-v3.49.0.phar b/src/Resources/bin/php-cs-fixer-v3.49.0.phar
diff --git a/src/Util/TemplateLinter.php b/src/Util/TemplateLinter.php
@@ -12,7 +12,6 @@
 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;
@@ -26,12 +25,14 @@
  */
 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,
     ) {
@@ -97,15 +98,9 @@ public function writeLinterMessage(OutputInterface $output): void
 
     private function setBinary(): void
     {
-        // Use Bundled (shim) PHP-CS-Fixer
+        // Use Bundled PHP-CS-Fixer
         if (null === $this->phpCsFixerBinaryPath) {
-            $shimLocation = \sprintf('%s/vendor/bin/php-cs-fixer', \dirname(__DIR__, 2));
-
-            if (is_file($shimLocation)) {
-                $this->phpCsFixerBinaryPath = $shimLocation;
-
-                return;
-            }
+            $this->phpCsFixerBinaryPath = \sprintf('%s/Resources/bin/php-cs-fixer-v%s.phar', \dirname(__DIR__), self::BUNDLED_PHP_CS_FIXER_VERSION);
 
             return;
         }
@@ -134,8 +129,7 @@ private function setBinary(): void
     private function setConfig(): void
     {
         // No config provided, but there is a dist config file in the project dir
-        $defaultConfigPath = \sprintf('%s/.php-cs-fixer.dist.php', $this->fileManager->getRootDirectory());
-        if (null === $this->phpCsFixerConfigPath && file_exists($defaultConfigPath)) {
+        if (null === $this->phpCsFixerConfigPath && file_exists($defaultConfigPath = '.php-cs-fixer.dist.php')) {
             $this->phpCsFixerConfigPath = $defaultConfigPath;
 
             $this->usingBundledPhpCsFixerConfig = false;

diff --git a/tests/Command/MakerCommandTest.php b/tests/Command/MakerCommandTest.php
@@ -40,7 +40,7 @@ public function testExceptionOnMissingDependencies(): void
 
         $fileManager = $this->createMock(FileManager::class);
 
-        $command = new MakerCommand($maker, $fileManager, new Generator($fileManager, 'App'), new TemplateLinter($fileManager));
+        $command = new MakerCommand($maker, $fileManager, new Generator($fileManager, 'App'), new TemplateLinter());
         // needed because it's normally set by the Application
         $command->setName('make:foo');
         $tester = new CommandTester($command);
@@ -53,7 +53,7 @@ public function testExceptionOnUnknownRootNamespace(): void
 
         $fileManager = $this->createMock(FileManager::class);
 
-        $command = new MakerCommand($maker, $fileManager, new Generator($fileManager, 'Unknown'), new TemplateLinter($fileManager));
+        $command = new MakerCommand($maker, $fileManager, new Generator($fileManager, 'Unknown'), new TemplateLinter());
         // needed because it's normally set by the Application
         $command->setName('make:foo');
         $tester = new CommandTester($command);

diff --git a/tests/Maker/TemplateLinterTest.php b/tests/Maker/TemplateLinterTest.php
@@ -33,6 +33,7 @@ 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');
 
@@ -52,12 +53,13 @@ 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';
+                $expectedOutput = 'System PHP-CS-Fixer (bin/php-cs-fixer) & System PHP-CS-Fixer Configuration (php-cs-fixer.test.php)';
                 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',
@@ -77,14 +79,13 @@ public function getTestDetails(): \Generator
 
                 self::assertStringContainsString('Linted with stock php-cs-config', $generatedTemplate);
 
-                $expectedOutput = 'Bundled PHP-CS-Fixer & System PHP-CS-Fixer Configuration';
+                $expectedOutput = 'Bundled PHP-CS-Fixer & System PHP-CS-Fixer Configuration (.php-cs-fixer.dist.php)';
                 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']);
 

diff --git a/tests/Util/TemplateLinterTest.php b/tests/Util/TemplateLinterTest.php
@@ -11,9 +11,7 @@
 
 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;
 
@@ -30,30 +28,27 @@ public function testExceptionBinaryPathDoesntExist(): void
     {
         $this->expectExceptionMessage('The MAKER_PHP_CS_FIXER_BINARY_PATH provided: /some/bad/path does not exist.');
 
-        new TemplateLinter($this->createMock(FileManager::class), phpCsFixerBinaryPath: '/some/bad/path');
+        new TemplateLinter(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($this->createMock(FileManager::class), phpCsFixerConfigPath: '/bad/config/path');
+        new TemplateLinter(phpCsFixerConfigPath: '/bad/config/path');
     }
 
     public function testPhpCsFixerVersion(): void
     {
         $this->markTestSkippedOnWindows();
 
-        $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');
+        $fixerPath = \sprintf('%s/src/Resources/bin/php-cs-fixer-v%s.phar', \dirname(__DIR__, 2), TemplateLinter::BUNDLED_PHP_CS_FIXER_VERSION);
 
         $process = Process::fromShellCommandline(\sprintf('%s -V', $fixerPath));
 
         $process->run();
 
-        self::assertStringContainsString($expectedVersion, $process->getOutput());
+        self::assertStringContainsString(TemplateLinter::BUNDLED_PHP_CS_FIXER_VERSION, $process->getOutput());
     }
 
     private function markTestSkippedOnWindows(): void

diff --git a/tests/fixtures/make-serializer-normalizer/EntityFixtureNormalizer.php b/tests/fixtures/make-serializer-normalizer/EntityFixtureNormalizer.php
@@ -10,7 +10,7 @@ class EntityFixtureNormalizer implements NormalizerInterface
 {
     public function __construct(
         #[Autowire(service: 'serializer.normalizer.object')]
-        private NormalizerInterface $normalizer,
+        private NormalizerInterface $normalizer
     ) {
     }
 

diff --git a/tests/fixtures/make-serializer-normalizer/FooBarNormalizer.php b/tests/fixtures/make-serializer-normalizer/FooBarNormalizer.php
@@ -9,7 +9,7 @@ class FooBarNormalizer implements NormalizerInterface
 {
     public function __construct(
         #[Autowire(service: 'serializer.normalizer.object')]
-        private NormalizerInterface $normalizer,
+        private NormalizerInterface $normalizer
     ) {
     }
 

diff --git a/tests/fixtures/make-validator/expected/FooBar.php b/tests/fixtures/make-validator/expected/FooBar.php
@@ -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);
     }

diff --git a/tests/fixtures/make-voter/expected/FooBarVoter.php b/tests/fixtures/make-voter/expected/FooBarVoter.php
@@ -22,6 +22,7 @@ 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;
@@ -33,6 +34,7 @@ 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

diff --git a/tests/fixtures/make-voter/expected/not_final_FooBarVoter.php b/tests/fixtures/make-voter/expected/not_final_FooBarVoter.php
@@ -22,6 +22,7 @@ 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;
@@ -33,6 +34,7 @@ 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