Repository navigation
Split NodeScopeResolver into smaller services - #6433
Merged
Merged
Conversation
…ntsHandler The statement-list walk (processNodesWithStorage, doProcessStmtNodes, processStatementStep, the two-pass body walk), goto convergence, unreachable statement detection, variable mention collection and the statement-level PHPDoc handling (@var via processStmtVarAnnotation / emitVarTagChangedNode, @throws via getOverridingThrowPoints) now live in the StatementsHandler service. NodeScopeResolver keeps its public entry points and delegates; the handlers that called the moved public methods inject StatementsHandler instead. TemplateArgumentResolver, FileTypeMapper and the unresolvedTemplateArguments toggle move from the NodeScopeResolver constructor to StatementsHandler. Gatherer suspension goes through the new suspendNodeGatherers() / restoreNodeGatherers() pair. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014hgcbwfKQnye2coQXtX5XU
…sHandler processArgs(), processDroppedArgs() and their helpers (argument type gathering, acceptor selection, closure parameter types and bound $this, parameter out types, callback invalidation) now live in the ArgumentsHandler service. The call handlers inject it; NodeScopeResolver delegates to it from processAttributeGroups(). The nine parameter out/closure-this/closure-type extension collections and the implicitThrows parameter move from the NodeScopeResolver constructor to ArgumentsHandler. NodeScopeResolver exposes the stored-result walk modes through isReturningStoredExpressionResults() / isConsumingStoredExpressionResults(), and resolveCallableTypeForScope() becomes public. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014hgcbwfKQnye2coQXtX5XU
…ndler processAttributeGroups() now lives in the AttributesHandler service. The statement handlers and PropertyHooksProcessor inject it; NodeScopeResolver delegates to it from processParamNode(). ReflectionProvider and ArgumentsHandler, used only by attribute processing, leave the NodeScopeResolver constructor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014hgcbwfKQnye2coQXtX5XU
processClosureNode(), processArrowFunctionNode(), processImmediatelyCalledCallable() and refineClosureNodeScope() now live in the ClosureProcessor service, injected by ClosureHandler, ArrowFunctionHandler, FuncCallHandler and ArgumentsHandler. The callable parameter helpers (createCallableParameters(), createNativeCallableParameters(), resolveCallableTypeForScope()) move into ClosureParameterResolver, their only consumer besides the closure walk. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014hgcbwfKQnye2coQXtX5XU
…ocessor processParamNode() becomes ParametersProcessor::processParams(), which takes the whole Param[] list, so the function, method, property hook, closure and arrow function callers no longer loop over their parameters. AttributesHandler, used only by parameter processing, leaves the NodeScopeResolver constructor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014hgcbwfKQnye2coQXtX5XU
The virtual assign is a thin wrapper over AssignHandler::prepareTarget() and applyWrite(), so it now lives next to them as a public method and uses the handler's own VirtualExprResultHelper and ExpressionResultFactory instead of container lookups. The inc/dec handlers, FuncCallHandler, FuncCallScopeEffectsHelper, ForeachHandler, UnsetHandler and ArgumentsHandler inject AssignHandler. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014hgcbwfKQnye2coQXtX5XU
…hem from NodeScopeResolver IfHandler, ForHandler, WhileHandler, DoWhileHandler and ForeachHandler receive treatPhpDocTypesAsCertain, polluteScopeWithLoopInitialAssignments and polluteScopeWithAlwaysIterableForeach as #[AutowiredParameter]s, and ClassLikeHandler receives the ReadWritePropertiesExtension collection. NodeScopeResolver drops the accessors and the constructor parameters. Tests configure these through getAdditionalConfigFiles() now: cases that ran with non-default values move into sibling test classes backed by shared neon files, and the ReadWritePropertiesExtension doubles become services. RuleTestCase::getReadWritePropertiesExtensions(), shouldPolluteScopeWithLoopInitialAssignments() and shouldPolluteScopeWithAlwaysIterableForeach() no longer configure the analysis - overriding them with a value that differs from the configuration fails the test instead of silently changing its meaning. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014hgcbwfKQnye2coQXtX5XU
…rameterResolver ClosureTypeResolver built closure parameters through ClosureParameterResolver, and ClosureParameterResolver priced call arguments through ClosureTypeResolver::getClosureType() - a cycle hidden behind a container lookup. ClosureTypeResolver never passes call arguments, and only the call-argument refinement needs closure types, so the contextual part (array_map() and immediate-invocation values, the passed-to callable type) moves into ContextualClosureParameterResolver, injected into both resolvers. ClosureParameterResolver now injects ClosureTypeResolver directly, and ArgumentsHandler and ClosureProcessor inject both resolvers instead of looking them up. The one lookup left is ClosureProcessor::getParametersProcessor(): closure parameters walk their attributes, attribute arguments are call arguments walked by ArgumentsHandler, and those walk closure arguments through ClosureProcessor again - genuine recursion constructor injection cannot express. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014hgcbwfKQnye2coQXtX5XU
AssignHandler is its only caller, so it becomes a private method there, observing through the handler's own TemplateArgumentObserver. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014hgcbwfKQnye2coQXtX5XU
Collaborator
|
You've opened the pull request against the latest branch 2.3.x. PHPStan 2.3 is not going to be released for months. If your code is relevant on 2.2.x and you want it to be released sooner, please rebase your pull request and change its target to 2.2.x. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NodeScopeResolver goes from 3,956 to 1,070 lines. The extracted
#[AutowiredService]classes takeNodeScopeResolveras a method parameter (likePropertyHooksProcessor), so there is no constructor cycle with it and the walk's recursion depth is unchanged. NodeScopeResolver keeps the@apientry points and the statement/expression dispatchers.StatementsHandler– statement-list walk, goto convergence, unreachable statements, the two-pass body walk, statement-level@var/@throwsArgumentsHandler–processArgs(),processDroppedArgs()and their helpersAttributesHandler,ParametersProcessor(processParams()takesParam[])ClosureProcessor– closure and arrow function walking; the callable parameter helpers move intoClosureParameterResolverprocessVirtualAssign()(public) andcollectOffsetSetUsage()(private) move toAssignHandlerClosureTypeResolverandClosureParameterResolverdepended on each other through a container lookup. The context-only part (array_map() / immediate invocation values, passed-to callable type) never needs closure types, so it is nowContextualClosureParameterResolver, injected into both. The one lookup left isClosureProcessor::getParametersProcessor(): closure parameters walk their attributes, attribute arguments are call arguments, and those can contain closures again – genuine recursion that constructor injection cannot express.Analysis settings injected into the handlers
treatPhpDocTypesAsCertain,polluteScopeWithLoopInitialAssignments,polluteScopeWithAlwaysIterableForeachand theReadWritePropertiesExtensioncollection are injected into the handlers that read them instead of going throughNodeScopeResolveraccessors.BC note for extension tests:
RuleTestCase::getReadWritePropertiesExtensions(),shouldPolluteScopeWithLoopInitialAssignments()andshouldPolluteScopeWithAlwaysIterableForeach()no longer configure the analysis – set the parameters and register the extensions in a config file returned fromgetAdditionalConfigFiles(). Overriding them with a value that differs from the configuration fails the test with that instruction instead of silently changing its meaning.shouldTreatPhpDocTypesAsCertain()still feeds the scope and the rules under test, but the walk reads thetreatPhpDocTypesAsCertainparameter. In-repo tests that ran with non-default values moved into sibling test classes backed by shared neon files.🤖 Generated with Claude Code
https://claude.ai/code/session_014hgcbwfKQnye2coQXtX5XU