diff --git a/src/Psalm/Checker/AlgebraChecker.php b/src/Psalm/Checker/AlgebraChecker.php index 72474a875..ada76c165 100644 --- a/src/Psalm/Checker/AlgebraChecker.php +++ b/src/Psalm/Checker/AlgebraChecker.php @@ -194,6 +194,7 @@ class AlgebraChecker * @param array $formula2 * @param StatementsChecker $statements_checker, * @param PhpParser\Node $stmt + * @param array $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( diff --git a/src/Psalm/Checker/Statements/Block/IfChecker.php b/src/Psalm/Checker/Statements/Block/IfChecker.php index d3ac145dc..519f4a2e6 100644 --- a/src/Psalm/Checker/Statements/Block/IfChecker.php +++ b/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 */ + $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( diff --git a/src/Psalm/Checker/Statements/Block/SwitchChecker.php b/src/Psalm/Checker/Statements/Block/SwitchChecker.php index 1b4fb669c..84cf2c60d 100644 --- a/src/Psalm/Checker/Statements/Block/SwitchChecker.php +++ b/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)); diff --git a/tests/RedundantConditionTest.php b/tests/RedundantConditionTest.php index ad14acbd7..4856db645 100644 --- a/tests/RedundantConditionTest.php +++ b/tests/RedundantConditionTest.php @@ -48,6 +48,16 @@ class RedundantConditionTest extends TestCase } }', ], + 'assignmentInIf' => [ + '