Fix #4368 - improve handling of try with finally

vimeo · Oct 20, 2020 · 53ffb21 · 53ffb21
1 parent a463c89
commit 53ffb21
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 16 deletions.
diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php
@@ -129,10 +129,8 @@ public static function analyze(
                 if (!isset($try_context->vars_in_scope[$var_id])) {
                     $try_context->vars_in_scope[$var_id] = clone $type;
 
-                    if (!$stmt->catches) {
-                        $context->vars_in_scope[$var_id]->possibly_undefined = true;
-                        $context->vars_in_scope[$var_id]->possibly_undefined_from_try = true;
-                    }
+                    $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],
@@ -474,16 +472,6 @@ function ($fq_catch_class) use ($codebase): Type\Atomic {
             }
         }
 
-        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;
-                }
-            }
-        }
-
         if ($context->loop_scope
             && !$try_leaves_loop
             && !in_array(ScopeAnalyzer::ACTION_NONE, $context->loop_scope->final_actions, true)
@@ -538,6 +526,13 @@ function ($fq_catch_class) use ($codebase): Type\Atomic {
                 /** @var string $var_id */
                 foreach ($finally_context->assigned_var_ids as $var_id => $_) {
                     if (isset($context->vars_in_scope[$var_id])) {
+                        if ($context->vars_in_scope[$var_id]->possibly_undefined
+                            && $context->vars_in_scope[$var_id]->possibly_undefined_from_try
+                        ) {
+                            $context->vars_in_scope[$var_id]->possibly_undefined = false;
+                            $context->vars_in_scope[$var_id]->possibly_undefined_from_try = false;
+                        }
+
                         $context->vars_in_scope[$var_id] = Type::combineUnionTypes(
                             $context->vars_in_scope[$var_id],
                             $finally_context->vars_in_scope[$var_id],
@@ -573,6 +568,19 @@ 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];
+
+                if ($new_type->possibly_undefined_from_try) {
+                    $new_type->possibly_undefined = false;
+                    $new_type->possibly_undefined_from_try = false;
+                }
+
+                $context->vars_in_scope[$var_id] = $new_type;
+            }
+        }
+
         foreach ($existing_thrown_exceptions as $possibly_thrown_exception => $codelocations) {
             foreach ($codelocations as $hash => $codelocation) {
                 $context->possibly_thrown_exceptions[$possibly_thrown_exception][$hash] = $codelocation;

diff --git a/src/Psalm/Internal/Type/Comparator/UnionTypeComparator.php b/src/Psalm/Internal/Type/Comparator/UnionTypeComparator.php
@@ -35,7 +35,10 @@ public static function isContainedBy(
             $union_comparison_result->scalar_type_match_found = true;
         }
 
-        if ($input_type->possibly_undefined && !$container_type->possibly_undefined) {
+        if ($input_type->possibly_undefined
+            && !$input_type->possibly_undefined_from_try
+            && !$container_type->possibly_undefined
+        ) {
             return false;
         }
 

diff --git a/src/Psalm/Internal/Type/TypeExpander.php b/src/Psalm/Internal/Type/TypeExpander.php
@@ -71,6 +71,7 @@ public static function expandUnion(
         $fleshed_out_type->ignore_nullable_issues = $return_type->ignore_nullable_issues;
         $fleshed_out_type->ignore_falsable_issues = $return_type->ignore_falsable_issues;
         $fleshed_out_type->possibly_undefined = $return_type->possibly_undefined;
+        $fleshed_out_type->possibly_undefined_from_try = $return_type->possibly_undefined_from_try;
         $fleshed_out_type->by_ref = $return_type->by_ref;
         $fleshed_out_type->initialized = $return_type->initialized;
         $fleshed_out_type->had_template = $return_type->had_template;

diff --git a/tests/TryCatchTest.php b/tests/TryCatchTest.php
@@ -413,7 +413,34 @@ public function doWork(): void {
 
                         private function workThatMayOrMayNotThrow(): void {}
                     }'
-            ]
+            ],
+            'finallyArgIsNotUndefinedIfSet' => [
+                '<?php
+                    function fooFunction (): string {
+                        try{
+                            $foo = "foo";
+                        } finally {
+                            /** @psalm-suppress PossiblyUndefinedVariable */
+                            echo $foo;
+                            $foo = "bar";
+                        }
+
+                        return $foo;
+                    }'
+            ],
+            'allowReturningPossiblyUndefinedFromTry' => [
+                '<?php
+                    function fooFunction (): string {
+                        try{
+                            $foo = "foo";
+                        } finally {
+                            /** @psalm-suppress PossiblyUndefinedVariable */
+                            echo $foo;
+                        }
+
+                        return $foo;
+                    }'
+            ],
         ];
     }
 

diff --git a/tests/UnusedVariableTest.php b/tests/UnusedVariableTest.php
@@ -1277,6 +1277,7 @@ function foo() : void {
                             $a = 4;
                             throw new Exception("bad");
                         } finally {
+                            /** @psalm-suppress PossiblyUndefinedVariable */
                             echo $a;
                         }
                     }'