Skip to content

Commit 9b0b84a

Browse files
ondrejmirtesVincentLanglet
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 5ab83ce commit 9b0b84a

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
@@ -1531,4 +1531,35 @@ public function testBug6688(): void
15311531
]);
15321532
}
15331533

1534+
#[RequiresPhp('>= 8.0')]
1535+
public function testBug10729(): void
1536+
{
1537+
$this->cliArgumentsVariablesRegistered = true;
1538+
$this->polluteScopeWithLoopInitialAssignments = false;
1539+
$this->checkMaybeUndefinedVariables = true;
1540+
$this->polluteScopeWithAlwaysIterableForeach = true;
1541+
$this->analyse([__DIR__ . '/data/bug-10729.php'], [
1542+
[
1543+
'Variable $format might not be defined.',
1544+
12,
1545+
],
1546+
[
1547+
'Undefined variable: $format',
1548+
25,
1549+
],
1550+
[
1551+
'Variable $format might not be defined.',
1552+
31,
1553+
],
1554+
[
1555+
'Variable $value might not be defined.',
1556+
32,
1557+
],
1558+
[
1559+
'Variable $format might not be defined.',
1560+
38,
1561+
],
1562+
]);
1563+
}
1564+
15341565
}
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)