Repository navigation
Fall back to the full branch type for a conditional guard when the partial set difference still overlaps the other branch - #6830
Open
phpstan-bot wants to merge 1 commit into
Conversation
…rtial set difference still overlaps the other branch - ScopeOps::createConditionalExpressions() derives guard types from TypeCombinator::remove(ours, theirs). When remove() can only subtract part of their type (e.g. null from string|null, but not non-empty-string from string), the remainder (string) still overlaps the other branch, so seeing it later does not prove our branch ran. Such guards produced conditional expressions like "if $title=string then $input->isInteractive() is true". Now the guard falls back to our full type when the remainder is not disjoint from their type, which restores the merged-type comparison that was used before. - Ported the same check to the native mirror in turbo-ext/src/ScopeOps.cpp. - Variants also covered: plain ?string narrowed against non-empty-string|null, and ?array against non-empty-array|null (both were broken). The int<1, max>|null variant already worked, because int<min, 0> is representable; it is kept as coverage.
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, a method call that runs on only one side of
$title === null && $input->isInteractive()was remembered astrueafter a laterif ($title === null) return;, as long as$title's types in the two branches only partly overlapped (e.g.string|nullvsnon-empty-string|null). This PR stops a guard type that still overlaps the other branch from being used for a conditional expression.Changes
src/Analyser/ScopeOps.php(createConditionalExpressions()): if the set-difference remainder is not the full our-branch type and is not disjoint from the their-branch type, use the full our-branch type as the guard instead.turbo-ext/src/ScopeOps.cpp: same change in the native mirror.non-empty-string|nullvs?string(broken, fixed),non-empty-array|nullvs?array(broken, fixed),int<1, max>|nullvs?int(already correct, since the remainderint<min, 0>can be represented; kept as coverage).Root cause
The guard type comes from
TypeCombinator::remove($ours, $theirs). The code assumed one of two outcomes: an exact difference, or our full type unchanged when the difference can't be represented. There is a third outcome: a partial subtraction. Hereremove(string|null, non-empty-string|null)givesstring, which dropsnullbut still overlapsnon-empty-string. Thisstringdiffers from the merged typestring|null, so it was accepted as a guard, and the merge recorded "if$titleisstringthenisInteractive()istrue". Narrowing$titleto non-null later triggered that conditional. Now a remainder that still overlaps their type falls back to the full type, and the merged-type comparison then drops the guard.Test
tests/PHPStan/Analyser/nsrt/bug-15483.phpcontains the reproducer from the issue (f()), theis_string-only control (g()) and the parameter-based variants for non-empty-string, int range and non-empty-array. Without the fix,f(),h()andj()fail withtrueinstead ofbool. The full suite passes with and without the turbo extension loaded, and the turbo smoke test passes.Fixes phpstan/phpstan#15483