Narrow array element type after a keyless foreach whose body guards each element with an early exit

[phpstan-bot]

Summary

When an array is iterated with a keyless foreach and the loop body validates each
element with an early-exit guard, PHPStan failed to understand that, after the loop,
every element satisfies the guard. For example:

/**
 * @param list<mixed> $ids
 * @return list<string>|null
 */
function validate(array $ids): array|null
{
    foreach ($ids as $id) {
        if (!is_string($id)) {
            return null;
        }
    }

    return $ids; // PHPStan thought $ids was still list<mixed>
}

PHPStan already supported the keyed form (foreach ($a as $k => $v)), but the
post-loop array rewrite was gated on a key variable being present. This PR
generalizes that mechanism to the keyless form, so $ids is now correctly narrowed
to list<string> after the loop.

Changes

• src/Analyser/NodeScopeResolver.php (processStmtNode(), foreach handling):
  • Replaced the $stmt->keyVar !== null gate with ($keyVarExpr !== null || $originalValueExpr !== null).
  • Introduced a single "tracking expression" used to decide whether an iteration's
    narrowing may be projected back onto the array: the original key expression when
    a key variable exists, otherwise the original value expression
    (OriginalForeachValueExpr).
  • When there is no key variable, the loop's element type is read directly from the
    (narrowed) value variable instead of from new ArrayDimFetch($expr, $keyVar),
    which is impossible without a key. Key-type remapping is skipped in that case.
  • The existing keyed behavior is unchanged.
• tests/PHPStan/Analyser/nsrt/bug-7076.php: the noKeyVar case asserted the old,
  limited behavior (array<int|string, mixed>); updated to the now-correct
  array<int|string, string>.
• src/Analyser/ExprHandler/MatchHandler.php: removed a defensive
  if (!$cond instanceof Expr\ClassConstFetch) throw … re-check that the sharper
  inference now flags as always-true. The preceding pre-validation loop already
  guarantees every reached condition is an enum-case ClassConstFetch (it does
  break 2 otherwise), so the check was dead.

Root cause

The post-loop array element/key rewrite in NodeScopeResolver was written only for
the keyed foreach, where each iteration is identified by OriginalForeachKeyExpr
and the element is addressed via $array[$key]. The keyless form has no key
expression and no addressable dim fetch, so the whole block was skipped — the array
type never picked up the element narrowing that the body had proven. The fix tracks
the keyless case through OriginalForeachValueExpr (which is always assigned in
MutatingScope::enterForeach(), independent of the key) and rewrites the array's
value type from the value variable directly.

Analogous cases probed

Because the single code path now handles all keyless guards, the following sibling
constructs were verified to narrow correctly with the same fix (tests added in
bug-5755.php for the representative ones):

• is_string/is_int/… guards with return — the reported case.
• instanceof guards.
• throw as the early exit (in addition to return).
• by-reference value variables (foreach ($a as &$v)).
• non-list arrays (array<int|string, …>), in addition to list<…>.
• continue guards correctly do not narrow (non-matching elements remain).
• reassigning the value variable inside the loop correctly prevents narrowing.

Test

• New tests/PHPStan/Analyser/nsrt/bug-5755.php reproduces every snippet linked in
  the issue (the original Test::Value()/Test::Strings() example and the reduced
  validate() example) plus the analogous cases above, using assertType(). The
  assertions fail without the fix (the array stays un-narrowed) and pass with it.
• Updated bug-7076.php to reflect the corrected behavior.
• Full NodeScopeResolverTest, the Analyser suite, the Comparison/DeadCode/Match
  rule tests and PHPStan self-analysis all pass.

Fixes phpstan/phpstan#5755

[ondrejmirtes]

Fix the tests:

There was 1 failure:

1. PHPStan\Build\RequiredPhpVersionCommentTest::testFixtureHasRequiredLintComment@/home/runner/work/phpstan-src/phpstan-src/tests/PHPStan/Build/../Analyser/nsrt/bug-5755.php with data ('/home/runner/work/phpstan-src...55.php')
   Fixture uses union types on line 11 which requires PHP 8.0. Add a <?php // lint >= 8.0 comment on the first line so the fixture is skipped on older PHP versions in CI.
   Failed asserting that 0 is equal to 80000 or is greater than 80000.

/home/runner/work/phpstan-src/phpstan-src/tests/PHPStan/Build/RequiredPhpVersionCommentTest.php:101

[phpstan-bot]

Done. The reviewer's requested change has been addressed.

Summary

The review pointed out a CI test failure: RequiredPhpVersionCommentTest flagged that bug-5755.php uses union types (array|null on line 11) which require PHP 8.0, but the fixture lacked the required lint comment that skips it on older PHP versions.

Fix: Added <?php // lint >= 8.0 to the first line of tests/PHPStan/Analyser/nsrt/bug-5755.php, moving the declare(strict_types = 1); to its own line below — matching the existing convention used by other PHP 8.0+ fixtures (e.g. bug-6663.php).

Verification:

• RequiredPhpVersionCommentTest now passes (4712 tests OK).
• The bug-5755 analyser test (NodeScopeResolverTest) still passes, confirming the fixture loads and its assertType() assertions hold.

The change was committed and pushed to the PR branch.

[ondrejmirtes]

Would be nice to address the escaped mutants in CI in Infection with more tests

[phpstan-bot]

Done. Here's what I did to address the review ("Would be nice to address the escaped mutants in CI in Infection with more tests").

Investigation

Since CI logs weren't accessible, I reproduced Infection's escaped-mutant detection locally via manual mutation testing: I built a coverage set of all 193 nsrt fixtures containing foreach (a 10s test run that covers the changed processStmtNode foreach lines), then applied each candidate mutation Infection would generate to the diff's lines and checked whether the suite still passed.

I tested 17 mutations across the changed conditions (coalesce swap, ===/!== flips, &&/|| flips, false→true, negation removal). Exactly one survived:

($keyVarExpr !== null || $originalValueExpr !== null)
//  ^^^^^^^^^^^^^^^^^ mutating this operand changed nothing

The two operands only diverge when keyVarExpr !== null && originalValueExpr === null — i.e. a keyed foreach whose value is destructured (foreach ($a as $k => [$v])), so there's no simple value variable to track and narrowing flows purely through the key path. No existing test exercised that combination, so the operand was untested.

The MatchHandler.php change only added comments (not mutable), and the dead-looking key-type init lines are bare method-call assignments Infection doesn't mutate — so neither produced escaped mutants.

Fix

Added a regression case to tests/PHPStan/Analyser/nsrt/bug-5755.php:

foreach ($a as $k => [$v]) {
    if (!is_int($k)) {
        return;
    }
}
assertType('array<int, array{int}>', $a);

I verified it kills the mutant (fails → array<int|string, array{int}> when the operand is dropped, passes otherwise) — the proper before/after check.

Verification

• Full NodeScopeResolverTest: 1652 tests pass.
• make phpstan self-analysis: no errors.
• Test-only change (no source modified); committed and pushed.