diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php index 843bbbce0de..490d51a279d 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php @@ -41,12 +41,9 @@ public static function analyze( IfScope $if_scope, int $branch_point, ): IfConditionalScope { - $entry_clauses = []; // used when evaluating elseifs if ($if_scope->negated_clauses) { - $entry_clauses = [...$outer_context->clauses, ...$if_scope->negated_clauses]; - $changed_var_ids = []; if ($if_scope->negated_types) { @@ -74,15 +71,6 @@ public static function analyze( $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( - $entry_clauses, - static fn(Clause $c): bool => count($c->possibilities) > 1 - || $c->wedge - || !isset($changed_var_ids[array_keys($c->possibilities)[0]]) - ), - ); } } } @@ -226,7 +214,6 @@ public static function analyze( $post_if_context, $cond_referenced_var_ids, $assigned_in_conditional_var_ids, - $entry_clauses, ); } diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php deleted file mode 100644 index 4a7769247b2..00000000000 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php +++ /dev/null @@ -1,422 +0,0 @@ -cond, - $else_context, - $codebase, - $if_scope, - $branch_point, - ); - - $elseif_context = $if_conditional_scope->if_context; - $cond_referenced_var_ids = $if_conditional_scope->cond_referenced_var_ids; - $assigned_in_conditional_var_ids = $if_conditional_scope->assigned_in_conditional_var_ids; - } catch (ScopeAnalysisException $e) { - return false; - } - - $mixed_var_ids = []; - - foreach ($elseif_context->vars_in_scope as $var_id => $type) { - if ($type->hasMixed()) { - $mixed_var_ids[] = $var_id; - } - } - - $elseif_cond_id = spl_object_id($elseif->cond); - - $elseif_clauses = FormulaGenerator::getFormula( - $elseif_cond_id, - $elseif_cond_id, - $elseif->cond, - $else_context->self, - $statements_analyzer, - $codebase, - ); - - $elseif_clauses_handled = []; - - foreach ($elseif_clauses as $clause) { - $keys = array_keys($clause->possibilities); - $mixed_var_ids = array_diff($mixed_var_ids, $keys); - - foreach ($keys as $key) { - foreach ($mixed_var_ids as $mixed_var_id) { - if (preg_match('/^' . preg_quote($mixed_var_id, '/') . '(\[|-)/', $key)) { - $elseif_clauses_handled[] = new Clause([], $elseif_cond_id, $elseif_cond_id, true); - break 2; - } - } - } - - $elseif_clauses_handled[] = $clause; - } - - $elseif_clauses = $elseif_clauses_handled; - - $entry_clauses = []; - - foreach ($if_conditional_scope->entry_clauses as $c) { - foreach ($c->possibilities as $key => $_value) { - foreach ($assigned_in_conditional_var_ids as $conditional_assigned_var_id => $_) { - if (preg_match('/^'.preg_quote($conditional_assigned_var_id, '/').'(\[|-|$)/', $key)) { - $entry_clauses[] = new Clause([], $elseif_cond_id, $elseif_cond_id, true); - break 2; - } - } - } - - $entry_clauses[] = $c; - } - - // this will see whether any of the clauses in set A conflict with the clauses in set B - AlgebraAnalyzer::checkForParadox( - $entry_clauses, - $elseif_clauses, - $statements_analyzer, - $elseif->cond, - $assigned_in_conditional_var_ids, - ); - - $elseif_context_clauses = [...$entry_clauses, ...$elseif_clauses]; - - if ($elseif_context->reconciled_expression_clauses) { - $reconciled_expression_clauses = $elseif_context->reconciled_expression_clauses; - - $elseif_context_clauses = array_values( - array_filter( - $elseif_context_clauses, - static fn(Clause $c): bool => !in_array($c->hash, $reconciled_expression_clauses, true) - ), - ); - } - - $elseif_context->clauses = Algebra::simplifyCNF($elseif_context_clauses); - - $active_elseif_types = []; - - try { - if (array_filter( - $entry_clauses, - static fn(Clause $clause): bool => (bool) $clause->possibilities, - )) { - $omit_keys = array_reduce( - $entry_clauses, - /** - * @param array $carry - * @return array - */ - static fn(array $carry, Clause $clause): array - => array_merge($carry, array_keys($clause->possibilities)), - [], - ); - - $omit_keys = array_combine($omit_keys, $omit_keys); - $omit_keys = array_diff_key($omit_keys, Algebra::getTruthsFromFormula($entry_clauses)); - - $cond_referenced_var_ids = array_diff_key( - $cond_referenced_var_ids, - $omit_keys, - ); - } - $reconcilable_elseif_types = Algebra::getTruthsFromFormula( - $elseif_context->clauses, - spl_object_id($elseif->cond), - $cond_referenced_var_ids, - $active_elseif_types, - ); - $negated_elseif_types = Algebra::getTruthsFromFormula( - Algebra::negateFormula($elseif_clauses), - ); - } catch (ComplicatedExpressionException $e) { - $reconcilable_elseif_types = []; - $negated_elseif_types = []; - } - - $all_negated_vars = array_unique( - [...array_keys($negated_elseif_types), ...array_keys($if_scope->negated_types)], - ); - - foreach ($all_negated_vars as $var_id) { - if (isset($negated_elseif_types[$var_id])) { - if (isset($if_scope->negated_types[$var_id])) { - $if_scope->negated_types[$var_id] = [ - ...$if_scope->negated_types[$var_id], - ...$negated_elseif_types[$var_id], - ]; - } else { - $if_scope->negated_types[$var_id] = $negated_elseif_types[$var_id]; - } - } - } - - $newly_reconciled_var_ids = []; - - // if the elseif has an || in the conditional, we cannot easily reason about it - if ($reconcilable_elseif_types) { - [$elseif_context->vars_in_scope, $elseif_context->references_in_scope] = Reconciler::reconcileKeyedTypes( - $reconcilable_elseif_types, - $active_elseif_types, - $elseif_context->vars_in_scope, - $elseif_context->references_in_scope, - $newly_reconciled_var_ids, - $cond_referenced_var_ids, - $statements_analyzer, - $statements_analyzer->getTemplateTypeMap() ?: [], - $elseif_context->inside_loop, - new CodeLocation( - $statements_analyzer->getSource(), - $elseif->cond instanceof PhpParser\Node\Expr\BooleanNot - ? $elseif->cond->expr - : $elseif->cond, - $outer_context->include_location, - ), - ); - - if ($newly_reconciled_var_ids) { - $elseif_context->clauses = Context::removeReconciledClauses( - $elseif_context->clauses, - $newly_reconciled_var_ids, - )[0]; - - foreach ($newly_reconciled_var_ids as $changed_var_id => $_) { - foreach ($elseif_context->vars_in_scope as $var_id => $_) { - if (preg_match('/' . preg_quote($changed_var_id, '/') . '[\]\[\-]/', $var_id) - && !array_key_exists($var_id, $newly_reconciled_var_ids) - && !array_key_exists($var_id, $cond_referenced_var_ids) - ) { - $elseif_context->removePossibleReference($var_id); - } - } - } - } - } - - $pre_stmts_assigned_var_ids = $elseif_context->assigned_var_ids; - $elseif_context->assigned_var_ids = []; - $pre_stmts_possibly_assigned_var_ids = $elseif_context->possibly_assigned_var_ids; - $elseif_context->possibly_assigned_var_ids = []; - - if ($statements_analyzer->analyze( - $elseif->stmts, - $elseif_context, - ) === false - ) { - return false; - } - - foreach ($elseif_context->parent_remove_vars as $var_id => $_) { - $outer_context->removeVarFromConflictingClauses($var_id); - } - - /** @var array */ - $new_stmts_assigned_var_ids = $elseif_context->assigned_var_ids; - $elseif_context->assigned_var_ids = $pre_stmts_assigned_var_ids + $new_stmts_assigned_var_ids; - - /** @var array */ - $new_stmts_possibly_assigned_var_ids = $elseif_context->possibly_assigned_var_ids; - $elseif_context->possibly_assigned_var_ids = - $pre_stmts_possibly_assigned_var_ids + $new_stmts_possibly_assigned_var_ids; - - foreach ($elseif_context->byref_constraints as $var_id => $byref_constraint) { - if (isset($outer_context->byref_constraints[$var_id]) - && ($outer_constraint_type = $outer_context->byref_constraints[$var_id]->type) - && $byref_constraint->type - && !UnionTypeComparator::isContainedBy( - $codebase, - $byref_constraint->type, - $outer_constraint_type, - ) - ) { - IssueBuffer::maybeAdd( - new ConflictingReferenceConstraint( - 'There is more than one pass-by-reference constraint on ' . $var_id, - new CodeLocation($statements_analyzer, $elseif, $outer_context->include_location, true), - ), - $statements_analyzer->getSuppressedIssues(), - ); - } else { - $outer_context->byref_constraints[$var_id] = $byref_constraint; - } - } - - $final_actions = ScopeAnalyzer::getControlActions( - $elseif->stmts, - $statements_analyzer->node_data, - [], - ); - // has a return/throw at end - $has_ending_statements = $final_actions === [ScopeAnalyzer::ACTION_END]; - $has_leaving_statements = $has_ending_statements - || (count($final_actions) && !in_array(ScopeAnalyzer::ACTION_NONE, $final_actions, true)); - - $has_break_statement = $final_actions === [ScopeAnalyzer::ACTION_BREAK]; - $has_continue_statement = $final_actions === [ScopeAnalyzer::ACTION_CONTINUE]; - - $if_scope->final_actions = array_merge($final_actions, $if_scope->final_actions); - - // update the parent context as necessary - if (!$has_leaving_statements) { - IfAnalyzer::updateIfScope( - $codebase, - $if_scope, - $elseif_context, - $outer_context, - array_merge($new_stmts_assigned_var_ids, $assigned_in_conditional_var_ids), - $new_stmts_possibly_assigned_var_ids, - $newly_reconciled_var_ids, - ); - - $reasonable_clause_count = count($if_scope->reasonable_clauses); - - if ($reasonable_clause_count && $reasonable_clause_count < 20_000 && $elseif_clauses) { - $if_scope->reasonable_clauses = Algebra::combineOredClauses( - $if_scope->reasonable_clauses, - $elseif_clauses, - $elseif_cond_id, - ); - } else { - $if_scope->reasonable_clauses = []; - } - } else { - $if_scope->reasonable_clauses = []; - } - - if ($negated_elseif_types) { - if ($has_leaving_statements) { - $newly_reconciled_var_ids = []; - - $implied_outer_context = clone $elseif_context; - [$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), - ); - } - } - - if (!$has_ending_statements) { - $vars_possibly_in_scope = array_diff_key( - $elseif_context->vars_possibly_in_scope, - $outer_context->vars_possibly_in_scope, - ); - - $possibly_assigned_var_ids = $new_stmts_possibly_assigned_var_ids; - - if ($has_leaving_statements && $elseif_context->loop_scope) { - if (!$has_continue_statement && !$has_break_statement) { - $if_scope->new_vars_possibly_in_scope = array_merge( - $vars_possibly_in_scope, - $if_scope->new_vars_possibly_in_scope, - ); - $if_scope->possibly_assigned_var_ids = array_merge( - $possibly_assigned_var_ids, - $if_scope->possibly_assigned_var_ids, - ); - } - - $elseif_context->loop_scope->vars_possibly_in_scope = array_merge( - $vars_possibly_in_scope, - $elseif_context->loop_scope->vars_possibly_in_scope, - ); - } elseif (!$has_leaving_statements) { - $if_scope->new_vars_possibly_in_scope = array_merge( - $vars_possibly_in_scope, - $if_scope->new_vars_possibly_in_scope, - ); - $if_scope->possibly_assigned_var_ids = array_merge( - $possibly_assigned_var_ids, - $if_scope->possibly_assigned_var_ids, - ); - } - } - - if ($outer_context->collect_exceptions) { - $outer_context->mergeExceptions($elseif_context); - } - - try { - $if_scope->negated_clauses = Algebra::simplifyCNF( - [...$if_scope->negated_clauses, ...Algebra::negateFormula($elseif_clauses)], - ); - } catch (ComplicatedExpressionException $e) { - $if_scope->negated_clauses = []; - } - - // Track references set in the elseif to make sure they aren't reused later - $outer_context->updateReferencesPossiblyFromConfusingScope( - $elseif_context, - $statements_analyzer, - ); - - return null; - } -} diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php index 273842a3722..c6a206b0ed9 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php @@ -15,7 +15,6 @@ use Psalm\Internal\Analyzer\FunctionLikeAnalyzer; use Psalm\Internal\Analyzer\ScopeAnalyzer; use Psalm\Internal\Analyzer\Statements\Block\IfElse\ElseAnalyzer; -use Psalm\Internal\Analyzer\Statements\Block\IfElse\ElseIfAnalyzer; use Psalm\Internal\Analyzer\Statements\Block\IfElse\IfAnalyzer; use Psalm\Internal\Analyzer\StatementsAnalyzer; use Psalm\Internal\Analyzer\TraitAnalyzer; @@ -31,6 +30,7 @@ use function array_intersect_key; use function array_keys; use function array_merge; +use function array_slice; use function array_unique; use function array_values; use function count; @@ -278,21 +278,17 @@ public static function analyze( [...$else_context->clauses, ...$if_scope->negated_clauses], ); - // check the elseifs - foreach ($stmt->elseifs as $elseif) { - if (ElseIfAnalyzer::analyze( - $statements_analyzer, - $elseif, - $if_scope, - $else_context, - $context, - $codebase, - $else_context->branch_point ?: (int) $stmt->getAttribute('startFilePos'), - ) === false) { - return false; - } + $else = $stmt->else; + // convert elseifs to if/else stmts + $elseifs = array_values($stmt->elseifs); + if ($elseifs) { + $newStmt = new PhpParser\Node\Stmt\If_($elseifs[0]->cond, [ + 'stmts' => $elseifs[0]->stmts, + 'elseifs' => array_slice($elseifs, 1), + 'else' => $stmt->else, + ], $elseifs[0]->getAttributes()); + $else = new PhpParser\Node\Stmt\Else_([$newStmt], $elseifs[0]->getAttributes()); } - if ($stmt->else) { if ($codebase->alter_code) { $else_context->branch_point = @@ -302,7 +298,7 @@ public static function analyze( if (ElseAnalyzer::analyze( $statements_analyzer, - $stmt->else, + $else, $if_scope, $else_context, $context, @@ -310,9 +306,7 @@ public static function analyze( return false; } - if (count($if_scope->if_actions) && !in_array(ScopeAnalyzer::ACTION_NONE, $if_scope->if_actions, true) - && !$stmt->elseifs - ) { + if (count($if_scope->if_actions) && !in_array(ScopeAnalyzer::ACTION_NONE, $if_scope->if_actions, true)) { $context->clauses = $else_context->clauses; foreach ($else_context->vars_in_scope as $var_id => $type) { $context->vars_in_scope[$var_id] = $type; diff --git a/src/Psalm/Internal/Scope/IfConditionalScope.php b/src/Psalm/Internal/Scope/IfConditionalScope.php index 386d8daf034..0f2d6d66f43 100644 --- a/src/Psalm/Internal/Scope/IfConditionalScope.php +++ b/src/Psalm/Internal/Scope/IfConditionalScope.php @@ -26,25 +26,19 @@ final class IfConditionalScope */ public array $assigned_in_conditional_var_ids; - /** @var list */ - public array $entry_clauses; - /** * @param array $cond_referenced_var_ids * @param array $assigned_in_conditional_var_ids - * @param list $entry_clauses */ public function __construct( Context $if_context, Context $post_if_context, array $cond_referenced_var_ids, array $assigned_in_conditional_var_ids, - array $entry_clauses, ) { $this->if_context = $if_context; $this->post_if_context = $post_if_context; $this->cond_referenced_var_ids = $cond_referenced_var_ids; $this->assigned_in_conditional_var_ids = $assigned_in_conditional_var_ids; - $this->entry_clauses = $entry_clauses; } } diff --git a/tests/TypeReconciliation/TypeAlgebraTest.php b/tests/TypeReconciliation/TypeAlgebraTest.php index 522096eb3eb..d8465c656a2 100644 --- a/tests/TypeReconciliation/TypeAlgebraTest.php +++ b/tests/TypeReconciliation/TypeAlgebraTest.php @@ -1338,7 +1338,7 @@ function foo(?string $a, ?string $b, ?string $c): string { if (!$a) return $b; return $a; }', - 'error_message' => 'TypeDoesNotContainType', + 'error_message' => 'NullableReturnStatement', ], 'twoVarLogicNotNestedWithElseifIncorrectlyReinforcedInIf' => [ 'code' => '