Proper fix to handle ClassReflection refresh on ClassLike changed

.github/workflows/rector_drupal_rector_dev.yaml

@@ -12,6 +12,7 @@ env:
 jobs:
     rector_drupal_rector_dev:
         runs-on: ubuntu-latest
+        timeout-minutes: 4
 
         steps:
             -

.github/workflows/rector_laravel_rector_dev.yaml

@@ -13,6 +13,7 @@ env:
 jobs:
     rector_laravel_rector_dev:
         runs-on: ubuntu-latest
+        timeout-minutes: 4
 
         steps:
             -

src/Application/ChangedNodeScopeRefresher.php

@@ -27,17 +27,22 @@
 use PhpParser\Node\Stmt\Declare_;
 use PhpParser\Node\Stmt\Else_;
 use PhpParser\Node\Stmt\ElseIf_;
+use PhpParser\Node\Stmt\Enum_;
 use PhpParser\Node\Stmt\Expression;
 use PhpParser\Node\Stmt\If_;
+use PhpParser\Node\Stmt\Interface_;
 use PhpParser\Node\Stmt\Property;
 use PhpParser\Node\Stmt\Static_;
 use PhpParser\Node\Stmt\Use_;
 use PhpParser\Node\UseItem;
 use PHPStan\Analyser\MutatingScope;
+use PHPStan\Reflection\ClassReflection;
 use Rector\Exception\ShouldNotHappenException;
 use Rector\NodeAnalyzer\ScopeAnalyzer;
 use Rector\NodeTypeResolver\Node\AttributeKey;
+use Rector\NodeTypeResolver\PHPStan\Scope\AstClassReflectionFactory;
 use Rector\NodeTypeResolver\PHPStan\Scope\PHPStanNodeScopeResolver;
+use Rector\NodeTypeResolver\PHPStan\Scope\ScopeFactory;
 use Rector\PhpDocParser\NodeTraverser\SimpleCallableNodeTraverser;
 
 /**
@@ -48,7 +53,9 @@
     public function __construct(
         private PHPStanNodeScopeResolver $phpStanNodeScopeResolver,
         private ScopeAnalyzer $scopeAnalyzer,
-        private SimpleCallableNodeTraverser $simpleCallableNodeTraverser
+        private SimpleCallableNodeTraverser $simpleCallableNodeTraverser,
+        private AstClassReflectionFactory $astClassReflectionFactory,
+        private ScopeFactory $scopeFactory
     ) {
     }
 
@@ -81,6 +88,8 @@ public function refresh(Node $node, string $filePath, ?MutatingScope $mutatingSc
         // a class-like scope is already inside the class, so PHPStan cannot enter it again and would fail
         // on the class node, leaving its methods with the class scope; process the class stmts directly instead
         if ($node instanceof ClassLike && $mutatingScope->isInClass()) {
+            $mutatingScope = $this->refreshClassReflection($node, $mutatingScope);
+
             // the class node itself is not part of its stmts, e.g. an Enum_ replaced by a new Class_ has no scope yet
             $node->setAttribute(AttributeKey::SCOPE, $mutatingScope);
             $this->phpStanNodeScopeResolver->processNodes($node->stmts, $filePath, $mutatingScope);
@@ -91,6 +100,43 @@ public function refresh(Node $node, string $filePath, ?MutatingScope $mutatingSc
         $this->phpStanNodeScopeResolver->processNodes($stmts, $filePath, $mutatingScope);
     }
 
+    /**
+     * The in-class scope holds the class reflection of the original file, which does not know about
+     * members the rule just added or removed; re-enter the class with the reflection of the changed node
+     */
+    private function refreshClassReflection(ClassLike $classLike, MutatingScope $mutatingScope): MutatingScope
+    {
+        if (! $classLike instanceof Class_ && ! $classLike instanceof Interface_ && ! $classLike instanceof Enum_) {
+            return $mutatingScope;
+        }
+
+        if ($classLike instanceof Class_ && $classLike->isAnonymous()) {
+            return $mutatingScope;
+        }
+
+        $classReflection = $this->astClassReflectionFactory->create($classLike, $mutatingScope);
+        if (! $classReflection instanceof ClassReflection) {
+            return $mutatingScope;
+        }
+
+        $currentClassReflection = $mutatingScope->getClassReflection();
+        if (! $currentClassReflection instanceof ClassReflection || $currentClassReflection->getName() !== $classReflection->getName()) {
+            return $mutatingScope;
+        }
+
+        $classOutsideScope = $this->scopeFactory->createFromFile($mutatingScope->getFile());
+        if ($mutatingScope->isDeclareStrictTypes()) {
+            $classOutsideScope = $classOutsideScope->enterDeclareStrictTypes();
+        }
+
+        $namespace = $mutatingScope->getNamespace();
+        if ($namespace !== null) {
+            $classOutsideScope = $classOutsideScope->enterNamespace($namespace);
+        }
+
+        return $classOutsideScope->enterClass($classReflection);
+    }
+
     /**
      * @return Stmt[]
      */

src/DependencyInjection/LazyContainerFactory.php

@@ -9,10 +9,12 @@
 use PhpParser\Lexer;
 use PHPStan\Analyser\NodeScopeResolver;
 use PHPStan\Analyser\ScopeFactory;
+use PHPStan\BetterReflection\Reflector\Reflector;
 use PHPStan\Parser\Parser;
 use PHPStan\Php\PhpVersionFactory;
 use PHPStan\PhpDoc\TypeNodeResolver;
 use PHPStan\PhpDocParser\ParserConfig;
+use PHPStan\Reflection\ClassReflectionFactory;
 use PHPStan\Reflection\ReflectionProvider;
 use Rector\Application\FileProcessor;
 use Rector\Application\Provider\CurrentFileProvider;
@@ -110,6 +112,8 @@ final class LazyContainerFactory
         NodeScopeResolver::class,
         ReflectionProvider::class,
         PhpVersionFactory::class,
+        Reflector::class,
+        ClassReflectionFactory::class,
     ];
 
     /**

src/NodeTypeResolver/PHPStan/Scope/AstClassReflectionFactory.php

@@ -0,0 +1,68 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\NodeTypeResolver\PHPStan\Scope;
+
+use PhpParser\Node\Name;
+use PhpParser\Node\Stmt\Class_;
+use PhpParser\Node\Stmt\Enum_;
+use PhpParser\Node\Stmt\Interface_;
+use PhpParser\Node\Stmt\Namespace_;
+use PHPStan\Analyser\MutatingScope;
+use PHPStan\BetterReflection\Reflection\Adapter\ReflectionClass as ReflectionClassAdapter;
+use PHPStan\BetterReflection\Reflection\Adapter\ReflectionEnum as ReflectionEnumAdapter;
+use PHPStan\BetterReflection\Reflection\ReflectionClass;
+use PHPStan\BetterReflection\Reflection\ReflectionEnum;
+use PHPStan\BetterReflection\Reflector\Reflector;
+use PHPStan\BetterReflection\SourceLocator\Ast\Strategy\NodeToReflection;
+use PHPStan\BetterReflection\SourceLocator\Located\LocatedSource;
+use PHPStan\Reflection\ClassReflection;
+use PHPStan\Reflection\ClassReflectionFactory;
+
+/**
+ * The class reflection from the reflection provider is built from the original file,
+ * so it does not know about members a rule added or removed; build it from the current class node instead
+ *
+ * @see \PHPStan\Analyser\StmtHandler\ClassLikeHandler::createAstClassReflection()
+ */
+final readonly class AstClassReflectionFactory
+{
+    public function __construct(
+        private Reflector $reflector,
+        private ClassReflectionFactory $classReflectionFactory,
+    ) {
+    }
+
+    public function create(Class_|Interface_|Enum_ $classLike, MutatingScope $mutatingScope): ?ClassReflection
+    {
+        $filePath = $mutatingScope->getFile();
+        $namespace = $mutatingScope->getNamespace();
+
+        $betterReflectionClass = new NodeToReflection()->__invoke(
+            $this->reflector,
+            $classLike,
+            new LocatedSource(null, null, $filePath),
+            $namespace !== null ? new Namespace_(new Name($namespace)) : null
+        );
+
+        if (! $betterReflectionClass instanceof ReflectionClass) {
+            return null;
+        }
+
+        $reflectionClassAdapter = $betterReflectionClass instanceof ReflectionEnum
+            ? new ReflectionEnumAdapter($betterReflectionClass)
+            : new ReflectionClassAdapter($betterReflectionClass);
+
+        // the member caches are keyed by the class reflection cache key,
+        // so each rebuilt reflection needs its own key not to read members of the former one
+        return $this->classReflectionFactory->create(
+            $betterReflectionClass->getName(),
+            $reflectionClassAdapter,
+            null,
+            null,
+            null,
+            uniqid($filePath . ':', true)
+        );
+    }
+}

tests/Issues/ClassReflectionRefresh/ClassReflectionRefreshTest.php

@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Issues\ClassReflectionRefresh;
+
+use Iterator;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+
+final class ClassReflectionRefreshTest 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';
+    }
+}

tests/Issues/ClassReflectionRefresh/Fixture/add_trait_once.php.inc

@@ -0,0 +1,26 @@
+<?php
+
+namespace Rector\Tests\Issues\ClassReflectionRefresh\Fixture;
+
+final class AddTraitOnce
+{
+    public function run()
+    {
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\Issues\ClassReflectionRefresh\Fixture;
+
+final class AddTraitOnce
+{
+    use \Rector\Tests\Issues\ClassReflectionRefresh\Source\SomeTrait;
+    public function run()
+    {
+    }
+}
+
+?>

tests/Issues/ClassReflectionRefresh/Fixture/add_trait_with_imported_parent.php.inc

@@ -0,0 +1,32 @@
+<?php
+
+namespace Rector\Tests\Issues\ClassReflectionRefresh\Fixture;
+
+use Rector\Tests\Issues\ClassReflectionRefresh\Source\SomeParentClass;
+
+final class AddTraitWithImportedParent extends SomeParentClass
+{
+    public function run(): SomeParentClass
+    {
+        return $this;
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\Issues\ClassReflectionRefresh\Fixture;
+
+use Rector\Tests\Issues\ClassReflectionRefresh\Source\SomeParentClass;
+
+final class AddTraitWithImportedParent extends SomeParentClass
+{
+    use \Rector\Tests\Issues\ClassReflectionRefresh\Source\SomeTrait;
+    public function run(): SomeParentClass
+    {
+        return $this;
+    }
+}
+
+?>

tests/Issues/ClassReflectionRefresh/Fixture/skip_already_used_imported_trait.php.inc

@@ -0,0 +1,10 @@
+<?php
+
+namespace Rector\Tests\Issues\ClassReflectionRefresh\Fixture;
+
+use Rector\Tests\Issues\ClassReflectionRefresh\Source\SomeTrait;
+
+final class SkipAlreadyUsedImportedTrait
+{
+    use SomeTrait;
+}

tests/Issues/ClassReflectionRefresh/Source/AddTraitByReflectionRector.php

@@ -0,0 +1,58 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Issues\ClassReflectionRefresh\Source;
+
+use PhpParser\Node;
+use PhpParser\Node\Name\FullyQualified;
+use PhpParser\Node\Stmt\Class_;
+use PhpParser\Node\Stmt\TraitUse;
+use PHPStan\Reflection\ClassReflection;
+use Rector\Rector\AbstractRector;
+use Rector\Reflection\ReflectionResolver;
+use Rector\RuleDoc\RuleDefinition;
+
+/**
+ * Decides on the class reflection only, so it adds the trait again on every pass
+ * unless the class reflection reflects the changed class node
+ */
+final class AddTraitByReflectionRector extends AbstractRector
+{
+    public function __construct(
+        private readonly ReflectionResolver $reflectionResolver
+    ) {
+    }
+
+    public function getRuleDefinition(): RuleDefinition
+    {
+        return new RuleDefinition('', []);
+    }
+
+    /**
+     * @return array<class-string<Node>>
+     */
+    public function getNodeTypes(): array
+    {
+        return [Class_::class];
+    }
+
+    /**
+     * @param Class_ $node
+     */
+    public function refactor(Node $node): ?Class_
+    {
+        $classReflection = $this->reflectionResolver->resolveClassReflection($node);
+        if (! $classReflection instanceof ClassReflection) {
+            return null;
+        }
+
+        if ($classReflection->hasTraitUse(SomeTrait::class)) {
+            return null;
+        }
+
+        $node->stmts = array_merge([new TraitUse([new FullyQualified(SomeTrait::class)])], $node->stmts);
+
+        return $node;
+    }
+}

tests/Issues/ClassReflectionRefresh/Source/SomeParentClass.php

@@ -0,0 +1,9 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Issues\ClassReflectionRefresh\Source;
+
+abstract class SomeParentClass
+{
+}

tests/Issues/ClassReflectionRefresh/Source/SomeTrait.php

@@ -0,0 +1,9 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Issues\ClassReflectionRefresh\Source;
+
+trait SomeTrait
+{
+}

tests/Issues/ClassReflectionRefresh/config/configured_rule.php

@@ -0,0 +1,9 @@
+<?php
+
+declare(strict_types=1);
+
+use Rector\Config\RectorConfig;
+use Rector\Tests\Issues\ClassReflectionRefresh\Source\AddTraitByReflectionRector;
+
+return RectorConfig::configure()
+    ->withRules([AddTraitByReflectionRector::class]);