Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Prevent array{a: Foo} going cleanly into array<Foo> #8691

Merged
merged 5 commits into from Nov 10, 2022
Merged

Prevent array{a: Foo} going cleanly into array<Foo> #8691

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
344cca1
Prevent array{a: Foo} going cleanly into array<Foo>
muglug Nov 10, 2022
bfe6651
Add test for new behaviour
muglug Nov 10, 2022
bb991b5
Fix code style issues
muglug Nov 10, 2022
bfff411
Allow unions to be cloned again
muglug Nov 10, 2022
2579554
Simplify params properties
muglug Nov 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions phpcs.xml
View file
Open in desktop
Expand Up @@ -27,6 +27,14 @@
<exclude-pattern>src/Psalm/Internal/Stubs/</exclude-pattern>
<exclude-pattern>tests/fixtures/</exclude-pattern>

<!-- temporarily bypassing these -->
<exclude-pattern>src/Psalm/Internal/Codebase/Analyzer.php</exclude-pattern>
<exclude-pattern>src/Psalm/Internal/Codebase/TaintFlowGraph.php</exclude-pattern>
<exclude-pattern>src/Psalm/Internal/Codebase/Scanner.php</exclude-pattern>
<exclude-pattern>src/Psalm/Type/Atomic/TNonEmptyArray.php</exclude-pattern>
<exclude-pattern>src/Psalm/Type/Atomic/TIterable.php</exclude-pattern>
<exclude-pattern>src/Psalm/Type/Atomic/TArray.php</exclude-pattern>


<!-- **************************************************************************************************************
* STANDARD: Generic *
Expand Down
2 changes: 1 addition & 1 deletion src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php
View file
Open in desktop
Expand Up @@ -470,7 +470,7 @@ public static function checkIteratorType(
|| $iterator_atomic_type instanceof TList
) {
if ($iterator_atomic_type instanceof TKeyedArray) {
if ($iterator_atomic_type->sealed) {
if ($iterator_atomic_type->fallback_value_type === null) {
$all_possibly_undefined = true;
foreach ($iterator_atomic_type->properties as $prop) {
if (!$prop->possibly_undefined) {
Expand Down
1 change: 0 additions & 1 deletion src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php
View file
Open in desktop
Expand Up @@ -123,7 +123,6 @@ public static function analyze(
$atomic_type = new TKeyedArray(
$array_creation_info->property_types,
$array_creation_info->class_strings,
$array_creation_info->can_create_objectlike,
$array_creation_info->can_create_objectlike ? null : ($item_key_type ?? Type::getArrayKey()),
$array_creation_info->can_create_objectlike ? null : ($item_value_type ?? Type::getMixed()),
$array_creation_info->all_list
Expand Down
2 changes: 1 addition & 1 deletion src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php
View file
Open in desktop
Expand Up @@ -3606,7 +3606,7 @@ private static function getInarrayAssertions(
$value_type = $atomic_type->type_param;
} elseif ($atomic_type instanceof TKeyedArray) {
$value_type = $atomic_type->getGenericValueType();
$is_sealed = $atomic_type->sealed;
$is_sealed = $atomic_type->fallback_value_type === null;
} else {
$value_type = $atomic_type->type_params[1];
}
Expand Down
5 changes: 2 additions & 3 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/ArrayAssignmentAnalyzer.php
View file
Open in desktop
Expand Up @@ -375,8 +375,7 @@ private static function updateTypeWithKeyValues(
[$key_value->value => $current_type],
$key_value instanceof TLiteralClassString
? [$key_value->value => true]
: null,
true
: null
);

$array_assignment_type = new Union([
Expand Down Expand Up @@ -614,7 +613,7 @@ private static function updateArrayAssignmentChildType(
/** @psalm-suppress InaccessibleProperty We just created this object */
$array_atomic_type->count = $atomic_root_types['array']->count;
} elseif ($atomic_root_types['array'] instanceof TKeyedArray
&& $atomic_root_types['array']->sealed
&& $atomic_root_types['array']->fallback_value_type === null
) {
/** @psalm-suppress InaccessibleProperty We just created this object */
$array_atomic_type->count = count($atomic_root_types['array']->properties);
Expand Down
4 changes: 2 additions & 2 deletions src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php
View file
Open in desktop
Expand Up @@ -1274,7 +1274,7 @@ private static function analyzeDestructuringAssignment(
continue;
}

if ($assign_value_atomic_type->sealed) {
if ($assign_value_atomic_type->fallback_value_type === null) {
IssueBuffer::maybeAdd(
new InvalidArrayOffset(
'Cannot access value with offset ' . $offset,
Expand Down Expand Up @@ -1451,7 +1451,7 @@ private static function analyzeDestructuringAssignment(
);
}

$can_be_empty = !$assign_value_atomic_type->sealed;
$can_be_empty = $assign_value_atomic_type->fallback_value_type !== null;
} elseif ($assign_value_atomic_type->hasArrayAccessInterface($codebase)) {
ForeachAnalyzer::getKeyValueParamsForTraversableObject(
$assign_value_atomic_type,
Expand Down
37 changes: 35 additions & 2 deletions src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ArithmeticOpAnalyzer.php
View file
Open in desktop
Expand Up @@ -576,16 +576,49 @@ private static function analyzeOperands(
}
}

if (!$left_type_part->sealed) {
if ($left_type_part->fallback_value_type !== null) {
foreach ($definitely_existing_mixed_right_properties as $key => $type) {
$properties[$key] = Type::combineUnionTypes(Type::getMixed(), $type);
}
}

if ($left_type_part->fallback_key_type === null
&& $right_type_part->fallback_key_type === null
) {
$fallback_key_type = null;
} elseif ($left_type_part->fallback_key_type !== null
&& $right_type_part->fallback_key_type !== null
) {
$fallback_key_type = Type::combineUnionTypes(
$left_type_part->fallback_key_type,
$right_type_part->fallback_key_type
);
} else {
$fallback_key_type = $left_type_part->fallback_key_type
?: $right_type_part->fallback_key_type;
}

if ($left_type_part->fallback_value_type === null
&& $right_type_part->fallback_value_type === null
) {
$fallback_value_type = null;
} elseif ($left_type_part->fallback_value_type !== null
&& $right_type_part->fallback_value_type !== null
) {
$fallback_value_type = Type::combineUnionTypes(
$left_type_part->fallback_value_type,
$right_type_part->fallback_value_type
);
} else {
$fallback_value_type = $left_type_part->fallback_value_type
?: $right_type_part->fallback_value_type;
}

$new_keyed_array = new TKeyedArray(
$properties,
null,
$left_type_part->sealed && $right_type_part->sealed
$fallback_key_type,
$fallback_value_type
);
$result_type_member = new Union([$new_keyed_array]);
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php
View file
Open in desktop
Expand Up @@ -1713,7 +1713,7 @@ private static function checkArgCount(
) {
$packed_var_definite_args_tmp[] = 2;
} elseif ($atomic_arg_type instanceof TKeyedArray) {
if (!$atomic_arg_type->sealed) {
if ($atomic_arg_type->fallback_value_type !== null) {
return;
}

Expand Down
6 changes: 3 additions & 3 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallReturnTypeFetcher.php
View file
Open in desktop
Expand Up @@ -329,7 +329,7 @@ private static function getReturnTypeFromCallMapWithArgs(
$keyed_array = new TKeyedArray([
Type::getInt(),
Type::getInt()
], null, true, null, null, true);
], null, null, null, true);
return new Union([$keyed_array]);

case 'get_called_class':
Expand Down Expand Up @@ -401,7 +401,7 @@ private static function getReturnTypeFromCallMapWithArgs(
}
}

if ($atomic_types['array']->sealed) {
if ($atomic_types['array']->fallback_value_type === null) {
//the KeyedArray is sealed, we can use the min and max
if ($min === $max) {
return new Union([new TLiteralInt($max)]);
Expand Down Expand Up @@ -438,7 +438,7 @@ private static function getReturnTypeFromCallMapWithArgs(
$keyed_array = new TKeyedArray([
Type::getInt(),
Type::getInt()
], null, true, null, null, true);
], null, null, null, true);

if ((string) $first_arg_type === 'false') {
return new Union([$keyed_array]);
Expand Down
2 changes: 1 addition & 1 deletion src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php
View file
Open in desktop
Expand Up @@ -246,7 +246,7 @@ public static function analyze(

foreach ($stmt_expr_type->getAtomicTypes() as $type) {
if ($type instanceof Scalar) {
$keyed_array = new TKeyedArray([new Union([$type])], null, true, null, null, true);
$keyed_array = new TKeyedArray([new Union([$type])], null, null, null, true);
$permissible_atomic_types[] = $keyed_array;
} elseif ($type instanceof TNull) {
$permissible_atomic_types[] = new TArray([Type::getNever(), Type::getNever()]);
Expand Down
59 changes: 29 additions & 30 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php
View file
Open in desktop
Expand Up @@ -1126,7 +1126,7 @@ private static function handleArrayAccessOnArray(
$single_atomic = $key_values[0];
$from_mixed_array = $type->type_params[1]->isMixed();

[$previous_key_type, $previous_value_type] = $type->type_params;
[$fallback_key_type, $fallback_value_type] = $type->type_params;

// ok, type becomes an TKeyedArray
$type = new TKeyedArray(
Expand All @@ -1136,17 +1136,16 @@ private static function handleArrayAccessOnArray(
$single_atomic instanceof TLiteralClassString ? [
$single_atomic->value => true
] : null,
$from_empty_array,
$from_empty_array ? null : $previous_key_type,
$from_empty_array ? null : $previous_value_type,
$from_empty_array ? null : $fallback_key_type,
$from_empty_array ? null : $fallback_value_type,
);
} elseif (!$stmt->dim && $from_empty_array && $replacement_type) {
$type = new TNonEmptyList($replacement_type);
return;
}
} elseif ($type instanceof TKeyedArray
&& $type->previous_value_type
&& $type->previous_value_type->isMixed()
&& $type->fallback_value_type
&& $type->fallback_value_type->isMixed()
&& count($key_values) === 1
) {
$properties = $type->properties;
Expand Down Expand Up @@ -1525,7 +1524,7 @@ private static function handleArrayAccessOnKeyedArray(
): void {
$generic_key_type = $type->getGenericKeyType();

if (!$stmt->dim && $type->sealed && $type->is_list) {
if (!$stmt->dim && $type->fallback_value_type === null && $type->is_list) {
$key_values[] = new TLiteralInt(count($type->properties));
}

Expand Down Expand Up @@ -1553,7 +1552,7 @@ private static function handleArrayAccessOnKeyedArray(
$array_access_type,
$properties[$key_value->value]
);
} elseif ($type->previous_value_type) {
} elseif ($type->fallback_value_type) {
if ($codebase->config->ensure_array_string_offsets_exist) {
self::checkLiteralStringArrayOffset(
$offset_type,
Expand All @@ -1576,38 +1575,36 @@ private static function handleArrayAccessOnKeyedArray(
);
}

$properties[$key_value->value] = $type->previous_value_type;
$properties[$key_value->value] = $type->fallback_value_type;

$array_access_type = $type->previous_value_type;
$array_access_type = $type->fallback_value_type;
} elseif ($hasMixed) {
$has_valid_offset = true;

$array_access_type = Type::getMixed();
} else {
if ($type->sealed || !$context->inside_isset) {
$object_like_keys = array_keys($properties);
$object_like_keys = array_keys($properties);

$last_key = array_pop($object_like_keys);
$last_key = array_pop($object_like_keys);

$key_string = '';
$key_string = '';

if ($object_like_keys) {
$formatted_keys = implode(
', ',
array_map(
/** @param int|string $key */
static fn($key): string => is_int($key) ? "$key" : '\'' . $key . '\'',
$object_like_keys
)
);
if ($object_like_keys) {
$formatted_keys = implode(
', ',
array_map(
/** @param int|string $key */
static fn($key): string => is_int($key) ? "$key" : '\'' . $key . '\'',
$object_like_keys
)
);

$key_string = $formatted_keys . ' or ';
}
$key_string = $formatted_keys . ' or ';
}

$key_string .= is_int($last_key) ? $last_key : '\'' . $last_key . '\'';
$key_string .= is_int($last_key) ? $last_key : '\'' . $last_key . '\'';

$expected_offset_types[] = $key_string;
}
$expected_offset_types[] = $key_string;

$array_access_type = Type::getMixed();
}
Expand Down Expand Up @@ -1657,7 +1654,9 @@ private static function handleArrayAccessOnKeyedArray(
$offset_type->isMixed() ? Type::getArrayKey() : $offset_type->freeze()
);

$property_count = $type->sealed ? count($type->properties) : null;
$property_count = $type->fallback_value_type === null
? count($type->properties)
: null;

if (!$stmt->dim && $property_count) {
++$property_count;
Expand Down Expand Up @@ -1690,7 +1689,7 @@ private static function handleArrayAccessOnKeyedArray(
$has_valid_offset = true;
} else {
if (!$context->inside_isset
|| ($type->sealed && !$union_comparison_results->type_coerced)
|| ($type->fallback_value_type === null && !$union_comparison_results->type_coerced)
) {
$expected_offset_types[] = $generic_key_type->getId();
}
Expand Down
1 change: 0 additions & 1 deletion src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/VariableFetchAnalyzer.php
View file
Open in desktop
Expand Up @@ -775,7 +775,6 @@ private static function getGlobalTypeInner(string $var_id, bool $files_full_path
$detailed_type = new TKeyedArray(
$arr,
null,
false,
Type::getNonEmptyString(),
Type::getString()
);
Expand Down
1 change: 0 additions & 1 deletion src/Psalm/Internal/Analyzer/Statements/Expression/SimpleTypeInferer.php
View file
Open in desktop
Expand Up @@ -577,7 +577,6 @@ private static function inferArrayType(
$objectlike = new TKeyedArray(
$array_creation_info->property_types,
$array_creation_info->class_strings,
true,
null,
null,
$array_creation_info->all_list
Expand Down
18 changes: 8 additions & 10 deletions src/Psalm/Internal/Analyzer/Statements/UnsetAnalyzer.php
View file
Open in desktop
Expand Up @@ -83,13 +83,13 @@ public static function analyze(

/** @psalm-suppress DocblockTypeContradiction https://github.com/vimeo/psalm/issues/8518 */
if (!$properties) {
if ($atomic_root_type->previous_value_type) {
if ($atomic_root_type->fallback_value_type) {
$root_types [] =
new TArray([
$atomic_root_type->previous_key_type
? $atomic_root_type->previous_key_type
$atomic_root_type->fallback_key_type
? $atomic_root_type->fallback_key_type
: new Union([new TArrayKey]),
$atomic_root_type->previous_value_type,
$atomic_root_type->fallback_value_type,
])
;
} else {
Expand All @@ -104,9 +104,8 @@ public static function analyze(
$root_types []= new TKeyedArray(
$properties,
null,
$atomic_root_type->sealed,
$atomic_root_type->previous_key_type,
$atomic_root_type->previous_value_type,
$atomic_root_type->fallback_key_type,
$atomic_root_type->fallback_value_type,
$is_list
);
}
Expand All @@ -118,9 +117,8 @@ public static function analyze(
$root_types []= new TKeyedArray(
$properties,
null,
false,
$atomic_root_type->previous_key_type,
$atomic_root_type->previous_value_type,
$atomic_root_type->fallback_key_type,
$atomic_root_type->fallback_value_type,
false,
);
}
Expand Down