Force the verification re-run to a dry run, so it also runs on apply runs

The verifier re-processes skipped files only to read which rules would
change them. Building its own dry-run Configuration keeps FileProcessor
from writing the skipped file, so the check is safe outside a dry run
too. Drop the dry-run gate in ProcessCommand.

Author: TomasVotruba

src/Console/Command/ProcessCommand.php

@@ -202,10 +202,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         $this->missConfigurationReporter->reportSkippedNonRectorClasses();
         $this->missConfigurationReporter->reportUnusedSkips($processResult);
 
-        // re-runs skipped rules on their files, so only safe on a dry run that never writes
-        if ($configuration->isDryRun()) {
-            $this->missConfigurationReporter->reportUnnecessarySkips($configuration);
-        }
+        // the verification re-run is forced to a dry run, so it is safe on an apply run too
+        $this->missConfigurationReporter->reportUnnecessarySkips();
 
         return $this->resolveReturnCode($processResult, $configuration);
     }

src/Reporting/MissConfigurationReporter.php

@@ -9,7 +9,6 @@
 use Rector\Configuration\VendorMissAnalyseGuard;
 use Rector\Contract\Rector\RectorInterface;
 use Rector\PostRector\Contract\Rector\PostRectorInterface;
-use Rector\ValueObject\Configuration;
 use Rector\ValueObject\ProcessResult;
 use Symfony\Component\Console\Style\SymfonyStyle;
 
@@ -51,12 +50,12 @@ public function reportUnusedSkips(ProcessResult $processResult): void
 
     /**
      * Reports rule-scoped skips that matched a file but whose rule would not have changed it, so the
-     * skip is unnecessary. Enabled with "->reportUnusedSkips()" and only on dry runs, as it re-runs
-     * the rules on the skipped files.
+     * skip is unnecessary. Enabled with "->reportUnusedSkips()". The re-run is forced to a dry run,
+     * so it is safe on an apply run too.
      */
-    public function reportUnnecessarySkips(Configuration $configuration): void
+    public function reportUnnecessarySkips(): void
     {
-        $unnecessarySkips = $this->neededSkipVerifier->verify($configuration);
+        $unnecessarySkips = $this->neededSkipVerifier->verify();
         if ($unnecessarySkips === []) {
             return;
         }

src/Reporting/NeededSkipVerifier.php

@@ -14,8 +14,8 @@
 
 /**
  * Re-runs each rule-scoped skip on its exact file with skipping off. A skip whose rule still does
- * not change the file never prevented anything and can be removed. Runs on dry runs only, so the
- * re-processing never writes to disk.
+ * not change the file never prevented anything and can be removed. The re-processing is forced to a
+ * dry run, so it never writes, no matter the mode of the real run.
  *
  * @see \Rector\Tests\Reporting\NeededSkipVerifierTest
  */
@@ -34,14 +34,14 @@ public function __construct(
      *
      * @return string[]
      */
-    public function verify(Configuration $configuration): array
+    public function verify(): array
     {
         $skippedFilePathsByRule = $this->skipper->provideSkippedFilePathsByRule();
         if ($skippedFilePathsByRule === []) {
             return [];
         }
 
-        $changedRulesByFilePath = $this->resolveChangedRulesByFilePath($skippedFilePathsByRule, $configuration);
+        $changedRulesByFilePath = $this->resolveChangedRulesByFilePath($skippedFilePathsByRule);
 
         $unnecessarySkips = [];
         foreach ($skippedFilePathsByRule as $rectorClass => $filePaths) {
@@ -75,7 +75,7 @@ public function verify(Configuration $configuration): array
      * @param array<class-string, string[]> $skippedFilePathsByRule
      * @return array<string, string[]> real file path => rule classes that changed it
      */
-    private function resolveChangedRulesByFilePath(array $skippedFilePathsByRule, Configuration $configuration): array
+    private function resolveChangedRulesByFilePath(array $skippedFilePathsByRule): array
     {
         $uniqueFilePaths = [];
         foreach ($skippedFilePathsByRule as $filePaths) {
@@ -84,6 +84,9 @@ private function resolveChangedRulesByFilePath(array $skippedFilePathsByRule, Co
             }
         }
 
+        // force a dry run, so re-processing detects the change but never writes the skipped file
+        $dryRunConfiguration = new Configuration(isDryRun: true);
+
         $this->skipper->startVerifyingSkips();
 
         $changedRulesByFilePath = [];
@@ -92,7 +95,7 @@ private function resolveChangedRulesByFilePath(array $skippedFilePathsByRule, Co
                 $file = new File($filePath, FileSystem::read($filePath));
                 $this->currentFileProvider->setFile($file);
 
-                $fileProcessResult = $this->fileProcessor->processFile($file, $configuration);
+                $fileProcessResult = $this->fileProcessor->processFile($file, $dryRunConfiguration);
 
                 // a broken file is not verifiable, leave it out so its skips are kept
                 if ($fileProcessResult->getSystemErrors() !== []) {

tests/Reporting/NeededSkipVerifierTest.php

@@ -13,7 +13,6 @@
 use Rector\Skipper\Skipper\Skipper;
 use Rector\Testing\PHPUnit\AbstractLazyTestCase;
 use Rector\Tests\Reporting\Source\MakeClassFinalRector;
-use Rector\ValueObject\Configuration;
 
 final class NeededSkipVerifierTest extends AbstractLazyTestCase
 {
@@ -58,21 +57,24 @@ public function testReportsSkipThatRuleWouldNotHaveChanged(): void
         $this->assertTrue($skipper->shouldSkipRectorAndFile($rule, $unnecessaryFilePath));
 
         $neededSkipVerifier = $rectorConfig->make(NeededSkipVerifier::class);
-        $unnecessarySkips = $neededSkipVerifier->verify(new Configuration(isDryRun: true));
+        $unnecessarySkips = $neededSkipVerifier->verify();
 
         // one rule group, listing only the unnecessary file
         $this->assertCount(1, $unnecessarySkips);
         $this->assertStringContainsString(MakeClassFinalRector::class, $unnecessarySkips[0]);
         $this->assertStringContainsString('unnecessary_skip.php', $unnecessarySkips[0]);
         $this->assertStringNotContainsString('needed_skip.php', $unnecessarySkips[0]);
+
+        // the verification is forced to a dry run, so the skipped file on disk stays untouched
+        $this->assertSame('<?php class SomeClass {}', FileSystem::read($neededFilePath));
     }
 
     public function testReportsNothingWithoutCapturedSkips(): void
     {
         $rectorConfig = $this->createContainerWithRule();
 
         $neededSkipVerifier = $rectorConfig->make(NeededSkipVerifier::class);
-        $this->assertSame([], $neededSkipVerifier->verify(new Configuration(isDryRun: true)));
+        $this->assertSame([], $neededSkipVerifier->verify());
     }
 
     private function createContainerWithRule(): RectorConfig