Skip to content

Commit

Permalink
further improve string-int juggling handling which was previously alr…
Browse files Browse the repository at this point in the history 
…eady improved by me in #10481

Also fix https://psalm.dev/r/3b401c6f88
  • Loading branch information
@kkmuffme
kkmuffme committed Mar 22, 2024
1 parent 6030995 commit cd302f0
Show file tree
Hide file tree
Showing 10 changed files with 107 additions and 51 deletions.
13 changes: 4 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 @@ -49,7 +49,6 @@
use function is_int;
use function is_numeric;
use function is_string;
use function preg_match;
use function trim;

use const FILTER_VALIDATE_INT;
Expand Down Expand Up @@ -351,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 @@ -421,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
2 changes: 1 addition & 1 deletion src/Psalm/Internal/Type/TypeParser.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1475,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
24 changes: 18 additions & 6 deletions src/Psalm/Type/Reconciler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
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;
Expand Down Expand Up @@ -67,11 +68,11 @@
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;
Expand Down Expand Up @@ -470,9 +471,15 @@ private static function addNestedAssertions(array $new_types, array $existing_ty
$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()];
Expand Down Expand Up @@ -781,7 +788,8 @@ private static function getValueForKey(
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;
}
Expand All @@ -790,7 +798,10 @@ private static function getValueForKey(
} 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) {
Expand Down Expand Up @@ -1182,13 +1193,14 @@ private static function adjustTKeyedArrayType(
$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(),
);
Expand Down

0 comments on commit cd302f0

Please sign in to comment.