Skip to content

Commit

Permalink
feature symfony#1575 [make:*] Use a PHP-CS-Fixer shim rather than an …
Browse files Browse the repository at this point in the history
…external PHAR

* build: Use a PHP-CS-Fixer shim rather than an external PHAR

* minor fixes

* use sprintf

* fix config file path

* use file manager to get the root project path

* php-cs-fixer expectations for makers w/ generated comparison

* fix missing fileManager instance in test

---------

Co-authored-by: Jesse Rushlow <jr@rushlow.dev>
  • Loading branch information
theofidry and jrushlow authored Sep 25, 2024
1 parent 3cf70d8 commit 4735ff1
Show file tree
Hide file tree
Showing 12 changed files with 31 additions and 23 deletions.
1 change: 1 addition & 0 deletions composer.json
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",
"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
Binary file not shown.
1 change: 1 addition & 0 deletions src/Resources/config/services.xml
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
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
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
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
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
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
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
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
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
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

0 comments on commit 4735ff1

Please sign in to comment.