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

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/Exception/Configuration/RectorRuleShouldNotBeAppliedException.php

@@ -0,0 +1,11 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Exception\Configuration;
+
+use Exception;
+
+final class RectorRuleShouldNotBeAppliedException extends Exception
+{
+}

src/Testing/PHPUnit/AbstractRectorTestCase.php

@@ -18,6 +18,7 @@
 use Rector\Contract\DependencyInjection\ResetableInterface;
 use Rector\Contract\Rector\RectorInterface;
 use Rector\DependencyInjection\Laravel\ContainerMemento;
+use Rector\Exception\Configuration\RectorRuleShouldNotBeAppliedException;
 use Rector\Exception\ShouldNotHappenException;
 use Rector\NodeTypeResolver\Reflection\BetterReflection\SourceLocatorProvider\DynamicSourceLocatorProvider;
 use Rector\PhpParser\NodeTraverser\RectorNodeTraverser;
@@ -207,18 +208,22 @@ private function doTestFileMatchesExpectedContent(
         $changedContents = $rectorTestResult->getChangedContents();
 
         $fixtureFilename = basename($fixtureFilePath);
-        $failureMessage = sprintf('Failed on fixture file "%s"', $fixtureFilename);
+        $fixtureNameMessage = sprintf('Failed on fixture file "%s"', $fixtureFilename);
 
+        $failureMessage = $fixtureNameMessage;
+        $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 +232,12 @@ 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 = $fixtureNameMessage . PHP_EOL . PHP_EOL
+                . 'File not changed but some Rector rules applied:' . PHP_EOL . $appliedRulesList;
+            throw new RectorRuleShouldNotBeAppliedException($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

@@ -6,13 +6,15 @@
 
 use Iterator;
 use PHPUnit\Framework\Attributes\DataProvider;
+use Rector\Exception\Configuration\RectorRuleShouldNotBeAppliedException;
 use Rector\Testing\PHPUnit\AbstractRectorTestCase;
 
 final class ForeachValueScopeTest extends AbstractRectorTestCase
 {
     #[DataProvider('provideData')]
     public function test(string $filePath): void
     {
+        $this->expectException(RectorRuleShouldNotBeAppliedException::class);
         $this->doTestFile($filePath);
     }
 

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,36 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Testing\RectorRuleShouldNotBeApplied;
+
+use Iterator;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Rector\Exception\Configuration\RectorRuleShouldNotBeAppliedException;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+
+final class RectorRuleShouldNotBeAppliedTest extends AbstractRectorTestCase
+{
+    #[DataProvider('provideData')]
+    public function test(string $filePath): void
+    {
+        $this->expectException(RectorRuleShouldNotBeAppliedException::class);
+        $this->expectExceptionMessage(
+            'Failed on fixture file "no_change.php.inc"' . PHP_EOL . PHP_EOL
+            . 'File not changed but some Rector rules applied:' . PHP_EOL
+            . ' * Rector\Tests\Testing\RectorRuleShouldNotBeApplied\Source\NoChangeRector'
+        );
+
+        $this->doTestFile($filePath);
+    }
+
+    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]);