From e9211ff5ab55e1958b6b8b674ef2525b0617cdaa Mon Sep 17 00:00:00 2001 From: ondrejmirtes <104888+ondrejmirtes@users.noreply.github.com> Date: Tue, 14 Apr 2026 07:31:56 +0000 Subject: [PATCH 1/3] 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 --- .../ExprHandler/NullsafeMethodCallHandler.php | 12 +++++ tests/PHPStan/Analyser/nsrt/bug-10729.php | 44 +++++++++++++++++ .../Variables/DefinedVariableRuleTest.php | 31 ++++++++++++ .../Rules/Variables/data/bug-10729.php | 47 +++++++++++++++++++ 4 files changed, 134 insertions(+) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-10729.php create mode 100644 tests/PHPStan/Rules/Variables/data/bug-10729.php diff --git a/src/Analyser/ExprHandler/NullsafeMethodCallHandler.php b/src/Analyser/ExprHandler/NullsafeMethodCallHandler.php index 13c0577f859..b71ea65f5d2 100644 --- a/src/Analyser/ExprHandler/NullsafeMethodCallHandler.php +++ b/src/Analyser/ExprHandler/NullsafeMethodCallHandler.php @@ -60,6 +60,9 @@ public function resolveType(MutatingScope $scope, Expr $expr): Type public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult { + $scopeBeforeNullsafe = $scope; + $varType = $scope->getType($expr->var); + $nonNullabilityResult = $this->nonNullabilityHelper->ensureShallowNonNullability($scope, $scope, $expr->var); $attributes = array_merge($expr->getAttributes(), ['virtualNullsafeMethodCall' => true]); unset($attributes[ExprPrinter::ATTRIBUTE_CACHE_KEY]); @@ -78,6 +81,15 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex ); $scope = $this->nonNullabilityHelper->revertNonNullability($exprResult->getScope(), $nonNullabilityResult->getSpecifiedExpressions()); + if ($varType->isNull()->yes()) { + // Arguments are never evaluated when the var is always null. + $scope = $scopeBeforeNullsafe; + } elseif (TypeCombinator::containsNull($varType)) { + // Arguments might not be evaluated (short-circuit). + // Merge with the original scope so variables assigned in arguments become "maybe defined". + $scope = $scope->mergeWith($scopeBeforeNullsafe); + } + return new ExpressionResult( $scope, hasYield: $exprResult->hasYield(), diff --git a/tests/PHPStan/Analyser/nsrt/bug-10729.php b/tests/PHPStan/Analyser/nsrt/bug-10729.php new file mode 100644 index 00000000000..66be14727e2 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-10729.php @@ -0,0 +1,44 @@ += 8.0 + +declare(strict_types = 1); + +namespace Bug10729Types; + +use function PHPStan\Testing\assertType; + +class Foo +{ + public function bar(string $a, string $b): string + { + return $a . $b; + } +} + +function nullable(?Foo $foo): void +{ + $foo?->bar($a = 'hello', $b = 'world'); + assertType("'hello'|null", $a ?? null); + assertType("'world'|null", $b ?? null); +} + +function nonNullable(Foo $foo): void +{ + $foo->bar($a = 'hello', $b = 'world'); + assertType("'hello'", $a); + assertType("'world'", $b); +} + +function alwaysNull(): void +{ + $foo = null; + $foo?->bar($a = 'hello', $b = 'world'); + assertType('null', $a ?? null); // $a is never assigned when $foo is always null +} + +function chainedNullsafe(?Foo $foo): void +{ + $result = $foo?->bar($x = 'a', $y = 'b'); + assertType('string|null', $result); + assertType("'a'|null", $x ?? null); + assertType("'b'|null", $y ?? null); +} diff --git a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php index 207070c7627..db45cf0dbb1 100644 --- a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php +++ b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php @@ -1517,4 +1517,35 @@ public function testBug11218(): void $this->analyse([__DIR__ . '/data/bug-11218.php'], []); } + #[RequiresPhp('>= 8.0')] + public function testBug10729(): void + { + $this->cliArgumentsVariablesRegistered = true; + $this->polluteScopeWithLoopInitialAssignments = false; + $this->checkMaybeUndefinedVariables = true; + $this->polluteScopeWithAlwaysIterableForeach = true; + $this->analyse([__DIR__ . '/data/bug-10729.php'], [ + [ + 'Variable $format might not be defined.', + 12, + ], + [ + 'Undefined variable: $format', + 25, + ], + [ + 'Variable $format might not be defined.', + 31, + ], + [ + 'Variable $value might not be defined.', + 32, + ], + [ + 'Variable $format might not be defined.', + 38, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Variables/data/bug-10729.php b/tests/PHPStan/Rules/Variables/data/bug-10729.php new file mode 100644 index 00000000000..e570ac3694a --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/bug-10729.php @@ -0,0 +1,47 @@ += 8.0 + +declare(strict_types = 1); + +namespace Bug10729; + +class HelloWorld +{ + public function sayHello(?\DateTimeImmutable $date): void + { + var_dump($date?->format($format = "Y-m-d")); + var_dump($format); // might not be defined if $date is null + } + + public function nonNullable(\DateTimeImmutable $date): void + { + var_dump($date->format($format = "Y-m-d")); + var_dump($format); // always defined, $date can't be null + } + + public function nullOnly(): void + { + $date = null; + var_dump($date?->format($format = "Y-m-d")); + var_dump($format); // undefined, $date is always null + } + + public function multipleArgs(?\DateTimeImmutable $date): void + { + $date?->createFromFormat($format = 'Y-m-d', $value = '2024-01-01'); + var_dump($format); // might not be defined + var_dump($value); // might not be defined + } + + public function nestedAssignment(?\DateTimeImmutable $date): void + { + $result = $date?->format($format = "Y-m-d"); + var_dump($format); // might not be defined + } + + public function existingVarStillDefined(?\DateTimeImmutable $date): void + { + $existing = 'before'; + $date?->format($format = "Y-m-d"); + var_dump($existing); // always defined, not affected by nullsafe + } +} From 1d911402c001d487b330460337e27b2b00b472bc Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Tue, 14 Apr 2026 07:54:13 +0000 Subject: [PATCH 2/3] Use $varType->isNull()->maybe() instead of TypeCombinator::containsNull() Address review feedback: cache $varType->isNull() in a variable to avoid computing it twice, and use ->maybe() instead of TypeCombinator::containsNull() for the nullable check. Update throw-points test: doesntThrow() returns mixed which can be null, so ?-> may short-circuit and variables assigned in the method name/arguments should be "maybe defined", not "yes defined". Co-Authored-By: Claude Opus 4.6 --- src/Analyser/ExprHandler/NullsafeMethodCallHandler.php | 5 +++-- .../nsrt/throw-points/php8/null-safe-method-call.php | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Analyser/ExprHandler/NullsafeMethodCallHandler.php b/src/Analyser/ExprHandler/NullsafeMethodCallHandler.php index b71ea65f5d2..6dc0ddad48b 100644 --- a/src/Analyser/ExprHandler/NullsafeMethodCallHandler.php +++ b/src/Analyser/ExprHandler/NullsafeMethodCallHandler.php @@ -81,10 +81,11 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex ); $scope = $this->nonNullabilityHelper->revertNonNullability($exprResult->getScope(), $nonNullabilityResult->getSpecifiedExpressions()); - if ($varType->isNull()->yes()) { + $varIsNull = $varType->isNull(); + if ($varIsNull->yes()) { // Arguments are never evaluated when the var is always null. $scope = $scopeBeforeNullsafe; - } elseif (TypeCombinator::containsNull($varType)) { + } elseif ($varIsNull->maybe()) { // Arguments might not be evaluated (short-circuit). // Merge with the original scope so variables assigned in arguments become "maybe defined". $scope = $scope->mergeWith($scopeBeforeNullsafe); diff --git a/tests/PHPStan/Analyser/nsrt/throw-points/php8/null-safe-method-call.php b/tests/PHPStan/Analyser/nsrt/throw-points/php8/null-safe-method-call.php index 7be88b90903..7d387229c6d 100644 --- a/tests/PHPStan/Analyser/nsrt/throw-points/php8/null-safe-method-call.php +++ b/tests/PHPStan/Analyser/nsrt/throw-points/php8/null-safe-method-call.php @@ -42,8 +42,9 @@ function () { try { doesntThrow()?->{$foo = 1}($bar = 2); } finally { - assertVariableCertainty(TrinaryLogic::createYes(), $foo); - assertVariableCertainty(TrinaryLogic::createYes(), $bar); + // doesntThrow() returns mixed which can be null, so ?-> may short-circuit + assertVariableCertainty(TrinaryLogic::createMaybe(), $foo); + assertVariableCertainty(TrinaryLogic::createMaybe(), $bar); } }; From 12bdb774144f43c94d7803f2bdb3040b502f3526 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Tue, 14 Apr 2026 08:19:32 +0000 Subject: [PATCH 3/3] Add throw-points test for mixed~null (non-nullable) nullsafe method call Add a companion test case where the left operand of ?-> is mixed~null (obtained via null-narrowing). Since the value can never be null, ?-> never short-circuits and variables assigned in arguments have Yes certainty. Co-Authored-By: Claude Opus 4.6 --- .../throw-points/php8/null-safe-method-call.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/PHPStan/Analyser/nsrt/throw-points/php8/null-safe-method-call.php b/tests/PHPStan/Analyser/nsrt/throw-points/php8/null-safe-method-call.php index 7d387229c6d..666bbfcef90 100644 --- a/tests/PHPStan/Analyser/nsrt/throw-points/php8/null-safe-method-call.php +++ b/tests/PHPStan/Analyser/nsrt/throw-points/php8/null-safe-method-call.php @@ -48,6 +48,20 @@ function () { } }; +function () { + $result = doesntThrow(); + if ($result === null) { + return; + } + // $result is mixed~null, so ?-> never short-circuits + try { + $result?->{$foo = 1}($bar = 2); + } finally { + assertVariableCertainty(TrinaryLogic::createYes(), $foo); + assertVariableCertainty(TrinaryLogic::createYes(), $bar); + } +}; + function () { try { maybeThrows()?->{$foo = 1}($bar = 2);