Skip to content

Commit

Permalink
Fix ElseIfAnalyzer inconsistency and remove dead code it found
Browse files Browse the repository at this point in the history
  • Loading branch information
@theodorejb
theodorejb committed Oct 5, 2024
1 parent 522e3db commit 4e20c28
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 36 deletions.
20 changes: 13 additions & 7 deletions src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ public static function analyze(
$codebase,
);

if (count($elseif_clauses) > 200) {
$elseif_clauses = [];
}

$elseif_clauses_handled = [];

foreach ($elseif_clauses as $clause) {
Expand All @@ -101,7 +105,7 @@ public static function analyze(
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);
$clause = new Clause([], $elseif_cond_id, $elseif_cond_id, true);
break 2;
}
}
Expand All @@ -118,7 +122,7 @@ public static function analyze(
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);
$c = new Clause([], $elseif_cond_id, $elseif_cond_id, true);
break 2;
}
}
Expand All @@ -136,21 +140,23 @@ public static function analyze(
$assigned_in_conditional_var_ids,
);

$elseif_context_clauses = [...$entry_clauses, ...$elseif_clauses];
$elseif_clauses = Algebra::simplifyCNF($elseif_clauses);

$elseif_context->clauses = $entry_clauses
? Algebra::simplifyCNF([...$entry_clauses, ...$elseif_clauses])
: $elseif_clauses;

if ($elseif_context->reconciled_expression_clauses) {
$reconciled_expression_clauses = $elseif_context->reconciled_expression_clauses;

$elseif_context_clauses = array_values(
$elseif_context->clauses = array_values(
array_filter(
$elseif_context_clauses,
$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 {
Expand Down
15 changes: 7 additions & 8 deletions src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,16 @@ public static function analyze(
}
}

$branch_point = $context->branch_point ?: (int) $stmt->getAttribute('startFilePos');

try {
$if_conditional_scope = IfConditionalAnalyzer::analyze(
$statements_analyzer,
$stmt->cond,
$context,
$codebase,
$if_scope,
$context->branch_point ?: (int) $stmt->getAttribute('startFilePos'),
$branch_point,
);

// this is the context for stuff that happens within the `if` block
Expand Down Expand Up @@ -140,7 +142,6 @@ public static function analyze(
$if_clauses_handled = [];
foreach ($if_clauses as $clause) {
$keys = array_keys($clause->possibilities);

$mixed_var_ids = array_diff($mixed_var_ids, $keys);

foreach ($keys as $key) {
Expand All @@ -161,7 +162,7 @@ public static function analyze(

// this will see whether any of the clauses in set A conflict with the clauses in set B
AlgebraAnalyzer::checkForParadox(
$context->clauses,
$entry_clauses,
$if_clauses,
$statements_analyzer,
$stmt->cond,
Expand All @@ -170,19 +171,17 @@ public static function analyze(

$if_clauses = Algebra::simplifyCNF($if_clauses);

$if_context_clauses = [...$entry_clauses, ...$if_clauses];

$if_context->clauses = $entry_clauses
? Algebra::simplifyCNF($if_context_clauses)
: $if_context_clauses;
? Algebra::simplifyCNF([...$entry_clauses, ...$if_clauses])
: $if_clauses;

if ($if_context->reconciled_expression_clauses) {
$reconciled_expression_clauses = $if_context->reconciled_expression_clauses;

$if_context->clauses = array_values(
array_filter(
$if_context->clauses,
static fn(Clause $c): bool => !in_array($c->hash, $reconciled_expression_clauses),
static fn(Clause $c): bool => !in_array($c->hash, $reconciled_expression_clauses, true),
),
);

Expand Down
29 changes: 10 additions & 19 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/ArrayAssignmentAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -572,8 +572,6 @@ private static function updateArrayAssignmentChildType(

$from_countable_object_like = false;

$new_child_type = null;

$array_atomic_type = null;
if (!$current_dim && !$context->inside_loop) {
$atomic_root_types = $root_type->getAtomicTypes();
Expand Down Expand Up @@ -638,11 +636,6 @@ private static function updateArrayAssignmentChildType(
$array_atomic_type_array,
count($atomic_root_type_array->properties),
);
} elseif ($atomic_root_type_array->is_list) {
$array_atomic_type = $atomic_root_type_array;
$new_child_type = new Union([$array_atomic_type], [
'parent_nodes' => $root_type->parent_nodes,
]);
} else {
assert($array_atomic_type_list !== null);
$array_atomic_type = array_fill(
Expand Down Expand Up @@ -685,18 +678,16 @@ private static function updateArrayAssignmentChildType(

$array_assignment_type = new Union([$array_atomic_type]);

if (!$new_child_type) {
if ($templated_assignment) {
$new_child_type = $root_type;
} else {
$new_child_type = Type::combineUnionTypes(
$root_type,
$array_assignment_type,
$codebase,
true,
true,
);
}
if ($templated_assignment) {
$new_child_type = $root_type;
} else {
$new_child_type = Type::combineUnionTypes(
$root_type,
$array_assignment_type,
$codebase,
true,
true,
);
}

if ($from_countable_object_like) {
Expand Down
4 changes: 2 additions & 2 deletions src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ static function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause {

// this will see whether any of the clauses in set A conflict with the clauses in set B
AlgebraAnalyzer::checkForParadox(
$context->clauses,
$entry_clauses,
$if_clauses,
$statements_analyzer,
$stmt->cond,
Expand All @@ -139,7 +139,7 @@ static function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause {
$ternary_context_clauses = array_values(
array_filter(
$ternary_context_clauses,
static fn(Clause $c): bool => !in_array($c->hash, $reconciled_expression_clauses),
static fn(Clause $c): bool => !in_array($c->hash, $reconciled_expression_clauses, true),
),
);

Expand Down

0 comments on commit 4e20c28

Please sign in to comment.