Skip to content

Commit

Permalink
Improved isset() and ternary operator handling
Browse files Browse the repository at this point in the history
  • Loading branch information
@staabm
staabm committed Nov 28, 2023
1 parent aec0406 commit f449d98
Show file tree
Hide file tree
Showing 32 changed files with 1,821 additions and 108 deletions.
7 changes: 7 additions & 0 deletions src/Analyser/EnsuredNonNullabilityResultExpression.php
View file
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
View file
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
View file
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

0 comments on commit f449d98

Please sign in to comment.