From f455851f89da411747a38081d46296fdf9a2cb1d Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Fri, 9 Sep 2016 18:36:35 -0400 Subject: [PATCH] Fix bugs in array key checks --- src/Psalm/Checker/FunctionChecker.php | 10 ++++----- src/Psalm/Checker/StatementsChecker.php | 15 +++++++++----- src/Psalm/Checker/TypeChecker.php | 5 +++++ src/Psalm/Type.php | 27 ++++++++++++++++--------- src/Psalm/Type/ParseTree.php | 10 ++++++++- tests/TypeCombinationTest.php | 11 ++++++++++ 6 files changed, 57 insertions(+), 21 deletions(-) diff --git a/src/Psalm/Checker/FunctionChecker.php b/src/Psalm/Checker/FunctionChecker.php index 879bc799f..6742a1846 100644 --- a/src/Psalm/Checker/FunctionChecker.php +++ b/src/Psalm/Checker/FunctionChecker.php @@ -241,7 +241,7 @@ class FunctionChecker extends FunctionLikeChecker else { $inner_type = new Type\Union($closure_return_types); - return new Type\Union([new Type\Generic('array', [$inner_type])]); + return new Type\Union([new Type\Generic('array', [Type::getInt(), $inner_type])]); } } elseif ($call_args[0]->value instanceof PhpParser\Node\Scalar\String_) { @@ -250,7 +250,7 @@ class FunctionChecker extends FunctionLikeChecker if (isset($call_map[$mapped_function_id][0])) { if ($call_map[$mapped_function_id][0]) { $mapped_function_return = Type::parseString($call_map[$mapped_function_id][0]); - return new Type\Union([new Type\Generic('array', [$mapped_function_return])]); + return new Type\Union([new Type\Generic('array', [Type::getInt(), $mapped_function_return])]); } } else { @@ -272,7 +272,7 @@ class FunctionChecker extends FunctionLikeChecker $inner_value_types = []; $inner_key_types = []; - foreach ($call_args as $call_arg) { + foreach ($call_args as $offset => $call_arg) { if (!isset($call_arg->value->inferredType)) { return Type::getArray(); } @@ -290,7 +290,7 @@ class FunctionChecker extends FunctionLikeChecker $inner_value_types = array_merge(array_values($type_part->type_params[1]->types), $inner_value_types); } - if ($inner_types) { + if ($inner_value_types) { return new Type\Union([ new Type\Generic('array', [ @@ -306,7 +306,7 @@ class FunctionChecker extends FunctionLikeChecker } if ($call_map_key === 'explode') { - return Type::parseString('array'); + return Type::parseString('array'); } if (!isset($call_map[$call_map_key]) || !$call_map[$call_map_key][0]) { diff --git a/src/Psalm/Checker/StatementsChecker.php b/src/Psalm/Checker/StatementsChecker.php index 166b02fdc..390fb66d6 100644 --- a/src/Psalm/Checker/StatementsChecker.php +++ b/src/Psalm/Checker/StatementsChecker.php @@ -683,8 +683,6 @@ class StatementsChecker $context->update($old_else_context, $else_context, $has_leaving_statements, array_keys($negatable_if_types), $updated_vars); } - - if (!$has_ending_statements) { $vars = array_diff_key($else_context->vars_possibly_in_scope, $context->vars_possibly_in_scope); @@ -1793,7 +1791,7 @@ class StatementsChecker } if ($value_index) { - $key_type_part = $return_type->type_params[$key_index]; + $key_type_part = $return_type->type_params[0]; if (!$key_type) { $key_type = $key_type_part; @@ -2286,7 +2284,7 @@ class StatementsChecker if ($type->type_params[1]->isEmpty()) { // boil this down to a regular array if ($assignment_value_type->isMixed()) { - return Type::getArray(); + return Type::getArray()->types['array']; } $type->type_params[0] = $assignment_key_type; @@ -3496,6 +3494,10 @@ class StatementsChecker } } + if (!isset($stmt->inferredType)) { + $stmt->inferredType = Type::getMixed(); + } + if (!$key_type) { $key_type = new Type\Union([ new Type\Atomic('int'), @@ -3510,7 +3512,10 @@ class StatementsChecker if (isset($stmt->dim->inferredType) && $key_type) { foreach ($stmt->dim->inferredType->types as $at) { - if (!$at->isIn($key_type)) { + if ($at->isMixed()) { + // @todo emit issue + } + elseif (!$at->isIn($key_type)) { $var_id = self::getVarId($stmt->var); if (IssueBuffer::accepts( diff --git a/src/Psalm/Checker/TypeChecker.php b/src/Psalm/Checker/TypeChecker.php index 98de2e5de..3990c5eab 100644 --- a/src/Psalm/Checker/TypeChecker.php +++ b/src/Psalm/Checker/TypeChecker.php @@ -727,6 +727,11 @@ class TypeChecker $line_number, $suppressed_issues ); + + // special case if result is just a simple array + if ((string) $result_type === 'array') { + $result_type = Type::getArray(); + } } //echo((string) $new_types[$key] . ' and ' . (isset($existing_types[$key]) ? (string) $existing_types[$key] : '') . ' => ' . $result_type . PHP_EOL); diff --git a/src/Psalm/Type.php b/src/Psalm/Type.php index 360f8aa50..bf314e82e 100644 --- a/src/Psalm/Type.php +++ b/src/Psalm/Type.php @@ -27,6 +27,10 @@ abstract class Type if (count($type_tokens) === 1) { $type_tokens[0] = self::fixScalarTerms($type_tokens[0]); + if ($type_tokens[0] === 'array') { + return Type::getArray(); + } + return new Union([new Atomic($type_tokens[0])]); } @@ -547,7 +551,10 @@ abstract class Type } } else { - $key_types[$type->value][(string) $type] = null; + if ($type->value === 'array') { + throw new \InvalidArgumentException('Cannot have a non-generic array'); + } + $value_types[$type->value][(string) $type] = null; } } @@ -561,15 +568,21 @@ abstract class Type $new_types = []; foreach ($value_types as $generic_type => $value_type) { - $key_type = $key_types[$generic_type]; + $key_type = isset($key_types[$generic_type]) ? $key_types[$generic_type] : []; + + $expanded_key_types = []; + + foreach ($key_type as $expandable_key_type) { + $expanded_key_types = array_merge($expanded_key_types, array_values($expandable_key_type->types)); + } if (count($value_type) === 1) { $value_type_param = array_values($value_type)[0]; $generic_type_params = [$value_type_param]; // if we're continuing, also add the correspoinding key type param if it exists - if (count($key_type) === 1) { - array_unshift($generic_type_params, array_values($key_type)[0]); + if ($expanded_key_types) { + array_unshift($generic_type_params, self::combineTypes($expanded_key_types)); } $new_types[] = $value_type_param ? new Generic($generic_type, $generic_type_params) : new Atomic($generic_type); @@ -582,12 +595,6 @@ abstract class Type $expanded_value_types = array_merge($expanded_value_types, array_values($expandable_value_type->types)); } - $expanded_key_types = []; - - foreach ($key_type as $expandable_key_type) { - $expanded_key_types = array_merge($expanded_key_types, array_values($expandable_key_type->types)); - } - $generic_type_params = [self::combineTypes($expanded_value_types)]; if ($expanded_key_types) { diff --git a/src/Psalm/Type/ParseTree.php b/src/Psalm/Type/ParseTree.php index 3d99a3400..d777cdd69 100644 --- a/src/Psalm/Type/ParseTree.php +++ b/src/Psalm/Type/ParseTree.php @@ -74,10 +74,18 @@ class ParseTree case ',': $current_parent = $current_leaf->parent; - if (!$current_parent || $current_parent->value !== self::GENERIC) { + if (!$current_parent) { throw new \InvalidArgumentException('Cannot parse comma in non-generic type'); } + if ($current_parent->value !== self::GENERIC) { + if (!$current_parent->parent->value) { + throw new \InvalidArgumentException('Cannot parse comma in non-generic type'); + } + + $current_leaf = $current_leaf->parent; + } + break; case '|': diff --git a/tests/TypeCombinationTest.php b/tests/TypeCombinationTest.php index d6afb68e3..dd8de649d 100644 --- a/tests/TypeCombinationTest.php +++ b/tests/TypeCombinationTest.php @@ -87,6 +87,17 @@ class TypeCombinationTest extends PHPUnit_Framework_TestCase ); } + public function testArrayMixedOrStringKeys() + { + $this->assertEquals( + 'array', + (string) Type::combineTypes([ + self::getAtomic('array'), + self::getAtomic('array') + ]) + ); + } + public function testArrayMixedOrEmpty() { $this->assertEquals(