Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions src/Analyser/ExprHandler/NullsafeMethodCallHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand All @@ -78,6 +81,16 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
);
$scope = $this->nonNullabilityHelper->revertNonNullability($exprResult->getScope(), $nonNullabilityResult->getSpecifiedExpressions());

$varIsNull = $varType->isNull();
if ($varIsNull->yes()) {
// Arguments are never evaluated when the var is always null.
$scope = $scopeBeforeNullsafe;
} 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);
}

return new ExpressionResult(
$scope,
hasYield: $exprResult->hasYield(),
Expand Down
44 changes: 44 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-10729.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?php // lint >= 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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,21 @@ function () {
function () {
try {
doesntThrow()?->{$foo = 1}($bar = 2);
} finally {
// doesntThrow() returns mixed which can be null, so ?-> may short-circuit
assertVariableCertainty(TrinaryLogic::createMaybe(), $foo);
assertVariableCertainty(TrinaryLogic::createMaybe(), $bar);
}
};

function () {
$result = doesntThrow();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The call need to be chaine inside the try catch

try {
doesntThrow2()?->{$foo = 1}($bar = 2);
} finally {
		assertVariableCertainty(TrinaryLogic::createYes(), $foo);
		assertVariableCertainty(TrinaryLogic::createYes(), $bar);
	}

If mixed~null is complex to indicate in phpdoc, then maybe use object as return type of doesntThrow2

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);
Expand Down
31 changes: 31 additions & 0 deletions tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
],
]);
}

}
47 changes: 47 additions & 0 deletions tests/PHPStan/Rules/Variables/data/bug-10729.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php // lint >= 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
}
}