Browse Source

feat: Introduce `check` command (alias for `fix --dry-run`) (#7322)

Greg Korba 1 year ago
parent
commit
d80a314afe

+ 2 - 2
.github/workflows/ci.yml

@@ -160,13 +160,13 @@ jobs:
 
       - name: Run PHP CS Fixer with PHPDoc to type rules
         if: matrix.phpdoc-to-type-rules == 'yes'
-        run: php php-cs-fixer fix --diff --dry-run -v --config .php-cs-fixer.php-lowest.php
+        run: php php-cs-fixer check --diff -v --config .php-cs-fixer.php-lowest.php
 
       - name: Run PHP CS Fixer
         env:
           PHP_CS_FIXER_IGNORE_ENV: ${{ matrix.PHP_CS_FIXER_IGNORE_ENV }}
           PHP_CS_FIXER_FUTURE_MODE: 1
-        run: php php-cs-fixer fix --diff --dry-run -v
+        run: php php-cs-fixer check --diff -v
 
       - name: Execute deployment checks
         if: matrix.execute-deployment == 'yes'

+ 1 - 1
ci-integration.sh

@@ -5,4 +5,4 @@ IFS='
 '
 CHANGED_FILES=$(git diff --name-only --diff-filter=ACMRTUXB "${COMMIT_RANGE}")
 if ! echo "${CHANGED_FILES}" | grep -qE "^(\\.php-cs-fixer(\\.dist)?\\.php|composer\\.lock)$"; then EXTRA_ARGS=$(printf -- '--path-mode=intersection\n--\n%s' "${CHANGED_FILES}"); else EXTRA_ARGS=''; fi
-vendor/bin/php-cs-fixer fix --config=.php-cs-fixer.dist.php -v --dry-run --stop-on-violation --using-cache=no ${EXTRA_ARGS}
+vendor/bin/php-cs-fixer check --config=.php-cs-fixer.dist.php -v --stop-on-violation --using-cache=no ${EXTRA_ARGS}

+ 1 - 1
composer.json

@@ -84,7 +84,7 @@
         "post-autoload-dump": [
             "@install-tools"
         ],
-        "cs:check": "@php php-cs-fixer fix --dry-run --diff",
+        "cs:check": "@php php-cs-fixer check --diff",
         "cs:fix": "@php php-cs-fixer fix",
         "cs:fix:parallel": "echo '🔍 Will run in batches of 50 files.'; if [[ -f .php-cs-fixer.php ]]; then FIXER_CONFIG=.php-cs-fixer.php; else FIXER_CONFIG=.php-cs-fixer.dist.php; fi; php php-cs-fixer list-files --config=$FIXER_CONFIG | xargs -n 50 -P 8 php php-cs-fixer fix --config=$FIXER_CONFIG --path-mode intersection 2> /dev/null",
         "docs": "@php dev-tools/doc.php",

+ 8 - 2
doc/usage.rst

@@ -75,7 +75,7 @@ Complete configuration for rules can be supplied using a ``json`` formatted stri
 
     php php-cs-fixer.phar fix /path/to/project --rules='{"concat_space": {"spacing": "none"}}'
 
-The ``--dry-run`` flag will run the fixer without making changes to your files.
+The ``--dry-run`` flag will run the fixer without making changes to your files (implicitly set when you use `check` command).
 
 The ``--diff`` flag can be used to let the fixer output all the changes it makes in ``udiff`` format.
 
@@ -119,6 +119,12 @@ fixed but without actually modifying them:
 By using ``--using-cache`` option with ``yes`` or ``no`` you can set if the caching
 mechanism should be used.
 
+The ``check`` command
+---------------------
+
+This command is a shorthand for ``fix --dry-run`` and offers all the options and arguments as ``fix`` command.
+The only difference is that ``check`` command won't apply any changes, but will only print analysis result.
+
 The ``list-files`` command
 --------------------------
 
@@ -207,7 +213,7 @@ Then, add the following command to your CI:
     '
     CHANGED_FILES=$(git diff --name-only --diff-filter=ACMRTUXB "${COMMIT_RANGE}")
     if ! echo "${CHANGED_FILES}" | grep -qE "^(\\.php-cs-fixer(\\.dist)?\\.php|composer\\.lock)$"; then EXTRA_ARGS=$(printf -- '--path-mode=intersection\n--\n%s' "${CHANGED_FILES}"); else EXTRA_ARGS=''; fi
-    vendor/bin/php-cs-fixer fix --config=.php-cs-fixer.dist.php -v --dry-run --stop-on-violation --using-cache=no ${EXTRA_ARGS}
+    vendor/bin/php-cs-fixer check --config=.php-cs-fixer.dist.php -v --stop-on-violation --using-cache=no ${EXTRA_ARGS}
 
 Where ``$COMMIT_RANGE`` is your range of commits, e.g. ``$TRAVIS_COMMIT_RANGE`` or ``HEAD~..HEAD``.
 

+ 1 - 1
phpstan.dist.neon

@@ -14,7 +14,7 @@ parameters:
     reportUnmatchedIgnoredErrors: true # Do not allow outdated errors in the baseline
     treatPhpDocTypesAsCertain: false
     ignoreErrors:
-        - '/^Class [a-zA-Z\\]+ extends @final class PhpCsFixer\\(ConfigurationException\\InvalidConfigurationException|ConfigurationException\\InvalidFixerConfigurationException|Tokenizer\\Tokens)\.$/'
+        - '/^Class [a-zA-Z\\]+ extends @final class PhpCsFixer\\(ConfigurationException\\InvalidConfigurationException|ConfigurationException\\InvalidFixerConfigurationException|Tokenizer\\Tokens|Console\\Command\\FixCommand)\.$/'
         - '/^\$this\(PhpCsFixer\\Tokenizer\\Tokens\) does not accept PhpCsFixer\\Tokenizer\\Token\|null\.$/'
 
         # ignore PHPUnit data providers return type as they are not checked against the test methods anyway

+ 2 - 0
src/Console/Application.php

@@ -14,6 +14,7 @@ declare(strict_types=1);
 
 namespace PhpCsFixer\Console;
 
+use PhpCsFixer\Console\Command\CheckCommand;
 use PhpCsFixer\Console\Command\DescribeCommand;
 use PhpCsFixer\Console\Command\FixCommand;
 use PhpCsFixer\Console\Command\HelpCommand;
@@ -52,6 +53,7 @@ final class Application extends BaseApplication
 
         // in alphabetical order
         $this->add(new DescribeCommand());
+        $this->add(new CheckCommand($this->toolInfo));
         $this->add(new FixCommand($this->toolInfo));
         $this->add(new ListFilesCommand($this->toolInfo));
         $this->add(new ListSetsCommand());

+ 63 - 0
src/Console/Command/CheckCommand.php

@@ -0,0 +1,63 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of PHP CS Fixer.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *     Dariusz Rumiński <dariusz.ruminski@gmail.com>
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace PhpCsFixer\Console\Command;
+
+use PhpCsFixer\ToolInfoInterface;
+use Symfony\Component\Console\Attribute\AsCommand;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Input\InputOption;
+
+/**
+ * @author Greg Korba <greg@codito.dev>
+ *
+ * @internal
+ */
+#[AsCommand(name: 'check', description: 'Checks if configured files/directories comply with configured rules.')]
+final class CheckCommand extends FixCommand
+{
+    protected static $defaultName = 'check';
+    protected static $defaultDescription = 'Checks if configured files/directories comply with configured rules.';
+
+    public function __construct(ToolInfoInterface $toolInfo)
+    {
+        parent::__construct($toolInfo);
+    }
+
+    public function getHelp(): string
+    {
+        $help = explode('<comment>--dry-run</comment>', parent::getHelp());
+
+        return substr($help[0], 0, strrpos($help[0], "\n") - 1)
+            .substr($help[1], strpos($help[1], "\n"));
+    }
+
+    protected function configure(): void
+    {
+        parent::configure();
+
+        $this->setDefinition([
+            ...array_values($this->getDefinition()->getArguments()),
+            ...array_values(array_filter(
+                $this->getDefinition()->getOptions(),
+                static fn (InputOption $option): bool => 'dry-run' !== $option->getName()
+            )),
+        ]);
+    }
+
+    protected function isDryRun(InputInterface $input): bool
+    {
+        return true;
+    }
+}

+ 30 - 25
src/Console/Command/FixCommand.php

@@ -43,12 +43,15 @@ use Symfony\Component\Stopwatch\Stopwatch;
  * @author Fabien Potencier <fabien@symfony.com>
  * @author Dariusz Rumiński <dariusz.ruminski@gmail.com>
  *
+ * @final
+ *
  * @internal
  */
-#[AsCommand(name: 'fix')]
-final class FixCommand extends Command
+#[AsCommand(name: 'fix', description: 'Fixes a directory or a file.')]
+/* final */ class FixCommand extends Command
 {
     protected static $defaultName = 'fix';
+    protected static $defaultDescription = 'Fixes a directory or a file.';
 
     private EventDispatcherInterface $eventDispatcher;
 
@@ -82,7 +85,7 @@ final class FixCommand extends Command
     public function getHelp(): string
     {
         return <<<'EOF'
-            The <info>%command.name%</info> command tries to fix as much coding standards
+            The <info>%command.name%</info> command tries to %command.name% as much coding standards
             problems as possible on a given file or files in a given directory and its subdirectories:
 
                 <info>$ php %command.full_name% /path/to/dir</info>
@@ -159,7 +162,7 @@ final class FixCommand extends Command
 
                 <info>$ php %command.full_name% --verbose --show-progress=dots</info>
 
-            By using <command>--using-cache</command> option with `yes` or `no` you can set if the caching
+            By using <comment>--using-cache</comment> option with `yes` or `no` you can set if the caching
             mechanism should be used.
 
             The command can also read from standard input, in which case it won't
@@ -175,7 +178,7 @@ final class FixCommand extends Command
             Exit code
             ---------
 
-            Exit code of the fix command is built using following bit flags:
+            Exit code of the `%command.name%` command is built using following bit flags:
 
             *  0 - OK.
             *  1 - General error (or PHP minimal requirement not matched).
@@ -190,25 +193,22 @@ final class FixCommand extends Command
 
     protected function configure(): void
     {
-        $this
-            ->setDefinition(
-                [
-                    new InputArgument('path', InputArgument::IS_ARRAY, 'The path.'),
-                    new InputOption('path-mode', '', InputOption::VALUE_REQUIRED, 'Specify path mode (can be override or intersection).', ConfigurationResolver::PATH_MODE_OVERRIDE),
-                    new InputOption('allow-risky', '', InputOption::VALUE_REQUIRED, 'Are risky fixers allowed (can be yes or no).'),
-                    new InputOption('config', '', InputOption::VALUE_REQUIRED, 'The path to a .php-cs-fixer.php file.'),
-                    new InputOption('dry-run', '', InputOption::VALUE_NONE, 'Only shows which files would have been modified.'),
-                    new InputOption('rules', '', InputOption::VALUE_REQUIRED, 'The rules.'),
-                    new InputOption('using-cache', '', InputOption::VALUE_REQUIRED, 'Does cache should be used (can be yes or no).'),
-                    new InputOption('cache-file', '', InputOption::VALUE_REQUIRED, 'The path to the cache file.'),
-                    new InputOption('diff', '', InputOption::VALUE_NONE, 'Also produce diff for each file.'),
-                    new InputOption('format', '', InputOption::VALUE_REQUIRED, 'To output results in other formats.'),
-                    new InputOption('stop-on-violation', '', InputOption::VALUE_NONE, 'Stop execution on first violation.'),
-                    new InputOption('show-progress', '', InputOption::VALUE_REQUIRED, 'Type of progress indicator (none, dots).'),
-                ]
-            )
-            ->setDescription('Fixes a directory or a file.')
-        ;
+        $this->setDefinition(
+            [
+                new InputArgument('path', InputArgument::IS_ARRAY, 'The path(s) that rules will be run against (each path can be a file or directory).'),
+                new InputOption('path-mode', '', InputOption::VALUE_REQUIRED, 'Specify path mode (can be `override` or `intersection`).', ConfigurationResolver::PATH_MODE_OVERRIDE),
+                new InputOption('allow-risky', '', InputOption::VALUE_REQUIRED, 'Are risky fixers allowed (can be `yes` or `no`).'),
+                new InputOption('config', '', InputOption::VALUE_REQUIRED, 'The path to a config file.'),
+                new InputOption('dry-run', '', InputOption::VALUE_NONE, 'Only shows which files would have been modified.'),
+                new InputOption('rules', '', InputOption::VALUE_REQUIRED, 'List of rules that should be run against configured paths.'),
+                new InputOption('using-cache', '', InputOption::VALUE_REQUIRED, 'Does cache should be used (can be `yes` or `no`).'),
+                new InputOption('cache-file', '', InputOption::VALUE_REQUIRED, 'The path to the cache file.'),
+                new InputOption('diff', '', InputOption::VALUE_NONE, 'Prints diff for each file.'),
+                new InputOption('format', '', InputOption::VALUE_REQUIRED, 'To output results in other formats.'),
+                new InputOption('stop-on-violation', '', InputOption::VALUE_NONE, 'Stop execution on first violation.'),
+                new InputOption('show-progress', '', InputOption::VALUE_REQUIRED, 'Type of progress indicator (none, dots).'),
+            ]
+        );
     }
 
     protected function execute(InputInterface $input, OutputInterface $output): int
@@ -227,7 +227,7 @@ final class FixCommand extends Command
             [
                 'allow-risky' => $input->getOption('allow-risky'),
                 'config' => $passedConfig,
-                'dry-run' => $input->getOption('dry-run'),
+                'dry-run' => $this->isDryRun($input),
                 'rules' => $passedRules,
                 'path' => $input->getArgument('path'),
                 'path-mode' => $input->getOption('path-mode'),
@@ -351,4 +351,9 @@ final class FixCommand extends Command
             \count($lintErrors) > 0
         );
     }
+
+    protected function isDryRun(InputInterface $input): bool
+    {
+        return $input->getOption('dry-run');
+    }
 }

+ 62 - 0
tests/Console/Command/CheckCommandTest.php

@@ -0,0 +1,62 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of PHP CS Fixer.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *     Dariusz Rumiński <dariusz.ruminski@gmail.com>
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace PhpCsFixer\Tests\Console\Command;
+
+use PhpCsFixer\Console\Application;
+use PhpCsFixer\Console\Command\CheckCommand;
+use PhpCsFixer\Tests\TestCase;
+use PhpCsFixer\ToolInfo;
+use Symfony\Component\Console\Output\OutputInterface;
+use Symfony\Component\Console\Tester\CommandTester;
+
+/**
+ * @internal
+ *
+ * @covers \PhpCsFixer\Console\Command\CheckCommand
+ */
+final class CheckCommandTest extends TestCase
+{
+    /**
+     * This test ensures that `--dry-run` option is not available in `check` command,
+     * because this command is a proxy for `fix` command which always set `--dry-run` during proxying,
+     * so it does not make sense to provide this option again.
+     */
+    public function testDryRunModeIsUnavailable(): void
+    {
+        $application = new Application();
+        $application->add(new CheckCommand(new ToolInfo()));
+
+        $command = $application->find('check');
+        $commandTester = new CommandTester($command);
+
+        $commandTester->execute(
+            [
+                'command' => $command->getName(),
+                '--help' => true,
+                'path' => [__FILE__], // just to get rid of Finder error
+            ],
+            [
+                'interactive' => false,
+                'decorated' => false,
+                'verbosity' => OutputInterface::VERBOSITY_DEBUG,
+            ]
+        );
+
+        self::assertStringNotContainsString(
+            '--dry-run',
+            $commandTester->getDisplay()
+        );
+    }
+}