Skip to content

[CodingStyle] Use native php-parser node API over class reflection - #8218

Merged
TomasVotruba merged 2 commits into
mainfrom
native-node-api-over-reflection
Jul 29, 2026
Merged

[CodingStyle] Use native php-parser node API over class reflection#8218
TomasVotruba merged 2 commits into
mainfrom
native-node-api-over-reflection

Conversation

@TomasVotruba

Copy link
Copy Markdown
Member

Draft. Companion to #8217. Same thread: replace PHPStan with native php-parser logic where the answer is already in the node being refactored — no reflection lookup, no file re-parse.

Why only these

The constraint that makes this safe and faster: the node is already in hand. That rules out AstResolver-style replacements, which trade a reflection lookup for parsing a whole source file — the thing #8100, #8099, #8196 and friends have been removing.

PhpParser\Node\Stmt\Class_ natively exposes:

isAbstract, isAnonymous, isFinal, isReadonly,
getMethods, getMethod, getProperties, getProperty, getConstants, getTraitUses

All three rules here declare getNodeTypes(): [Class_::class], so $node is the class whose reflection they were querying.

Changes

ThisCallOnStaticMethodToStaticCallRector$class was already captured in the enclosing closure, so no plumbing needed. isFinalByKeyword() maps exactly to isFinal(): both mean the final keyword, neither counts @final:

-            $objectReference = $this->resolveClassSelf($classReflection, $subNode);
+            $objectReference = $this->resolveClassSelf($class, $subNode);
...
-    private function resolveClassSelf(ClassReflection $classReflection, MethodCall $methodCall): string
+    private function resolveClassSelf(Class_ $class, MethodCall $methodCall): string
     {
-        if ($classReflection->isFinalByKeyword()) {
+        if ($class->isFinal()) {
             return ObjectReference::SELF;
         }

RemoveUnusedPrivateMethodRector — anonymous-ness is a property of the class, but was being re-checked per method inside the loop. Hoisted to an early return:

         if ($node->getMethods() === []) {
             return null;
         }
 
+        // unreliable to detect on anonymous class: doesn't make sense
+        if ($node->isAnonymous()) {
+            return null;
+        }
+
         $hasChanged = false;
...
-        if ($classReflection->isAnonymous()) {
-            return true;
-        }
-
         // skip magic methods

MakeInheritedMethodVisibilitySameAsParentRector — moved above the reflection resolution, so anonymous classes now bail before the lookup happens at all:

+        if ($node->isAnonymous()) {
+            return null;
+        }
+
         $classReflection = $this->reflectionResolver->resolveClassReflection($node);
         if (! $classReflection instanceof ClassReflection) {
             return null;
         }
 
-        if ($classReflection->isAnonymous()) {
-            return null;
-        }
-
         $parentClassReflections = $classReflection->getParents();

Scope

No behaviour change. Two of the three also skip work earlier than before.

This is deliberately the whole verified set for Class_-node rules, not a sample — a scan of every rule whose getNodeTypes() is [Class_::class] found only these three asking AST-answerable questions of their own reflection. Sites on ancestor reflections ($parentClassReflection->isAnonymous(), ->isTrait()) need real reflection and are untouched.

Touches the same shouldSkip() as #8217; whichever lands second needs a trivial rebase.

isAnonymous() and isFinalByKeyword() on the node own class reflection
answer questions the Class_ node already answers via isAnonymous() and
isFinal(). Read them off the node instead, and hoist the anonymous-class
checks above the reflection lookup so it is skipped entirely.
@TomasVotruba

Copy link
Copy Markdown
Member Author

@copilot resolve the merge conflicts in this pull request

@TomasVotruba
TomasVotruba marked this pull request as ready for review July 29, 2026 16:57
…reflection

# Conflicts:
#	rules/DeadCode/Rector/ClassMethod/RemoveUnusedPrivateMethodRector.php

Copilot AI commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved merge conflicts and merged latest origin/main into this branch in commit 564585e121.

@TomasVotruba
TomasVotruba merged commit 7c4ef12 into main Jul 29, 2026
65 checks passed
@TomasVotruba
TomasVotruba deleted the native-node-api-over-reflection branch July 29, 2026 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants