Merge pull request #7664 from AndrolGenhald/bugfix/7613-unused-variab…

…le-in-try Fix false positive for unused variable in try (fixes #7613).
vimeo · Feb 13, 2022 · 06ce3ad · 06ce3ad
2 parents 7f304be + fd0ecf2
commit 06ce3ad
Show file tree

Hide file tree

Showing 5 changed files with 117 additions and 0 deletions.
diff --git a/src/Psalm/Context.php b/src/Psalm/Context.php
@@ -108,6 +108,13 @@ class Context
      */
     public $inside_assignment = false;
 
+    /**
+     * Whether or not we're inside a try block.
+     *
+     * @var bool
+     */
+    public $inside_try = false;
+
     /**
      * @var null|CodeLocation
      */

diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php
@@ -86,9 +86,12 @@ public static function analyze(
 
         $old_referenced_var_ids = $try_context->referenced_var_ids;
 
+        $was_inside_try = $context->inside_try;
+        $context->inside_try = true;
         if ($statements_analyzer->analyze($stmt->stmts, $context) === false) {
             return false;
         }
+        $context->inside_try = $was_inside_try;
 
         if ($try_context->finally_scope) {
             foreach ($context->vars_in_scope as $var_id => $type) {

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php
@@ -315,6 +315,14 @@ public static function analyze(
             $assign_value_type->parent_nodes = [
                 $assignment_node->id => $assignment_node
             ];
+
+            if ($context->inside_try) {
+                // Copy previous assignment's parent nodes inside a try. Since an exception could be thrown at any
+                // point this is a workaround to ensure that use of a variable also uses all previous assignments.
+                if (isset($context->vars_in_scope[$array_var_id])) {
+                    $assign_value_type->parent_nodes += $context->vars_in_scope[$array_var_id]->parent_nodes;
+                }
+            }
         }
 
         if ($array_var_id && isset($context->vars_in_scope[$array_var_id])) {

diff --git a/tests/TaintTest.php b/tests/TaintTest.php
@@ -2285,6 +2285,25 @@ function sinkNotWorking($sink) : string {}
                     echo sinkNotWorking($_GET["taint"]);',
                 'error_message' => 'TaintedHtml',
             ],
+            'taintEscapedInTryMightNotWork' => [
+                '<?php
+                    /** @psalm-taint-escape html */
+                    function escapeHtml(string $arg): string
+                    {
+                        return htmlspecialchars($arg);
+                    }
+
+                    $tainted = $_GET["foo"];
+
+                    try {
+                        $tainted = escapeHtml($tainted);
+                    } catch (Throwable $_) {
+                    }
+
+                    echo $tainted;
+                ',
+                'error_message' => 'TaintedHtml',
+            ],
         ];
     }
 

diff --git a/tests/UnusedVariableTest.php b/tests/UnusedVariableTest.php
@@ -2436,6 +2436,43 @@ function setProxySettingsFromEnv(): void {
                         $a = false;
                     }'
             ],
+            'usedInCatchIsAlwaysUsedInTry' => [
+                '<?php
+                    $step = 0;
+                    try {
+                        $step = 1;
+                        $step = 2;
+                    } catch (Throwable $_) {
+                        echo $step;
+                    }
+                ',
+            ],
+            'usedInFinallyIsAlwaysUsedInTry' => [
+                '<?php
+                    $step = 0;
+                    try {
+                        $step = 1;
+                        $step = 2;
+                    } finally {
+                        echo $step;
+                    }
+                ',
+            ],
+            'usedInFinallyIsAlwaysUsedInTryWithNestedTry' => [
+                '<?php
+                    $step = 0;
+                    try {
+                        try {
+                            $step = 1;
+                        } finally {
+                        }
+                        $step = 2;
+                        $step = 3;
+                    } finally {
+                        echo $step;
+                    }
+                ',
+            ],
         ];
     }
 
@@ -3416,6 +3453,49 @@ function takesArray($a) : void {
                     }',
                 'error_message' => 'MixedArgumentTypeCoercion - src' . DIRECTORY_SEPARATOR . 'somefile.php:12:44 - Argument 1 of takesArrayOfString expects array<array-key, string>, parent type array{mixed} provided. Consider improving the type at src' . DIRECTORY_SEPARATOR . 'somefile.php:10:41'
             ],
+            'warnAboutUnusedVariableInTryReassignedInCatch' => [
+                '<?php
+                    $step = 0;
+                    try {
+                        $step = 1;
+                        $step = 2;
+                    } catch (Throwable $_) {
+                        $step = 3;
+                        echo $step;
+                    }
+                ',
+                'error_message' => 'UnusedVariable',
+            ],
+            'warnAboutUnusedVariableInTryReassignedInFinally' => [
+                '<?php
+                    $step = 0;
+                    try {
+                        $step = 1;
+                        $step = 2;
+                    } finally {
+                        $step = 3;
+                        echo $step;
+                    }
+                ',
+                'error_message' => 'UnusedVariable',
+            ],
+            'SKIPPED-warnAboutVariableUsedInNestedTryNotUsedInOuterTry' => [
+                '<?php
+                    $step = 0;
+                    try {
+                        $step = 1; // Unused
+                        $step = 2;
+                        try {
+                            $step = 3;
+                            $step = 4;
+                        } finally {
+                            echo $step;
+                        }
+                    } finally {
+                    }
+                ',
+                'error_message' => 'UnusedVariable',
+            ],
         ];
     }
 }