Respect original variable type when using extract on optional keys (#4450)

Co-authored-by: Markus Staab <maggus.staab@googlemail.com>

Author: VincentLanglet

src/Analyser/NodeScopeResolver.php

@@ -3068,7 +3068,17 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto
 					}
 					foreach ($properties as $name => $type) {
 						$optional = in_array($name, $optionalProperties, true) || $refCount[$name] < count($constantArrays);
-						$scope = $scope->assignVariable($name, $type, $type, $optional ? TrinaryLogic::createMaybe() : TrinaryLogic::createYes());
+
+						if (!$optional) {
+							$scope = $scope->assignVariable($name, $type, $type, TrinaryLogic::createYes());
+						} else {
+							$hasVariable = $scope->hasVariableType($name);
+							if (!$hasVariable->no()) {
+								$type = TypeCombinator::union($scope->getVariableType($name), $type);
+							}
+
+							$scope = $scope->assignVariable($name, $type, $type, $scope->hasVariableType($name)->or(TrinaryLogic::createMaybe()));
+						}
 					}
 				} else {
 					$scope = $scope->afterExtractCall();

tests/PHPStan/Analyser/NodeScopeResolverTest.php

@@ -235,6 +235,7 @@ private static function findTestFiles(): iterable
 		yield __DIR__ . '/../Rules/Comparison/data/bug-5365.php';
 		yield __DIR__ . '/../Rules/Comparison/data/bug-6551.php';
 		yield __DIR__ . '/../Rules/Variables/data/bug-9403.php';
+		yield __DIR__ . '/../Rules/Variables/data/bug-12364.php';
 		yield __DIR__ . '/../Rules/Methods/data/bug-9542.php';
 		yield __DIR__ . '/../Rules/Functions/data/bug-9803.php';
 		yield __DIR__ . '/../Rules/PhpDoc/data/bug-10594.php';

tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php

@@ -1011,6 +1011,24 @@ public function testIsStringNarrowsCertainty(): void
 		]);
 	}
 
+	public function testBug12364(): void
+	{
+		$this->cliArgumentsVariablesRegistered = true;
+		$this->polluteScopeWithLoopInitialAssignments = true;
+		$this->checkMaybeUndefinedVariables = true;
+		$this->polluteScopeWithAlwaysIterableForeach = true;
+		$this->analyse([__DIR__ . '/data/bug-12364.php'], [
+			[
+				'Variable $z might not be defined.',
+				20,
+			],
+			[
+				'Variable $z might not be defined.',
+				23,
+			],
+		]);
+	}
+
 	public function testDiscussion10252(): void
 	{
 		$this->cliArgumentsVariablesRegistered = true;

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

@@ -0,0 +1,46 @@
+<?php
+
+namespace Bug12364;
+
+use PHPStan\TrinaryLogic;
+use function PHPStan\Testing\assertType;
+use function PHPStan\Testing\assertVariableCertainty;
+
+/** @return array{x: string, y?: string, z?: string} */
+function foo(): array {
+	return [ 'x' => 'foo' ];
+}
+
+$x = $y = null;
+assertType('null', $x);
+assertType('null', $y);
+extract(foo());
+assertType('string', $x);
+assertType('string|null', $y); // <-- should be: null|string
+assertType('mixed', $z);
+assertVariableCertainty(TrinaryLogic::createYes(), $x);
+assertVariableCertainty(TrinaryLogic::createYes(), $y);
+assertVariableCertainty(TrinaryLogic::createMaybe(), $z);
+var_dump($x);
+var_dump($y); // <-- does exist
+
+/** @return array{xx: string, yy?: string} */
+function foo2(): array {
+	return [ 'xx' => 'foo' ];
+}
+
+function testUndefined()
+{
+	if (rand(0, 1)) {
+		$xx = $yy = 0;
+		assertType('0', $xx);
+		assertType('0', $yy);
+	}
+
+	extract(foo2());
+	assertType('string', $xx);
+
+	if (isset($yy)) {
+		assertType('0|string', $yy);
+	}
+}