From 1b91f566c4d601e9df936ede84964bb89ddca6e4 Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Tue, 12 Mar 2024 22:52:15 +0100 Subject: [PATCH 1/5] add tests for int-like string array keys --- tests/ArrayKeysTest.php | 42 ++++++++++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/tests/ArrayKeysTest.php b/tests/ArrayKeysTest.php index 659cd5af5..2f8593865 100644 --- a/tests/ArrayKeysTest.php +++ b/tests/ArrayKeysTest.php @@ -130,36 +130,36 @@ class ArrayKeysTest extends TestCase * @psalm-type TAlias = 123 */ class a {} - + /** * @psalm-import-type TAlias from a * @template TKey as array-key * @template TValue as array-key * @template T as array - * + * * @template TOrig as a|b * @template TT as class-string - * + * * @template TBool as bool */ class b { - /** - * @var array + /** + * @var array */ private array $a = [123 => 123]; - + /** @var array, int> */ public array $c = []; - + /** @var array, int> */ public array $d = []; - + /** @var array */ public array $e = []; - + /** @var array>, int> */ private array $f = [123 => 123]; - + /** @var array>, int> */ private array $g = ["test" => 123]; @@ -173,7 +173,7 @@ class ArrayKeysTest extends TestCase return $v ? ["a" => 123] : [123 => 123]; } } - + /** @var b<"testKey", "testValue", array<"testKey", "testValue">, b, class-string, true> */ $b = new b; $b->d["testKey"] = 123; @@ -183,6 +183,26 @@ class ArrayKeysTest extends TestCase //$b->e["b"] = 123; ', ], + 'intStringKeyAsInt' => [ + 'code' => ' "a"]; + $b = ["15.7" => "a"]; + // since PHP 8 this is_numeric but will not be int key + $c = ["15 " => "a"]; + $d = ["-15" => "a"]; + // see https://github.com/php/php-src/issues/9029#issuecomment-1186226676 + $e = ["+15" => "a"]; + $f = ["015" => "a"]; + ', + 'assertions' => [ + '$a===' => "array{15: 'a'}", + '$b===' => "array{'15.7': 'a'}", + '$c===' => "array{'15 ': 'a'}", + '$d===' => "array{-15: 'a'}", + '$e===' => "array{'+15': 'a'}", + '$f===' => "array{'015': 'a'}", + ], + ], ]; } From 120e3122b5c4a9875697d13dc48ffc0ac3b57533 Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Fri, 22 Mar 2024 14:24:50 +0100 Subject: [PATCH 2/5] fix tests --- src/Psalm/Type/Atomic/TKeyedArray.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Psalm/Type/Atomic/TKeyedArray.php b/src/Psalm/Type/Atomic/TKeyedArray.php index c1c29b991..98915b7df 100644 --- a/src/Psalm/Type/Atomic/TKeyedArray.php +++ b/src/Psalm/Type/Atomic/TKeyedArray.php @@ -720,6 +720,11 @@ class TKeyedArray extends Atomic $quote = true; } + // 08 should be quoted since it's numeric but it's handled as string and not cast to int + if (preg_match('/^0[0-9]+$/', $name)) { + $quote = true; + } + if ($quote) { $name = '\'' . str_replace("\n", '\n', addslashes($name)) . '\''; } From 6030995d8dd1ea9853c96bb2e96105416d31b691 Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Fri, 22 Mar 2024 15:34:10 +0100 Subject: [PATCH 3/5] separate fix of https://github.com/vimeo/psalm/pull/10481 to a reusable function --- .../Statements/Expression/ArrayAnalyzer.php | 37 +++++++++++++++++++ src/Psalm/Internal/Type/TypeParser.php | 10 +---- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php index 3c1034df2..4c6aa2cff 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php @@ -44,10 +44,15 @@ use Psalm\Type\Union; use function array_merge; use function array_values; use function count; +use function filter_var; use function in_array; +use function is_int; +use function is_numeric; use function is_string; use function preg_match; +use function trim; +use const FILTER_VALIDATE_INT; use const PHP_INT_MAX; /** @@ -237,6 +242,38 @@ final class ArrayAnalyzer return true; } + /** + * @param string|int $literal_array_key + * @return false|int + * @psalm-assert-if-false !numeric $literal_array_key + */ + public static function getLiteralArrayKeyInt( + $literal_array_key + ) { + if (is_int($literal_array_key)) { + return $literal_array_key; + } + + if (!is_numeric($literal_array_key)) { + return false; + } + + // PHP 8 values with whitespace after number are counted as numeric + // and filter_var treats them as such too + // ensures that '15 ' will stay '15 ' + if (trim($literal_array_key) !== $literal_array_key) { + return false; + } + + // '+5' will pass the filter_var check but won't be changed in keys + if ($literal_array_key[0] === '+') { + return false; + } + + // e.g. 015 is numeric but won't be typecast as it's not a valid int + return filter_var($literal_array_key, FILTER_VALIDATE_INT); + } + private static function analyzeArrayItem( StatementsAnalyzer $statements_analyzer, Context $context, diff --git a/src/Psalm/Internal/Type/TypeParser.php b/src/Psalm/Internal/Type/TypeParser.php index f79cc3cfd..30558a837 100644 --- a/src/Psalm/Internal/Type/TypeParser.php +++ b/src/Psalm/Internal/Type/TypeParser.php @@ -7,6 +7,7 @@ use LogicException; use Psalm\Codebase; use Psalm\Exception\TypeParseTreeException; use Psalm\Internal\Analyzer\ProjectAnalyzer; +use Psalm\Internal\Analyzer\Statements\Expression\ArrayAnalyzer; use Psalm\Internal\Type\ParseTree\CallableParamTree; use Psalm\Internal\Type\ParseTree\CallableTree; use Psalm\Internal\Type\ParseTree\CallableWithReturnTypeTree; @@ -86,7 +87,6 @@ use function count; use function defined; use function end; use function explode; -use function filter_var; use function get_class; use function in_array; use function is_int; @@ -100,9 +100,6 @@ use function strpos; use function strtolower; use function strtr; use function substr; -use function trim; - -use const FILTER_VALIDATE_INT; /** * @psalm-suppress InaccessibleProperty Allowed during construction @@ -669,11 +666,8 @@ final class TypeParser } foreach ($generic_params[0]->getAtomicTypes() as $key => $atomic_type) { - // PHP 8 values with whitespace after number are counted as numeric - // and filter_var treats them as such too if ($atomic_type instanceof TLiteralString - && ($string_to_int = filter_var($atomic_type->value, FILTER_VALIDATE_INT)) !== false - && trim($atomic_type->value) === $atomic_type->value + && ($string_to_int = ArrayAnalyzer::getLiteralArrayKeyInt($atomic_type->value)) !== false ) { $builder = $generic_params[0]->getBuilder(); $builder->removeType($key); From cd302f040bb5ada652590b5361c6e3e99c0c85b7 Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Fri, 22 Mar 2024 16:22:11 +0100 Subject: [PATCH 4/5] further improve string-int juggling handling which was previously already improved by me in https://github.com/vimeo/psalm/pull/10481 Also fix https://psalm.dev/r/3b401c6f88 --- .../Statements/Expression/ArrayAnalyzer.php | 13 ++---- .../Statements/Expression/AssertionFinder.php | 29 ++++++++++--- .../Assignment/ArrayAssignmentAnalyzer.php | 7 ++-- .../Expression/AssignmentAnalyzer.php | 7 ++++ .../Expression/ExpressionIdentifier.php | 13 ++++-- .../Expression/Fetch/ArrayFetchAnalyzer.php | 42 ++++++++++++------- .../Expression/SimpleTypeInferer.php | 12 ++---- src/Psalm/Internal/Type/TypeParser.php | 2 +- src/Psalm/Type/Reconciler.php | 24 ++++++++--- .../RedundantConditionTest.php | 9 ++++ 10 files changed, 107 insertions(+), 51 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php index 4c6aa2cff..b6380df0f 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php @@ -49,7 +49,6 @@ use function in_array; use function is_int; use function is_numeric; use function is_string; -use function preg_match; use function trim; use const FILTER_VALIDATE_INT; @@ -351,20 +350,17 @@ final class ArrayAnalyzer } if ($item->key instanceof PhpParser\Node\Scalar\String_ - && preg_match('/^(0|[1-9][0-9]*)$/', $item->key->value) - && ( - (int) $item->key->value < PHP_INT_MAX || - $item->key->value === (string) PHP_INT_MAX - ) + && self::getLiteralArrayKeyInt($item->key->value) !== false ) { $key_type = Type::getInt(false, (int) $item->key->value); } if ($key_type->isSingleStringLiteral()) { $item_key_literal_type = $key_type->getSingleStringLiteral(); - $item_key_value = $item_key_literal_type->value; + $string_to_int = self::getLiteralArrayKeyInt($item_key_literal_type->value); + $item_key_value = $string_to_int === false ? $item_key_literal_type->value : $string_to_int; - if ($item_key_literal_type instanceof TLiteralClassString) { + if (is_string($item_key_value) && $item_key_literal_type instanceof TLiteralClassString) { $array_creation_info->class_strings[$item_key_value] = true; } } elseif ($key_type->isSingleIntLiteral()) { @@ -421,7 +417,6 @@ final class ArrayAnalyzer $array_creation_info->array_keys[$item_key_value] = true; } - if (($data_flow_graph = $statements_analyzer->data_flow_graph) && ($data_flow_graph instanceof VariableUseGraph || !in_array('TaintedInput', $statements_analyzer->getSuppressedIssues())) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php b/src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php index 4cf8e778e..c3a169ce9 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php @@ -3742,7 +3742,7 @@ final class AssertionFinder if (isset($expr->getArgs()[0]) && isset($expr->getArgs()[1]) && $first_var_type - && $first_var_name + && $first_var_name !== null && !$expr->getArgs()[0]->value instanceof PhpParser\Node\Expr\ClassConstFetch && $source instanceof StatementsAnalyzer && ($second_var_type = $source->node_data->getType($expr->getArgs()[1]->value)) @@ -3765,7 +3765,12 @@ final class AssertionFinder if ($key_type->allStringLiterals() && !$key_type->possibly_undefined) { foreach ($key_type->getLiteralStrings() as $array_literal_type) { - $literal_assertions[] = new IsIdentical($array_literal_type); + $string_to_int = ArrayAnalyzer::getLiteralArrayKeyInt($array_literal_type->value); + if ($string_to_int === false) { + $literal_assertions[] = new IsIdentical($array_literal_type); + } else { + $literal_assertions[] = new IsLooselyEqual(new TLiteralInt($string_to_int)); + } } } elseif ($key_type->allIntLiterals() && !$key_type->possibly_undefined) { foreach ($key_type->getLiteralInts() as $array_literal_type) { @@ -3778,7 +3783,7 @@ final class AssertionFinder } } - if ($literal_assertions && $first_var_name && $safe_to_track_literals) { + if ($literal_assertions && $first_var_name !== null && $safe_to_track_literals) { $if_types[$first_var_name] = [$literal_assertions]; } else { $array_root = isset($expr->getArgs()[1]->value) @@ -3794,7 +3799,10 @@ final class AssertionFinder $first_arg = $expr->getArgs()[0]; if ($first_arg->value instanceof PhpParser\Node\Scalar\String_) { - $first_var_name = '\'' . $first_arg->value->value . '\''; + $string_to_int = ArrayAnalyzer::getLiteralArrayKeyInt($first_arg->value->value); + $first_var_name = $string_to_int === false + ? '\'' . $first_arg->value->value . '\'' + : (string) $string_to_int; } elseif ($first_arg->value instanceof PhpParser\Node\Scalar\LNumber) { $first_var_name = (string)$first_arg->value->value; } @@ -3812,7 +3820,12 @@ final class AssertionFinder if ($const_type) { if ($const_type->isSingleStringLiteral()) { - $first_var_name = '\''.$const_type->getSingleStringLiteral()->value.'\''; + $string_to_int = ArrayAnalyzer::getLiteralArrayKeyInt( + $const_type->getSingleStringLiteral()->value, + ); + $first_var_name = $string_to_int === false + ? '\'' . $const_type->getSingleStringLiteral()->value . '\'' + : (string) $string_to_int; } elseif ($const_type->isSingleIntLiteral()) { $first_var_name = (string)$const_type->getSingleIntLiteral()->value; } else { @@ -3829,7 +3842,11 @@ final class AssertionFinder && ($first_var_type = $source->node_data->getType($expr->getArgs()[0]->value)) ) { foreach ($first_var_type->getLiteralStrings() as $array_literal_type) { - $if_types[$array_root . "['" . $array_literal_type->value . "']"] = [[new ArrayKeyExists()]]; + $string_to_int = ArrayAnalyzer::getLiteralArrayKeyInt($array_literal_type->value); + $literal_key = $string_to_int === false + ? "'" . $array_literal_type->value . "'" + : $string_to_int; + $if_types[$array_root . "[" . $literal_key . "]"] = [[new ArrayKeyExists()]]; } foreach ($first_var_type->getLiteralInts() as $array_literal_type) { $if_types[$array_root . "[" . $array_literal_type->value . "]"] = [[new ArrayKeyExists()]]; diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/ArrayAssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/ArrayAssignmentAnalyzer.php index 01150233a..9347390fa 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/ArrayAssignmentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/ArrayAssignmentAnalyzer.php @@ -9,6 +9,7 @@ use Psalm\CodeLocation; use Psalm\Codebase; use Psalm\Context; use Psalm\Internal\Analyzer\ClassLikeAnalyzer; +use Psalm\Internal\Analyzer\Statements\Expression\ArrayAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\ExpressionIdentifier; use Psalm\Internal\Analyzer\Statements\Expression\Fetch\ArrayFetchAnalyzer; use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer; @@ -48,7 +49,6 @@ use function end; use function implode; use function in_array; use function is_string; -use function preg_match; use function strlen; use function strpos; @@ -1084,8 +1084,9 @@ final class ArrayAssignmentAnalyzer $offset_type = $child_stmt_dim_type->getSingleStringLiteral(); } - if (preg_match('/^(0|[1-9][0-9]*)$/', $offset_type->value)) { - $var_id_addition = '[' . $offset_type->value . ']'; + $string_to_int = ArrayAnalyzer::getLiteralArrayKeyInt($offset_type->value); + if ($string_to_int !== false) { + $var_id_addition = '[' . $string_to_int . ']'; } else { $var_id_addition = '[\'' . $offset_type->value . '\']'; } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php index 904d801fb..679abaa34 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php @@ -1217,6 +1217,13 @@ final class AssignmentAnalyzer $offset_value = $assign_var_item->key->value; } + if ($offset_value !== null) { + $string_to_int = ArrayAnalyzer::getLiteralArrayKeyInt($offset_value); + if ($string_to_int !== false) { + $offset_value = $string_to_int; + } + } + $list_var_id = ExpressionIdentifier::getExtendedVarId( $var, $statements_analyzer->getFQCLN(), diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/ExpressionIdentifier.php b/src/Psalm/Internal/Analyzer/Statements/Expression/ExpressionIdentifier.php index 00a249ec8..634d49bca 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/ExpressionIdentifier.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/ExpressionIdentifier.php @@ -116,9 +116,10 @@ final class ExpressionIdentifier if ($stmt->dim instanceof PhpParser\Node\Scalar\String_ || $stmt->dim instanceof PhpParser\Node\Scalar\LNumber ) { - $offset = $stmt->dim instanceof PhpParser\Node\Scalar\String_ + $string_to_int = ArrayAnalyzer::getLiteralArrayKeyInt($stmt->dim->value); + $offset = $string_to_int === false ? '\'' . $stmt->dim->value . '\'' - : $stmt->dim->value; + : (int) $stmt->dim->value; } elseif ($stmt->dim instanceof PhpParser\Node\Expr\Variable && is_string($stmt->dim->name) ) { @@ -146,7 +147,13 @@ final class ExpressionIdentifier ) ) { if ($stmt_dim_type->isSingleStringLiteral()) { - $offset = '\'' . $stmt_dim_type->getSingleStringLiteral()->value . '\''; + $string_to_int = ArrayAnalyzer::getLiteralArrayKeyInt( + $stmt_dim_type->getSingleStringLiteral()->value, + ); + + $offset = $string_to_int === false + ? '\'' . $stmt_dim_type->getSingleStringLiteral()->value . '\'' + : (int) $stmt_dim_type->getSingleStringLiteral()->value; } elseif ($stmt_dim_type->isSingleIntLiteral()) { $offset = $stmt_dim_type->getSingleIntLiteral()->value; } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php index 34ed8f510..9bc860de4 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php @@ -7,6 +7,7 @@ use Psalm\CodeLocation; use Psalm\Codebase; use Psalm\Context; use Psalm\Internal\Analyzer\FunctionLikeAnalyzer; +use Psalm\Internal\Analyzer\Statements\Expression\ArrayAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\Call\MethodCallAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\ExpressionIdentifier; use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer; @@ -92,8 +93,6 @@ use function count; use function implode; use function in_array; use function is_int; -use function is_numeric; -use function preg_match; use function strlen; use function strtolower; @@ -968,16 +967,25 @@ final class ArrayFetchAnalyzer $found_match = false; foreach ($offset_type->getAtomicTypes() as $offset_type_part) { - if ($extended_var_id - && $offset_type_part instanceof TLiteralString - && isset( - $context->vars_in_scope[ - $extended_var_id . '[\'' . $offset_type_part->value . '\']' - ], - ) - && !$context->vars_in_scope[ - $extended_var_id . '[\'' . $offset_type_part->value . '\']' - ]->possibly_undefined + if ($extended_var_id === null + || !($offset_type_part instanceof TLiteralString)) { + continue; + } + + $string_to_int = ArrayAnalyzer::getLiteralArrayKeyInt( + $offset_type_part->value, + ); + + $literal_access = $string_to_int === false + ? '\'' . $offset_type_part->value . '\'' + : $string_to_int; + if (isset( + $context->vars_in_scope[ + $extended_var_id . '[' . $literal_access . ']' + ], + ) && !$context->vars_in_scope[ + $extended_var_id . '[' . $literal_access . ']' + ]->possibly_undefined ) { $found_match = true; break; @@ -1007,8 +1015,9 @@ final class ArrayFetchAnalyzer foreach ($offset_types as $key => $offset_type_part) { if ($offset_type_part instanceof TLiteralString) { - if (preg_match('/^(0|[1-9][0-9]*)$/', $offset_type_part->value)) { - $offset_type->addType(new TLiteralInt((int) $offset_type_part->value)); + $string_to_int = ArrayAnalyzer::getLiteralArrayKeyInt($offset_type_part->value); + if ($string_to_int !== false) { + $offset_type->addType(new TLiteralInt($string_to_int)); $offset_type->removeType($key); } } elseif ($offset_type_part instanceof TBool) { @@ -1546,7 +1555,10 @@ final class ArrayFetchAnalyzer if ($key_values) { $properties = $type->properties; foreach ($key_values as $key_value) { - if ($type->is_list && (!is_numeric($key_value->value) || $key_value->value < 0)) { + $string_to_int = ArrayAnalyzer::getLiteralArrayKeyInt($key_value->value); + $key_value = $string_to_int === false ? $key_value : new TLiteralInt($string_to_int); + + if ($type->is_list && (!is_int($key_value->value) || $key_value->value < 0)) { $expected_offset_types[] = $type->getGenericKeyType(); $has_valid_offset = false; } elseif ((isset($properties[$key_value->value]) && !( diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/SimpleTypeInferer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/SimpleTypeInferer.php index 8b39f7987..1b0b183ae 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/SimpleTypeInferer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/SimpleTypeInferer.php @@ -36,7 +36,6 @@ use function array_merge; use function array_values; use function count; use function is_string; -use function preg_match; use function strtolower; use const PHP_INT_MAX; @@ -671,11 +670,7 @@ final class SimpleTypeInferer $key_type = Type::getString(''); } if ($item->key instanceof PhpParser\Node\Scalar\String_ - && preg_match('/^(0|[1-9][0-9]*)$/', $item->key->value) - && ( - (int) $item->key->value < PHP_INT_MAX || - $item->key->value === (string) PHP_INT_MAX - ) + && ArrayAnalyzer::getLiteralArrayKeyInt($item->key->value) !== false ) { $key_type = Type::getInt(false, (int) $item->key->value); } @@ -687,9 +682,10 @@ final class SimpleTypeInferer if ($key_type->isSingleStringLiteral()) { $item_key_literal_type = $key_type->getSingleStringLiteral(); - $item_key_value = $item_key_literal_type->value; + $string_to_int = ArrayAnalyzer::getLiteralArrayKeyInt($item_key_literal_type->value); + $item_key_value = $string_to_int === false ? $item_key_literal_type->value : $string_to_int; - if ($item_key_literal_type instanceof TLiteralClassString) { + if (is_string($item_key_value) && $item_key_literal_type instanceof TLiteralClassString) { $array_creation_info->class_strings[$item_key_value] = true; } } elseif ($key_type->isSingleIntLiteral()) { diff --git a/src/Psalm/Internal/Type/TypeParser.php b/src/Psalm/Internal/Type/TypeParser.php index 30558a837..5a02c1fb1 100644 --- a/src/Psalm/Internal/Type/TypeParser.php +++ b/src/Psalm/Internal/Type/TypeParser.php @@ -1475,7 +1475,7 @@ final class TypeParser $property_key = $property_branch->value; } if ($is_list && ( - !is_numeric($property_key) + ArrayAnalyzer::getLiteralArrayKeyInt($property_key) === false || ($had_optional && !$property_maybe_undefined) || $type === 'array' || $type === 'callable-array' diff --git a/src/Psalm/Type/Reconciler.php b/src/Psalm/Type/Reconciler.php index 744959d01..b520ab9d5 100644 --- a/src/Psalm/Type/Reconciler.php +++ b/src/Psalm/Type/Reconciler.php @@ -5,6 +5,7 @@ namespace Psalm\Type; use InvalidArgumentException; use Psalm\CodeLocation; use Psalm\Codebase; +use Psalm\Internal\Analyzer\Statements\Expression\ArrayAnalyzer; use Psalm\Internal\Analyzer\StatementsAnalyzer; use Psalm\Internal\Codebase\TaintFlowGraph; use Psalm\Internal\Codebase\VariableUseGraph; @@ -67,11 +68,11 @@ use function count; use function explode; use function implode; use function is_numeric; +use function is_string; use function key; use function ksort; use function preg_match; use function preg_quote; -use function str_replace; use function str_split; use function strlen; use function strpos; @@ -470,9 +471,15 @@ class Reconciler $array_key = array_shift($key_parts); array_shift($key_parts); + if ($array_key[0] === '\'' || $array_key[0] === '"') { + $possibly_property_key = substr($array_key, 1, -1); + $string_to_int = ArrayAnalyzer::getLiteralArrayKeyInt($possibly_property_key); + $array_key = $string_to_int === false ? $array_key : $string_to_int; + } + $new_base_key = $base_key . '[' . $array_key . ']'; - if (strpos($array_key, '\'') !== false) { + if (is_string($array_key) && strpos($array_key, '\'') !== false) { $new_types[$base_key][] = [new HasStringArrayAccess()]; } else { $new_types[$base_key][] = [new HasIntOrStringArrayAccess()]; @@ -781,7 +788,8 @@ class Reconciler return null; } elseif (!$existing_key_type_part instanceof TKeyedArray) { return Type::getMixed(); - } elseif ($array_key[0] === '$' || ($array_key[0] !== '\'' && !is_numeric($array_key[0]))) { + } elseif ($array_key[0] === '$' + || ($array_key[0] !== '\'' && ArrayAnalyzer::getLiteralArrayKeyInt($array_key) === false)) { if ($has_empty) { return null; } @@ -790,7 +798,10 @@ class Reconciler } else { $array_properties = $existing_key_type_part->properties; - $key_parts_key = str_replace('\'', '', $array_key); + $key_parts_key = $array_key; + if ($array_key[0] === '\'' || $array_key[0] === '"') { + $key_parts_key = substr($array_key, 1, -1); + } if (!isset($array_properties[$key_parts_key])) { if ($existing_key_type_part->fallback_params !== null) { @@ -1182,13 +1193,14 @@ class Reconciler $properties = $base_atomic_type->properties; $properties[$array_key_offset] = $result_type; if ($base_atomic_type->is_list - && (!is_numeric($array_key_offset) + && (ArrayAnalyzer::getLiteralArrayKeyInt($array_key_offset) === false || ($array_key_offset && !isset($properties[$array_key_offset-1]) ) ) ) { - if ($base_atomic_type->fallback_params && is_numeric($array_key_offset)) { + if ($base_atomic_type->fallback_params + && ArrayAnalyzer::getLiteralArrayKeyInt($array_key_offset) !== false) { $fallback = $base_atomic_type->fallback_params[1]->setPossiblyUndefined( $result_type->isNever(), ); diff --git a/tests/TypeReconciliation/RedundantConditionTest.php b/tests/TypeReconciliation/RedundantConditionTest.php index e54aa2bd3..289bc2073 100644 --- a/tests/TypeReconciliation/RedundantConditionTest.php +++ b/tests/TypeReconciliation/RedundantConditionTest.php @@ -1576,6 +1576,15 @@ class RedundantConditionTest extends TestCase }', 'error_message' => 'DocblockTypeContradiction', ], + 'array_key_exists_int_string_juggle' => [ + 'code' => ' 'RedundantCondition', + ], ]; } } From 1bfa684d7f805f20c9c474ab0bb59994a572d09e Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Sat, 23 Mar 2024 01:01:51 +0100 Subject: [PATCH 5/5] scientific and underscore notation should be quoted too, since they won't be type juggled --- src/Psalm/Type/Atomic/TKeyedArray.php | 8 ++++++++ tests/ArrayKeysTest.php | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/src/Psalm/Type/Atomic/TKeyedArray.php b/src/Psalm/Type/Atomic/TKeyedArray.php index 98915b7df..848ba5f6d 100644 --- a/src/Psalm/Type/Atomic/TKeyedArray.php +++ b/src/Psalm/Type/Atomic/TKeyedArray.php @@ -720,11 +720,19 @@ class TKeyedArray extends Atomic $quote = true; } + if (preg_match('/^[1-9][0-9]*_([0-9]+_)*[0-9]+$/', $name)) { + $quote = true; + } + // 08 should be quoted since it's numeric but it's handled as string and not cast to int if (preg_match('/^0[0-9]+$/', $name)) { $quote = true; } + if (preg_match('/^[0-9]+e-?[0-9]+$/', $name)) { + $quote = true; + } + if ($quote) { $name = '\'' . str_replace("\n", '\n', addslashes($name)) . '\''; } diff --git a/tests/ArrayKeysTest.php b/tests/ArrayKeysTest.php index 2f8593865..0f5087330 100644 --- a/tests/ArrayKeysTest.php +++ b/tests/ArrayKeysTest.php @@ -193,6 +193,8 @@ class ArrayKeysTest extends TestCase // see https://github.com/php/php-src/issues/9029#issuecomment-1186226676 $e = ["+15" => "a"]; $f = ["015" => "a"]; + $g = ["1e2" => "a"]; + $h = ["1_0" => "a"]; ', 'assertions' => [ '$a===' => "array{15: 'a'}", @@ -201,6 +203,8 @@ class ArrayKeysTest extends TestCase '$d===' => "array{-15: 'a'}", '$e===' => "array{'+15': 'a'}", '$f===' => "array{'015': 'a'}", + '$g===' => "array{'1e2': 'a'}", + '$h===' => "array{'1_0': 'a'}", ], ], ];