Don’t complain when there are duplicate clauses if due to an assignment

Fixes #356

src/Psalm/Checker/AlgebraChecker.php

@@ -194,6 +194,7 @@ class AlgebraChecker
      * @param  array<int, Clause>   $formula2
      * @param  StatementsChecker    $statements_checker,
      * @param  PhpParser\Node       $stmt
+     * @param  array<string, bool>  $new_assigned_var_ids
      *
      * @return void
      */
@@ -201,7 +202,8 @@ class AlgebraChecker
         array $formula1,
         array $formula2,
         StatementsChecker $statements_checker,
-        PhpParser\Node $stmt
+        PhpParser\Node $stmt,
+        array $new_assigned_var_ids
     ) {
         $negated_formula2 = self::negateFormula($formula2);
 
@@ -210,6 +212,7 @@ class AlgebraChecker
             foreach ($clause_a->possibilities as $key => $values) {
                 if (count($values) > 1
                     && count(array_unique($values)) < count($values)
+                    && !isset($new_assigned_var_ids[$key])
                 ) {
                     if (IssueBuffer::accepts(
                         new RedundantCondition(

src/Psalm/Checker/Statements/Block/IfChecker.php

@@ -65,6 +65,7 @@ class IfChecker
         $context->referenced_var_ids = [];
 
         $pre_assigned_var_ids = $context->assigned_var_ids;
+        $context->assigned_var_ids = [];
 
         $project_checker = $statements_checker->getFileChecker()->project_checker;
 
@@ -74,6 +75,12 @@ class IfChecker
             return false;
         }
 
+        $first_cond_assigned_var_ids = $context->assigned_var_ids;
+        $context->assigned_var_ids = array_merge(
+            $pre_assigned_var_ids,
+            $first_cond_assigned_var_ids
+        );
+
         $first_cond_referenced_var_ids = $context->referenced_var_ids;
         $context->referenced_var_ids = array_merge(
             $referenced_var_ids,
@@ -96,6 +103,9 @@ class IfChecker
 
         $if_context->inside_conditional = true;
 
+        $assigned_var_ids = $context->assigned_var_ids;
+        $if_context->assigned_var_ids = [];
+
         $referenced_var_ids = $context->referenced_var_ids;
         $if_context->referenced_var_ids = [];
 
@@ -116,10 +126,20 @@ class IfChecker
             $more_cond_referenced_var_ids
         );
 
-        $new_assigned_var_ids = array_diff_key($context->assigned_var_ids, $pre_assigned_var_ids);
+        /** @var array<string, bool> */
+        $more_cond_assigned_var_ids = $if_context->assigned_var_ids;
+        $if_context->assigned_var_ids = array_merge(
+            $more_cond_assigned_var_ids,
+            $assigned_var_ids
+        );
+
+        $cond_assigned_var_ids = array_merge(
+            $first_cond_assigned_var_ids,
+            $more_cond_assigned_var_ids
+        );
 
         // get all the var ids that were referened in the conditional, but not assigned in it
-        $cond_referenced_var_ids = array_diff_key($cond_referenced_var_ids, $new_assigned_var_ids);
+        $cond_referenced_var_ids = array_diff_key($cond_referenced_var_ids, $cond_assigned_var_ids);
 
         $if_context->inside_conditional = false;
 
@@ -132,7 +152,18 @@ class IfChecker
         );
 
         // this will see whether any of the clauses in set A conflict with the clauses in set B
-        AlgebraChecker::checkForParadox($context->clauses, $if_clauses, $statements_checker, $stmt->cond);
+        AlgebraChecker::checkForParadox(
+            $context->clauses,
+            $if_clauses,
+            $statements_checker,
+            $stmt->cond,
+            $cond_assigned_var_ids
+        );
+
+        // if we have assignments in the if, we may have duplicate clauses
+        if ($cond_assigned_var_ids) {
+            $if_clauses = AlgebraChecker::simplifyCNF($if_clauses);
+        }
 
         $if_context->clauses = AlgebraChecker::simplifyCNF(array_merge($context->clauses, $if_clauses));
 
@@ -614,7 +645,13 @@ class IfChecker
         );
 
         // this will see whether any of the clauses in set A conflict with the clauses in set B
-        AlgebraChecker::checkForParadox($entry_clauses, $elseif_clauses, $statements_checker, $elseif->cond);
+        AlgebraChecker::checkForParadox(
+            $entry_clauses,
+            $elseif_clauses,
+            $statements_checker,
+            $elseif->cond,
+            $new_assigned_var_ids
+        );
 
         $elseif_context->clauses = AlgebraChecker::simplifyCNF(
             array_merge(

src/Psalm/Checker/Statements/Block/SwitchChecker.php

@@ -94,7 +94,7 @@ class SwitchChecker
                 );
 
                 // this will see whether any of the clauses in set A conflict with the clauses in set B
-                AlgebraChecker::checkForParadox($context->clauses, $case_clauses, $statements_checker, $stmt->cond);
+                AlgebraChecker::checkForParadox($context->clauses, $case_clauses, $statements_checker, $stmt->cond, []);
 
                 $case_context->clauses = AlgebraChecker::simplifyCNF(array_merge($context->clauses, $case_clauses));
 

tests/RedundantConditionTest.php

@@ -48,6 +48,16 @@ class RedundantConditionTest extends TestCase
                         }
                     }',
             ],
+            'assignmentInIf' => [
+                '<?php
+                    function test(int $x = null) : int {
+                        if (!$x && !($x = rand(0, 10))) {
+                            echo "Failed to get non-empty x\n";
+                            return -1;
+                        }
+                        return $x;
+                    }',
+            ],
         ];
     }