Fix #3808 - allow detection of paradoxes in switch condition function…

… calls
vimeo · Jul 14, 2020 · 3c9028c · 3c9028c
1 parent f0a5463
commit 3c9028c
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 6 deletions.
diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/SwitchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/SwitchAnalyzer.php
@@ -46,6 +46,19 @@ public static function analyze(
             $statements_analyzer
         );
 
+        if (!$switch_var_id
+            && ($stmt->cond instanceof PhpParser\Node\Expr\FuncCall
+                || $stmt->cond instanceof PhpParser\Node\Expr\MethodCall
+                || $stmt->cond instanceof PhpParser\Node\Expr\StaticCall
+            )
+        ) {
+            $switch_var_id = '$__tmp_switch__' . (int) $stmt->cond->getAttribute('startFilePos');
+
+            $condition_type = $statements_analyzer->node_data->getType($stmt->cond) ?: Type::getMixed();
+
+            $context->vars_in_scope[$switch_var_id] = $condition_type;
+        }
+
         $original_context = clone $context;
 
         // the last statement always breaks, by default

diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/SwitchCaseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/SwitchCaseAnalyzer.php
@@ -65,6 +65,19 @@ public static function analyze(
 
         $old_node_data = $statements_analyzer->node_data;
 
+        $fake_switch_condition = false;
+
+        if ($switch_var_id && substr($switch_var_id, 0, 15) === '$__tmp_switch__') {
+            $switch_condition = new PhpParser\Node\Expr\Variable(
+                substr($switch_var_id, 1),
+                $stmt->cond->getAttributes()
+            );
+
+            $fake_switch_condition = true;
+        } else {
+            $switch_condition = $stmt->cond;
+        }
+
         if ($case->cond) {
             $was_inside_conditional = $case_context->inside_conditional;
             $case_context->inside_conditional = true;
@@ -92,7 +105,14 @@ public static function analyze(
             );
 
             /** @var PhpParser\Node\Expr */
-            $switch_condition = $traverser->traverse([$stmt->cond])[0];
+            $switch_condition = $traverser->traverse([$switch_condition])[0];
+
+            if ($fake_switch_condition) {
+                $statements_analyzer->node_data->setType(
+                    $switch_condition,
+                    $case_context->vars_in_scope[$switch_var_id] ?? Type::getMixed()
+                );
+            }
 
             if ($switch_condition instanceof PhpParser\Node\Expr\Variable
                 && is_string($switch_condition->name)
@@ -129,6 +149,13 @@ public static function analyze(
 
                 if ($type_statements && count($type_statements) === 1) {
                     $switch_condition = $type_statements[0];
+
+                    if ($fake_switch_condition) {
+                        $statements_analyzer->node_data->setType(
+                            $switch_condition,
+                            $case_context->vars_in_scope[$switch_var_id] ?? Type::getMixed()
+                        );
+                    }
                 }
             }
 
@@ -228,13 +255,13 @@ public static function analyze(
         }
 
         if ($case_equality_expr
-            && $stmt->cond instanceof PhpParser\Node\Expr\Variable
-            && is_string($stmt->cond->name)
-            && isset($context->vars_in_scope['$' . $stmt->cond->name])
+            && $switch_condition instanceof PhpParser\Node\Expr\Variable
+            && is_string($switch_condition->name)
+            && isset($context->vars_in_scope['$' . $switch_condition->name])
         ) {
             $new_case_equality_expr = self::simplifyCaseEqualityExpression(
                 $case_equality_expr,
-                $stmt->cond
+                $switch_condition
             );
 
             if ($new_case_equality_expr) {

diff --git a/tests/SwitchTypeTest.php b/tests/SwitchTypeTest.php
@@ -1188,7 +1188,25 @@ function foo(Exception $e) {
                                 break;
                         }
                     }',
-                'error_message' => 'TypeDoesNotContainType - src' . DIRECTORY_SEPARATOR . 'somefile.php:5:34 - string(InvalidArgumentException) cannot be identical to class-string',
+                'error_message' => 'TypeDoesNotContainType',
+            ],
+            'paradoxInFunctionCall' => [
+                '<?php
+                    /** @psalm-return 1|2|3 */
+                    function foo() {
+                        /** @psalm-var 1|2|3 $bar */
+                        $bar = rand(1, 3);
+                        return $bar;
+                    }
+
+                    switch(foo()) {
+                        case 1: break;
+                        case 2: break;
+                        case 3: break;
+                        default:
+                            echo "bar";
+                    }',
+                'error_message' => 'ParadoxicalCondition'
             ],
         ];
     }