Fix phpstan/phpstan#10786: Invalid return type definition#5397
Open
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Open
Fix phpstan/phpstan#10786: Invalid return type definition#5397phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Conversation
…tches in && conditions - Extended ConditionalExpressionHolder creation in TypeSpecifier to support property fetches, static property fetches, and array dim fetches (not just variables) - Fixed issetCheck for nullable native typed properties to fall through to type callback instead of returning undefined, enabling proper narrowing in ?? operator - New regression test in tests/PHPStan/Analyser/nsrt/bug-10786.php
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
When checking
is_null($a->value) && is_null($b->value)and throwing, PHPStan should know that at least one property is non-null afterward. This already worked for plain variables but not for property fetches and other general expressions. The fix extends conditional expression holders to work with property fetches and fixes the isset handling for nullable native typed properties.Changes
src/Analyser/TypeSpecifier.php: Replacedinstanceof Expr\Variablechecks inprocessBooleanSureConditionalTypesandprocessBooleanNotSureConditionalTypeswith a newcanBeConditionalExpressionHolder()helper that also acceptsPropertyFetch,StaticPropertyFetch, andArrayDimFetch. Simplified the self-exclusion loop to a simpleunset($conditions[$exprString]).src/Analyser/MutatingScope.php: InissetCheckfor native typed properties not tracked in expression types, nullable properties now fall through to the type callback instead of returningissetCheckUndefined(). This allowsisset()falsey to properly produce specified types that trigger conditional expression holders. Non-nullable properties retain the old behavior (may be uninitialized).Root cause
Two issues combined:
processBooleanSureConditionalTypesandprocessBooleanNotSureConditionalTypesin TypeSpecifier only created conditional expression holders forExpr\Variable, skipping property fetches entirely.issetCheckin MutatingScope returnednull(viaissetCheckUndefined) for nullable native typed properties not tracked in the scope, causingfilterByFalseyValue(isset($a->value))to produce empty specified types. Without specified types, the conditional holders could never fire.Test
Added
tests/PHPStan/Analyser/nsrt/bug-10786.phpwith two test methods verifying that$a->value ?? $b->valueresolves toint(notint|null) after checking both properties for null and throwing.Fixes phpstan/phpstan#10786