mirror of
https://github.com/danog/psalm.git
synced 2024-11-27 04:45:20 +01:00
Add slower (but more accurate) loop checks
This commit is contained in:
parent
6cfe540c98
commit
9b41bc9b37
@ -162,10 +162,6 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
|
||||
$have_emitted = false;
|
||||
|
||||
foreach ($implemented_method_ids as $implemented_method_id) {
|
||||
if ($have_emitted) {
|
||||
break;
|
||||
}
|
||||
|
||||
if ($this->function->name === '__construct') {
|
||||
continue;
|
||||
}
|
||||
@ -207,7 +203,7 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
|
||||
}
|
||||
|
||||
$have_emitted = true;
|
||||
break;
|
||||
break 2;
|
||||
}
|
||||
|
||||
if ($class_storage->user_defined &&
|
||||
@ -229,7 +225,7 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
|
||||
}
|
||||
|
||||
$have_emitted = true;
|
||||
break;
|
||||
break 2;
|
||||
}
|
||||
|
||||
if (!$class_storage->user_defined &&
|
||||
@ -252,7 +248,7 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
|
||||
}
|
||||
|
||||
$have_emitted = true;
|
||||
break;
|
||||
break 2;
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -21,6 +21,7 @@ class ForChecker
|
||||
Context $context
|
||||
) {
|
||||
$for_context = clone $context;
|
||||
$before_context = clone $context;
|
||||
$for_context->inside_loop = true;
|
||||
|
||||
foreach ($stmt->init as $init) {
|
||||
@ -37,7 +38,9 @@ class ForChecker
|
||||
$for_context->inside_conditional = false;
|
||||
}
|
||||
|
||||
$statements_checker->analyzeLoop($stmt->stmts, $for_context, $context);
|
||||
$changed_vars = Context::getNewOrUpdatedVarIds($before_context, $for_context);
|
||||
|
||||
$statements_checker->analyzeLoop($stmt->stmts, $changed_vars, $for_context, $context);
|
||||
|
||||
foreach ($stmt->loop as $expr) {
|
||||
if (ExpressionChecker::analyze($statements_checker, $expr, $for_context) === false) {
|
||||
|
@ -169,6 +169,8 @@ class ForeachChecker
|
||||
}
|
||||
}
|
||||
|
||||
$before_context = clone $foreach_context;
|
||||
|
||||
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();
|
||||
@ -202,7 +204,9 @@ class ForeachChecker
|
||||
);
|
||||
}
|
||||
|
||||
$statements_checker->analyzeLoop($stmt->stmts, $foreach_context, $context);
|
||||
$changed_vars = Context::getNewOrUpdatedVarIds($before_context, $foreach_context);
|
||||
|
||||
$statements_checker->analyzeLoop($stmt->stmts, $changed_vars, $foreach_context, $context);
|
||||
|
||||
foreach ($context->vars_in_scope as $var => $type) {
|
||||
if ($type->isMixed()) {
|
||||
|
@ -67,7 +67,9 @@ class WhileChecker
|
||||
$while_context->vars_in_scope = $while_vars_in_scope_reconciled;
|
||||
}
|
||||
|
||||
if ($statements_checker->analyzeLoop($stmt->stmts, $while_context, $context) === false) {
|
||||
$while_cond_vars = array_keys($reconcilable_while_types);
|
||||
|
||||
if ($statements_checker->analyzeLoop($stmt->stmts, $while_cond_vars, $while_context, $context) === false) {
|
||||
return false;
|
||||
}
|
||||
|
||||
|
@ -1341,14 +1341,14 @@ class ExpressionChecker
|
||||
/**
|
||||
* @param PhpParser\Node\Expr $stmt
|
||||
* @param string|null $this_class_name
|
||||
* @param StatementsSource $source
|
||||
* @param StatementsSource|null $source
|
||||
* @param int|null &$nesting
|
||||
* @return string|null
|
||||
*/
|
||||
public static function getVarId(
|
||||
PhpParser\Node\Expr $stmt,
|
||||
$this_class_name,
|
||||
StatementsSource $source,
|
||||
StatementsSource $source = null,
|
||||
&$nesting = null
|
||||
) {
|
||||
if ($stmt instanceof PhpParser\Node\Expr\Variable && is_string($stmt->name)) {
|
||||
@ -1361,15 +1361,17 @@ class ExpressionChecker
|
||||
) {
|
||||
if (count($stmt->class->parts) === 1 && in_array($stmt->class->parts[0], ['self', 'static', 'parent'])) {
|
||||
if (!$this_class_name) {
|
||||
throw new \UnexpectedValueException('$this_class_name should not be null');
|
||||
$fq_class_name = $stmt->class->parts[0];
|
||||
} else {
|
||||
$fq_class_name = $this_class_name;
|
||||
}
|
||||
|
||||
$fq_class_name = $this_class_name;
|
||||
} else {
|
||||
$fq_class_name = ClassLikeChecker::getFQCLNFromNameObject(
|
||||
$stmt->class,
|
||||
$source
|
||||
);
|
||||
$fq_class_name = $source
|
||||
? ClassLikeChecker::getFQCLNFromNameObject(
|
||||
$stmt->class,
|
||||
$source
|
||||
)
|
||||
: implode('\\', $stmt->class->parts);
|
||||
}
|
||||
|
||||
return $fq_class_name . '::$' . $stmt->name;
|
||||
@ -1396,13 +1398,13 @@ class ExpressionChecker
|
||||
/**
|
||||
* @param PhpParser\Node\Expr $stmt
|
||||
* @param string|null $this_class_name
|
||||
* @param StatementsSource $source
|
||||
* @param StatementsSource|null $source
|
||||
* @return string|null
|
||||
*/
|
||||
public static function getArrayVarId(
|
||||
PhpParser\Node\Expr $stmt,
|
||||
$this_class_name,
|
||||
StatementsSource $source
|
||||
StatementsSource $source = null
|
||||
) {
|
||||
if ($stmt instanceof PhpParser\Node\Expr\Assign) {
|
||||
return self::getArrayVarId($stmt->var, $this_class_name, $source);
|
||||
|
@ -329,36 +329,67 @@ class StatementsChecker extends SourceChecker implements StatementsSource
|
||||
* Checks an array of statements in a loop
|
||||
*
|
||||
* @param array<PhpParser\Node\Stmt|PhpParser\Node\Expr> $stmts
|
||||
* @param array<int, string> $asserted_vars
|
||||
* @param Context $loop_context
|
||||
* @param Context $outer_context
|
||||
* @return void
|
||||
*/
|
||||
public function analyzeLoop(
|
||||
array $stmts,
|
||||
array $asserted_vars,
|
||||
Context $loop_context,
|
||||
Context $outer_context
|
||||
) {
|
||||
// record all the vars that existed before we did the first pass through the loop
|
||||
$pre_loop_context = clone $loop_context;
|
||||
$traverser = new PhpParser\NodeTraverser;
|
||||
|
||||
IssueBuffer::startRecording();
|
||||
$this->analyze($stmts, $loop_context, $outer_context);
|
||||
$recorded_issues = IssueBuffer::clearRecordingLevel();
|
||||
IssueBuffer::stopRecording();
|
||||
$assignment_mapper = new \Psalm\Visitor\AssignmentMapVisitor($loop_context->self);
|
||||
$traverser->addVisitor($assignment_mapper);
|
||||
|
||||
if ($recorded_issues) {
|
||||
do {
|
||||
$traverser->traverse($stmts);
|
||||
|
||||
$assignment_map = $assignment_mapper->getAssignmentMap();
|
||||
|
||||
$assignment_depth = 0;
|
||||
|
||||
if ($assignment_map) {
|
||||
$first_var_id = array_keys($assignment_map)[0];
|
||||
|
||||
$assignment_depth = self::getAssignmentMapDepth($first_var_id, $assignment_map);
|
||||
}
|
||||
|
||||
if ($assignment_depth === 0) {
|
||||
$this->analyze($stmts, $loop_context, $outer_context);
|
||||
} else {
|
||||
// record all the vars that existed before we did the first pass through the loop
|
||||
$pre_loop_context = clone $loop_context;
|
||||
$pre_outer_context = clone $outer_context;
|
||||
|
||||
IssueBuffer::startRecording();
|
||||
$this->analyze($stmts, $loop_context, $outer_context);
|
||||
$recorded_issues = IssueBuffer::clearRecordingLevel();
|
||||
IssueBuffer::stopRecording();
|
||||
|
||||
for ($i = 0; $i < $assignment_depth; $i++) {
|
||||
$vars_to_remove = [];
|
||||
|
||||
// widen the foreach context type with the initial context type
|
||||
foreach ($loop_context->vars_in_scope as $var_id => $type) {
|
||||
if (isset($pre_loop_context->vars_in_scope[$var_id])) {
|
||||
$loop_context->vars_in_scope[$var_id] = Type::combineUnionTypes(
|
||||
$type,
|
||||
$pre_loop_context->vars_in_scope[$var_id]
|
||||
);
|
||||
$has_changes = false;
|
||||
|
||||
if (isset($outer_context->vars_in_scope[$var_id])) {
|
||||
foreach ($loop_context->vars_in_scope as $var_id => $type) {
|
||||
if (in_array($var_id, $asserted_vars)) {
|
||||
// set the vars to whatever the while/foreach loop expects them to be
|
||||
if ((string)$type !== (string)$pre_loop_context->vars_in_scope[$var_id]) {
|
||||
$loop_context->vars_in_scope[$var_id] = $pre_loop_context->vars_in_scope[$var_id];
|
||||
$has_changes = true;
|
||||
}
|
||||
} elseif (isset($pre_outer_context->vars_in_scope[$var_id])) {
|
||||
$pre_outer = (string)$pre_outer_context->vars_in_scope[$var_id];
|
||||
|
||||
if ((string)$type !== $pre_outer ||
|
||||
(string)$outer_context->vars_in_scope[$var_id] !== $pre_outer
|
||||
) {
|
||||
$has_changes = true;
|
||||
|
||||
// widen the foreach context type with the initial context type
|
||||
$loop_context->vars_in_scope[$var_id] = Type::combineUnionTypes(
|
||||
$loop_context->vars_in_scope[$var_id],
|
||||
$outer_context->vars_in_scope[$var_id]
|
||||
@ -369,6 +400,11 @@ class StatementsChecker extends SourceChecker implements StatementsSource
|
||||
}
|
||||
}
|
||||
|
||||
// if there are no changes to the types, no need to re-examine
|
||||
if (!$has_changes) {
|
||||
break;
|
||||
}
|
||||
|
||||
// remove vars that were defined in the foreach
|
||||
foreach ($vars_to_remove as $var_id) {
|
||||
unset($loop_context->vars_in_scope[$var_id]);
|
||||
@ -376,11 +412,9 @@ class StatementsChecker extends SourceChecker implements StatementsSource
|
||||
|
||||
IssueBuffer::startRecording();
|
||||
$this->analyze($stmts, $loop_context, $outer_context);
|
||||
$last_recorded_issues_count = count($recorded_issues);
|
||||
$recorded_issues = IssueBuffer::clearRecordingLevel();
|
||||
|
||||
IssueBuffer::stopRecording();
|
||||
} while (count($recorded_issues) < $last_recorded_issues_count);
|
||||
}
|
||||
|
||||
if ($recorded_issues) {
|
||||
foreach ($recorded_issues as $recorded_issue) {
|
||||
@ -391,6 +425,33 @@ class StatementsChecker extends SourceChecker implements StatementsSource
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @param string $first_var_id
|
||||
* @param array<string, array<string, bool>> $assignment_map
|
||||
* @return int
|
||||
*/
|
||||
private static function getAssignmentMapDepth($first_var_id, array $assignment_map)
|
||||
{
|
||||
$max_depth = 0;
|
||||
|
||||
$assignment_var_ids = $assignment_map[$first_var_id];
|
||||
unset($assignment_map[$first_var_id]);
|
||||
|
||||
foreach ($assignment_var_ids as $assignment_var_id => $_) {
|
||||
$depth = 1;
|
||||
|
||||
if (isset($assignment_map[$assignment_var_id])) {
|
||||
$depth = 1 + self::getAssignmentMapDepth($assignment_var_id, $assignment_map);
|
||||
}
|
||||
|
||||
if ($depth > $max_depth) {
|
||||
$max_depth = $depth;
|
||||
}
|
||||
}
|
||||
|
||||
return $max_depth;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param PhpParser\Node\Stmt\Static_ $stmt
|
||||
* @param Context $context
|
||||
@ -474,7 +535,7 @@ class StatementsChecker extends SourceChecker implements StatementsSource
|
||||
{
|
||||
$do_context = clone $context;
|
||||
|
||||
if ($this->analyzeLoop($stmt->stmts, $do_context, $context) === false) {
|
||||
if ($this->analyzeLoop($stmt->stmts, [], $do_context, $context) === false) {
|
||||
return false;
|
||||
}
|
||||
|
||||
|
@ -500,16 +500,16 @@ class TypeChecker
|
||||
if ((string) $result_type === 'array') {
|
||||
$result_type = Type::getArray();
|
||||
}
|
||||
|
||||
if ($result_type === false) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
if ($result_type === null) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if ($result_type === false) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ((string)$result_type !== $before_adjustment) {
|
||||
$changed_types[] = $key;
|
||||
}
|
||||
|
@ -206,6 +206,26 @@ class Context
|
||||
return $redefined_vars;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param Context $original_context
|
||||
* @param Context $new_context
|
||||
* @return array<int, string>
|
||||
*/
|
||||
public static function getNewOrUpdatedVarIds(Context $original_context, Context $new_context)
|
||||
{
|
||||
$redefined_var_ids = [];
|
||||
|
||||
foreach ($new_context->vars_in_scope as $var_id => $context_type) {
|
||||
if (!isset($original_context->vars_in_scope[$var_id]) ||
|
||||
(string)$original_context->vars_in_scope[$var_id] !== (string)$context_type
|
||||
) {
|
||||
$redefined_var_ids[] = $var_id;
|
||||
}
|
||||
}
|
||||
|
||||
return $redefined_var_ids;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param string $remove_var_id
|
||||
* @return void
|
||||
|
50
src/Psalm/Visitor/AssignmentMapVisitor.php
Normal file
50
src/Psalm/Visitor/AssignmentMapVisitor.php
Normal file
@ -0,0 +1,50 @@
|
||||
<?php
|
||||
namespace Psalm\Visitor;
|
||||
|
||||
use PhpParser;
|
||||
use Psalm\Checker\Statements\ExpressionChecker;
|
||||
|
||||
class AssignmentMapVisitor extends PhpParser\NodeVisitorAbstract implements PhpParser\NodeVisitor
|
||||
{
|
||||
/**
|
||||
* @var array<string, array<string, bool>>
|
||||
*/
|
||||
protected $assignment_map = [];
|
||||
|
||||
/**
|
||||
* @var string|null
|
||||
*/
|
||||
protected $this_class_name;
|
||||
|
||||
/**
|
||||
* @param string|null $this_class_name
|
||||
*/
|
||||
public function __construct($this_class_name)
|
||||
{
|
||||
$this->this_class_name = $this_class_name;
|
||||
}
|
||||
|
||||
public function enterNode(PhpParser\Node $node)
|
||||
{
|
||||
if ($node instanceof PhpParser\Node\Expr\Assign) {
|
||||
$left_var_id = ExpressionChecker::getVarId($node->var, $this->this_class_name);
|
||||
$right_var_id = ExpressionChecker::getVarId($node->expr, $this->this_class_name);
|
||||
|
||||
if ($left_var_id) {
|
||||
$this->assignment_map[$left_var_id][(string)$right_var_id] = true;
|
||||
}
|
||||
|
||||
return PhpParser\NodeTraverser::DONT_TRAVERSE_CHILDREN;
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return array<string, array<string, bool>>
|
||||
*/
|
||||
public function getAssignmentMap()
|
||||
{
|
||||
return $this->assignment_map;
|
||||
}
|
||||
}
|
Loading…
Reference in New Issue
Block a user