Do not record conditional expressions for MethodCall/StaticCall when assigned expression contains nullsafe chain#5496
Closed
phpstan-bot wants to merge 4 commits intophpstan:2.1.xfrom
Closed
Conversation
…hen assigned expression contains nullsafe chain - When assigning from a nullsafe method/property chain (e.g. `$x = $a->b()?->c()?->d()`), the TypeSpecifier decomposes the chain and produces sure types for intermediate sub-expressions like `$a->b()`. These were being recorded as conditional expressions tied to the assigned variable, causing false positives in `NullsafeMethodCallRule` and `NullsafePropertyFetchRule` when the same method was called again later. - Add `exprContainsNullsafe()` helper to detect nullsafe operators in the assigned expression - Skip recording MethodCall/StaticCall conditional expressions when the assigned expression contains any NullsafeMethodCall or NullsafePropertyFetch - The fix for #9455 (direct method call narrowing like `$hasA = $b->getA() !== null`) is preserved because those assigned expressions do not contain nullsafe operators - Also fixes the analogous case with NullsafePropertyFetch chains
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.
Summary
PR #5447 added
MethodCallandStaticCallto the allowed expression types inprocessSureTypesForConditionalExpressionsAfterAssign, enabling type narrowing for method call results assigned to variables (e.g.$hasA = $b->getA() !== null; if ($hasA) { $b->getA(); }). However, this also caused false positives when the assigned expression was a nullsafe chain: the TypeSpecifier's chain decomposition produces sure types for intermediate sub-expressions (like$order->getOrderCustomer()from$order->getOrderCustomer()?->getCustomer()?->getAccountType()), and these were incorrectly persisted as conditional expressions.Changes
exprContainsNullsafe()helper method insrc/Analyser/ExprHandler/AssignHandler.phpthat recursively checks whether an expression contains anyNullsafeMethodCallorNullsafePropertyFetch$assignedExprContainsNullsafeparameter toprocessSureTypesForConditionalExpressionsAfterAssign()andprocessSureNotTypesForConditionalExpressionsAfterAssign()MethodCallandStaticCallexpressions in the sure types are skipped (not recorded as conditional expressions)Analogous cases probed
$a->getB()?->prop?->subPropwould narrow$a->getB()through conditional expressions. Fixed by the same mechanism (the assigned expression containsNullsafePropertyFetch). Added test.$order::getStaticOrderCustomer()?->...).$hasA = $b->getA() !== nulldoes NOT contain any nullsafe operator, so the conditional expression is still recorded correctly.Cannot call method getId() on A|null#5447 and property accesses are stable within a scope.Cannot call method getId() on A|null#5447 and is not skipped by the new filter.Root cause
The TypeSpecifier decomposes
NullsafeMethodCallchains intoBooleanAnd(var !== null, MethodCall(var, name, args))conditions. This produces sure types for intermediate expressions (e.g.,$order->getOrderCustomer()is not null when the chain result is non-null). PR #5447 allowed theseMethodCall/StaticCallsure types to be recorded as conditional expressions tied to the assigned variable. After a subsequent check narrowed the assigned variable, the conditional expressions fired and incorrectly narrowed the intermediate method call for all future uses — triggering thenullsafe.neverNullrule.Test
tests/PHPStan/Rules/Methods/data/bug-14493.php— reproduces the reported false positive with both instance method calls and static method calls in nullsafe chainstests/PHPStan/Rules/Properties/data/bug-14493.php— reproduces the analogous false positive with NullsafePropertyFetch chains where the var is a method callFixes phpstan/phpstan#14493