From ba6270c06aeb2ecc9628586242fadd489202056f Mon Sep 17 00:00:00 2001 From: AndrolGenhald Date: Thu, 21 Jul 2022 14:27:21 -0500 Subject: [PATCH] Fix type reconciliation breaking Context::$references_in_scope (fixes #8289). --- .../Analyzer/Statements/Block/DoAnalyzer.php | 2 +- .../Block/IfConditionalAnalyzer.php | 3 +- .../Statements/Block/IfElse/ElseAnalyzer.php | 4 +- .../Block/IfElse/ElseIfAnalyzer.php | 31 +++++----- .../Statements/Block/IfElse/IfAnalyzer.php | 41 ++++++------- .../Statements/Block/IfElseAnalyzer.php | 41 +++++++------ .../Statements/Block/LoopAnalyzer.php | 6 +- .../Statements/Block/SwitchAnalyzer.php | 2 +- .../Statements/Block/SwitchCaseAnalyzer.php | 3 +- .../Expression/BinaryOp/AndAnalyzer.php | 4 +- .../Expression/BinaryOp/OrAnalyzer.php | 3 +- .../Expression/Call/FunctionCallAnalyzer.php | 3 +- .../Statements/Expression/CallAnalyzer.php | 3 +- .../Statements/Expression/MatchAnalyzer.php | 2 +- .../Statements/Expression/TernaryAnalyzer.php | 8 +-- .../ArrayFilterReturnTypeProvider.php | 4 +- src/Psalm/Type/Reconciler.php | 59 +++++++++++++++---- tests/ReferenceTest.php | 29 +++++++++ tests/TypeReconciliation/ReconcilerTest.php | 28 --------- 19 files changed, 149 insertions(+), 127 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/DoAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/DoAnalyzer.php index fa463769c98..0056c1b77dd 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/DoAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/DoAnalyzer.php @@ -123,7 +123,7 @@ static function (Clause $c) use ($mixed_var_ids): bool { if ($negated_while_types) { $changed_var_ids = []; - $inner_loop_context->vars_in_scope = + [$inner_loop_context->vars_in_scope, $inner_loop_context->references_in_scope] = Reconciler::reconcileKeyedTypes( $negated_while_types, [], diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php index 188b094a55f..7ab5c32fcae 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php @@ -48,7 +48,7 @@ public static function analyze( $changed_var_ids = []; if ($if_scope->negated_types) { - $vars_reconciled = Reconciler::reconcileKeyedTypes( + [$vars_reconciled, $references_reconciled] = Reconciler::reconcileKeyedTypes( $if_scope->negated_types, [], $outer_context->vars_in_scope, @@ -71,6 +71,7 @@ public static function analyze( if ($changed_var_ids) { $outer_context = clone $outer_context; $outer_context->vars_in_scope = $vars_reconciled; + $outer_context->references_in_scope = $references_reconciled; $entry_clauses = array_values( array_filter( diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseAnalyzer.php index 06c88b06cb1..136ce7daec8 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseAnalyzer.php @@ -64,7 +64,7 @@ public static function analyze( if ($else_types) { $changed_var_ids = []; - $else_vars_reconciled = Reconciler::reconcileKeyedTypes( + [$else_context->vars_in_scope, $else_context->references_in_scope] = Reconciler::reconcileKeyedTypes( $else_types, [], $else_context->vars_in_scope, @@ -79,8 +79,6 @@ public static function analyze( : null ); - $else_context->vars_in_scope = $else_vars_reconciled; - $else_context->clauses = Context::removeReconciledClauses($else_context->clauses, $changed_var_ids)[0]; foreach ($changed_var_ids as $changed_var_id => $_) { diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php index 8e24075719f..dfa52aaad5e 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php @@ -215,7 +215,7 @@ public static function analyze( // if the elseif has an || in the conditional, we cannot easily reason about it if ($reconcilable_elseif_types) { - $elseif_vars_reconciled = Reconciler::reconcileKeyedTypes( + [$elseif_context->vars_in_scope, $elseif_context->references_in_scope] = Reconciler::reconcileKeyedTypes( $reconcilable_elseif_types, $active_elseif_types, $elseif_context->vars_in_scope, @@ -234,8 +234,6 @@ public static function analyze( ) ); - $elseif_context->vars_in_scope = $elseif_vars_reconciled; - if ($newly_reconciled_var_ids) { $elseif_context->clauses = Context::removeReconciledClauses( $elseif_context->clauses, @@ -349,21 +347,20 @@ public static function analyze( if ($has_leaving_statements) { $newly_reconciled_var_ids = []; - $leaving_vars_reconciled = Reconciler::reconcileKeyedTypes( - $negated_elseif_types, - [], - $pre_conditional_context->vars_in_scope, - $pre_conditional_context->references_in_scope, - $newly_reconciled_var_ids, - [], - $statements_analyzer, - $statements_analyzer->getTemplateTypeMap() ?: [], - $elseif_context->inside_loop, - new CodeLocation($statements_analyzer->getSource(), $elseif, $outer_context->include_location) - ); - $implied_outer_context = clone $elseif_context; - $implied_outer_context->vars_in_scope = $leaving_vars_reconciled; + [$implied_outer_context->vars_in_scope, $implied_outer_context->references_in_scope] = + Reconciler::reconcileKeyedTypes( + $negated_elseif_types, + [], + $pre_conditional_context->vars_in_scope, + $pre_conditional_context->references_in_scope, + $newly_reconciled_var_ids, + [], + $statements_analyzer, + $statements_analyzer->getTemplateTypeMap() ?: [], + $elseif_context->inside_loop, + new CodeLocation($statements_analyzer->getSource(), $elseif, $outer_context->include_location) + ); $updated_vars = []; diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php index d06c4d9b2d5..d4459c9f5c5 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php @@ -93,28 +93,25 @@ public static function analyze( 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; + [$if_context->vars_in_scope, $if_context->references_in_scope] = 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 + ); foreach ($reconcilable_if_types as $var_id => $_) { $if_context->vars_possibly_in_scope[$var_id] = true; diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php index 117482f7335..2c314b2f7cf 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php @@ -228,27 +228,26 @@ public static function analyze( $changed_var_ids = []; if ($if_scope->negated_types) { - $else_vars_reconciled = Reconciler::reconcileKeyedTypes( - $if_scope->negated_types, - [], - $temp_else_context->vars_in_scope, - $temp_else_context->references_in_scope, - $changed_var_ids, - [], - $statements_analyzer, - $statements_analyzer->getTemplateTypeMap() ?: [], - $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 - ); - - $temp_else_context->vars_in_scope = $else_vars_reconciled; + [$temp_else_context->vars_in_scope, $temp_else_context->references_in_scope] = + Reconciler::reconcileKeyedTypes( + $if_scope->negated_types, + [], + $temp_else_context->vars_in_scope, + $temp_else_context->references_in_scope, + $changed_var_ids, + [], + $statements_analyzer, + $statements_analyzer->getTemplateTypeMap() ?: [], + $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 + ); } // we calculate the vars redefined in a hypothetical else statement to determine diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php index 15be5e47d25..6be05572729 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php @@ -469,7 +469,7 @@ public static function analyze( if ($negated_pre_condition_types) { $changed_var_ids = []; - $vars_in_scope_reconciled = Reconciler::reconcileKeyedTypes( + [$vars_in_scope_reconciled, $_] = Reconciler::reconcileKeyedTypes( $negated_pre_condition_types, [], $continue_context->vars_in_scope, @@ -613,7 +613,7 @@ private static function applyPreConditionToLoopContext( $changed_var_ids = []; if ($reconcilable_while_types) { - $pre_condition_vars_in_scope_reconciled = Reconciler::reconcileKeyedTypes( + [$loop_context->vars_in_scope, $loop_context->references_in_scope] = Reconciler::reconcileKeyedTypes( $reconcilable_while_types, $active_while_types, $loop_context->vars_in_scope, @@ -625,8 +625,6 @@ private static function applyPreConditionToLoopContext( true, new CodeLocation($statements_analyzer->getSource(), $pre_condition) ); - - $loop_context->vars_in_scope = $pre_condition_vars_in_scope_reconciled; } if ($is_do) { diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/SwitchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/SwitchAnalyzer.php index d579af5208b..66f00d34b8c 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/SwitchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/SwitchAnalyzer.php @@ -154,7 +154,7 @@ public static function analyze( if ($reconcilable_if_types && isset($reconcilable_if_types[$switch_var_id])) { $changed_var_ids = []; - $case_vars_in_scope_reconciled = + [$case_vars_in_scope_reconciled, $_] = Reconciler::reconcileKeyedTypes( $reconcilable_if_types, [], diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/SwitchCaseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/SwitchCaseAnalyzer.php index c9198328c0c..8fea6b8ca3c 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/SwitchCaseAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/SwitchCaseAnalyzer.php @@ -400,7 +400,7 @@ public static function analyze( $statements_analyzer->addSuppressedIssues(['RedundantConditionGivenDocblockType']); } - $case_vars_in_scope_reconciled = + [$case_vars_in_scope_reconciled, $case_references_in_scope_reconciled] = Reconciler::reconcileKeyedTypes( $reconcilable_if_types, [], @@ -427,6 +427,7 @@ public static function analyze( } $case_context->vars_in_scope = $case_vars_in_scope_reconciled; + $case_context->references_in_scope = $case_references_in_scope_reconciled; foreach ($reconcilable_if_types as $var_id => $_) { $case_context->vars_possibly_in_scope[$var_id] = true; } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/AndAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/AndAnalyzer.php index 6f877ae0414..4dd5bfb2f87 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/AndAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/AndAnalyzer.php @@ -134,7 +134,7 @@ public static function analyze( $right_context = clone $context; // while in an and, we allow scope to boil over to support // statements of the form if ($x && $x->foo()) - $right_vars_in_scope = Reconciler::reconcileKeyedTypes( + [$right_context->vars_in_scope, $right_context->references_in_scope] = Reconciler::reconcileKeyedTypes( $left_type_assertions, $active_left_assertions, $right_context->vars_in_scope, @@ -147,8 +147,6 @@ public static function analyze( new CodeLocation($statements_analyzer->getSource(), $stmt->left), $context->inside_negation ); - - $right_context->vars_in_scope = $right_vars_in_scope; } else { $right_context = clone $left_context; } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php index 12720dbc19d..203165c9d84 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php @@ -221,7 +221,7 @@ public static function analyze( if ($negated_type_assertions) { // while in an or, we allow scope to boil over to support // statements of the form if ($x === null || $x->foo()) - $right_vars_in_scope = Reconciler::reconcileKeyedTypes( + [$right_context->vars_in_scope, $right_context->references_in_scope] = Reconciler::reconcileKeyedTypes( $negated_type_assertions, $active_negated_type_assertions, $right_context->vars_in_scope, @@ -234,7 +234,6 @@ public static function analyze( new CodeLocation($statements_analyzer->getSource(), $stmt->left), !$context->inside_negation ); - $right_context->vars_in_scope = $right_vars_in_scope; } $right_context->clauses = $clauses_for_right_analysis; diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php index 2aa19e42d2c..2f8bdb1a483 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php @@ -957,7 +957,7 @@ private static function processAssertFunctionEffects( if ($assert_type_assertions) { // while in an and, we allow scope to boil over to support // statements of the form if ($x && $x->foo()) - $op_vars_in_scope = Reconciler::reconcileKeyedTypes( + [$op_vars_in_scope, $op_references_in_scope] = Reconciler::reconcileKeyedTypes( $assert_type_assertions, $assert_type_assertions, $context->vars_in_scope, @@ -1003,6 +1003,7 @@ private static function processAssertFunctionEffects( } $context->vars_in_scope = $op_vars_in_scope; + $context->references_in_scope = $op_references_in_scope; } if ($changed_var_ids) { diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php index 968e9aef79c..bd086f1921d 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php @@ -899,7 +899,7 @@ public static function applyAssertionsToContext( // while in an and, we allow scope to boil over to support // statements of the form if ($x && $x->foo()) - $op_vars_in_scope = Reconciler::reconcileKeyedTypes( + [$op_vars_in_scope, $op_references_in_scope] = Reconciler::reconcileKeyedTypes( $type_assertions, $type_assertions, $context->vars_in_scope, @@ -954,6 +954,7 @@ public static function applyAssertionsToContext( } $context->vars_in_scope = $op_vars_in_scope; + $context->references_in_scope = $op_references_in_scope; } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/MatchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/MatchAnalyzer.php index 568213be301..08f1749e6c5 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/MatchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/MatchAnalyzer.php @@ -251,7 +251,7 @@ public static function analyze( if ($reconcilable_types) { $changed_var_ids = []; - $vars_in_scope_reconciled = Reconciler::reconcileKeyedTypes( + [$vars_in_scope_reconciled, $_] = Reconciler::reconcileKeyedTypes( $reconcilable_types, [], $context->vars_in_scope, diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php index 601370e4187..7be2e617b3e 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php @@ -191,7 +191,7 @@ static function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause { $changed_var_ids = []; if ($reconcilable_if_types) { - $if_vars_in_scope_reconciled = Reconciler::reconcileKeyedTypes( + [$if_context->vars_in_scope, $if_context->references_in_scope] = Reconciler::reconcileKeyedTypes( $reconcilable_if_types, $active_if_types, $if_context->vars_in_scope, @@ -203,8 +203,6 @@ static function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause { $if_context->inside_loop, new CodeLocation($statements_analyzer->getSource(), $stmt->cond) ); - - $if_context->vars_in_scope = $if_vars_in_scope_reconciled; } $t_else_context = clone $context; @@ -230,7 +228,7 @@ static function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause { $changed_var_ids = []; if ($if_scope->negated_types) { - $else_vars_reconciled = Reconciler::reconcileKeyedTypes( + [$t_else_context->vars_in_scope, $t_else_context->references_in_scope] = Reconciler::reconcileKeyedTypes( $if_scope->negated_types, $if_scope->negated_types, $t_else_context->vars_in_scope, @@ -243,8 +241,6 @@ static function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause { new CodeLocation($statements_analyzer->getSource(), $stmt->else) ); - $t_else_context->vars_in_scope = $else_vars_reconciled; - $t_else_context->clauses = Context::removeReconciledClauses($t_else_context->clauses, $changed_var_ids)[0]; } diff --git a/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php b/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php index 4f14e5d3009..be8e069e7e6 100644 --- a/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php +++ b/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php @@ -213,7 +213,7 @@ static function ($keyed_type) use ($statements_source, $context) { '$inner_type' => $assertions[$assertion_id], ]; - $reconciled_types = Reconciler::reconcileKeyedTypes( + [$reconciled_types, $_] = Reconciler::reconcileKeyedTypes( $assertions, $assertions, ['$inner_type' => $inner_type], @@ -293,7 +293,7 @@ static function ($keyed_type) use ($statements_source, $context) { $assertions = ['$inner_type' => $assertions['$' . $first_param->var->name]]; - $reconciled_types = Reconciler::reconcileKeyedTypes( + [$reconciled_types, $_] = Reconciler::reconcileKeyedTypes( $assertions, $assertions, ['$inner_type' => $inner_type], diff --git a/src/Psalm/Type/Reconciler.php b/src/Psalm/Type/Reconciler.php index e8cf63c91ca..19b706bcb62 100644 --- a/src/Psalm/Type/Reconciler.php +++ b/src/Psalm/Type/Reconciler.php @@ -56,6 +56,7 @@ use ReflectionProperty; use UnexpectedValueException; +use function array_keys; use function array_merge; use function array_pop; use function array_shift; @@ -64,6 +65,7 @@ use function explode; use function implode; use function is_numeric; +use function key; use function ksort; use function preg_match; use function preg_quote; @@ -83,7 +85,8 @@ class Reconciler private static $broken_paths = []; /** - * Takes two arrays and consolidates them, removing null values from existing types where applicable + * Takes two arrays and consolidates them, removing null values from existing types where applicable. + * Returns a tuple of [new_types, new_references]. * * @param array>> $new_types * @param array>> $active_new_types - types we can complain about @@ -94,7 +97,9 @@ class Reconciler * @param array $referenced_var_ids * @param array> $template_type_map * - * @return array + * @return array{array, array} + * + * @psalm-suppress ComplexMethod */ public static function reconcileKeyedTypes( array $new_types, @@ -110,10 +115,10 @@ public static function reconcileKeyedTypes( bool $negated = false ): array { if (!$new_types) { - return $existing_types; + return [$existing_types, $existing_references]; } - $reference_map = []; + $reference_graph = []; if (!empty($existing_references)) { // PHP behaves oddly when passing an array containing references: https://bugs.php.net/bug.php?id=20993 // To work around the issue, if there are any references, we have to recreate the array and fix the @@ -124,7 +129,7 @@ public static function reconcileKeyedTypes( $cloned_referenceds = []; foreach ($existing_references as $reference => $referenced) { - if (!isset($cloned_referenceds[$referenced]) && isset($old_existing_types[$referenced])) { + if (!isset($cloned_referenceds[$referenced])) { $existing_types[$referenced] = clone $old_existing_types[$referenced]; $cloned_referenceds[$referenced] = true; } @@ -134,8 +139,12 @@ public static function reconcileKeyedTypes( // Build a map from reference/referenced variables to other variables with the same reference foreach ($existing_references as $reference => $referenced) { - $reference_map[$reference][] = $referenced; - $reference_map[$referenced][] = $reference; + $reference_graph[$reference][$referenced] = true; + foreach ($reference_graph[$referenced] ?? [] as $existing_referenced => $_) { + $reference_graph[$existing_referenced][$reference] = true; + $reference_graph[$reference][$existing_referenced] = true; + } + $reference_graph[$referenced][$reference] = true; } } @@ -239,7 +248,7 @@ public static function reconcileKeyedTypes( $nested_negated = $negated; } - $existing_types = self::reconcileKeyedTypes( + [$existing_types, $_] = self::reconcileKeyedTypes( $data, $data, $existing_types, @@ -336,7 +345,33 @@ public static function reconcileKeyedTypes( && preg_match('/' . preg_quote($key, '/') . '[\]\[\-]/', $new_key) && $is_real ) { - unset($existing_types[$new_key]); + // Fix any references to the type before removing it. + $references_to_fix = array_keys($reference_graph[$new_key] ?? []); + if (count($references_to_fix) > 1) { + // Still multiple references, just remove $new_key + foreach ($references_to_fix as $reference_to_fix) { + unset($reference_graph[$reference_to_fix][$new_key]); + } + // Set references pointing to $new_key to point + // to the first other reference from the same group + $new_primary_reference = key($reference_graph[$references_to_fix[0]]); + unset($existing_references[$new_primary_reference]); + foreach ($existing_references as $existing_reference => $existing_referenced) { + if ($existing_referenced === $new_key) { + $existing_references[$existing_reference] = $new_primary_reference; + } + } + } elseif (count($references_to_fix) === 1) { + // Since reference target is going to be removed, + // pretend the reference is just a normal variable + $reference_to_fix = $references_to_fix[0]; + unset($reference_graph[$reference_to_fix], $existing_references[$reference_to_fix]); + } + unset( + $existing_types[$new_key], + $reference_graph[$new_key], + $existing_references[$new_key], + ); } } } @@ -352,10 +387,10 @@ public static function reconcileKeyedTypes( $existing_types[$key] = $result_type; } - if (!$did_type_exist && isset($existing_types[$key]) && isset($reference_map[$key_parts[0]])) { + if (!$did_type_exist && isset($existing_types[$key]) && isset($reference_graph[$key_parts[0]])) { // If key is new, create references for other variables that reference the root variable. $reference_key_parts = $key_parts; - foreach ($reference_map[$key_parts[0]] as $reference) { + foreach ($reference_graph[$key_parts[0]] as $reference => $_) { $reference_key_parts[0] = $reference; $reference_key = implode("", $reference_key_parts); $existing_types[$reference_key] = &$existing_types[$key]; @@ -363,7 +398,7 @@ public static function reconcileKeyedTypes( } } - return $existing_types; + return [$existing_types, $existing_references]; } /** diff --git a/tests/ReferenceTest.php b/tests/ReferenceTest.php index 17936868e2e..3f3262fcb65 100644 --- a/tests/ReferenceTest.php +++ b/tests/ReferenceTest.php @@ -257,6 +257,35 @@ function func(array &$a): void '$b' => 'string', ], ], + 'referenceToArrayVariableOffsetDoesntCrashWhenOffsetVariableChangesDueToReconciliation' => [ + 'code' => ' ["id" => 1]]; + $reference = &$doesNotMatter[$a]; + /** @psalm-suppress TypeDoesNotContainType */ + $result = ($a === "not-a" && ($b || false)); + ', + 'assertions' => [ + '$reference===' => 'array{id: 1}', + ], + ], + 'multipleReferencesToArrayVariableOffsetThatChangesDueToReconciliation' => [ + 'code' => ' ["id" => 1]]; + $reference1 = &$doesNotMatter[$a]; + $reference2 = &$doesNotMatter[$a]; + /** @psalm-suppress TypeDoesNotContainType */ + $result = ($a === "not-a" && ($b || false)); + $reference1["id"] = 2; + ', + 'assertions' => [ + '$reference1===' => 'array{id: 2}', + '$reference2===' => 'array{id: 2}', + ], + ], ]; } diff --git a/tests/TypeReconciliation/ReconcilerTest.php b/tests/TypeReconciliation/ReconcilerTest.php index ac7ceba270d..1d8ebde693d 100644 --- a/tests/TypeReconciliation/ReconcilerTest.php +++ b/tests/TypeReconciliation/ReconcilerTest.php @@ -18,7 +18,6 @@ use Psalm\Storage\Assertion\NonEmpty; use Psalm\Storage\Assertion\Truthy; use Psalm\Tests\TestCase; -use Psalm\Tests\TestConfig; use Psalm\Type; use Psalm\Type\Atomic\TArray; use Psalm\Type\Atomic\TClassConstant; @@ -282,31 +281,4 @@ public function constantAssertions(): array ], ]; } - - /** - * @test - */ - public function arrayReferencesAreHandled(): void - { - $this->addFile( - 'somefile.php', - <<< 'EOT' - ['id' => 1]]; - $reference = &$doesNotMatter[$a]; - $result = ($a === 'not-a' && ($b || false)); - EOT - ); - - TestConfig::getInstance()->throw_exception = false; - $codebase = $this->project_analyzer->getCodebase(); - $codebase->scanner->addFileToDeepScan('somefile.php'); - $codebase->addFilesToAnalyze(['somefile.php' => 'somefile.php']); - $codebase->scanFiles(); - $this->file_analyzer->analyze($this->file_analyzer->context); - // just asserting this test reaches the end - self::assertTrue(true); - } }