danog/psalm
1
0
Fork 0
mirror of https://github.com/danog/psalm.git synced 2025-01-21 21:31:13 +01:00
Code Issues Projects Releases Wiki Activity

Fix analysis of switch statements

Browse Source
This commit is contained in:
Matthew Brown 2016-08-24 17:06:20 -04:00
parent 7e6d3b90ce
commit 4d388d197c
3 changed files with 78 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

68
src/Psalm/Checker/ScopeChecker.php
View File

@ -136,7 +136,61 @@ class ScopeChecker
return false;
}
public static function doesReturnOrThrow(array $stmts)
public static function doesAlwaysBreakOrContinue(array $stmts, $ignore_break = false)
{
if (empty($stmts)) {
return false;
}
for ($i = count($stmts) - 1; $i >= 0; $i--) {
$stmt = $stmts[$i];
if ($stmt instanceof PhpParser\Node\Stmt\Continue_ || (!$ignore_break && $stmt instanceof PhpParser\Node\Stmt\Break_)) {
return true;
}
if ($stmt instanceof PhpParser\Node\Stmt\If_) {
$all_branches_break_continue;
if (!self::doesAlwaysBreakOrContinue($stmt->stmts, $ignore_break)) {
return false;
}
if (!$stmt->else || !self::doesAlwaysBreakOrContinue($stmt->else->stmts, $ignore_break)) {
return false;
}
foreach ($stmt->elseifs as $elseif) {
if (!self::doesAlwaysBreakOrContinue($elseif->stmts, $ignore_break)) {
return false;
}
}
return true;
}
if ($stmt instanceof PhpParser\Node\Stmt\Switch_) {
// iterate backwards
// in switch statements we only care here about continue
for ($i = count($stmt->cases) - 1; $i >= 0; $i--) {
$case = $stmt->cases[$i];
if (!self::doesAlwaysBreakOrContinue($case->stmts, true)) {
return false;
}
}
return true;
}
if ($stmt instanceof PhpParser\Node\Stmt\Nop) {
continue;
}
}
return false;
}
public static function doesAlwaysReturnOrThrow(array $stmts)
{
if (empty($stmts)) {
return false;
@ -153,13 +207,13 @@ class ScopeChecker
}
if ($stmt instanceof PhpParser\Node\Stmt\If_) {
if ($stmt->else && self::doesReturnOrThrow($stmt->stmts) && self::doesReturnOrThrow($stmt->else->stmts)) {
if ($stmt->else && self::doesAlwaysReturnOrThrow($stmt->stmts) && self::doesAlwaysReturnOrThrow($stmt->else->stmts)) {
if (empty($stmt->elseifs)) {
return true;
}
foreach ($stmt->elseifs as $elseif) {
if (!self::doesReturnOrThrow($elseif->stmts)) {
if (!self::doesAlwaysReturnOrThrow($elseif->stmts)) {
return false;
}
}
@ -180,7 +234,7 @@ class ScopeChecker
return false;
}
$case_does_return = self::doesReturnOrThrow($case->stmts);
$case_does_return = self::doesAlwaysReturnOrThrow($case->stmts);
if ($case_does_return) {
$has_returned = true;
@ -199,15 +253,15 @@ class ScopeChecker
}
if ($stmt instanceof PhpParser\Node\Stmt\While_) {
if (self::doesReturnOrThrow($stmt->stmts)) {
if (self::doesAlwaysReturnOrThrow($stmt->stmts)) {
return true;
}
}
if ($stmt instanceof PhpParser\Node\Stmt\TryCatch) {
if (self::doesReturnOrThrow($stmt->stmts)) {
if (self::doesAlwaysReturnOrThrow($stmt->stmts)) {
foreach ($stmt->catches as $catch) {
if (!self::doesReturnOrThrow($catch->stmts)) {
if (!self::doesAlwaysReturnOrThrow($catch->stmts)) {
return false;
}
}

30
src/Psalm/Checker/StatementsChecker.php
View File

@ -314,9 +314,9 @@ class StatementsChecker
$reconcilable_if_types = $negatable_if_types = $this->type_checker->getTypeAssertions($stmt->cond, true);
}
$has_ending_statements = ScopeChecker::doesReturnOrThrow($stmt->stmts);
$has_ending_statements = ScopeChecker::doesAlwaysReturnOrThrow($stmt->stmts);
$has_leaving_statements = $has_ending_statements || ScopeChecker::doesLeaveBlock($stmt->stmts, true, true);
$has_leaving_statements = $has_ending_statements || ScopeChecker::doesAlwaysBreakOrContinue($stmt->stmts);
$negated_types = $negatable_if_types ? TypeChecker::negateTypes($negatable_if_types) : [];
@ -502,7 +502,10 @@ class StatementsChecker
}
if (count($elseif->stmts)) {
$has_leaving_statements = ScopeChecker::doesLeaveBlock($elseif->stmts, true, true);
// has a return/throw at end
$has_ending_statements = ScopeChecker::doesAlwaysReturnOrThrow($elseif->stmts);
$has_leaving_statements = $has_ending_statements || ScopeChecker::doesAlwaysBreakOrContinue($elseif->stmts);
// update the parent context as necessary
$elseif_redefined_vars = Context::getRedefinedVars($original_context, $elseif_context);
@ -554,9 +557,6 @@ class StatementsChecker
$context->update($old_elseif_context, $elseif_context, $has_leaving_statements, array_keys($negated_elseif_types), $updated_vars);
}
// has a return/throw at end
$has_ending_statements = ScopeChecker::doesReturnOrThrow($elseif->stmts);
if (!$has_ending_statements) {
$vars = array_diff_key($elseif_context->vars_possibly_in_scope, $context->vars_possibly_in_scope);
@ -623,7 +623,10 @@ class StatementsChecker
}
if (count($stmt->else->stmts)) {
$has_leaving_statements = ScopeChecker::doesLeaveBlock($stmt->else->stmts, true, true);
// has a return/throw at end
$has_ending_statements = ScopeChecker::doesAlwaysReturnOrThrow($stmt->else->stmts);
$has_leaving_statements = $has_ending_statements || ScopeChecker::doesAlwaysBreakOrContinue($stmt->else->stmts);
/** @var Context $original_context */
$else_redefined_vars = Context::getRedefinedVars($original_context, $else_context);
@ -677,8 +680,7 @@ class StatementsChecker
$context->update($old_else_context, $else_context, $has_leaving_statements, array_keys($negatable_if_types), $updated_vars);
}
// has a return/throw at end
$has_ending_statements = ScopeChecker::doesReturnOrThrow($stmt->else->stmts);
if (!$has_ending_statements) {
$vars = array_diff_key($else_context->vars_possibly_in_scope, $context->vars_possibly_in_scope);
@ -1647,7 +1649,7 @@ class StatementsChecker
$this->check($catch->stmts, $catch_context);
if (!ScopeChecker::doesReturnOrThrow($catch->stmts)) {
if (!ScopeChecker::doesAlwaysReturnOrThrow($catch->stmts)) {
foreach ($catch_context->vars_in_scope as $catch_var => $type) {
if ($catch->var !== $catch_var && isset($context->vars_in_scope[$catch_var]) && (string) $context->vars_in_scope[$catch_var] !== (string) $type) {
$context->vars_in_scope[$catch_var] = Type::combineUnionTypes($context->vars_in_scope[$catch_var], $type);
@ -3086,13 +3088,13 @@ class StatementsChecker
for ($i = count($stmt->cases) - 1; $i >= 0; $i--) {
$case = $stmt->cases[$i];
if (ScopeChecker::doesReturnOrThrow($case->stmts)) {
if (ScopeChecker::doesAlwaysReturnOrThrow($case->stmts)) {
$last_case_exit_type = 'return_throw';
}
elseif (ScopeChecker::doesEverBreakOrContinue($case->stmts, true)) {
elseif (ScopeChecker::doesAlwaysBreakOrContinue($case->stmts, true)) {
$last_case_exit_type = 'continue';
}
elseif (ScopeChecker::doesEverBreakOrContinue($case->stmts)) {
elseif (ScopeChecker::doesAlwaysBreakOrContinue($case->stmts)) {
$last_case_exit_type = 'break';
}
@ -3131,7 +3133,7 @@ class StatementsChecker
$last_stmt = $case->stmts[count($case->stmts) - 1];
// has a return/throw at end
$has_ending_statements = ScopeChecker::doesReturnOrThrow($case->stmts);
$has_ending_statements = ScopeChecker::doesAlwaysReturnOrThrow($case->stmts);
if ($case_exit_type !== 'return_throw') {
$vars = array_diff_key($case_context->vars_possibly_in_scope, $original_context->vars_possibly_in_scope);

2
src/Psalm/EffectsAnalyser.php
View File

@ -77,7 +77,7 @@ class EffectsAnalyser
}
// if we're at the top level and we're not ending in a return, make sure to add possible null
if ($collapse_types && !$last_stmt instanceof PhpParser\Node\Stmt\Return_ && !Checker\ScopeChecker::doesReturnOrThrow($stmts)) {
if ($collapse_types && !$last_stmt instanceof PhpParser\Node\Stmt\Return_ && !Checker\ScopeChecker::doesAlwaysReturnOrThrow($stmts)) {
$return_types[] = Type::getNull(false);
}