danog/psalm
1
0
Fork 0
mirror of https://github.com/danog/psalm.git synced 2024-11-30 04:39:00 +01:00
Code Issues Projects Releases Wiki Activity

Improve handling of vars set in always-entered for/foreach/while loops

Browse Source
This commit is contained in:
Matthew Brown 2018-11-10 16:10:59 -05:00
parent 03ea94e087
commit 9f2fe748e8
12 changed files with 357 additions and 270 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

66
src/Psalm/Checker/Statements/Block/ForChecker.php
View File

@ -7,6 +7,7 @@ use Psalm\Checker\Statements\ExpressionChecker;
use Psalm\Checker\StatementsChecker;
use Psalm\Context;
use Psalm\Scope\LoopScope;
use Psalm\Type;
class ForChecker
{
@ -40,7 +41,11 @@ class ForChecker
$while_true = !$stmt->cond && !$stmt->init && !$stmt->loop;
$pre_context = $while_true ? clone $context : null;
$pre_context = null;
if ($while_true) {
$pre_context = clone $context;
}
$for_context = clone $context;
@ -68,33 +73,54 @@ class ForChecker
$inner_loop_context
);
if ($inner_loop_context && $while_true) {
// if we actually leave the loop
if (in_array(ScopeChecker::ACTION_BREAK, $loop_scope->final_actions, true)
|| in_array(ScopeChecker::ACTION_END, $loop_scope->final_actions, true)
) {
foreach ($inner_loop_context->vars_in_scope as $var_id => $type) {
if (!isset($context->vars_in_scope[$var_id])) {
$context->vars_in_scope[$var_id] = $type;
}
if (!$inner_loop_context) {
throw new \UnexpectedValueException('There should be an inner loop context');
}
$always_enters_loop = false;
foreach ($stmt->cond as $cond) {
if (isset($cond->inferredType)) {
foreach ($cond->inferredType->getTypes() as $iterator_type) {
$always_enters_loop = $iterator_type instanceof Type\Atomic\TTrue;
break;
}
}
}
if (!$while_true
|| in_array(ScopeChecker::ACTION_BREAK, $loop_scope->final_actions, true)
|| in_array(ScopeChecker::ACTION_END, $loop_scope->final_actions, true)
|| !$pre_context
) {
if ($while_true) {
$always_enters_loop = true;
}
$can_leave_loop = !$while_true
|| in_array(ScopeChecker::ACTION_BREAK, $loop_scope->final_actions, true);
if ($always_enters_loop && $can_leave_loop) {
foreach ($inner_loop_context->vars_in_scope as $var_id => $type) {
// if there are break statements in the loop it's not certain
// that the loop has finished executing
if (in_array(ScopeChecker::ACTION_BREAK, $loop_scope->final_actions, true)
|| in_array(ScopeChecker::ACTION_CONTINUE, $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;
}
}
}
if ($can_leave_loop) {
$context->vars_possibly_in_scope = array_merge(
$context->vars_possibly_in_scope,
$for_context->vars_possibly_in_scope
);
$context->possibly_assigned_var_ids =
$for_context->possibly_assigned_var_ids + $context->possibly_assigned_var_ids;
} else {
$context->vars_in_scope = $pre_context->vars_in_scope;
} elseif ($pre_context) {
$context->vars_possibly_in_scope = $pre_context->vars_possibly_in_scope;
}

59
src/Psalm/Checker/Statements/Block/ForeachChecker.php
View File

@ -4,6 +4,7 @@ namespace Psalm\Checker\Statements\Block;
use PhpParser;
use Psalm\Checker\ClassLikeChecker;
use Psalm\Checker\CommentChecker;
use Psalm\Checker\ScopeChecker;
use Psalm\Checker\Statements\Expression\AssignmentChecker;
use Psalm\Checker\Statements\ExpressionChecker;
use Psalm\Checker\StatementsChecker;
@ -40,20 +41,12 @@ class ForeachChecker
return false;
}
$foreach_context = clone $context;
$foreach_context->inside_loop = true;
$project_checker = $statements_checker->getFileChecker()->project_checker;
$codebase = $project_checker->codebase;
if ($project_checker->alter_code) {
$foreach_context->branch_point =
$foreach_context->branch_point ?: (int) $stmt->getAttribute('startFilePos');
}
$key_type = null;
$value_type = null;
$always_non_empty_array = true;
$var_id = ExpressionChecker::getVarId(
$stmt->expr,
@ -63,8 +56,8 @@ class ForeachChecker
if (isset($stmt->expr->inferredType)) {
$iterator_type = $stmt->expr->inferredType;
} elseif ($var_id && $foreach_context->hasVariable($var_id, $statements_checker)) {
$iterator_type = $foreach_context->vars_in_scope[$var_id];
} elseif ($var_id && $context->hasVariable($var_id, $statements_checker)) {
$iterator_type = $context->vars_in_scope[$var_id];
} else {
$iterator_type = null;
}
@ -110,11 +103,13 @@ class ForeachChecker
if ($iterator_type instanceof Type\Atomic\TArray
&& $iterator_type->type_params[1]->isEmpty()
) {
$always_non_empty_array = false;
$has_valid_iterator = true;
continue;
}
if ($iterator_type instanceof Type\Atomic\TNull || $iterator_type instanceof Type\Atomic\TFalse) {
$always_non_empty_array = false;
continue;
}
@ -122,7 +117,12 @@ class ForeachChecker
|| $iterator_type instanceof Type\Atomic\ObjectLike
) {
if ($iterator_type instanceof Type\Atomic\ObjectLike) {
if (!$iterator_type->sealed) {
$always_non_empty_array = false;
}
$iterator_type = $iterator_type->getGenericArrayType();
} elseif (!$iterator_type instanceof Type\Atomic\TNonEmptyArray) {
$always_non_empty_array = false;
}
if (!$value_type) {
@ -143,6 +143,8 @@ class ForeachChecker
continue;
}
$always_non_empty_array = false;
if ($iterator_type instanceof Type\Atomic\Scalar ||
$iterator_type instanceof Type\Atomic\TVoid
) {
@ -364,6 +366,15 @@ class ForeachChecker
}
}
$foreach_context = clone $context;
$foreach_context->inside_loop = true;
if ($project_checker->alter_code) {
$foreach_context->branch_point =
$foreach_context->branch_point ?: (int) $stmt->getAttribute('startFilePos');
}
if ($stmt->keyVar && $stmt->keyVar instanceof PhpParser\Node\Expr\Variable && is_string($stmt->keyVar->name)) {
$key_var_id = '$' . $stmt->keyVar->name;
$foreach_context->vars_in_scope[$key_var_id] = $key_type ?: Type::getMixed();
@ -457,7 +468,31 @@ class ForeachChecker
}
$loop_scope->protected_var_ids = $protected_var_ids;
LoopChecker::analyze($statements_checker, $stmt->stmts, [], [], $loop_scope);
LoopChecker::analyze($statements_checker, $stmt->stmts, [], [], $loop_scope, $inner_loop_context);
if (!$inner_loop_context) {
throw new \UnexpectedValueException('There should be an inner loop context');
}
if ($always_non_empty_array) {
foreach ($inner_loop_context->vars_in_scope as $var_id => $type) {
// 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)
|| in_array(ScopeChecker::ACTION_CONTINUE, $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;
}
}
}
$context->vars_possibly_in_scope = array_merge(
$foreach_context->vars_possibly_in_scope,

81
src/Psalm/Checker/Statements/Block/WhileChecker.php
View File

@ -6,6 +6,7 @@ use Psalm\Checker\ScopeChecker;
use Psalm\Checker\StatementsChecker;
use Psalm\Context;
use Psalm\Scope\LoopScope;
use Psalm\Type;
class WhileChecker
{
@ -54,30 +55,80 @@ class WhileChecker
return false;
}
if ($inner_loop_context && $while_true) {
// if we actually leave the loop
if (in_array(ScopeChecker::ACTION_BREAK, $loop_scope->final_actions, true)
|| in_array(ScopeChecker::ACTION_END, $loop_scope->final_actions, true)
) {
foreach ($inner_loop_context->vars_in_scope as $var_id => $type) {
if (!isset($context->vars_in_scope[$var_id])) {
$context->vars_in_scope[$var_id] = $type;
if (!$inner_loop_context) {
throw new \UnexpectedValueException('Should always enter loop');
}
$always_enters_loop = false;
if (isset($stmt->cond->inferredType)) {
$always_enters_loop = true;
foreach ($stmt->cond->inferredType->getTypes() as $iterator_type) {
if ($iterator_type instanceof Type\Atomic\TArray
|| $iterator_type instanceof Type\Atomic\ObjectLike
) {
if ($iterator_type instanceof Type\Atomic\ObjectLike) {
if (!$iterator_type->sealed) {
$always_enters_loop = false;
}
} elseif (!$iterator_type instanceof Type\Atomic\TNonEmptyArray) {
$always_enters_loop = false;
}
continue;
}
if ($iterator_type instanceof Type\Atomic\TTrue) {
continue;
}
if ($iterator_type instanceof Type\Atomic\TLiteralString
&& $iterator_type->value
) {
continue;
}
if ($iterator_type instanceof Type\Atomic\TLiteralInt
&& $iterator_type->value
) {
continue;
}
$always_enters_loop = false;
break;
}
}
$can_leave_loop = !$while_true
|| in_array(ScopeChecker::ACTION_BREAK, $loop_scope->final_actions, true);
if ($always_enters_loop && $can_leave_loop) {
foreach ($inner_loop_context->vars_in_scope as $var_id => $type) {
// 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)
|| in_array(ScopeChecker::ACTION_CONTINUE, $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;
}
}
}
if (!$while_true
|| in_array(ScopeChecker::ACTION_BREAK, $loop_scope->final_actions, true)
|| in_array(ScopeChecker::ACTION_END, $loop_scope->final_actions, true)
|| !$pre_context
) {
if ($can_leave_loop) {
$context->vars_possibly_in_scope = array_merge(
$context->vars_possibly_in_scope,
$while_context->vars_possibly_in_scope
);
} else {
$context->vars_in_scope = $pre_context->vars_in_scope;
} elseif ($pre_context) {
$context->vars_possibly_in_scope = $pre_context->vars_possibly_in_scope;
}

4
src/Psalm/Codebase.php
View File

@ -1012,8 +1012,6 @@ class Codebase
}
$recent_type = null;
$start_pos = null;
$end_pos = null;
krsort($type_map);
@ -1035,8 +1033,6 @@ class Codebase
if (!$recent_type
|| $recent_type === 'mixed'
|| $end_pos === null
|| $start_pos === null
) {
return null;
}

4
src/Psalm/Type/TypeCombination.php
View File

@ -226,10 +226,6 @@ class TypeCombination
}
}
if (!$objectlike_generic_type) {
throw new \InvalidArgumentException('Cannot be null');
}
$objectlike_generic_type->possibly_undefined = false;
$objectlike_key_type = new Type\Union(array_values($objectlike_keys));

36
tests/ForTest.php
View File

@ -1,36 +0,0 @@
<?php
namespace Psalm\Tests;
class ForTest extends TestCase
{
use Traits\FileCheckerValidCodeParseTestTrait;
/**
* @return array
*/
public function providerFileCheckerValidCodeParse()
{
return [
'continueOutsideLoop' => [
'<?php
class Node {
/** @var Node|null */
public $next;
}
/** @return void */
function test(Node $head) {
for ($node = $head; $node; $node = $next) {
$next = $node->next;
$node->next = null;
}
}',
],
'echoAfterFor' => [
'<?php
for ($i = 0; $i < 5; $i++);
echo $i;',
],
];
}
}

140
tests/ForeachTest.php
View File

@ -1,140 +0,0 @@
<?php
namespace Psalm\Tests;
class ForeachTest extends TestCase
{
use Traits\FileCheckerInvalidCodeParseTestTrait;
use Traits\FileCheckerValidCodeParseTestTrait;
/**
* @return array
*/
public function providerFileCheckerValidCodeParse()
{
return [
'iteratorAggregateIteration' => [
'<?php
class C implements IteratorAggregate
{
public function getIterator(): Iterator
{
return new ArrayIterator([]);
}
}
function loopT(Traversable $coll): void
{
foreach ($coll as $item) {}
}
function loopI(IteratorAggregate $coll): void
{
foreach ($coll as $item) {}
}
loopT(new C);
loopI(new C);',
'assignments' => [],
'error_levels' => [
'MixedAssignment', 'UndefinedThisPropertyAssignment',
],
],
'intersectionIterator' => [
'<?php
/**
* @param \Traversable<int>&\Countable $object
*/
function doSomethingUseful($object) : void {
echo count($object);
foreach ($object as $foo) {}
}'
],
'arrayIteratorIteration' => [
'<?php
class Item {
/**
* @var string
*/
public $prop = "var";
}
class Collection implements IteratorAggregate {
/**
* @var Item[]
*/
private $items = [];
public function add(Item $item): void
{
$this->items[] = $item;
}
/**
* @return \ArrayIterator<mixed, Item>
*/
public function getIterator(): \ArrayIterator
{
return new \ArrayIterator($this->items);
}
}
$collection = new Collection();
$collection->add(new Item());
foreach ($collection as $item) {
echo $item->prop;
}'
],
'foreachIntersectionTraversable' => [
'<?php
/** @var Countable&Traversable<int> */
$c = null;
foreach ($c as $i) {}',
],
];
}
/**
* @return array
*/
public function providerFileCheckerInvalidCodeParse()
{
return [
'continueOutsideLoop' => [
'<?php
continue;',
'error_message' => 'ContinueOutsideLoop',
],
'invalidIterator' => [
'<?php
foreach (5 as $a) {
}',
'error_message' => 'InvalidIterator',
],
'rawObjectIteration' => [
'<?php
class A {
/** @var ?string */
public $foo;
}
class B extends A {}
function bar(A $a): void {}
$arr = [];
if (rand(0, 10) > 5) {
$arr[] = new A;
} else {
$arr = new B;
}
foreach ($arr as $a) {
bar($a);
}',
'error_message' => 'RawObjectIteration',
],
];
}
}

22
tests/Loop/DoTest.php
View File

@ -123,20 +123,20 @@ class DoTest extends \Psalm\Tests\TestCase
'doWhileWithNotEmptyCheck' => [
'<?php
class A {
/** @var A|null */
public $a;
/** @var A|null */
public $a;
public function __construct() {
$this->a = rand(0, 1) ? new A : null;
}
public function __construct() {
$this->a = rand(0, 1) ? new A : null;
}
}
function takesA(A $a): void {}
$a = new A();
do {
takesA($a);
$a = $a->a;
takesA($a);
$a = $a->a;
} while ($a);',
'assertions' => [
'$a' => 'null',
@ -145,15 +145,15 @@ class DoTest extends \Psalm\Tests\TestCase
'doWhileWithMethodCall' => [
'<?php
class A {
public function getParent(): ?A {
return rand(0, 1) ? new A() : null;
}
public function getParent(): ?A {
return rand(0, 1) ? new A() : null;
}
}
$a = new A();
do {
$a = $a->getParent();
$a = $a->getParent();
} while ($a);',
'assertions' => [
'$a' => 'null',

35
tests/Loop/ForTest.php
View File

@ -73,6 +73,41 @@ class ForTest extends \Psalm\Tests\TestCase
}
}'
],
'whileTrueWithBreak' => [
'<?php
for (;;) {
$a = "hello";
break;
}
for (;;) {
$b = 5;
break;
}',
'assertions' => [
'$a' => 'string',
'$b' => 'int',
],
],
'continueOutsideLoop' => [
'<?php
class Node {
/** @var Node|null */
public $next;
}
/** @return void */
function test(Node $head) {
for ($node = $head; $node; $node = $next) {
$next = $node->next;
$node->next = null;
}
}',
],
'echoAfterFor' => [
'<?php
for ($i = 0; $i < 5; $i++);
echo $i;',
],
];
}

159
tests/Loop/ForeachTest.php
View File

@ -307,7 +307,7 @@ class ForeachTest extends \Psalm\Tests\TestCase
foreach (["a", "b", "c"] as $tag) {
}',
'assignments' => [
'$tag' => 'string|null',
'$tag' => 'string',
],
],
'bleedVarIntoOuterContextWithRedefinedAsNull' => [
@ -542,7 +542,7 @@ class ForeachTest extends \Psalm\Tests\TestCase
'<?php
$a = false;
foreach ([1, 2, 3] as $i) {
if ($i > 0) {
if ($i > 1) {
$a = true;
continue;
}
@ -631,6 +631,103 @@ class ForeachTest extends \Psalm\Tests\TestCase
}
if ($i) {}',
],
'possiblyUndefinedVariableInForeach' => [
'<?php
foreach ([1, 2, 3, 4] as $b) {
$car = "Volvo";
}
echo $car;',
],
'possiblyUndefinedVariableInForeachDueToBreakAfter' => [
'<?php
foreach ([1, 2, 3, 4] as $b) {
$car = "Volvo";
if (rand(0, 1)) {
break;
}
}
echo $car;',
],
'iteratorAggregateIteration' => [
'<?php
class C implements IteratorAggregate
{
public function getIterator(): Iterator
{
return new ArrayIterator([]);
}
}
function loopT(Traversable $coll): void
{
foreach ($coll as $item) {}
}
function loopI(IteratorAggregate $coll): void
{
foreach ($coll as $item) {}
}
loopT(new C);
loopI(new C);',
'assignments' => [],
'error_levels' => [
'MixedAssignment', 'UndefinedThisPropertyAssignment',
],
],
'intersectionIterator' => [
'<?php
/**
* @param \Traversable<int>&\Countable $object
*/
function doSomethingUseful($object) : void {
echo count($object);
foreach ($object as $foo) {}
}'
],
'arrayIteratorIteration' => [
'<?php
class Item {
/**
* @var string
*/
public $prop = "var";
}
class Collection implements IteratorAggregate {
/**
* @var Item[]
*/
private $items = [];
public function add(Item $item): void
{
$this->items[] = $item;
}
/**
* @return \ArrayIterator<mixed, Item>
*/
public function getIterator(): \ArrayIterator
{
return new \ArrayIterator($this->items);
}
}
$collection = new Collection();
$collection->add(new Item());
foreach ($collection as $item) {
echo $item->prop;
}'
],
'foreachIntersectionTraversable' => [
'<?php
/** @var Countable&Traversable<int> */
$c = null;
foreach ($c as $i) {}',
],
];
}
@ -668,16 +765,6 @@ class ForeachTest extends \Psalm\Tests\TestCase
'error_message' => 'PossiblyUndefinedGlobalVariable - src' . DIRECTORY_SEPARATOR . 'somefile.php:3 - Possibly undefined ' .
'global variable $array, first seen on line 3',
],
'possiblyUndefinedVariableInForeach' => [
'<?php
foreach ([1, 2, 3, 4] as $b) {
$car = "Volvo";
}
echo $car;',
'error_message' => 'PossiblyUndefinedGlobalVariable - src' . DIRECTORY_SEPARATOR . 'somefile.php:6 - Possibly undefined ' .
'global variable $car, first seen on line 3',
],
'possibleUndefinedVariableInForeachAndIfWithBreak' => [
'<?php
foreach ([1,2,3,4] as $i) {
@ -779,6 +866,54 @@ class ForeachTest extends \Psalm\Tests\TestCase
}',
'error_message' => 'LoopInvalidation',
],
'possiblyUndefinedVariableInForeachDueToBreakBefore' => [
'<?php
foreach ([1, 2, 3, 4] as $b) {
if (rand(0, 1)) {
break;
}
$car = "Volvo";
}
echo $car;',
'error_message' => 'PossiblyUndefinedGlobalVariable',
],
'continueOutsideLoop' => [
'<?php
continue;',
'error_message' => 'ContinueOutsideLoop',
],
'invalidIterator' => [
'<?php
foreach (5 as $a) {
}',
'error_message' => 'InvalidIterator',
],
'rawObjectIteration' => [
'<?php
class A {
/** @var ?string */
public $foo;
}
class B extends A {}
function bar(A $a): void {}
$arr = [];
if (rand(0, 10) > 5) {
$arr[] = new A;
} else {
$arr = new B;
}
foreach ($arr as $a) {
bar($a);
}',
'error_message' => 'RawObjectIteration',
],
];
}
}

7
tests/Loop/WhileTest.php
View File

@ -150,7 +150,7 @@ class WhileTest extends \Psalm\Tests\TestCase
if ($a->bar) {}
}',
],
'whileTrue' => [
'whileTrueWithBreak' => [
'<?php
while (true) {
$a = "hello";
@ -159,15 +159,10 @@ class WhileTest extends \Psalm\Tests\TestCase
while (1) {
$b = 5;
break;
}
for(;;) {
$c = true;
break;
}',
'assertions' => [
'$a' => 'string',
'$b' => 'int',
'$c' => 'true',
],
],
'whileWithNotEmptyCheck' => [

14
tests/Php71Test.php
View File

@ -148,17 +148,14 @@ class Php71Test extends TestCase
["id" => 2, "name" => "Fred"],
];
$last_id = null;
$last_name = null;
// list() style
foreach ($data as list("id" => $id, "name" => $name)) {
$last_id = $id;
$last_name = $name;
}',
'assertions' => [
'$last_id' => 'int|null',
'$last_name' => 'string|null',
'$last_id' => 'int',
'$last_name' => 'string',
],
],
'arrayDestructuringInForeachWithKeys' => [
@ -168,17 +165,14 @@ class Php71Test extends TestCase
["id" => 2, "name" => "Fred"],
];
$last_id = null;
$last_name = null;
// [] style
foreach ($data as ["id" => $id, "name" => $name]) {
$last_id = $id;
$last_name = $name;
}',
'assertions' => [
'$last_id' => 'int|null',
'$last_name' => 'string|null',
'$last_id' => 'int',
'$last_name' => 'string',
],
],
'iterableArg' => [