Skip to content

Commit 3de3c85

Browse files
staabmstaabm
authored
Fix variable certainty for ArrayDimFetch in falsey isset context
1 parent 1308c52 commit 3de3c85

11 files changed

Lines changed: 379 additions & 17 deletions

‎src/Analyser/EnsuredNonNullabilityResultExpression.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace PHPStan\Analyser;
44

55
use PhpParser\Node\Expr;
6+
use PHPStan\TrinaryLogic;
67
use PHPStan\Type\Type;
78

89
class EnsuredNonNullabilityResultExpression
@@ -12,6 +13,7 @@ public function __construct(
1213
private Expr $expression,
1314
private Type $originalType,
1415
private Type $originalNativeType,
16+
private TrinaryLogic $certainty,
1517
)
1618
{
1719
}
@@ -31,4 +33,9 @@ public function getOriginalNativeType(): Type
3133
return $this->originalNativeType;
3234
}
3335

36+
public function getCertainty(): TrinaryLogic
37+
{
38+
return $this->certainty;
39+
}
40+
3441
}

‎src/Analyser/MutatingScope.php‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2340,6 +2340,10 @@ public function isSpecified(Expr $node): bool
23402340
/** @api */
23412341
public function hasExpressionType(Expr $node): TrinaryLogic
23422342
{
2343+
if ($node instanceof Variable && is_string($node->name)) {
2344+
return $this->hasVariableType($node->name);
2345+
}
2346+
23432347
$exprString = $this->getNodeKey($node);
23442348
if (!isset($this->expressionTypes[$exprString])) {
23452349
return TrinaryLogic::createNo();
@@ -3432,7 +3436,7 @@ public function unsetExpression(Expr $expr): self
34323436
return $scope->invalidateExpression($expr);
34333437
}
34343438

3435-
public function specifyExpressionType(Expr $expr, Type $type, Type $nativeType): self
3439+
public function specifyExpressionType(Expr $expr, Type $type, Type $nativeType, ?TrinaryLogic $certainty = null): self
34363440
{
34373441
if ($expr instanceof ConstFetch) {
34383442
$loweredConstName = strtolower($expr->name->toString());
@@ -3466,23 +3470,31 @@ public function specifyExpressionType(Expr $expr, Type $type, Type $nativeType):
34663470
if ($dimType instanceof ConstantIntegerType) {
34673471
$types[] = new StringType();
34683472
}
3473+
34693474
$scope = $scope->specifyExpressionType(
34703475
$expr->var,
34713476
TypeCombinator::intersect(
34723477
TypeCombinator::intersect($exprVarType, TypeCombinator::union(...$types)),
34733478
new HasOffsetValueType($dimType, $type),
34743479
),
34753480
$scope->getNativeType($expr->var),
3481+
$certainty,
34763482
);
34773483
}
34783484
}
34793485
}
34803486

3487+
if ($certainty === null) {
3488+
$certainty = TrinaryLogic::createYes();
3489+
} elseif ($certainty->no()) {
3490+
throw new ShouldNotHappenException();
3491+
}
3492+
34813493
$exprString = $this->getNodeKey($expr);
34823494
$expressionTypes = $scope->expressionTypes;
3483-
$expressionTypes[$exprString] = ExpressionTypeHolder::createYes($expr, $type);
3495+
$expressionTypes[$exprString] = new ExpressionTypeHolder($expr, $type, $certainty);
34843496
$nativeTypes = $scope->nativeExpressionTypes;
3485-
$nativeTypes[$exprString] = ExpressionTypeHolder::createYes($expr, $nativeType);
3497+
$nativeTypes[$exprString] = new ExpressionTypeHolder($expr, $nativeType, $certainty);
34863498

34873499
return $this->scopeFactory->create(
34883500
$this->context,

‎src/Analyser/NodeScopeResolver.php‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1753,14 +1753,22 @@ private function ensureShallowNonNullability(MutatingScope $scope, Scope $origin
17531753
if ($isNull->yes()) {
17541754
return new EnsuredNonNullabilityResult($scope, []);
17551755
}
1756+
1757+
// keep certainty
1758+
$certainty = TrinaryLogic::createYes();
1759+
$hasExpressionType = $originalScope->hasExpressionType($exprToSpecify);
1760+
if (!$hasExpressionType->no()) {
1761+
$certainty = $hasExpressionType;
1762+
}
1763+
17561764
$exprTypeWithoutNull = TypeCombinator::removeNull($exprType);
17571765
if ($exprType->equals($exprTypeWithoutNull)) {
17581766
$originalExprType = $originalScope->getType($exprToSpecify);
17591767
if (!$originalExprType->equals($exprTypeWithoutNull)) {
17601768
$originalNativeType = $originalScope->getNativeType($exprToSpecify);
17611769

17621770
return new EnsuredNonNullabilityResult($scope, [
1763-
new EnsuredNonNullabilityResultExpression($exprToSpecify, $originalExprType, $originalNativeType),
1771+
new EnsuredNonNullabilityResultExpression($exprToSpecify, $originalExprType, $originalNativeType, $certainty),
17641772
]);
17651773
}
17661774
return new EnsuredNonNullabilityResult($scope, []);
@@ -1776,7 +1784,7 @@ private function ensureShallowNonNullability(MutatingScope $scope, Scope $origin
17761784
return new EnsuredNonNullabilityResult(
17771785
$scope,
17781786
[
1779-
new EnsuredNonNullabilityResultExpression($exprToSpecify, $exprType, $nativeType),
1787+
new EnsuredNonNullabilityResultExpression($exprToSpecify, $exprType, $nativeType, $certainty),
17801788
],
17811789
);
17821790
}
@@ -1806,6 +1814,7 @@ private function revertNonNullability(MutatingScope $scope, array $specifiedExpr
18061814
$specifiedExpressionResult->getExpression(),
18071815
$specifiedExpressionResult->getOriginalType(),
18081816
$specifiedExpressionResult->getOriginalNativeType(),
1817+
$specifiedExpressionResult->getCertainty(),
18091818
);
18101819
}
18111820

‎src/Analyser/TypeSpecifier.php‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -717,11 +717,14 @@ public function specifyTypesInCondition(
717717

718718
$types = null;
719719
foreach ($vars as $var) {
720+
$type = new SpecifiedTypes();
721+
720722
if ($var instanceof Expr\Variable && is_string($var->name)) {
721723
if ($scope->hasVariableType($var->name)->no()) {
722724
return new SpecifiedTypes([], [], false, [], $rootExpr);
723725
}
724726
}
727+
725728
if (
726729
$var instanceof ArrayDimFetch
727730
&& $var->dim !== null
@@ -738,36 +741,32 @@ public function specifyTypesInCondition(
738741
$scope,
739742
$rootExpr,
740743
);
741-
} else {
742-
$type = new SpecifiedTypes();
743744
}
744-
745-
$type = $type->unionWith(
746-
$this->create($var, new NullType(), TypeSpecifierContext::createFalse(), false, $scope, $rootExpr),
747-
);
748-
} else {
749-
$type = $this->create($var, new NullType(), TypeSpecifierContext::createFalse(), false, $scope, $rootExpr);
750745
}
751746

752747
if (
753748
$var instanceof PropertyFetch
754749
&& $var->name instanceof Node\Identifier
755750
) {
756-
$type = $type->unionWith($this->create($var->var, new IntersectionType([
751+
$type = $this->create($var->var, new IntersectionType([
757752
new ObjectWithoutClassType(),
758753
new HasPropertyType($var->name->toString()),
759-
]), TypeSpecifierContext::createTruthy(), false, $scope, $rootExpr));
754+
]), TypeSpecifierContext::createTruthy(), false, $scope, $rootExpr);
760755
} elseif (
761756
$var instanceof StaticPropertyFetch
762757
&& $var->class instanceof Expr
763758
&& $var->name instanceof Node\VarLikeIdentifier
764759
) {
765-
$type = $type->unionWith($this->create($var->class, new IntersectionType([
760+
$type = $this->create($var->class, new IntersectionType([
766761
new ObjectWithoutClassType(),
767762
new HasPropertyType($var->name->toString()),
768-
]), TypeSpecifierContext::createTruthy(), false, $scope, $rootExpr));
763+
]), TypeSpecifierContext::createTruthy(), false, $scope, $rootExpr);
769764
}
770765

766+
$type = $type->unionWith(
767+
$this->create($var, new NullType(), TypeSpecifierContext::createFalse(), false, $scope, $rootExpr),
768+
);
769+
771770
if ($types === null) {
772771
$types = $type;
773772
} else {
@@ -792,6 +791,15 @@ public function specifyTypesInCondition(
792791
} elseif (
793792
$expr instanceof Expr\Empty_
794793
) {
794+
if (!$scope instanceof MutatingScope) {
795+
throw new ShouldNotHappenException();
796+
}
797+
798+
$isset = $scope->issetCheck($expr->expr, static fn () => true);
799+
if ($isset === false) {
800+
return new SpecifiedTypes();
801+
}
802+
795803
return $this->specifyTypesInCondition($scope, new BooleanOr(
796804
new Expr\BooleanNot(new Expr\Isset_([$expr->expr])),
797805
new Expr\BooleanNot($expr->expr),

‎tests/PHPStan/Analyser/NodeScopeResolverTest.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1356,6 +1356,9 @@ public function dataFileAsserts(): iterable
13561356
yield from $this->gatherAssertTypes(__DIR__ . '/data/impure-connection-fns.php');
13571357
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-9963.php');
13581358
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-9995.php');
1359+
yield from $this->gatherAssertTypes(__DIR__ . '/data/falsey-isset-certainty.php');
1360+
yield from $this->gatherAssertTypes(__DIR__ . '/data/falsey-empty-certainty.php');
1361+
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-7291.php');
13591362
}
13601363

13611364
/**

‎tests/PHPStan/Analyser/data/bug-7291.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace Bug7291;
4+
5+
use PHPStan\TrinaryLogic;
6+
use function PHPStan\Testing\assertType;
7+
use function PHPStan\Testing\assertVariableCertainty;
8+
9+
class HelloWorld
10+
{
11+
public function doFoo(): void
12+
{
13+
if (rand(0, 1)) {
14+
$a = rand(0, 1) ? new \stdClass() : null;
15+
}
16+
17+
assertType('stdClass|null', $a);
18+
assertVariableCertainty(TrinaryLogic::createMaybe(), $a);
19+
20+
echo $a?->foo;
21+
22+
assertType('stdClass|null', $a);
23+
assertVariableCertainty(TrinaryLogic::createMaybe(), $a);
24+
}
25+
}

‎tests/PHPStan/Analyser/data/falsey-empty-certainty.php‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
<?php
2+
3+
namespace FalseyEmptyCertainty;
4+
5+
use function PHPStan\Testing\assertVariableCertainty;
6+
use PHPStan\TrinaryLogic;
7+
8+
function falseyEmptyArrayDimFetch(): void
9+
{
10+
if (rand() % 2) {
11+
$a = ['bar' => null];
12+
if (rand() % 3) {
13+
$a = ['bar' => 'hello'];
14+
}
15+
}
16+
17+
assertVariableCertainty(TrinaryLogic::createMaybe(), $a);
18+
if (empty($a['bar'])) {
19+
assertVariableCertainty(TrinaryLogic::createMaybe(), $a);
20+
} else {
21+
assertVariableCertainty(TrinaryLogic::createYes(), $a);
22+
}
23+
24+
assertVariableCertainty(TrinaryLogic::createMaybe(), $a);
25+
}
26+
27+
function falseyEmptyUncertainPropertyFetch(): void
28+
{
29+
if (rand() % 2) {
30+
$a = new \stdClass();
31+
if (rand() % 3) {
32+
$a->x = 'hello';
33+
}
34+
}
35+
36+
assertVariableCertainty(TrinaryLogic::createMaybe(), $a);
37+
if (empty($a->x)) {
38+
assertVariableCertainty(TrinaryLogic::createMaybe(), $a);
39+
} else {
40+
assertVariableCertainty(TrinaryLogic::createYes(), $a);
41+
}
42+
43+
assertVariableCertainty(TrinaryLogic::createMaybe(), $a);
44+
}
45+
46+
function justEmpty(): void
47+
{
48+
assertVariableCertainty(TrinaryLogic::createNo(), $foo);
49+
if (!empty($foo)) {
50+
assertVariableCertainty(TrinaryLogic::createNo(), $foo);
51+
} else {
52+
assertVariableCertainty(TrinaryLogic::createNo(), $foo);
53+
}
54+
assertVariableCertainty(TrinaryLogic::createNo(), $foo);
55+
}
56+
57+
function maybeEmpty(): void
58+
{
59+
if (rand() % 2) {
60+
$foo = 1;
61+
}
62+
63+
assertVariableCertainty(TrinaryLogic::createMaybe(), $foo);
64+
if (!empty($foo)) {
65+
assertVariableCertainty(TrinaryLogic::createYes(), $foo);
66+
} else {
67+
assertVariableCertainty(TrinaryLogic::createMaybe(), $foo);
68+
}
69+
}

0 commit comments

Comments
 (0)