Fix #4366 - possibly-undefined vars in finally block should not error

vimeo · Oct 19, 2020 · bffa064 · bffa064
1 parent 9e3b069
commit bffa064
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 6 deletions.
diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php
@@ -128,7 +128,11 @@ public static function analyze(
             foreach ($context->vars_in_scope as $var_id => $type) {
                 if (!isset($try_context->vars_in_scope[$var_id])) {
                     $try_context->vars_in_scope[$var_id] = clone $type;
-                    $try_context->vars_in_scope[$var_id]->from_docblock = true;
+
+                    if (!$stmt->catches) {
+                        $context->vars_in_scope[$var_id]->possibly_undefined = true;
+                        $context->vars_in_scope[$var_id]->possibly_undefined_from_try = true;
+                    }
                 } else {
                     $try_context->vars_in_scope[$var_id] = Type::combineUnionTypes(
                         $try_context->vars_in_scope[$var_id],
@@ -470,11 +474,13 @@ function ($fq_catch_class) use ($codebase): Type\Atomic {
             }
         }
 
-        foreach ($definitely_newly_assigned_var_ids as $var_id => $_) {
-            if (isset($context->vars_in_scope[$var_id])) {
-                $new_type = clone $context->vars_in_scope[$var_id];
-                $new_type->possibly_undefined_from_try = false;
-                $context->vars_in_scope[$var_id] = $new_type;
+        if ($stmt->catches) {
+            foreach ($definitely_newly_assigned_var_ids as $var_id => $_) {
+                if (isset($context->vars_in_scope[$var_id])) {
+                    $new_type = clone $context->vars_in_scope[$var_id];
+                    $new_type->possibly_undefined_from_try = false;
+                    $context->vars_in_scope[$var_id] = $new_type;
+                }
             }
         }
 

diff --git a/tests/TryCatchTest.php b/tests/TryCatchTest.php
@@ -391,6 +391,29 @@ public function handle(string $arg): string
                         }
                     }'
             ],
+            'finallyArgMaybeUndefined' => [
+                '<?php
+                    class TestMe {
+                        private function startTransaction(): void {
+                        }
+
+                        private function endTransaction(bool $commit): void {
+                            echo $commit ? "Committing" : "Rolling back";
+                        }
+
+                        public function doWork(): void {
+                            $this->startTransaction();
+                            try {
+                                $this->workThatMayOrMayNotThrow();
+                                $success = true;
+                            } finally {
+                                $this->endTransaction($success ?? false);
+                            }
+                        }
+
+                        private function workThatMayOrMayNotThrow(): void {}
+                    }'
+            ]
         ];
     }