Skip to content
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.

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

Fix !isset() with Variable #2710

Merged
merged 37 commits into from
Nov 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
1ac5e19
Regression test
ondrejmirtes Nov 3, 2023
c75d892
Fix variable certainty for ArrayDimFetch in falsey isset context
staabm Oct 30, 2023
28592a9
More precise falsey `isset()`
staabm Nov 2, 2023
f3106f3
Rewrite multi-arg `isset()` to BooleanAnd chain
staabm Nov 2, 2023
ccb4f63
Improve dependent type inference in truthy BooleanOr and falsy Boolea…
ondrejmirtes Nov 3, 2023
fe30fd5
Fix build
ondrejmirtes Nov 3, 2023
96c909a
Fix conditional expressions when assigning a ternary operator
ondrejmirtes Nov 4, 2023
41357e5
Make it work for short ternaries too
ondrejmirtes Nov 4, 2023
04d44ab
Fix conditional expressions logic
ondrejmirtes Nov 4, 2023
b9acee8
Fix !isset() with Variable
staabm Nov 2, 2023
442f047
more tests
staabm Nov 2, 2023
2046db0
mixed tests
staabm Nov 2, 2023
f99e6d3
simplify?
staabm Nov 2, 2023
850eb15
more tests
staabm Nov 3, 2023
3809fc9
refactor
staabm Nov 3, 2023
cd1fc73
remove unnecessary unset()
staabm Nov 3, 2023
762e6a6
More tests
ondrejmirtes Nov 4, 2023
a9cab71
more tests, fixes
staabm Nov 3, 2023
e497572
added drupal test
staabm Nov 5, 2023
f86dbe3
Revert "added drupal test"
staabm Nov 5, 2023
20be372
relax test
staabm Nov 5, 2023
dd9a745
added failling coalesce test
staabm Nov 5, 2023
77389f0
experimental coalesce fix
staabm Nov 10, 2023
7be4cff
added ternary certainty tests
staabm Nov 19, 2023
9c082f0
test isset()/empty() with unset()
staabm Nov 19, 2023
91f2532
restore certainty in ternary isset()
staabm Nov 20, 2023
d0dede2
fix conditional types after ternary, when types overlap
staabm Nov 20, 2023
5e8c3b8
more reverse ternary tests
staabm Nov 20, 2023
20e5054
assert more types
staabm Nov 20, 2023
a01cb4c
simplify
staabm Nov 24, 2023
a917377
Fix certainty of by-ref variables in ternary condition
staabm Nov 27, 2023
7bcda04
lazier type resolving
staabm Nov 27, 2023
4bab4bb
Fix conditional types
staabm Nov 27, 2023
b9948f5
Fix conditional bug
ondrejmirtes Nov 28, 2023
32cbe96
added regression test for conditional bug
staabm Nov 28, 2023
6e72a32
Restore original Ternary handling
ondrejmirtes Nov 28, 2023
e718636
Fix ternary scope merging for always-true and always-false condition
ondrejmirtes Nov 28, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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;
}

}
85 changes: 70 additions & 15 deletions src/Analyser/MutatingScope.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
use PHPStan\Node\Expr\PropertyInitializationExpr;
use PHPStan\Node\Expr\SetOffsetValueTypeExpr;
use PHPStan\Node\Expr\TypeExpr;
use PHPStan\Node\IssetExpr;
use PHPStan\Node\Printer\ExprPrinter;
use PHPStan\Parser\ArrayMapArgVisitor;
use PHPStan\Parser\NewAssignedToPropertyVisitor;
Expand Down Expand Up @@ -2348,6 +2349,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 @@ -3440,7 +3445,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 @@ -3474,23 +3479,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 Expand Up @@ -3707,6 +3720,23 @@ private function invalidateMethodsOnExpression(Expr $expressionToInvalidate): se
);
}

private function setExpressionCertainty(Expr $expr, TrinaryLogic $certainty): self
{
if ($this->hasExpressionType($expr)->no()) {
throw new ShouldNotHappenException();
}

$originalExprType = $this->getType($expr);
$nativeType = $this->getNativeType($expr);

return $this->specifyExpressionType(
$expr,
$originalExprType,
$nativeType,
$certainty,
);
}

private function addTypeToExpression(Expr $expr, Type $type): self
{
$originalExprType = $this->getType($expr);
Expand Down Expand Up @@ -3816,6 +3846,23 @@ public function filterBySpecifiedTypes(SpecifiedTypes $specifiedTypes): self
foreach ($typeSpecifications as $typeSpecification) {
$expr = $typeSpecification['expr'];
$type = $typeSpecification['type'];

if ($expr instanceof IssetExpr) {
$issetExpr = $expr;
$expr = $issetExpr->getExpr();

if ($typeSpecification['sure']) {
$scope = $scope->setExpressionCertainty(
$expr,
TrinaryLogic::createMaybe(),
);
} else {
$scope = $scope->unsetExpression($expr);
}

continue;
}

if ($typeSpecification['sure']) {
if ($specifiedTypes->shouldOverwrite()) {
$scope = $scope->assignExpression($expr, $type, $type);
Expand All @@ -3828,6 +3875,7 @@ public function filterBySpecifiedTypes(SpecifiedTypes $specifiedTypes): self
$specifiedExpressions[$this->getNodeKey($expr)] = ExpressionTypeHolder::createYes($expr, $scope->getType($expr));
}

$conditions = [];
foreach ($scope->conditionalExpressions as $conditionalExprString => $conditionalExpressions) {
foreach ($conditionalExpressions as $conditionalExpression) {
foreach ($conditionalExpression->getConditionExpressionTypeHolders() as $holderExprString => $conditionalTypeHolder) {
Expand All @@ -3836,18 +3884,25 @@ public function filterBySpecifiedTypes(SpecifiedTypes $specifiedTypes): self
}
}

if ($conditionalExpression->getTypeHolder()->getCertainty()->no()) {
unset($scope->expressionTypes[$conditionalExprString]);
} else {
$scope->expressionTypes[$conditionalExprString] = array_key_exists($conditionalExprString, $scope->expressionTypes)
? new ExpressionTypeHolder(
$scope->expressionTypes[$conditionalExprString]->getExpr(),
TypeCombinator::intersect($scope->expressionTypes[$conditionalExprString]->getType(), $conditionalExpression->getTypeHolder()->getType()),
TrinaryLogic::maxMin($scope->expressionTypes[$conditionalExprString]->getCertainty(), $conditionalExpression->getTypeHolder()->getCertainty()),
)
: $conditionalExpression->getTypeHolder();
$specifiedExpressions[$conditionalExprString] = $conditionalExpression->getTypeHolder();
}
$conditions[$conditionalExprString][] = $conditionalExpression;
$specifiedExpressions[$conditionalExprString] = $conditionalExpression->getTypeHolder();
}
}

foreach ($conditions as $conditionalExprString => $expressions) {
$certainty = TrinaryLogic::lazyExtremeIdentity($expressions, static fn (ConditionalExpressionHolder $holder) => $holder->getTypeHolder()->getCertainty());
if ($certainty->no()) {
unset($scope->expressionTypes[$conditionalExprString]);
} else {
$type = TypeCombinator::intersect(...array_map(static fn (ConditionalExpressionHolder $holder) => $holder->getTypeHolder()->getType(), $expressions));

$scope->expressionTypes[$conditionalExprString] = array_key_exists($conditionalExprString, $scope->expressionTypes)
? new ExpressionTypeHolder(
$scope->expressionTypes[$conditionalExprString]->getExpr(),
TypeCombinator::intersect($scope->expressionTypes[$conditionalExprString]->getType(), $type),
TrinaryLogic::maxMin($scope->expressionTypes[$conditionalExprString]->getCertainty(), $certainty),
)
: $expressions[0]->getTypeHolder();
}
}

Expand Down
64 changes: 52 additions & 12 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -1759,14 +1759,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 @@ -1782,7 +1790,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 @@ -1812,6 +1820,7 @@ private function revertNonNullability(MutatingScope $scope, array $specifiedExpr
$specifiedExpressionResult->getExpression(),
$specifiedExpressionResult->getOriginalType(),
$specifiedExpressionResult->getOriginalNativeType(),
$specifiedExpressionResult->getCertainty(),
);
}

Expand Down Expand Up @@ -2568,7 +2577,7 @@ static function (): void {
$scope = $this->revertNonNullability($condResult->getScope(), $nonNullabilityResult->getSpecifiedExpressions());
$scope = $this->lookForUnsetAllowedUndefinedExpressions($scope, $expr->left);

$rightScope = $scope->filterByFalseyValue(new Expr\Isset_([$expr->left]));
$rightScope = $scope->filterByFalseyValue($expr);
$rightResult = $this->processExprNode($expr->right, $rightScope, $nodeCallback, $context->enterDeep());
$rightExprType = $scope->getType($expr->right);
if ($rightExprType instanceof NeverType && $rightExprType->isExplicit()) {
Expand Down Expand Up @@ -2787,15 +2796,22 @@ static function (Node $node, Scope $scope) use ($nodeCallback): void {
$throwPoints = array_merge($throwPoints, $elseResult->getThrowPoints());
$ifFalseScope = $elseResult->getScope();

if ($ifTrueType instanceof NeverType && $ifTrueType->isExplicit()) {
$condType = $scope->getType($expr->cond);
if ($condType->isTrue()->yes()) {
$finalScope = $ifTrueScope;
} elseif ($condType->isFalse()->yes()) {
$finalScope = $ifFalseScope;
} else {
$ifFalseType = $ifFalseScope->getType($expr->else);

if ($ifFalseType instanceof NeverType && $ifFalseType->isExplicit()) {
$finalScope = $ifTrueScope;
if ($ifTrueType instanceof NeverType && $ifTrueType->isExplicit()) {
$finalScope = $ifFalseScope;
} else {
$finalScope = $ifTrueScope->mergeWith($ifFalseScope);
$ifFalseType = $ifFalseScope->getType($expr->else);

if ($ifFalseType instanceof NeverType && $ifFalseType->isExplicit()) {
$finalScope = $ifTrueScope;
} else {
$finalScope = $ifTrueScope->mergeWith($ifFalseScope);
}
}
}

Expand Down Expand Up @@ -3751,10 +3767,35 @@ private function processAssignVar(
$throwPoints = $result->getThrowPoints();
$assignedExpr = $this->unwrapAssign($assignedExpr);
$type = $scope->getType($assignedExpr);
$truthySpecifiedTypes = $this->typeSpecifier->specifyTypesInCondition($scope, $assignedExpr, TypeSpecifierContext::createTruthy());
$falseySpecifiedTypes = $this->typeSpecifier->specifyTypesInCondition($scope, $assignedExpr, TypeSpecifierContext::createFalsey());

$conditionalExpressions = [];
if ($assignedExpr instanceof Ternary) {
$if = $assignedExpr->if;
if ($if === null) {
$if = $assignedExpr->cond;
}
$condScope = $this->processExprNode($assignedExpr->cond, $scope, static function (): void {
}, ExpressionContext::createDeep())->getScope();
$truthySpecifiedTypes = $this->typeSpecifier->specifyTypesInCondition($condScope, $assignedExpr->cond, TypeSpecifierContext::createTruthy());
$falseySpecifiedTypes = $this->typeSpecifier->specifyTypesInCondition($condScope, $assignedExpr->cond, TypeSpecifierContext::createFalsey());
$truthyScope = $condScope->filterBySpecifiedTypes($truthySpecifiedTypes);
$falsyScope = $condScope->filterBySpecifiedTypes($falseySpecifiedTypes);
$truthyType = $truthyScope->getType($if);
$falseyType = $falsyScope->getType($assignedExpr->else);

if (
$truthyType->isSuperTypeOf($falseyType)->no()
&& $falseyType->isSuperTypeOf($truthyType)->no()
) {
$conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $truthySpecifiedTypes, $truthyType);
$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $truthySpecifiedTypes, $truthyType);
$conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType);
$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType);
}
}

$truthySpecifiedTypes = $this->typeSpecifier->specifyTypesInCondition($scope, $assignedExpr, TypeSpecifierContext::createTruthy());
$falseySpecifiedTypes = $this->typeSpecifier->specifyTypesInCondition($scope, $assignedExpr, TypeSpecifierContext::createFalsey());

$truthyType = TypeCombinator::removeFalsey($type);
$falseyType = TypeCombinator::intersect($type, StaticTypeFactory::falsey());
Expand All @@ -3764,7 +3805,6 @@ private function processAssignVar(
$conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType);
$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType);

// TODO conditional expressions for native type should be handled too
$scope = $result->getScope()->assignVariable($var->name, $type, $scope->getNativeType($assignedExpr));
foreach ($conditionalExpressions as $exprString => $holders) {
$scope = $scope->addConditionalExpressions($exprString, $holders);
Expand Down