Skip to content
Permalink
Browse files

Fix #2073 - better understand assignments inside elseif

  • Loading branch information...
muglug committed Aug 27, 2019
1 parent 25487a5 commit 853e92e7fcefb300c8153a401da1188f696d427a
@@ -78,9 +78,11 @@ public static function analyze(
try {
$if_conditional_scope = self::analyzeIfConditional(
$statements_analyzer,
$stmt,
$stmt->cond,
$context,
$codebase
$codebase,
$if_scope,
$context->branch_point ?: (int) $stmt->getAttribute('startFilePos')
);
$if_context = $if_conditional_scope->if_context;
@@ -432,17 +434,62 @@ function (Clause $c) use ($mixed_var_ids) {
*/
private static function analyzeIfConditional(
StatementsAnalyzer $statements_analyzer,
PhpParser\Node\Stmt\If_ $stmt,
PhpParser\Node\Expr $cond,
Context $context,
Codebase $codebase
Codebase $codebase,
IfScope $if_scope,
?int $branch_point
) {
// get the first expression in the if, which should be evaluated on its own
// this allows us to update the context of $matches in
// if (!preg_match('/a/', 'aa', $matches)) {
// exit
// }
// echo $matches[0];
$first_if_cond_expr = self::getDefinitelyEvaluatedExpression($stmt->cond);
$first_if_cond_expr = self::getDefinitelyEvaluatedExpression($cond);
$entry_clauses = [];
if ($if_scope->negated_clauses) {
$entry_clauses = array_merge($context->clauses, $if_scope->negated_clauses);
$changed_var_ids = [];
if ($if_scope->negated_types) {
$vars_reconciled = Reconciler::reconcileKeyedTypes(
$if_scope->negated_types,
$context->vars_in_scope,
$changed_var_ids,
[],
$statements_analyzer,
[],
$context->inside_loop,
new CodeLocation(
$statements_analyzer->getSource(),
$cond instanceof PhpParser\Node\Expr\BooleanNot
? $cond->expr
: $cond,
$context->include_location,
false
)
);
if ($changed_var_ids) {
$context = clone $context;
$context->vars_in_scope = $vars_reconciled;
$entry_clauses = array_filter(
$entry_clauses,
/** @return bool */
function (Clause $c) use ($changed_var_ids) {
return count($c->possibilities) > 1
|| $c->wedge
|| !in_array(array_keys($c->possibilities)[0], $changed_var_ids, true);
}
);
}
}
}
$context->inside_conditional = true;
@@ -477,22 +524,22 @@ private static function analyzeIfConditional(
$if_context = clone $context;
if ($codebase->alter_code) {
$if_context->branch_point = $if_context->branch_point ?: (int) $stmt->getAttribute('startFilePos');
$if_context->branch_point = $branch_point;
}
// we need to clone the current context so our ongoing updates to $context don't mess with elseif/else blocks
$original_context = clone $context;
$if_context->inside_conditional = true;
if ($first_if_cond_expr !== $stmt->cond) {
if ($first_if_cond_expr !== $cond) {
$assigned_var_ids = $context->assigned_var_ids;
$if_context->assigned_var_ids = [];
$referenced_var_ids = $context->referenced_var_ids;
$if_context->referenced_var_ids = [];
if (ExpressionAnalyzer::analyze($statements_analyzer, $stmt->cond, $if_context) === false) {
if (ExpressionAnalyzer::analyze($statements_analyzer, $cond, $if_context) === false) {
throw new \Psalm\Exception\ScopeAnalysisException();
}
@@ -567,7 +614,8 @@ function ($var_id) use ($pre_condition_vars_in_scope) {
$if_context,
$original_context,
$cond_referenced_var_ids,
$cond_assigned_var_ids
$cond_assigned_var_ids,
$entry_clauses
);
}
@@ -869,89 +917,27 @@ protected static function analyzeElseIfBlock(
Codebase $codebase,
?int $branch_point
) {
$elseif_context = clone $else_context;
if ($codebase->alter_code) {
$elseif_context->branch_point = $branch_point;
}
$pre_conditional_context = clone $else_context;
$original_context = clone $elseif_context;
$entry_clauses = array_merge($original_context->clauses, $if_scope->negated_clauses);
$changed_var_ids = [];
if ($if_scope->negated_types) {
$elseif_vars_reconciled = Reconciler::reconcileKeyedTypes(
$if_scope->negated_types,
$elseif_context->vars_in_scope,
$changed_var_ids,
[],
try {
$if_conditional_scope = self::analyzeIfConditional(
$statements_analyzer,
[],
$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,
false
)
$elseif->cond,
$else_context,
$codebase,
$if_scope,
$branch_point
);
$elseif_context->vars_in_scope = $elseif_vars_reconciled;
if ($changed_var_ids) {
$entry_clauses = array_filter(
$entry_clauses,
/** @return bool */
function (Clause $c) use ($changed_var_ids) {
return count($c->possibilities) > 1
|| $c->wedge
|| !in_array(array_keys($c->possibilities)[0], $changed_var_ids, true);
}
);
}
}
$pre_conditional_context = clone $elseif_context;
$elseif_context->inside_conditional = true;
$pre_assigned_var_ids = $elseif_context->assigned_var_ids;
$referenced_var_ids = $elseif_context->referenced_var_ids;
$elseif_context->referenced_var_ids = [];
// check the elseif
if (ExpressionAnalyzer::analyze($statements_analyzer, $elseif->cond, $elseif_context) === false) {
$elseif_context = $if_conditional_scope->if_context;
$original_context = $if_conditional_scope->original_context;
$cond_referenced_var_ids = $if_conditional_scope->cond_referenced_var_ids;
$cond_assigned_var_ids = $if_conditional_scope->cond_assigned_var_ids;
$entry_clauses = $if_conditional_scope->entry_clauses;
} catch (\Psalm\Exception\ScopeAnalysisException $e) {
return false;
}
/** @var array<string, bool> */
$new_referenced_var_ids = $elseif_context->referenced_var_ids;
$elseif_context->referenced_var_ids = array_merge(
$referenced_var_ids,
$elseif_context->referenced_var_ids
);
$conditional_assigned_var_ids = $elseif_context->assigned_var_ids;
$elseif_context->assigned_var_ids = array_merge(
$pre_assigned_var_ids,
$conditional_assigned_var_ids
);
$new_assigned_var_ids = array_diff_key(
$conditional_assigned_var_ids,
$pre_assigned_var_ids
);
$new_referenced_var_ids = array_diff_key($new_referenced_var_ids, $new_assigned_var_ids);
$elseif_context->inside_conditional = false;
$mixed_var_ids = [];
foreach ($elseif_context->vars_in_scope as $var_id => $type) {
@@ -991,11 +977,11 @@ function (Clause $c) use ($mixed_var_ids) {
/**
* @return Clause
*/
function (Clause $c) use ($conditional_assigned_var_ids) {
function (Clause $c) use ($cond_assigned_var_ids) {
$keys = array_keys($c->possibilities);
foreach ($keys as $key) {
foreach ($conditional_assigned_var_ids as $conditional_assigned_var_id => $_) {
foreach ($cond_assigned_var_ids as $conditional_assigned_var_id => $_) {
if (preg_match('/^' . preg_quote($conditional_assigned_var_id, '/') . '(\[|-|$)/', $key)) {
return new Clause([], true);
}
@@ -1013,7 +999,7 @@ function (Clause $c) use ($conditional_assigned_var_ids) {
$elseif_clauses,
$statements_analyzer,
$elseif->cond,
$new_assigned_var_ids
$cond_assigned_var_ids
);
$elseif_context->clauses = Algebra::simplifyCNF(
@@ -1051,15 +1037,15 @@ function (Clause $c) use ($conditional_assigned_var_ids) {
}
}
$changed_var_ids = $changed_var_ids ?: [];
$changed_var_ids = [];
// if the elseif has an || in the conditional, we cannot easily reason about it
if ($reconcilable_elseif_types) {
$elseif_vars_reconciled = Reconciler::reconcileKeyedTypes(
$reconcilable_elseif_types,
$elseif_context->vars_in_scope,
$changed_var_ids,
$new_referenced_var_ids,
$cond_referenced_var_ids,
$statements_analyzer,
$statements_analyzer->getTemplateTypeMap() ?: [],
$elseif_context->inside_loop,
@@ -1175,7 +1161,7 @@ function (Clause $c) use ($conditional_assigned_var_ids) {
}
}
$assigned_var_ids = array_merge($new_stmts_assigned_var_ids, $new_assigned_var_ids);
$assigned_var_ids = array_merge($new_stmts_assigned_var_ids, $cond_assigned_var_ids);
if ($if_scope->assigned_var_ids === null) {
$if_scope->assigned_var_ids = $assigned_var_ids;
@@ -22,19 +22,25 @@ class IfConditionalScope
*/
public $cond_assigned_var_ids;
/** @var array<int, \Psalm\Internal\Clause> */
public $entry_clauses;
/**
* @param array<string, bool> $cond_referenced_var_ids
* @param array<string, bool> $cond_assigned_var_ids
* @param array<int, \Psalm\Internal\Clause> $entry_clauses
*/
public function __construct(
Context $if_context,
Context $original_context,
array $cond_referenced_var_ids,
array $cond_assigned_var_ids
array $cond_assigned_var_ids,
array $entry_clauses
) {
$this->if_context = $if_context;
$this->original_context = $original_context;
$this->cond_referenced_var_ids = $cond_referenced_var_ids;
$this->cond_assigned_var_ids = $cond_assigned_var_ids;
$this->entry_clauses = $entry_clauses;
}
}
@@ -259,6 +259,14 @@ function a(): ?int {
?>
<h1><?= $this->getMessage() ?></h1>',
],
'bleedElseifAssignedVarsIntoElseScope' => [
'<?php
if (rand(0, 1) === 0) {
$foo = 0;
} elseif ($foo = rand(0, 10)) {}
echo substr("banana", $foo);',
],
];
}
@@ -1102,6 +1102,18 @@ function foo(A $a) : void {
echo $path;'
],
'assignedInElseif' => [
'<?php
function bar(): int {
if (rand(0, 1) === 0) {
$foo = 0;
} elseif ($foo = rand(0, 10)) {
return 5;
}
return $foo;
}',
],
];
}

0 comments on commit 853e92e

Please sign in to comment.
You can’t perform that action at this time.