Skip to content

Commit

Permalink
Merge pull request #10814 from kkmuffme/int-string-keys-arrays-are-int
Browse files Browse the repository at this point in the history
  • Loading branch information
@weirdan
weirdan committed Mar 23, 2024
2 parents 63ea4de + 1bfa684 commit 85ff673
Show file tree
Hide file tree
Showing 12 changed files with 194 additions and 70 deletions.
50 changes: 41 additions & 9 deletions src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,14 @@
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;

/**
Expand Down Expand Up @@ -237,6 +241,38 @@ public static function analyze(
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,
Expand Down Expand Up @@ -314,20 +350,17 @@ private static function analyzeArrayItem(
}

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()) {
Expand Down Expand Up @@ -384,7 +417,6 @@ private static function analyzeArrayItem(
$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()))
Expand Down
29 changes: 23 additions & 6 deletions src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3742,7 +3742,7 @@ private static function getArrayKeyExistsAssertions(
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))
Expand All @@ -3765,7 +3765,12 @@ private static function getArrayKeyExistsAssertions(

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) {
Expand All @@ -3778,7 +3783,7 @@ private static function getArrayKeyExistsAssertions(
}
}

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)
Expand All @@ -3794,7 +3799,10 @@ private static function getArrayKeyExistsAssertions(
$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;
}
Expand All @@ -3812,7 +3820,12 @@ private static function getArrayKeyExistsAssertions(

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 {
Expand All @@ -3829,7 +3842,11 @@ private static function getArrayKeyExistsAssertions(
&& ($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()]];
Expand Down
7 changes: 4 additions & 3 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/ArrayAssignmentAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
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;
Expand Down Expand Up @@ -48,7 +49,6 @@
use function implode;
use function in_array;
use function is_string;
use function preg_match;
use function strlen;
use function strpos;

Expand Down Expand Up @@ -1084,8 +1084,9 @@ private static function getArrayAssignmentOffsetType(
$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 . '\']';
}
Expand Down
7 changes: 7 additions & 0 deletions src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1217,6 +1217,13 @@ private static function analyzeDestructuringAssignment(
$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(),
Expand Down
13 changes: 10 additions & 3 deletions src/Psalm/Internal/Analyzer/Statements/Expression/ExpressionIdentifier.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,10 @@ public static function getExtendedVarId(
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)
) {
Expand Down Expand Up @@ -146,7 +147,13 @@ public static function getExtendedVarId(
)
) {
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;
}
Expand Down
42 changes: 27 additions & 15 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
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;
Expand Down Expand Up @@ -92,8 +93,6 @@
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;

Expand Down Expand Up @@ -968,16 +967,25 @@ private static function checkLiteralStringArrayOffset(
$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;
Expand Down Expand Up @@ -1007,8 +1015,9 @@ public static function replaceOffsetTypeWithInts(Union $offset_type): Union

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) {
Expand Down Expand Up @@ -1546,7 +1555,10 @@ private static function handleArrayAccessOnKeyedArray(
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]) && !(
Expand Down
12 changes: 4 additions & 8 deletions src/Psalm/Internal/Analyzer/Statements/Expression/SimpleTypeInferer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
use function array_values;
use function count;
use function is_string;
use function preg_match;
use function strtolower;

use const PHP_INT_MAX;
Expand Down Expand Up @@ -671,11 +670,7 @@ private static function handleArrayItem(
$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);
}
Expand All @@ -687,9 +682,10 @@ private static function handleArrayItem(

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()) {
Expand Down
12 changes: 3 additions & 9 deletions src/Psalm/Internal/Type/TypeParser.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
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;
Expand Down Expand Up @@ -86,7 +87,6 @@
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;
Expand All @@ -100,9 +100,6 @@
use function strtolower;
use function strtr;
use function substr;
use function trim;

use const FILTER_VALIDATE_INT;

/**
* @psalm-suppress InaccessibleProperty Allowed during construction
Expand Down Expand Up @@ -669,11 +666,8 @@ private static function getTypeFromGenericTree(
}

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);
Expand Down Expand Up @@ -1481,7 +1475,7 @@ private static function getTypeFromKeyedArrayTree(
$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'
Expand Down

0 comments on commit 85ff673

Please sign in to comment.