Support for --stop-on-violation in parallel mode

PHP-CS-Fixer · May 8, 2024 · e635078 · e635078
1 parent 1e2f05f
commit e635078
Show file tree

Hide file tree

Showing 6 changed files with 62 additions and 9 deletions.
diff --git a/src/Console/Command/WorkerCommand.php b/src/Console/Command/WorkerCommand.php
@@ -86,6 +86,7 @@ protected function configure(): void
                 new InputOption('using-cache', '', InputOption::VALUE_REQUIRED, 'Should cache 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('stop-on-violation', '', InputOption::VALUE_NONE, 'Stop execution on first violation.'),
             ]
         );
     }
@@ -217,7 +218,7 @@ private function createRunner(InputInterface $input): Runner
                 'using-cache' => $input->getOption('using-cache'),
                 'cache-file' => $input->getOption('cache-file'),
                 'diff' => $input->getOption('diff'),
-                'stop-on-violation' => false, // @TODO Pass this option to the runner
+                'stop-on-violation' => $input->getOption('stop-on-violation'),
             ],
             getcwd(), // @phpstan-ignore-line
             $this->toolInfo

diff --git a/src/Runner/Parallel/ProcessFactory.php b/src/Runner/Parallel/ProcessFactory.php
@@ -91,6 +91,10 @@ public function getCommandArgs(int $serverPort, ProcessIdentifier $identifier, R
             $commandArgs[] = '--diff';
         }
 
+        if (filter_var($this->input->getOption('stop-on-violation'), FILTER_VALIDATE_BOOLEAN)) {
+            $commandArgs[] = '--stop-on-violation';
+        }
+
         foreach (['allow-risky', 'config', 'rules', 'using-cache', 'cache-file'] as $option) {
             $optionValue = $this->input->getOption($option);
 

diff --git a/src/Runner/Parallel/ProcessPool.php b/src/Runner/Parallel/ProcessPool.php
@@ -69,7 +69,7 @@ public function endProcessIfKnown(ProcessIdentifier $identifier): void
     public function endAll(): void
     {
         foreach (array_keys($this->processes) as $identifier) {
-            $this->endProcess(ProcessIdentifier::fromRaw($identifier));
+            $this->endProcessIfKnown(ProcessIdentifier::fromRaw($identifier));
         }
     }
 

diff --git a/src/Runner/Runner.php b/src/Runner/Runner.php
@@ -247,17 +247,26 @@ function (array $workerResponse) use ($processPool, $process, $identifier, $getF
                         $fileAbsolutePath = $workerResponse['file'];
                         $fileRelativePath = $this->directory->getRelativePathTo($fileAbsolutePath);
 
-                        // Pass-back information about applied changes (only if there are any)
-                        if (isset($workerResponse['fixInfo'])) {
-                            $changed[$fileRelativePath] = $workerResponse['fixInfo'];
-                        }
+                        // Dispatch an event for each file processed and dispatch its status (required for progress output)
+                        $this->dispatchEvent(
+                            FixerFileProcessedEvent::NAME,
+                            new FixerFileProcessedEvent($workerResponse['status'])
+                        );
 
                         if (isset($workerResponse['fileHash'])) {
                             $this->cacheManager->setFileHash($fileRelativePath, $workerResponse['fileHash']);
                         }
 
-                        // Dispatch an event for each file processed and dispatch its status (required for progress output)
-                        $this->dispatchEvent(FixerFileProcessedEvent::NAME, new FixerFileProcessedEvent($workerResponse['status']));
+                        // Pass-back information about applied changes (only if there are any)
+                        if (isset($workerResponse['fixInfo'])) {
+                            $changed[$fileRelativePath] = $workerResponse['fixInfo'];
+
+                            if ($this->stopOnViolation) {
+                                $processPool->endAll();
+
+                                return;
+                            }
+                        }
 
                         foreach ($workerResponse['errors'] ?? [] as $error) {
                             $this->errorsManager->report(new Error(

diff --git a/tests/Runner/Parallel/ProcessFactoryTest.php b/tests/Runner/Parallel/ProcessFactoryTest.php
@@ -105,6 +105,8 @@ public static function provideCreateCases(): iterable
 
         yield 'diff enabled' => [['--diff' => true], self::createRunnerConfig(false), '--diff'];
 
+        yield 'stop-on-violation enabled' => [['--stop-on-violation' => true], self::createRunnerConfig(false), '--stop-on-violation'];
+
         yield 'allow risky' => [['--allow-risky' => 'yes'], self::createRunnerConfig(false), '--allow-risky \'yes\''];
 
         yield 'config' => [['--config' => 'foo.php'], self::createRunnerConfig(false), '--config \'foo.php\''];
@@ -124,9 +126,10 @@ public static function provideCreateCases(): iterable
                 '--config' => 'conf.php',
                 '--diff' => true,
                 '--using-cache' => 'yes',
+                '--stop-on-violation' => true,
             ],
             self::createRunnerConfig(true),
-            '--dry-run --diff --config \'conf.php\' --using-cache \'yes\'',
+            '--dry-run --diff --stop-on-violation --config \'conf.php\' --using-cache \'yes\'',
         ];
     }
 

diff --git a/tests/Runner/RunnerTest.php b/tests/Runner/RunnerTest.php
@@ -181,6 +181,42 @@ public function testThatParallelFixOfInvalidFileReportsToErrorManager(): void
         self::assertSame($pathToInvalidFile, $error->getFilePath());
     }
 
+    /**
+     * @requires OS Darwin|Windows
+     * @TODO Fix it for Linux - somehow this test reaches timeout.
+     *
+     * @covers \PhpCsFixer\Runner\Runner::fix
+     * @covers \PhpCsFixer\Runner\Runner::fixFile
+     * @covers \PhpCsFixer\Runner\Runner::fixParallel
+     */
+    public function testThatParallelFixStopsOnFirstViolationIfSuchOptionIsEnabled(): void
+    {
+        $errorsManager = new ErrorsManager();
+
+        $path = realpath(__DIR__.\DIRECTORY_SEPARATOR.'..').\DIRECTORY_SEPARATOR.'Fixtures'.\DIRECTORY_SEPARATOR.'FixerTest'.\DIRECTORY_SEPARATOR.'fix';
+        $runner = new Runner(
+            Finder::create()->in($path),
+            [
+                new Fixer\ClassNotation\VisibilityRequiredFixer(),
+                new Fixer\Import\NoUnusedImportsFixer(), // will be ignored cause of test keyword in namespace
+            ],
+            new NullDiffer(),
+            null,
+            $errorsManager,
+            new Linter(),
+            true,
+            new NullCacheManager(),
+            null,
+            true, // <-- IMPORTANT!
+            new ParallelConfig(2, 1, 50),
+            new ArrayInput([], (new FixCommand(new ToolInfo()))->getDefinition())
+        );
+        $changed = $runner->fix();
+
+        // Without "stop on violation" there would be 2 files changed, but in this case it should stop after first one
+        self::assertCount(1, $changed);
+    }
+
     /**
      * @covers \PhpCsFixer\Runner\Runner::fix
      * @covers \PhpCsFixer\Runner\Runner::fixFile