Create cross-kind conditional expression holders in BooleanAnd/BooleanOr type specifier with truthy fallback for isset()#5760
Conversation
VincentLanglet
left a comment
There was a problem hiding this comment.
Is there some refacto with processBooleanNotSureConditionalTypes and processBooleanSureConditionalTypes methods ?
|
Pushed. The refactoring unifies the four methods ( |
daeb428 to
6de7bd1
Compare
…leanOr` type specifier with truthy fallback for `isset()`
- Add `processBooleanNotSureWithSureConditionalTypes()` which builds conditions
from sureNotTypes and holders from sureTypes (cross-kind pairing)
- Add `processBooleanSureWithNotSureConditionalTypes()` which builds conditions
from sureTypes and holders from sureNotTypes (reverse cross-kind pairing)
- Call both new functions (with both argument orderings) in BooleanAnd falsey
and BooleanOr truthy conditional holder creation
- Add truthy fallback: when left/right falsey types produce empty SpecifiedTypes
(e.g. isset() on non-constant array dim fetch), recompute in truthy context
and swap sureTypes/sureNotTypes to derive conditions for holders
- Guard the truthy fallback with `allExpressionsTrackable()` to prevent
over-narrowing when non-trackable expressions (method calls) are involved
- Fixes patterns like `if ($a && !is_string($y)) { throw; }` followed by
`if ($a) { /* $y is now string */ }` and
`if (isset($data['x']) && !is_string($data['x'])) { throw; }` followed by
`$data['x'] ?? ''` being correctly inferred as string
6de7bd1 to
ed816ea
Compare
VincentLanglet
left a comment
There was a problem hiding this comment.
Add non regression test for
| assertType("string", $data['subtitle'] ?? ''); | ||
| } | ||
|
|
||
| function testSimpleBool(bool $a, mixed $y): void |
There was a problem hiding this comment.
Use mixed as phpdoc type to avoid failure on php 7.4
There was a problem hiding this comment.
Done. Two commits pushed:
-
Use PHPDoc mixed type instead of native mixed for PHP 7.4 compatibility — replaced native
mixedtype hints with@param mixedPHPDoc annotations inbug-10644.php -
Add regression tests for issues #11918, Update crate-ci/typos action to v1.24.3 #3385, #6202, #14455 — four new test files:
bug-11918.php—array_key_exists+!is_stringguard with??fallbackbug-3385.php— decomposed XOR null checks across multiple&&conditions, verifying both variables are narrowed to non-nullbug-6202.php—isset+!is_stringwith?? ''coalescebug-14455.php—empty()+ type equality condition with conditional narrowing viahasOffset
All tests pass (12168 tests, 79859 assertions) and make phpstan reports no errors.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
After
if (isset($data['key']) && !is_string($data['key'])) { throw ...; }, PHPStan did not narrow$data['key']tostringwhen subsequently accessed viaisset()check or??operator. The root cause was that theBooleanAndfalsey handler only created conditional expression holders when both left and right arms had the same "kind" of type specification (both sureTypes or both sureNotTypes), but common patterns produce mismatched kinds.Changes
processBooleanNotSureWithSureConditionalTypes()insrc/Analyser/TypeSpecifier.php— creates conditional holders using conditions fromsureNotTypes(left arm) and holders fromsureTypes(right arm)processBooleanSureWithNotSureConditionalTypes()insrc/Analyser/TypeSpecifier.php— creates conditional holders using conditions fromsureTypes(left arm) and holders fromsureNotTypes(right arm)BooleanAndfalsey handler and theBooleanOrtruthy handlerSpecifiedTypesin falsey context (asisset()does for non-constant array dim fetch), the code recomputes in truthy context and swaps sureTypes/sureNotTypes to derive usable conditionsallExpressionsTrackable()guard to prevent the truthy fallback from creating incomplete conditions when non-trackable expressions (method calls) are involved!== nullcondition,instanceofcondition,isset()with??coalesce, multiple keys, andarray_key_exists()Root cause
The
TypeSpecifiercreates conditional expression holders in the falsey branch ofBooleanAnd(and truthy branch ofBooleanOr) to encode relationships like "if A is true, then B has type X". These holders are created byprocessBooleanSureConditionalTypes(conditions from sureTypes, holders from sureTypes) andprocessBooleanNotSureConditionalTypes(conditions from sureNotTypes, holders from sureNotTypes).For
$a && !is_string($y)in falsey context:$afalsey): produces sureNotTypes{$a → truthy()}!is_string($y)falsey =is_string($y)truthy): produces sureTypes{$y → StringType}Since left has sureNotTypes and right has sureTypes, neither existing function could create the holder "if $a is true, then $y is string". The new cross-kind functions handle this mismatch.
For
isset($data['x'])specifically, the falsey context returns empty SpecifiedTypes for non-constant arrays, so even the cross-kind functions had nothing to work with. The truthy fallback addresses this by deriving conditions from the truthy narrowing ofisset()(which includesHasOffsetTypeon the array variable).Test
tests/PHPStan/Analyser/nsrt/bug-10644.phpcovers:isset($data['subtitle']) && !is_string(...)with??coalesce andisset()recheckis_string,is_int,is_array!== nullcondition withis_stringinstanceofcondition withis_intarray_key_exists()withis_stringFixes phpstan/phpstan#10644
Fixes phpstan/phpstan#11918
Fixes phpstan/phpstan#3385
Fixes phpstan/phpstan#6202
Fixes phpstan/phpstan#14455