diff --git a/src/Psalm/Context.php b/src/Psalm/Context.php index 17329fa6d27..286cbf72ee4 100644 --- a/src/Psalm/Context.php +++ b/src/Psalm/Context.php @@ -459,7 +459,7 @@ public function update( array $vars_to_update, array &$updated_vars ): void { - foreach ($start_context->vars_in_scope as $var_id => $_) { + foreach ($start_context->vars_in_scope as $var_id => $old_type) { // this is only true if there was some sort of type negation if (in_array($var_id, $vars_to_update, true)) { // if we're leaving, we're effectively deleting the possibility of the if types @@ -469,10 +469,32 @@ public function update( $existing_type = $this->vars_in_scope[$var_id] ?? null; - if (!$existing_type && $new_type) { - $this->vars_in_scope[$var_id] = clone $new_type; + if (!$existing_type) { + if ($new_type) { + $this->vars_in_scope[$var_id] = clone $new_type; + $updated_vars[$var_id] = true; + } + + continue; + } + + $existing_type = clone $existing_type; + + // if the type changed within the block of statements, process the replacement + // also never allow ourselves to remove all types from a union + if ((!$new_type || !$old_type->equals($new_type)) + && ($new_type || count($existing_type->getAtomicTypes()) > 1) + ) { + $existing_type->substitute($old_type, $new_type); + + if ($new_type && $new_type->from_docblock) { + $existing_type->setFromDocblock(); + } + $updated_vars[$var_id] = true; } + + $this->vars_in_scope[$var_id] = $existing_type; } } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php index 0fc2ffa9845..5d6cdb28e91 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php @@ -23,20 +23,25 @@ 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; /** * @internal @@ -44,6 +49,8 @@ class IfAnalyzer { /** + * @param array $pre_assignment_else_redefined_vars + * * @return false|null */ public static function analyze( @@ -52,7 +59,8 @@ public static function analyze( IfScope $if_scope, IfConditionalScope $if_conditional_scope, Context $if_context, - Context $outer_context + Context $outer_context, + array $pre_assignment_else_redefined_vars ): ?bool { $cond_referenced_var_ids = $if_conditional_scope->cond_referenced_var_ids; @@ -142,6 +150,8 @@ public static function analyze( $outer_context->vars_possibly_in_scope ); + $old_if_context = clone $if_context; + $codebase = $statements_analyzer->getCodebase(); $assigned_var_ids = $if_context->assigned_var_ids; @@ -254,6 +264,38 @@ public static function analyze( } } + // 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) { + $vars_to_update = array_intersect( + array_keys($pre_assignment_else_redefined_vars), + 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, + $has_leaving_statements, + $vars_to_update, + $if_scope->updated_vars + ); + } + if (!$has_ending_statements) { $vars_possibly_in_scope = array_diff_key( $if_context->vars_possibly_in_scope, diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php index 944512ae98a..fd7751def33 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php @@ -264,7 +264,8 @@ public static function analyze( $if_scope, $if_conditional_scope, $if_context, - $context + $context, + $pre_assignment_else_redefined_vars ) === false) { return false; } diff --git a/tests/TypeReconciliation/ConditionalTest.php b/tests/TypeReconciliation/ConditionalTest.php index 171d6967fa2..07d4706204b 100644 --- a/tests/TypeReconciliation/ConditionalTest.php +++ b/tests/TypeReconciliation/ConditionalTest.php @@ -17,6 +17,21 @@ class ConditionalTest extends TestCase public function providerValidCodeParse(): iterable { return [ + 'arrayAssignmentPropagation' => [ + 'code' => ' 123]; + + /** @var array{test: ?int} */ + $a = ["test" => null]; + + if ($a["test"] === null) { + $a = $dummy; + } + $var = $a["test"];', + 'assertions' => [ + '$var' => 'int', + ], + ], 'intIsMixed' => [ 'code' => ' [ + 'twoVarLogicNotNestedWithElseifCorrectlyReinforcedInIf' => [ 'code' => '