From 92bddb95a59b47e068417a83ac7b3b3bb9132bf8 Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Tue, 25 Jan 2022 10:59:46 -0500 Subject: [PATCH 1/9] Remove mic-drop hack from if analysis --- .../Statements/Block/IfElse/ElseAnalyzer.php | 18 +-- .../Statements/Block/IfElse/IfAnalyzer.php | 118 +----------------- .../Statements/Block/IfElseAnalyzer.php | 48 +++++++ .../Expression/AssignmentAnalyzer.php | 27 +++- .../Expression/IncDecExpressionAnalyzer.php | 2 +- .../Statements/ExpressionAnalyzer.php | 4 +- .../Internal/Codebase/VariableUseGraph.php | 1 + src/Psalm/Internal/Scope/IfScope.php | 5 + tests/Loop/DoTest.php | 1 + tests/TypeReconciliation/ConditionalTest.php | 1 + .../RedundantConditionTest.php | 13 ++ tests/TypeReconciliation/TypeAlgebraTest.php | 6 +- tests/UnusedVariableTest.php | 36 ++++-- 13 files changed, 129 insertions(+), 151 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseAnalyzer.php index 4d878054555..d957d3c8293 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseAnalyzer.php @@ -59,16 +59,6 @@ public static function analyze( $else_types = Algebra::getTruthsFromFormula($else_context->clauses); - if (!$else && !$else_types) { - $if_scope->final_actions = array_merge([ScopeAnalyzer::ACTION_NONE], $if_scope->final_actions); - $if_scope->assigned_var_ids = []; - $if_scope->new_vars = []; - $if_scope->redefined_vars = []; - $if_scope->reasonable_clauses = []; - - return null; - } - $original_context = clone $else_context; if ($else_types) { @@ -120,10 +110,10 @@ public static function analyze( ) { return false; } + } - foreach ($else_context->parent_remove_vars as $var_id => $_) { - $outer_context->removeVarFromConflictingClauses($var_id); - } + foreach ($else_context->parent_remove_vars as $var_id => $_) { + $outer_context->removeVarFromConflictingClauses($var_id); } /** @var array */ @@ -185,7 +175,7 @@ public static function analyze( $new_assigned_var_ids, $new_possibly_assigned_var_ids, [], - (bool) $else + true ); $if_scope->reasonable_clauses = []; diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php index fda3b851617..7c9dfcac145 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php @@ -6,12 +6,9 @@ use Psalm\CodeLocation; use Psalm\Codebase; use Psalm\Context; -use Psalm\Internal\Algebra; -use Psalm\Internal\Analyzer\FunctionLikeAnalyzer; use Psalm\Internal\Analyzer\ScopeAnalyzer; use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer; use Psalm\Internal\Analyzer\StatementsAnalyzer; -use Psalm\Internal\Analyzer\TraitAnalyzer; use Psalm\Internal\Scope\IfConditionalScope; use Psalm\Internal\Scope\IfScope; use Psalm\Internal\Type\Comparator\UnionTypeComparator; @@ -23,7 +20,6 @@ use Psalm\Node\Name\VirtualFullyQualified; use Psalm\Node\VirtualArg; use Psalm\Type; -use Psalm\Type\Reconciler; use Psalm\Type\Union; use function array_diff_key; @@ -77,7 +73,7 @@ public static function analyze( $outer_context->removeVarFromConflictingClauses($var_id); } - $final_actions = ScopeAnalyzer::getControlActions( + $if_scope->if_actions = $final_actions = ScopeAnalyzer::getControlActions( $stmt->stmts, $statements_analyzer->node_data, [] @@ -91,6 +87,7 @@ public static function analyze( $has_break_statement = $final_actions === [ScopeAnalyzer::ACTION_BREAK]; $has_continue_statement = $final_actions === [ScopeAnalyzer::ACTION_CONTINUE]; + $if_scope->if_actions = $final_actions; $if_scope->final_actions = $final_actions; /** @var array */ @@ -128,8 +125,6 @@ public static function analyze( } } - $mic_drop = false; - if (!$has_leaving_statements) { self::updateIfScope( $codebase, @@ -168,27 +163,13 @@ public static function analyze( $if_conditional_scope->assigned_in_conditional_var_ids ); } - - if (!$stmt->else && !$stmt->elseifs) { - $mic_drop = self::handleMicDrop( - $statements_analyzer, - $stmt->cond, - $if_scope, - $outer_context, - $new_assigned_var_ids - ); - - $outer_context->clauses = Algebra::simplifyCNF( - array_merge($outer_context->clauses, $if_scope->negated_clauses) - ); - } } } // update the parent context as necessary, but only if we can safely reason about type negation. // We only update vars that changed both at the start of the if block and then again by an assignment // in the if statement. - if ($if_scope->negated_types && !$mic_drop) { + if ($if_scope->negated_types) { $vars_to_update = array_intersect( array_keys($pre_assignment_else_redefined_vars), array_keys($if_scope->negated_types) @@ -263,99 +244,6 @@ public static function analyze( return null; } - /** - * This handles the situation when returning inside an - * if block with no else or elseifs - * - * @param array $new_assigned_var_ids - */ - private static function handleMicDrop( - StatementsAnalyzer $statements_analyzer, - PhpParser\Node\Expr $cond, - IfScope $if_scope, - Context $post_if_context, - array $new_assigned_var_ids - ): bool { - if (!$if_scope->negated_types) { - return false; - } - - $newly_reconciled_var_ids = []; - - $post_if_context_vars_reconciled = Reconciler::reconcileKeyedTypes( - $if_scope->negated_types, - [], - $post_if_context->vars_in_scope, - $post_if_context->references_in_scope, - $newly_reconciled_var_ids, - [], - $statements_analyzer, - $statements_analyzer->getTemplateTypeMap() ?: [], - $post_if_context->inside_loop, - new CodeLocation( - $statements_analyzer->getSource(), - $cond instanceof PhpParser\Node\Expr\BooleanNot - ? $cond->expr - : $cond, - $post_if_context->include_location, - false - ) - ); - - foreach ($newly_reconciled_var_ids as $changed_var_id => $_) { - $post_if_context->removeVarFromConflictingClauses($changed_var_id); - } - - $newly_reconciled_var_ids += $new_assigned_var_ids; - - foreach ($newly_reconciled_var_ids as $var_id => $_) { - $if_scope->negated_clauses = Context::filterClauses( - $var_id, - $if_scope->negated_clauses - ); - } - - foreach ($newly_reconciled_var_ids as $var_id => $_) { - $first_appearance = $statements_analyzer->getFirstAppearance($var_id); - - if ($first_appearance - && isset($post_if_context->vars_in_scope[$var_id]) - && isset($post_if_context_vars_reconciled[$var_id]) - && $post_if_context->vars_in_scope[$var_id]->hasMixed() - && !$post_if_context_vars_reconciled[$var_id]->hasMixed() - ) { - if (!$post_if_context->collect_initializations - && !$post_if_context->collect_mutations - && $statements_analyzer->getFilePath() === $statements_analyzer->getRootFilePath() - ) { - $parent_source = $statements_analyzer->getSource(); - - $functionlike_storage = $parent_source instanceof FunctionLikeAnalyzer - ? $parent_source->getFunctionLikeStorage($statements_analyzer) - : null; - - if (!$functionlike_storage - || (!$parent_source->getSource() instanceof TraitAnalyzer - && !isset($functionlike_storage->param_lookup[substr($var_id, 1)])) - ) { - $codebase = $statements_analyzer->getCodebase(); - $codebase->analyzer->decrementMixedCount($statements_analyzer->getFilePath()); - } - } - - IssueBuffer::remove( - $statements_analyzer->getFilePath(), - 'MixedAssignment', - $first_appearance->raw_file_start - ); - } - } - - $post_if_context->vars_in_scope = $post_if_context_vars_reconciled; - - return true; - } - /** * @param array $assigned_in_conditional_var_ids */ diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php index 316b91a8961..370c17466d7 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php @@ -10,13 +10,16 @@ use Psalm\Internal\Algebra; use Psalm\Internal\Algebra\FormulaGenerator; use Psalm\Internal\Analyzer\AlgebraAnalyzer; +use Psalm\Internal\Analyzer\FunctionLikeAnalyzer; use Psalm\Internal\Analyzer\ScopeAnalyzer; use Psalm\Internal\Analyzer\Statements\Block\IfElse\ElseAnalyzer; use Psalm\Internal\Analyzer\Statements\Block\IfElse\ElseIfAnalyzer; use Psalm\Internal\Analyzer\Statements\Block\IfElse\IfAnalyzer; use Psalm\Internal\Analyzer\StatementsAnalyzer; +use Psalm\Internal\Analyzer\TraitAnalyzer; use Psalm\Internal\Clause; use Psalm\Internal\Scope\IfScope; +use Psalm\IssueBuffer; use Psalm\Node\Expr\VirtualBooleanNot; use Psalm\Type; use Psalm\Type\Reconciler; @@ -38,6 +41,7 @@ use function preg_match; use function preg_quote; use function spl_object_id; +use function substr; /** * @internal @@ -405,6 +409,50 @@ function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause { return false; } + if (count($if_scope->if_actions) && !in_array(ScopeAnalyzer::ACTION_NONE, $if_scope->if_actions, true) + && !$stmt->elseifs + ) { + $context->clauses = $else_context->clauses; + foreach ($else_context->vars_in_scope as $var_id => $type) { + $context->vars_in_scope[$var_id] = clone $type; + } + + foreach ($pre_assignment_else_redefined_vars as $var_id => $reconciled_type) { + $first_appearance = $statements_analyzer->getFirstAppearance($var_id); + + if ($first_appearance + && isset($post_if_context->vars_in_scope[$var_id]) + && $post_if_context->vars_in_scope[$var_id]->hasMixed() + && !$reconciled_type->hasMixed() + ) { + if (!$post_if_context->collect_initializations + && !$post_if_context->collect_mutations + && $statements_analyzer->getFilePath() === $statements_analyzer->getRootFilePath() + ) { + $parent_source = $statements_analyzer->getSource(); + + $functionlike_storage = $parent_source instanceof FunctionLikeAnalyzer + ? $parent_source->getFunctionLikeStorage($statements_analyzer) + : null; + + if (!$functionlike_storage + || (!$parent_source->getSource() instanceof TraitAnalyzer + && !isset($functionlike_storage->param_lookup[substr($var_id, 1)])) + ) { + $codebase = $statements_analyzer->getCodebase(); + $codebase->analyzer->decrementMixedCount($statements_analyzer->getFilePath()); + } + } + + IssueBuffer::remove( + $statements_analyzer->getFilePath(), + 'MixedAssignment', + $first_appearance->raw_file_start + ); + } + } + } + if ($context->loop_scope) { $context->loop_scope->final_actions = array_unique( array_merge( diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php index 797de49243d..b3daa26555c 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php @@ -112,7 +112,8 @@ public static function analyze( ?Union $assign_value_type, Context $context, ?PhpParser\Comment\Doc $doc_comment, - array $not_ignored_docblock_var_ids = [] + array $not_ignored_docblock_var_ids = [], + ?PhpParser\Node\Expr $assign_expr = null ) { $var_id = ExpressionIdentifier::getVarId( $assign_var, @@ -616,6 +617,26 @@ public static function analyze( $added_taints ); } + + if ($assign_expr) { + $new_parent_node = DataFlowNode::getForAssignment( + 'assignment_expr', + new CodeLocation($statements_analyzer->getSource(), $assign_expr) + ); + + $data_flow_graph->addNode($new_parent_node); + + foreach ($context->vars_in_scope[$var_id]->parent_nodes as $old_parent_node) { + $data_flow_graph->addPath( + $old_parent_node, + $new_parent_node, + '=', + ); + } + + $assign_value_type = clone $assign_value_type; + $assign_value_type->parent_nodes = [$new_parent_node->id => $new_parent_node]; + } } } } @@ -1083,7 +1104,7 @@ public static function assignByRefParam( $by_ref_out_type->parent_nodes += $existing_type->parent_nodes; } - if (!$existing_type->isEmptyArray()) { + if (!$context->inside_conditional) { $context->vars_in_scope[$var_id] = $by_ref_out_type; if (!($stmt_type = $statements_analyzer->node_data->getType($stmt)) @@ -1505,7 +1526,7 @@ private static function analyzeDestructuringAssignment( $assignment_node->id => $assignment_node ]; } else { - if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph + if ($data_flow_graph instanceof TaintFlowGraph && in_array('TaintedInput', $statements_analyzer->getSuppressedIssues()) ) { $context->vars_in_scope[$list_var_id]->parent_nodes = []; diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/IncDecExpressionAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/IncDecExpressionAnalyzer.php index 4ad506e9207..aff24fec7c1 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/IncDecExpressionAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/IncDecExpressionAnalyzer.php @@ -129,7 +129,7 @@ public static function analyze( if ($stmt instanceof PreInc || $stmt instanceof PreDec) { $old_node_data->setType( $stmt, - $statements_analyzer->node_data->getType($operation) ?? Type::getMixed() + $statements_analyzer->node_data->getType($fake_assignment) ?? Type::getMixed() ); } diff --git a/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php index a33d2e3c973..0d3c64c542f 100644 --- a/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php @@ -180,7 +180,9 @@ private static function handleExpression( $stmt->expr, null, $context, - $stmt->getDocComment() + $stmt->getDocComment(), + [], + !$from_stmt ? $stmt : null ); if ($assignment_type === false) { diff --git a/src/Psalm/Internal/Codebase/VariableUseGraph.php b/src/Psalm/Internal/Codebase/VariableUseGraph.php index 2e8c192e180..d07772d9bad 100644 --- a/src/Psalm/Internal/Codebase/VariableUseGraph.php +++ b/src/Psalm/Internal/Codebase/VariableUseGraph.php @@ -160,6 +160,7 @@ private function getChildNodes( || $path->type === 'use-inside-conditional' || $path->type === 'use-inside-isset' || $path->type === 'arg' + || $path->type === 'comparison' ) { return null; } diff --git a/src/Psalm/Internal/Scope/IfScope.php b/src/Psalm/Internal/Scope/IfScope.php index 93de1e9359b..696e405ee10 100644 --- a/src/Psalm/Internal/Scope/IfScope.php +++ b/src/Psalm/Internal/Scope/IfScope.php @@ -76,6 +76,11 @@ class IfScope */ public $reasonable_clauses = []; + /** + * @var string[] + */ + public $if_actions = []; + /** * @var string[] */ diff --git a/tests/Loop/DoTest.php b/tests/Loop/DoTest.php index 35fbbc0f7df..45f6056ce12 100644 --- a/tests/Loop/DoTest.php +++ b/tests/Loop/DoTest.php @@ -121,6 +121,7 @@ function foo(string $b) {} break; } + /** @psalm-suppress MixedArgument */ foo($a); } while (rand(0,100) === 10);', diff --git a/tests/TypeReconciliation/ConditionalTest.php b/tests/TypeReconciliation/ConditionalTest.php index 733743da3e7..ff02c810be4 100644 --- a/tests/TypeReconciliation/ConditionalTest.php +++ b/tests/TypeReconciliation/ConditionalTest.php @@ -2083,6 +2083,7 @@ class C { /** * @psalm-suppress MixedReturnStatement * @psalm-suppress MixedInferredReturnType + * @psalm-suppress MixedArrayAccess */ public static function get(string $k1, string $k2) : ?string { if (!isset(static::$cache[$k1][$k2])) { diff --git a/tests/TypeReconciliation/RedundantConditionTest.php b/tests/TypeReconciliation/RedundantConditionTest.php index 919b747eeb2..1038e354c87 100644 --- a/tests/TypeReconciliation/RedundantConditionTest.php +++ b/tests/TypeReconciliation/RedundantConditionTest.php @@ -1527,6 +1527,19 @@ function f(array $p) : void { }', 'error_message' => 'RedundantCondition' ], + 'secondFalsyTwiceWithoutChangeWithElse' => [ + 'code' => ' 'RedundantCondition' + ], ]; } } diff --git a/tests/TypeReconciliation/TypeAlgebraTest.php b/tests/TypeReconciliation/TypeAlgebraTest.php index ef44446565b..818fd90c858 100644 --- a/tests/TypeReconciliation/TypeAlgebraTest.php +++ b/tests/TypeReconciliation/TypeAlgebraTest.php @@ -1194,8 +1194,6 @@ function foo(?string $a, ?string $b, ?string $c): void { ], 'threeVarLogicWithException' => [ 'code' => ' 'PossiblyNullArgument', + 'error_message' => 'RedundantCondition', ], 'invertedTwoVarLogicNotNestedWithVarChange' => [ 'code' => ' [ - 'code' => ' 10) { - break; - } else {} - } - }', - ], 'referenceUseUsesReferencedVariable' => [ 'code' => ' [ + 'code' => ' 10) { + break; + } else {} + } + }', + ], + 'usedPlusInUnaryAddition' => [ + 'code' => ' 10) { + break; + } else {} + } + }', + ], ]; } From ab0bec817f74a7a724b4cd4e36a17a7112139271 Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Tue, 25 Jan 2022 16:53:37 -0500 Subject: [PATCH 2/9] Remove more special handling --- .../Statements/Block/IfElse/IfAnalyzer.php | 30 ------------------- 1 file changed, 30 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php index 7c9dfcac145..d8c007a0ebd 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php @@ -28,11 +28,8 @@ use function array_intersect_key; use function array_keys; use function array_merge; -use function array_unique; use function count; use function in_array; -use function strpos; -use function substr; /** * @internal @@ -175,33 +172,6 @@ public static function analyze( array_keys($if_scope->negated_types) ); - $extra_vars_to_update = []; - - // if there's an object-like array in there, we also need to update the root array variable - foreach ($vars_to_update as $var_id) { - $bracked_pos = strpos($var_id, '['); - if ($bracked_pos !== false) { - $extra_vars_to_update[] = substr($var_id, 0, $bracked_pos); - } - } - - if ($extra_vars_to_update) { - $vars_to_update = array_unique(array_merge($extra_vars_to_update, $vars_to_update)); - } - - //update $if_context vars to include the pre-assignment else vars - if (!$stmt->else && !$has_leaving_statements) { - foreach ($pre_assignment_else_redefined_vars as $var_id => $type) { - if (isset($if_context->vars_in_scope[$var_id])) { - $if_context->vars_in_scope[$var_id] = Type::combineUnionTypes( - $if_context->vars_in_scope[$var_id], - $type, - $codebase - ); - } - } - } - $outer_context->update( $old_if_context, $if_context, From 5659596e566fd9a6903b5d716279be393feff038 Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Wed, 26 Jan 2022 12:00:38 -0500 Subject: [PATCH 3/9] Remove some unnecessary ElseAnalyzer code --- .../Statements/Block/IfElse/ElseAnalyzer.php | 31 +++++++++---------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseAnalyzer.php index d957d3c8293..06c88b06cb1 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseAnalyzer.php @@ -118,11 +118,11 @@ public static function analyze( /** @var array */ $new_assigned_var_ids = $else_context->assigned_var_ids; - $else_context->assigned_var_ids = $pre_stmts_assigned_var_ids; + $else_context->assigned_var_ids += $pre_stmts_assigned_var_ids; /** @var array */ $new_possibly_assigned_var_ids = $else_context->possibly_assigned_var_ids; - $else_context->possibly_assigned_var_ids = $pre_possibly_assigned_var_ids + $new_possibly_assigned_var_ids; + $else_context->possibly_assigned_var_ids += $pre_possibly_assigned_var_ids; if ($else) { foreach ($else_context->byref_constraints as $var_id => $byref_constraint) { @@ -200,24 +200,21 @@ public static function analyze( $possibly_assigned_var_ids = $new_possibly_assigned_var_ids; - if ($has_leaving_statements && $else_context->loop_scope) { - if (!$has_continue_statement && !$has_break_statement) { - $if_scope->new_vars_possibly_in_scope = array_merge( - $vars_possibly_in_scope, - $if_scope->new_vars_possibly_in_scope - ); + if ($has_leaving_statements) { + if ($else_context->loop_scope) { + if (!$has_continue_statement && !$has_break_statement) { + $if_scope->new_vars_possibly_in_scope = array_merge( + $vars_possibly_in_scope, + $if_scope->new_vars_possibly_in_scope + ); + } - $if_scope->possibly_assigned_var_ids = array_merge( - $possibly_assigned_var_ids, - $if_scope->possibly_assigned_var_ids + $else_context->loop_scope->vars_possibly_in_scope = array_merge( + $vars_possibly_in_scope, + $else_context->loop_scope->vars_possibly_in_scope ); } - - $else_context->loop_scope->vars_possibly_in_scope = array_merge( - $vars_possibly_in_scope, - $else_context->loop_scope->vars_possibly_in_scope - ); - } elseif (!$has_leaving_statements) { + } else { $if_scope->new_vars_possibly_in_scope = array_merge( $vars_possibly_in_scope, $if_scope->new_vars_possibly_in_scope From c630dda7336739b6edeab3d5ed6bcc31e878cc9c Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Wed, 26 Jan 2022 15:07:28 -0500 Subject: [PATCH 4/9] Add back necessary code --- .../Statements/Block/IfElse/IfAnalyzer.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php index d8c007a0ebd..52e3223d8f8 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php @@ -28,8 +28,11 @@ use function array_intersect_key; use function array_keys; use function array_merge; +use function array_unique; use function count; use function in_array; +use function strpos; +use function substr; /** * @internal @@ -172,6 +175,20 @@ public static function analyze( array_keys($if_scope->negated_types) ); + $extra_vars_to_update = []; + + // if there's an object-like array in there, we also need to update the root array variable + foreach ($vars_to_update as $var_id) { + $bracked_pos = strpos($var_id, '['); + if ($bracked_pos !== false) { + $extra_vars_to_update[] = substr($var_id, 0, $bracked_pos); + } + } + + if ($extra_vars_to_update) { + $vars_to_update = array_unique(array_merge($extra_vars_to_update, $vars_to_update)); + } + $outer_context->update( $old_if_context, $if_context, From 7aca2ba9e9368594f5c05197c2ba7c3d781b33a8 Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Wed, 26 Jan 2022 16:00:25 -0500 Subject: [PATCH 5/9] Fix return type of method never returning null --- .../Statements/Block/IfConditionalAnalyzer.php | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php index 7c672ca138d..12164e22efa 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php @@ -114,14 +114,12 @@ public static function analyze( $outer_context->inside_conditional = true; - if ($externally_applied_if_cond_expr) { - if (ExpressionAnalyzer::analyze( - $statements_analyzer, - $externally_applied_if_cond_expr, - $outer_context - ) === false) { - throw new ScopeAnalysisException(); - } + if (ExpressionAnalyzer::analyze( + $statements_analyzer, + $externally_applied_if_cond_expr, + $outer_context + ) === false) { + throw new ScopeAnalysisException(); } $first_cond_assigned_var_ids = $outer_context->assigned_var_ids; @@ -236,7 +234,7 @@ public static function analyze( * Returns statements that are definitely evaluated before any statements after the end of the * if/elseif/else blocks */ - private static function getDefinitelyEvaluatedExpressionAfterIf(PhpParser\Node\Expr $stmt): ?PhpParser\Node\Expr + private static function getDefinitelyEvaluatedExpressionAfterIf(PhpParser\Node\Expr $stmt): PhpParser\Node\Expr { if ($stmt instanceof PhpParser\Node\Expr\BinaryOp\Equal || $stmt instanceof PhpParser\Node\Expr\BinaryOp\Identical @@ -280,7 +278,7 @@ private static function getDefinitelyEvaluatedExpressionAfterIf(PhpParser\Node\E * Returns statements that are definitely evaluated before any statements inside * the if block */ - private static function getDefinitelyEvaluatedExpressionInsideIf(PhpParser\Node\Expr $stmt): ?PhpParser\Node\Expr + private static function getDefinitelyEvaluatedExpressionInsideIf(PhpParser\Node\Expr $stmt): PhpParser\Node\Expr { if ($stmt instanceof PhpParser\Node\Expr\BinaryOp\Equal || $stmt instanceof PhpParser\Node\Expr\BinaryOp\Identical From a421988d2dea15d00ad1e04981428dfe39c72b52 Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Thu, 27 Jan 2022 18:53:16 -0500 Subject: [PATCH 6/9] Add a comment --- .../Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php index f5ef20d7a8f..10d6eaad601 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php @@ -64,6 +64,8 @@ public static function analyze( $post_leaving_if_context = null; + // we cap this at max depth of 4 to prevent quadratic behaviour + // when analysing || || || || if (!$stmt->left instanceof PhpParser\Node\Expr\BinaryOp\BooleanOr || !$stmt->left->left instanceof PhpParser\Node\Expr\BinaryOp\BooleanOr || !$stmt->left->left->left instanceof PhpParser\Node\Expr\BinaryOp\BooleanOr From 9c6f380f6d24d47f12a9ca4f96901b06f8d3b2bf Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Thu, 27 Jan 2022 20:59:33 -0500 Subject: [PATCH 7/9] Simplify && handling --- .../Expression/BinaryOp/AndAnalyzer.php | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/AndAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/AndAnalyzer.php index 7e3937c38ac..d890838b441 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/AndAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/AndAnalyzer.php @@ -181,25 +181,22 @@ public static function analyze( } if ($context->if_context && !$context->inside_negation) { - $context->vars_in_scope = $right_context->vars_in_scope; $if_context = $context->if_context; - foreach ($right_context->vars_in_scope as $var_id => $type) { - if (!isset($if_context->vars_in_scope[$var_id])) { - $if_context->vars_in_scope[$var_id] = $type; - } elseif (isset($context->vars_in_scope[$var_id])) { - $if_context->vars_in_scope[$var_id] = $context->vars_in_scope[$var_id]; - } - } + $context->vars_in_scope = $right_context->vars_in_scope; + $if_context->vars_in_scope = array_merge( + $if_context->vars_in_scope, + $context->vars_in_scope + ); $if_context->referenced_var_ids = array_merge( + $if_context->referenced_var_ids, $context->referenced_var_ids, - $if_context->referenced_var_ids ); $if_context->assigned_var_ids = array_merge( + $if_context->assigned_var_ids, $context->assigned_var_ids, - $if_context->assigned_var_ids ); $if_context->reconciled_expression_clauses = array_merge( @@ -211,8 +208,8 @@ public static function analyze( ); $if_context->vars_possibly_in_scope = array_merge( + $if_context->vars_possibly_in_scope, $context->vars_possibly_in_scope, - $if_context->vars_possibly_in_scope ); $if_context->updateChecks($context); From be7de56aba056871d67be8c35b67af0ec7294836 Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Fri, 28 Jan 2022 18:04:57 -0500 Subject: [PATCH 8/9] Add comments to make stuff clearer --- src/Psalm/Context.php | 2 +- .../Block/IfConditionalAnalyzer.php | 5 +++- .../Statements/Block/IfElseAnalyzer.php | 1 + .../Expression/BinaryOp/AndAnalyzer.php | 27 +++++++++---------- .../Expression/BinaryOp/OrAnalyzer.php | 26 +++++++++--------- tests/TypeReconciliation/TypeAlgebraTest.php | 12 ++++----- 6 files changed, 37 insertions(+), 36 deletions(-) diff --git a/src/Psalm/Context.php b/src/Psalm/Context.php index 23718645ded..ebc68666a2d 100644 --- a/src/Psalm/Context.php +++ b/src/Psalm/Context.php @@ -349,7 +349,7 @@ class Context /** * @var Context|null */ - public $if_context; + public $if_body_context; /** * @var bool diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php index 12164e22efa..2973fcaf96d 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php @@ -141,7 +141,10 @@ public static function analyze( } $if_conditional_context = clone $if_context; - $if_conditional_context->if_context = $if_context; + + // here we set up a context specifically for the statements in the first `if`, which can + // be affected by statements in the if condition + $if_conditional_context->if_body_context = $if_context; if ($codebase->alter_code) { $if_context->branch_point = $branch_point; diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php index 370c17466d7..deebd33b44f 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php @@ -112,6 +112,7 @@ public static function analyze( $if_context = $if_conditional_scope->if_context; + // this is the context for stuff that happens after the `if` block $post_if_context = $if_conditional_scope->post_if_context; $cond_referenced_var_ids = $if_conditional_scope->cond_referenced_var_ids; $assigned_in_conditional_var_ids = $if_conditional_scope->assigned_in_conditional_var_ids; diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/AndAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/AndAnalyzer.php index d890838b441..793a1ea162e 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/AndAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/AndAnalyzer.php @@ -180,39 +180,38 @@ public static function analyze( ); } - if ($context->if_context && !$context->inside_negation) { - $if_context = $context->if_context; - + if ($context->if_body_context && !$context->inside_negation) { + $if_body_context = $context->if_body_context; $context->vars_in_scope = $right_context->vars_in_scope; - $if_context->vars_in_scope = array_merge( - $if_context->vars_in_scope, + $if_body_context->vars_in_scope = array_merge( + $if_body_context->vars_in_scope, $context->vars_in_scope ); - $if_context->referenced_var_ids = array_merge( - $if_context->referenced_var_ids, + $if_body_context->referenced_var_ids = array_merge( + $if_body_context->referenced_var_ids, $context->referenced_var_ids, ); - $if_context->assigned_var_ids = array_merge( - $if_context->assigned_var_ids, + $if_body_context->assigned_var_ids = array_merge( + $if_body_context->assigned_var_ids, $context->assigned_var_ids, ); - $if_context->reconciled_expression_clauses = array_merge( - $if_context->reconciled_expression_clauses, + $if_body_context->reconciled_expression_clauses = array_merge( + $if_body_context->reconciled_expression_clauses, array_map( fn($c) => $c->hash, $partitioned_clauses[1] ) ); - $if_context->vars_possibly_in_scope = array_merge( - $if_context->vars_possibly_in_scope, + $if_body_context->vars_possibly_in_scope = array_merge( + $if_body_context->vars_possibly_in_scope, $context->vars_possibly_in_scope, ); - $if_context->updateChecks($context); + $if_body_context->updateChecks($context); } else { $context->vars_in_scope = $left_context->vars_in_scope; } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php index 10d6eaad601..78fa57971a6 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php @@ -102,7 +102,7 @@ public static function analyze( $post_leaving_if_context = clone $context; $left_context = clone $context; - $left_context->if_context = null; + $left_context->if_body_context = null; $left_context->assigned_var_ids = []; if (ExpressionAnalyzer::analyze($statements_analyzer, $stmt->left, $left_context) === false) { @@ -261,7 +261,7 @@ public static function analyze( ); } - $right_context->if_context = null; + $right_context->if_body_context = null; $pre_referenced_var_ids = $right_context->referenced_var_ids; $right_context->referenced_var_ids = []; @@ -371,18 +371,18 @@ public static function analyze( $right_context->assigned_var_ids ); - if ($context->if_context) { - $if_context = $context->if_context; + if ($context->if_body_context) { + $if_body_context = $context->if_body_context; foreach ($right_context->vars_in_scope as $var_id => $type) { - if (isset($if_context->vars_in_scope[$var_id])) { - $if_context->vars_in_scope[$var_id] = Type::combineUnionTypes( + if (isset($if_body_context->vars_in_scope[$var_id])) { + $if_body_context->vars_in_scope[$var_id] = Type::combineUnionTypes( $type, - $if_context->vars_in_scope[$var_id], + $if_body_context->vars_in_scope[$var_id], $codebase ); } elseif (isset($left_context->vars_in_scope[$var_id])) { - $if_context->vars_in_scope[$var_id] = Type::combineUnionTypes( + $if_body_context->vars_in_scope[$var_id] = Type::combineUnionTypes( $type, $left_context->vars_in_scope[$var_id], $codebase @@ -390,17 +390,17 @@ public static function analyze( } } - $if_context->referenced_var_ids = array_merge( + $if_body_context->referenced_var_ids = array_merge( $context->referenced_var_ids, - $if_context->referenced_var_ids + $if_body_context->referenced_var_ids ); - $if_context->assigned_var_ids = array_merge( + $if_body_context->assigned_var_ids = array_merge( $context->assigned_var_ids, - $if_context->assigned_var_ids + $if_body_context->assigned_var_ids ); - $if_context->updateChecks($context); + $if_body_context->updateChecks($context); } $context->vars_possibly_in_scope = array_merge( diff --git a/tests/TypeReconciliation/TypeAlgebraTest.php b/tests/TypeReconciliation/TypeAlgebraTest.php index 818fd90c858..c1e2a763da3 100644 --- a/tests/TypeReconciliation/TypeAlgebraTest.php +++ b/tests/TypeReconciliation/TypeAlgebraTest.php @@ -19,18 +19,16 @@ public function providerValidCodeParse(): iterable return [ 'twoVarLogicSimple' => [ 'code' => ' [ From 941f7369495d52b2ab2c847f72508c8983ad843c Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Fri, 28 Jan 2022 18:17:08 -0500 Subject: [PATCH 9/9] Move if-specfic logic to more appropriate setting --- .../Statements/Block/IfElse/IfAnalyzer.php | 107 +++++++++++++++++- .../Statements/Block/IfElseAnalyzer.php | 101 +---------------- 2 files changed, 107 insertions(+), 101 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php index 52e3223d8f8..095b885209f 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php @@ -6,9 +6,11 @@ use Psalm\CodeLocation; use Psalm\Codebase; use Psalm\Context; +use Psalm\Internal\Algebra; use Psalm\Internal\Analyzer\ScopeAnalyzer; use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer; use Psalm\Internal\Analyzer\StatementsAnalyzer; +use Psalm\Internal\Clause; use Psalm\Internal\Scope\IfConditionalScope; use Psalm\Internal\Scope\IfScope; use Psalm\Internal\Type\Comparator\UnionTypeComparator; @@ -20,17 +22,24 @@ use Psalm\Node\Name\VirtualFullyQualified; use Psalm\Node\VirtualArg; use Psalm\Type; +use Psalm\Type\Reconciler; use Psalm\Type\Union; +use function array_combine; use function array_diff_key; use function array_filter; use function array_intersect; use function array_intersect_key; +use function array_key_exists; use function array_keys; use function array_merge; +use function array_reduce; use function array_unique; use function count; use function in_array; +use function preg_match; +use function preg_quote; +use function spl_object_id; use function strpos; use function substr; @@ -50,10 +59,106 @@ public static function analyze( IfScope $if_scope, IfConditionalScope $if_conditional_scope, Context $if_context, - Context $old_if_context, Context $outer_context, array $pre_assignment_else_redefined_vars ): ?bool { + $cond_referenced_var_ids = $if_conditional_scope->cond_referenced_var_ids; + + $active_if_types = []; + + $reconcilable_if_types = Algebra::getTruthsFromFormula( + $if_context->clauses, + spl_object_id($stmt->cond), + $cond_referenced_var_ids, + $active_if_types + ); + + if (array_filter( + $outer_context->clauses, + fn($clause): bool => (bool)$clause->possibilities + )) { + $omit_keys = array_reduce( + $outer_context->clauses, + /** + * @param array $carry + * @return array + */ + fn(array $carry, Clause $clause): array => array_merge($carry, array_keys($clause->possibilities)), + [] + ); + + $omit_keys = array_combine($omit_keys, $omit_keys); + $omit_keys = array_diff_key($omit_keys, Algebra::getTruthsFromFormula($outer_context->clauses)); + + $cond_referenced_var_ids = array_diff_key( + $cond_referenced_var_ids, + $omit_keys + ); + } + + // if the if has an || in the conditional, we cannot easily reason about it + if ($reconcilable_if_types) { + $changed_var_ids = []; + + $if_vars_in_scope_reconciled = + Reconciler::reconcileKeyedTypes( + $reconcilable_if_types, + $active_if_types, + $if_context->vars_in_scope, + $if_context->references_in_scope, + $changed_var_ids, + $cond_referenced_var_ids, + $statements_analyzer, + $statements_analyzer->getTemplateTypeMap() ?: [], + $if_context->inside_loop, + $outer_context->check_variables + ? new CodeLocation( + $statements_analyzer->getSource(), + $stmt->cond instanceof PhpParser\Node\Expr\BooleanNot + ? $stmt->cond->expr + : $stmt->cond, + $outer_context->include_location + ) : null + ); + + $if_context->vars_in_scope = $if_vars_in_scope_reconciled; + + foreach ($reconcilable_if_types as $var_id => $_) { + $if_context->vars_possibly_in_scope[$var_id] = true; + } + + if ($changed_var_ids) { + $if_context->clauses = Context::removeReconciledClauses($if_context->clauses, $changed_var_ids)[0]; + + foreach ($changed_var_ids as $changed_var_id => $_) { + foreach ($if_context->vars_in_scope as $var_id => $_) { + if (preg_match('/' . preg_quote($changed_var_id, '/') . '[\]\[\-]/', $var_id) + && !array_key_exists($var_id, $changed_var_ids) + && !array_key_exists($var_id, $cond_referenced_var_ids) + ) { + $if_context->removePossibleReference($var_id); + } + } + } + } + + $if_scope->if_cond_changed_var_ids = $changed_var_ids; + } + + $if_context->reconciled_expression_clauses = []; + + $outer_context->vars_possibly_in_scope = array_merge( + $if_context->vars_possibly_in_scope, + $outer_context->vars_possibly_in_scope + ); + + $outer_context->referenced_var_ids = array_merge( + $if_context->referenced_var_ids, + $outer_context->referenced_var_ids + ); + + $old_if_context = clone $if_context; + $codebase = $statements_analyzer->getCodebase(); $assigned_var_ids = $if_context->assigned_var_ids; diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php index deebd33b44f..73046ada3fc 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php @@ -24,16 +24,12 @@ use Psalm\Type; use Psalm\Type\Reconciler; -use function array_combine; use function array_diff; -use function array_diff_key; use function array_filter; use function array_intersect_key; -use function array_key_exists; use function array_keys; use function array_map; use function array_merge; -use function array_reduce; use function array_unique; use function array_values; use function count; @@ -110,11 +106,11 @@ public static function analyze( $context->branch_point ?: (int) $stmt->getAttribute('startFilePos') ); + // this is the context for stuff that happens within the `if` block $if_context = $if_conditional_scope->if_context; // this is the context for stuff that happens after the `if` block $post_if_context = $if_conditional_scope->post_if_context; - $cond_referenced_var_ids = $if_conditional_scope->cond_referenced_var_ids; $assigned_in_conditional_var_ids = $if_conditional_scope->assigned_in_conditional_var_ids; } catch (ScopeAnalysisException $e) { return false; @@ -231,100 +227,6 @@ function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause { ) ); - $active_if_types = []; - - $reconcilable_if_types = Algebra::getTruthsFromFormula( - $if_context->clauses, - spl_object_id($stmt->cond), - $cond_referenced_var_ids, - $active_if_types - ); - - if (array_filter( - $context->clauses, - fn($clause): bool => (bool)$clause->possibilities - )) { - $omit_keys = array_reduce( - $context->clauses, - /** - * @param array $carry - * @return array - */ - fn(array $carry, Clause $clause): array => array_merge($carry, array_keys($clause->possibilities)), - [] - ); - - $omit_keys = array_combine($omit_keys, $omit_keys); - $omit_keys = array_diff_key($omit_keys, Algebra::getTruthsFromFormula($context->clauses)); - - $cond_referenced_var_ids = array_diff_key( - $cond_referenced_var_ids, - $omit_keys - ); - } - - // if the if has an || in the conditional, we cannot easily reason about it - if ($reconcilable_if_types) { - $changed_var_ids = []; - - $if_vars_in_scope_reconciled = - Reconciler::reconcileKeyedTypes( - $reconcilable_if_types, - $active_if_types, - $if_context->vars_in_scope, - $if_context->references_in_scope, - $changed_var_ids, - $cond_referenced_var_ids, - $statements_analyzer, - $statements_analyzer->getTemplateTypeMap() ?: [], - $if_context->inside_loop, - $context->check_variables - ? new CodeLocation( - $statements_analyzer->getSource(), - $stmt->cond instanceof PhpParser\Node\Expr\BooleanNot - ? $stmt->cond->expr - : $stmt->cond, - $context->include_location - ) : null - ); - - $if_context->vars_in_scope = $if_vars_in_scope_reconciled; - - foreach ($reconcilable_if_types as $var_id => $_) { - $if_context->vars_possibly_in_scope[$var_id] = true; - } - - if ($changed_var_ids) { - $if_context->clauses = Context::removeReconciledClauses($if_context->clauses, $changed_var_ids)[0]; - - foreach ($changed_var_ids as $changed_var_id => $_) { - foreach ($if_context->vars_in_scope as $var_id => $_) { - if (preg_match('/' . preg_quote($changed_var_id, '/') . '[\]\[\-]/', $var_id) - && !array_key_exists($var_id, $changed_var_ids) - && !array_key_exists($var_id, $cond_referenced_var_ids) - ) { - $if_context->removePossibleReference($var_id); - } - } - } - } - - $if_scope->if_cond_changed_var_ids = $changed_var_ids; - } - - $if_context->reconciled_expression_clauses = []; - - $old_if_context = clone $if_context; - $context->vars_possibly_in_scope = array_merge( - $if_context->vars_possibly_in_scope, - $context->vars_possibly_in_scope - ); - - $context->referenced_var_ids = array_merge( - $if_context->referenced_var_ids, - $context->referenced_var_ids - ); - $temp_else_context = clone $post_if_context; $changed_var_ids = []; @@ -367,7 +269,6 @@ function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause { $if_scope, $if_conditional_scope, $if_context, - $old_if_context, $context, $pre_assignment_else_redefined_vars ) === false) {