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

Narrow variable type in switch cases #3789

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@
use function array_merge;
use function array_pop;
use function array_reverse;
use function array_shift;
use function array_slice;
use function array_values;
use function base64_decode;
Expand Down Expand Up @@ -1513,9 +1514,11 @@ private function processStmtNode(
$exitPointsForOuterLoop = [];
$throwPoints = $condResult->getThrowPoints();
$impurePoints = $condResult->getImpurePoints();
$defaultCondExprs = [];
foreach ($stmt->cases as $caseNode) {
if ($caseNode->cond !== null) {
$condExpr = new BinaryOp\Equal($stmt->cond, $caseNode->cond);
$defaultCondExprs[] = new BinaryOp\NotEqual($stmt->cond, $caseNode->cond);
$caseResult = $this->processExprNode($stmt, $caseNode->cond, $scopeForBranches, $nodeCallback, ExpressionContext::createDeep());
$scopeForBranches = $caseResult->getScope();
$hasYield = $hasYield || $caseResult->hasYield();
Expand All @@ -1525,6 +1528,11 @@ private function processStmtNode(
} else {
$hasDefaultCase = true;
$branchScope = $scopeForBranches;
$defaultConditions = $this->createBooleanAndFromExpressions($defaultCondExprs);
if ($defaultConditions !== null) {
$branchScope = $this->processExprNode($stmt, $defaultConditions, $scope, static function (): void {
}, ExpressionContext::createDeep())->getTruthyScope()->filterByTruthyValue($defaultConditions);
}
}

$branchScope = $branchScope->mergeWith($prevScope);
Expand Down Expand Up @@ -6570,6 +6578,29 @@ private function getPhpDocReturnType(ResolvedPhpDocBlock $resolvedPhpDoc, Type $
return null;
}

/**
* @param list<Expr> $expressions
*/
private function createBooleanAndFromExpressions(array $expressions): ?Expr
{
if (count($expressions) === 0) {
return null;
}

if (count($expressions) === 1) {
return $expressions[0];
}

$left = array_shift($expressions);
$right = $this->createBooleanAndFromExpressions($expressions);

if ($right === null) {
throw new ShouldNotHappenException();
}

return new BooleanAnd($left, $right);
}

/**
* @param array<Node> $nodes
* @return list<Node\Stmt>
Expand Down
81 changes: 64 additions & 17 deletions src/Analyser/TypeSpecifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -1572,15 +1572,8 @@ private function findTypeExpressionsFromBinaryOperation(Scope $scope, Node\Expr\
$leftType = $scope->getType($binaryOperation->left);
$rightType = $scope->getType($binaryOperation->right);

$rightExpr = $binaryOperation->right;
if ($rightExpr instanceof AlwaysRememberedExpr) {
$rightExpr = $rightExpr->getExpr();
}

$leftExpr = $binaryOperation->left;
if ($leftExpr instanceof AlwaysRememberedExpr) {
$leftExpr = $leftExpr->getExpr();
}
$rightExpr = $this->extractExpression($binaryOperation->right);
$leftExpr = $this->extractExpression($binaryOperation->left);

if (
$leftType instanceof ConstantScalarType
Expand All @@ -1599,6 +1592,39 @@ private function findTypeExpressionsFromBinaryOperation(Scope $scope, Node\Expr\
return null;
}

/**
* @return array{Expr, Type, Type}|null
*/
private function findEnumTypeExpressionsFromBinaryOperation(Scope $scope, Node\Expr\BinaryOp $binaryOperation): ?array
{
$leftType = $scope->getType($binaryOperation->left);
$rightType = $scope->getType($binaryOperation->right);

$rightExpr = $this->extractExpression($binaryOperation->right);
$leftExpr = $this->extractExpression($binaryOperation->left);

if (
$leftType->getEnumCases() === [$leftType]
&& !$rightExpr instanceof ConstFetch
&& !$rightExpr instanceof ClassConstFetch
) {
return [$binaryOperation->right, $leftType, $rightType];
} elseif (
$rightType->getEnumCases() === [$rightType]
&& !$leftExpr instanceof ConstFetch
&& !$leftExpr instanceof ClassConstFetch
) {
return [$binaryOperation->left, $rightType, $leftType];
}

return null;
}

private function extractExpression(Expr $expr): Expr
{
return $expr instanceof AlwaysRememberedExpr ? $expr->getExpr() : $expr;
}

/** @api */
public function create(
Expr $expr,
Expand Down Expand Up @@ -1894,10 +1920,10 @@ public function resolveEqual(Expr\BinaryOp\Equal $expr, Scope $scope, TypeSpecif
$expressions = $this->findTypeExpressionsFromBinaryOperation($scope, $expr);
if ($expressions !== null) {
$exprNode = $expressions[0];
$constantType = $expressions[1];
$enumCaseObjectType = $expressions[1];
$otherType = $expressions[2];

if (!$context->null() && $constantType->getValue() === null) {
if (!$context->null() && $enumCaseObjectType->getValue() === null) {
$trueTypes = [
new NullType(),
new ConstantBooleanType(false),
Expand All @@ -1909,23 +1935,23 @@ public function resolveEqual(Expr\BinaryOp\Equal $expr, Scope $scope, TypeSpecif
return $this->create($exprNode, new UnionType($trueTypes), $context, $scope)->setRootExpr($expr);
}

if (!$context->null() && $constantType->getValue() === false) {
if (!$context->null() && $enumCaseObjectType->getValue() === false) {
return $this->specifyTypesInCondition(
$scope,
$exprNode,
$context->true() ? TypeSpecifierContext::createFalsey() : TypeSpecifierContext::createFalsey()->negate(),
)->setRootExpr($expr);
}

if (!$context->null() && $constantType->getValue() === true) {
if (!$context->null() && $enumCaseObjectType->getValue() === true) {
return $this->specifyTypesInCondition(
$scope,
$exprNode,
$context->true() ? TypeSpecifierContext::createTruthy() : TypeSpecifierContext::createTruthy()->negate(),
)->setRootExpr($expr);
}

if (!$context->null() && $constantType->getValue() === 0 && !$otherType->isInteger()->yes() && !$otherType->isBoolean()->yes()) {
if (!$context->null() && $enumCaseObjectType->getValue() === 0 && !$otherType->isInteger()->yes() && !$otherType->isBoolean()->yes()) {
/* There is a difference between php 7.x and 8.x on the equality
* behavior between zero and the empty string, so to be conservative
* we leave it untouched regardless of the language version */
Expand All @@ -1949,7 +1975,7 @@ public function resolveEqual(Expr\BinaryOp\Equal $expr, Scope $scope, TypeSpecif
return $this->create($exprNode, new UnionType($trueTypes), $context, $scope)->setRootExpr($expr);
}

if (!$context->null() && $constantType->getValue() === '') {
if (!$context->null() && $enumCaseObjectType->getValue() === '') {
/* There is a difference between php 7.x and 8.x on the equality
* behavior between zero and the empty string, so to be conservative
* we leave it untouched regardless of the language version */
Expand All @@ -1976,7 +2002,7 @@ public function resolveEqual(Expr\BinaryOp\Equal $expr, Scope $scope, TypeSpecif
&& $exprNode->name instanceof Name
&& in_array(strtolower($exprNode->name->toString()), ['gettype', 'get_class', 'get_debug_type'], true)
&& isset($exprNode->getArgs()[0])
&& $constantType->isString()->yes()
&& $enumCaseObjectType->isString()->yes()
) {
return $this->specifyTypesInCondition($scope, new Expr\BinaryOp\Identical($expr->left, $expr->right), $context)->setRootExpr($expr);
}
Expand All @@ -1986,10 +2012,31 @@ public function resolveEqual(Expr\BinaryOp\Equal $expr, Scope $scope, TypeSpecif
&& $exprNode instanceof FuncCall
&& $exprNode->name instanceof Name
&& $exprNode->name->toLowerString() === 'preg_match'
&& (new ConstantIntegerType(1))->isSuperTypeOf($constantType)->yes()
&& (new ConstantIntegerType(1))->isSuperTypeOf($enumCaseObjectType)->yes()
) {
return $this->specifyTypesInCondition($scope, new Expr\BinaryOp\Identical($expr->left, $expr->right), $context)->setRootExpr($expr);
}

if (!$context->null() && TypeCombinator::containsNull($otherType)) {
if ($enumCaseObjectType->toBoolean()->isTrue()->yes()) {
$otherType = TypeCombinator::remove($otherType, new NullType());
}

if (!$otherType->isSuperTypeOf($enumCaseObjectType)->no()) {
return $this->create($exprNode, TypeCombinator::intersect($enumCaseObjectType, $otherType), $context, $scope)->setRootExpr($expr);
}
}
}

$expressions = $this->findEnumTypeExpressionsFromBinaryOperation($scope, $expr);
if ($expressions !== null) {
$exprNode = $expressions[0];
$enumCaseObjectType = $expressions[1];
$otherType = $expressions[2];

if (!$context->null()) {
return $this->create($exprNode, TypeCombinator::intersect($enumCaseObjectType, $otherType), $context, $scope)->setRootExpr($expr);
}
}

$leftType = $scope->getType($expr->left);
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 @@ -101,9 +101,12 @@ private static function findTestFiles(): iterable
define('TEST_FALSE_CONSTANT', false);
define('TEST_ARRAY_CONSTANT', [true, false, null]);
define('TEST_ENUM_CONSTANT', Foo::ONE);
yield __DIR__ . '/data/bug-12432-nullable-enum.php';
yield __DIR__ . '/data/new-in-initializers-runtime.php';
}

yield __DIR__ . '/data/bug-12432-nullable-int.php';

yield __DIR__ . '/../Rules/Comparison/data/bug-6473.php';

yield __DIR__ . '/../Rules/Methods/data/filter-iterator-child-class.php';
Expand Down
35 changes: 35 additions & 0 deletions tests/PHPStan/Analyser/data/bug-12432-nullable-enum.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php

namespace Bug12432;

use function PHPStan\Testing\assertType;

enum Foo: int
{
case BAR = 1;
case BAZ = 2;
case QUX = 3;
}

function requireNullableEnum(?Foo $nullable): ?Foo
{
switch ($nullable) {
case Foo::BAR:
assertType('Bug12432\Foo::BAR', $nullable);
case Foo::BAZ:
assertType('Bug12432\Foo::BAR|Bug12432\Foo::BAZ', $nullable);
break;
case '':
assertType('null', $nullable);
case null:
assertType('null', $nullable);
break;
case 0:
assertType('*NEVER*', $nullable);
default:
assertType('Bug12432\Foo::QUX', $nullable);
break;
}

return $nullable;
}
26 changes: 26 additions & 0 deletions tests/PHPStan/Analyser/data/bug-12432-nullable-int.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

namespace Bug12432;

use function PHPStan\Testing\assertType;

function requireNullableInt(?int $nullable): ?int
{
switch ($nullable) {
case 1:
assertType('1', $nullable);
case 2:
assertType('1|2', $nullable);
break;
case '':
assertType('0|null', $nullable);
case 0:
assertType('0|null', $nullable);
break;
default:
assertType('int<min, -1>|int<3, max>', $nullable);
break;
}

return $nullable;
}
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/nsrt/in_array_loose.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public function looseComparison(
assertType('int|string', $stringOrInt); // could be '1'|'2'|1|2
}
if (in_array($stringOrNull, ['1', 'a'])) {
assertType('string|null', $stringOrNull); // could be '1'|'a'
assertType("'1'|'a'", $stringOrNull);
}
}
}
Loading