Merge pull request #6629 from orklah/loops-tweaking

tweaking with loops

src/Psalm/Internal/Analyzer/ScopeAnalyzer.php

@@ -359,6 +359,32 @@ class ScopeAnalyzer
                         return array_values(array_unique($control_actions));
                     }
                 }
+
+                if ($stmt instanceof PhpParser\Node\Stmt\For_
+                    && $nodes
+                ) {
+                    $is_infinite_loop = true;
+                    if ($stmt->cond) {
+                        foreach ($stmt->cond as $cond) {
+                            $stmt_expr_type = $nodes->getType($cond);
+                            if (!$stmt_expr_type || !$stmt_expr_type->isAlwaysTruthy()) {
+                                $is_infinite_loop = false;
+                            }
+                        }
+                    }
+
+                    if ($is_infinite_loop) {
+                        //infinite while loop that only return don't have an exit path
+                        $have_exit_path = (bool)array_diff(
+                            $control_actions,
+                            [self::ACTION_END, self::ACTION_RETURN]
+                        );
+
+                        if (!$have_exit_path) {
+                            return array_values(array_unique($control_actions));
+                        }
+                    }
+                }
             }
 
             if ($stmt instanceof PhpParser\Node\Stmt\TryCatch) {

src/Psalm/Internal/Analyzer/Statements/Block/DoAnalyzer.php

@@ -12,6 +12,7 @@ use Psalm\Internal\Scope\LoopScope;
 use Psalm\Type;
 
 use function array_filter;
+use function array_intersect_key;
 use function array_keys;
 use function array_merge;
 use function array_values;
@@ -28,7 +29,7 @@ class DoAnalyzer
         StatementsAnalyzer $statements_analyzer,
         PhpParser\Node\Stmt\Do_ $stmt,
         Context $context
-    ): void {
+    ): ?bool {
         $do_context = clone $context;
         $do_context->break_types[] = 'loop';
         $do_context->inside_loop = true;
@@ -88,20 +89,22 @@ class DoAnalyzer
             $while_clauses = [new Clause([], $cond_id, $cond_id, true)];
         }
 
-        LoopAnalyzer::analyze(
+        if (LoopAnalyzer::analyze(
             $statements_analyzer,
             $stmt->stmts,
-            [$stmt->cond],
+            WhileAnalyzer::getAndExpressions($stmt->cond),
             [],
             $loop_scope,
             $inner_loop_context,
             true,
             true
-        );
+        ) === false) {
+            return false;
+        }
 
         // because it's a do {} while, inner loop vars belong to the main context
         if (!$inner_loop_context) {
-            throw new \UnexpectedValueException('Should never be null');
+            throw new \UnexpectedValueException('There should be an inner loop context');
         }
 
         $negated_while_clauses = Algebra::negateFormula($while_clauses);
@@ -112,8 +115,6 @@ class DoAnalyzer
             )
         );
 
-        //var_dump($do_context->vars_in_scope);
-
         if ($negated_while_types) {
             $changed_var_ids = [];
 
@@ -154,14 +155,16 @@ class DoAnalyzer
             $do_context->vars_possibly_in_scope
         );
 
-        $context->referenced_var_ids = array_merge(
-            $context->referenced_var_ids,
-            $do_context->referenced_var_ids
+        $context->referenced_var_ids = array_intersect_key(
+            $do_context->referenced_var_ids,
+            $context->referenced_var_ids
         );
 
         if ($context->collect_exceptions) {
             $context->mergeExceptions($inner_loop_context);
         }
+
+        return null;
     }
 
     private static function analyzeDoNaively(

src/Psalm/Internal/Analyzer/Statements/Block/ForAnalyzer.php

@@ -78,14 +78,16 @@ class ForAnalyzer
             $context->protected_var_ids
         );
 
-        LoopAnalyzer::analyze(
+        if (LoopAnalyzer::analyze(
             $statements_analyzer,
             $stmt->stmts,
             $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');
@@ -142,7 +144,8 @@ class ForAnalyzer
         if ($always_enters_loop && $can_leave_loop) {
             foreach ($inner_loop_context->vars_in_scope as $var_id => $type) {
                 // if there are break statements in the loop it's not certain
-                // that the loop has finished executing
+                // that the loop has finished executing, so the assertions at the end
+                // the loop in the while conditional may not hold
                 if (in_array(ScopeAnalyzer::ACTION_BREAK, $loop_scope->final_actions, true)
                     || in_array(ScopeAnalyzer::ACTION_CONTINUE, $loop_scope->final_actions, true)
                 ) {

src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php

@@ -309,7 +309,7 @@ class ForeachAnalyzer
 
         $loop_scope->protected_var_ids = $context->protected_var_ids;
 
-        LoopAnalyzer::analyze(
+        if (LoopAnalyzer::analyze(
             $statements_analyzer,
             $stmt->stmts,
             [],
@@ -318,7 +318,9 @@ class ForeachAnalyzer
             $inner_loop_context,
             false,
             $always_non_empty_array
-        );
+        ) === false) {
+            return false;
+        }
 
         if (!$inner_loop_context) {
             throw new \UnexpectedValueException('There should be an inner loop context');

src/Psalm/Internal/Analyzer/Statements/Block/WhileAnalyzer.php

@@ -8,6 +8,7 @@ use Psalm\Internal\Analyzer\StatementsAnalyzer;
 use Psalm\Internal\Scope\LoopScope;
 use Psalm\Type;
 
+use function array_intersect_key;
 use function array_merge;
 use function in_array;
 
@@ -59,48 +60,17 @@ class WhileAnalyzer
         }
 
         if (!$inner_loop_context) {
-            throw new \UnexpectedValueException('Should always enter loop');
+            throw new \UnexpectedValueException('There should be an inner loop context');
         }
 
         $always_enters_loop = false;
 
         if ($stmt_cond_type = $statements_analyzer->node_data->getType($stmt->cond)) {
+            $always_enters_loop = $stmt_cond_type->isAlwaysTruthy();
+        }
+
+        if ($while_true) {
             $always_enters_loop = true;
-
-            foreach ($stmt_cond_type->getAtomicTypes() as $iterator_type) {
-                if ($iterator_type instanceof Type\Atomic\TArray
-                    || $iterator_type instanceof Type\Atomic\TKeyedArray
-                ) {
-                    if ($iterator_type instanceof Type\Atomic\TKeyedArray) {
-                        if (!$iterator_type->sealed) {
-                            $always_enters_loop = false;
-                        }
-                    } elseif (!$iterator_type instanceof Type\Atomic\TNonEmptyArray) {
-                        $always_enters_loop = false;
-                    }
-
-                    continue;
-                }
-
-                if ($iterator_type instanceof Type\Atomic\TTrue) {
-                    continue;
-                }
-
-                if ($iterator_type instanceof Type\Atomic\TLiteralString
-                    && $iterator_type->value
-                ) {
-                    continue;
-                }
-
-                if ($iterator_type instanceof Type\Atomic\TLiteralInt
-                    && $iterator_type->value
-                ) {
-                    continue;
-                }
-
-                $always_enters_loop = false;
-                break;
-            }
         }
 
         $can_leave_loop = !$while_true
@@ -137,18 +107,22 @@ class WhileAnalyzer
             $context->vars_possibly_in_scope = $pre_context->vars_possibly_in_scope;
         }
 
-        $context->referenced_var_ids = array_merge(
-            $context->referenced_var_ids,
-            $while_context->referenced_var_ids
+        $context->referenced_var_ids = array_intersect_key(
+            $while_context->referenced_var_ids,
+            $context->referenced_var_ids
         );
 
+        if ($context->collect_exceptions) {
+            $context->mergeExceptions($while_context);
+        }
+
         return null;
     }
 
     /**
      * @return list<PhpParser\Node\Expr>
      */
-    private static function getAndExpressions(
+    public static function getAndExpressions(
         PhpParser\Node\Expr $expr
     ) : array {
         if ($expr instanceof PhpParser\Node\Expr\BinaryOp\BooleanAnd) {

src/Psalm/Internal/Analyzer/StatementsAnalyzer.php

@@ -509,7 +509,9 @@ class StatementsAnalyzer extends SourceAnalyzer
                 return false;
             }
         } elseif ($stmt instanceof PhpParser\Node\Stmt\Do_) {
-            DoAnalyzer::analyze($statements_analyzer, $stmt, $context);
+            if (DoAnalyzer::analyze($statements_analyzer, $stmt, $context) === false) {
+                return false;
+            }
         } elseif ($stmt instanceof PhpParser\Node\Stmt\Const_) {
             ConstFetchAnalyzer::analyzeConstAssignment($statements_analyzer, $stmt, $context);
         } elseif ($stmt instanceof PhpParser\Node\Stmt\Unset_) {

tests/Loop/ForTest.php

@@ -156,6 +156,26 @@ class ForTest extends \Psalm\Tests\TestCase
                         return $data;
                     }'
             ],
+            'InfiniteForLoop' => [
+                '<?php
+                    /**
+                     * @return int
+                     */
+                    function g() {
+                        for (;;) {
+                            return 1;
+                        }
+                    }
+
+                    /**
+                     * @return int
+                     */
+                    function h() {
+                        for (;1;) {
+                            return 1;
+                        }
+                    }'
+            ],
         ];
     }