Deduplicate loop analysis

Also remove some risky truthy falsy comparisons
vimeo · Jul 1, 2024 · cb4158a · cb4158a
1 parent 61c056f
commit cb4158a
Show file tree

Hide file tree

Showing 11 changed files with 177 additions and 228 deletions.
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="5.x-dev@a46a07c5ca8bd7b54d1ca814c1f5b5ed38a0ba90">
+<files psalm-version="5.x-dev@690428cb9ff9f2d8b28d099b7e5cbe6cbab8d985">
   <file src="examples/TemplateChecker.php">
     <PossiblyUndefinedIntArrayOffset>
       <code><![CDATA[$comment_block->tags['variablesfrom'][0]]]></code>
@@ -297,18 +297,10 @@
       <code><![CDATA[$source_pos = strpos($source, '*')]]></code>
     </RiskyTruthyFalsyComparison>
   </file>
-  <file src="src/Psalm/Internal/Analyzer/Statements/Block/DoAnalyzer.php">
-    <RiskyTruthyFalsyComparison>
-      <code><![CDATA[$do_context->branch_point]]></code>
-    </RiskyTruthyFalsyComparison>
-  </file>
   <file src="src/Psalm/Internal/Analyzer/Statements/Block/ForAnalyzer.php">
     <ArgumentTypeCoercion>
       <code><![CDATA[$stmt->cond]]></code>
     </ArgumentTypeCoercion>
-    <RiskyTruthyFalsyComparison>
-      <code><![CDATA[$for_context->branch_point]]></code>
-    </RiskyTruthyFalsyComparison>
   </file>
   <file src="src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php">
     <ConflictingReferenceConstraint>
@@ -319,7 +311,6 @@
       <code><![CDATA[!$var_comment->var_id]]></code>
       <code><![CDATA[!$var_comment->var_id]]></code>
       <code><![CDATA[$calling_type_params]]></code>
-      <code><![CDATA[$foreach_context->branch_point]]></code>
       <code><![CDATA[$generic_storage->template_types]]></code>
       <code><![CDATA[$statements_analyzer->getTemplateTypeMap()]]></code>
       <code><![CDATA[$var_comment->line_number]]></code>
@@ -350,7 +341,6 @@
     <RiskyTruthyFalsyComparison>
       <code><![CDATA[$context->branch_point]]></code>
       <code><![CDATA[$else_context->branch_point]]></code>
-      <code><![CDATA[$else_context->branch_point]]></code>
       <code><![CDATA[$if_scope->assigned_var_ids]]></code>
       <code><![CDATA[$if_scope->new_vars]]></code>
       <code><![CDATA[$if_scope->redefined_vars]]></code>
@@ -378,24 +368,13 @@
       <code><![CDATA[$traverser->traverse([$switch_condition])[0]]]></code>
     </PossiblyUndefinedIntArrayOffset>
     <RiskyTruthyFalsyComparison>
-      <code><![CDATA[$case_context->branch_point]]></code>
       <code><![CDATA[$nested_or_options]]></code>
       <code><![CDATA[$switch_var_id]]></code>
       <code><![CDATA[$switch_var_id]]></code>
       <code><![CDATA[$switch_var_id]]></code>
       <code><![CDATA[$type_statements]]></code>
     </RiskyTruthyFalsyComparison>
   </file>
-  <file src="src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php">
-    <RiskyTruthyFalsyComparison>
-      <code><![CDATA[$try_context->branch_point]]></code>
-    </RiskyTruthyFalsyComparison>
-  </file>
-  <file src="src/Psalm/Internal/Analyzer/Statements/Block/WhileAnalyzer.php">
-    <RiskyTruthyFalsyComparison>
-      <code><![CDATA[$while_context->branch_point]]></code>
-    </RiskyTruthyFalsyComparison>
-  </file>
   <file src="src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php">
     <RiskyTruthyFalsyComparison>
       <code><![CDATA[$var_id]]></code>
@@ -457,8 +436,6 @@
       <code><![CDATA[$first_var_name]]></code>
       <code><![CDATA[$first_var_name]]></code>
       <code><![CDATA[$first_var_name]]></code>
-      <code><![CDATA[$first_var_name]]></code>
-      <code><![CDATA[$first_var_name]]></code>
       <code><![CDATA[$first_var_name_in_array_argument]]></code>
       <code><![CDATA[$get_debug_type_position]]></code>
       <code><![CDATA[$get_debug_type_position]]></code>
@@ -935,7 +912,6 @@
       <code><![CDATA[$dim_var_id]]></code>
       <code><![CDATA[$dim_var_id]]></code>
       <code><![CDATA[$extended_var_id]]></code>
-      <code><![CDATA[$extended_var_id]]></code>
       <code><![CDATA[$keyed_array_var_id]]></code>
       <code><![CDATA[$keyed_array_var_id]]></code>
     </RiskyTruthyFalsyComparison>
@@ -1363,6 +1339,9 @@
     </RiskyTruthyFalsyComparison>
   </file>
   <file src="src/Psalm/Internal/Fork/PsalmRestarter.php">
+    <MoreSpecificImplementedParamType>
+      <code><![CDATA[$command]]></code>
+    </MoreSpecificImplementedParamType>
     <RiskyTruthyFalsyComparison>
       <code><![CDATA[$this->tmpIni]]></code>
     </RiskyTruthyFalsyComparison>

diff --git a/src/Psalm/Context.php b/src/Psalm/Context.php
@@ -314,7 +314,7 @@ final class Context
     public $is_global = false;
 
     /**
-     * @var array<string, bool>
+     * @var array<string, bool|int>
      */
     public $protected_var_ids = [];
 

diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/DoAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/DoAnalyzer.php
@@ -39,8 +39,8 @@ public static function analyze(
 
         $codebase = $statements_analyzer->getCodebase();
 
-        if ($codebase->alter_code) {
-            $do_context->branch_point = $do_context->branch_point ?: (int) $stmt->getAttribute('startFilePos');
+        if ($codebase->alter_code && $do_context->branch_point === null) {
+            $do_context->branch_point = (int) $stmt->getAttribute('startFilePos');
         }
 
         $loop_scope = new LoopScope($do_context, $context);

diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/ForAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/ForAnalyzer.php
@@ -4,15 +4,10 @@
 
 use PhpParser;
 use Psalm\Context;
-use Psalm\Internal\Analyzer\ScopeAnalyzer;
 use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer;
 use Psalm\Internal\Analyzer\StatementsAnalyzer;
-use Psalm\Internal\Scope\LoopScope;
-use UnexpectedValueException;
 
 use function array_merge;
-use function count;
-use function in_array;
 use function is_string;
 
 /**
@@ -56,113 +51,15 @@ public static function analyze(
 
         $while_true = !$stmt->cond && !$stmt->init && !$stmt->loop;
 
-        $pre_context = null;
-
-        if ($while_true) {
-            $pre_context = clone $context;
-        }
-
-        $for_context = clone $context;
-
-        $for_context->inside_loop = true;
-        $for_context->break_types[] = 'loop';
-
-        $codebase = $statements_analyzer->getCodebase();
-
-        if ($codebase->alter_code) {
-            $for_context->branch_point = $for_context->branch_point ?: (int) $stmt->getAttribute('startFilePos');
-        }
-
-        $loop_scope = new LoopScope($for_context, $context);
-
-        $loop_scope->protected_var_ids = array_merge(
-            $assigned_var_ids,
-            $context->protected_var_ids,
-        );
-
-        if (LoopAnalyzer::analyze(
+        return LoopAnalyzer::analyzeForOrWhile(
             $statements_analyzer,
-            $stmt->stmts,
+            $stmt,
+            $context,
+            $while_true,
+            $init_var_types,
+            $assigned_var_ids,
             $stmt->cond,
             $stmt->loop,
-            $loop_scope,
-            $inner_loop_context,
-        ) === false) {
-            return false;
-        }
-
-        if (!$inner_loop_context) {
-            throw new UnexpectedValueException('There should be an inner loop context');
-        }
-
-        $always_enters_loop = false;
-
-        foreach ($stmt->cond as $cond) {
-            if ($cond_type = $statements_analyzer->node_data->getType($cond)) {
-                $always_enters_loop = $cond_type->isAlwaysTruthy();
-            }
-
-            if (count($stmt->init) === 1
-                && count($stmt->cond) === 1
-                && $cond instanceof PhpParser\Node\Expr\BinaryOp
-                && ($cond_value = $statements_analyzer->node_data->getType($cond->right))
-                && ($cond_value->isSingleIntLiteral() || $cond_value->isSingleStringLiteral())
-                && $cond->left instanceof PhpParser\Node\Expr\Variable
-                && is_string($cond->left->name)
-                && isset($init_var_types[$cond->left->name])
-                && $init_var_types[$cond->left->name]->isSingleIntLiteral()
-            ) {
-                $init_value = $init_var_types[$cond->left->name]->getSingleLiteral()->value;
-                $cond_value = $cond_value->getSingleLiteral()->value;
-
-                if ($cond instanceof PhpParser\Node\Expr\BinaryOp\Smaller && $init_value < $cond_value) {
-                    $always_enters_loop = true;
-                    break;
-                }
-
-                if ($cond instanceof PhpParser\Node\Expr\BinaryOp\SmallerOrEqual && $init_value <= $cond_value) {
-                    $always_enters_loop = true;
-                    break;
-                }
-
-                if ($cond instanceof PhpParser\Node\Expr\BinaryOp\Greater && $init_value > $cond_value) {
-                    $always_enters_loop = true;
-                    break;
-                }
-
-                if ($cond instanceof PhpParser\Node\Expr\BinaryOp\GreaterOrEqual && $init_value >= $cond_value) {
-                    $always_enters_loop = true;
-                    break;
-                }
-            }
-        }
-
-        if ($while_true) {
-            $always_enters_loop = true;
-        }
-
-        $can_leave_loop = !$while_true
-            || in_array(ScopeAnalyzer::ACTION_BREAK, $loop_scope->final_actions, true);
-
-        if ($always_enters_loop && $can_leave_loop) {
-            LoopAnalyzer::setLoopVars($inner_loop_context, $context, $loop_scope);
-        }
-
-        $for_context->loop_scope = null;
-
-        if ($can_leave_loop) {
-            $context->vars_possibly_in_scope = array_merge(
-                $context->vars_possibly_in_scope,
-                $for_context->vars_possibly_in_scope,
-            );
-        } elseif ($pre_context) {
-            $context->vars_possibly_in_scope = $pre_context->vars_possibly_in_scope;
-        }
-
-        if ($context->collect_exceptions) {
-            $context->mergeExceptions($for_context);
-        }
-
-        return null;
+        );
     }
 }
diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php
@@ -283,9 +283,8 @@ public static function analyze(
         $foreach_context->inside_loop = true;
         $foreach_context->break_types[] = 'loop';
 
-        if ($codebase->alter_code) {
-            $foreach_context->branch_point =
-                $foreach_context->branch_point ?: (int) $stmt->getAttribute('startFilePos');
+        if ($codebase->alter_code && $foreach_context->branch_point === null) {
+            $foreach_context->branch_point = (int) $stmt->getAttribute('startFilePos');
         }
 
         if ($stmt->keyVar instanceof PhpParser\Node\Expr\Variable && is_string($stmt->keyVar->name)) {

diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php
@@ -292,9 +292,8 @@ public static function analyze(
         }
 
         if ($stmt->else) {
-            if ($codebase->alter_code) {
-                $else_context->branch_point =
-                    $else_context->branch_point ?: (int) $stmt->getAttribute('startFilePos');
+            if ($codebase->alter_code && $else_context->branch_point === null) {
+                $else_context->branch_point = (int) $stmt->getAttribute('startFilePos');
             }
         }