Introducing Collectors - final step 🥳 - processing collected data + t…

…est case (#4970)

.github/workflows/code_analysis.yaml

@@ -48,7 +48,8 @@ jobs:
                                 --skip-type="Rector\\NodeTypeResolver\\PHPStan\\Scope\\Contract\\NodeVisitor\\ScopeResolverNodeVisitorInterface" \
                                 --skip-type="Rector\\BetterPhpDocParser\\Contract\\BasePhpDocNodeVisitorInterface" \
                                 --skip-type="Rector\\BetterPhpDocParser\\Contract\\PhpDocParser\\PhpDocNodeDecoratorInterface" \
-                                --skip-type="Rector\\BetterPhpDocParser\\ValueObject\\Type\\FullyQualifiedIdentifierTypeNode"
+                                --skip-type="Rector\\BetterPhpDocParser\\ValueObject\\Type\\FullyQualifiedIdentifierTypeNode" \
+                                --skip-type="Rector\\Core\\Contract\\Rector\\CollectorRectorInterface"
 
                     -
                         name: 'Compatible PHPStan versions'

config/config.php

@@ -14,6 +14,7 @@
     $rectorConfig->autoloadPaths([]);
     $rectorConfig->bootstrapFiles([]);
     $rectorConfig->parallel();
+    $rectorConfig->disableCollectors();
 
     // to avoid autoimporting out of the box
     $rectorConfig->importNames(false, false);

packages/Config/RectorConfig.php

@@ -9,6 +9,7 @@
 use Rector\Caching\Contract\ValueObject\Storage\CacheStorageInterface;
 use Rector\Core\Configuration\Option;
 use Rector\Core\Configuration\Parameter\SimpleParameterProvider;
+use Rector\Core\Contract\Rector\CollectorRectorInterface;
 use Rector\Core\Contract\Rector\ConfigurableRectorInterface;
 use Rector\Core\Contract\Rector\RectorInterface;
 use Rector\Core\DependencyInjection\Laravel\ContainerMemento;
@@ -59,6 +60,14 @@ public function disableParallel(): void
         SimpleParameterProvider::setParameter(Option::PARALLEL, false);
     }
 
+    /**
+     * @experimental since Rector 0.17.x
+     */
+    public function enableCollectors(): void
+    {
+        SimpleParameterProvider::setParameter(Option::COLLECTORS, true);
+    }
+
     public function parallel(int $seconds = 120, int $maxNumberOfProcess = 16, int $jobSize = 15): void
     {
         SimpleParameterProvider::setParameter(Option::PARALLEL, true);
@@ -203,6 +212,10 @@ public function rule(string $rectorClass): void
         $this->singleton($rectorClass);
         $this->tag($rectorClass, RectorInterface::class);
 
+        if (is_a($rectorClass, CollectorRectorInterface::class, true)) {
+            $this->tag($rectorClass, CollectorRectorInterface::class);
+        }
+
         if (is_a($rectorClass, AbstractScopeAwareRector::class, true)) {
             $this->extend(
                 $rectorClass,
@@ -377,6 +390,14 @@ public function boot(): void
         }
     }
 
+    /**
+     * @experimental since Rector 0.17.x
+     */
+    public function disableCollectors(): void
+    {
+        SimpleParameterProvider::setParameter(Option::COLLECTORS, false);
+    }
+
     private function isRuleNoLongerExists(mixed $skipRule): bool
     {
         return // only validate string

packages/Parallel/Application/ParallelFileProcessor.php

@@ -80,8 +80,8 @@ public function process(
         /** @var FileDiff[] $fileDiffs */
         $fileDiffs = [];
 
-        /** @var CollectedData[] $collectedDatas */
-        $collectedDatas = [];
+        /** @var CollectedData[] $collectedData */
+        $collectedData = [];
 
         /** @var SystemError[] $systemErrors */
         $systemErrors = [];
@@ -117,9 +117,11 @@ public function process(
                 }
 
                 $jobsChunk = array_pop($jobs);
+
                 $parallelProcess->request([
                     ReactCommand::ACTION => Action::MAIN,
                     Content::FILES => $jobsChunk,
+                    Bridge::PREVIOUSLY_COLLECTED_DATA => $configuration->getCollectedData(),
                 ]);
             });
         });
@@ -160,6 +162,7 @@ public function process(
         $processSpawner = function () use (
             &$systemErrors,
             &$fileDiffs,
+            &$collectedData,
             &$jobs,
             $postFileCallback,
             &$systemErrorsCount,
@@ -195,7 +198,7 @@ function (array $json) use (
                     &$jobs,
                     $postFileCallback,
                     &$systemErrorsCount,
-                    &$collectedDatas,
+                    &$collectedData,
                     &$reachedInternalErrorsCountLimit,
                     $processIdentifier,
                     &$fileChunksBudgetPerProcess,
@@ -215,8 +218,8 @@ function (array $json) use (
                         $fileDiffs[] = FileDiff::decode($jsonFileDiff);
                     }
 
-                    foreach ($json[Bridge::COLLECTED_DATA] as $jsonCollectedData) {
-                        $collectedDatas[] = CollectedData::decode($jsonCollectedData);
+                    foreach ($json[Bridge::COLLECTED_DATA] as $collectedDataItem) {
+                        $collectedData[] = CollectedData::decode($collectedDataItem);
                     }
 
                     $postFileCallback($json[Bridge::FILES_COUNT]);
@@ -287,6 +290,6 @@ function ($exitCode, string $stdErr) use (&$systemErrors, $processIdentifier): v
             ));
         }
 
-        return new ProcessResult($systemErrors, $fileDiffs, $collectedDatas);
+        return new ProcessResult($systemErrors, $fileDiffs, $collectedData);
     }
 }

packages/Parallel/ValueObject/Bridge.php

@@ -38,4 +38,9 @@ final class Bridge
      * @var string
      */
     public const COLLECTED_DATA = 'collected_data';
+
+    /**
+     * @var string
+     */
+    public const PREVIOUSLY_COLLECTED_DATA = 'previously_collected_data';
 }

packages/Testing/PHPUnit/AbstractRectorTestCase.php

@@ -8,6 +8,7 @@
 use Iterator;
 use Nette\Utils\FileSystem;
 use Nette\Utils\Strings;
+use PHPStan\Collectors\Collector;
 use PHPUnit\Framework\ExpectationFailedException;
 use Rector\Core\Application\ApplicationFileProcessor;
 use Rector\Core\Autoloading\AdditionalAutoloader;
@@ -16,10 +17,11 @@
 use Rector\Core\Configuration\Option;
 use Rector\Core\Configuration\Parameter\SimpleParameterProvider;
 use Rector\Core\Contract\DependencyInjection\ResetableInterface;
+use Rector\Core\Contract\Rector\CollectorRectorInterface;
 use Rector\Core\Contract\Rector\RectorInterface;
+use Rector\Core\DependencyInjection\Laravel\ContainerMemento;
 use Rector\Core\Exception\ShouldNotHappenException;
 use Rector\Core\PhpParser\NodeTraverser\RectorNodeTraverser;
-use Rector\Core\Provider\CurrentFileProvider;
 use Rector\Core\Rector\AbstractRector;
 use Rector\Core\Util\Reflection\PrivatesAccessor;
 use Rector\NodeTypeResolver\Reflection\BetterReflection\SourceLocatorProvider\DynamicSourceLocatorProvider;
@@ -41,8 +43,6 @@ abstract class AbstractRectorTestCase extends AbstractLazyTestCase implements Re
      */
     private static array $cacheByRuleAndConfig = [];
 
-    private CurrentFileProvider $currentFileProvider;
-
     /**
      * Restore default parameters
      */
@@ -55,6 +55,7 @@ public static function tearDownAfterClass(): void
 
         SimpleParameterProvider::setParameter(Option::INDENT_CHAR, ' ');
         SimpleParameterProvider::setParameter(Option::INDENT_SIZE, 4);
+        SimpleParameterProvider::setParameter(Option::COLLECTORS, false);
     }
 
     protected function setUp(): void
@@ -81,24 +82,27 @@ protected function setUp(): void
                 $resetable->reset();
             }
 
-            $this->forgetRectorsRules();
+            $this->forgetRectorsRulesAndCollectors();
             $rectorConfig->resetRuleConfigurations();
 
             // this has to be always empty, so we can add new rules with their configuration
             $this->assertEmpty($rectorConfig->tagged(RectorInterface::class));
+            $this->assertEmpty($rectorConfig->tagged(CollectorRectorInterface::class));
+            $this->assertEmpty($rectorConfig->tagged(Collector::class));
 
             $this->bootFromConfigFiles([$configFile]);
 
             $rectorsGenerator = $rectorConfig->tagged(RectorInterface::class);
             if ($rectorsGenerator instanceof RewindableGenerator) {
-                $phpRectors = iterator_to_array($rectorsGenerator->getIterator());
+                $rectors = iterator_to_array($rectorsGenerator->getIterator());
             } else {
                 // no rules at all, e.g. in case of only post rector run
-                $phpRectors = [];
+                $rectors = [];
             }
 
+            /** @var RectorNodeTraverser $rectorNodeTraverser */
             $rectorNodeTraverser = $rectorConfig->make(RectorNodeTraverser::class);
-            $rectorNodeTraverser->refreshPhpRectors($phpRectors);
+            $rectorNodeTraverser->refreshPhpRectors($rectors);
 
             // store cache
             self::$cacheByRuleAndConfig[$cacheKey] = true;
@@ -114,8 +118,6 @@ protected function setUp(): void
         /** @var BootstrapFilesIncluder $bootstrapFilesIncluder */
         $bootstrapFilesIncluder = $this->make(BootstrapFilesIncluder::class);
         $bootstrapFilesIncluder->includeBootstrapFiles();
-
-        $this->currentFileProvider = $this->make(CurrentFileProvider::class);
     }
 
     protected function tearDown(): void
@@ -169,24 +171,17 @@ protected function doTestFile(string $fixtureFilePath): void
         $this->doTestFileMatchesExpectedContent($inputFilePath, $expectedFileContents, $fixtureFilePath);
     }
 
-    protected function forgetRectorsRules(): void
+    protected function forgetRectorsRulesAndCollectors(): void
     {
         $rectorConfig = self::getContainer();
 
-        // 1. forget instance first, then remove tags
-        $rectors = $rectorConfig->tagged(RectorInterface::class);
-        foreach ($rectors as $rector) {
-            $rectorConfig->offsetUnset($rector::class);
-        }
+        // 1. forget tagged services
+        ContainerMemento::forgetTag($rectorConfig, RectorInterface::class);
+        ContainerMemento::forgetTag($rectorConfig, Collector::class);
+        ContainerMemento::forgetTag($rectorConfig, CollectorRectorInterface::class);
 
-        // 2. remove all tagged rules
+        // 2. remove after binding too, to avoid setting configuration over and over again
         $privatesAccessor = new PrivatesAccessor();
-        $privatesAccessor->propertyClosure($rectorConfig, 'tags', static function (array $tags): array {
-            unset($tags[RectorInterface::class]);
-            return $tags;
-        });
-
-        // 3. remove after binding too, to avoid setting configuration over and over again
         $privatesAccessor->propertyClosure(
             $rectorConfig,
             'afterResolvingCallbacks',
@@ -230,8 +225,7 @@ private function doTestFileMatchesExpectedContent(
         SimpleParameterProvider::setParameter(Option::SOURCE, [$originalFilePath]);
 
         $changedContent = $this->processFilePath($originalFilePath);
-        $originalFileContent = $this->currentFileProvider->getFile()
-            ->getOriginalFileContent();
+        $originalFileContent = FileSystem::read($originalFilePath);
 
         $fixtureFilename = basename($fixtureFilePath);
         $failureMessage = sprintf('Failed on fixture file "%s"', $fixtureFilename);
@@ -254,10 +248,21 @@ private function processFilePath(string $filePath): string
         $configurationFactory = $this->make(ConfigurationFactory::class);
         $configuration = $configurationFactory->createForTests([$filePath]);
 
-        $this->applicationFileProcessor->processFiles([$filePath], $configuration);
+        $processResult = $this->applicationFileProcessor->processFiles([$filePath], $configuration);
+
+        if ($processResult->getCollectedData() !== [] && $configuration->isCollectors()) {
+            // second run with collected data
+            $configuration->setCollectedData($processResult->getCollectedData());
+            $configuration->enableSecondRun();
+
+            $rectorNodeTraverser = $this->make(RectorNodeTraverser::class);
+            $rectorNodeTraverser->prepareCollectorRectorsRun($configuration);
+
+            $this->applicationFileProcessor->processFiles([$filePath], $configuration);
+        }
 
-        $currentFile = $this->currentFileProvider->getFile();
-        return $currentFile->getFileContent();
+        // return changed file contents
+        return FileSystem::read($filePath);
     }
 
     private function createInputFilePath(string $fixtureFilePath): string

phpstan.neon

@@ -39,9 +39,6 @@ parameters:
         # too modern code for PHPStan 0.12.2
         - bin/validate-phpstan-version.php
 
-        # handle later after di transition
-        - packages-tests/BetterPhpDocParser/PhpDocInfo/PhpDocInfoPrinter/MultilineTest.
-
         # CI test
         - tests/Fixtures/*
 
@@ -159,6 +156,7 @@ parameters:
                 - packages/BetterPhpDocParser/ValueObject/PhpDoc/DoctrineAnnotation/AbstractValuesAwareNode.php
                 - packages/PostRector/Rector/AbstractPostRector.php
                 - src/Rector/AbstractRector.php
+                - src/Rector/AbstractCollectorRector.php
                 - src/Rector/AbstractScopeAwareRector.php
 
         -
@@ -252,8 +250,6 @@ parameters:
         - '#Cognitive complexity for "Rector\\TypeDeclaration\\PHPStan\\ObjectTypeSpecifier\:\:matchShortenedObjectType\(\)" is \d+, keep it under 11#'
         - '#Cognitive complexity for "Rector\\TypeDeclaration\\PHPStan\\ObjectTypeSpecifier\:\:narrowToFullyQualifiedOrAliasedObjectType\(\)" is \d+, keep it under 11#'
 
-        - '#Parameter \#1 \$node (.*?) of method Rector\\(.*?)\:\:refactorWithScope\(\) should be contravariant with parameter \$node \(PhpParser\\Node\) of method Rector\\Core\\Contract\\Rector\\ScopeAwarePhpRectorInterface\:\:refactorWithScope\(\)#'
-
         -
             message: '#Cognitive complexity for "Rector\\(.*?)Rector\:\:refactor\(\)" is \d+, keep it under 11#'
             paths:
@@ -415,7 +411,6 @@ parameters:
             message: '#Anonymous (variable|variables) in#'
             path: utils/ChangelogGenerator
 
-        # std class api
         -
             message: '#Parameter \#1 \$phpVersion of method Rector\\Config\\RectorConfig\:\:phpVersion\(\) expects 50200\|50300\|50400\|50500\|50600\|70000\|70100\|70200\|70300\|70400\|80000\|80100\|80200\|100000, (.*?) given#'
             path: rules-tests
@@ -551,10 +546,13 @@ parameters:
 
         # rector collectors
         - '#Creating new PHPStan\\Collectors\\CollectedData is not covered by backward compatibility promise\. The class might change in a minor PHPStan version#'
-        -
-            message: '#Anonymous function has an unused use \$configuration#'
-            path: packages/Parallel/Application/ParallelFileProcessor.php
 
-        # stmts aware interface
-        - '#Property Rector\\Core\\Contract\\PhpParser\\Node\\StmtsAwareInterface\:\:\$stmts#'
-        - '#undefined property (.*?)\\Core\\Contract\\PhpParser\\Node\\StmtsAwareInterface::\$stmts#'
+        # collectors
+        - '#Creating new PHPStan\\Node\\CollectedDataNode is not covered by backward compatibility promise\. The class might change in a minor PHPStan version#'
+
+        - '#class\-string<Rector\\Core\\Contract\\Rector\\CollectorRectorInterface\|Rector\\Core\\Contract\\Rector\\RectorInterface#'
+
+        - '#Parameter \#1 \$node \(PhpParser\\Node\\Stmt\\Class_\) of method Rector\\(.*?)Collector\:\:processNode\(\) should be contravariant with parameter \$node \(PhpParser\\Node\) of method PHPStan\\Collectors\\Collector<PhpParser\\Node,mixed>\:\:processNode\(\)#'
+
+        - '#Access to an undefined property (.*?)\\Core\\Contract\\PhpParser\\Node\\StmtsAwareInterface\:\:\$stmts#'
+        - '#Property Rector\\Core\\Contract\\PhpParser\\Node\\StmtsAwareInterface\:\:\$stmts \(array<PhpParser\\Node\\Stmt>\|null\) does not accept array<PhpParser\\Node\\Stmt\|null>#'

rector.php

@@ -13,6 +13,7 @@
 use Rector\PHPUnit\Set\PHPUnitSetList;
 use Rector\Set\ValueObject\LevelSetList;
 use Rector\Set\ValueObject\SetList;
+use Rector\TypeDeclaration\Collector\ParentClassCollector;
 use Rector\TypeDeclaration\Rector\ClassMethod\AddMethodCallBasedStrictParamTypeRector;
 use Rector\TypeDeclaration\Rector\StmtsAwareInterface\DeclareStrictTypesRector;
 
@@ -32,6 +33,10 @@
         PHPUnitSetList::PHPUNIT_100,
     ]);
 
+    // @todo collectors - just for testing purpose
+    $rectorConfig->collector(ParentClassCollector::class);
+    $rectorConfig->rule(\Rector\Privatization\Rector\Class_\FinalizeClassesWithoutChildrenCollectorRector::class);
+
     $rectorConfig->rules([DeclareStrictTypesRector::class]);
 
     $rectorConfig->paths([

rules-tests/Privatization/Rector/Class_/FinalizeClassesWithoutChildrenCollectorRector/FinalizeClassesWithoutChildrenCollectorRectorTest.php

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