Skip to content

Fix spurious error nullCoalesce.offset #5145

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
staabm merged 2 commits into from
Mar 9, 2026
Merged

Fix spurious error nullCoalesce.offset #5145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
47e1e30
Fix spurious error nullCoalesce.offset
staabm Mar 8, 2026
dce1d56
simplify
staabm Mar 9, 2026
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: 6 additions & 7 deletions src/Analyser/ExprHandler/Helper/NonNullabilityHelper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public function ensureShallowNonNullability(MutatingScope $scope, Scope $origina
$originalNativeType = $originalScope->getNativeType($exprToSpecify);

return new EnsuredNonNullabilityResult($scope, [
new EnsuredNonNullabilityResultExpression($exprToSpecify, $originalExprType, $originalNativeType, $certainty),
new EnsuredNonNullabilityResultExpression($exprToSpecify, $originalExprType, $originalNativeType, $hasExpressionType),
Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet Mar 9, 2026

Choose a reason for hiding this comment

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

Do we still need $certainty var ?

Copy link
Copy Markdown
Contributor Author

@staabm staabm Mar 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

yes - its used a few lines below

Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet Mar 9, 2026

Choose a reason for hiding this comment

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

We could maybe move the block

// keep certainty
		$certainty = TrinaryLogic::createYes();
		$hasExpressionType = $originalScope->hasExpressionType($exprToSpecify);
		if (!$hasExpressionType->no()) {
			$certainty = $hasExpressionType;
		}

where it's used in order to avoid those check when they are not needed, but it doesn't cost a lot i think.

]);
}
return new EnsuredNonNullabilityResult($scope, []);
Expand All @@ -55,16 +55,11 @@ public function ensureShallowNonNullability(MutatingScope $scope, Scope $origina
// To properly revert this, we must also save and restore the parent expression's type.
if ($exprToSpecify instanceof Expr\ArrayDimFetch && $exprToSpecify->dim !== null) {
$parentExpr = $exprToSpecify->var;
$parentCertainty = TrinaryLogic::createYes();
$hasParentExpressionType = $originalScope->hasExpressionType($parentExpr);
if (!$hasParentExpressionType->no()) {
$parentCertainty = $hasParentExpressionType;
}
$specifiedExpressions[] = new EnsuredNonNullabilityResultExpression(
$parentExpr,
$scope->getType($parentExpr),
$scope->getNativeType($parentExpr),
$parentCertainty,
$originalScope->hasExpressionType($parentExpr),
);
}

Expand Down Expand Up @@ -104,6 +99,10 @@ public function ensureNonNullability(MutatingScope $scope, Expr $expr): EnsuredN
public function revertNonNullability(MutatingScope $scope, array $specifiedExpressions): MutatingScope
{
foreach ($specifiedExpressions as $specifiedExpressionResult) {
if ($specifiedExpressionResult->getCertainty()->no()) {
$scope = $scope->invalidateExpression($specifiedExpressionResult->getExpression());
continue;
}
$scope = $scope->specifyExpressionType(
$specifiedExpressionResult->getExpression(),
$specifiedExpressionResult->getOriginalType(),
Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ private static function findTestFiles(): iterable
yield __DIR__ . '/data/dio-functions.php';
}

yield __DIR__ . '/../Rules/Variables/data/bug-13921.php';
yield __DIR__ . '/../Rules/Arrays/data/bug-14234.php';
yield __DIR__ . '/../Rules/Arrays/data/bug-11679.php';
yield __DIR__ . '/../Rules/Methods/data/bug-4801.php';
Expand Down
13 changes: 13 additions & 0 deletions tests/PHPStan/Rules/Comparison/BooleanOrConstantConditionRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,4 +371,17 @@ public function testBug4004(): void
$this->analyse([__DIR__ . '/data/bug-4004.php'], []);
}

#[RequiresPhp('>= 8.0')]
public function testBug10305(): void
{
$this->treatPhpDocTypesAsCertain = true;

$this->analyse([__DIR__ . '/data/bug-10305.php'], [
[
'Result of || is always true.',
10,
],
]);
}

}
12 changes: 12 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-10305.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php // lint >= 8.0

namespace Bug10305;

class HelloWorld
{
public null|string $prop;

public function isPropertySet(): bool {
return isset($this->prop) || null === $this->prop;
}
}
10 changes: 10 additions & 0 deletions tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,4 +362,14 @@ public function testBug14213(): void
]);
}

public function testBug13921(): void
{
$this->analyse([__DIR__ . '/data/bug-13921.php'], [
[
'Offset 0 on non-empty-list<array<string|null>> on left side of ?? always exists and is not nullable.',
19,
],
]);
}

}
56 changes: 56 additions & 0 deletions tests/PHPStan/Rules/Variables/data/bug-13921.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<?php

namespace Bug13921;

use SimpleXMLElement;
use function PHPStan\Testing\assertType;

/** @param list<array<?string>> $x */
function foo(array $x): void {
var_dump($x[0]['bar'] ?? null);
assertType("list<array<string|null>>", $x);
var_dump($x[0] ?? null);
}

/** @param non-empty-list<array<?string>> $x */
function nonEmptyFoo(array $x): void {
var_dump($x[0]['bar'] ?? null);
assertType("non-empty-list<array<string|null>>", $x);
var_dump($x[0] ?? null);
}

/** @param list<array<?string>> $x */
function bar(array $x): void {
var_dump($x[0] ?? null);
assertType("list<array<string|null>>", $x);
var_dump($x[0]['bar'] ?? null);
}

/** @param list<array<?string>> $x */
function baz(array $x): void {
var_dump($x[1] ?? null);
assertType("list<array<string|null>>", $x);
var_dump($x[0]['bar'] ?? null);
}

/** @param list<array<?string>> $x */
function boo(array $x): void {
var_dump($x[0]['bar'] ?? null);
assertType("list<array<string|null>>", $x);
var_dump($x[1] ?? null);
}

function doBar(array $array)
{
if (isset($array['foo'])) {
assertType("mixed~null", $array['foo']);
assertType("non-empty-array&hasOffsetValue('foo', mixed~null)", $array);
}
}

/** @param list<SimpleXMLElement> $x */
function sooSimpleElement(array $x): void {
var_dump($x[0]['bar'] ?? null);
assertType("list<SimpleXMLElement>", $x);
var_dump($x[0] ?? null);
}
Loading