Fix phpstan/phpstan#14446: Incorrect type-narrowing when using strict rules

[phpstan-bot]

Summary

When polluteScopeWithAlwaysIterableForeach is false (enabled by strict rules), PHPStan incorrectly narrowed variables that were aliased via assignment. For example, $current = $initial followed by if (!$current) would incorrectly narrow $initial to false after a foreach loop reassigned $current, because the alias relationship was not properly invalidated.

Changes

• src/Analyser/MutatingScope.php: In processAlwaysIterableForeachScopeWithoutPollute(), changed from using $this->conditionalExpressions (pre-foreach) to $finalScope->conditionalExpressions (post-foreach body). This ensures that conditional expressions invalidated by assignments inside the foreach body (like $current = false) are not incorrectly preserved.
• Added regression test tests/PHPStan/Analyser/Bug14446Test.php with test data and config.

Root cause

processAlwaysIterableForeachScopeWithoutPollute() takes expression types from $finalScope (the scope after the foreach body runs) but was using conditional expressions from $this (the scope before the foreach). When $current = $initial creates a conditional expression linking the two variables, and then $current = false inside a foreach invalidates that link in $finalScope, the method was still using the old (stale) conditional expressions from $this. This caused $initial to be incorrectly narrowed to false when $current was false (from the foreach reassignment), even though the alias was no longer valid.

In contrast, when polluteScopeWithAlwaysIterableForeach is true, the generalizeWith() path widens the type of $current back to bool, so the conditional expression never fires — masking the issue.

Test

Added tests/PHPStan/Analyser/Bug14446Test.php which uses polluteScopeWithAlwaysIterableForeach: false config and verifies that $initial remains bool throughout the function — at the top of the while loop, inside the if (!$current) block, and after the while loop exits.

Fixes phpstan/phpstan#14446

[VincentLanglet]

This produce the unwanted error

Foreach overwrites $key with its key variable.     

with the code

if (is_array($value) && $value !== []) {
			$hasOnlyStringKey = true;
			foreach (array_keys($value) as $key) {
				if (is_int($key)) {
					$hasOnlyStringKey = false;
					break;
				}
			}

			if ($hasOnlyStringKey) {
				$keyTypes = [];
				$valueTypes = [];
				foreach ($value as $key => $element) {
					$keyType = $scope->getTypeFromValue($key);
					$keyStringTypes = $keyType->getConstantStrings();
					if (count($keyStringTypes) !== 1) {
						throw new ShouldNotHappenException();
					}
					$keyTypes[] = $keyStringTypes[0];
					$valueTypes[] = $this->generalizeTypeFromValue($scope, $element);
				}

				return ConstantArrayTypeBuilder::createFromConstantArray(
					new ConstantArrayType($keyTypes, $valueTypes),
				)->getArray();
			}

			return new ArrayType(
				TypeCombinator::union(...array_map(fn ($item): Type => $this->generalizeTypeFromValue($scope, $item), array_keys($value))),
				TypeCombinator::union(...array_map(fn ($item): Type => $this->generalizeTypeFromValue($scope, $item), array_values($value))),
			);
		}

Since $key is only defined in the foreach loop the error shouldn't be reported with pollute false

[phpstan-bot]

Pushed. Here's what I did:

Problem: The PR's change to use $finalScope->conditionalExpressions caused conditional expressions referencing foreach-only variables (like $key) to leak into the outer scope. When those conditional expressions fired inside a subsequent if block, the foreach variable was re-added to scope with Yes certainty, causing a false "Foreach overwrites $key with its key variable" error.

Fix (src/Analyser/MutatingScope.php:3873-3892): Filter $finalScope->conditionalExpressions to only keep entries where:

1. The target expression exists in $this->expressionTypes (pre-foreach scope)
2. All condition expression holders reference expressions that exist in $this->expressionTypes

This preserves the original bug 14446 fix (stale conditional expressions from $this are not used) while preventing foreach-body-only variables from leaking via conditional expressions.

Test: Added testForeachKeyOverwrite() to the test data file reproducing the reviewer's exact scenario — two foreach loops with $key, where the second should not trigger the overwrite warning.

[VincentLanglet]

This now produce

------ ---------------------------------------------------------------------------------------------------- 
  Line   src\Analyser\NodeScopeResolver.php                                                                  
 ------ ---------------------------------------------------------------------------------------------------- 
  437    Call to an undefined method PhpParser\Node::getReturnType().                                        
         🪪  method.notFound                                                                                 
  451    Call to an undefined method PhpParser\Node::getReturnType().                                        
         🪪  method.notFound                                                                                 
  471    Call to an undefined method PhpParser\Node::getReturnType().                                        
         🪪  method.notFound                                                                                 
  476    Parameter #1 $node of class PHPStan\Node\ExecutionEndNode constructor expects PhpParser\Node\Stmt,  
         PhpParser\Node given.                                                                               
         🪪  argument.type                                                                                   
 ------ ---------------------------------------------------------------------------------------------------- 

So I assume your fix is not correct

[phpstan-bot]

Pushed the fix. Here's a summary of the change:

Problem: The previous approach used $finalScope->conditionalExpressions (post-foreach) as the base, which lost valid conditional expressions that existed before the foreach but weren't preserved in $finalScope. This caused false positives like Call to an undefined method PhpParser\Node::getReturnType() when PHPStan analyzed its own codebase with strict rules (polluteScopeWithAlwaysIterableForeach: false).

Fix: Start from $this->conditionalExpressions (pre-foreach) and only remove entries where a condition variable's type changed between $this and $finalScope. A type change indicates the variable was reassigned inside the foreach body, making the conditional expression stale. When detected, the entire key is removed (not individual holders) to avoid partial/inconsistent state.

Verification:

• make phpstan - no errors (was 4 errors before)
• make tests - all 11826 tests pass
• Bug 14446 regression test passes