Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
VincentLanglet wants to merge 3 commits into
base: 2.1.x
Choose a base branch
from
+67 −1
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ac0e738
Respect original variable type when using extract on optional keys
VincentLanglet Oct 17, 2025
a3eb7f9
Add test
VincentLanglet Dec 5, 2025
b6eaf14
Add test
VincentLanglet Feb 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/Analyser/NodeScopeResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3072,7 +3072,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();
Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
14 changes: 14 additions & 0 deletions tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1011,6 +1011,20 @@ 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.',
18,
],
]);
}

public function testDiscussion10252(): void
{
$this->cliArgumentsVariablesRegistered = true;
Expand Down
41 changes: 41 additions & 0 deletions tests/PHPStan/Rules/Variables/data/bug-12364.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php

namespace Bug12364;

use function PHPStan\Testing\assertType;

/** @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);
Comment on lines +13 to +18
Copy link
Contributor

@staabm staabm Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would also make sense to assertVariableCertainty on the 3 vars

Copy link
Member

@ondrejmirtes ondrejmirtes Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup

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);
}
}
Loading