Skip to content

Split LazyContainerFactory::create() into named sections - #8233

Merged
TomasVotruba merged 1 commit into
mainfrom
fix-stale-skip-voter-comment
Jul 30, 2026
Merged

Split LazyContainerFactory::create() into named sections#8233
TomasVotruba merged 1 commit into
mainfrom
fix-stale-skip-voter-comment

Conversation

@TomasVotruba

Copy link
Copy Markdown
Member

LazyContainerFactory is 739 lines across three methods, and create() is 319 of them — every container registration in one body, separated only by comments like // caching, // tagged services, // node name resolvers.

Each of those sections becomes a private method, so create() now reads as a table of contents:

public function create(): RectorConfig
{
    $rectorConfig = new RectorConfig();

    $rectorConfig->import(__DIR__ . '/../../config/config.php');

    $this->registerConsole($rectorConfig);
    $this->registerFileProcessing($rectorConfig);
    $this->registerCachingAndResettables($rectorConfig);
    $this->registerTypeMappers($rectorConfig);
    $this->registerNodeNameResolvers($rectorConfig);
    $this->registerRectorAutowiring($rectorConfig);
    $this->registerTaggedServices($rectorConfig);
    $this->registerAnnotationToAttributeSetters($rectorConfig);
    $this->registerNodeVisitorsAndPhpDoc($rectorConfig);

    return $rectorConfig;
}

No statement is added, removed, or reordered — the sections are called in exactly their original order. That is deliberate rather than incidental: tagged service lists are consumed in registration order, so moving any block would be a behavior change.

This is one of the few refactors that costs lines rather than saving them: +40 / -3, all of it method signatures. The win is structural, not size.

Verification

Beyond the usual gates, I compared the built container on this branch against main — the binding keys, aliases, contextual bindings, and the ordered tag lists — and they are identical.

To make sure the comparison was real (a shared vendor/ autoloader would otherwise resolve both runs to the same sources), the baseline ran in a separate worktree with a checkout-first autoloader prepended. It loaded main's factory with 3 methods and no registerConsole(); this branch's has 12 and does.

I also checked that no local variable was shared across the extracted blocks — $rectorConfig is the only one that flows between them, so extraction cannot change semantics.

Drive-by

One stale comment, left over from removing ClassSkipVoter in #8226:

-        // shared state: collects used skips across skipper voters and the file processor
+        // shared state: collects used skips across the skipper, the path skipper and the file processor

Skip voters no longer exist; the actual consumers are Skipper, PathSkipper and ApplicationFileProcessor.

create() was one 319-line method holding every container registration, split
only by comments. Turn each of those comment-delimited sections into a private
method, so create() reads as a list of what gets registered.

Call order is unchanged, which matters: tagged service lists are consumed in
registration order.

Also correct a comment left over from removing ClassSkipVoter - UsedSkipCollector
is shared by Skipper, PathSkipper and ApplicationFileProcessor, not by "skipper
voters", which no longer exist.
@TomasVotruba
TomasVotruba enabled auto-merge (squash) July 30, 2026 21:23
@TomasVotruba
TomasVotruba merged commit ab27b92 into main Jul 30, 2026
64 of 65 checks passed
@TomasVotruba
TomasVotruba deleted the fix-stale-skip-voter-comment branch July 30, 2026 21:24
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