[6.x] Support @param-closure-this docblock tag#11853
Draft
alies-dev wants to merge 6 commits into
Draft
Conversation
Binds $this inside a closure or arrow-function argument to a caller-specified type. Mirrors PHPStan's @param-closure-this and is already shipped by Laravel (Macroable::macro, Manager::extend), Carbon, and Pest. Without this tag, closure bodies that reference $this trip InvalidScope even when the receiving method binds the closure via Closure::call() / Closure::bind(). Implementation: - Parse @param-closure-this, @psalm-param-closure-this, @phpstan-param-closure-this in the docblock parser; store on a new FunctionLikeParameter::$closure_this_type field next to $out_type. - At the call site, ArgumentsAnalyzer resolves the param's closure_this_type against self / static / template parameters (self resolves to the declaring class via declaring_method_ids so inherited static methods bind correctly; static resolves to the runtime called class) and stamps the resolved Union as the psalm-closure-this-type attribute on the Closure / ArrowFunction PHP-Parser node. - ClosureAnalyzer reads the attribute, overrides $this in the closure body use_context, and FunctionLikeAnalyzer calls setFQCLN on the inner StatementsAnalyzer so top-level closures pass the InvalidScope check. - Property fetches inside the bound closure resolve against the bound class storage rather than the caller's class. Closes vimeo#11851
- TypeExpander now receives parent_class (from declaring class storage) and the called class's final flag, so @param-closure-this parent and final classes expand correctly. - TypeTokenizer in handleParamClosureThis now receives the declaring class's name and parent_class so compound expressions involving self/parent in the tag (e.g., self::TYPE_ALIAS) tokenize correctly during scanning. - TemplateStandinTypeReplacer now receives $context->self as $calling_class, matching the @param-out pattern. - ClosureAnalyzer validates that the bound class actually exists in storage before threading it through; an unknown class falls back to the unbound path instead of corrupting the closure's FQCLN. - addContextProperties keeps the pre-existing $statements_analyzer->getParentFQCLN() for unbound closures, isolating the bound-class parent_class lookup to the bound path. - Tests: cover parent, self with a class constant in scope, and the inheritance cases from the previous commit.
- Parser now strips the leading ... from a variadic param name (e.g., '@param-closure-this Bound ...$cbs') after stripping the optional &. Without this, the parameter-name lookup in handleParamClosureThis searched for the literal '..cbs' against storage names like 'cbs' and never matched, so the hint was silently dropped for variadic callbacks. - Tests: cover variadic closure binding and class-generic template binding (the latter pins down the case where T is a class-level template, which unlike free-function templates is already resolved at arg-evaluation time). Free-function template binding (e.g. '@template T' on a function with '@param-closure-this T $cb') remains a known limitation in this PR because template inference for free-function params runs in checkArgumentsMatch after analyze() has already evaluated the closure body.
- ArgumentsAnalyzer now nulls the psalm-closure-this-type AST attribute on every Closure/ArrowFunction arg before deciding whether to stamp the hint, so a prior bound-call to the same closure node (or a previous method-resolution candidate) cannot leak its binding into a subsequent unbound code path. Adds a regression test. - Docs: announce the @phpstan-param-closure-this alias and list parent among the supported bound-type forms.
alies-dev
changed the title
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
1 participant
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.
Adds support for
@param-closure-this <type> $param, which binds$thisinside aClosureor arrow-function argument to a caller-specified type. PHPStan has shipped this since March 2024 (commit), andphpstan/phpdoc-parseralready recognizes the token. Closes #11851.The tag exists in source today in widely used packages. Laravel's
Macroable::macro(every facade,Collection,Builder,Response,Route,Request) and the elevenManager::extendsiblings (AuthManager,CacheManager,FilesystemManager, etc.) all ship it. Carbon'sMacrotrait carries it on five files. Pest'stest/it/beforeEach/afterEachuse it to bind$thistoTestCall. Without Psalm honoring the tag, every$this->...call inside those closures tripsInvalidScope.At the call site,
ArgumentsAnalyzerresolves the parameter'sclosure_this_typeagainstself,static, and template parameters and stamps the resolved Union as apsalm-closure-this-typeattribute on the Closure / ArrowFunction node.selfresolves to the declaring class (viadeclaring_method_ids), so an inheritedBase::runwith@param-closure-this selfstill binds toBasewhen called asChild::run.staticresolves to the runtime called class.ClosureAnalyzerreads the attribute, overrides$thisin the closure body'suse_context, andFunctionLikeAnalyzercallssetFQCLNon the innerStatementsAnalyzerso closures passed at top level pass theInvalidScopecheck. Property fetches inside the bound closure resolve against the bound class storage rather than the caller's class.The diff is ~300 LoC (~150 of analyzer code, ~150 of tests), matching the issue's estimate. New storage field on
FunctionLikeParametermirrors$out_type. NewhandleParamClosureThisinFunctionLikeDocblockScannermirrorshandleParamOut. Tag is registered inDocComment::PSALM_ANNOTATIONSand recognized viacombined_tagsfor thepsalm-/phpstan-prefixes.Tests in
tests/ParamClosureThisTest.phpcover the documented use cases (explicit class, arrow function,staticon a Macroable-style helper,$thisliteral,@phpstan-and@psalm-aliases, binding from inside a method,selfresolving to the declaring class on inherited static methods,staticresolving to the called class) plus two invalid-code cases (caller's property is not visible inside the bound closure, undefined method on the bound class). Psalm self-analysis is clean. The 76InternalCallMapHandlerTestfailures in the full suite reproduce on unmodifiedorigin/6.xand are unrelated environmental mismatches.Marked draft because I'd like maintainer review on (a) the AST-attribute approach for plumbing the bound type from
ArgumentsAnalyzerintoClosureAnalyzer, and (b) thedeclaring_method_idslookup forselfresolution. Both follow patterns already used in the codebase (@param-outfor storage,@psalm-self-outfor docblock parsing, attribute lookup is used elsewhere in PHP-Parser-based plugins) but feedback welcome.