Skip to content

Commit ab6c621

Browse files
dmytro-dymarchukdmytro-dymarchuk
committed
Narrow variable type in switch cases
1 parent 961b404 commit ab6c621

File tree

6 files changed

+160
-18
lines changed

6 files changed

+160
-18
lines changed

‎src/Analyser/NodeScopeResolver.php‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,7 @@
204204
use function array_merge;
205205
use function array_pop;
206206
use function array_reverse;
207+
use function array_shift;
207208
use function array_slice;
208209
use function array_values;
209210
use function base64_decode;
@@ -1513,9 +1514,11 @@ private function processStmtNode(
15131514
$exitPointsForOuterLoop = [];
15141515
$throwPoints = $condResult->getThrowPoints();
15151516
$impurePoints = $condResult->getImpurePoints();
1517+
$defaultCondExprs = [];
15161518
foreach ($stmt->cases as $caseNode) {
15171519
if ($caseNode->cond !== null) {
15181520
$condExpr = new BinaryOp\Equal($stmt->cond, $caseNode->cond);
1521+
$defaultCondExprs[] = new BinaryOp\NotEqual($stmt->cond, $caseNode->cond);
15191522
$caseResult = $this->processExprNode($stmt, $caseNode->cond, $scopeForBranches, $nodeCallback, ExpressionContext::createDeep());
15201523
$scopeForBranches = $caseResult->getScope();
15211524
$hasYield = $hasYield || $caseResult->hasYield();
@@ -1525,6 +1528,11 @@ private function processStmtNode(
15251528
} else {
15261529
$hasDefaultCase = true;
15271530
$branchScope = $scopeForBranches;
1531+
$defaultConditions = $this->createBooleanAndFromExpressions($defaultCondExprs);
1532+
if ($defaultConditions !== null) {
1533+
$branchScope = $this->processExprNode($stmt, $defaultConditions, $scope, static function (): void {
1534+
}, ExpressionContext::createDeep())->getTruthyScope()->filterByTruthyValue($defaultConditions);
1535+
}
15281536
}
15291537

15301538
$branchScope = $branchScope->mergeWith($prevScope);
@@ -6570,6 +6578,29 @@ private function getPhpDocReturnType(ResolvedPhpDocBlock $resolvedPhpDoc, Type $
65706578
return null;
65716579
}
65726580

6581+
/**
6582+
* @param list<Expr> $expressions
6583+
*/
6584+
private function createBooleanAndFromExpressions(array $expressions): ?Expr
6585+
{
6586+
if (count($expressions) === 0) {
6587+
return null;
6588+
}
6589+
6590+
if (count($expressions) === 1) {
6591+
return $expressions[0];
6592+
}
6593+
6594+
$left = array_shift($expressions);
6595+
$right = $this->createBooleanAndFromExpressions($expressions);
6596+
6597+
if ($right === null) {
6598+
throw new ShouldNotHappenException();
6599+
}
6600+
6601+
return new BooleanAnd($left, $right);
6602+
}
6603+
65736604
/**
65746605
* @param array<Node> $nodes
65756606
* @return list<Node\Stmt>

‎src/Analyser/TypeSpecifier.php‎

Lines changed: 64 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1572,15 +1572,8 @@ private function findTypeExpressionsFromBinaryOperation(Scope $scope, Node\Expr\
15721572
$leftType = $scope->getType($binaryOperation->left);
15731573
$rightType = $scope->getType($binaryOperation->right);
15741574

1575-
$rightExpr = $binaryOperation->right;
1576-
if ($rightExpr instanceof AlwaysRememberedExpr) {
1577-
$rightExpr = $rightExpr->getExpr();
1578-
}
1579-
1580-
$leftExpr = $binaryOperation->left;
1581-
if ($leftExpr instanceof AlwaysRememberedExpr) {
1582-
$leftExpr = $leftExpr->getExpr();
1583-
}
1575+
$rightExpr = $this->extractExpression($binaryOperation->right);
1576+
$leftExpr = $this->extractExpression($binaryOperation->left);
15841577

15851578
if (
15861579
$leftType instanceof ConstantScalarType
@@ -1599,6 +1592,39 @@ private function findTypeExpressionsFromBinaryOperation(Scope $scope, Node\Expr\
15991592
return null;
16001593
}
16011594

1595+
/**
1596+
* @return array{Expr, Type, Type}|null
1597+
*/
1598+
private function findEnumTypeExpressionsFromBinaryOperation(Scope $scope, Node\Expr\BinaryOp $binaryOperation): ?array
1599+
{
1600+
$leftType = $scope->getType($binaryOperation->left);
1601+
$rightType = $scope->getType($binaryOperation->right);
1602+
1603+
$rightExpr = $this->extractExpression($binaryOperation->right);
1604+
$leftExpr = $this->extractExpression($binaryOperation->left);
1605+
1606+
if (
1607+
$leftType->getEnumCases() === [$leftType]
1608+
&& !$rightExpr instanceof ConstFetch
1609+
&& !$rightExpr instanceof ClassConstFetch
1610+
) {
1611+
return [$binaryOperation->right, $leftType, $rightType];
1612+
} elseif (
1613+
$rightType->getEnumCases() === [$rightType]
1614+
&& !$leftExpr instanceof ConstFetch
1615+
&& !$leftExpr instanceof ClassConstFetch
1616+
) {
1617+
return [$binaryOperation->left, $rightType, $leftType];
1618+
}
1619+
1620+
return null;
1621+
}
1622+
1623+
private function extractExpression(Expr $expr): Expr
1624+
{
1625+
return $expr instanceof AlwaysRememberedExpr ? $expr->getExpr() : $expr;
1626+
}
1627+
16021628
/** @api */
16031629
public function create(
16041630
Expr $expr,
@@ -1894,10 +1920,10 @@ public function resolveEqual(Expr\BinaryOp\Equal $expr, Scope $scope, TypeSpecif
18941920
$expressions = $this->findTypeExpressionsFromBinaryOperation($scope, $expr);
18951921
if ($expressions !== null) {
18961922
$exprNode = $expressions[0];
1897-
$constantType = $expressions[1];
1923+
$enumCaseObjectType = $expressions[1];
18981924
$otherType = $expressions[2];
18991925

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

1912-
if (!$context->null() && $constantType->getValue() === false) {
1938+
if (!$context->null() && $enumCaseObjectType->getValue() === false) {
19131939
return $this->specifyTypesInCondition(
19141940
$scope,
19151941
$exprNode,
19161942
$context->true() ? TypeSpecifierContext::createFalsey() : TypeSpecifierContext::createFalsey()->negate(),
19171943
)->setRootExpr($expr);
19181944
}
19191945

1920-
if (!$context->null() && $constantType->getValue() === true) {
1946+
if (!$context->null() && $enumCaseObjectType->getValue() === true) {
19211947
return $this->specifyTypesInCondition(
19221948
$scope,
19231949
$exprNode,
19241950
$context->true() ? TypeSpecifierContext::createTruthy() : TypeSpecifierContext::createTruthy()->negate(),
19251951
)->setRootExpr($expr);
19261952
}
19271953

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

1952-
if (!$context->null() && $constantType->getValue() === '') {
1978+
if (!$context->null() && $enumCaseObjectType->getValue() === '') {
19531979
/* There is a difference between php 7.x and 8.x on the equality
19541980
* behavior between zero and the empty string, so to be conservative
19551981
* we leave it untouched regardless of the language version */
@@ -1976,7 +2002,7 @@ public function resolveEqual(Expr\BinaryOp\Equal $expr, Scope $scope, TypeSpecif
19762002
&& $exprNode->name instanceof Name
19772003
&& in_array(strtolower($exprNode->name->toString()), ['gettype', 'get_class', 'get_debug_type'], true)
19782004
&& isset($exprNode->getArgs()[0])
1979-
&& $constantType->isString()->yes()
2005+
&& $enumCaseObjectType->isString()->yes()
19802006
) {
19812007
return $this->specifyTypesInCondition($scope, new Expr\BinaryOp\Identical($expr->left, $expr->right), $context)->setRootExpr($expr);
19822008
}
@@ -1986,10 +2012,31 @@ public function resolveEqual(Expr\BinaryOp\Equal $expr, Scope $scope, TypeSpecif
19862012
&& $exprNode instanceof FuncCall
19872013
&& $exprNode->name instanceof Name
19882014
&& $exprNode->name->toLowerString() === 'preg_match'
1989-
&& (new ConstantIntegerType(1))->isSuperTypeOf($constantType)->yes()
2015+
&& (new ConstantIntegerType(1))->isSuperTypeOf($enumCaseObjectType)->yes()
19902016
) {
19912017
return $this->specifyTypesInCondition($scope, new Expr\BinaryOp\Identical($expr->left, $expr->right), $context)->setRootExpr($expr);
19922018
}
2019+
2020+
if (!$context->null() && TypeCombinator::containsNull($otherType)) {
2021+
if ($enumCaseObjectType->toBoolean()->isTrue()->yes()) {
2022+
$otherType = TypeCombinator::remove($otherType, new NullType());
2023+
}
2024+
2025+
if (!$otherType->isSuperTypeOf($enumCaseObjectType)->no()) {
2026+
return $this->create($exprNode, TypeCombinator::intersect($enumCaseObjectType, $otherType), $context, $scope)->setRootExpr($expr);
2027+
}
2028+
}
2029+
}
2030+
2031+
$expressions = $this->findEnumTypeExpressionsFromBinaryOperation($scope, $expr);
2032+
if ($expressions !== null) {
2033+
$exprNode = $expressions[0];
2034+
$enumCaseObjectType = $expressions[1];
2035+
$otherType = $expressions[2];
2036+
2037+
if (!$context->null()) {
2038+
return $this->create($exprNode, TypeCombinator::intersect($enumCaseObjectType, $otherType), $context, $scope)->setRootExpr($expr);
2039+
}
19932040
}
19942041

19952042
$leftType = $scope->getType($expr->left);

‎tests/PHPStan/Analyser/NodeScopeResolverTest.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,12 @@ private static function findTestFiles(): iterable
101101
define('TEST_FALSE_CONSTANT', false);
102102
define('TEST_ARRAY_CONSTANT', [true, false, null]);
103103
define('TEST_ENUM_CONSTANT', Foo::ONE);
104+
yield __DIR__ . '/data/bug-12432-nullable-enum.php';
104105
yield __DIR__ . '/data/new-in-initializers-runtime.php';
105106
}
106107

108+
yield __DIR__ . '/data/bug-12432-nullable-int.php';
109+
107110
yield __DIR__ . '/../Rules/Comparison/data/bug-6473.php';
108111

109112
yield __DIR__ . '/../Rules/Methods/data/filter-iterator-child-class.php';

‎tests/PHPStan/Analyser/data/bug-12432-nullable-enum.php‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?php
2+
3+
namespace Bug12432;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
enum Foo: int
8+
{
9+
case BAR = 1;
10+
case BAZ = 2;
11+
case QUX = 3;
12+
}
13+
14+
function requireNullableEnum(?Foo $nullable): ?Foo
15+
{
16+
switch ($nullable) {
17+
case Foo::BAR:
18+
assertType('Bug12432\Foo::BAR', $nullable);
19+
case Foo::BAZ:
20+
assertType('Bug12432\Foo::BAR|Bug12432\Foo::BAZ', $nullable);
21+
break;
22+
case '':
23+
assertType('null', $nullable);
24+
case null:
25+
assertType('null', $nullable);
26+
break;
27+
case 0:
28+
assertType('*NEVER*', $nullable);
29+
default:
30+
assertType('Bug12432\Foo::QUX', $nullable);
31+
break;
32+
}
33+
34+
return $nullable;
35+
}

‎tests/PHPStan/Analyser/data/bug-12432-nullable-int.php‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<?php
2+
3+
namespace Bug12432;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
function requireNullableInt(?int $nullable): ?int
8+
{
9+
switch ($nullable) {
10+
case 1:
11+
assertType('1', $nullable);
12+
case 2:
13+
assertType('1|2', $nullable);
14+
break;
15+
case '':
16+
assertType('0|null', $nullable);
17+
case 0:
18+
assertType('0|null', $nullable);
19+
break;
20+
default:
21+
assertType('int<min, -1>|int<3, max>', $nullable);
22+
break;
23+
}
24+
25+
return $nullable;
26+
}

‎tests/PHPStan/Analyser/nsrt/in_array_loose.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public function looseComparison(
4242
assertType('int|string', $stringOrInt); // could be '1'|'2'|1|2
4343
}
4444
if (in_array($stringOrNull, ['1', 'a'])) {
45-
assertType('string|null', $stringOrNull); // could be '1'|'a'
45+
assertType("'1'|'a'", $stringOrNull);
4646
}
4747
}
4848
}

0 commit comments

Comments
 (0)