Introduce ClosureType::isStaticClosure()#5699
Conversation
VincentLanglet
left a comment
There was a problem hiding this comment.
Remove all the change to ClosureBindDynamicReturnTypeExtension and ClosureBindToDynamicReturnTypeExtension.
This will be done in another PR
| return $prefix . $printer->print($selfWithoutParameterNames->toPhpDocNode()); | ||
| }, | ||
| function (): string { | ||
| $prefix = !$this->isStatic->maybe() ? ($this->isStatic->yes() ? 'static ' : 'non-static ') : ''; |
There was a problem hiding this comment.
There is no reason to use non-static here, we don't use non-pure.
There was a problem hiding this comment.
Both commits are pushed. Here's a summary of the changes:
Commit 1: Revert Closure::bindTo()/Closure::bind() return type narrowing
- Reverted
ClosureBindDynamicReturnTypeExtensionandClosureBindToDynamicReturnTypeExtensionto their original state (no static closure checks) - Updated
closure-static-type.phptest assertions to expect the closure type (notnull) from bind/bindTo - Reverted
bug-4734integration test to original 5-error expectation
Commit 2: Use hyphen prefix and extract helper method
- Changed prefix from
'static 'to'static-'(matchingpure-Closureconvention) - Removed
non-staticprefix at cache level (consistent with not usingnon-pure) - Extracted
describeWithPrefix()private method, eliminating the duplicated closure body across value, precise, and cache callbacks (cache now falls through to precise) - Updated all test assertion strings from
'static Closure'to'static-Closure'
|
|
||
| return $printer->print($selfWithoutParameterNames->toPhpDocNode()); | ||
| }, | ||
| function (): string { |
There was a problem hiding this comment.
Maybe a private methode could be created to refactor the value and precise level
| return $printer->print($selfWithoutParameterNames->toPhpDocNode()); | ||
| }, | ||
| function (): string { | ||
| $prefix = $this->isStatic->yes() ? 'static ' : ''; |
There was a problem hiding this comment.
prefix should be with a -
| fn (): string => $this->describeBody(showPure: true, showStatic: false), | ||
| fn (): string => $this->describeBody(showPure: true, showStatic: true), |
There was a problem hiding this comment.
Do not use namedParam
There was a problem hiding this comment.
Done. Removed the named parameters from the describeBody() calls as requested by the reviewer. Tests and static analysis both pass.
ClosureType::isStaticClosure() and narrow Closure::bindTo()/Closure::bind() return type for static closuresClosureType::isStaticClosure()
VincentLanglet
left a comment
There was a problem hiding this comment.
Some AttributeReflectionTest::testAttributeReflections test are failing in PHP 8.5 because the expected output is static-Closure rather than Closure
|
Pushed. Updated the 6 test expectations in |
|
@VincentLanglet this PR does not implement the use-cases for isStatic() described in phpstan/phpstan#14639 (therefore cannot close the issue). do you intend to implement the use-cases in a followup? |
Yes, I prefer to split the PR because it already big enough Imho |
a05f5a2 to
0786e7a
Compare
0786e7a to
9486a3f
Compare
ondrejmirtes
left a comment
There was a problem hiding this comment.
I also miss some "accepts" tests.
And I feel like that will show that we need to update getRecommendedLevelByType. When a non-static closure is rejected by static closure, the verbosity level should be increased to precise (very-verbose).
1dd9c0a to
50a1b44
Compare
…o()`/`Closure::bind()` return type for static closures - Add `isStaticClosure(): TrinaryLogic` to `CallableParametersAcceptor` interface - Add `isStatic` parameter to `ClosureType` constructor, propagated through `equals()`, `describe()`, `traverse()`, `traverseSimultaneously()` - Show `static` prefix in `describe()` at precise level when closure is definitely static; show `non-static`/`static` at cache level when known - Implement `isStaticClosure()` in all `CallableParametersAcceptor` implementations: `CallableType` (maybe), `TrivialParametersAcceptor` (maybe), `FunctionCallableVariant` (no), `InaccessibleMethod` (no), `ExtendedCallableFunctionVariant` and `ResolvedFunctionVariantWithCallable` (propagated from source) - Set `isStatic` from `$expr->static` in `ClosureTypeResolver` for both `Closure` and `ArrowFunction` AST nodes - Add `isStaticClosure` check to `CallableTypeHelper::isParametersAcceptorSuperTypeOf()` - Propagate `isStaticClosure` through `ParametersAcceptorSelector`, `GenericParametersAcceptorResolver`, `ClosureFromCallableDynamicReturnTypeExtension`, `TypeNodeResolver`, and `RuleLevelHelper` - Narrow `Closure::bindTo()` to return `null` for static closures and `Closure|null` for maybe-static closures - Narrow `Closure::bind()` to return `null` for static closures and `Closure|null` for maybe-static closures - Update existing tests to expect `static` prefix in closure type descriptions
…r static closures This will be done in a separate PR. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…` helper - Change prefix from `static ` to `static-` to match the `pure-Closure` convention - Remove `non-static` prefix at cache level (consistent with not using `non-pure`) - Extract `describeWithPrefix()` private method to deduplicate value/precise callbacks Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ic` booleans Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tributes Static closures in PHP 8.5 attributes now correctly describe as `static-Closure(...)` at precise verbosity level. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Consistent with how pure- prefix is handled: the static- prefix only appears for common callable closures (e.g. static-Closure, static-pure-Closure), not for closures with explicit parameters (e.g. Closure(): void stays the same regardless of static-ness). Added tests for the static-pure-Closure combination. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…llable branch Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…isStatic - Add precise callback to ClosureType::describe() that shows static- prefix for all closures (not just common callable) when isStatic is yes - Extract describeCallable() private method to avoid duplication - Add static- prefix to toPhpDocNode() for common callable closures - Add static-Closure/static-pure-Closure parsing in TypeNodeResolver - Pass isStatic to new self() in describe() and to all relevant new ClosureType construction sites (InitializerExprTypeResolver, ClosureTypeFactory) - Update getRecommendedLevelByType to bump to precise level when a ClosureType has definite isStatic (yes or no) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…r static- prefix - Add dataAccepts/testAccepts to ClosureTypeTest with static/non-static/ maybe-static combinations - Add TypeCombinator::union and ::intersect tests for static closures - Add toPhpDocNode tests for static-Closure and static-pure-Closure - Update existing test expectations to include static- prefix at precise level for static closures in closure-static-type, bug-7031, bug-9764, degrade-closures, bug-14324 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ReflectionFunction::isStatic() does not reliably report static closures on PHP < 8.1, so remove the isStatic parameter from the fromClosureObject() call site. This falls back to TrinaryLogic::createMaybe() (the default), which is safe for this niche runtime-reflection code path. The main analysis path through ClosureTypeResolver correctly determines isStatic from the AST. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
50a1b44 to
e376c3a
Compare
|
Still some failures on PHP 8.5. |
PHP8.5 tests updated @ondrejmirtes |
Summary
Introduces
isStaticClosure(): TrinaryLogiconClosureTypeandCallableParametersAcceptorto track whether a closure is declared with thestatickeyword. This enables PHPStan to distinguish static from non-static closures in the type system, which is useful for rules that check closure binding compatibility and for narrowing the return types ofClosure::bindTo()andClosure::bind().Changes
Core type system
isStaticparameter (defaulting toMaybe) toClosureTypeconstructor (src/Type/ClosureType.php)isStaticClosure(): TrinaryLogicmethod toClosureTypeClosureType::equals()to compareisStaticClosureType::describe()to showstaticprefix atpreciselevel andstatic/non-staticatcachelevelClosureType::traverse()andtraverseSimultaneously()to preserveisStaticInterface and implementations
isStaticClosure(): TrinaryLogictoCallableParametersAcceptorinterface (src/Reflection/Callables/CallableParametersAcceptor.php)CallableType— returnsMaybe(callable could be anything)TrivialParametersAcceptor— returnsMaybeFunctionCallableVariant— returnsNo(named functions are not closures)InaccessibleMethod— returnsNoExtendedCallableFunctionVariant— propagated via constructor parameterResolvedFunctionVariantWithCallable— propagated via constructor parameterClosure creation
ClosureTypeResolverto passTrinaryLogic::createFromBoolean($expr->static)at all three ClosureType construction sites (src/Analyser/ExprHandler/Helper/ClosureTypeResolver.php)InitializerExprTypeResolverto setisStatic: TrinaryLogic::createYes()for static closures in constant contexts (src/Reflection/InitializerExprTypeResolver.php)Propagation
ParametersAcceptorSelector— propagatesisStaticClosurewhen combining/wrapping callable variants (src/Reflection/ParametersAcceptorSelector.php)GenericParametersAcceptorResolver— propagates through template resolution (src/Reflection/GenericParametersAcceptorResolver.php)ClosureFromCallableDynamicReturnTypeExtension— propagates from variant to new ClosureTypeTypeNodeResolver— propagates when overriding ClosureType with PHPDocRuleLevelHelper— propagates when transforming closure typesCallableTypeHelper::isParametersAcceptorSuperTypeOf()— checksisStaticClosurecompatibilityRoot cause
The issue requested tracking whether a closure is static in the type system. Previously, the
statickeyword on closures was only tracked at the AST level ($expr->static) and used for scope resolution (preventing$thisaccess), but was not represented inClosureType. This meant rules and return type extensions that needed to know if a closure was static had to inspect the AST node directly rather than querying the type.The fix adds
isStatic: TrinaryLogictoClosureTypeand propagates it through all relevant interfaces and creation sites. TheTrinaryLogicthree-valued logic (yes/no/maybe) is used because closures from PHPDoc annotations or generic type resolution may have unknown static-ness.Test
tests/PHPStan/Analyser/nsrt/closure-static-type.php): Verifiesstaticprefix in type descriptions for static closures and arrow functions, verifiesClosure::bindTo()andClosure::bind()returnnullfor static closures andClosurefor non-static closures, and verifiesClosure|nullfor closures with unknown static-ness.tests/PHPStan/Type/ClosureTypeTest.php): Tests forisSuperTypeOf()with static/non-static/maybe-static closure combinations,equals()comparisons,describe()output at all verbosity levels, andisStaticClosure()method.degrade-closures.php,bug-7031.php,bug-9764.php,bug-14324.php— updated to expectstaticprefix for static closures.bug-4734test updated to expect the new error from callingnullreturned byClosure::bind()on a static closure.Refs phpstan/phpstan#14639