Skip to content

Commit

Permalink
Fix variable certainty for ArrayDimFetch in falsey isset context
Browse files Browse the repository at this point in the history
  • Loading branch information
staabm authored Oct 30, 2023
1 parent 1308c52 commit 3de3c85
Show file tree
Hide file tree
Showing 11 changed files with 379 additions and 17 deletions.
7 changes: 7 additions & 0 deletions src/Analyser/EnsuredNonNullabilityResultExpression.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace PHPStan\Analyser;

use PhpParser\Node\Expr;
use PHPStan\TrinaryLogic;
use PHPStan\Type\Type;

class EnsuredNonNullabilityResultExpression
Expand All @@ -12,6 +13,7 @@ public function __construct(
private Expr $expression,
private Type $originalType,
private Type $originalNativeType,
private TrinaryLogic $certainty,
)
{
}
Expand All @@ -31,4 +33,9 @@ public function getOriginalNativeType(): Type
return $this->originalNativeType;
}

public function getCertainty(): TrinaryLogic
{
return $this->certainty;
}

}
18 changes: 15 additions & 3 deletions src/Analyser/MutatingScope.php
Original file line number Diff line number Diff line change
Expand Up @@ -2340,6 +2340,10 @@ public function isSpecified(Expr $node): bool
/** @api */
public function hasExpressionType(Expr $node): TrinaryLogic
{
if ($node instanceof Variable && is_string($node->name)) {
return $this->hasVariableType($node->name);
}

$exprString = $this->getNodeKey($node);
if (!isset($this->expressionTypes[$exprString])) {
return TrinaryLogic::createNo();
Expand Down Expand Up @@ -3432,7 +3436,7 @@ public function unsetExpression(Expr $expr): self
return $scope->invalidateExpression($expr);
}

public function specifyExpressionType(Expr $expr, Type $type, Type $nativeType): self
public function specifyExpressionType(Expr $expr, Type $type, Type $nativeType, ?TrinaryLogic $certainty = null): self
{
if ($expr instanceof ConstFetch) {
$loweredConstName = strtolower($expr->name->toString());
Expand Down Expand Up @@ -3466,23 +3470,31 @@ public function specifyExpressionType(Expr $expr, Type $type, Type $nativeType):
if ($dimType instanceof ConstantIntegerType) {
$types[] = new StringType();
}

$scope = $scope->specifyExpressionType(
$expr->var,
TypeCombinator::intersect(
TypeCombinator::intersect($exprVarType, TypeCombinator::union(...$types)),
new HasOffsetValueType($dimType, $type),
),
$scope->getNativeType($expr->var),
$certainty,
);
}
}
}

if ($certainty === null) {
$certainty = TrinaryLogic::createYes();
} elseif ($certainty->no()) {
throw new ShouldNotHappenException();
}

$exprString = $this->getNodeKey($expr);
$expressionTypes = $scope->expressionTypes;
$expressionTypes[$exprString] = ExpressionTypeHolder::createYes($expr, $type);
$expressionTypes[$exprString] = new ExpressionTypeHolder($expr, $type, $certainty);
$nativeTypes = $scope->nativeExpressionTypes;
$nativeTypes[$exprString] = ExpressionTypeHolder::createYes($expr, $nativeType);
$nativeTypes[$exprString] = new ExpressionTypeHolder($expr, $nativeType, $certainty);

return $this->scopeFactory->create(
$this->context,
Expand Down
13 changes: 11 additions & 2 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -1753,14 +1753,22 @@ private function ensureShallowNonNullability(MutatingScope $scope, Scope $origin
if ($isNull->yes()) {
return new EnsuredNonNullabilityResult($scope, []);
}

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

$exprTypeWithoutNull = TypeCombinator::removeNull($exprType);
if ($exprType->equals($exprTypeWithoutNull)) {
$originalExprType = $originalScope->getType($exprToSpecify);
if (!$originalExprType->equals($exprTypeWithoutNull)) {
$originalNativeType = $originalScope->getNativeType($exprToSpecify);

return new EnsuredNonNullabilityResult($scope, [
new EnsuredNonNullabilityResultExpression($exprToSpecify, $originalExprType, $originalNativeType),
new EnsuredNonNullabilityResultExpression($exprToSpecify, $originalExprType, $originalNativeType, $certainty),
]);
}
return new EnsuredNonNullabilityResult($scope, []);
Expand All @@ -1776,7 +1784,7 @@ private function ensureShallowNonNullability(MutatingScope $scope, Scope $origin
return new EnsuredNonNullabilityResult(
$scope,
[
new EnsuredNonNullabilityResultExpression($exprToSpecify, $exprType, $nativeType),
new EnsuredNonNullabilityResultExpression($exprToSpecify, $exprType, $nativeType, $certainty),
],
);
}
Expand Down Expand Up @@ -1806,6 +1814,7 @@ private function revertNonNullability(MutatingScope $scope, array $specifiedExpr
$specifiedExpressionResult->getExpression(),
$specifiedExpressionResult->getOriginalType(),
$specifiedExpressionResult->getOriginalNativeType(),
$specifiedExpressionResult->getCertainty(),
);
}

Expand Down
32 changes: 20 additions & 12 deletions src/Analyser/TypeSpecifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -717,11 +717,14 @@ public function specifyTypesInCondition(

$types = null;
foreach ($vars as $var) {
$type = new SpecifiedTypes();

if ($var instanceof Expr\Variable && is_string($var->name)) {
if ($scope->hasVariableType($var->name)->no()) {
return new SpecifiedTypes([], [], false, [], $rootExpr);
}
}

if (
$var instanceof ArrayDimFetch
&& $var->dim !== null
Expand All @@ -738,36 +741,32 @@ public function specifyTypesInCondition(
$scope,
$rootExpr,
);
} else {
$type = new SpecifiedTypes();
}

$type = $type->unionWith(
$this->create($var, new NullType(), TypeSpecifierContext::createFalse(), false, $scope, $rootExpr),
);
} else {
$type = $this->create($var, new NullType(), TypeSpecifierContext::createFalse(), false, $scope, $rootExpr);
}

if (
$var instanceof PropertyFetch
&& $var->name instanceof Node\Identifier
) {
$type = $type->unionWith($this->create($var->var, new IntersectionType([
$type = $this->create($var->var, new IntersectionType([
new ObjectWithoutClassType(),
new HasPropertyType($var->name->toString()),
]), TypeSpecifierContext::createTruthy(), false, $scope, $rootExpr));
]), TypeSpecifierContext::createTruthy(), false, $scope, $rootExpr);
} elseif (
$var instanceof StaticPropertyFetch
&& $var->class instanceof Expr
&& $var->name instanceof Node\VarLikeIdentifier
) {
$type = $type->unionWith($this->create($var->class, new IntersectionType([
$type = $this->create($var->class, new IntersectionType([
new ObjectWithoutClassType(),
new HasPropertyType($var->name->toString()),
]), TypeSpecifierContext::createTruthy(), false, $scope, $rootExpr));
]), TypeSpecifierContext::createTruthy(), false, $scope, $rootExpr);
}

$type = $type->unionWith(
$this->create($var, new NullType(), TypeSpecifierContext::createFalse(), false, $scope, $rootExpr),
);

if ($types === null) {
$types = $type;
} else {
Expand All @@ -792,6 +791,15 @@ public function specifyTypesInCondition(
} elseif (
$expr instanceof Expr\Empty_
) {
if (!$scope instanceof MutatingScope) {
throw new ShouldNotHappenException();
}

$isset = $scope->issetCheck($expr->expr, static fn () => true);
if ($isset === false) {
return new SpecifiedTypes();
}

return $this->specifyTypesInCondition($scope, new BooleanOr(
new Expr\BooleanNot(new Expr\Isset_([$expr->expr])),
new Expr\BooleanNot($expr->expr),
Expand Down
3 changes: 3 additions & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1356,6 +1356,9 @@ public function dataFileAsserts(): iterable
yield from $this->gatherAssertTypes(__DIR__ . '/data/impure-connection-fns.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-9963.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-9995.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/falsey-isset-certainty.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/falsey-empty-certainty.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-7291.php');
}

/**
Expand Down
25 changes: 25 additions & 0 deletions tests/PHPStan/Analyser/data/bug-7291.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php declare(strict_types=1);

namespace Bug7291;

use PHPStan\TrinaryLogic;
use function PHPStan\Testing\assertType;
use function PHPStan\Testing\assertVariableCertainty;

class HelloWorld
{
public function doFoo(): void
{
if (rand(0, 1)) {
$a = rand(0, 1) ? new \stdClass() : null;
}

assertType('stdClass|null', $a);
assertVariableCertainty(TrinaryLogic::createMaybe(), $a);

echo $a?->foo;

assertType('stdClass|null', $a);
assertVariableCertainty(TrinaryLogic::createMaybe(), $a);
}
}
69 changes: 69 additions & 0 deletions tests/PHPStan/Analyser/data/falsey-empty-certainty.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
<?php

namespace FalseyEmptyCertainty;

use function PHPStan\Testing\assertVariableCertainty;
use PHPStan\TrinaryLogic;

function falseyEmptyArrayDimFetch(): void
{
if (rand() % 2) {
$a = ['bar' => null];
if (rand() % 3) {
$a = ['bar' => 'hello'];
}
}

assertVariableCertainty(TrinaryLogic::createMaybe(), $a);
if (empty($a['bar'])) {
assertVariableCertainty(TrinaryLogic::createMaybe(), $a);
} else {
assertVariableCertainty(TrinaryLogic::createYes(), $a);
}

assertVariableCertainty(TrinaryLogic::createMaybe(), $a);
}

function falseyEmptyUncertainPropertyFetch(): void
{
if (rand() % 2) {
$a = new \stdClass();
if (rand() % 3) {
$a->x = 'hello';
}
}

assertVariableCertainty(TrinaryLogic::createMaybe(), $a);
if (empty($a->x)) {
assertVariableCertainty(TrinaryLogic::createMaybe(), $a);
} else {
assertVariableCertainty(TrinaryLogic::createYes(), $a);
}

assertVariableCertainty(TrinaryLogic::createMaybe(), $a);
}

function justEmpty(): void
{
assertVariableCertainty(TrinaryLogic::createNo(), $foo);
if (!empty($foo)) {
assertVariableCertainty(TrinaryLogic::createNo(), $foo);
} else {
assertVariableCertainty(TrinaryLogic::createNo(), $foo);
}
assertVariableCertainty(TrinaryLogic::createNo(), $foo);
}

function maybeEmpty(): void
{
if (rand() % 2) {
$foo = 1;
}

assertVariableCertainty(TrinaryLogic::createMaybe(), $foo);
if (!empty($foo)) {
assertVariableCertainty(TrinaryLogic::createYes(), $foo);
} else {
assertVariableCertainty(TrinaryLogic::createMaybe(), $foo);
}
}
Loading

0 comments on commit 3de3c85

Please sign in to comment.