diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 987aab124ad..901547ac44f 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1,5 +1,5 @@ - + tags['variablesfrom'][0]]]> @@ -297,18 +297,10 @@ - - - branch_point]]> - - cond]]> - - branch_point]]> - @@ -319,7 +311,6 @@ var_id]]> var_id]]> - branch_point]]> template_types]]> getTemplateTypeMap()]]> line_number]]> @@ -350,7 +341,6 @@ branch_point]]> branch_point]]> - branch_point]]> assigned_var_ids]]> new_vars]]> redefined_vars]]> @@ -378,7 +368,6 @@ traverse([$switch_condition])[0]]]> - branch_point]]> @@ -386,16 +375,6 @@ - - - branch_point]]> - - - - - branch_point]]> - - diff --git a/src/Psalm/Context.php b/src/Psalm/Context.php index bf73229560a..1385eba16b2 100644 --- a/src/Psalm/Context.php +++ b/src/Psalm/Context.php @@ -314,7 +314,7 @@ final class Context public $is_global = false; /** - * @var array + * @var array */ public $protected_var_ids = []; diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/DoAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/DoAnalyzer.php index 234f8bab45e..c3fb9c82b64 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/DoAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/DoAnalyzer.php @@ -7,11 +7,9 @@ use Psalm\Context; use Psalm\Internal\Algebra; use Psalm\Internal\Algebra\FormulaGenerator; -use Psalm\Internal\Analyzer\ScopeAnalyzer; use Psalm\Internal\Analyzer\StatementsAnalyzer; use Psalm\Internal\Clause; use Psalm\Internal\Scope\LoopScope; -use Psalm\Type; use Psalm\Type\Reconciler; use UnexpectedValueException; @@ -41,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); @@ -138,21 +136,7 @@ static function (Clause $c) use ($mixed_var_ids): bool { ); } - 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, 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)) { - if (isset($loop_scope->possibly_defined_loop_parent_vars[$var_id])) { - $context->vars_in_scope[$var_id] = Type::combineUnionTypes( - $type, - $loop_scope->possibly_defined_loop_parent_vars[$var_id], - ); - } - } else { - $context->vars_in_scope[$var_id] = $type; - } - } + LoopAnalyzer::setLoopVars($inner_loop_context, $context, $loop_scope); $do_context->loop_scope = null; diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/ForAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/ForAnalyzer.php index dbad2cd129a..dcf44f244b5 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/ForAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/ForAnalyzer.php @@ -4,16 +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 Psalm\Type; -use UnexpectedValueException; use function array_merge; -use function count; -use function in_array; use function is_string; /** @@ -57,129 +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) { - 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, 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) - ) { - if (isset($loop_scope->possibly_defined_loop_parent_vars[$var_id])) { - $context->vars_in_scope[$var_id] = Type::combineUnionTypes( - $type, - $loop_scope->possibly_defined_loop_parent_vars[$var_id], - ); - } - } else { - $context->vars_in_scope[$var_id] = $type; - } - } - } - - $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 index 19099c9c247..7bd6f6cbd87 100644 --- 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 index 99ef6c45291..19f91dfecf4 100644 --- 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'); } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php index 7d36fd3399b..97f27bdf650 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php @@ -18,11 +18,15 @@ use Psalm\IssueBuffer; use Psalm\Type; use Psalm\Type\Reconciler; +use Psalm\Type\Union; +use UnexpectedValueException; use function array_keys; use function array_merge; use function array_unique; +use function count; use function in_array; +use function is_string; use function spl_object_id; /** @@ -137,10 +141,7 @@ public static function analyze( } } - $loop_parent_context->vars_possibly_in_scope = array_merge( - $continue_context->vars_possibly_in_scope, - $loop_parent_context->vars_possibly_in_scope, - ); + $loop_parent_context->vars_possibly_in_scope += $continue_context->vars_possibly_in_scope; } else { $original_parent_context = clone $loop_parent_context; @@ -268,10 +269,7 @@ public static function analyze( $continue_context->has_returned = false; - $loop_parent_context->vars_possibly_in_scope = array_merge( - $continue_context->vars_possibly_in_scope, - $loop_parent_context->vars_possibly_in_scope, - ); + $loop_parent_context->vars_possibly_in_scope += $continue_context->vars_possibly_in_scope; // if there are no changes to the types, no need to re-examine if (!$has_changes) { @@ -490,23 +488,7 @@ public static function analyze( } if ($always_enters_loop) { - foreach ($continue_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, 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) - ) { - if (isset($loop_scope->possibly_defined_loop_parent_vars[$var_id])) { - $loop_parent_context->vars_in_scope[$var_id] = Type::combineUnionTypes( - $type, - $loop_scope->possibly_defined_loop_parent_vars[$var_id], - ); - } - } else { - $loop_parent_context->vars_in_scope[$var_id] = $type; - } - } + self::setLoopVars($continue_context, $loop_parent_context, $loop_scope); } if ($inner_do_context) { @@ -522,6 +504,168 @@ public static function analyze( return null; } + /** + * @param PhpParser\Node\Stmt\For_|PhpParser\Node\Stmt\While_ $stmt + * @param array $init_var_types + * @param array $assigned_var_ids + * @param list $pre_conditions + * @param PhpParser\Node\Expr[] $post_expressions + * @return false|null + */ + public static function analyzeForOrWhile( + StatementsAnalyzer $statements_analyzer, + $stmt, + Context $context, + bool $while_true, + array $init_var_types, + array $assigned_var_ids, + array $pre_conditions, + array $post_expressions + ): ?bool { + $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 === null) { + $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 (self::analyze( + $statements_analyzer, + $stmt->stmts, + $pre_conditions, + $post_expressions, + $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 = $while_true || self::doesEnterLoop($statements_analyzer, $stmt, $init_var_types); + + $can_leave_loop = !$while_true + || in_array(ScopeAnalyzer::ACTION_BREAK, $loop_scope->final_actions, true); + + if ($always_enters_loop && $can_leave_loop) { + self::setLoopVars($inner_loop_context, $context, $loop_scope); + } + + $for_context->loop_scope = null; + + if ($can_leave_loop) { + $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; + } + + public static function setLoopVars(Context $inner_context, Context $context, LoopScope $loop_scope): void + { + foreach ($inner_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, 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) + ) { + if (isset($loop_scope->possibly_defined_loop_parent_vars[$var_id])) { + $context->vars_in_scope[$var_id] = Type::combineUnionTypes( + $type, + $loop_scope->possibly_defined_loop_parent_vars[$var_id], + ); + } + } else { + $context->vars_in_scope[$var_id] = $type; + } + } + } + + /** + * @param PhpParser\Node\Stmt\For_|PhpParser\Node\Stmt\While_ $stmt + * @param array $init_var_types + */ + private static function doesEnterLoop( + StatementsAnalyzer $statements_analyzer, + $stmt, + array $init_var_types + ): bool { + $always_enters_loop = false; + + if ($stmt instanceof PhpParser\Node\Stmt\While_) { + if ($cond_type = $statements_analyzer->node_data->getType($stmt->cond)) { + $always_enters_loop = $cond_type->isAlwaysTruthy(); + } + } else { + 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; + } + } + } + } + + return $always_enters_loop; + } + private static function updateLoopScopeContexts( LoopScope $loop_scope, Context $loop_context, diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/SwitchCaseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/SwitchCaseAnalyzer.php index 54b7e7f3f01..d9c06e3551b 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/SwitchCaseAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/SwitchCaseAnalyzer.php @@ -78,8 +78,8 @@ public static function analyze( $case_context = clone $original_context; - if ($codebase->alter_code) { - $case_context->branch_point = $case_context->branch_point ?: (int) $stmt->getAttribute('startFilePos'); + if ($codebase->alter_code && $case_context->branch_point === null) { + $case_context->branch_point = (int) $stmt->getAttribute('startFilePos'); } $case_scope = $case_context->case_scope = new CaseScope($case_context); diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php index fab72f60413..b09a8837156 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php @@ -65,8 +65,8 @@ public static function analyze( $try_context = clone $context; - if ($codebase->alter_code) { - $try_context->branch_point = $try_context->branch_point ?: (int) $stmt->getAttribute('startFilePos'); + if ($codebase->alter_code && $try_context->branch_point === null) { + $try_context->branch_point = (int) $stmt->getAttribute('startFilePos'); } if ($stmt->finally) { diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/WhileAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/WhileAnalyzer.php index 6eea413c4ce..c498e3c3cee 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/WhileAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/WhileAnalyzer.php @@ -4,14 +4,7 @@ use PhpParser; use Psalm\Context; -use Psalm\Internal\Analyzer\ScopeAnalyzer; use Psalm\Internal\Analyzer\StatementsAnalyzer; -use Psalm\Internal\Scope\LoopScope; -use Psalm\Type; -use UnexpectedValueException; - -use function array_merge; -use function in_array; /** * @internal @@ -31,90 +24,16 @@ public static function analyze( || (($t = $statements_analyzer->node_data->getType($stmt->cond)) && $t->isAlwaysTruthy()); - $pre_context = null; - - if ($while_true) { - $pre_context = clone $context; - } - - $while_context = clone $context; - - $while_context->inside_loop = true; - $while_context->break_types[] = 'loop'; - - $codebase = $statements_analyzer->getCodebase(); - - if ($codebase->alter_code) { - $while_context->branch_point = $while_context->branch_point ?: (int) $stmt->getAttribute('startFilePos'); - } - - $loop_scope = new LoopScope($while_context, $context); - $loop_scope->protected_var_ids = $context->protected_var_ids; - - if (LoopAnalyzer::analyze( + return LoopAnalyzer::analyzeForOrWhile( $statements_analyzer, - $stmt->stmts, + $stmt, + $context, + $while_true, + [], + [], self::getAndExpressions($stmt->cond), [], - $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; - - 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; - } - - $can_leave_loop = !$while_true - || in_array(ScopeAnalyzer::ACTION_BREAK, $loop_scope->final_actions, true); - - 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, 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) - ) { - if (isset($loop_scope->possibly_defined_loop_parent_vars[$var_id])) { - $context->vars_in_scope[$var_id] = Type::combineUnionTypes( - $type, - $loop_scope->possibly_defined_loop_parent_vars[$var_id], - ); - } - } else { - $context->vars_in_scope[$var_id] = $type; - } - } - } - - $while_context->loop_scope = null; - - if ($can_leave_loop) { - $context->vars_possibly_in_scope = array_merge( - $context->vars_possibly_in_scope, - $while_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($while_context); - } - - return null; + ); } /** diff --git a/src/Psalm/Internal/Scope/LoopScope.php b/src/Psalm/Internal/Scope/LoopScope.php index 76b059329f1..ba2f30e498d 100644 --- a/src/Psalm/Internal/Scope/LoopScope.php +++ b/src/Psalm/Internal/Scope/LoopScope.php @@ -42,7 +42,7 @@ final class LoopScope public array $vars_possibly_in_scope = []; /** - * @var array + * @var array */ public array $protected_var_ids = [];