Re-check scalar types after integer range expansion in TypeCombinator::union

[phpstan-bot]

Summary

• Fixed order-dependent bug in TypeCombinator::union() where the scalar processing loop did not re-check previously-processed scalar items after an IntegerRangeType expanded by absorbing a ConstantIntegerType. For example, union(0|int<2,3>, 1) incorrectly produced 0|int<1,3> (a UnionType) instead of int<0,3> (a single IntegerRangeType).
• This incomplete union caused ExpressionTypeHolder::equalTypes() to return false during scope merging in MutatingScope::createConditionalExpressions(), which created spurious conditional type guards. These guards incorrectly narrowed superglobal types (e.g., $_SESSION) in branches guarded by comparisons like $value == 0, leading to false positive "expression is always true" reports from IssetCheck.
• The fix restarts the inner scalar loop ($j = -1) when a scalar is absorbed into a range, ensuring all remaining scalar items are tested against the newly expanded range.

Test plan

• New TypeCombinatorTest case verifies union(0|int<2,3>, 1) produces int<0, 3>
• New IssetRuleTest::testBug14610() verifies no false positives for isset(), ??, and empty() on superglobal keys after conditional variable assignment
• New NSRT test bug-14610.php verifies correct type inference: $_SESSION stays array<mixed> (not narrowed to hasOffsetValue) inside if ($value == 0) branch
• make tests passes (12066 tests, 27563 assertions)
• make phpstan passes with no errors
• make cs-fix passes with no violations

Closes phpstan/phpstan#14610

🤖 Generated with Claude Code

[staabm]

do we need the same fix for the if ($b !== null) { block below?
if so, come up with tests currently failling and fix it

[phpstan-bot]

All tests pass (12066 tests, 79688 assertions) and PHPStan reports no errors.

Here's my analysis for the reviewer's question:

The $b !== null branch does NOT need the same fix. The issue in the $a !== null branch occurs because an IntegerRangeType in $types[$i] expands by absorbing a ConstantIntegerType scalar, and previously-checked scalars might now be adjacent to the expanded range. The $b !== null branch has the inverse relationship — the scalar $scalarTypeItems[$j] is supposed to absorb $types[$i]. But this path can never trigger the expanding-range scenario because:

1. $scalarTypeItems contains only constant scalar types (ConstantIntegerType, ConstantStringType, etc.) — never IntegerRangeType, since IntegerRangeType::isConstantScalarValue() returns no and these are separated into $integerRangeTypes then merged back into $types.

2. The [null, $b] return from compareTypesInUnion for the IntegerRangeType path (line 517-522) requires $b instanceof IntegerRangeType, which is never true for a scalar item.

3. The other [null, $b] paths (SubtractableType, $b->isSuperTypeOf($a)) either don't apply to constant scalar types or return $b unchanged — the scalar doesn't grow.

4. Since the scalar never expands, there's no need to re-check earlier $types entries.

I was unable to construct a test case that fails due to the $b !== null branch lacking a restart, which confirms the analysis.

No code changes are needed. Tests and static analysis both pass.

[VincentLanglet]

Explain why it only occurs for superglobal and not

/** @param array<mixed> $a */
function test($a): void
{
    $value = 0;

    if (isset($a['test'])) {
        $value = rand(0,3);
        if ($value == 1) {
        }
    }

    if ($value == 0) {
        $result = isset($a['test']);
    }
}

[phpstan-bot]

Pushed. Here's the explanation for VincentLanglet's review question:

Why the bug only affects superglobals, not array<mixed> parameters:

The root cause is how superglobals vs parameters are tracked in MutatingScope::expressionTypes:

1. Parameters ($a) are always present in expressionTypes in both branches of the isset() check — they're set up when entering the function scope. So during scope merging in createConditionalExpressions(), $a exists in theirExpressionTypes (the false branch), and its merged type (array<mixed>) equals the false-branch type (array<mixed>). This causes $a to be removed from newVariableTypes at line 3705, so it never becomes a type guard and no conditional expression links $a's narrowed type to $value.

2. Superglobals ($_SESSION) are NOT stored in expressionTypes by default — they use lazy default types via getVariableType() (line 679-680). When isset($_SESSION['test']) narrows $_SESSION, it gets added to expressionTypes only in the true branch. In the false branch, $_SESSION is absent from expressionTypes. During scope merging, $_SESSION is never iterated in the false-branch loop, so it stays in newVariableTypes and becomes a type guard. This creates a conditional expression: "when $value matches 0|int<1,3>, then $_SESSION has hasOffsetValue."

3. The TypeCombinator bug then makes $value a type guard too, because its true-branch type 0|int<1,3> is structurally different from the merged type int<0,3> (despite being semantically equivalent). With the fix, these are identical, so $value is NOT a type guard, and no conditional expression can fire.

I added the reviewer's array<mixed> parameter test case to both test files to demonstrate this difference explicitly.

[VincentLanglet]

Remove testWithParam and merge the two bug-14610 files into a single one

[phpstan-bot]

Done. The changes:

• Removed testWithParam functions from both test files
• Merged the two bug-14610.php files into a single file at tests/PHPStan/Analyser/nsrt/bug-14610.php (combining the assertType calls with the isset rule test scenarios)
• Deleted tests/PHPStan/Rules/Variables/data/bug-14610.php
• Updated IssetRuleTest to reference the nsrt file path
• All 12066 tests pass and make phpstan reports no errors

[staabm]

3. The TypeCombinator bug then makes $value a type guard too, because its true-branch type 0|int<1,3> is structurally different from the merged type int<0,3>

@phpstan-bot where does this structured comparison happen? why does check not realize 0|int<1,3> and int<0,3> are the same thing?

[VincentLanglet]

3. The TypeCombinator bug then makes $value a type guard too, because its true-branch type 0|int<1,3> is structurally different from the merged type int<0,3>

@phpstan-bot where does this structured comparison happen? why does check not realize 0|int<1,3> and int<0,3> are the same thing?

In the same way I assume than int<0,1> and 0|1 are not equals ; that might creates issues one day.
But I think the fix proposed is still an ok improvement

[staabm]

I think these tests make clear that a fix in TypeCombinator is valid (and the super-global example is just a "bad example" which is fixed by it at the same time)

[phpstan-bot]

All 12,068 tests pass (79,692 assertions) and make phpstan reports no errors.

The reviewer's comment is positive feedback — they're agreeing that the TypeCombinator unit tests clearly demonstrate the fix is valid on their own, independent of the superglobal scenario. No code changes are needed to address this comment.