Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix type reconciliation breaking Context::$references_in_scope (fixes #8289). #8306

Merged
merged 4 commits into from Jul 21, 2022
Merged

Fix type reconciliation breaking Context::$references_in_scope (fixes #8289). #8306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ac315fb
Demonstrate flaw in type reconciliation on array references
ohader Jul 20, 2022
6c5e5f7
Avoid cloning undefined array index
ohader Jul 20, 2022
98502d4
Assert ReconcilerTest::arrayReferencesAreHandled reaches the end
ohader Jul 20, 2022
ba6270c
Fix type reconciliation breaking Context::$references_in_scope (fixes…
AndrolGenhald Jul 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Psalm/Internal/Analyzer/Statements/Block/DoAnalyzer.php
View file
Open in desktop
Expand Up @@ -123,7 +123,7 @@ static function (Clause $c) use ($mixed_var_ids): bool {
if ($negated_while_types) {
$changed_var_ids = [];

$inner_loop_context->vars_in_scope =
[$inner_loop_context->vars_in_scope, $inner_loop_context->references_in_scope] =
Reconciler::reconcileKeyedTypes(
$negated_while_types,
[],
Expand Down
3 changes: 2 additions & 1 deletion src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php
View file
Open in desktop
Expand Up @@ -48,7 +48,7 @@ public static function analyze(
$changed_var_ids = [];

if ($if_scope->negated_types) {
$vars_reconciled = Reconciler::reconcileKeyedTypes(
[$vars_reconciled, $references_reconciled] = Reconciler::reconcileKeyedTypes(
$if_scope->negated_types,
[],
$outer_context->vars_in_scope,
Expand All @@ -71,6 +71,7 @@ public static function analyze(
if ($changed_var_ids) {
$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(
Expand Down
4 changes: 1 addition & 3 deletions src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseAnalyzer.php
View file
Open in desktop
Expand Up @@ -64,7 +64,7 @@ public static function analyze(
if ($else_types) {
$changed_var_ids = [];

$else_vars_reconciled = Reconciler::reconcileKeyedTypes(
[$else_context->vars_in_scope, $else_context->references_in_scope] = Reconciler::reconcileKeyedTypes(
$else_types,
[],
$else_context->vars_in_scope,
Expand All @@ -79,8 +79,6 @@ public static function analyze(
: null
);

$else_context->vars_in_scope = $else_vars_reconciled;

$else_context->clauses = Context::removeReconciledClauses($else_context->clauses, $changed_var_ids)[0];

foreach ($changed_var_ids as $changed_var_id => $_) {
Expand Down
31 changes: 14 additions & 17 deletions src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php
View file
Open in desktop
Expand Up @@ -215,7 +215,7 @@ public static function analyze(

// if the elseif has an || in the conditional, we cannot easily reason about it
if ($reconcilable_elseif_types) {
$elseif_vars_reconciled = Reconciler::reconcileKeyedTypes(
[$elseif_context->vars_in_scope, $elseif_context->references_in_scope] = Reconciler::reconcileKeyedTypes(
$reconcilable_elseif_types,
$active_elseif_types,
$elseif_context->vars_in_scope,
Expand All @@ -234,8 +234,6 @@ public static function analyze(
)
);

$elseif_context->vars_in_scope = $elseif_vars_reconciled;

if ($newly_reconciled_var_ids) {
$elseif_context->clauses = Context::removeReconciledClauses(
$elseif_context->clauses,
Expand Down Expand Up @@ -349,21 +347,20 @@ public static function analyze(
if ($has_leaving_statements) {
$newly_reconciled_var_ids = [];

$leaving_vars_reconciled = 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)
);

$implied_outer_context = clone $elseif_context;
$implied_outer_context->vars_in_scope = $leaving_vars_reconciled;
[$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)
);

$updated_vars = [];

Expand Down
41 changes: 19 additions & 22 deletions src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php
View file
Open in desktop
Expand Up @@ -93,28 +93,25 @@ public static function analyze(
if ($reconcilable_if_types) {
$changed_var_ids = [];

$if_vars_in_scope_reconciled =
Reconciler::reconcileKeyedTypes(
$reconcilable_if_types,
$active_if_types,
$if_context->vars_in_scope,
$if_context->references_in_scope,
$changed_var_ids,
$cond_referenced_var_ids,
$statements_analyzer,
$statements_analyzer->getTemplateTypeMap() ?: [],
$if_context->inside_loop,
$outer_context->check_variables
? new CodeLocation(
$statements_analyzer->getSource(),
$stmt->cond instanceof PhpParser\Node\Expr\BooleanNot
? $stmt->cond->expr
: $stmt->cond,
$outer_context->include_location
) : null
);

$if_context->vars_in_scope = $if_vars_in_scope_reconciled;
[$if_context->vars_in_scope, $if_context->references_in_scope] = Reconciler::reconcileKeyedTypes(
$reconcilable_if_types,
$active_if_types,
$if_context->vars_in_scope,
$if_context->references_in_scope,
$changed_var_ids,
$cond_referenced_var_ids,
$statements_analyzer,
$statements_analyzer->getTemplateTypeMap() ?: [],
$if_context->inside_loop,
$outer_context->check_variables
? new CodeLocation(
$statements_analyzer->getSource(),
$stmt->cond instanceof PhpParser\Node\Expr\BooleanNot
? $stmt->cond->expr
: $stmt->cond,
$outer_context->include_location
) : null
);

foreach ($reconcilable_if_types as $var_id => $_) {
$if_context->vars_possibly_in_scope[$var_id] = true;
Expand Down
41 changes: 20 additions & 21 deletions src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php
View file
Open in desktop
Expand Up @@ -228,27 +228,26 @@ public static function analyze(
$changed_var_ids = [];

if ($if_scope->negated_types) {
$else_vars_reconciled = Reconciler::reconcileKeyedTypes(
$if_scope->negated_types,
[],
$temp_else_context->vars_in_scope,
$temp_else_context->references_in_scope,
$changed_var_ids,
[],
$statements_analyzer,
$statements_analyzer->getTemplateTypeMap() ?: [],
$context->inside_loop,
$context->check_variables
? new CodeLocation(
$statements_analyzer->getSource(),
$stmt->cond instanceof PhpParser\Node\Expr\BooleanNot
? $stmt->cond->expr
: $stmt->cond,
$context->include_location
) : null
);

$temp_else_context->vars_in_scope = $else_vars_reconciled;
[$temp_else_context->vars_in_scope, $temp_else_context->references_in_scope] =
Reconciler::reconcileKeyedTypes(
$if_scope->negated_types,
[],
$temp_else_context->vars_in_scope,
$temp_else_context->references_in_scope,
$changed_var_ids,
[],
$statements_analyzer,
$statements_analyzer->getTemplateTypeMap() ?: [],
$context->inside_loop,
$context->check_variables
? new CodeLocation(
$statements_analyzer->getSource(),
$stmt->cond instanceof PhpParser\Node\Expr\BooleanNot
? $stmt->cond->expr
: $stmt->cond,
$context->include_location
) : null
);
}

// we calculate the vars redefined in a hypothetical else statement to determine
Expand Down
6 changes: 2 additions & 4 deletions src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php
View file
Open in desktop
Expand Up @@ -469,7 +469,7 @@ public static function analyze(
if ($negated_pre_condition_types) {
$changed_var_ids = [];

$vars_in_scope_reconciled = Reconciler::reconcileKeyedTypes(
[$vars_in_scope_reconciled, $_] = Reconciler::reconcileKeyedTypes(
$negated_pre_condition_types,
[],
$continue_context->vars_in_scope,
Expand Down Expand Up @@ -613,7 +613,7 @@ private static function applyPreConditionToLoopContext(
$changed_var_ids = [];

if ($reconcilable_while_types) {
$pre_condition_vars_in_scope_reconciled = Reconciler::reconcileKeyedTypes(
[$loop_context->vars_in_scope, $loop_context->references_in_scope] = Reconciler::reconcileKeyedTypes(
$reconcilable_while_types,
$active_while_types,
$loop_context->vars_in_scope,
Expand All @@ -625,8 +625,6 @@ private static function applyPreConditionToLoopContext(
true,
new CodeLocation($statements_analyzer->getSource(), $pre_condition)
);

$loop_context->vars_in_scope = $pre_condition_vars_in_scope_reconciled;
}

if ($is_do) {
Expand Down
2 changes: 1 addition & 1 deletion src/Psalm/Internal/Analyzer/Statements/Block/SwitchAnalyzer.php
View file
Open in desktop
Expand Up @@ -154,7 +154,7 @@ public static function analyze(
if ($reconcilable_if_types && isset($reconcilable_if_types[$switch_var_id])) {
$changed_var_ids = [];

$case_vars_in_scope_reconciled =
[$case_vars_in_scope_reconciled, $_] =
Reconciler::reconcileKeyedTypes(
$reconcilable_if_types,
[],
Expand Down
3 changes: 2 additions & 1 deletion src/Psalm/Internal/Analyzer/Statements/Block/SwitchCaseAnalyzer.php
View file
Open in desktop
Expand Up @@ -400,7 +400,7 @@ public static function analyze(
$statements_analyzer->addSuppressedIssues(['RedundantConditionGivenDocblockType']);
}

$case_vars_in_scope_reconciled =
[$case_vars_in_scope_reconciled, $case_references_in_scope_reconciled] =
Reconciler::reconcileKeyedTypes(
$reconcilable_if_types,
[],
Expand All @@ -427,6 +427,7 @@ public static function analyze(
}

$case_context->vars_in_scope = $case_vars_in_scope_reconciled;
$case_context->references_in_scope = $case_references_in_scope_reconciled;
foreach ($reconcilable_if_types as $var_id => $_) {
$case_context->vars_possibly_in_scope[$var_id] = true;
}
Expand Down
4 changes: 1 addition & 3 deletions src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/AndAnalyzer.php
View file
Open in desktop
Expand Up @@ -134,7 +134,7 @@ public static function analyze(
$right_context = clone $context;
// while in an and, we allow scope to boil over to support
// statements of the form if ($x && $x->foo())
$right_vars_in_scope = Reconciler::reconcileKeyedTypes(
[$right_context->vars_in_scope, $right_context->references_in_scope] = Reconciler::reconcileKeyedTypes(
$left_type_assertions,
$active_left_assertions,
$right_context->vars_in_scope,
Expand All @@ -147,8 +147,6 @@ public static function analyze(
new CodeLocation($statements_analyzer->getSource(), $stmt->left),
$context->inside_negation
);

$right_context->vars_in_scope = $right_vars_in_scope;
} else {
$right_context = clone $left_context;
}
Expand Down
3 changes: 1 addition & 2 deletions src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php
View file
Open in desktop
Expand Up @@ -221,7 +221,7 @@ public static function analyze(
if ($negated_type_assertions) {
// while in an or, we allow scope to boil over to support
// statements of the form if ($x === null || $x->foo())
$right_vars_in_scope = Reconciler::reconcileKeyedTypes(
[$right_context->vars_in_scope, $right_context->references_in_scope] = Reconciler::reconcileKeyedTypes(
$negated_type_assertions,
$active_negated_type_assertions,
$right_context->vars_in_scope,
Expand All @@ -234,7 +234,6 @@ public static function analyze(
new CodeLocation($statements_analyzer->getSource(), $stmt->left),
!$context->inside_negation
);
$right_context->vars_in_scope = $right_vars_in_scope;
}

$right_context->clauses = $clauses_for_right_analysis;
Expand Down
3 changes: 2 additions & 1 deletion src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php
View file
Open in desktop
Expand Up @@ -957,7 +957,7 @@ private static function processAssertFunctionEffects(
if ($assert_type_assertions) {
// while in an and, we allow scope to boil over to support
// statements of the form if ($x && $x->foo())
$op_vars_in_scope = Reconciler::reconcileKeyedTypes(
[$op_vars_in_scope, $op_references_in_scope] = Reconciler::reconcileKeyedTypes(
$assert_type_assertions,
$assert_type_assertions,
$context->vars_in_scope,
Expand Down Expand Up @@ -1003,6 +1003,7 @@ private static function processAssertFunctionEffects(
}

$context->vars_in_scope = $op_vars_in_scope;
$context->references_in_scope = $op_references_in_scope;
}

if ($changed_var_ids) {
Expand Down
3 changes: 2 additions & 1 deletion src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php
View file
Open in desktop
Expand Up @@ -899,7 +899,7 @@ public static function applyAssertionsToContext(

// while in an and, we allow scope to boil over to support
// statements of the form if ($x && $x->foo())
$op_vars_in_scope = Reconciler::reconcileKeyedTypes(
[$op_vars_in_scope, $op_references_in_scope] = Reconciler::reconcileKeyedTypes(
$type_assertions,
$type_assertions,
$context->vars_in_scope,
Expand Down Expand Up @@ -954,6 +954,7 @@ public static function applyAssertionsToContext(
}

$context->vars_in_scope = $op_vars_in_scope;
$context->references_in_scope = $op_references_in_scope;
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/Psalm/Internal/Analyzer/Statements/Expression/MatchAnalyzer.php
View file
Open in desktop
Expand Up @@ -251,7 +251,7 @@ public static function analyze(
if ($reconcilable_types) {
$changed_var_ids = [];

$vars_in_scope_reconciled = Reconciler::reconcileKeyedTypes(
[$vars_in_scope_reconciled, $_] = Reconciler::reconcileKeyedTypes(
$reconcilable_types,
[],
$context->vars_in_scope,
Expand Down
8 changes: 2 additions & 6 deletions src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php
View file
Open in desktop
Expand Up @@ -191,7 +191,7 @@ static function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause {
$changed_var_ids = [];

if ($reconcilable_if_types) {
$if_vars_in_scope_reconciled = Reconciler::reconcileKeyedTypes(
[$if_context->vars_in_scope, $if_context->references_in_scope] = Reconciler::reconcileKeyedTypes(
$reconcilable_if_types,
$active_if_types,
$if_context->vars_in_scope,
Expand All @@ -203,8 +203,6 @@ static function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause {
$if_context->inside_loop,
new CodeLocation($statements_analyzer->getSource(), $stmt->cond)
);

$if_context->vars_in_scope = $if_vars_in_scope_reconciled;
}

$t_else_context = clone $context;
Expand All @@ -230,7 +228,7 @@ static function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause {
$changed_var_ids = [];

if ($if_scope->negated_types) {
$else_vars_reconciled = Reconciler::reconcileKeyedTypes(
[$t_else_context->vars_in_scope, $t_else_context->references_in_scope] = Reconciler::reconcileKeyedTypes(
$if_scope->negated_types,
$if_scope->negated_types,
$t_else_context->vars_in_scope,
Expand All @@ -243,8 +241,6 @@ static function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause {
new CodeLocation($statements_analyzer->getSource(), $stmt->else)
);

$t_else_context->vars_in_scope = $else_vars_reconciled;

$t_else_context->clauses = Context::removeReconciledClauses($t_else_context->clauses, $changed_var_ids)[0];
}

Expand Down
4 changes: 2 additions & 2 deletions src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php
View file
Open in desktop
Expand Up @@ -213,7 +213,7 @@ static function ($keyed_type) use ($statements_source, $context) {
'$inner_type' => $assertions[$assertion_id],
];

$reconciled_types = Reconciler::reconcileKeyedTypes(
[$reconciled_types, $_] = Reconciler::reconcileKeyedTypes(
$assertions,
$assertions,
['$inner_type' => $inner_type],
Expand Down Expand Up @@ -293,7 +293,7 @@ static function ($keyed_type) use ($statements_source, $context) {

$assertions = ['$inner_type' => $assertions['$' . $first_param->var->name]];

$reconciled_types = Reconciler::reconcileKeyedTypes(
[$reconciled_types, $_] = Reconciler::reconcileKeyedTypes(
$assertions,
$assertions,
['$inner_type' => $inner_type],
Expand Down