Fix preload.php order to included firstly used classes first, to avoi…

…d missing parent bugs (#929) * skip false positive phpstan * [preload] fix order to go from first node usages * improve build-preload.php * [CI] test preload.php order * [CI] add prelaod to test * Add priority files to the top
rectorphp · Sep 28, 2021 · af0fc79 · af0fc79
1 parent d21dcc3
commit af0fc79
Show file tree

Hide file tree

Showing 5 changed files with 363 additions and 240 deletions.
diff --git a/.github/workflows/code_analysis.yaml b/.github/workflows/code_analysis.yaml
@@ -17,6 +17,10 @@ jobs:
                         name: 'Composer Validate'
                         run: composer validate --ansi
 
+                    -
+                        name: 'Preload php-parser Order'
+                        run: php preload.php
+
                     -
                         name: 'Validate Max File Length'
                         run: vendor/bin/easy-ci validate-file-length packages rules src tests

diff --git a/build/build-preload.php b/build/build-preload.php
@@ -5,50 +5,167 @@
 declare(strict_types=1);
 
 use Nette\Utils\Strings;
+use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Finder\Finder;
+use Symplify\PackageBuilder\Console\Style\SymfonyStyleFactory;
 
 require __DIR__ . '/../vendor/autoload.php';
 
 $buildDirectory = $argv[1];
 
-buildPreloadScript($buildDirectory);
+$symfonyStyleFactory = new SymfonyStyleFactory();
+$symfonyStyle = $symfonyStyleFactory->create();
 
-function buildPreloadScript(string $buildDirectory): void
-{
-    $vendorDir = $buildDirectory . '/vendor';
-    if (!is_dir($vendorDir . '/nikic/php-parser/lib/PhpParser')) {
-        return;
-    }
+if (! is_string($buildDirectory)) {
+    $errorMessage = 'Provide build directory path as an argument, e.g. "php build-preload.php rector-build-directory"';
+    $symfonyStyle->error($errorMessage);
+    exit(Command::FAILURE);
+}
 
-    $preloadFileContent = <<<'php'
+$preloadBuilder = new PreloadBuilder();
+$preloadBuilder->buildPreloadScript($buildDirectory);
+
+final class PreloadBuilder
+{
+    /**
+     * @var string
+     */
+    private const PRELOAD_FILE_TEMPLATE = <<<'PHP'
 <?php
 
 declare(strict_types=1);
 
 
-php;
+PHP;
+
+    /**
+     * @var int
+     */
+    private const PRIORITY_LESS_FILE_POSITION = -1;
+
+    /**
+     * These files are parent to another files, so they have to be included first
+     * See https://github.com/rectorphp/rector/issues/6709 for more
+     *
+     * @var string[]
+     */
+    private  const HIGH_PRIORITY_FILES = [
+        'Node.php',
+        'NodeAbstract.php',
+        'Expr.php',
+        'NodeVisitor.php',
+        'NodeVisitorAbstract.php',
+        'Lexer.php',
+        'TokenEmulator.php',
+        'KeywordEmulator.php',
+        'Comment.php',
+        'PrettyPrinterAbstract.php',
+        'Parser.php',
+        'ParserAbstract.php',
+        'ErrorHandler.php',
+        'Stmt.php',
+        'FunctionLike.php',
+        'ClassLike.php',
+        'Builder.php',
+        'TraitUseAdaptation.php',
+        'ComplexType.php',
+        'CallLike.php',
+        'AssignOp.php',
+        'BinaryOp.php',
+        'Name.php',
+        'Scalar.php',
+        'MagicConst.php',
+        'NodeTraverserInterface.php',
+        'Declaration.php',
+        'Builder/FunctionLike.php',
+        'Stmt/FunctionLike.php',
+    ];
+
+    public function buildPreloadScript(string $buildDirectory): void
+    {
+        $vendorDir = $buildDirectory . '/vendor';
+        if (! is_dir($vendorDir . '/nikic/php-parser/lib/PhpParser')) {
+            return;
+        }
+
+        // 1. fine php-parser file infos
+        $fileInfos = $this->findPhpParserFiles($vendorDir);
+
+
+        // append ContainerConfiguration to avoid accidental load of prefixed one from another tool
+        $fileInfos[] = new SplFileInfo(__DIR__ . '/../vendor/symfony/dependency-injection/Loader/Configurator/AbstractConfigurator.php');
+        $fileInfos[] = new SplFileInfo(__DIR__ . '/../vendor/symfony/dependency-injection/Loader/Configurator/ContainerConfigurator.php');
 
-    $finder = (new Finder())
-        ->files()
-        ->name('*.php')
-        ->in($vendorDir . '/nikic/php-parser/lib/PhpParser')
-        ->notPath('#\/tests\/#')
-        ->notPath('#\/config\/#')
-        ->notPath('#\/set\/#')
-        ->in($vendorDir . '/symplify/symfony-php-config');
+        // 2. put first-class usages first
+        usort($fileInfos, function (SplFileInfo $firstFileInfo, SplFileInfo $secondFileInfo) {
+            $firstFilePosition = $this->matchFilePriorityPosition($firstFileInfo);
+            $secondFilePosition = $this->matchFilePriorityPosition($secondFileInfo);
 
-    $fileInfos = iterator_to_array($finder->getIterator());
-    $fileInfos[] = new SplFileInfo(__DIR__ . '/../vendor/symfony/dependency-injection/Loader/Configurator/ContainerConfigurator.php');
+            return $secondFilePosition <=> $firstFilePosition;
+        });
+
+        // 3. create preload.php from provided files
+        $preloadFileContent = $this->createPreloadFileContent($fileInfos);
+
+        file_put_contents($buildDirectory . '/preload.php', $preloadFileContent);
+    }
 
-    foreach ($fileInfos as $fileInfo) {
-        $realPath = $fileInfo->getRealPath();
-        if ($realPath === false) {
-            continue;
+    /**
+     * @return SplFileInfo[]
+     */
+    private function findPhpParserFiles(string $vendorDir): array
+    {
+        $finder = (new Finder())
+            ->files()
+            ->name('*.php')
+            ->in($vendorDir . '/nikic/php-parser/lib/PhpParser')
+            ->notPath('#\/tests\/#')
+            ->notPath('#\/config\/#')
+            ->notPath('#\/set\/#')
+            ->in($vendorDir . '/symplify/symfony-php-config')
+            ->sortByName();
+
+        return iterator_to_array($finder->getIterator());
+    }
+
+    /**
+     * @param SplFileInfo[] $fileInfos
+     */
+    private function createPreloadFileContent(array $fileInfos): string
+    {
+        $preloadFileContent = self::PRELOAD_FILE_TEMPLATE;
+
+        foreach ($fileInfos as $fileInfo) {
+            $realPath = $fileInfo->getRealPath();
+            if ($realPath === false) {
+                continue;
+            }
+
+            $preloadFileContent .= $this->createRequireOnceFilePathLine($realPath);
         }
 
+        return $preloadFileContent;
+    }
+
+    private function createRequireOnceFilePathLine(string $realPath): string
+    {
         $filePath = '/vendor/' . Strings::after($realPath, 'vendor/');
-        $preloadFileContent .= "require_once __DIR__ . '" . $filePath . "';" . PHP_EOL;
+        return "require_once __DIR__ . '" . $filePath . "';" . PHP_EOL;
     }
 
-    file_put_contents($buildDirectory . '/preload.php', $preloadFileContent);
+    private function matchFilePriorityPosition(SplFileInfo $splFileInfo): int
+    {
+        // to make <=> operator work
+        $highPriorityFiles = array_reverse(self::HIGH_PRIORITY_FILES);
+
+        $fileRealPath = $splFileInfo->getRealPath();
+
+        foreach ($highPriorityFiles as $position => $highPriorityFile) {
+            if (str_ends_with($fileRealPath, '/' . $highPriorityFile)) {
+                return $position;
+            }
+        }
+
+        return self::PRIORITY_LESS_FILE_POSITION;
+    }
 }
diff --git a/phpstan.neon b/phpstan.neon
@@ -507,3 +507,6 @@ parameters:
                 - packages/FileFormatter/ValueObject/NewLine.php #15
                 - src/Application/VersionResolver.php #16
                 - utils/compiler/src/Unprefixer.php #9
+
+        # false positive - class_exists check is right above it
+        - '#Parameter \#1 \$objectOrClass of class ReflectionClass constructor expects class\-string<Rector\\RectorInstaller\\GeneratedConfig\>\|Rector\\RectorInstaller\\GeneratedConfig, string given#'