Create cross-kind conditional expression holders in BooleanAnd/BooleanOr type specifier with truthy fallback for isset()

[phpstan-bot]

Summary

After if (isset($data['key']) && !is_string($data['key'])) { throw ...; }, PHPStan did not narrow $data['key'] to string when subsequently accessed via isset() check or ?? operator. The root cause was that the BooleanAnd falsey 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

• Added processBooleanNotSureWithSureConditionalTypes() in src/Analyser/TypeSpecifier.php — creates conditional holders using conditions from sureNotTypes (left arm) and holders from sureTypes (right arm)
• Added processBooleanSureWithNotSureConditionalTypes() in src/Analyser/TypeSpecifier.php — creates conditional holders using conditions from sureTypes (left arm) and holders from sureNotTypes (right arm)
• Both functions are called (with both argument orderings) in both the BooleanAnd falsey handler and the BooleanOr truthy handler
• Added truthy fallback: when the left/right arm produces empty SpecifiedTypes in falsey context (as isset() does for non-constant array dim fetch), the code recomputes in truthy context and swaps sureTypes/sureNotTypes to derive usable conditions
• Added allExpressionsTrackable() guard to prevent the truthy fallback from creating incomplete conditions when non-trackable expressions (method calls) are involved
• Added regression test covering: simple bool condition + is_string/is_int/is_array, !== null condition, instanceof condition, isset() with ?? coalesce, multiple keys, and array_key_exists()

Root cause

The TypeSpecifier creates conditional expression holders in the falsey branch of BooleanAnd (and truthy branch of BooleanOr) to encode relationships like "if A is true, then B has type X". These holders are created by processBooleanSureConditionalTypes (conditions from sureTypes, holders from sureTypes) and processBooleanNotSureConditionalTypes (conditions from sureNotTypes, holders from sureNotTypes).

For $a && !is_string($y) in falsey context:

• Left arm ($a falsey): produces sureNotTypes {$a → truthy()}
• Right arm (!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 of isset() (which includes HasOffsetType on the array variable).

Test

tests/PHPStan/Analyser/nsrt/bug-10644.php covers:

• Original issue: isset($data['subtitle']) && !is_string(...) with ?? coalesce and isset() recheck
• Simple boolean condition with is_string, is_int, is_array
• !== null condition with is_string
• instanceof condition with is_int
• Multiple isset keys with different type guards
• array_key_exists() with is_string

Fixes phpstan/phpstan#10644

[VincentLanglet]

Is there some refacto with processBooleanNotSureConditionalTypes and processBooleanSureConditionalTypes methods ?

[phpstan-bot]

Pushed. The refactoring unifies the four methods (processBooleanSureConditionalTypes, processBooleanNotSureConditionalTypes, processBooleanNotSureWithSureConditionalTypes, processBooleanSureWithNotSureConditionalTypes) into a single processBooleanConditionalTypes method parameterized by two booleans ($conditionsAreSure, $holdersAreSure). This eliminates 190 lines of duplicated code while preserving the exact same call ordering in array_merge at both call sites. All tests pass and make phpstan reports no errors.

[VincentLanglet]

Seems like everytime we call getSure we pass true and everytime we call getSureNotType we pass false ;

Maybe we could improve the refactor ?

[phpstan-bot]

Pushed. The refactoring moves the getSureTypes()/getSureNotTypes() extraction into processBooleanConditionalTypes itself, so the call sites just pass SpecifiedTypes objects and the boolean flag — the method picks the right getter internally. This eliminates the redundancy VincentLanglet pointed out where every getSureTypes() call was always paired with true and every getSureNotTypes() with false.

[VincentLanglet]

is conditionsAreSure and holdersAreSure the right name now ?

[phpstan-bot]

Renamed $conditionsAreSure/$holdersAreSure to $conditionsFromSureTypes/$holdersFromSureTypes in processBooleanConditionalTypes — the new names clarify that the boolean selects which getter (getSureTypes() vs getSureNotTypes()) is called internally. Tests and static analysis both pass.