Skip to content

Commit 12edc22

Browse files
staabmphpstan-bot
authored andcommitted
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
1 parent 6bac0de commit 12edc22

5 files changed

Lines changed: 38 additions & 40 deletions

File tree

src/Rules/IssetCheck.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,11 @@ static function (Type $type) use ($typeMessageCallback): ?string {
252252
}
253253

254254
if ($expr instanceof Expr\NullsafePropertyFetch) {
255+
$calledOnType = $this->treatPhpDocTypesAsCertain ? $scope->getScopeType($expr->var) : $scope->getScopeNativeType($expr->var);
256+
if (!$calledOnType->isNull()->no()) {
257+
return null;
258+
}
259+
255260
if ($expr->name instanceof Node\Identifier) {
256261
return RuleErrorBuilder::message(sprintf('Using nullsafe property access "?->%s" %s is unnecessary. Use -> instead.', $expr->name->name, $operatorDescription))
257262
->identifier('nullsafe.neverNull')

tests/PHPStan/Rules/Variables/EmptyRuleTest.php

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -115,30 +115,14 @@ public function testBug7109(): void
115115
$this->treatPhpDocTypesAsCertain = true;
116116

117117
$this->analyse([__DIR__ . '/../Properties/data/bug-7109.php'], [
118-
[
119-
'Using nullsafe property access "?->aaa" in empty() is unnecessary. Use -> instead.',
120-
19,
121-
],
122-
[
123-
'Using nullsafe property access "?->aaa" in empty() is unnecessary. Use -> instead.',
124-
30,
125-
],
126118
[
127119
'Using nullsafe property access "?->aaa" in empty() is unnecessary. Use -> instead.',
128120
42,
129121
],
130-
[
131-
'Using nullsafe property access "?->notFalsy" in empty() is unnecessary. Use -> instead.',
132-
54,
133-
],
134122
[
135123
'Expression in empty() is not falsy.',
136124
59,
137125
],
138-
[
139-
'Using nullsafe property access "?->aaa" in empty() is unnecessary. Use -> instead.',
140-
68,
141-
],
142126
[
143127
'Using nullsafe property access "?->(Expression)" in empty() is unnecessary. Use -> instead.',
144128
75,

tests/PHPStan/Rules/Variables/IssetRuleTest.php

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -333,22 +333,10 @@ public function testBug7109(): void
333333
$this->treatPhpDocTypesAsCertain = true;
334334

335335
$this->analyse([__DIR__ . '/../Properties/data/bug-7109.php'], [
336-
[
337-
'Using nullsafe property access "?->aaa" in isset() is unnecessary. Use -> instead.',
338-
18,
339-
],
340-
[
341-
'Using nullsafe property access "?->aaa" in isset() is unnecessary. Use -> instead.',
342-
29,
343-
],
344336
[
345337
'Expression in isset() is not nullable.',
346338
41,
347339
],
348-
[
349-
'Using nullsafe property access "?->aaa" in isset() is unnecessary. Use -> instead.',
350-
67,
351-
],
352340
[
353341
'Expression in isset() is not nullable.',
354342
74,

tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -253,22 +253,10 @@ public function testBug5933(): void
253253
public function testBug7109(): void
254254
{
255255
$this->analyse([__DIR__ . '/../Properties/data/bug-7109.php'], [
256-
[
257-
'Using nullsafe property access "?->aaa" on left side of ?? is unnecessary. Use -> instead.',
258-
17,
259-
],
260-
[
261-
'Using nullsafe property access "?->aaa" on left side of ?? is unnecessary. Use -> instead.',
262-
28,
263-
],
264256
[
265257
'Expression on left side of ?? is not nullable.',
266258
40,
267259
],
268-
[
269-
'Using nullsafe property access "?->aaa" on left side of ?? is unnecessary. Use -> instead.',
270-
66,
271-
],
272260
[
273261
'Expression on left side of ?? is not nullable.',
274262
73,
@@ -377,4 +365,10 @@ public function testBug13921(): void
377365
]);
378366
}
379367

368+
#[RequiresPhp('>= 8.0')]
369+
public function testBug14311(): void
370+
{
371+
$this->analyse([__DIR__ . '/data/bug-14311.php'], []);
372+
}
373+
380374
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?php // lint >= 8.0
2+
3+
declare(strict_types = 1);
4+
5+
namespace Bug14311;
6+
7+
class Station {
8+
public string $address = '';
9+
}
10+
11+
/** @param array<string, Station> $stations */
12+
function withNullCoalesce(array $stations, string $key): string {
13+
$bar = $stations[$key] ?? null;
14+
return $bar?->address ?? 'Unknown';
15+
}
16+
17+
/** @param array<string, Station> $stations */
18+
function withIsset(array $stations, string $key): string {
19+
$bar = isset($stations[$key]) ? $stations[$key] : null;
20+
return $bar?->address ?? 'Unknown';
21+
}
22+
23+
/** @param array<string, Station> $stations */
24+
function withArrayKeyExists(array $stations, string $key): string {
25+
$bar = array_key_exists($key, $stations) ? $stations[$key] : null;
26+
return $bar?->address ?? 'Unknown';
27+
}

0 commit comments

Comments
 (0)