From 12edc220d2a2ea7b92598d75802a4669d12b768b Mon Sep 17 00:00:00 2001 From: staabm <120441+staabm@users.noreply.github.com> Date: Thu, 19 Mar 2026 07:06:48 +0000 Subject: [PATCH] Fix phpstan/phpstan#14311: nullsafe.neverNull false positive when object can be null - In IssetCheck, the nullsafe.neverNull error was reported unconditionally for NullsafePropertyFetch inside ??, isset(), and empty(), without checking if the left side of ?-> could actually be null - Added a check on $expr->var's type before reporting the error, consistent with how NullsafePropertyFetchRule already handles this - Updated test expectations for bug-7109 in NullCoalesceRuleTest, IssetRuleTest, and EmptyRuleTest to remove false positive expectations - New regression test in tests/PHPStan/Rules/Variables/data/bug-14311.php --- src/Rules/IssetCheck.php | 5 ++++ .../PHPStan/Rules/Variables/EmptyRuleTest.php | 16 ----------- .../PHPStan/Rules/Variables/IssetRuleTest.php | 12 --------- .../Rules/Variables/NullCoalesceRuleTest.php | 18 +++++-------- .../Rules/Variables/data/bug-14311.php | 27 +++++++++++++++++++ 5 files changed, 38 insertions(+), 40 deletions(-) create mode 100644 tests/PHPStan/Rules/Variables/data/bug-14311.php diff --git a/src/Rules/IssetCheck.php b/src/Rules/IssetCheck.php index 2e1adcf1c6..1515854ea3 100644 --- a/src/Rules/IssetCheck.php +++ b/src/Rules/IssetCheck.php @@ -252,6 +252,11 @@ static function (Type $type) use ($typeMessageCallback): ?string { } if ($expr instanceof Expr\NullsafePropertyFetch) { + $calledOnType = $this->treatPhpDocTypesAsCertain ? $scope->getScopeType($expr->var) : $scope->getScopeNativeType($expr->var); + if (!$calledOnType->isNull()->no()) { + return null; + } + if ($expr->name instanceof Node\Identifier) { return RuleErrorBuilder::message(sprintf('Using nullsafe property access "?->%s" %s is unnecessary. Use -> instead.', $expr->name->name, $operatorDescription)) ->identifier('nullsafe.neverNull') diff --git a/tests/PHPStan/Rules/Variables/EmptyRuleTest.php b/tests/PHPStan/Rules/Variables/EmptyRuleTest.php index 873c89fb86..7d51f3371e 100644 --- a/tests/PHPStan/Rules/Variables/EmptyRuleTest.php +++ b/tests/PHPStan/Rules/Variables/EmptyRuleTest.php @@ -115,30 +115,14 @@ public function testBug7109(): void $this->treatPhpDocTypesAsCertain = true; $this->analyse([__DIR__ . '/../Properties/data/bug-7109.php'], [ - [ - 'Using nullsafe property access "?->aaa" in empty() is unnecessary. Use -> instead.', - 19, - ], - [ - 'Using nullsafe property access "?->aaa" in empty() is unnecessary. Use -> instead.', - 30, - ], [ 'Using nullsafe property access "?->aaa" in empty() is unnecessary. Use -> instead.', 42, ], - [ - 'Using nullsafe property access "?->notFalsy" in empty() is unnecessary. Use -> instead.', - 54, - ], [ 'Expression in empty() is not falsy.', 59, ], - [ - 'Using nullsafe property access "?->aaa" in empty() is unnecessary. Use -> instead.', - 68, - ], [ 'Using nullsafe property access "?->(Expression)" in empty() is unnecessary. Use -> instead.', 75, diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php index be2f2b3c68..ea94c105fa 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -333,22 +333,10 @@ public function testBug7109(): void $this->treatPhpDocTypesAsCertain = true; $this->analyse([__DIR__ . '/../Properties/data/bug-7109.php'], [ - [ - 'Using nullsafe property access "?->aaa" in isset() is unnecessary. Use -> instead.', - 18, - ], - [ - 'Using nullsafe property access "?->aaa" in isset() is unnecessary. Use -> instead.', - 29, - ], [ 'Expression in isset() is not nullable.', 41, ], - [ - 'Using nullsafe property access "?->aaa" in isset() is unnecessary. Use -> instead.', - 67, - ], [ 'Expression in isset() is not nullable.', 74, diff --git a/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php b/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php index 1833e4f681..c941aa032f 100644 --- a/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php +++ b/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php @@ -253,22 +253,10 @@ public function testBug5933(): void public function testBug7109(): void { $this->analyse([__DIR__ . '/../Properties/data/bug-7109.php'], [ - [ - 'Using nullsafe property access "?->aaa" on left side of ?? is unnecessary. Use -> instead.', - 17, - ], - [ - 'Using nullsafe property access "?->aaa" on left side of ?? is unnecessary. Use -> instead.', - 28, - ], [ 'Expression on left side of ?? is not nullable.', 40, ], - [ - 'Using nullsafe property access "?->aaa" on left side of ?? is unnecessary. Use -> instead.', - 66, - ], [ 'Expression on left side of ?? is not nullable.', 73, @@ -377,4 +365,10 @@ public function testBug13921(): void ]); } + #[RequiresPhp('>= 8.0')] + public function testBug14311(): void + { + $this->analyse([__DIR__ . '/data/bug-14311.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Variables/data/bug-14311.php b/tests/PHPStan/Rules/Variables/data/bug-14311.php new file mode 100644 index 0000000000..53593eb848 --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/bug-14311.php @@ -0,0 +1,27 @@ += 8.0 + +declare(strict_types = 1); + +namespace Bug14311; + +class Station { + public string $address = ''; +} + +/** @param array $stations */ +function withNullCoalesce(array $stations, string $key): string { + $bar = $stations[$key] ?? null; + return $bar?->address ?? 'Unknown'; +} + +/** @param array $stations */ +function withIsset(array $stations, string $key): string { + $bar = isset($stations[$key]) ? $stations[$key] : null; + return $bar?->address ?? 'Unknown'; +} + +/** @param array $stations */ +function withArrayKeyExists(array $stations, string $key): string { + $bar = array_key_exists($key, $stations) ? $stations[$key] : null; + return $bar?->address ?? 'Unknown'; +}