From df8150e521d747e31fcd3595a83d881394f69ab2 Mon Sep 17 00:00:00 2001
From: Richard van Velzen <rvanvelzen1@gmail.com>
Date: Sun, 23 Mar 2025 13:22:13 +0100
Subject: [PATCH] Fix condition of fall-through case not used for exhaustive
 checks

---
 src/Analyser/NodeScopeResolver.php            |  8 ++-
 tests/PHPStan/Analyser/nsrt/bug-11064.php     | 30 +++++++++++
 .../Rules/Missing/MissingReturnRuleTest.php   | 21 ++++++++
 .../PHPStan/Rules/Missing/data/bug-12722.php  | 32 ++++++++++++
 .../PHPStan/Rules/Missing/data/bug-3488-2.php | 52 +++++++++++++++++++
 .../Variables/DefinedVariableRuleTest.php     |  9 ++++
 .../PHPStan/Rules/Variables/IssetRuleTest.php |  4 +-
 .../PHPStan/Rules/Variables/data/bug-8719.php | 50 ++++++++++++++++++
 8 files changed, 203 insertions(+), 3 deletions(-)
 create mode 100644 tests/PHPStan/Analyser/nsrt/bug-11064.php
 create mode 100644 tests/PHPStan/Rules/Missing/data/bug-12722.php
 create mode 100644 tests/PHPStan/Rules/Missing/data/bug-3488-2.php
 create mode 100644 tests/PHPStan/Rules/Variables/data/bug-8719.php

diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php
index 8599b4bd6e..7beb64aed1 100644
--- a/src/Analyser/NodeScopeResolver.php
+++ b/src/Analyser/NodeScopeResolver.php
@@ -1449,9 +1449,11 @@ private function processStmtNode(
 			$exitPointsForOuterLoop = [];
 			$throwPoints = $condResult->getThrowPoints();
 			$impurePoints = $condResult->getImpurePoints();
+			$fullCondExpr = null;
 			foreach ($stmt->cases as $caseNode) {
 				if ($caseNode->cond !== null) {
 					$condExpr = new BinaryOp\Equal($stmt->cond, $caseNode->cond);
+					$fullCondExpr = $fullCondExpr === null ? $condExpr : new BooleanOr($fullCondExpr, $condExpr);
 					$caseResult = $this->processExprNode($stmt, $caseNode->cond, $scopeForBranches, $nodeCallback, ExpressionContext::createDeep());
 					$scopeForBranches = $caseResult->getScope();
 					$hasYield = $hasYield || $caseResult->hasYield();
@@ -1460,6 +1462,7 @@ private function processStmtNode(
 					$branchScope = $caseResult->getTruthyScope()->filterByTruthyValue($condExpr);
 				} else {
 					$hasDefaultCase = true;
+					$fullCondExpr = null;
 					$branchScope = $scopeForBranches;
 				}
 
@@ -1481,8 +1484,9 @@ private function processStmtNode(
 				if ($branchScopeResult->isAlwaysTerminating()) {
 					$alwaysTerminating = $alwaysTerminating && $branchFinalScopeResult->isAlwaysTerminating();
 					$prevScope = null;
-					if (isset($condExpr)) {
-						$scopeForBranches = $scopeForBranches->filterByFalseyValue($condExpr);
+					if (isset($fullCondExpr)) {
+						$scopeForBranches = $scopeForBranches->filterByFalseyValue($fullCondExpr);
+						$fullCondExpr = null;
 					}
 					if (!$branchFinalScopeResult->isAlwaysTerminating()) {
 						$finalScope = $branchScope->mergeWith($finalScope);
diff --git a/tests/PHPStan/Analyser/nsrt/bug-11064.php b/tests/PHPStan/Analyser/nsrt/bug-11064.php
new file mode 100644
index 0000000000..8f0876667a
--- /dev/null
+++ b/tests/PHPStan/Analyser/nsrt/bug-11064.php
@@ -0,0 +1,30 @@
+<?php declare(strict_types = 1);
+
+namespace Bug11064;
+
+use function PHPStan\Testing\assertType;
+
+interface IClass {}
+class ClassA implements IClass {}
+class ClassB implements IClass {}
+
+/**
+ * @param null|'nil'|IClass $val
+ */
+function test($val): string {
+	switch (true) {
+		case $val === null:
+		case $val === 'nil':
+			assertType("'nil'|null", $val);
+			return 'null';
+
+		case $val instanceof ClassA:
+			assertType('Bug11064\\ClassA', $val);
+			return 'class a';
+
+		default:
+			assertType('Bug11064\\IClass~Bug11064\\ClassA', $val);
+			throw new RuntimeException('unsupported class: ' . get_class($val));
+	}
+}
+
diff --git a/tests/PHPStan/Rules/Missing/MissingReturnRuleTest.php b/tests/PHPStan/Rules/Missing/MissingReturnRuleTest.php
index f99d9352d9..c836705fbe 100644
--- a/tests/PHPStan/Rules/Missing/MissingReturnRuleTest.php
+++ b/tests/PHPStan/Rules/Missing/MissingReturnRuleTest.php
@@ -343,4 +343,25 @@ public function testBug9374(): void
 		$this->analyse([__DIR__ . '/data/bug-9374.php'], []);
 	}
 
+	public function testBug3488Two(): void
+	{
+		$this->checkExplicitMixedMissingReturn = true;
+		$this->analyse([__DIR__ . '/data/bug-3488-2.php'], [
+			[
+				'Method Bug3488\C::invalidCase() should return int but return statement is missing.',
+				30,
+			],
+		]);
+	}
+
+	public function testBug12722(): void
+	{
+		if (PHP_VERSION_ID < 80100) {
+			$this->markTestSkipped('Test requires PHP 8.1.');
+		}
+
+		$this->checkExplicitMixedMissingReturn = true;
+		$this->analyse([__DIR__ . '/data/bug-12722.php'], []);
+	}
+
 }
diff --git a/tests/PHPStan/Rules/Missing/data/bug-12722.php b/tests/PHPStan/Rules/Missing/data/bug-12722.php
new file mode 100644
index 0000000000..6c42edbd19
--- /dev/null
+++ b/tests/PHPStan/Rules/Missing/data/bug-12722.php
@@ -0,0 +1,32 @@
+<?php declare(strict_types = 1); // lint >= 8.1
+
+namespace Bug12722;
+
+enum states {
+	case state1;
+	case statealmost1;
+	case state3;
+}
+
+class HelloWorld
+{
+	public function intentional_fallthrough(states $state): int
+	{
+		switch($state) {
+
+			case states::state1: //intentional fall-trough this case...
+			case states::statealmost1: return 1;
+			case states::state3: return 3;
+		}
+	}
+
+	public function no_fallthrough(states $state): int
+	{
+		switch($state) {
+
+			case states::state1: return 1;
+			case states::statealmost1: return 1;
+			case states::state3: return 3;
+		}
+	}
+}
diff --git a/tests/PHPStan/Rules/Missing/data/bug-3488-2.php b/tests/PHPStan/Rules/Missing/data/bug-3488-2.php
new file mode 100644
index 0000000000..87d3466236
--- /dev/null
+++ b/tests/PHPStan/Rules/Missing/data/bug-3488-2.php
@@ -0,0 +1,52 @@
+<?php declare(strict_types = 1);
+
+namespace Bug3488;
+
+interface I {
+	const THING_A = 'a';
+	const THING_B = 'b';
+	const THING_C = 'c';
+}
+
+class C
+{
+	/**
+	 * @param I::THING_* $thing
+	 */
+	public function allCovered(string $thing): int
+	{
+		switch ($thing) { // should not error
+			case I::THING_A:
+			case I::THING_B:
+			case I::THING_C:
+				return 0;
+		}
+	}
+	/**
+	 * @param I::THING_* $thing
+	 */
+	public function invalidCase(string $thing): int
+	{
+		switch ($thing) {
+			case I::THING_A:
+			case I::THING_B:
+				return 0;
+			case 'd':
+				throw new Exception('The error marker should be on the line above');
+		}
+	}
+	/**
+	 * @param I::THING_* $thing
+	 */
+	public function defaultUnnecessary(string $thing): int
+	{
+		switch ($thing) {
+			case I::THING_A:
+			case I::THING_B:
+			case I::THING_C:
+				return 0;
+			default:
+				throw new Exception('This should be detected as unreachable');
+		}
+	}
+}
diff --git a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php
index 94f0b0ffeb..ae2ceeb350 100644
--- a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php
+++ b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php
@@ -1068,4 +1068,13 @@ public function testBug10228(): void
 		$this->analyse([__DIR__ . '/data/bug-10228.php'], []);
 	}
 
+	public function testBug8719(): void
+	{
+		$this->cliArgumentsVariablesRegistered = true;
+		$this->polluteScopeWithLoopInitialAssignments = true;
+		$this->checkMaybeUndefinedVariables = true;
+		$this->polluteScopeWithAlwaysIterableForeach = true;
+		$this->analyse([__DIR__ . '/data/bug-8719.php'], []);
+	}
+
 }
diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php
index 16d92ed767..f96185de69 100644
--- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php
+++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php
@@ -277,10 +277,12 @@ public function testVariableCertaintyInIsset(): void
 				112,
 			],
 			[
-				'Variable $variableInFirstCase in isset() always exists and is not nullable.',
+				// could be Variable $variableInFirstCase in isset() always exists and is not nullable.
+				'Variable $variableInFirstCase in isset() is never defined.',
 				116,
 			],
 			[
+				// could be Variable $variableInSecondCase in isset() always exists and is not nullable.
 				'Variable $variableInSecondCase in isset() is never defined.',
 				117,
 			],
diff --git a/tests/PHPStan/Rules/Variables/data/bug-8719.php b/tests/PHPStan/Rules/Variables/data/bug-8719.php
new file mode 100644
index 0000000000..d1d3337111
--- /dev/null
+++ b/tests/PHPStan/Rules/Variables/data/bug-8719.php
@@ -0,0 +1,50 @@
+<?php declare(strict_types = 1);
+
+namespace Bug8719;
+
+class HelloWorld
+{
+	private const CASE_1 = 1;
+	private const CASE_2 = 2;
+	private const CASE_3 = 3;
+
+	/**
+	 * @return self::CASE_*
+	 */
+	private function getCase(): int
+	{
+		return random_int(1,3);
+	}
+
+	public function ok(): string
+	{
+		switch($this->getCase()) {
+			case self::CASE_1:
+				$foo = 'bar';
+				break;
+			case self::CASE_2:
+				$foo = 'baz';
+				break;
+			case self::CASE_3:
+				$foo = 'barbaz';
+				break;
+		}
+
+		return $foo;
+	}
+
+	public function not_ok(): string
+	{
+		switch($this->getCase()) {
+			case self::CASE_1:
+				$foo = 'bar';
+				break;
+			case self::CASE_2:
+			case self::CASE_3:
+				$foo = 'barbaz';
+				break;
+		}
+
+		return $foo;
+	}
+}