Skip to content

Commit 3b17acc

Browse files
ondrejmirtesondrejmirtes
committed
Fix resolving type of isset(), empty() and null coalesce operator (??)
1 parent 5cf5b9b commit 3b17acc

14 files changed

+905
-67
lines changed

‎src/Analyser/MutatingScope.php‎

Lines changed: 230 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -602,51 +602,68 @@ private function resolveType(Expr $node): Type
602602
if (
603603
$node instanceof Expr\BinaryOp\Equal
604604
|| $node instanceof Expr\BinaryOp\NotEqual
605-
|| $node instanceof Expr\Empty_
606605
) {
607606
return new BooleanType();
608607
}
609608

610-
if ($node instanceof Expr\Isset_) {
611-
$result = new ConstantBooleanType(true);
612-
foreach ($node->vars as $var) {
613-
if ($var instanceof Expr\ArrayDimFetch && $var->dim !== null) {
614-
$variableType = $this->getType($var->var);
615-
$dimType = $this->getType($var->dim);
616-
$hasOffset = $variableType->hasOffsetValueType($dimType);
617-
$offsetValueType = $variableType->getOffsetValueType($dimType);
618-
$offsetValueIsNotNull = (new NullType())->isSuperTypeOf($offsetValueType)->negate();
619-
$isset = $hasOffset->and($offsetValueIsNotNull)->toBooleanType();
620-
if ($isset instanceof ConstantBooleanType) {
621-
if (!$isset->getValue()) {
622-
return $isset;
623-
}
609+
if ($node instanceof Expr\Empty_) {
610+
$result = $this->issetCheck($node->expr, static function (Type $type): ?bool {
611+
$isNull = (new NullType())->isSuperTypeOf($type);
612+
$isFalsey = (new ConstantBooleanType(false))->isSuperTypeOf($type->toBoolean());
613+
if ($isNull->maybe()) {
614+
return null;
615+
}
616+
if ($isFalsey->maybe()) {
617+
return null;
618+
}
624619

625-
continue;
620+
if ($isNull->yes()) {
621+
if ($isFalsey->yes()) {
622+
return false;
623+
}
624+
if ($isFalsey->no()) {
625+
return true;
626626
}
627627

628-
$result = $isset;
629-
continue;
628+
return false;
630629
}
631630

632-
if ($var instanceof Expr\Variable && is_string($var->name)) {
633-
$variableType = $this->getType($var);
634-
$isNullSuperType = (new NullType())->isSuperTypeOf($variableType);
635-
$has = $this->hasVariableType($var->name);
636-
if ($has->no() || $isNullSuperType->yes()) {
637-
return new ConstantBooleanType(false);
631+
return !$isFalsey->yes();
632+
});
633+
if ($result === null) {
634+
return new BooleanType();
635+
}
636+
637+
return new ConstantBooleanType(!$result);
638+
}
639+
640+
if ($node instanceof Expr\Isset_) {
641+
$issetResult = true;
642+
foreach ($node->vars as $var) {
643+
$result = $this->issetCheck($var, static function (Type $type): ?bool {
644+
$isNull = (new NullType())->isSuperTypeOf($type);
645+
if ($isNull->maybe()) {
646+
return null;
638647
}
639648

640-
if ($has->maybe() || !$isNullSuperType->no()) {
641-
$result = new BooleanType();
649+
return !$isNull->yes();
650+
});
651+
if ($result !== null) {
652+
if (!$result) {
653+
return new ConstantBooleanType($result);
642654
}
655+
643656
continue;
644657
}
645658

659+
$issetResult = $result;
660+
}
661+
662+
if ($issetResult === null) {
646663
return new BooleanType();
647664
}
648665

649-
return $result;
666+
return new ConstantBooleanType($issetResult);
650667
}
651668

652669
if ($node instanceof Node\Expr\BooleanNot) {
@@ -1928,53 +1945,32 @@ private function resolveType(Expr $node): Type
19281945
}
19291946

19301947
if ($node instanceof Expr\BinaryOp\Coalesce) {
1931-
if ($node->left instanceof Expr\ArrayDimFetch && $node->left->dim !== null) {
1932-
$dimType = $this->getType($node->left->dim);
1933-
$varType = $this->getType($node->left->var);
1934-
$hasOffset = $varType->hasOffsetValueType($dimType);
1935-
$leftType = $this->getType($node->left);
1936-
$rightType = $this->filterByFalseyValue(
1937-
new BinaryOp\NotIdentical($node->left, new ConstFetch(new Name('null'))),
1938-
)->getType($node->right);
1939-
if ($hasOffset->no()) {
1940-
return $rightType;
1941-
} elseif ($hasOffset->yes()) {
1942-
$offsetValueType = $varType->getOffsetValueType($dimType);
1943-
if ($offsetValueType->isSuperTypeOf(new NullType())->no()) {
1944-
return TypeCombinator::removeNull($leftType);
1945-
}
1946-
}
1947-
1948-
return TypeCombinator::union(
1949-
TypeCombinator::removeNull($leftType),
1950-
$rightType,
1951-
);
1952-
}
1953-
19541948
$leftType = $this->getType($node->left);
19551949
$rightType = $this->filterByFalseyValue(
19561950
new BinaryOp\NotIdentical($node->left, new ConstFetch(new Name('null'))),
19571951
)->getType($node->right);
1958-
if ($leftType instanceof ErrorType || $leftType instanceof NullType) {
1959-
return $rightType;
1960-
}
19611952

1962-
if (
1963-
TypeCombinator::containsNull($leftType)
1964-
|| $node->left instanceof PropertyFetch
1965-
|| (
1966-
$node->left instanceof Variable
1967-
&& is_string($node->left->name)
1968-
&& !$this->hasVariableType($node->left->name)->yes()
1969-
)
1970-
) {
1953+
$result = $this->issetCheck($node->left, static function (Type $type): ?bool {
1954+
$isNull = (new NullType())->isSuperTypeOf($type);
1955+
if ($isNull->maybe()) {
1956+
return null;
1957+
}
1958+
1959+
return !$isNull->yes();
1960+
});
1961+
1962+
if ($result === null) {
19711963
return TypeCombinator::union(
19721964
TypeCombinator::removeNull($leftType),
19731965
$rightType,
19741966
);
19751967
}
19761968

1977-
return TypeCombinator::removeNull($leftType);
1969+
if ($result) {
1970+
return TypeCombinator::removeNull($leftType);
1971+
}
1972+
1973+
return $rightType;
19781974
}
19791975

19801976
if ($node instanceof ConstFetch) {
@@ -2590,6 +2586,177 @@ private function getNullsafeShortCircuitingType(Expr $expr, Type $type): Type
25902586
return $type;
25912587
}
25922588

2589+
/**
2590+
* @param callable(Type): ?bool $typeCallback
2591+
*/
2592+
private function issetCheck(Expr $expr, callable $typeCallback, ?bool $result = null): ?bool
2593+
{
2594+
// mirrored in PHPStan\Rules\IssetCheck
2595+
if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) {
2596+
$hasVariable = $this->hasVariableType($expr->name);
2597+
if ($hasVariable->maybe()) {
2598+
return null;
2599+
}
2600+
2601+
if ($result === null) {
2602+
if ($hasVariable->yes()) {
2603+
if ($expr->name === '_SESSION') {
2604+
return null;
2605+
}
2606+
2607+
return $typeCallback($this->getVariableType($expr->name));
2608+
}
2609+
2610+
return false;
2611+
}
2612+
2613+
return $result;
2614+
} elseif ($expr instanceof Node\Expr\ArrayDimFetch && $expr->dim !== null) {
2615+
$type = $this->treatPhpDocTypesAsCertain
2616+
? $this->getType($expr->var)
2617+
: $this->getNativeType($expr->var);
2618+
$dimType = $this->treatPhpDocTypesAsCertain
2619+
? $this->getType($expr->dim)
2620+
: $this->getNativeType($expr->dim);
2621+
$hasOffsetValue = $type->hasOffsetValueType($dimType);
2622+
if (!$type->isOffsetAccessible()->yes()) {
2623+
return $result ?? $this->issetCheckUndefined($expr->var);
2624+
}
2625+
2626+
if ($hasOffsetValue->no()) {
2627+
if ($result !== null) {
2628+
return $result;
2629+
}
2630+
2631+
return false;
2632+
}
2633+
2634+
if ($hasOffsetValue->maybe()) {
2635+
return null;
2636+
}
2637+
2638+
// If offset is cannot be null, store this error message and see if one of the earlier offsets is.
2639+
// E.g. $array['a']['b']['c'] ?? null; is a valid coalesce if a OR b or C might be null.
2640+
if ($hasOffsetValue->yes()) {
2641+
if ($result !== null) {
2642+
return $result;
2643+
}
2644+
2645+
$result = $typeCallback($type->getOffsetValueType($dimType));
2646+
2647+
if ($result !== null) {
2648+
return $this->issetCheck($expr->var, $typeCallback, $result);
2649+
}
2650+
}
2651+
2652+
// Has offset, it is nullable
2653+
return null;
2654+
2655+
} elseif ($expr instanceof Node\Expr\PropertyFetch || $expr instanceof Node\Expr\StaticPropertyFetch) {
2656+
2657+
$propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($expr, $this);
2658+
2659+
if ($propertyReflection === null) {
2660+
if ($expr instanceof Node\Expr\PropertyFetch) {
2661+
return $this->issetCheckUndefined($expr->var);
2662+
}
2663+
2664+
if ($expr->class instanceof Expr) {
2665+
return $this->issetCheckUndefined($expr->class);
2666+
}
2667+
2668+
return null;
2669+
}
2670+
2671+
if (!$propertyReflection->isNative()) {
2672+
if ($expr instanceof Node\Expr\PropertyFetch) {
2673+
return $this->issetCheckUndefined($expr->var);
2674+
}
2675+
2676+
if ($expr->class instanceof Expr) {
2677+
return $this->issetCheckUndefined($expr->class);
2678+
}
2679+
2680+
return null;
2681+
}
2682+
2683+
$nativeType = $propertyReflection->getNativeType();
2684+
if (!$nativeType instanceof MixedType) {
2685+
if (!$this->isSpecified($expr)) {
2686+
if ($expr instanceof Node\Expr\PropertyFetch) {
2687+
return $this->issetCheckUndefined($expr->var);
2688+
}
2689+
2690+
if ($expr->class instanceof Expr) {
2691+
return $this->issetCheckUndefined($expr->class);
2692+
}
2693+
2694+
return null;
2695+
}
2696+
}
2697+
2698+
if ($result !== null) {
2699+
return $result;
2700+
}
2701+
2702+
$result = $typeCallback($propertyReflection->getWritableType());
2703+
if ($result !== null) {
2704+
if ($expr instanceof Node\Expr\PropertyFetch) {
2705+
return $this->issetCheck($expr->var, $typeCallback, $result);
2706+
}
2707+
2708+
if ($expr->class instanceof Expr) {
2709+
return $this->issetCheck($expr->class, $typeCallback, $result);
2710+
}
2711+
}
2712+
2713+
return $result;
2714+
}
2715+
2716+
if ($result !== null) {
2717+
return $result;
2718+
}
2719+
2720+
return $typeCallback($this->getType($expr));
2721+
}
2722+
2723+
private function issetCheckUndefined(Expr $expr): ?bool
2724+
{
2725+
if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) {
2726+
$hasVariable = $this->hasVariableType($expr->name);
2727+
if (!$hasVariable->no()) {
2728+
return null;
2729+
}
2730+
2731+
return false;
2732+
}
2733+
2734+
if ($expr instanceof Node\Expr\ArrayDimFetch && $expr->dim !== null) {
2735+
$type = $this->getType($expr->var);
2736+
$dimType = $this->getType($expr->dim);
2737+
$hasOffsetValue = $type->hasOffsetValueType($dimType);
2738+
if (!$type->isOffsetAccessible()->yes()) {
2739+
return $this->issetCheckUndefined($expr->var);
2740+
}
2741+
2742+
if (!$hasOffsetValue->no()) {
2743+
return $this->issetCheckUndefined($expr->var);
2744+
}
2745+
2746+
return false;
2747+
}
2748+
2749+
if ($expr instanceof Expr\PropertyFetch) {
2750+
return $this->issetCheckUndefined($expr->var);
2751+
}
2752+
2753+
if ($expr instanceof Expr\StaticPropertyFetch && $expr->class instanceof Expr) {
2754+
return $this->issetCheckUndefined($expr->class);
2755+
}
2756+
2757+
return null;
2758+
}
2759+
25932760
/**
25942761
* @param ParametersAcceptor[] $variants
25952762
*/

‎src/Rules/IssetCheck.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ public function __construct(
3030
*/
3131
public function check(Expr $expr, Scope $scope, string $operatorDescription, callable $typeMessageCallback, ?RuleError $error = null): ?RuleError
3232
{
33+
// mirrored in PHPStan\Analyser\MutatingScope::issetCheck()
3334
if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) {
3435
$hasVariable = $scope->hasVariableType($expr->name);
3536
if ($hasVariable->maybe()) {

‎tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2606,11 +2606,11 @@ public function dataBinaryOperations(): array
26062606
'!isset($foo)',
26072607
],
26082608
[
2609-
'bool',
2609+
'false',
26102610
'empty($foo)',
26112611
],
26122612
[
2613-
'bool',
2613+
'true',
26142614
'!empty($foo)',
26152615
],
26162616
[

‎tests/PHPStan/Analyser/NodeScopeResolverTest.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -650,7 +650,21 @@ public function dataFileAsserts(): iterable
650650
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-4357.php');
651651
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-5817.php');
652652
yield from $this->gatherAssertTypes(__DIR__ . '/data/array-column.php');
653+
654+
if (PHP_VERSION_ID >= 70400) {
655+
yield from $this->gatherAssertTypes(__DIR__ . '/data/isset-coalesce-empty-type.php');
656+
yield from $this->gatherAssertTypes(__DIR__ . '/data/isset-coalesce-empty-type-root.php');
657+
}
658+
659+
if (PHP_VERSION_ID < 80100) {
660+
yield from $this->gatherAssertTypes(__DIR__ . '/data/isset-coalesce-empty-type-pre-81.php');
661+
} else {
662+
yield from $this->gatherAssertTypes(__DIR__ . '/data/isset-coalesce-empty-type-post-81.php');
663+
}
664+
653665
yield from $this->gatherAssertTypes(__DIR__ . '/data/template-null-bound.php');
666+
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-4592.php');
667+
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-4903.php');
654668
}
655669

656670
/**

0 commit comments

Comments
 (0)