Skip to content

Commit e9211ff

Browse files
ondrejmirtesphpstan-bot
authored andcommitted
Merge scope after ?-> method call to account for argument short-circuiting
- In NullsafeMethodCallHandler::processExpr(), merge the post-call scope with the pre-call scope when the var type is nullable, so variables assigned in arguments become "maybe defined" rather than "definitely defined" - When the var type is always null, use the pre-call scope directly since arguments are never evaluated - Add DefinedVariableRule regression tests for nullable, always-null, and multi-argument cases - Add NSRT type inference tests verifying correct types after nullsafe calls
1 parent 03834ec commit e9211ff

File tree

4 files changed

+134
-0
lines changed

4 files changed

+134
-0
lines changed

src/Analyser/ExprHandler/NullsafeMethodCallHandler.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@ public function resolveType(MutatingScope $scope, Expr $expr): Type
6060

6161
public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult
6262
{
63+
$scopeBeforeNullsafe = $scope;
64+
$varType = $scope->getType($expr->var);
65+
6366
$nonNullabilityResult = $this->nonNullabilityHelper->ensureShallowNonNullability($scope, $scope, $expr->var);
6467
$attributes = array_merge($expr->getAttributes(), ['virtualNullsafeMethodCall' => true]);
6568
unset($attributes[ExprPrinter::ATTRIBUTE_CACHE_KEY]);
@@ -78,6 +81,15 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
7881
);
7982
$scope = $this->nonNullabilityHelper->revertNonNullability($exprResult->getScope(), $nonNullabilityResult->getSpecifiedExpressions());
8083

84+
if ($varType->isNull()->yes()) {
85+
// Arguments are never evaluated when the var is always null.
86+
$scope = $scopeBeforeNullsafe;
87+
} elseif (TypeCombinator::containsNull($varType)) {
88+
// Arguments might not be evaluated (short-circuit).
89+
// Merge with the original scope so variables assigned in arguments become "maybe defined".
90+
$scope = $scope->mergeWith($scopeBeforeNullsafe);
91+
}
92+
8193
return new ExpressionResult(
8294
$scope,
8395
hasYield: $exprResult->hasYield(),
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?php // lint >= 8.0
2+
3+
declare(strict_types = 1);
4+
5+
namespace Bug10729Types;
6+
7+
use function PHPStan\Testing\assertType;
8+
9+
class Foo
10+
{
11+
public function bar(string $a, string $b): string
12+
{
13+
return $a . $b;
14+
}
15+
}
16+
17+
function nullable(?Foo $foo): void
18+
{
19+
$foo?->bar($a = 'hello', $b = 'world');
20+
assertType("'hello'|null", $a ?? null);
21+
assertType("'world'|null", $b ?? null);
22+
}
23+
24+
function nonNullable(Foo $foo): void
25+
{
26+
$foo->bar($a = 'hello', $b = 'world');
27+
assertType("'hello'", $a);
28+
assertType("'world'", $b);
29+
}
30+
31+
function alwaysNull(): void
32+
{
33+
$foo = null;
34+
$foo?->bar($a = 'hello', $b = 'world');
35+
assertType('null', $a ?? null); // $a is never assigned when $foo is always null
36+
}
37+
38+
function chainedNullsafe(?Foo $foo): void
39+
{
40+
$result = $foo?->bar($x = 'a', $y = 'b');
41+
assertType('string|null', $result);
42+
assertType("'a'|null", $x ?? null);
43+
assertType("'b'|null", $y ?? null);
44+
}

tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1517,4 +1517,35 @@ public function testBug11218(): void
15171517
$this->analyse([__DIR__ . '/data/bug-11218.php'], []);
15181518
}
15191519

1520+
#[RequiresPhp('>= 8.0')]
1521+
public function testBug10729(): void
1522+
{
1523+
$this->cliArgumentsVariablesRegistered = true;
1524+
$this->polluteScopeWithLoopInitialAssignments = false;
1525+
$this->checkMaybeUndefinedVariables = true;
1526+
$this->polluteScopeWithAlwaysIterableForeach = true;
1527+
$this->analyse([__DIR__ . '/data/bug-10729.php'], [
1528+
[
1529+
'Variable $format might not be defined.',
1530+
12,
1531+
],
1532+
[
1533+
'Undefined variable: $format',
1534+
25,
1535+
],
1536+
[
1537+
'Variable $format might not be defined.',
1538+
31,
1539+
],
1540+
[
1541+
'Variable $value might not be defined.',
1542+
32,
1543+
],
1544+
[
1545+
'Variable $format might not be defined.',
1546+
38,
1547+
],
1548+
]);
1549+
}
1550+
15201551
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php // lint >= 8.0
2+
3+
declare(strict_types = 1);
4+
5+
namespace Bug10729;
6+
7+
class HelloWorld
8+
{
9+
public function sayHello(?\DateTimeImmutable $date): void
10+
{
11+
var_dump($date?->format($format = "Y-m-d"));
12+
var_dump($format); // might not be defined if $date is null
13+
}
14+
15+
public function nonNullable(\DateTimeImmutable $date): void
16+
{
17+
var_dump($date->format($format = "Y-m-d"));
18+
var_dump($format); // always defined, $date can't be null
19+
}
20+
21+
public function nullOnly(): void
22+
{
23+
$date = null;
24+
var_dump($date?->format($format = "Y-m-d"));
25+
var_dump($format); // undefined, $date is always null
26+
}
27+
28+
public function multipleArgs(?\DateTimeImmutable $date): void
29+
{
30+
$date?->createFromFormat($format = 'Y-m-d', $value = '2024-01-01');
31+
var_dump($format); // might not be defined
32+
var_dump($value); // might not be defined
33+
}
34+
35+
public function nestedAssignment(?\DateTimeImmutable $date): void
36+
{
37+
$result = $date?->format($format = "Y-m-d");
38+
var_dump($format); // might not be defined
39+
}
40+
41+
public function existingVarStillDefined(?\DateTimeImmutable $date): void
42+
{
43+
$existing = 'before';
44+
$date?->format($format = "Y-m-d");
45+
var_dump($existing); // always defined, not affected by nullsafe
46+
}
47+
}

0 commit comments

Comments
 (0)