Repository navigation
Consult ExpressionTypeResolverExtensions in MutatingScope::resolveScopeStateType() - #6816
Open
phpstan-bot wants to merge 1 commit into
Open
phpstan-bot wants to merge 1 commit into
phpstan-bot wants to merge 1 commit into
Conversation
…ScopeStateType()` - The state-type read (getStateType()) used by the non-nullability ensure of `?->`, `??`, isset()/empty() and by narrowing skipped expression type resolver extensions, unlike resolveType(). A receiver typed only by an extension (e.g. a universal object crate property) was read as mixed, so `$crate->holder->day` was never ensured non-null and the plain twin of `$crate->holder->day?->toDateString()` was checked against `Day|null`. - Non-variable expressions now ask the extensions first (on the native- promoted scope for the native flavour), mirroring resolveType(). - Ported the same change to the turbo-ext mirror (MutatingScope.cpp). - The same root cause also broke, and this fixes: chained `?->` over an extension-typed receiver, `?->` property fetch on an offset of it, `$crate->holder->day->date ?? null` (was mixed), and `!== null` narrowing of `$crate->holder->day` (was mixed~null).
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
Since 2.3.0,
$crate->holder->day?->toDateString()was reported asmethod.nonObject("Cannot call method toDateString() on Day|null") when the type of$crate->holdercame from anExpressionTypeResolverExtension(e.g. Pest'sTestCaseDynamicPropertyTypeExtension). This change makes the scope's state-type read ask the extensions, the same way the regular type resolution does.Changes
src/Analyser/MutatingScope.php:resolveScopeStateType()now asksExpressionTypeResolverExtensions first for non-variable expressions, mirroringresolveType(). For the native flavour it asks on the native-promoted scope.turbo-ext/src/MutatingScope.cpp: the same change in the native mirror.Crate::$holderasHolder.Root cause
The nullsafe handlers,
??,isset()/empty()and condition narrowing make the receiver non-null withNonNullabilityHelperandMutatingScope::getStateType(). That read works out the type from tracked state and reflection, and it skipped the extension hook on purpose.resolveType()asks the extensions before anything else. So for$crate->holder->day, the state read saw$crate->holderasmixed(from the universal object crate) instead of the extension'sHolder. The non-null ensure then had no nullable type to narrow, and the plain twin of the?->call was checked against the realDay|null.The same mismatch affected every user of the state read. Before the fix these also failed:
$crate->holder->day?->next()?->next(): method.nonObject reported twice$crate->holder->days['day']?->date: "Cannot access property $date on Day|null"$crate->holder->day->date ?? nullwas inferred asmixedinstead ofstring|nullif ($crate->holder->day !== null)narrowed tomixed~nullinstead ofDayI also checked
?->on a method-call receiver ($crate->holder->getDay()?->…), a nullsafe property fetch directly on the extension-typed chain, andisset()/empty(). Those already worked; they stay in the test file as coverage.Test
tests/PHPStan/Analyser/data/bug-15421.php, with an extension intests/PHPStan/Analyser/data/Bug15421ExpressionTypeResolverExtension.phpregistered inexpression-type-resolver-extension.neon:ExpressionTypeResolverExtensionTestCallMethodsRuleExpressionTypeResolverExtensionTest: no errors expected; four method.nonObject errors before the fixAccessPropertiesRuleExpressionTypeResolverExtensionTest: no errors expected; one error before the fixturbo-ext/tests/smoke.phpandwalk-trace.phpreport identical behaviour.Fixes phpstan/phpstan#15421