Resolve method reflection for dynamic static calls ($var::method()) to enable purity and side-effect checking#5572
Conversation
|
@staabm You're the one who wrote that no error should be reported on line 32 with the comment But still we consider Do you agree that
|
I am not sure we can/should enforce purity across inheritance. feels like a topic which needs to be discussed with @ondrejmirtes before we implement somthing |
It's already allowed to add
That will change a lot of existing behavior.
Indeed |
Seems decided by this answer phpstan/phpstan#14563 (comment) So IMHO this PR is ready. |
VincentLanglet
left a comment
There was a problem hiding this comment.
Add non regression test for phpstan/phpstan#5020
It should be a RuleTest which check that there is no If condition is always false. error anymore with the snippet https://phpstan.org/r/ada88746-fd60-4187-abfc-046ed28bf750
|
Done. Added a non-regression test for issue #5020 that verifies PHPStan no longer reports "If condition is always false" when a dynamic static call ( |
| * @param class-string<MyEnum> $enum | ||
| * @phpstan-pure | ||
| */ | ||
| function tryFromClassString(string $enum): ?MyEnum |
There was a problem hiding this comment.
add another test for
/**
* @phpstan-pure
*/
function fromEnum(MyEnum $enum): MyEnum
{
return $enum::from('foo');
}which unexpectedly errors before this PR
https://phpstan.org/r/1dccccac-c269-4df9-9cb1-d7715a501d5b
There was a problem hiding this comment.
Done. Added the test case staabm requested — a pure function/method calling $enum::from('foo') where $enum is typed as MyEnum (an object instance, not a class-string or enum-string). Added to both the function-level and method-level test files, updated the error line expectation, and all tests + make phpstan pass.
… to enable purity and side-effect checking - In `StaticCallHandler::processExpr()`, when the class part of a static call is an expression (e.g. `$enum::from()`), resolve the method reflection via `getObjectTypeOrClassStringObjectType()` — the same approach already used in `resolveType()` for return type computation. - Previously, `$methodReflection` was always null for `$expr->class instanceof Expr`, causing a false "call to unknown method" impure point for every dynamic static call. - Guard `$this`-invalidation and promoted-property initialization logic with `$expr->class instanceof Name` to prevent incorrect scope effects when the call target is an expression (e.g. `$other::__construct()`). - Update `CallToStaticMethodStatementWithoutSideEffectsRuleTest::testDynamicStaticCall` expectations: pure dynamic static calls are now correctly detected as having no effect.
…ameter Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
f914a4c to
998d101
Compare
Summary
When calling a static method on a class-string or object variable (e.g.
$enum::from('foo')where$enumisenum-string<BackedEnum>), PHPStan previously failed to resolve the method reflection, treating the call as "call to unknown method" with a possibly-impure point. This caused false positives inside@phpstan-purefunctions/methods.Changes
src/Analyser/ExprHandler/StaticCallHandler.php:elseif ($expr->class instanceof Expr)branch inprocessExpr()that resolves the method reflection usinggetObjectTypeOrClassStringObjectType()— matching the existing approach inresolveType().$expr->class instanceof Nameguards to the$this-invalidation block (constructor side effects) and the promoted-property initialization block, preventing incorrect scope mutations when calling constructors on other objects via expressions (e.g.$other::__construct()).tests/PHPStan/Rules/Pure/PureMethodRuleTest.php: AddedtestBug14557coveringenum-string<T>::from(),enum-string<T>::tryFrom(),class-string<T>::from(),class-string<T>::tryFrom(), pure static methods via class-string, and impure static methods via class-string.tests/PHPStan/Rules/Pure/PureFunctionRuleTest.php: AddedtestBug14557covering the same patterns in pure functions.tests/PHPStan/Rules/Methods/CallToStaticMethodStatementWithoutSideEffectsRuleTest.php: UpdatedtestDynamicStaticCallexpectations — pure dynamic static calls ($foo::pureMethod()) are now correctly detected as having no side effects, which is a natural improvement from resolving the method reflection.Analogous cases probed
class-string<T>(not justenum-string<T>): Both are handled bygetObjectTypeOrClassStringObjectType()— tested withclass-string<SomeClass>calling pure and impure static methods.tryFrom()alongsidefrom(): Both tested.PureFunctionRuleTest.$other::__construct()): Verified that the$this-invalidation and property-initialization guards prevent incorrect behavior (theMissingReadOnlyPropertyAssignRuleTest::testRedeclaredReadonlyPropertiestest).CallToStaticMethodStatementWithoutSideEffectsRule): Now correctly detects pure dynamic static calls as having no effect.Root cause
StaticCallHandler::processExpr()only resolved method reflection when the class part was aNamenode (direct class reference likeFoo::method()). When the class part was anExprnode (variable like$enum::method()),$methodReflectionstayednull, leading to a fallback "call to unknown method" impure point. TheresolveType()method already handled this case correctly viagetObjectTypeOrClassStringObjectType(), but the purity/side-effect logic inprocessExpr()did not.Test
tests/PHPStan/Rules/Pure/data/bug-14557.php— method-level test withenum-string,class-string, pure and impure static methodstests/PHPStan/Rules/Pure/data/bug-14557-function.php— function-level test withenum-stringandclass-stringCallToStaticMethodStatementWithoutSideEffectsRuleTest::testDynamicStaticCallfor the improved detectionFixes phpstan/phpstan#14557
Fixes phpstan/phpstan#5020