Browse Source

FixCommand - add stop-on-violation option

Dariusz Ruminski 8 years ago
parent
commit
75e8715133

+ 3 - 1
README.rst

@@ -185,6 +185,8 @@ display a summary of proposed fixes, leaving your files unchanged.
 The ``--allow-risky`` option allows you to set whether risky rules may run. Default value is taken from config file.
 Risky rule is a rule, which could change code behaviour. By default no risky rules are run.
 
+The ``--stop-on-violation`` flag stops execution upon first file that needs to be fixed.
+
 The command can also read from standard input, in which case it won't
 automatically fix anything:
 
@@ -795,7 +797,7 @@ Then, add the following command to your CI:
 .. code-block:: bash
 
     $ IFS=$'\n'; COMMIT_SCA_FILES=($(git diff --name-only --diff-filter=ACMRTUXB "${COMMIT_RANGE}")); unset IFS
-    $ vendor/bin/php-cs-fixer fix --config=.php_cs.dist -v --dry-run --using-cache=no --path-mode=intersection "${COMMIT_SCA_FILES[@]}"
+    $ vendor/bin/php-cs-fixer fix --config=.php_cs.dist -v --dry-run --stop-on-violation --using-cache=no --path-mode=intersection "${COMMIT_SCA_FILES[@]}"
 
 Where ``$COMMIT_RANGE`` is your range of commits, eg ``$TRAVIS_COMMIT_RANGE`` or ``HEAD~..HEAD``.
 

+ 4 - 1
src/Console/Command/FixCommand.php

@@ -102,6 +102,7 @@ final class FixCommand extends Command
                     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.'),
                 )
             )
             ->setDescription('Fixes a directory or a file.')
@@ -132,6 +133,7 @@ final class FixCommand extends Command
                 'cache-file' => $input->getOption('cache-file'),
                 'format' => $input->getOption('format'),
                 'diff' => $input->getOption('diff'),
+                'stop-on-violation' => $input->getOption('stop-on-violation'),
                 'verbosity' => $verbosity,
             ),
             getcwd()
@@ -177,7 +179,8 @@ final class FixCommand extends Command
             $resolver->getLinter(),
             $resolver->isDryRun(),
             $resolver->getCacheManager(),
-            $resolver->getDirectory()
+            $resolver->getDirectory(),
+            $resolver->shouldStopOnViolation()
         );
 
         $progressOutput = $showProgress && $stdErr

+ 3 - 1
src/Console/Command/FixCommandHelp.php

@@ -83,6 +83,8 @@ display a summary of proposed fixes, leaving your files unchanged.
 The <comment>--allow-risky</comment> option allows you to set whether risky rules may run. Default value is taken from config file.
 Risky rule is a rule, which could change code behaviour. By default no risky rules are run.
 
+The <comment>--stop-on-violation<comment> flag stops execution upon first file that needs to be fixed.
+
 The command can also read from standard input, in which case it won't
 automatically fix anything:
 
@@ -208,7 +210,7 @@ Require ``friendsofphp/php-cs-fixer`` as a ``dev`` dependency:
 Then, add the following command to your CI:
 
     $ IFS=\$'\\n'; COMMIT_SCA_FILES=($(git diff --name-only --diff-filter=ACMRTUXB "\${COMMIT_RANGE}")); unset IFS
-    $ vendor/bin/php-cs-fixer fix --config=.php_cs.dist -v --dry-run --using-cache=no --path-mode=intersection "\${COMMIT_SCA_FILES[@]}"
+    $ vendor/bin/php-cs-fixer fix --config=.php_cs.dist -v --dry-run --stop-on-violation --using-cache=no --path-mode=intersection "\${COMMIT_SCA_FILES[@]}"
 
 Where ``\$COMMIT_RANGE`` is your range of commits, eg ``\$TRAVIS_COMMIT_RANGE`` or ``HEAD~..HEAD``.
 

+ 6 - 0
src/Console/ConfigurationResolver.php

@@ -113,6 +113,7 @@ final class ConfigurationResolver
         'rules' => null,
         'diff' => null,
         'verbosity' => null,
+        'stop-on-violation' => null,
     );
 
     private $cacheFile;
@@ -471,6 +472,11 @@ final class ConfigurationResolver
         return $this->isDryRun;
     }
 
+    public function shouldStopOnViolation()
+    {
+        return $this->options['stop-on-violation'];
+    }
+
     /**
      * Compute file candidates for config file.
      *

+ 15 - 4
src/Runner/Runner.php

@@ -78,6 +78,11 @@ final class Runner
      */
     private $fixers;
 
+    /**
+     * @var bool
+     */
+    private $stopOnViolation;
+
     public function __construct(
         $finder,
         array $fixers,
@@ -87,7 +92,8 @@ final class Runner
         LinterInterface $linter,
         $isDryRun,
         CacheManagerInterface $cacheManager,
-        DirectoryInterface $directory = null
+        DirectoryInterface $directory = null,
+        $stopOnViolation = false
     ) {
         $this->finder = $finder;
         $this->fixers = $fixers;
@@ -98,6 +104,7 @@ final class Runner
         $this->isDryRun = $isDryRun;
         $this->cacheManager = $cacheManager;
         $this->directory = $directory ?: new Directory('');
+        $this->stopOnViolation = $stopOnViolation;
     }
 
     /**
@@ -122,13 +129,17 @@ final class Runner
         foreach ($collection as $file) {
             $fixInfo = $this->fixFile($file, $collection->currentLintingResult());
 
+            // we do not need Tokens to still caching just fixed file - so clear the cache
+            Tokens::clearCache();
+
             if ($fixInfo) {
                 $name = $this->directory->getRelativePathTo($file);
                 $changed[$name] = $fixInfo;
-            }
 
-            // we do not need Tokens to still caching just fixed file - so clear the cache
-            Tokens::clearCache();
+                if ($this->stopOnViolation) {
+                    break;
+                }
+            }
         }
 
         return $changed;

+ 3 - 1
tests/Console/ConfigurationResolverTest.php

@@ -915,7 +915,7 @@ final class ConfigurationResolverTest extends \PHPUnit_Framework_TestCase
 
         $options = $definition->getOptions();
         $this->assertSame(
-            array('path-mode', 'allow-risky', 'config', 'dry-run', 'rules', 'using-cache', 'cache-file', 'diff', 'format'),
+            array('path-mode', 'allow-risky', 'config', 'dry-run', 'rules', 'using-cache', 'cache-file', 'diff', 'format', 'stop-on-violation'),
             array_keys($options),
             'Expected options mismatch, possibly test needs updating.'
         );
@@ -931,10 +931,12 @@ final class ConfigurationResolverTest extends \PHPUnit_Framework_TestCase
                 'using-cache' => false,
                 'diff' => true,
                 'format' => 'json',
+                'stop-on-violation' => true,
             ),
             ''
         );
 
+        $this->assertTrue($resolver->shouldStopOnViolation());
         $this->assertTrue($resolver->getRiskyAllowed());
         $this->assertTrue($resolver->isDryRun());
         $this->assertSame(array('php_unit_construct' => true), $resolver->getRules());

+ 10 - 0
tests/Fixtures/FixerTest/fix/somefile2.php

@@ -0,0 +1,10 @@
+<?php
+
+namespace Test\AAaa;
+
+class test
+{
+    var $testA;
+    public $test_B;
+        public $testC;
+}

+ 26 - 10
tests/Runner/RunnerTest.php

@@ -50,11 +50,10 @@ final class RunnerTest extends \PHPUnit_Framework_TestCase
             new Fixer\Import\NoUnusedImportsFixer(), // will be ignored cause of test keyword in namespace
         );
 
-        foreach ($fixers as $fixer) {
-            if ($fixer instanceof Fixer\ConfigurableFixerInterface) {
-                $fixer->configure(null);
-            }
-        }
+        $expectedChangedInfo = array(
+            'appliedFixers' => array('visibility_required'),
+            'diff' => '',
+        );
 
         $path = __DIR__.DIRECTORY_SEPARATOR.'..'.DIRECTORY_SEPARATOR.'Fixtures'.DIRECTORY_SEPARATOR.'FixerTest'.DIRECTORY_SEPARATOR.'fix';
         $runner = new Runner(
@@ -66,17 +65,34 @@ final class RunnerTest extends \PHPUnit_Framework_TestCase
             $linterProphecy->reveal(),
             true,
             new NullCacheManager(),
-            new Directory($path)
+            new Directory($path),
+            false
         );
 
         $changed = $runner->fix();
 
-        $pathToInvalidFile = 'somefile.php';
+        $this->assertCount(2, $changed);
+        $this->assertArraySubset($expectedChangedInfo, array_pop($changed));
+        $this->assertArraySubset($expectedChangedInfo, array_pop($changed));
+
+        $path = __DIR__.DIRECTORY_SEPARATOR.'..'.DIRECTORY_SEPARATOR.'Fixtures'.DIRECTORY_SEPARATOR.'FixerTest'.DIRECTORY_SEPARATOR.'fix';
+        $runner = new Runner(
+            Finder::create()->in($path),
+            $fixers,
+            new NullDiffer(),
+            null,
+            new ErrorsManager(),
+            $linterProphecy->reveal(),
+            true,
+            new NullCacheManager(),
+            new Directory($path),
+            true
+        );
+
+        $changed = $runner->fix();
 
         $this->assertCount(1, $changed);
-        $this->assertCount(2, $changed[$pathToInvalidFile]);
-        $this->assertSame(array('appliedFixers', 'diff'), array_keys($changed[$pathToInvalidFile]));
-        $this->assertSame('visibility_required', $changed[$pathToInvalidFile]['appliedFixers'][0]);
+        $this->assertArraySubset($expectedChangedInfo, array_pop($changed));
     }
 
     /**