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

Remove mic-drop hack from if analysis #7484

Merged
merged 9 commits into from Jan 28, 2022
Merged

Remove mic-drop hack from if analysis #7484

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
92bddb9
Remove mic-drop hack from if analysis
muglug Jan 25, 2022
ab0bec8
Remove more special handling
muglug Jan 25, 2022
5659596
Remove some unnecessary ElseAnalyzer code
muglug Jan 26, 2022
c630dda
Add back necessary code
muglug Jan 26, 2022
7aca2ba
Fix return type of method never returning null
muglug Jan 26, 2022
a421988
Add a comment
muglug Jan 27, 2022
9c6f380
Simplify && handling
muglug Jan 28, 2022
be7de56
Add comments to make stuff clearer
muglug Jan 28, 2022
941f736
Move if-specfic logic to more appropriate setting
muglug Jan 28, 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/Context.php
View file
Open in desktop
Expand Up @@ -349,7 +349,7 @@ class Context
/**
* @var Context|null
*/
public $if_context;
public $if_body_context;

/**
* @var bool
Expand Down
23 changes: 12 additions & 11 deletions src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php
View file
Open in desktop
Expand Up @@ -114,14 +114,12 @@ public static function analyze(

$outer_context->inside_conditional = true;

if ($externally_applied_if_cond_expr) {
if (ExpressionAnalyzer::analyze(
$statements_analyzer,
$externally_applied_if_cond_expr,
$outer_context
) === false) {
throw new ScopeAnalysisException();
}
if (ExpressionAnalyzer::analyze(
$statements_analyzer,
$externally_applied_if_cond_expr,
$outer_context
) === false) {
throw new ScopeAnalysisException();
}

$first_cond_assigned_var_ids = $outer_context->assigned_var_ids;
Expand All @@ -143,7 +141,10 @@ public static function analyze(
}

$if_conditional_context = clone $if_context;
$if_conditional_context->if_context = $if_context;

// here we set up a context specifically for the statements in the first `if`, which can
// be affected by statements in the if condition
$if_conditional_context->if_body_context = $if_context;

if ($codebase->alter_code) {
$if_context->branch_point = $branch_point;
Expand Down Expand Up @@ -236,7 +237,7 @@ public static function analyze(
* Returns statements that are definitely evaluated before any statements after the end of the
* if/elseif/else blocks
*/
private static function getDefinitelyEvaluatedExpressionAfterIf(PhpParser\Node\Expr $stmt): ?PhpParser\Node\Expr
private static function getDefinitelyEvaluatedExpressionAfterIf(PhpParser\Node\Expr $stmt): PhpParser\Node\Expr
{
if ($stmt instanceof PhpParser\Node\Expr\BinaryOp\Equal
|| $stmt instanceof PhpParser\Node\Expr\BinaryOp\Identical
Expand Down Expand Up @@ -280,7 +281,7 @@ private static function getDefinitelyEvaluatedExpressionAfterIf(PhpParser\Node\E
* Returns statements that are definitely evaluated before any statements inside
* the if block
*/
private static function getDefinitelyEvaluatedExpressionInsideIf(PhpParser\Node\Expr $stmt): ?PhpParser\Node\Expr
private static function getDefinitelyEvaluatedExpressionInsideIf(PhpParser\Node\Expr $stmt): PhpParser\Node\Expr
{
if ($stmt instanceof PhpParser\Node\Expr\BinaryOp\Equal
|| $stmt instanceof PhpParser\Node\Expr\BinaryOp\Identical
Expand Down
49 changes: 18 additions & 31 deletions src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseAnalyzer.php
View file
Open in desktop
Expand Up @@ -59,16 +59,6 @@ public static function analyze(

$else_types = Algebra::getTruthsFromFormula($else_context->clauses);

if (!$else && !$else_types) {
$if_scope->final_actions = array_merge([ScopeAnalyzer::ACTION_NONE], $if_scope->final_actions);
$if_scope->assigned_var_ids = [];
$if_scope->new_vars = [];
$if_scope->redefined_vars = [];
$if_scope->reasonable_clauses = [];

return null;
}

$original_context = clone $else_context;

if ($else_types) {
Expand Down Expand Up @@ -120,19 +110,19 @@ public static function analyze(
) {
return false;
}
}

foreach ($else_context->parent_remove_vars as $var_id => $_) {
$outer_context->removeVarFromConflictingClauses($var_id);
}
foreach ($else_context->parent_remove_vars as $var_id => $_) {
$outer_context->removeVarFromConflictingClauses($var_id);
}

/** @var array<string, int> */
$new_assigned_var_ids = $else_context->assigned_var_ids;
$else_context->assigned_var_ids = $pre_stmts_assigned_var_ids;
$else_context->assigned_var_ids += $pre_stmts_assigned_var_ids;

/** @var array<string, bool> */
$new_possibly_assigned_var_ids = $else_context->possibly_assigned_var_ids;
$else_context->possibly_assigned_var_ids = $pre_possibly_assigned_var_ids + $new_possibly_assigned_var_ids;
$else_context->possibly_assigned_var_ids += $pre_possibly_assigned_var_ids;

if ($else) {
foreach ($else_context->byref_constraints as $var_id => $byref_constraint) {
Expand Down Expand Up @@ -185,7 +175,7 @@ public static function analyze(
$new_assigned_var_ids,
$new_possibly_assigned_var_ids,
[],
(bool) $else
true
);

$if_scope->reasonable_clauses = [];
Expand All @@ -210,24 +200,21 @@ public static function analyze(

$possibly_assigned_var_ids = $new_possibly_assigned_var_ids;

if ($has_leaving_statements && $else_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 ($has_leaving_statements) {
if ($else_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
$else_context->loop_scope->vars_possibly_in_scope = array_merge(
$vars_possibly_in_scope,
$else_context->loop_scope->vars_possibly_in_scope
);
}

$else_context->loop_scope->vars_possibly_in_scope = array_merge(
$vars_possibly_in_scope,
$else_context->loop_scope->vars_possibly_in_scope
);
} elseif (!$has_leaving_statements) {
} else {
$if_scope->new_vars_possibly_in_scope = array_merge(
$vars_possibly_in_scope,
$if_scope->new_vars_possibly_in_scope
Expand Down