Fix phpstan/phpstan#9455: Bug: False report Cannot call method getId() on A|null#5447
Conversation
a634fcf to
2535811
Compare
staabm
left a comment
There was a problem hiding this comment.
add regression test for phpstan/phpstan#5207
|
Done. Added regression test |
VincentLanglet
left a comment
There was a problem hiding this comment.
Add a test with an impure method, asserting the error is still reported
…d boolean variable for method calls - Added MethodCall and StaticCall to allowed expression types in processSureTypesForConditionalExpressionsAfterAssign and processSureNotTypesForConditionalExpressionsAfterAssign - New regression test in tests/PHPStan/Analyser/nsrt/bug-9455.php - The root cause was that conditional expression holders were not created for method call results when assigned to a boolean variable
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Addresses review feedback: adds test cases with @phpstan-impure methods to both bug-9455 and bug-5207 regression tests, asserting that impure method results are correctly NOT narrowed when stored in boolean variables. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
7877d19 to
656a527
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
I processed this review but have nothing to report. |
|
since this change we get this error
@VincentLanglet do we agree that this error is correct? if so, I would add a test for it |
|
The error is correct |
…ain sub-expressions through conditional expressions - PR phpstan#5447 added MethodCall/StaticCall to the allowed expression types in processSureTypesForConditionalExpressionsAfterAssign and processSureNotTypesForConditionalExpressionsAfterAssign. This correctly fixed bug-9455 ($hasA = $b->getA() !== null; if ($hasA) { ... }). - However, it also caused nullsafe chain sub-expressions to leak into the broader scope: $x = $a?->b()?->c() would narrow $a->b() to non-null via conditional expressions, causing "nullsafe on non-nullable type" false positives on subsequent $a->b()?->... calls. - Fix: walk the assigned expression's nullsafe chain and collect exprStrings of MethodCall/StaticCall var nodes. Skip these in conditional expression propagation. Variable and PropertyFetch vars are not affected (they were already in the allowed list before PR phpstan#5447 and their narrowing is correct). - Also tested the NullsafePropertyFetch analogous case (already fixed by the same change) and verified bug-9455/bug-5207 still pass.
Summary
When a null check on a method call result was stored in a boolean variable (e.g.,
$hasA = $b->getA() !== null), and that variable was later used in anifcondition, PHPStan failed to narrow the type of$b->getA()inside the branch. The inline equivalent (if ($b->getA() !== null)) worked correctly.Changes
MethodCallandExpr\StaticCallto the allowed expression types inprocessSureTypesForConditionalExpressionsAfterAssign()andprocessSureNotTypesForConditionalExpressionsAfterAssign()insrc/Analyser/ExprHandler/AssignHandler.phptests/PHPStan/Analyser/nsrt/bug-9455.phpRoot cause
When PHPStan processes an assignment like
$hasA = $b->getA() !== null, it createsConditionalExpressionHolderobjects that record type narrowing to apply when the assigned variable is later used in a condition. However, the methodsprocessSureTypesForConditionalExpressionsAfterAssignandprocessSureNotTypesForConditionalExpressionsAfterAssignonly allowedVariable,PropertyFetch,ArrayDimFetch, andFuncCallexpressions to create these holders —MethodCallandStaticCallwere filtered out. This meant that type narrowing from method call null checks (like$b->getA() !== null) was never stored and thus never applied when the boolean variable was checked.Test
The regression test verifies that
$b->getA()is correctly narrowed toBug9455\A(notBug9455\A|null) inside anif ($hasA)block where$hasA = $b->getA() !== null.Fixes phpstan/phpstan#9455
Fixes phpstan/phpstan#5207