rework AbstractRunnableRectorTestCase from eval to include

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,9 +6,9 @@ namespace Rector\CodeQuality\Tests\Rector\FuncCall\ArrayKeysAndInArrayToArrayKey
 
 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()
@@ -16,7 +16,6 @@ final class ArrayKeysAndInArrayToArrayKeyExistsRectorTest extends AbstractRunnab
     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/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;
 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 @@ list($key, $callback) = each($callbacks);
 PHP
                     ,
                     <<<'PHP'
-$key = key($opt->option);
-$val = current($opt->option);
+$key = key($callbacks);
+$callback = current($callbacks);
+next($callbacks);
 PHP
                 ),
             ]
@@ -94,15 +94,13 @@ PHP
         // ↓
         // $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 @@ PHP
         // 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/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 @@ declare(strict_types=1);
 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\Exception\ShouldNotHappenException;
 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 @@ use Symplify\SmartFileSystem\SmartFileInfo;
 
 abstract class AbstractRectorTestCase extends AbstractGenericRectorTestCase
 {
+    use RunnableRectorTrait;
+
     /**
      * @var FileProcessor
      */
@@ -42,16 +46,16 @@ abstract class AbstractRectorTestCase extends AbstractGenericRectorTestCase
      */
     protected $originalTempFile;
 
+    /**
+     * @var FixtureSplitter
+     */
+    protected $fixtureSplitter;
+
     /**
      * @var bool
      */
     private $autoloadTestFixture = true;
 
-    /**
-     * @var FixtureSplitter
-     */
-    private $fixtureSplitter;
-
     /**
      * @var Container|ContainerInterface|null
      */
@@ -145,6 +149,14 @@ abstract class AbstractRectorTestCase extends AbstractGenericRectorTestCase
         );
 
         $this->originalTempFile = $originalFile;
+
+        // runnable?
+        if (Strings::contains(FileSystem::read($originalFile), RunnableInterface::class)) {
+            $originalFileInfo = new SmartFileInfo($originalFile);
+            $changedFileInfo = new SmartFileInfo($changedFile);
+
+            $this->assertOriginalAndFixedFileResultEquals($originalFileInfo, $changedFileInfo);
+        }
     }
 
     protected function getTempPath(): string

src/Testing/PHPUnit/AbstractRunnableRectorTestCase.php

@@ -1,68 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace Rector\Core\Testing\PHPUnit;
-
-use Nette\Utils\Strings;
-use Rector\Core\Testing\ValueObject\SplitLine;
-use ReflectionClass;
-use Symplify\SmartFileSystem\SmartFileInfo;
-
-abstract class AbstractRunnableRectorTestCase extends AbstractRectorTestCase
-{
-    protected function assertOriginalAndFixedFileResultEquals(string $file): void
-    {
-        /**
-         * Todo: Duplicate from
-         *
-         * @see FixtureSplitter::splitContentToOriginalFileAndExpectedFile
-         * ==> refactor in a method
-         */
-        $smartFileInfo = new SmartFileInfo($file);
-        if (Strings::match($smartFileInfo->getContents(), SplitLine::SPLIT_LINE)) {
-            [$originalContent, $expectedContent] = Strings::split($smartFileInfo->getContents(), SplitLine::SPLIT_LINE);
-        } else {
-            $originalContent = $smartFileInfo->getContents();
-            $expectedContent = $originalContent;
-        }
-
-        $originalInstance = $this->loadClass($originalContent);
-        if ($originalInstance !== null) {
-            $expectedInstance = $this->loadClass($expectedContent);
-            if ($expectedInstance !== null) {
-                $actual = $originalInstance->run();
-                $expected = $expectedInstance->run();
-
-                $this->assertSame($actual, $expected);
-            }
-        }
-    }
-
-    protected function getTemporaryClassName(): string
-    {
-        $testName = (new ReflectionClass(static::class))->getShortName();
-        // Todo - pull in Ramsey UUID to generate temporay class names?
-//        $uuid      = Uuid::uuid4()->toString();
-        $uuid = md5((string) random_int(0, 100000000));
-        $className = $testName . '_' . $uuid;
-
-        return Strings::replace($className, '#[^0-9a-zA-Z]#', '_');
-    }
-
-    protected function loadClass(string $classContent): ?RunnableInterface
-    {
-        $className = $this->getTemporaryClassName();
-        $loadable = Strings::replace($classContent, '#\\s*<\\?php#', '');
-        $loadable = Strings::replace($loadable, '#\\s*namespace.*;#', '');
-        $loadable = Strings::replace($loadable, '#class\\s+(\\S*)\\s+#', sprintf('class %s ', $className));
-        eval($loadable);
-        if (is_a($className, RunnableInterface::class, true)) {
-            /**
-             * @var RunnableInterface
-             */
-            return new $className();
-        }
-        return null;
-    }
-}

src/Testing/PHPUnit/FixtureSplitter.php

@@ -28,14 +28,7 @@ final class FixtureSplitter
         SmartFileInfo $smartFileInfo,
         bool $autoloadTestFixture
     ): array {
-        if (Strings::match($smartFileInfo->getContents(), SplitLine::SPLIT_LINE)) {
-            // original → expected
-            [$originalContent, $expectedContent] = Strings::split($smartFileInfo->getContents(), SplitLine::SPLIT_LINE);
-        } else {
-            // no changes
-            $originalContent = $smartFileInfo->getContents();
-            $expectedContent = $originalContent;
-        }
+        [$originalContent, $expectedContent] = $this->resolveBeforeAfterFixtureContent($smartFileInfo);
 
         $originalFile = $this->createTemporaryPathWithPrefix($smartFileInfo, 'original');
         $expectedFile = $this->createTemporaryPathWithPrefix($smartFileInfo, 'expected');
@@ -51,11 +44,28 @@ final class FixtureSplitter
         return [$originalFile, $expectedFile];
     }
 
-    private function createTemporaryPathWithPrefix(SmartFileInfo $smartFileInfo, string $prefix): string
+    public function createTemporaryPathWithPrefix(SmartFileInfo $smartFileInfo, string $prefix): string
     {
         // warning: if this hash is too short, the file can becom "identical"; took me 1 hour to find out
-        $hash = Strings::substring(md5($smartFileInfo->getRealPath()), 0, 12);
+        $hash = Strings::substring(md5($smartFileInfo->getRealPath()), 0, 15);
 
         return sprintf($this->tempPath . '/%s_%s_%s', $prefix, $hash, $smartFileInfo->getBasename('.inc'));
     }
+
+    /**
+     * @return string[]
+     */
+    private function resolveBeforeAfterFixtureContent(SmartFileInfo $smartFileInfo): array
+    {
+        if (Strings::match($smartFileInfo->getContents(), SplitLine::SPLIT_LINE)) {
+            // original → expected
+            [$originalContent, $expectedContent] = Strings::split($smartFileInfo->getContents(), SplitLine::SPLIT_LINE);
+        } else {
+            // no changes
+            $originalContent = $smartFileInfo->getContents();
+            $expectedContent = $originalContent;
+        }
+
+        return [$originalContent, $expectedContent];
+    }
 }

src/Testing/PHPUnit/RunnableRectorTrait.php

@@ -0,0 +1,61 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Core\Testing\PHPUnit;
+
+use Nette\Utils\FileSystem;
+use Nette\Utils\Random;
+use Nette\Utils\Strings;
+use Rector\Core\Exception\ShouldNotHappenException;
+use Rector\Core\Testing\Contract\RunnableInterface;
+use Symplify\SmartFileSystem\SmartFileInfo;
+
+/**
+ * @property FixtureSplitter $fixtureSplitter
+ */
+trait RunnableRectorTrait
+{
+    protected function assertOriginalAndFixedFileResultEquals(
+        SmartFileInfo $originalFileInfo,
+        SmartFileInfo $expectedFileInfo
+    ): void {
+        $originalInstance = $this->createRunnableClass($originalFileInfo);
+        $expectedInstance = $this->createRunnableClass($expectedFileInfo);
+
+        $actualResult = $originalInstance->run();
+        $expectedResult = $expectedInstance->run();
+
+        $this->assertSame($expectedResult, $actualResult);
+    }
+
+    private function getTemporaryClassName(): string
+    {
+        return 'ClassName_' . Random::generate(20);
+    }
+
+    private function createRunnableClass(SmartFileInfo $classFileInfo): RunnableInterface
+    {
+        $temporaryPath = $this->fixtureSplitter->createTemporaryPathWithPrefix($classFileInfo, 'runnable');
+
+        $fileContent = $classFileInfo->getContents();
+
+        // use unique class name for before and for after class, so both can be instantiated
+        $className = $this->getTemporaryClassName();
+        $classFileInfo = Strings::replace($fileContent, '#class\\s+(\\S*)\\s+#', sprintf('class %s ', $className));
+
+        FileSystem::write($temporaryPath, $classFileInfo);
+        include_once $temporaryPath;
+
+        $matches = Strings::match($classFileInfo, '#\bnamespace (?<namespace>.*?);#');
+        $namespace = $matches['namespace'] ?? '';
+
+        $fullyQualifiedClassName = $namespace . '\\' . $className;
+
+        if (! is_a($fullyQualifiedClassName, RunnableInterface::class, true)) {
+            throw new ShouldNotHappenException();
+        }
+
+        return new $fullyQualifiedClassName();
+    }
+}