Skip to content

Commit

Permalink
Merge pull request #10439 from nicelocal/fix_literal_union_key
Browse files Browse the repository at this point in the history 
Use keyed arrays when assigning literal union keys & assertion fixes
  • Loading branch information
@orklah
orklah committed Dec 3, 2023
2 parents 1cca558 + 390df68 commit c620f6e
Show file tree
Hide file tree
Showing 10 changed files with 204 additions and 113 deletions.
11 changes: 9 additions & 2 deletions psalm-baseline.xml
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="UTF-8"?>
<files psalm-version="5.x-dev@18a6c0b6e9aade82a2f3cc36e3a644ba70eaf539">
<files psalm-version="5.x-dev@5acde045f126440ded206b406cf37b649ede84fc">
<file src="examples/TemplateChecker.php">
<PossiblyUndefinedIntArrayOffset>
<code><![CDATA[$comment_block->tags['variablesfrom'][0]]]></code>
Expand Down Expand Up @@ -216,20 +216,27 @@
</PossiblyUndefinedIntArrayOffset>
</file>
<file src="src/Psalm/Internal/Analyzer/Statements/UnusedAssignmentRemover.php">
<PossiblyUndefinedArrayOffset>
<code>$token_list[$iter]</code>
</PossiblyUndefinedArrayOffset>
<PossiblyUndefinedIntArrayOffset>
<code>$token_list[$iter]</code>
<code>$token_list[$iter]</code>
<code>$token_list[$iter]</code>
<code>$token_list[$iter]</code>
<code>$token_list[0]</code>
<code>$token_list[1]</code>
</PossiblyUndefinedIntArrayOffset>
</file>
<file src="src/Psalm/Internal/Analyzer/StatementsAnalyzer.php">
<PossiblyUndefinedArrayOffset>
<code><![CDATA[$stmt->expr->getArgs()[0]]]></code>
</PossiblyUndefinedArrayOffset>
</file>
<file src="src/Psalm/Internal/Cli/Psalm.php">
<PossiblyInvalidArgument>
<code><![CDATA[$options['f'] ?? null]]></code>
</PossiblyInvalidArgument>
</file>
<file src="src/Psalm/Internal/Cli/Refactor.php">
<PossiblyUndefinedIntArrayOffset>
<code>$identifier_name</code>
Expand Down
17 changes: 0 additions & 17 deletions src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,11 @@
use function array_keys;
use function array_merge;
use function array_reduce;
use function array_unique;
use function count;
use function in_array;
use function preg_match;
use function preg_quote;
use function spl_object_id;
use function strpos;
use function substr;

/**
* @internal
Expand Down Expand Up @@ -272,20 +269,6 @@ public static function analyze(
array_keys($if_scope->negated_types),
);

$extra_vars_to_update = [];

// if there's an object-like array in there, we also need to update the root array variable
foreach ($vars_to_update as $var_id) {
$bracked_pos = strpos($var_id, '[');
if ($bracked_pos !== false) {
$extra_vars_to_update[] = substr($var_id, 0, $bracked_pos);
}
}

if ($extra_vars_to_update) {
$vars_to_update = array_unique(array_merge($extra_vars_to_update, $vars_to_update));
}

$outer_context->update(
$old_if_context,
$if_context,
Expand Down
38 changes: 16 additions & 22 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/ArrayAssignmentAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -349,29 +349,23 @@ private static function updateTypeWithKeyValues(
}

if (!$has_matching_objectlike_property && !$has_matching_string) {
if (count($key_values) === 1) {
$key_value = $key_values[0];

$object_like = new TKeyedArray(
[$key_value->value => $current_type],
$key_value instanceof TLiteralClassString
? [$key_value->value => true]
: null,
);

$array_assignment_type = new Union([
$object_like,
]);
} else {
$array_assignment_literals = $key_values;

$array_assignment_type = new Union([
new TNonEmptyArray([
new Union($array_assignment_literals),
$current_type,
]),
]);
$properties = [];
$classStrings = [];
$current_type = $current_type->setPossiblyUndefined(count($key_values) > 1);
foreach ($key_values as $key_value) {
$properties[$key_value->value] = $current_type;
if ($key_value instanceof TLiteralClassString) {
$classStrings[$key_value->value] = true;
}
}
$object_like = new TKeyedArray(
$properties,
$classStrings ?: null,
);

$array_assignment_type = new Union([
$object_like,
]);

return Type::combineUnionTypes(
$child_stmt_type,
Expand Down
8 changes: 3 additions & 5 deletions src/Psalm/Internal/Cli/LanguageServer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,11 +315,9 @@ static function (string $arg) use ($valid_long_options): void {

$path_to_config = CliUtils::getPathToConfig($options);

if (isset($options['tcp'])) {
if (!is_string($options['tcp'])) {
fwrite(STDERR, 'tcp url should be a string' . PHP_EOL);
exit(1);
}
if (isset($options['tcp']) && !is_string($options['tcp'])) {
fwrite(STDERR, 'tcp url should be a string' . PHP_EOL);
exit(1);
}

$config = CliUtils::initializeConfig(
Expand Down
141 changes: 82 additions & 59 deletions src/Psalm/Type/Reconciler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use Psalm\Issue\TypeDoesNotContainType;
use Psalm\IssueBuffer;
use Psalm\Storage\Assertion;
use Psalm\Storage\Assertion\ArrayKeyDoesNotExist;
use Psalm\Storage\Assertion\ArrayKeyExists;
use Psalm\Storage\Assertion\Empty_;
use Psalm\Storage\Assertion\Falsy;
Expand All @@ -45,6 +46,8 @@
use Psalm\Type\Atomic\TInt;
use Psalm\Type\Atomic\TKeyedArray;
use Psalm\Type\Atomic\TList;
use Psalm\Type\Atomic\TLiteralInt;
use Psalm\Type\Atomic\TLiteralString;
use Psalm\Type\Atomic\TMixed;
use Psalm\Type\Atomic\TNamedObject;
use Psalm\Type\Atomic\TNever;
Expand Down Expand Up @@ -197,7 +200,9 @@ public static function reconcileKeyedTypes(
$is_equality = $is_equality
&& $new_type_part_part instanceof IsIdentical;

$has_inverted_isset = $has_inverted_isset || $new_type_part_part instanceof IsNotIsset;
$has_inverted_isset = $has_inverted_isset
|| $new_type_part_part instanceof IsNotIsset
|| $new_type_part_part instanceof ArrayKeyDoesNotExist;

$has_count_check = $has_count_check
|| $new_type_part_part instanceof NonEmptyCountable;
Expand Down Expand Up @@ -1105,88 +1110,106 @@ private static function adjustTKeyedArrayType(
throw new UnexpectedValueException('Not expecting null array key');
}

$array_key_offsets = [];
if ($array_key[0] === '$') {
return;
if (!isset($existing_types[$array_key])) {
return;
}
$t = $existing_types[$array_key];
foreach ($t->getAtomicTypes() as $lit) {
if ($lit instanceof TLiteralInt || $lit instanceof TLiteralString) {
$array_key_offsets []= $lit->value;
continue;
}
return;
}
} else {
$array_key_offsets []= $array_key[0] === '\'' || $array_key[0] === '"'
? substr($array_key, 1, -1)
: $array_key
;
}

$array_key_offset = $array_key[0] === '\'' || $array_key[0] === '"' ? substr($array_key, 1, -1) : $array_key;

$base_key = implode($key_parts);

if (isset($existing_types[$base_key]) && $array_key_offset !== false) {
foreach ($existing_types[$base_key]->getAtomicTypes() as $base_atomic_type) {
if ($base_atomic_type instanceof TList) {
$base_atomic_type = $base_atomic_type->getKeyedArray();
}
if ($base_atomic_type instanceof TKeyedArray
$result_type = $result_type->setPossiblyUndefined(count($array_key_offsets) > 1);

foreach ($array_key_offsets as $array_key_offset) {
if (isset($existing_types[$base_key]) && $array_key_offset !== false) {
foreach ($existing_types[$base_key]->getAtomicTypes() as $base_atomic_type) {
if ($base_atomic_type instanceof TList) {
$base_atomic_type = $base_atomic_type->getKeyedArray();
}
if ($base_atomic_type instanceof TKeyedArray
|| ($base_atomic_type instanceof TArray
&& !$base_atomic_type->isEmptyArray())
|| $base_atomic_type instanceof TClassStringMap
) {
$new_base_type = $existing_types[$base_key];
) {
$new_base_type = $existing_types[$base_key];

if ($base_atomic_type instanceof TArray) {
$fallback_key_type = $base_atomic_type->type_params[0];
$fallback_value_type = $base_atomic_type->type_params[1];
if ($base_atomic_type instanceof TArray) {
$fallback_key_type = $base_atomic_type->type_params[0];
$fallback_value_type = $base_atomic_type->type_params[1];

$base_atomic_type = new TKeyedArray(
[
$base_atomic_type = new TKeyedArray(
[
$array_key_offset => $result_type,
],
null,
$fallback_key_type->isNever() ? null : [$fallback_key_type, $fallback_value_type],
);
} elseif ($base_atomic_type instanceof TClassStringMap) {
// do nothing
} else {
$properties = $base_atomic_type->properties;
$properties[$array_key_offset] = $result_type;
if ($base_atomic_type->is_list
],
null,
$fallback_key_type->isNever() ? null : [$fallback_key_type, $fallback_value_type],
);
} elseif ($base_atomic_type instanceof TClassStringMap) {
// do nothing
} else {
$properties = $base_atomic_type->properties;
$properties[$array_key_offset] = $result_type;
if ($base_atomic_type->is_list
&& (!is_numeric($array_key_offset)
|| ($array_key_offset
&& !isset($properties[$array_key_offset-1])
)
)
) {
if ($base_atomic_type->fallback_params && is_numeric($array_key_offset)) {
$fallback = $base_atomic_type->fallback_params[1]->setPossiblyUndefined(
$result_type->isNever(),
);
for ($x = 0; $x < $array_key_offset; $x++) {
$properties[$x] ??= $fallback;
) {
if ($base_atomic_type->fallback_params && is_numeric($array_key_offset)) {
$fallback = $base_atomic_type->fallback_params[1]->setPossiblyUndefined(
$result_type->isNever(),
);
for ($x = 0; $x < $array_key_offset; $x++) {
$properties[$x] ??= $fallback;
}
ksort($properties);
$base_atomic_type = $base_atomic_type->setProperties($properties);
} else {
// This should actually be a paradox
$base_atomic_type = new TKeyedArray(
$properties,
null,
$base_atomic_type->fallback_params,
false,
$base_atomic_type->from_docblock,
);
}
ksort($properties);
$base_atomic_type = $base_atomic_type->setProperties($properties);
} else {
// This should actually be a paradox
$base_atomic_type = new TKeyedArray(
$properties,
null,
$base_atomic_type->fallback_params,
false,
$base_atomic_type->from_docblock,
);
$base_atomic_type = $base_atomic_type->setProperties($properties);
}
} else {
$base_atomic_type = $base_atomic_type->setProperties($properties);
}
}

$new_base_type = $new_base_type->getBuilder()->addType($base_atomic_type)->freeze();
$new_base_type = $new_base_type->getBuilder()->addType($base_atomic_type)->freeze();

$changed_var_ids[$base_key . '[' . $array_key . ']'] = true;
$changed_var_ids[$base_key . '[' . $array_key . ']'] = true;

if ($key_parts[count($key_parts) - 1] === ']') {
self::adjustTKeyedArrayType(
$key_parts,
$existing_types,
$changed_var_ids,
$new_base_type,
);
}
if ($key_parts[count($key_parts) - 1] === ']') {
self::adjustTKeyedArrayType(
$key_parts,
$existing_types,
$changed_var_ids,
$new_base_type,
);
}

$existing_types[$base_key] = $new_base_type;
break;
$existing_types[$base_key] = $new_base_type;
break;
}
}
}
}
Expand Down
24 changes: 23 additions & 1 deletion tests/ArrayAssignmentTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,27 @@ public function testConditionalAssignment(): void
public function providerValidCodeParse(): iterable
{
return [
'assignUnionOfLiterals' => [
'code' => '<?php
$result = [];
foreach (["a", "b"] as $k) {
$result[$k] = true;
}
$resultOpt = [];
foreach (["a", "b"] as $k) {
if (random_int(0, 1)) {
continue;
}
$resultOpt[$k] = true;
}',
'assertions' => [
'$result===' => 'array{a: true, b: true}',
'$resultOpt===' => 'array{a?: true, b?: true}',
],
],
'genericArrayCreationWithSingleIntValue' => [
'code' => '<?php
$out = [];
Expand Down Expand Up @@ -192,7 +213,7 @@ class B {}
'assertions' => [
'$foo' => 'array{0: string, 1: string, 2: string}',
'$bar' => 'list{int, int, int}',
'$bat' => 'non-empty-array<string, int>',
'$bat' => 'array{a: int, b: int, c: int}',
],
],
'implicitStringArrayCreation' => [
Expand Down Expand Up @@ -979,6 +1000,7 @@ function updateArray(array $arr) : array {
$a = [];
foreach (["one", "two", "three"] as $key) {
$a[$key] ??= 0;
$a[$key] += rand(0, 10);
}
Expand Down
4 changes: 1 addition & 3 deletions tests/Loop/ForeachTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1027,9 +1027,7 @@ function foo() : void {
$arr = [];
foreach ([1, 2, 3] as $i) {
if (!isset($arr[$i]["a"])) {
$arr[$i]["a"] = 0;
}
$arr[$i]["a"] ??= 0;
$arr[$i]["a"] += 5;
}
Expand Down

0 comments on commit c620f6e

Please sign in to comment.