[testing] Rework AbstractRunnableRectorTestCase to be part of main test + fix ListEachRector behavior

composer.json

@@ -43,14 +43,14 @@
         "friendsofphp/php-cs-fixer": "^2.16",
         "ocramius/package-versions": "^1.4|^1.5",
         "phpunit/phpunit": "^8.5|^9.0",
+        "psr/event-dispatcher": "^1.0",
         "slam/phpstan-extensions": "^5.0",
+        "slevomat/coding-standard": "dev-master",
         "symplify/changelog-linker": "^8.0",
         "symplify/easy-coding-standard": "^8.0",
         "symplify/monorepo-builder": "^8.0",
         "symplify/phpstan-extensions": "^8.0",
-        "thecodingmachine/phpstan-strict-rules": "^0.12",
-        "psr/event-dispatcher": "^1.0",
-        "slevomat/coding-standard": "dev-master"
+        "thecodingmachine/phpstan-strict-rules": "^0.12"
     },
     "replace": {
         "rector/rector-prefixed": "self.version"

docs/rector_rules_overview.md

@@ -7371,8 +7371,9 @@ each() function is deprecated, use key() and current() instead
 
 ```diff
 -list($key, $callback) = each($callbacks);
-+$key = key($opt->option);
-+$val = current($opt->option);
++$key = key($callbacks);
++$callback = current($callbacks);
++next($callbacks);
 ```
 
 <br>

rules/code-quality/tests/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector/ArrayKeysAndInArrayToArrayKeyExistsRectorTest.php

@@ -6,17 +6,16 @@
 
 use Iterator;
 use Rector\CodeQuality\Rector\FuncCall\ArrayKeysAndInArrayToArrayKeyExistsRector;
-use Rector\Core\Testing\PHPUnit\AbstractRunnableRectorTestCase;
+use Rector\Core\Testing\PHPUnit\AbstractRectorTestCase;
 
-final class ArrayKeysAndInArrayToArrayKeyExistsRectorTest extends AbstractRunnableRectorTestCase
+final class ArrayKeysAndInArrayToArrayKeyExistsRectorTest extends AbstractRectorTestCase
 {
     /**
      * @dataProvider provideData()
      */
     public function test(string $file): void
     {
         $this->doTestFile($file);
-        $this->assertOriginalAndFixedFileResultEquals($file);
     }
 
     public function provideData(): Iterator

rules/code-quality/tests/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector/Fixture/fixture.php.inc

@@ -2,16 +2,17 @@
 
 namespace Rector\CodeQuality\Tests\Rector\FuncCall\ArrayKeysAndInArrayToArrayKeyExistsRector\Fixture;
 
-use Rector\Core\Testing\PHPUnit\RunnableInterface;
+use Rector\Core\Testing\Contract\RunnableInterface;
 
 class SomeClass implements RunnableInterface
 {
     public function run()
     {
         $packageName = "foo";
-        $values      = ["foo" => "bar"];
-        
+        $values = ["foo" => "bar"];
+
         $keys = array_keys($values);
+
         return in_array($packageName, $keys, true);
     }
 }
@@ -22,14 +23,15 @@ class SomeClass implements RunnableInterface
 
 namespace Rector\CodeQuality\Tests\Rector\FuncCall\ArrayKeysAndInArrayToArrayKeyExistsRector\Fixture;
 
-use Rector\Core\Testing\PHPUnit\RunnableInterface;
+use Rector\Core\Testing\Contract\RunnableInterface;
 
 class SomeClass implements RunnableInterface
 {
     public function run()
     {
         $packageName = "foo";
-        $values      = ["foo" => "bar"];
+        $values = ["foo" => "bar"];
+
         return array_key_exists($packageName, $values);
     }
 }

rules/nette-kdyby/tests/Rector/MethodCall/ReplaceEventManagerWithEventSubscriberRector/ReplaceEventManagerWithEventSubscriberRectorTest.php

@@ -13,7 +13,7 @@ public function test(): void
     {
         $this->doTestFile(__DIR__ . '/Fixture/fixture.php.inc');
 
-        $expectedEventFilePath = dirname($this->originalTempFile) . '/Event/SomeClassCopyEvent.php';
+        $expectedEventFilePath = $this->originalTempFileInfo->getPath() . '/Event/SomeClassCopyEvent.php';
 
         $this->assertFileExists($expectedEventFilePath);
         $this->assertFileEquals(__DIR__ . '/Source/ExpectedSomeClassCopyEvent.php', $expectedEventFilePath);

rules/nette-kdyby/tests/Rector/MethodCall/ReplaceMagicPropertyEventWithEventClassRector/ReplaceMagicPropertyEventWithEventClassRectorTest.php

@@ -18,7 +18,7 @@ public function testSimpleEvent(): void
     {
         $this->doTestFile(__DIR__ . '/Fixture/simple_event.php.inc');
 
-        $expectedEventFilePath = dirname($this->originalTempFile) . '/Event/FileManagerUploadEvent.php';
+        $expectedEventFilePath = $this->originalTempFileInfo->getPath() . '/Event/FileManagerUploadEvent.php';
         $this->assertFileExists($expectedEventFilePath);
         $this->assertFileEquals(__DIR__ . '/Source/ExpectedFileManagerUploadEvent.php', $expectedEventFilePath);
     }
@@ -27,7 +27,7 @@ public function testDuplicatedEventParams(): void
     {
         $this->doTestFile(__DIR__ . '/Fixture/duplicated_event_params.php.inc');
 
-        $expectedEventFilePath = dirname($this->originalTempFile) . '/Event/DuplicatedEventParamsUploadEvent.php';
+        $expectedEventFilePath = $this->originalTempFileInfo->getPath() . '/Event/DuplicatedEventParamsUploadEvent.php';
         $this->assertFileExists($expectedEventFilePath);
         $this->assertFileEquals(
             __DIR__ . '/Source/ExpectedDuplicatedEventParamsUploadEvent.php',

rules/php71/tests/Rector/BinaryOp/IsIterableRector/Fixture/polyfill_function.php.inc

@@ -4,7 +4,7 @@ namespace Rector\Php71\Tests\Rector\BinaryOp\IsIterableRector\Fixture;
 
 use Traversable;
 
-class PolyfillFunction
+class IsIterablePolyfillFunction
 {
     public function run($foo)
     {
@@ -28,7 +28,7 @@ namespace Rector\Php71\Tests\Rector\BinaryOp\IsIterableRector\Fixture;
 
 use Traversable;
 
-class PolyfillFunction
+class IsIterablePolyfillFunction
 {
     public function run($foo)
     {

rules/php72/src/Rector/Each/ListEachRector.php

@@ -8,7 +8,6 @@
 use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\FuncCall;
 use PhpParser\Node\Expr\List_;
-use PhpParser\Node\Stmt\Do_;
 use PhpParser\Node\Stmt\Expression;
 use Rector\Core\PhpParser\Node\Manipulator\AssignManipulator;
 use Rector\Core\Rector\AbstractRector;
@@ -44,8 +43,9 @@ public function getDefinition(): RectorDefinition
 PHP
                     ,
                     <<<'PHP'
-$key = key($opt->option);
-$val = current($opt->option);
+$key = key($callbacks);
+$callback = current($callbacks);
+next($callbacks);
 PHP
                 ),
             ]
@@ -94,15 +94,13 @@ public function refactor(Node $node): ?Node
         // ↓
         // $key = key($values);
         // $value = current($values);
-        // next($values); - only inside a loop
+        // next($values);
         $currentFuncCall = $this->createFuncCall('current', $eachFuncCall->args);
         $assignCurrentNode = new Assign($listNode->items[1]->value, $currentFuncCall);
         $this->addNodeAfterNode($assignCurrentNode, $node);
 
-        if ($this->isInsideDoWhile($node)) {
-            $nextFuncCall = $this->createFuncCall('next', $eachFuncCall->args);
-            $this->addNodeAfterNode($nextFuncCall, $node);
-        }
+        $nextFuncCall = $this->createFuncCall('next', $eachFuncCall->args);
+        $this->addNodeAfterNode($nextFuncCall, $node);
 
         $keyFuncCall = $this->createFuncCall('key', $eachFuncCall->args);
         return new Assign($listNode->items[0]->value, $keyFuncCall);
@@ -130,19 +128,4 @@ private function shouldSkip(Assign $assign): bool
         // empty list → cannot handle
         return $listNode->items[0] === null && $listNode->items[1] === null;
     }
-
-    /**
-     * Is inside the "do {} while ();" loop → need to add "next()"
-     */
-    private function isInsideDoWhile(Node $assignNode): bool
-    {
-        $parentNode = $assignNode->getAttribute(AttributeKey::PARENT_NODE);
-        if (! $parentNode instanceof Expression) {
-            return false;
-        }
-
-        $parentParentNode = $parentNode->getAttribute(AttributeKey::PARENT_NODE);
-
-        return $parentParentNode instanceof Do_;
-    }
 }

rules/php72/tests/Rector/Each/Fixture/fixture2.php.inc

@@ -23,6 +23,7 @@ function each2
 {
     $key = key($opt->option);
     $val = current($opt->option);
+    next($opt->option);
 
     $tid = key($option->option);
 

rules/php72/tests/Rector/Each/Fixture/list_each_next.php.inc

@@ -0,0 +1,47 @@
+<?php
+
+namespace Rector\Php72\Tests\Rector\Each\Fixture;
+
+use Rector\Core\Testing\Contract\RunnableInterface;
+
+final class ListEachNext implements RunnableInterface
+{
+    public function run()
+    {
+        $parentArray = ['a' => 1, 'b' => 2];
+
+        list($key, $value) = each($parentArray);
+
+        list($key2, $value2) = each($parentArray);
+
+        return [$key, $value, $parentArray, $key2, $value2];
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Php72\Tests\Rector\Each\Fixture;
+
+use Rector\Core\Testing\Contract\RunnableInterface;
+
+final class ListEachNext implements RunnableInterface
+{
+    public function run()
+    {
+        $parentArray = ['a' => 1, 'b' => 2];
+
+        $key = key($parentArray);
+        $value = current($parentArray);
+        next($parentArray);
+
+        $key2 = key($parentArray);
+        $value2 = current($parentArray);
+        next($parentArray);
+
+        return [$key, $value, $parentArray, $key2, $value2];
+    }
+}
+
+?>

src/Testing/PHPUnit/RunnableInterface.php → src/Testing/Contract/RunnableInterface.php

@@ -2,7 +2,7 @@
 
 declare(strict_types=1);
 
-namespace Rector\Core\Testing\PHPUnit;
+namespace Rector\Core\Testing\Contract;
 
 interface RunnableInterface
 {

src/Testing/PHPUnit/AbstractRectorTestCase.php

@@ -5,6 +5,7 @@
 namespace Rector\Core\Testing\PHPUnit;
 
 use Nette\Utils\FileSystem;
+use Nette\Utils\Strings;
 use PHPStan\Analyser\NodeScopeResolver;
 use PHPUnit\Framework\ExpectationFailedException;
 use Psr\Container\ContainerInterface;
@@ -17,6 +18,7 @@
 use Rector\Core\HttpKernel\RectorKernel;
 use Rector\Core\Stubs\StubLoader;
 use Rector\Core\Testing\Application\EnabledRectorsProvider;
+use Rector\Core\Testing\Contract\RunnableInterface;
 use Rector\Core\Testing\Finder\RectorsFinder;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Style\SymfonyStyle;
@@ -27,6 +29,8 @@
 
 abstract class AbstractRectorTestCase extends AbstractGenericRectorTestCase
 {
+    use RunnableRectorTrait;
+
     /**
      * @var FileProcessor
      */
@@ -38,19 +42,19 @@ abstract class AbstractRectorTestCase extends AbstractGenericRectorTestCase
     protected $parameterProvider;
 
     /**
-     * @var string
+     * @var SmartFileInfo
      */
-    protected $originalTempFile;
+    protected $originalTempFileInfo;
 
     /**
-     * @var bool
+     * @var FixtureSplitter
      */
-    private $autoloadTestFixture = true;
+    protected $fixtureSplitter;
 
     /**
-     * @var FixtureSplitter
+     * @var bool
      */
-    private $fixtureSplitter;
+    private $autoloadTestFixture = true;
 
     /**
      * @var Container|ContainerInterface|null
@@ -130,21 +134,23 @@ protected function doTestFileWithoutAutoload(string $file): void
 
     protected function doTestFile(string $fixtureFile): void
     {
-        $smartFileInfo = new SmartFileInfo($fixtureFile);
-        [$originalFile, $changedFile] = $this->fixtureSplitter->splitContentToOriginalFileAndExpectedFile(
-            $smartFileInfo,
+        $fixtureFileInfo = new SmartFileInfo($fixtureFile);
+
+        [$originalFileInfo, $expectedFileInfo] = $this->fixtureSplitter->splitContentToOriginalFileAndExpectedFile(
+            $fixtureFileInfo,
             $this->autoloadTestFixture
         );
 
-        $this->nodeScopeResolver->setAnalysedFiles([$originalFile]);
+        $this->nodeScopeResolver->setAnalysedFiles([$originalFileInfo->getRealPath()]);
 
-        $this->doTestFileMatchesExpectedContent(
-            $originalFile,
-            $changedFile,
-            $smartFileInfo->getRelativeFilePathFromCwd()
-        );
+        $this->doTestFileMatchesExpectedContent($originalFileInfo, $expectedFileInfo, $fixtureFileInfo);
+
+        $this->originalTempFileInfo = $originalFileInfo;
 
-        $this->originalTempFile = $originalFile;
+        // runnable?
+        if (Strings::contains($originalFileInfo->getContents(), RunnableInterface::class)) {
+            $this->assertOriginalAndFixedFileResultEquals($originalFileInfo, $expectedFileInfo);
+        }
     }
 
     protected function getTempPath(): string
@@ -238,39 +244,32 @@ private function configurePhpVersionFeatures(): void
     }
 
     private function doTestFileMatchesExpectedContent(
-        string $originalFile,
-        string $expectedFile,
-        string $fixtureFile
+        SmartFileInfo $originalFileInfo,
+        SmartFileInfo $expectedFileInfo,
+        SmartFileInfo $fixtureFileInfo
     ): void {
-        $this->setParameter(Option::SOURCE, [$originalFile]);
-
-        $smartFileInfo = new SmartFileInfo($originalFile);
+        $this->setParameter(Option::SOURCE, [$originalFileInfo->getRealPath()]);
 
         // life-cycle trio :)
-        $this->fileProcessor->parseFileInfoToLocalCache($smartFileInfo);
-        $this->fileProcessor->refactor($smartFileInfo);
+        $this->fileProcessor->parseFileInfoToLocalCache($originalFileInfo);
+        $this->fileProcessor->refactor($originalFileInfo);
 
-        $changedContent = $this->fileProcessor->printToString($smartFileInfo);
+        $changedContent = $this->fileProcessor->printToString($originalFileInfo);
 
-        $causedByFixtureMessage = $this->createCausedByFixtureMessage($fixtureFile);
+        $causedByFixtureMessage = $fixtureFileInfo->getRelativeFilePathFromCwd();
 
         $removedAndAddedFilesProcessor = self::$container->get(RemovedAndAddedFilesProcessor::class);
         $removedAndAddedFilesProcessor->run();
 
         try {
-            $this->assertStringEqualsFile($expectedFile, $changedContent, $causedByFixtureMessage);
+            $this->assertStringEqualsFile($expectedFileInfo->getRealPath(), $changedContent, $causedByFixtureMessage);
         } catch (ExpectationFailedException $expectationFailedException) {
-            $expectedFileContent = FileSystem::read($expectedFile);
+            $expectedFileContent = $expectedFileInfo->getContents();
 
             $this->assertStringMatchesFormat($expectedFileContent, $changedContent, $causedByFixtureMessage);
         }
     }
 
-    private function createCausedByFixtureMessage(string $fixtureFile): string
-    {
-        return (new SmartFileInfo($fixtureFile))->getRelativeFilePathFromCwd();
-    }
-
     private function createRectorRepositoryContainer(): void
     {
         if (self::$allRectorContainer === null) {