Skip to content

Handle goto and Label in top-level file statements processed by processNodes#5721

Merged
ondrejmirtes merged 2 commits into
phpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-5xhb925
May 20, 2026
Merged

Handle goto and Label in top-level file statements processed by processNodes#5721
ondrejmirtes merged 2 commits into
phpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-5xhb925

Conversation

@phpstan-bot
Copy link
Copy Markdown
Collaborator

Summary

Top-level goto and labels (not enclosed in a function, class, or namespace block) produced false positives because processNodes — the entry point for root-level AST statements — processed each statement individually without goto/label awareness. The same constructs worked correctly inside functions because function bodies go through processStmtNodesInternalWithoutFlushingPendingFibers, which already has full goto handling.

Changes

  • src/Parser/GotoLabelVisitor.php: afterTraverse now calls processStatementList on the root-level node array before popScope. Previously, processStatementList was only called from leaveNode for nodes with a stmts property (like Function_, If_, Namespace_), so the top-level statement list was never processed and labels never received HAS_BACKWARD_GOTO_ATTRIBUTE / NESTED_BACKWARD_GOTO_LABELS_ATTRIBUTE.

  • src/Analyser/NodeScopeResolver.php: processNodes now mirrors the goto handling from processStmtNodesInternalWithoutFlushingPendingFibers:

    • Collects exit points from statement results
    • Allows Label nodes to be processed after termination
    • Handles NESTED_BACKWARD_GOTO_LABELS_ATTRIBUTE pre-analysis loop
    • Merges forward goto scopes when encountering a Label
    • Handles HAS_BACKWARD_GOTO_ATTRIBUTE backward goto loop

Root cause

Two things were missing for top-level goto support:

  1. GotoLabelVisitor never processed the root-level statement list (only sub-lists inside AST nodes), so top-level labels/gotos never got the attributes that trigger scope analysis loops.
  2. processNodes processed each top-level statement independently without tracking exit points or handling label scope merging, so even if the attributes had been set, they would not have been acted upon.

Analogous cases probed

  • Namespace-level goto (inside namespace Foo { ... }): already works because Namespace_ bodies go through processStmtNodesInternal. Verified both before and after this fix.

Test

  • tests/PHPStan/Analyser/nsrt/bug-14660.php — function-level forward and backward goto (confirms existing behavior is preserved)
  • tests/PHPStan/Analyser/nsrt/bug-14660-top-level.php — top-level forward and backward goto (reproduces the bug; fails without the fix, passes with it)

Fixes phpstan/phpstan#14660

…processNodes`

- GotoLabelVisitor::afterTraverse now calls processStatementList on root-level
  nodes so that top-level labels get HAS_BACKWARD_GOTO_ATTRIBUTE and statements
  get NESTED_BACKWARD_GOTO_LABELS_ATTRIBUTE
- NodeScopeResolver::processNodes now tracks exit points, merges forward goto
  scopes at labels, and runs the backward goto pre-analysis loop — mirroring
  the goto handling already present in processStmtNodesInternalWithoutFlushingPendingFibers
- Namespace-level goto was already handled correctly because Namespace_ bodies
  go through processStmtNodesInternal
Copy link
Copy Markdown
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to deduplicate the code you added, gotos and labels already have similar loops added

…des and processStmtNodesInternalWithoutFlushingPendingFibers

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

Pushed. The refactoring extracts two shared helper methods from the duplicated goto/label handling code:

  1. resolveBackwardGotoScope() — the convergence do-while loop that iterates until scope stabilizes, used for both NESTED_BACKWARD_GOTO_LABELS_ATTRIBUTE and HAS_BACKWARD_GOTO_ATTRIBUTE handling. Parameterized by a Closure for goto name matching and a bool for whether to merge body scope each iteration (the two variants differ in this).

  2. mergeForwardGotoExitPoints() — merges forward goto exit points into the current scope when a Label node is encountered.

Both processNodes and processStmtNodesInternalWithoutFlushingPendingFibers now call these shared methods, eliminating ~63 net lines of duplication. All tests pass and make phpstan reports no errors.

@ondrejmirtes ondrejmirtes enabled auto-merge (squash) May 20, 2026 13:50
@ondrejmirtes ondrejmirtes merged commit f0931f5 into phpstan:2.1.x May 20, 2026
648 of 658 checks passed
@ondrejmirtes ondrejmirtes deleted the create-pull-request/patch-5xhb925 branch May 20, 2026 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants