Merge pull request #8273 from muglug/muglug-remove-special-elseif-han…

…dling

Remove special handling for elseifs that breaks for else if

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 => $old_type) {
+        foreach ($start_context->vars_in_scope as $var_id => $_) {
             // 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,32 +469,10 @@ public function update(
 
                 $existing_type = $this->vars_in_scope[$var_id] ?? null;
 
-                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();
-                    }
-
+                if (!$existing_type && $new_type) {
+                    $this->vars_in_scope[$var_id] = clone $new_type;
                     $updated_vars[$var_id] = true;
                 }
-
-                $this->vars_in_scope[$var_id] = $existing_type;
             }
         }
     }

src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php

@@ -23,34 +23,27 @@
 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
  */
 class IfAnalyzer
 {
     /**
-     * @param  array<string, Union> $pre_assignment_else_redefined_vars
-     *
      * @return false|null
      */
     public static function analyze(
@@ -59,8 +52,7 @@ public static function analyze(
         IfScope $if_scope,
         IfConditionalScope $if_conditional_scope,
         Context $if_context,
-        Context $outer_context,
-        array $pre_assignment_else_redefined_vars
+        Context $outer_context
     ): ?bool {
         $cond_referenced_var_ids = $if_conditional_scope->cond_referenced_var_ids;
 
@@ -153,8 +145,6 @@ 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;
@@ -267,38 +257,6 @@ 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,

src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php

@@ -265,8 +265,7 @@ public static function analyze(
             $if_scope,
             $if_conditional_scope,
             $if_context,
-            $context,
-            $pre_assignment_else_redefined_vars
+            $context
         ) === false) {
             return false;
         }

tests/TypeReconciliation/TypeAlgebraTest.php

@@ -212,7 +212,10 @@ function foo(?string $a): void {
                         }
                     }',
             ],
-            'twoVarLogicNotNestedWithElseifCorrectlyReinforcedInIf' => [
+            // skipping this old test because it breaks if the elseif becomes
+            // an else if, and I don't believe it's a pattern that happens often
+            // enough to warrant special-casing
+            'SKIPPED-twoVarLogicNotNestedWithElseifCorrectlyReinforcedInIf' => [
                 'code' => '<?php
                     class A {}
                     class B extends A {}