mirror of
https://github.com/danog/psalm.git
synced 2025-01-22 05:41:20 +01:00
Improve handling of do blocks
This commit is contained in:
parent
461a9667b5
commit
03ea94e087
@ -3,12 +3,14 @@ namespace Psalm\Checker\Statements\Block;
|
||||
|
||||
use PhpParser;
|
||||
use Psalm\Checker\AlgebraChecker;
|
||||
use Psalm\Checker\ScopeChecker;
|
||||
use Psalm\Checker\Statements\ExpressionChecker;
|
||||
use Psalm\Checker\StatementsChecker;
|
||||
use Psalm\Clause;
|
||||
use Psalm\Context;
|
||||
use Psalm\Scope\LoopScope;
|
||||
use Psalm\Type;
|
||||
use Psalm\Type\Algebra;
|
||||
|
||||
class DoChecker
|
||||
{
|
||||
@ -57,28 +59,20 @@ class DoChecker
|
||||
$statements_checker->removeSuppressedIssues(['TypeDoesNotContainType']);
|
||||
}
|
||||
|
||||
$loop_scope->iteration_count++;
|
||||
|
||||
foreach ($context->vars_in_scope as $var => $type) {
|
||||
if ($type->isMixed()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if ($do_context->hasVariable($var)) {
|
||||
if ($context->vars_in_scope[$var]->isMixed()) {
|
||||
$do_context->vars_in_scope[$var] = $do_context->vars_in_scope[$var];
|
||||
}
|
||||
|
||||
if ($do_context->vars_in_scope[$var]->getId() !== $type->getId()) {
|
||||
$do_context->vars_in_scope[$var] = Type::combineUnionTypes($do_context->vars_in_scope[$var], $type);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
foreach ($do_context->vars_in_scope as $var_id => $type) {
|
||||
if (!isset($context->vars_in_scope[$var_id])) {
|
||||
$context->vars_in_scope[$var_id] = clone $type;
|
||||
}
|
||||
}
|
||||
|
||||
$mixed_var_ids = [];
|
||||
|
||||
foreach ($do_context->vars_in_scope as $var_id => $type) {
|
||||
@ -87,7 +81,7 @@ class DoChecker
|
||||
}
|
||||
}
|
||||
|
||||
$while_clauses = \Psalm\Type\Algebra::getFormula(
|
||||
$while_clauses = Algebra::getFormula(
|
||||
$stmt->cond,
|
||||
$context->self,
|
||||
$statements_checker
|
||||
@ -173,19 +167,48 @@ class DoChecker
|
||||
true
|
||||
);
|
||||
|
||||
foreach ($do_context->vars_in_scope as $var_id => $type) {
|
||||
if (!isset($context->vars_in_scope[$var_id])) {
|
||||
$context->vars_in_scope[$var_id] = $type;
|
||||
}
|
||||
}
|
||||
|
||||
// because it's a do {} while, inner loop vars belong to the main context
|
||||
if (!$inner_loop_context) {
|
||||
throw new \UnexpectedValueException('Should never be null');
|
||||
}
|
||||
|
||||
$negated_while_clauses = Algebra::negateFormula($while_clauses);
|
||||
|
||||
$negated_while_types = Algebra::getTruthsFromFormula(
|
||||
Algebra::simplifyCNF(
|
||||
array_merge($context->clauses, $negated_while_clauses)
|
||||
)
|
||||
);
|
||||
|
||||
ExpressionChecker::analyze($statements_checker, $stmt->cond, $inner_loop_context);
|
||||
|
||||
if ($negated_while_types) {
|
||||
$changed_var_ids = [];
|
||||
|
||||
$inner_loop_context->vars_in_scope =
|
||||
Type\Reconciler::reconcileKeyedTypes(
|
||||
$negated_while_types,
|
||||
$inner_loop_context->vars_in_scope,
|
||||
$changed_var_ids,
|
||||
[],
|
||||
$statements_checker,
|
||||
new \Psalm\CodeLocation($statements_checker->getSource(), $stmt->cond),
|
||||
$statements_checker->getSuppressedIssues()
|
||||
);
|
||||
}
|
||||
|
||||
foreach ($inner_loop_context->vars_in_scope as $var_id => $type) {
|
||||
if (!isset($context->vars_in_scope[$var_id])) {
|
||||
// if there are break statements in the loop it's not certain
|
||||
// that the loop has finished executing, so the assertions at the end
|
||||
// the loop in the while conditional may not hold
|
||||
if (in_array(ScopeChecker::ACTION_BREAK, $loop_scope->final_actions, true)) {
|
||||
if (isset($loop_scope->possibly_defined_loop_parent_vars[$var_id])) {
|
||||
$context->vars_in_scope[$var_id] = Type::combineUnionTypes(
|
||||
$type,
|
||||
$loop_scope->possibly_defined_loop_parent_vars[$var_id]
|
||||
);
|
||||
}
|
||||
} else {
|
||||
$context->vars_in_scope[$var_id] = $type;
|
||||
}
|
||||
}
|
||||
@ -200,8 +223,6 @@ class DoChecker
|
||||
$do_context->referenced_var_ids
|
||||
);
|
||||
|
||||
ExpressionChecker::analyze($statements_checker, $stmt->cond, $inner_loop_context);
|
||||
|
||||
if ($context->collect_references) {
|
||||
$context->unreferenced_vars = $do_context->unreferenced_vars;
|
||||
}
|
||||
|
@ -189,6 +189,8 @@ class LoopChecker
|
||||
for ($i = 0; $i < $assignment_depth; ++$i) {
|
||||
$vars_to_remove = [];
|
||||
|
||||
$loop_scope->iteration_count++;
|
||||
|
||||
$has_changes = false;
|
||||
|
||||
// reset the $inner_context to what it was before we started the analysis,
|
||||
@ -235,9 +237,14 @@ class LoopChecker
|
||||
if ($recorded_issues) {
|
||||
$has_changes = true;
|
||||
}
|
||||
|
||||
// if we're in a do block we don't want to remove vars before evaluating
|
||||
// the where conditional
|
||||
if (!$is_do) {
|
||||
$vars_to_remove[] = $var_id;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
$loop_scope->loop_parent_context->vars_possibly_in_scope = array_merge(
|
||||
$inner_context->vars_possibly_in_scope,
|
||||
|
@ -321,6 +321,21 @@ class StatementsChecker extends SourceChecker implements StatementsSource
|
||||
}
|
||||
}
|
||||
|
||||
if ($loop_scope->iteration_count === 0) {
|
||||
foreach ($context->vars_in_scope as $var_id => $type) {
|
||||
if (!isset($loop_scope->loop_parent_context->vars_in_scope[$var_id])) {
|
||||
if (isset($loop_scope->possibly_defined_loop_parent_vars[$var_id])) {
|
||||
$loop_scope->possibly_defined_loop_parent_vars[$var_id] = Type::combineUnionTypes(
|
||||
$type,
|
||||
$loop_scope->possibly_defined_loop_parent_vars[$var_id]
|
||||
);
|
||||
} else {
|
||||
$loop_scope->possibly_defined_loop_parent_vars[$var_id] = $type;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if ($context->collect_references && (!$context->switch_scope || $stmt->num)) {
|
||||
foreach ($context->unreferenced_vars as $var_id => $locations) {
|
||||
if (isset($loop_scope->unreferenced_vars[$var_id])) {
|
||||
|
@ -7,6 +7,11 @@ use Psalm\Type;
|
||||
|
||||
class LoopScope
|
||||
{
|
||||
/**
|
||||
* @var int
|
||||
*/
|
||||
public $iteration_count = 0;
|
||||
|
||||
/**
|
||||
* @var Context
|
||||
*/
|
||||
@ -32,6 +37,11 @@ class LoopScope
|
||||
*/
|
||||
public $possibly_redefined_loop_parent_vars = null;
|
||||
|
||||
/**
|
||||
* @var array<string, Type\Union>
|
||||
*/
|
||||
public $possibly_defined_loop_parent_vars = [];
|
||||
|
||||
/**
|
||||
* @var array<string, bool>
|
||||
*/
|
||||
|
@ -5,6 +5,7 @@ use Psalm\Tests\Traits;
|
||||
|
||||
class DoTest extends \Psalm\Tests\TestCase
|
||||
{
|
||||
use Traits\FileCheckerInvalidCodeParseTestTrait;
|
||||
use Traits\FileCheckerValidCodeParseTestTrait;
|
||||
|
||||
/**
|
||||
@ -22,14 +23,88 @@ class DoTest extends \Psalm\Tests\TestCase
|
||||
}
|
||||
while (rand(0,100) === 10);',
|
||||
'assertions' => [
|
||||
'$worked' => 'bool',
|
||||
'$worked' => 'true',
|
||||
],
|
||||
],
|
||||
'doWhileVarWithPossibleBreak' => [
|
||||
'<?php
|
||||
$a = false;
|
||||
|
||||
do {
|
||||
if (rand(0, 1)) {
|
||||
break;
|
||||
}
|
||||
if (rand(0, 1)) {
|
||||
$a = true;
|
||||
break;
|
||||
}
|
||||
$a = true;
|
||||
}
|
||||
while (rand(0,100) === 10);',
|
||||
'assertions' => [
|
||||
'$a' => 'bool',
|
||||
],
|
||||
],
|
||||
'SKIPPED-doWhileVarWithPossibleBreakThatSetsToTrue' => [
|
||||
'<?php
|
||||
$a = false;
|
||||
$b = false;
|
||||
|
||||
do {
|
||||
$b = true;
|
||||
if (rand(0, 1)) {
|
||||
$a = true;
|
||||
break;
|
||||
}
|
||||
$a = true;
|
||||
}
|
||||
while (rand(0,1));',
|
||||
'assertions' => [
|
||||
'$a' => 'true',
|
||||
'$b' => 'true',
|
||||
],
|
||||
],
|
||||
'doWhileVarWithPossibleBreakThatMaybeSetsToTrue' => [
|
||||
'<?php
|
||||
$a = false;
|
||||
|
||||
do {
|
||||
if (rand(0, 1)) {
|
||||
if (rand(0, 1)) {
|
||||
$a = true;
|
||||
}
|
||||
|
||||
break;
|
||||
}
|
||||
$a = true;
|
||||
}
|
||||
while (rand(0,1));',
|
||||
'assertions' => [
|
||||
'$a' => 'bool',
|
||||
],
|
||||
],
|
||||
'doWhileVarWithPossibleInitialisingBreakNoInitialDefinition' => [
|
||||
'<?php
|
||||
do {
|
||||
if (rand(0, 1)) {
|
||||
$worked = true;
|
||||
break;
|
||||
}
|
||||
$worked = true;
|
||||
}
|
||||
while (rand(0,100) === 10);',
|
||||
'assertions' => [
|
||||
'$worked' => 'true',
|
||||
],
|
||||
],
|
||||
'doWhileUndefinedVar' => [
|
||||
'<?php
|
||||
do {
|
||||
$result = rand(0,1);
|
||||
$result = (bool) rand(0,1);
|
||||
} while (!$result);',
|
||||
'assertions' => [
|
||||
'$result' => 'true',
|
||||
],
|
||||
],
|
||||
'doWhileVarAndBreak' => [
|
||||
'<?php
|
||||
@ -140,6 +215,17 @@ class DoTest extends \Psalm\Tests\TestCase
|
||||
$value = 6;
|
||||
} while ($count);',
|
||||
],
|
||||
'doWhileDefinedVarWithPossibleBreak' => [
|
||||
'<?php
|
||||
$value = null;
|
||||
do {
|
||||
if (rand(0, 1)) {
|
||||
break;
|
||||
}
|
||||
$count = rand(0, 1);
|
||||
$value = 6;
|
||||
} while ($count);',
|
||||
],
|
||||
'invalidateBothByRefAssignmentsInDo' => [
|
||||
'<?php
|
||||
function foo(?string &$i) : void {}
|
||||
@ -186,6 +272,74 @@ class DoTest extends \Psalm\Tests\TestCase
|
||||
$foo["bar"] = "bat";
|
||||
} while (rand(0, 1));',
|
||||
],
|
||||
'noRedundantConditionAfterDoWhile' => [
|
||||
'<?php
|
||||
$i = 5;
|
||||
do {} while (--$i > 0);
|
||||
echo $i === 0;',
|
||||
],
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* @return array
|
||||
*/
|
||||
public function providerFileCheckerInvalidCodeParse()
|
||||
{
|
||||
return [
|
||||
'doWhileVarWithPossibleBreakWithoutDefining' => [
|
||||
'<?php
|
||||
do {
|
||||
if (rand(0, 1)) {
|
||||
break;
|
||||
}
|
||||
$worked = true;
|
||||
}
|
||||
while (rand(0,1));
|
||||
|
||||
echo $worked;',
|
||||
'error_message' => 'PossiblyUndefinedGlobalVariable',
|
||||
],
|
||||
'doWhileVarWithPossibleBreakThatMaybeSetsToTrueWithoutDefining' => [
|
||||
'<?php
|
||||
do {
|
||||
if (rand(0, 1)) {
|
||||
if (rand(0, 1)) {
|
||||
$a = true;
|
||||
}
|
||||
|
||||
break;
|
||||
}
|
||||
$a = true;
|
||||
}
|
||||
while (rand(0,1));
|
||||
|
||||
echo $a;',
|
||||
'error_message' => 'PossiblyUndefinedGlobalVariable',
|
||||
],
|
||||
'SKIPPED-doWhileVarWithPossibleContinueWithoutDefining' => [
|
||||
'<?php
|
||||
do {
|
||||
if (rand(0, 1)) {
|
||||
continue;
|
||||
}
|
||||
$worked = true;
|
||||
}
|
||||
while (rand(0,1));
|
||||
|
||||
echo $worked;',
|
||||
'error_message' => 'PossiblyUndefinedGlobalVariable',
|
||||
],
|
||||
'possiblyUndefinedArrayInDo' => [
|
||||
'<?php
|
||||
do {
|
||||
$array[] = "hello";
|
||||
} while (rand(0, 1));
|
||||
|
||||
echo $array;',
|
||||
'error_message' => 'PossiblyUndefinedGlobalVariable - src' . DIRECTORY_SEPARATOR . 'somefile.php:3 - Possibly undefined ' .
|
||||
'global variable $array, first seen on line 3',
|
||||
],
|
||||
];
|
||||
}
|
||||
}
|
||||
|
@ -327,6 +327,12 @@ class WhileTest extends \Psalm\Tests\TestCase
|
||||
if ($a === $foo) {}
|
||||
}',
|
||||
],
|
||||
'noRedundantConditionAfterWhile' => [
|
||||
'<?php
|
||||
$i = 5;
|
||||
while (--$i > 0) {}
|
||||
echo $i === 0;',
|
||||
],
|
||||
];
|
||||
}
|
||||
|
||||
|
@ -407,18 +407,6 @@ class ValueTest extends TestCase
|
||||
takesString($f[$i]);
|
||||
}'
|
||||
],
|
||||
'noRedundantConditionAfterWhile' => [
|
||||
'<?php
|
||||
$i = 5;
|
||||
while (--$i > 0) {}
|
||||
echo $i === 0;',
|
||||
],
|
||||
'noRedundantConditionAfterDoWhile' => [
|
||||
'<?php
|
||||
$i = 5;
|
||||
do {} while (--$i > 0);
|
||||
echo $i === 0;',
|
||||
],
|
||||
'coerceFromMixed' => [
|
||||
'<?php
|
||||
function type(int $b): void {}
|
||||
|
Loading…
x
Reference in New Issue
Block a user