Skip to content

Commit ce0aaf2

Browse files
rvanvelzenrvanvelzen
authored
Fix condition of fall-through case not used for exhaustive checks
1 parent 21923d3 commit ce0aaf2

File tree

8 files changed

+203
-3
lines changed

8 files changed

+203
-3
lines changed

‎src/Analyser/NodeScopeResolver.php

+6-2
Original file line numberDiff line numberDiff line change
@@ -1449,9 +1449,11 @@ private function processStmtNode(
14491449
$exitPointsForOuterLoop = [];
14501450
$throwPoints = $condResult->getThrowPoints();
14511451
$impurePoints = $condResult->getImpurePoints();
1452+
$fullCondExpr = null;
14521453
foreach ($stmt->cases as $caseNode) {
14531454
if ($caseNode->cond !== null) {
14541455
$condExpr = new BinaryOp\Equal($stmt->cond, $caseNode->cond);
1456+
$fullCondExpr = $fullCondExpr === null ? $condExpr : new BooleanOr($fullCondExpr, $condExpr);
14551457
$caseResult = $this->processExprNode($stmt, $caseNode->cond, $scopeForBranches, $nodeCallback, ExpressionContext::createDeep());
14561458
$scopeForBranches = $caseResult->getScope();
14571459
$hasYield = $hasYield || $caseResult->hasYield();
@@ -1460,6 +1462,7 @@ private function processStmtNode(
14601462
$branchScope = $caseResult->getTruthyScope()->filterByTruthyValue($condExpr);
14611463
} else {
14621464
$hasDefaultCase = true;
1465+
$fullCondExpr = null;
14631466
$branchScope = $scopeForBranches;
14641467
}
14651468

@@ -1481,8 +1484,9 @@ private function processStmtNode(
14811484
if ($branchScopeResult->isAlwaysTerminating()) {
14821485
$alwaysTerminating = $alwaysTerminating && $branchFinalScopeResult->isAlwaysTerminating();
14831486
$prevScope = null;
1484-
if (isset($condExpr)) {
1485-
$scopeForBranches = $scopeForBranches->filterByFalseyValue($condExpr);
1487+
if (isset($fullCondExpr)) {
1488+
$scopeForBranches = $scopeForBranches->filterByFalseyValue($fullCondExpr);
1489+
$fullCondExpr = null;
14861490
}
14871491
if (!$branchFinalScopeResult->isAlwaysTerminating()) {
14881492
$finalScope = $branchScope->mergeWith($finalScope);

‎tests/PHPStan/Analyser/nsrt/bug-11064.php

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug11064;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
interface IClass {}
8+
class ClassA implements IClass {}
9+
class ClassB implements IClass {}
10+
11+
/**
12+
* @param null|'nil'|IClass $val
13+
*/
14+
function test($val): string {
15+
switch (true) {
16+
case $val === null:
17+
case $val === 'nil':
18+
assertType("'nil'|null", $val);
19+
return 'null';
20+
21+
case $val instanceof ClassA:
22+
assertType('Bug11064\\ClassA', $val);
23+
return 'class a';
24+
25+
default:
26+
assertType('Bug11064\\IClass~Bug11064\\ClassA', $val);
27+
throw new RuntimeException('unsupported class: ' . get_class($val));
28+
}
29+
}
30+

‎tests/PHPStan/Rules/Missing/MissingReturnRuleTest.php

+21
Original file line numberDiff line numberDiff line change
@@ -343,4 +343,25 @@ public function testBug9374(): void
343343
$this->analyse([__DIR__ . '/data/bug-9374.php'], []);
344344
}
345345

346+
public function testBug3488Two(): void
347+
{
348+
$this->checkExplicitMixedMissingReturn = true;
349+
$this->analyse([__DIR__ . '/data/bug-3488-2.php'], [
350+
[
351+
'Method Bug3488\C::invalidCase() should return int but return statement is missing.',
352+
30,
353+
],
354+
]);
355+
}
356+
357+
public function testBug12722(): void
358+
{
359+
if (PHP_VERSION_ID < 80100) {
360+
$this->markTestSkipped('Test requires PHP 8.1.');
361+
}
362+
363+
$this->checkExplicitMixedMissingReturn = true;
364+
$this->analyse([__DIR__ . '/data/bug-12722.php'], []);
365+
}
366+
346367
}

‎tests/PHPStan/Rules/Missing/data/bug-12722.php

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php declare(strict_types = 1); // lint >= 8.1
2+
3+
namespace Bug12722;
4+
5+
enum states {
6+
case state1;
7+
case statealmost1;
8+
case state3;
9+
}
10+
11+
class HelloWorld
12+
{
13+
public function intentional_fallthrough(states $state): int
14+
{
15+
switch($state) {
16+
17+
case states::state1: //intentional fall-trough this case...
18+
case states::statealmost1: return 1;
19+
case states::state3: return 3;
20+
}
21+
}
22+
23+
public function no_fallthrough(states $state): int
24+
{
25+
switch($state) {
26+
27+
case states::state1: return 1;
28+
case states::statealmost1: return 1;
29+
case states::state3: return 3;
30+
}
31+
}
32+
}

‎tests/PHPStan/Rules/Missing/data/bug-3488-2.php

+52
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug3488;
4+
5+
interface I {
6+
const THING_A = 'a';
7+
const THING_B = 'b';
8+
const THING_C = 'c';
9+
}
10+
11+
class C
12+
{
13+
/**
14+
* @param I::THING_* $thing
15+
*/
16+
public function allCovered(string $thing): int
17+
{
18+
switch ($thing) { // should not error
19+
case I::THING_A:
20+
case I::THING_B:
21+
case I::THING_C:
22+
return 0;
23+
}
24+
}
25+
/**
26+
* @param I::THING_* $thing
27+
*/
28+
public function invalidCase(string $thing): int
29+
{
30+
switch ($thing) {
31+
case I::THING_A:
32+
case I::THING_B:
33+
return 0;
34+
case 'd':
35+
throw new Exception('The error marker should be on the line above');
36+
}
37+
}
38+
/**
39+
* @param I::THING_* $thing
40+
*/
41+
public function defaultUnnecessary(string $thing): int
42+
{
43+
switch ($thing) {
44+
case I::THING_A:
45+
case I::THING_B:
46+
case I::THING_C:
47+
return 0;
48+
default:
49+
throw new Exception('This should be detected as unreachable');
50+
}
51+
}
52+
}

‎tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php

+9
Original file line numberDiff line numberDiff line change
@@ -1068,4 +1068,13 @@ public function testBug10228(): void
10681068
$this->analyse([__DIR__ . '/data/bug-10228.php'], []);
10691069
}
10701070

1071+
public function testBug8719(): void
1072+
{
1073+
$this->cliArgumentsVariablesRegistered = true;
1074+
$this->polluteScopeWithLoopInitialAssignments = true;
1075+
$this->checkMaybeUndefinedVariables = true;
1076+
$this->polluteScopeWithAlwaysIterableForeach = true;
1077+
$this->analyse([__DIR__ . '/data/bug-8719.php'], []);
1078+
}
1079+
10711080
}

‎tests/PHPStan/Rules/Variables/IssetRuleTest.php

+3-1
Original file line numberDiff line numberDiff line change
@@ -277,10 +277,12 @@ public function testVariableCertaintyInIsset(): void
277277
112,
278278
],
279279
[
280-
'Variable $variableInFirstCase in isset() always exists and is not nullable.',
280+
// could be Variable $variableInFirstCase in isset() always exists and is not nullable.
281+
'Variable $variableInFirstCase in isset() is never defined.',
281282
116,
282283
],
283284
[
285+
// could be Variable $variableInSecondCase in isset() always exists and is not nullable.
284286
'Variable $variableInSecondCase in isset() is never defined.',
285287
117,
286288
],

‎tests/PHPStan/Rules/Variables/data/bug-8719.php

+50
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug8719;
4+
5+
class HelloWorld
6+
{
7+
private const CASE_1 = 1;
8+
private const CASE_2 = 2;
9+
private const CASE_3 = 3;
10+
11+
/**
12+
* @return self::CASE_*
13+
*/
14+
private function getCase(): int
15+
{
16+
return random_int(1,3);
17+
}
18+
19+
public function ok(): string
20+
{
21+
switch($this->getCase()) {
22+
case self::CASE_1:
23+
$foo = 'bar';
24+
break;
25+
case self::CASE_2:
26+
$foo = 'baz';
27+
break;
28+
case self::CASE_3:
29+
$foo = 'barbaz';
30+
break;
31+
}
32+
33+
return $foo;
34+
}
35+
36+
public function not_ok(): string
37+
{
38+
switch($this->getCase()) {
39+
case self::CASE_1:
40+
$foo = 'bar';
41+
break;
42+
case self::CASE_2:
43+
case self::CASE_3:
44+
$foo = 'barbaz';
45+
break;
46+
}
47+
48+
return $foo;
49+
}
50+
}

0 commit comments

Comments
 (0)