Code Quality: Resolve the block processor's variable.undefined PHPStan errors

[westonruter]

Resolves the twenty variable.undefined errors in src/wp-includes/class-wp-block-processor.php, the densest single file in that baseline. variable.undefined is a rule level 1 error — the lowest level, which the ticket asks contributors to prioritize.

Background

WP_Block_Processor was introduced in r60939 (Core-61401, WP 6.9), and next_token() has had its present shape since that changeset. It scans for the next block delimiter in a while loop that assigns nine locals, continues past every rejection, and breaks only once a delimiter is confirmed:

while ( $at < $end ) {
    // …
    // This must be a block delimiter!
    $this->state = self::MATCHED;
    break;
}

// The end of the document was reached without a match.
if ( self::MATCHED !== $this->state ) {
    $this->state = self::COMPLETE;
    return false;
}

$this->matched_delimiter_at = $comment_opening_at;
// …and nineteen more reads of the loop's locals.

The code is correct: the break is the only path that sets MATCHED, so reaching the block below guarantees every local was assigned. PHPStan cannot make that connection, though — the loop may run zero times, and $this->state is a mutable property it does not track across iterations. So all twenty post-loop reads were reported and baselined when the rule level was raised to 1 in r63019.

The twenty errors break down as $namespace_at ×5, $name_at ×3, $name_length ×3, $comment_opening_at ×3, $has_closer ×2, and one each for $comment_closing_at, $has_void_flag, $json_at, and $json_length.

Approach

Rather than initializing the nine locals before the loop — which would silence the analyzer with defaults that are never meaningfully used — the scan is extracted into a private scan_next_delimiter() that returns the matched spans and flags:

$delimiter = $this->scan_next_delimiter( $text, $end, $after_prev_delimiter );
if ( null === $delimiter ) {
    /*
     * The scan set the terminating state; only a trailing HTML span
     * leaves a token still to be visited.
     */
    return self::HTML_SPAN === $this->state;
}

The values now arrive as an array shape rather than as locals that may never have been assigned, so the errors are resolved by construction. The three no-match paths (trailing HTML span, exhausted document, incomplete input) set the terminating state exactly as before and return null; the ten goto incomplete sites and their label move with the loop.

next_token() drops from 399 lines to 137.

Two things worth calling out for review:

• @phpstan-impure on the new method. Without it PHPStan assumes $this->state cannot have changed across the call and reports identical.alwaysFalse on the self::HTML_SPAN === $this->state line. The method genuinely is impure — it writes $this->state, $this->last_error, $this->after_previous_delimiter, the matched-delimiter spans, and the open-blocks stack. This matches how WP_HTML_Tag_Processor::parse_next_tag() is annotated.
• The trailing stack-op switch now reads $this->namespace_at / $this->name_at / $this->name_length, which are assigned from $delimiter about thirty lines above it and untouched in between. That makes it textually identical to its counterpart in the HTML_SPAN re-entry branch, where the two previously differed only in locals-versus-properties. The obvious follow-up — folding the two into one private method — is left out of scope here.

There is one small trade-off: a match now allocates a nine-element array where it previously wrote straight to locals. In a parser that runs on every post render that is a real cost, though a small one beside the strpos()/strspn() scanning it wraps. The zero-allocation alternative is to leave the loop in place and instead move the post-match block into a helper taking those nine values as parameters — same guarantee, but a ten-parameter private method.

Verification

The scan is moved verbatim; diff against the original loop shows changes at exactly four sites — the two match sites, which now return array( … ), and the two no-match returns, which now yield null.

• variable.undefined baseline: 473 → 453 entries' worth of errors (194 → 185 entries). The file no longer appears in the baseline at all. Total across all baselines: 1,464 → 1,444.
• phpstan-diff --changed --staged --base=HEAD: no errors on changed lines.
• PHPCS: clean.
• PHPUnit --group blocks: 632 tests, 3,345 assertions, passing. That includes all 222 Tests_Blocks_BlockProcessor* tests.

Trac ticket: Core-65817

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Locating the densest baselined file, diagnosing why the errors occur, performing the extraction, and drafting this description. The approach was directed and the result reviewed by me.

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Juqe7ccrs72NtpDUYz6JbX

[github-actions]

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[dmsnell]

As noted in the description, this trades runtime performance to make up for PHPStan’s limited ability to analyze the control-flow. The associative arrays as used in this patch carry an outsized cost, and the purpose here is to provide an indirect route around the weak type system.

So I am skeptical of this change and don’t think it should go in, but at a minimum, if it’s going to happen anyway, there are a few ways to adjust it. Unless I’m mistaken, they all suffer by adding needless runtime work on every request just to appease PHPStan.

• set a dummy value to every variable before scanning so that PHPStan can recognize they have values. this is probably the cheapest of the alternatives and these might be able to be initialized once vs. on every scan.
• create a record-style class like WP_Block_Processor_Intermediate_Indirect_State_Overhead_Because_Linter with the known properties and use that instead of the associative array. that still ends up copying memory that doesn’t need to happen, but it’s significantly more efficient than associative arrays, plus it provides natural typing and self-documentation through docblock comments on the class.
• add the noisy ignore-comments to hush up PHPStan. this adds no penalty on site owners or visitors, but pollutes the code and removes the opportunity for PHPStan to catch something real.

another option is to expand the constructor to accept these parameters as arguments, which is kind-of mentioned somewhat in the description. when I worked through that I decided against it because it doesn’t really make sense to expose these offsets and values as part of the constructor, but it would be better to do that than it would be have end-users pay for the incompleteness of our code scanners.

---

my tone here is surely a bit sour, but that’s only because we keep seeing PHPStan efforts push for awkward patterns that we wouldn’t choose if we didn’t have the tool claiming something is wrong.

in all truthfulness I appreciate the work you and others are doing to build a more complete type map of the project, and to elevate the use of static-analysis. I just wish the tooling were more robust and that it didn’t lead to as much needless churn, runtime overhead, code pollution, and accidental regressions as it does.

maybe of these approaches, the constructor is the least-offensive. did you try that route?

[westonruter]

@dmsnell Sorry you saw this. I left it a draft because I wanted to throw it up to get it up somewhere before further iteration, as I also have my doubts about these changes. Thanks for your feedback regardless!

[dmsnell]

all good @westonruter — I support the work despite my grumbles