Narrow foreach body to non-empty regardless of polluteScopeWithAlwaysIterableForeach

[zonuexe]

Closes phpstan/phpstan#13312

/** @param list<mixed> $arr */
function foo(array $arr): void {
	foreach ($arr as $v) {
		bar($arr);
	}
}

/** @param non-empty-list<mixed> $arr */
function bar(array $arr): void {}

This reproduces with polluteScopeWithAlwaysIterableForeach: false, which phpstan-strict-rules sets.

Inside the loop body $arr is provably non-empty. PHPStan already narrowed it there, but only as a side effect of polluteScopeWithAlwaysIterableForeach: with the flag off the in-body narrowing disappeared. A previous fix (phpstan-src#4162) added a test for it, but only under the default flag value, so the reported case kept failing under phpstan-strict-rules and the issue was reopened.

Gated behind bleedingEdge

The narrowing is gated behind a new bleedingEdge toggle narrowForeachBodyNonEmpty.

• stable (toggle off): the body scope is built exactly as before. polluteScopeWithAlwaysIterableForeach: false behaves as it does today, no change.
• bleedingEdge (toggle on): the foreach body narrows the iterated expression regardless of the flag, so #13312 is fixed.

Narrowing the body also feeds the loop-exit scope, which with the flag off should stay conservative (we must not conclude the loop always iterated). After the body pass the iterated expression's possibly-empty-ness is restored in the after-loop scope, keeping any element types the body refined, so a value variable does not become always-defined after the loop just because the iteratee was narrowed. This containment is partial: definedness that flows through variables the body builds (rather than through the iterated expression itself) still reaches the after-loop scope. That only happens under bleedingEdge.

Second commit

Because the self-analysis config runs with bleedingEdge on, the more precise in-body narrowing proves two facts in PHPStan's own source: a reused foreach value variable in UnionType::accepts(), and a non-null $this->unsealed destructuring in ConstantArrayType. Both are adjusted in a separate commit.

[staabm]

Why are this changes necessary?
(Did your change broke the polluteScopeWithAlwaysIterableForeach use-case?)

[zonuexe]

Good catch, you were right. Those renames were compensating for a real behaviour change, not fixing latent issues.

My first version narrowed the iterated expression for the whole loop, which leaked the non-emptiness into the after-loop scope. With polluteScopeWithAlwaysIterableForeach: false the value variable ended up looking always-defined after the loop, via a conditional ("the iteratee is non-empty, so the value variable is defined") that a later count() > 0 could resolve. That surfaced the new foreach.valueOverwrite errors, and the renames papered over it.

The reworked version applies the narrowing only while analysing the loop body, then strips the non-emptiness back out of the after-loop scope when the flag is off. So the flag's contract holds: after the loop the value variable stays possibly-undefined and the iteratee stays possibly-empty. An iteratee that was already provably non-empty before the loop keeps that property, so nothing downstream regresses. The $internalError and $file renames are gone as a result.

The two adjustments in the second commit are genuine flag-independent facts the more precise in-body analysis now proves: a reused foreach value variable in UnionType::accepts(), and a non-null $this->unsealed destructuring in ConstantArrayType. Neither relates to the after-loop behaviour.

[phpstan-bot]

This pull request has been marked as ready for review.

[staabm]

I think we have the same problem here.. this change got necessary because the PR broke polluteScopeWithAlwaysIterableForeach?

[zonuexe]

You're right, and my earlier reply was wrong. Sorry. I checked: $i in UnionType::accepts() becomes defined after the first loop only because of this change, and it matches what polluteScopeWithAlwaysIterableForeach: true already produces. So it is the same leak, not a flag-independent fact.

The leak flows through conditionals between variables the loop body builds (here $innerAccepts), not through the iterated expression itself, and I could not strip it without also dropping non-emptiness that other code legitimately relies on. So instead of chasing full containment, I've gated the whole behaviour behind a bleedingEdge toggle, narrowForeachBodyNonEmpty.

With bleedingEdge off (stable), polluteScopeWithAlwaysIterableForeach: false behaves exactly as before: no in-body narrowing, no after-loop change. With bleedingEdge on, the body narrows and #13312 is fixed. This rename and the ConstantArrayType guards are only needed because the self-analysis config runs with bleedingEdge on.