Skip to content

Wire RichParser node visitors via DI factory instead of private property hack - #8215

Merged
TomasVotruba merged 1 commit into
mainfrom
remove-privates-accessor-rich-parser
Jul 28, 2026
Merged

Wire RichParser node visitors via DI factory instead of private property hack#8215
TomasVotruba merged 1 commit into
mainfrom
remove-privates-accessor-rich-parser

Conversation

@TomasVotruba

Copy link
Copy Markdown
Member

Follow-up to #8208 — same goal, no PrivatesAccessor on PHPStan internals.

Before

PHPStanContainerMemento::removeRichVisitors() read the private RichParser::$nodeVisitors property, filtered it, and wrote a new collection back. Every PHPStan internal change breaks it (that is what #8208 had to repair, and the 2.2.6 crash before it).

$nodeVisitorsCollection = $privatesAccessor->getPrivateProperty($richParser, 'nodeVisitors');
// ... filter ...
$privatesAccessor->setPrivateProperty($richParser, 'nodeVisitors', new DirectExtensionsCollection($nodeVisitorsToKeep));

After

A small factory service builds the parser through the public constructor, and the config picks the visitors:

    currentPhpVersionRichParser:
        class: PHPStan\Parser\RichParser
        factory: @rectorRichParserFactory::create()
        arguments!: []
        autowired: no

    rectorRichParserFactory:
        class: Rector\DependencyInjection\PHPStan\RichParserFactory
        arguments:
            parser: @currentPhpVersionPhpParser
            nodeVisitors:
                - @PHPStan\Parser\AnonymousClassVisitor
                - @PHPStan\Parser\ArrayMapArgVisitor
        autowired: no

PHPStanContainerMemento is deleted.

Why a factory, and not just a nodeVisitors: argument

A plain argument in the config is silently dropped. PHPStan\DependencyInjection\AutowiredExtensionsExtension::beforeCompile() walks all definitions and overwrites every #[AutowiredExtensions] parameter with the lazy container-backed collection — but it skips definitions whose creator is not the class itself, i.e. factory-created ones.

Do we still need the filtering at all?

Checked: with all 23 visitors enabled the whole test suite passes and bin/rector process --dry-run over this repo produces identical output — the #9492 node-replacement bug is fixed upstream. The filter is still worth keeping for speed, running all visitors on every parsed file costs roughly 10% more CPU on a full run of this repo (~510s -> ~550-600s user time, 30s -> 37s wall).

Regression test for #9492 (PHPStanPrinterTest) stays, now without the manual visitor-removal call, since the container wiring does it.

…rty hack

Replace PHPStanContainerMemento reflection with a RichParserFactory service,
registered in config/phpstan/parser.neon. The factory builds RichParser with a
DirectExtensionsCollection of the two visitors Rector needs, so no private
property of PHPStan internals is touched.

A plain "nodeVisitors" argument in the config is not enough: PHPStan's
AutowiredExtensionsExtension::beforeCompile() overwrites that argument for every
service definition that instantiates the class directly. Factory-created
definitions are skipped, so the explicit visitor list survives.
@TomasVotruba
TomasVotruba force-pushed the remove-privates-accessor-rich-parser branch from c0f210e to 070ca62 Compare July 28, 2026 13:10
@TomasVotruba
TomasVotruba merged commit bf9cc21 into main Jul 28, 2026
65 checks passed
@TomasVotruba
TomasVotruba deleted the remove-privates-accessor-rich-parser branch July 28, 2026 13:13
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.

1 participant