Skip to content

Commit

Permalink
Remove mic-drop hack from if analysis (#7484)
Browse files Browse the repository at this point in the history 
* Remove mic-drop hack from if analysis

* Remove more special handling

* Remove some unnecessary ElseAnalyzer code

* Add back necessary code

* Fix return type of method never returning null

* Add a comment

* Simplify && handling

* Add comments to make stuff clearer

* Move if-specfic logic to more appropriate setting
  • Loading branch information
@muglug
muglug committed Jan 28, 2022
1 parent 048025b commit faaf769
Show file tree
Hide file tree
Showing 17 changed files with 297 additions and 331 deletions.
2 changes: 1 addition & 1 deletion src/Psalm/Context.php
View file
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
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
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

0 comments on commit faaf769

Please sign in to comment.