Fix phpstan#5191

Author: ondrejmirtes

CLAUDE.md

@@ -257,6 +257,10 @@ Loops are a frequent source of false positives because PHPStan must reason about
 - **Always-overwritten arrays in foreach**: NodeScopeResolver examines `$a[$k]` at loop body end and `continue` statements. If no `break` exists, the entire array type can be rewritten based on the observed value types.
 - **Variable types across iterations**: PHP Fibers are used (PHP 8.1+) for more precise analysis of repeated variable assignments in loops, by running the loop body analysis multiple times to reach a fixpoint.
 
+### Match expression scope merging
+
+Match expressions in `NodeScopeResolver` (around line 4154) process each arm and merge the resulting scopes. The critical pattern for variable certainty is: when a match is exhaustive (has a `default` arm or an always-true condition), arm body scopes should be merged only with each other (not with the original pre-match scope). This mirrors how if/else merging works — `$finalScope` starts as `null`, and each branch's scope is merged via `$branchScope->mergeWith($finalScope)`. When the match is NOT exhaustive, the original scope must also participate in the merge (via `$scope->mergeWith($armBodyFinalScope)`) because execution may skip all arms and throw `UnhandledMatchError`. The `mergeVariableHolders()` method in `MutatingScope` uses `ExpressionTypeHolder::createMaybe()` for variables present in only one scope, so merging an arm scope that defines `$x` with the original scope that lacks `$x` degrades certainty to "maybe" — this is the root cause of false "might not be defined" reports for exhaustive match expressions.
+
 ### PHPDoc inheritance
 
 PHPDoc types (`@return`, `@param`, `@throws`, `@property`) are inherited through class hierarchies. Bugs arise when:

src/Analyser/NodeScopeResolver.php

@@ -4166,6 +4166,7 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto
 			$hasAlwaysTrueCond = false;
 			$arms = $expr->arms;
 			$armCondsToSkip = [];
+			$armBodyScopes = [];
 			if ($condType->isEnum()->yes()) {
 				// enum match analysis would work even without this if branch
 				// but would be much slower
@@ -4275,7 +4276,7 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto
 							ExpressionContext::createTopLevel(),
 						);
 						$armScope = $armResult->getScope();
-						$scope = $scope->mergeWith($armScope);
+						$armBodyScopes[] = $armScope;
 						$hasYield = $hasYield || $armResult->hasYield();
 						$throwPoints = array_merge($throwPoints, $armResult->getThrowPoints());
 						$impurePoints = array_merge($impurePoints, $armResult->getImpurePoints());
@@ -4312,7 +4313,7 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto
 					$hasYield = $hasYield || $armResult->hasYield();
 					$throwPoints = array_merge($throwPoints, $armResult->getThrowPoints());
 					$impurePoints = array_merge($impurePoints, $armResult->getImpurePoints());
-					$scope = $scope->mergeWith($matchScope);
+					$armBodyScopes[] = $matchScope;
 					continue;
 				}
 
@@ -4356,20 +4357,38 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto
 					ExpressionContext::createTopLevel(),
 				);
 				$armScope = $armResult->getScope();
-				$scope = $scope->mergeWith($armScope);
+				$armBodyScopes[] = $armScope;
 				$hasYield = $hasYield || $armResult->hasYield();
 				$throwPoints = array_merge($throwPoints, $armResult->getThrowPoints());
 				$impurePoints = array_merge($impurePoints, $armResult->getImpurePoints());
 				$matchScope = $matchScope->filterByFalseyValue($filteringExpr);
 			}
 
-			if (!$hasDefaultCond && !$hasAlwaysTrueCond) {
+			$isExhaustive = $hasDefaultCond || $hasAlwaysTrueCond;
+			if (!$isExhaustive) {
 				$remainingType = $matchScope->getType($expr->cond);
-				if (!$remainingType instanceof NeverType) {
-					$throwPoints[] = InternalThrowPoint::createExplicit($scope, new ObjectType(UnhandledMatchError::class), $expr, false);
+				if ($remainingType instanceof NeverType) {
+					$isExhaustive = true;
 				}
 			}
 
+			if ($isExhaustive) {
+				$armBodyFinalScope = null;
+				foreach ($armBodyScopes as $armBodyScope) {
+					$armBodyFinalScope = $armBodyScope->mergeWith($armBodyFinalScope);
+				}
+				$scope = $armBodyFinalScope ?? $scope;
+			} else {
+				$armBodyFinalScope = null;
+				foreach ($armBodyScopes as $armBodyScope) {
+					$armBodyFinalScope = $armBodyScope->mergeWith($armBodyFinalScope);
+				}
+				if ($armBodyFinalScope !== null) {
+					$scope = $scope->mergeWith($armBodyFinalScope);
+				}
+				$throwPoints[] = InternalThrowPoint::createExplicit($scope, new ObjectType(UnhandledMatchError::class), $expr, false);
+			}
+
 			ksort($armNodes, SORT_NUMERIC);
 
 			$this->callNodeCallback($nodeCallback, new MatchExpressionNode($expr->cond, array_values($armNodes), $expr, $matchScope), $scope, $storage);

tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php

@@ -1208,4 +1208,20 @@ public function testBug13694(): void
 		$this->analyse([__DIR__ . '/data/bug-13694.php'], []);
 	}
 
+	#[RequiresPhp('>= 8.0')]
+	public function testBug5191(): void
+	{
+		$this->cliArgumentsVariablesRegistered = true;
+		$this->polluteScopeWithLoopInitialAssignments = true;
+		$this->checkMaybeUndefinedVariables = true;
+		$this->polluteScopeWithAlwaysIterableForeach = true;
+
+		$this->analyse([__DIR__ . '/data/bug-5191.php'], [
+			[
+				'Variable $pow might not be defined.',
+				23,
+			],
+		]);
+	}
+
 }

tests/PHPStan/Rules/Variables/data/bug-5191.php

@@ -0,0 +1,33 @@
+<?php // lint >= 8.0
+
+declare(strict_types = 1);
+
+namespace Bug5191;
+
+function foo(int $value, ?string $suffix): int {
+	match ($suffix) {
+		'k', 'K' => $pow = 1,
+		'm', 'M' => $pow = 2,
+		'g', 'G' => $pow = 3,
+		default => $pow = 0,
+	};
+	return $value * (1024 ** $pow);
+}
+
+function bar(int $value, ?string $suffix): int {
+	match ($suffix) {
+		'k', 'K' => $pow = 1,
+		'm', 'M' => $pow = 2,
+		'g', 'G' => $pow = 3,
+	};
+	return $value * (1024 ** $pow); // no default, $pow might not be defined
+}
+
+function baz(int $x): int {
+	match (true) {
+		$x > 0 => $result = 1,
+		$x < 0 => $result = -1,
+		default => $result = 0,
+	};
+	return $result;
+}