From 2a7b48ce5f6f8c27e8ae6e4a901fba14cefcea72 Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Wed, 6 Dec 2017 23:46:41 -0500 Subject: [PATCH] Add support for infinite loops Fixes #381 --- .../Checker/Statements/Block/ForChecker.php | 40 ++++++++++++++-- .../Checker/Statements/Block/WhileChecker.php | 46 +++++++++++++++++-- tests/LoopScopeTest.php | 38 +++++++++++++++ 3 files changed, 114 insertions(+), 10 deletions(-) diff --git a/src/Psalm/Checker/Statements/Block/ForChecker.php b/src/Psalm/Checker/Statements/Block/ForChecker.php index f666c39b9..896541933 100644 --- a/src/Psalm/Checker/Statements/Block/ForChecker.php +++ b/src/Psalm/Checker/Statements/Block/ForChecker.php @@ -2,6 +2,7 @@ namespace Psalm\Checker\Statements\Block; use PhpParser; +use Psalm\Checker\ScopeChecker; use Psalm\Checker\Statements\ExpressionChecker; use Psalm\Checker\StatementsChecker; use Psalm\Context; @@ -27,21 +28,50 @@ class ForChecker } } + $while_true = !$stmt->cond && !$stmt->init && !$stmt->loop; + + $pre_context = $while_true ? clone $context : null; + $for_context = clone $context; $for_context->inside_loop = true; + $loop_scope = new LoopScope($for_context, $context); + LoopChecker::analyze( $statements_checker, $stmt->stmts, $stmt->cond, $stmt->loop, - new LoopScope($for_context, $context) + $loop_scope, + $inner_loop_context ); - $context->vars_possibly_in_scope = array_merge( - $for_context->vars_possibly_in_scope, - $context->vars_possibly_in_scope - ); + if ($inner_loop_context && $while_true) { + // if we actually leave the loop + if (in_array(ScopeChecker::ACTION_BREAK, $loop_scope->final_actions, true) + || in_array(ScopeChecker::ACTION_END, $loop_scope->final_actions, true) + ) { + foreach ($inner_loop_context->vars_in_scope as $var_id => $type) { + if (!isset($context->vars_in_scope[$var_id])) { + $context->vars_in_scope[$var_id] = $type; + } + } + } + } + + if (!$while_true + || in_array(ScopeChecker::ACTION_BREAK, $loop_scope->final_actions, true) + || in_array(ScopeChecker::ACTION_END, $loop_scope->final_actions, true) + || !$pre_context + ) { + $context->vars_possibly_in_scope = array_merge( + $context->vars_possibly_in_scope, + $for_context->vars_possibly_in_scope + ); + } else { + $context->vars_in_scope = $pre_context->vars_in_scope; + $context->vars_possibly_in_scope = $pre_context->vars_possibly_in_scope; + } $context->referenced_var_ids = array_merge( $for_context->referenced_var_ids, diff --git a/src/Psalm/Checker/Statements/Block/WhileChecker.php b/src/Psalm/Checker/Statements/Block/WhileChecker.php index 49e43b2dd..be7996442 100644 --- a/src/Psalm/Checker/Statements/Block/WhileChecker.php +++ b/src/Psalm/Checker/Statements/Block/WhileChecker.php @@ -2,6 +2,7 @@ namespace Psalm\Checker\Statements\Block; use PhpParser; +use Psalm\Checker\ScopeChecker; use Psalm\Checker\StatementsChecker; use Psalm\Context; use Psalm\Scope\LoopScope; @@ -20,20 +21,55 @@ class WhileChecker PhpParser\Node\Stmt\While_ $stmt, Context $context ) { + $while_true = $stmt->cond + && ($stmt->cond instanceof PhpParser\Node\Expr\ConstFetch && $stmt->cond->name->parts === ['true']) + || ($stmt->cond instanceof PhpParser\Node\Scalar\LNumber && $stmt->cond->value > 0); + + $pre_context = null; + + if ($while_true) { + $pre_context = clone $context; + } + $while_context = clone $context; + $loop_scope = new LoopScope($while_context, $context); + LoopChecker::analyze( $statements_checker, $stmt->stmts, $stmt->cond ? [$stmt->cond] : [], [], - new LoopScope($while_context, $context) + $loop_scope, + $inner_loop_context ); - $context->vars_possibly_in_scope = array_merge( - $context->vars_possibly_in_scope, - $while_context->vars_possibly_in_scope - ); + if ($inner_loop_context && $while_true) { + // if we actually leave the loop + if (in_array(ScopeChecker::ACTION_BREAK, $loop_scope->final_actions, true) + || in_array(ScopeChecker::ACTION_END, $loop_scope->final_actions, true) + ) { + foreach ($inner_loop_context->vars_in_scope as $var_id => $type) { + if (!isset($context->vars_in_scope[$var_id])) { + $context->vars_in_scope[$var_id] = $type; + } + } + } + } + + if (!$while_true + || in_array(ScopeChecker::ACTION_BREAK, $loop_scope->final_actions, true) + || in_array(ScopeChecker::ACTION_END, $loop_scope->final_actions, true) + || !$pre_context + ) { + $context->vars_possibly_in_scope = array_merge( + $context->vars_possibly_in_scope, + $while_context->vars_possibly_in_scope + ); + } else { + $context->vars_in_scope = $pre_context->vars_in_scope; + $context->vars_possibly_in_scope = $pre_context->vars_possibly_in_scope; + } $context->referenced_var_ids = array_merge( $context->referenced_var_ids, diff --git a/tests/LoopScopeTest.php b/tests/LoopScopeTest.php index e23714d9b..24ef91195 100644 --- a/tests/LoopScopeTest.php +++ b/tests/LoopScopeTest.php @@ -764,6 +764,26 @@ class LoopScopeTest extends TestCase 'MixedAssignment', 'MixedArrayAccess', ], ], + 'whileTrue' => [ + ' [ + '$a' => 'string', + '$b' => 'int', + '$c' => 'bool', + ], + ], ]; } @@ -898,6 +918,24 @@ class LoopScopeTest extends TestCase }', 'error_message' => 'RedundantCondition', ], + 'whileTrueNoBreak' => [ + ' 'UndefinedGlobalVariable', + ], + 'forInfiniteNoBreak' => [ + ' 'UndefinedGlobalVariable', + ], ]; } }