Handle goto and Label in top-level file statements processed by processNodes#5721
Conversation
…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
ondrejmirtes
left a comment
There was a problem hiding this comment.
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>
|
Pushed. The refactoring extracts two shared helper methods from the duplicated goto/label handling code:
Both |
Summary
Top-level
gotoand labels (not enclosed in a function, class, or namespace block) produced false positives becauseprocessNodes— 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 throughprocessStmtNodesInternalWithoutFlushingPendingFibers, which already has full goto handling.Changes
src/Parser/GotoLabelVisitor.php:afterTraversenow callsprocessStatementListon the root-level node array beforepopScope. Previously,processStatementListwas only called fromleaveNodefor nodes with astmtsproperty (likeFunction_,If_,Namespace_), so the top-level statement list was never processed and labels never receivedHAS_BACKWARD_GOTO_ATTRIBUTE/NESTED_BACKWARD_GOTO_LABELS_ATTRIBUTE.src/Analyser/NodeScopeResolver.php:processNodesnow mirrors the goto handling fromprocessStmtNodesInternalWithoutFlushingPendingFibers:Labelnodes to be processed after terminationNESTED_BACKWARD_GOTO_LABELS_ATTRIBUTEpre-analysis loopLabelHAS_BACKWARD_GOTO_ATTRIBUTEbackward goto loopRoot cause
Two things were missing for top-level goto support:
GotoLabelVisitornever 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.processNodesprocessed 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 Foo { ... }): already works becauseNamespace_bodies go throughprocessStmtNodesInternal. 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