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

Fix #515 - make Psalm aware of variable array keys

Browse Source
This commit is contained in:
Matthew Brown 2018-02-17 11:24:08 -05:00
parent f56edd3c04
commit 88ca7d2fa7
11 changed files with 141 additions and 111 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
src/Psalm/Checker/Statements/Block/IfChecker.php
View File

@ -284,7 +284,7 @@ class IfChecker
// we calculate the vars redefined in a hypothetical else statement to determine
// which vars of the if we can safely change
$pre_assignment_else_redefined_vars = $temp_else_context->getRedefinedVars($context->vars_in_scope);
$pre_assignment_else_redefined_vars = $temp_else_context->getRedefinedVars($context->vars_in_scope, true);
$old_unreferenced_vars = $context->unreferenced_vars;
$newly_unreferenced_locations = [];
@ -426,10 +426,6 @@ class IfChecker
}
}
} else {
if ($if_scope->forced_new_vars) {
$context->vars_in_scope = array_merge($context->vars_in_scope, $if_scope->forced_new_vars);
}
if ($loop_scope && !in_array(ScopeChecker::ACTION_NONE, $if_scope->final_actions, true)) {
$loop_scope->redefined_loop_vars = null;
}
@ -556,19 +552,6 @@ class IfChecker
if (!$has_leaving_statements) {
$if_scope->new_vars = array_diff_key($if_context->vars_in_scope, $outer_context->vars_in_scope);
// if we have a check like if (!isset($a)) { $a = true; } we want to make sure $a is always set
foreach ($if_scope->new_vars as $var_id => $_) {
if (isset($if_scope->negated_types[$var_id])
&& (
$if_scope->negated_types[$var_id] === 'isset'
|| $if_scope->negated_types[$var_id] === '^isset'
|| $if_scope->negated_types[$var_id] === '!empty'
)
) {
$if_scope->forced_new_vars[$var_id] = Type::getMixed();
}
}
$if_scope->redefined_vars = $if_context->getRedefinedVars($outer_context->vars_in_scope);
$if_scope->possibly_redefined_vars = $if_scope->redefined_vars;
$if_scope->assigned_var_ids = $new_assigned_var_ids;

2
src/Psalm/Checker/Statements/Block/SwitchChecker.php
View File

@ -220,8 +220,6 @@ class SwitchChecker
} else {
$case_redefined_vars = $case_context->getRedefinedVars($original_context->vars_in_scope);
Type::redefineGenericUnionTypes($case_redefined_vars, $context);
if ($possibly_redefined_vars === null) {
$possibly_redefined_vars = $case_redefined_vars;
} else {

6
src/Psalm/Checker/Statements/Expression/Assignment/ArrayAssignmentChecker.php
View File

@ -136,6 +136,12 @@ class ArrayAssignmentChecker
if ($child_stmt->dim instanceof PhpParser\Node\Scalar\String_) {
$var_id_additions[] = '[\'' . $child_stmt->dim->value . '\']';
} elseif ($child_stmt->dim instanceof PhpParser\Node\Scalar\LNumber) {
$var_id_additions[] = '[' . $child_stmt->dim->value . ']';
} elseif ($child_stmt->dim instanceof PhpParser\Node\Expr\Variable
&& is_string($child_stmt->dim->name)
) {
$var_id_additions[] = '[$' . $child_stmt->dim->name . ']';
} else {
$var_id_additions[] = '[' . $child_stmt->dim->inferredType . ']';
$real_var_id = false;

5
src/Psalm/Checker/Statements/Expression/Fetch/ArrayFetchChecker.php
View File

@ -82,6 +82,11 @@ class ArrayFetchChecker
return false;
}
if ($keyed_array_var_id && isset($context->vars_in_scope[$keyed_array_var_id])) {
$stmt->inferredType = clone $context->vars_in_scope[$keyed_array_var_id];
return;
}
if (isset($stmt->var->inferredType)) {
/** @var Type\Union */
$var_type = $stmt->var->inferredType;

26
src/Psalm/Checker/Statements/ExpressionChecker.php
View File

@ -672,16 +672,26 @@ class ExpressionChecker
return self::getArrayVarId($stmt->var, $this_class_name, $source);
}
if ($stmt instanceof PhpParser\Node\Expr\ArrayDimFetch &&
($stmt->dim instanceof PhpParser\Node\Scalar\String_ ||
$stmt->dim instanceof PhpParser\Node\Scalar\LNumber)
) {
if ($stmt instanceof PhpParser\Node\Expr\ArrayDimFetch) {
$root_var_id = self::getArrayVarId($stmt->var, $this_class_name, $source);
$offset = $stmt->dim instanceof PhpParser\Node\Scalar\String_
? '\'' . $stmt->dim->value . '\''
: $stmt->dim->value;
return $root_var_id ? $root_var_id . '[' . $offset . ']' : null;
$offset = null;
if ($root_var_id) {
if ($stmt->dim instanceof PhpParser\Node\Scalar\String_
|| $stmt->dim instanceof PhpParser\Node\Scalar\LNumber
) {
$offset = $stmt->dim instanceof PhpParser\Node\Scalar\String_
? '\'' . $stmt->dim->value . '\''
: $stmt->dim->value;
} elseif ($stmt->dim instanceof PhpParser\Node\Expr\Variable
&& is_string($stmt->dim->name)
) {
$offset = '$' . $stmt->dim->name;
}
return $root_var_id && $offset !== null ? $root_var_id . '[' . $offset . ']' : null;
}
}
return self::getVarId($stmt, $this_class_name, $source);

2
src/Psalm/Codebase/Scanner.php
View File

@ -227,7 +227,9 @@ class Scanner
$this->reflection->registerClass($reflected_class);
$this->reflected_classlikes_lc[$fq_classlike_name_lc] = true;
} elseif ($this->fileExistsForClassLike($classlikes, $fq_classlike_name)) {
// even though we've checked this above, calling the method invalidates it
if (isset($this->classlike_files[$fq_classlike_name_lc])) {
/** @var string */
$file_path = $this->classlike_files[$fq_classlike_name_lc];
$this->files_to_scan[$file_path] = $file_path;
if (isset($this->classes_to_deep_scan[$fq_classlike_name_lc])) {

91
src/Psalm/Context.php
View File

@ -248,57 +248,68 @@ class Context
array $vars_to_update,
array &$updated_vars
) {
foreach ($this->vars_in_scope as $var => &$context_type) {
if (isset($start_context->vars_in_scope[$var])) {
$old_type = $start_context->vars_in_scope[$var];
foreach ($start_context->vars_in_scope as $var_id => $old_type) {
// this is only true if there was some sort of type negation
if (in_array($var_id, $vars_to_update, true)) {
// if we're leaving, we're effectively deleting the possibility of the if types
$new_type = !$has_leaving_statements && $end_context->hasVariable($var_id)
? $end_context->vars_in_scope[$var_id]
: null;
// this is only true if there was some sort of type negation
if (in_array($var, $vars_to_update, true)) {
// if we're leaving, we're effectively deleting the possibility of the if types
$new_type = !$has_leaving_statements && $end_context->hasVariable($var)
? $end_context->vars_in_scope[$var]
: null;
$existing_type = isset($this->vars_in_scope[$var_id]) ? $this->vars_in_scope[$var_id] : null;
// if the type changed within the block of statements, process the replacement
// also never allow ourselves to remove all types from a union
if ((!$new_type || $old_type->getId() !== $new_type->getId())
&& ($new_type || count($context_type->getTypes()) > 1)
) {
$context_type->substitute($old_type, $new_type);
if ($new_type && $new_type->from_docblock) {
$context_type->setFromDocblock();
}
$updated_vars[$var] = true;
if (!$existing_type) {
if ($new_type) {
$this->vars_in_scope[$var_id] = clone $new_type;
$updated_vars[$var_id] = true;
}
continue;
}
// if the type changed within the block of statements, process the replacement
// also never allow ourselves to remove all types from a union
if ((!$new_type || $old_type->getId() !== $new_type->getId())
&& ($new_type || count($existing_type->getTypes()) > 1)
) {
$existing_type->substitute($old_type, $new_type);
if ($new_type && $new_type->from_docblock) {
$existing_type->setFromDocblock();
}
$updated_vars[$var_id] = true;
}
}
}
}
/**
* @param array<string, Type\Union> $vars_in_scope
* @param array<string, Type\Union> $new_vars_in_scope
* @param bool $include_new_vars
*
* @return array<string,Type\Union>
*/
public function getRedefinedVars(array $vars_in_scope)
public function getRedefinedVars(array $new_vars_in_scope, $include_new_vars = false)
{
$redefined_vars = [];
foreach ($vars_in_scope as $var => $context_type) {
if (!isset($this->vars_in_scope[$var])) {
foreach ($this->vars_in_scope as $var_id => $this_type) {
if (!isset($new_vars_in_scope[$var_id])) {
if ($include_new_vars) {
$redefined_vars[$var_id] = $this_type;
}
continue;
}
$this_var = $this->vars_in_scope[$var];
$new_type = $new_vars_in_scope[$var_id];
if (!$this_var->failed_reconciliation
&& !$this_var->isEmpty()
&& !$context_type->isEmpty()
&& $this_var->getId() !== $context_type->getId()
if (!$this_type->failed_reconciliation
&& !$this_type->isEmpty()
&& !$new_type->isEmpty()
&& $this_type->getId() !== $new_type->getId()
) {
$redefined_vars[$var] = $this_var;
$redefined_vars[$var_id] = $this_type;
}
}
@ -420,7 +431,7 @@ class Context
$quoted_remove_var_id = preg_quote($remove_var_id);
foreach ($clause->possibilities as $var_id => $_) {
if (preg_match('/^' . $quoted_remove_var_id . '[\[\-]/', $var_id)) {
if (preg_match('/' . $quoted_remove_var_id . '[\]\[\-]/', $var_id)) {
break 2;
}
}
@ -523,18 +534,16 @@ class Context
);
}
if ($existing_type->hasArray() || $existing_type->isMixed() || $existing_type->hasObjectType()) {
$vars_to_remove = [];
$vars_to_remove = [];
foreach ($this->vars_in_scope as $var_id => $_) {
if (preg_match('/^' . preg_quote($remove_var_id, DIRECTORY_SEPARATOR) . '[\[\-]/', $var_id)) {
$vars_to_remove[] = $var_id;
}
foreach ($this->vars_in_scope as $var_id => $_) {
if (preg_match('/' . preg_quote($remove_var_id, DIRECTORY_SEPARATOR) . '[\]\[\-]/', $var_id)) {
$vars_to_remove[] = $var_id;
}
}
foreach ($vars_to_remove as $var_id) {
unset($this->vars_in_scope[$var_id]);
}
foreach ($vars_to_remove as $var_id) {
unset($this->vars_in_scope[$var_id]);
}
}

5
src/Psalm/Scope/IfScope.php
View File

@ -16,11 +16,6 @@ class IfScope
*/
public $new_vars_possibly_in_scope = [];
/**
* @var array<string, Type\Union>|null
*/
public $forced_new_vars = null;
/**
* @var array<string, Type\Union>|null
*/

37
src/Psalm/Type.php
View File

@ -560,43 +560,6 @@ abstract class Type
return new Union([new TResource]);
}
/**
* @param array<string, Union> $redefined_vars
* @param Context $context
*
* @return void
*/
public static function redefineGenericUnionTypes(array $redefined_vars, Context $context)
{
foreach ($redefined_vars as $var_name => $redefined_union_type) {
foreach ($redefined_union_type->getTypes() as $redefined_atomic_type) {
foreach ($context->vars_in_scope[$var_name]->getTypes() as $context_type) {
if ($context_type instanceof Type\Atomic\TArray &&
$redefined_atomic_type instanceof Type\Atomic\TArray
) {
if ($context_type->type_params[1]->isEmpty()) {
$context_type->type_params[1] = $redefined_atomic_type->type_params[1];
} else {
$context_type->type_params[1] = Type::combineUnionTypes(
$redefined_atomic_type->type_params[1],
$context_type->type_params[1]
);
}
if ($context_type->type_params[0]->isEmpty()) {
$context_type->type_params[0] = $redefined_atomic_type->type_params[0];
} else {
$context_type->type_params[0] = Type::combineUnionTypes(
$redefined_atomic_type->type_params[0],
$context_type->type_params[0]
);
}
}
}
}
}
}
/**
* Combines two union types into one
*

2
src/Psalm/Type/Reconciler.php
View File

@ -944,6 +944,8 @@ class Reconciler
$new_base_type_candidate = clone $existing_key_type_part->type_params[1];
} elseif (!$existing_key_type_part instanceof Type\Atomic\ObjectLike) {
return null;
} elseif ($array_key[0] === '$') {
$new_base_type_candidate = $existing_key_type_part->getGenericValueType();
} else {
$array_properties = $existing_key_type_part->properties;

57
tests/IssetTest.php
View File

@ -131,6 +131,52 @@ class IssetTest extends TestCase
return new A();
}',
],
'issetVariableKeysWithoutChange' => [
'<?php
$arr = [[1, 2, 3], null, [1, 2, 3], null];
$b = 2;
$c = 0;
if (isset($arr[$b][$c])) {
echo $arr[$b][$c];
}',
],
'issetNonNullArrayKey' => [
'<?php
/**
* @param array<int, int> $arr
*/
function foo(array $arr) : int {
$b = rand(0, 3);
if (!isset($arr[$b])) {
throw new \Exception("bad");
}
return $arr[$b];
}',
],
'issetArrayOffsetConditionalCreationWithInt' => [
'<?php
/** @param array<int, string> $arr */
function foo(array $arr) : string {
if (!isset($arr[0])) {
$arr[0] = "hello";
}
return $arr[0];
}',
],
'issetArrayOffsetConditionalCreationWithVariable' => [
'<?php
/** @param array<int, string> $arr */
function foo(array $arr) : string {
$b = 5;
if (!isset($arr[$b])) {
$arr[$b] = "hello";
}
return $arr[$b];
}',
],
];
}
@ -146,6 +192,17 @@ class IssetTest extends TestCase
$a = isset(A::foo()[0]);',
'error_message' => 'UndefinedMethod',
],
'issetVariableKeysWithChange' => [
'<?php
$arr = [[1, 2, 3], null, [1, 2, 3], null];
$b = 2;
$c = 0;
if (isset($arr[$b][$c])) {
$b = 1;
echo $arr[$b][$c];
}',
'error_message' => 'PossiblyNullArrayAccess',
],
];
}
}