[dx] add test to check that a rule is not applied if it does not change the code (#6794)

* [dx] add test to check that a rule is not applied if it does not change the code

* Print warning instead of throwing an error

Author: carlos-granados

src/Application/FileProcessor.php

@@ -85,7 +85,10 @@ public function processFile(File $file, Configuration $configuration): FileProce
             // when changed, set final status changed to true
             // to ensure it make sense to verify in next process when needed
             $file->changeHasChanged(true);
+        }
 
+        $rectorWithLineChanges = $file->getRectorWithLineChanges();
+        if ($file->hasChanged() || $rectorWithLineChanges !== []) {
             $currentFileDiff = $this->fileDiffFactory->createFileDiffWithLineChanges(
                 $configuration->shouldShowDiffs(),
                 $file,

src/Testing/PHPUnit/AbstractRectorTestCase.php

@@ -165,6 +165,23 @@ protected function doTestFile(string $fixtureFilePath): void
         );
     }
 
+    protected function doTestFileExpectingWarningAboutRuleApplied(
+        string $fixtureFilePath,
+        string $expectedRuleApplied
+    ): void {
+        ob_start();
+        $this->doTestFile($fixtureFilePath);
+        $content = ob_get_clean();
+        $fixtureName = basename($fixtureFilePath);
+        $testClass = static::class;
+        $this->assertSame(
+            PHP_EOL . "WARNING: On fixture file \"" . $fixtureName . '" for test "' . $testClass . "\"" . PHP_EOL .
+            "File not changed but some Rector rules applied:" . PHP_EOL .
+            ' * ' . $expectedRuleApplied . PHP_EOL,
+            $content
+        );
+    }
+
     private function forgetRectorsRules(): void
     {
         $rectorConfig = self::getContainer();
@@ -209,16 +226,19 @@ private function doTestFileMatchesExpectedContent(
         $fixtureFilename = basename($fixtureFilePath);
         $failureMessage = sprintf('Failed on fixture file "%s"', $fixtureFilename);
 
+        $numAppliedRectorClasses = count($rectorTestResult->getAppliedRectorClasses());
         // give more context about used rules in case of set testing
-        if (count($rectorTestResult->getAppliedRectorClasses()) > 1) {
-            $failureMessage .= PHP_EOL . PHP_EOL;
-            $failureMessage .= 'Applied Rector rules:' . PHP_EOL;
-
+        $appliedRulesList = '';
+        if ($numAppliedRectorClasses > 0) {
             foreach ($rectorTestResult->getAppliedRectorClasses() as $appliedRectorClass) {
-                $failureMessage .= ' * ' . $appliedRectorClass . PHP_EOL;
+                $appliedRulesList .= ' * ' . $appliedRectorClass . PHP_EOL;
             }
         }
 
+        if ($numAppliedRectorClasses > 1) {
+            $failureMessage .= PHP_EOL . PHP_EOL . 'Applied Rector rules:' . PHP_EOL . $appliedRulesList;
+        }
+
         try {
             $this->assertSame($expectedFileContents, $changedContents, $failureMessage);
         } catch (ExpectationFailedException) {
@@ -227,6 +247,16 @@ private function doTestFileMatchesExpectedContent(
             // if not exact match, check the regex version (useful for generated hashes/uuids in the code)
             $this->assertStringMatchesFormat($expectedFileContents, $changedContents, $failureMessage);
         }
+
+        if ($inputFileContents === $expectedFileContents && $numAppliedRectorClasses > 0) {
+            $failureMessage = PHP_EOL . sprintf(
+                'WARNING: On fixture file "%s" for test "%s"',
+                $fixtureFilename,
+                static::class
+            ) . PHP_EOL
+                . 'File not changed but some Rector rules applied:' . PHP_EOL . $appliedRulesList;
+            echo $failureMessage;
+        }
     }
 
     private function processFilePath(string $filePath): RectorTestResult

src/Testing/PHPUnit/ValueObject/RectorTestResult.php

@@ -30,7 +30,7 @@ public function getAppliedRectorClasses(): array
     {
         $rectorClasses = [];
 
-        foreach ($this->processResult->getFileDiffs() as $fileDiff) {
+        foreach ($this->processResult->getFileDiffs(false) as $fileDiff) {
             $rectorClasses = array_merge($rectorClasses, $fileDiff->getRectorClasses());
         }
 

src/ValueObject/ProcessResult.php

@@ -33,8 +33,12 @@ public function getSystemErrors(): array
     /**
      * @return FileDiff[]
      */
-    public function getFileDiffs(): array
+    public function getFileDiffs(bool $onlyWithChanges = true): array
     {
+        if ($onlyWithChanges) {
+            return array_filter($this->fileDiffs, fn (FileDiff $fileDiff): bool => $fileDiff->getDiff() !== '');
+        }
+
         return $this->fileDiffs;
     }
 

tests/Issues/ScopeNotAvailable/ForeachValueScopeTest.php

@@ -7,13 +7,14 @@
 use Iterator;
 use PHPUnit\Framework\Attributes\DataProvider;
 use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+use Rector\Tests\Issues\ScopeNotAvailable\Variable\ArrayItemForeachValueRector;
 
 final class ForeachValueScopeTest extends AbstractRectorTestCase
 {
     #[DataProvider('provideData')]
     public function test(string $filePath): void
     {
-        $this->doTestFile($filePath);
+        $this->doTestFileExpectingWarningAboutRuleApplied($filePath, ArrayItemForeachValueRector::class);
     }
 
     public static function provideData(): Iterator

tests/Testing/RectorRuleShouldNotBeApplied/Fixture/no_change.php.inc

@@ -0,0 +1,11 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Testing\RectorRuleShouldNotBeApplied;
+
+class NoChange
+{
+}
+
+?>

tests/Testing/RectorRuleShouldNotBeApplied/RectorRuleShouldNotBeAppliedTest.php

@@ -0,0 +1,29 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Testing\RectorRuleShouldNotBeApplied;
+
+use Iterator;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+use Rector\Tests\Testing\RectorRuleShouldNotBeApplied\Source\NoChangeRector;
+
+final class RectorRuleShouldNotBeAppliedTest extends AbstractRectorTestCase
+{
+    #[DataProvider('provideData')]
+    public function test(string $filePath): void
+    {
+        $this->doTestFileExpectingWarningAboutRuleApplied($filePath, NoChangeRector::class);
+    }
+
+    public static function provideData(): Iterator
+    {
+        return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
+    }
+
+    public function provideConfigFilePath(): string
+    {
+        return __DIR__ . '/config/configured_rule.php';
+    }
+}

tests/Testing/RectorRuleShouldNotBeApplied/Source/NoChangeRector.php

@@ -0,0 +1,30 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Testing\RectorRuleShouldNotBeApplied\Source;
+
+use PhpParser\Node;
+use PhpParser\Node\Stmt\Class_;
+use Rector\Rector\AbstractRector;
+use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
+
+final class NoChangeRector extends AbstractRector
+{
+    public function getRuleDefinition(): RuleDefinition
+    {
+        return new RuleDefinition('no change', []);
+    }
+
+    public function getNodeTypes(): array
+    {
+        return [
+            Class_::class,
+        ];
+    }
+
+    public function refactor(Node $node)
+    {
+        return $node;
+    }
+}

tests/Testing/RectorRuleShouldNotBeApplied/config/configured_rule.php

@@ -0,0 +1,9 @@
+<?php
+
+declare(strict_types=1);
+
+use Rector\Config\RectorConfig;
+use Rector\Tests\Testing\RectorRuleShouldNotBeApplied\Source\NoChangeRector;
+
+return RectorConfig::configure()
+    ->withRules([NoChangeRector::class]);