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 all 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_params === null) {
$all_possibly_undefined = true;
foreach ($iterator_atomic_type->properties as $prop) {
if (!$prop->possibly_undefined) {
Expand Down
6 changes: 3 additions & 3 deletions src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php
View file
Open in desktop
Expand Up @@ -123,9 +123,9 @@ 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->can_create_objectlike
? null :
[$item_key_type ?? Type::getArrayKey(), $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_params === 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_params === 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_params === 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_params !== null;
} elseif ($assign_value_atomic_type->hasArrayAccessInterface($codebase)) {
ForeachAnalyzer::getKeyValueParamsForTraversableObject(
$assign_value_atomic_type,
Expand Down
25 changes: 23 additions & 2 deletions src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ArithmeticOpAnalyzer.php
View file
Open in desktop
Expand Up @@ -576,16 +576,37 @@ private static function analyzeOperands(
}
}

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

if ($left_type_part->fallback_params === null
&& $right_type_part->fallback_params === null
) {
$fallback_params = null;
} elseif ($left_type_part->fallback_params !== null
&& $right_type_part->fallback_params !== null
) {
$fallback_params = [
Type::combineUnionTypes(
$left_type_part->fallback_params[0],
$right_type_part->fallback_params[0]
),
Type::combineUnionTypes(
$left_type_part->fallback_params[1],
$right_type_part->fallback_params[1]
),
];
} else {
$fallback_params = $left_type_part->fallback_params ?: $right_type_part->fallback_params;
}

$new_keyed_array = new TKeyedArray(
$properties,
null,
$left_type_part->sealed && $right_type_part->sealed
$fallback_params
);
$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_params !== 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, 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_params === 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, 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, true);
$permissible_atomic_types[] = $keyed_array;
} elseif ($type instanceof TNull) {
$permissible_atomic_types[] = new TArray([Type::getNever(), Type::getNever()]);
Expand Down
58 changes: 28 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,15 @@ 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, $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_params !== null
&& $type->fallback_params[1]->isMixed()
&& count($key_values) === 1
) {
$properties = $type->properties;
Expand Down Expand Up @@ -1525,7 +1523,7 @@ private static function handleArrayAccessOnKeyedArray(
): void {
$generic_key_type = $type->getGenericKeyType();

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

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

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

$array_access_type = $type->previous_value_type;
$array_access_type = $type->fallback_params[1];
} 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 +1653,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_params === null
? count($type->properties)
: null;

if (!$stmt->dim && $property_count) {
++$property_count;
Expand Down Expand Up @@ -1690,7 +1688,7 @@ private static function handleArrayAccessOnKeyedArray(
$has_valid_offset = true;
} else {
if (!$context->inside_isset
|| ($type->sealed && !$union_comparison_results->type_coerced)
|| ($type->fallback_params === null && !$union_comparison_results->type_coerced)
) {
$expected_offset_types[] = $generic_key_type->getId();
}
Expand Down
4 changes: 1 addition & 3 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/VariableFetchAnalyzer.php
View file
Open in desktop
Expand Up @@ -775,9 +775,7 @@ private static function getGlobalTypeInner(string $var_id, bool $files_full_path
$detailed_type = new TKeyedArray(
$arr,
null,
false,
Type::getNonEmptyString(),
Type::getString()
[Type::getNonEmptyString(), Type::getString()]
);

return new Union([$detailed_type]);
Expand Down
2 changes: 0 additions & 2 deletions src/Psalm/Internal/Analyzer/Statements/Expression/SimpleTypeInferer.php
View file
Open in desktop
Expand Up @@ -577,8 +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
17 changes: 5 additions & 12 deletions src/Psalm/Internal/Analyzer/Statements/UnsetAnalyzer.php
View file
Open in desktop
Expand Up @@ -8,7 +8,6 @@
use Psalm\Internal\Analyzer\StatementsAnalyzer;
use Psalm\Type;
use Psalm\Type\Atomic\TArray;
use Psalm\Type\Atomic\TArrayKey;
use Psalm\Type\Atomic\TKeyedArray;
use Psalm\Type\Atomic\TList;
use Psalm\Type\Atomic\TMixed;
Expand Down Expand Up @@ -83,13 +82,11 @@ 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_params) {
$root_types [] =
new TArray([
$atomic_root_type->previous_key_type
? $atomic_root_type->previous_key_type
: new Union([new TArrayKey]),
$atomic_root_type->previous_value_type,
$atomic_root_type->fallback_params[0],
$atomic_root_type->fallback_params[1],
])
;
} else {
Expand All @@ -104,9 +101,7 @@ 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_params,
$is_list
);
}
Expand All @@ -118,9 +113,7 @@ 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_params,
false,
);
}
Expand Down
11 changes: 5 additions & 6 deletions src/Psalm/Internal/Codebase/ConstantTypeResolver.php
View file
Open in desktop
Expand Up @@ -141,8 +141,7 @@ public static function resolve(
if ($left instanceof TKeyedArray && $right instanceof TKeyedArray) {
$type = new TKeyedArray(
$left->properties + $right->properties,
null,
true
null
);
return $type;
}
Expand Down Expand Up @@ -266,7 +265,7 @@ public static function resolve(
new Union([new TNever()]),
]);
} else {
$resolved_type = new TKeyedArray($properties, null, true, null, null, $is_list);
$resolved_type = new TKeyedArray($properties, null, null, $is_list);
}

return $resolved_type;
Expand Down Expand Up @@ -340,7 +339,7 @@ public static function resolve(
*
* @param array|string|int|float|bool|null $value
*/
public static function getLiteralTypeFromScalarValue($value, bool $sealed_array = true): Atomic
public static function getLiteralTypeFromScalarValue($value): Atomic
{
if (is_array($value)) {
if (empty($value)) {
Expand All @@ -350,9 +349,9 @@ public static function getLiteralTypeFromScalarValue($value, bool $sealed_array
$types = [];
/** @var array|scalar|null $val */
foreach ($value as $key => $val) {
$types[$key] = new Union([self::getLiteralTypeFromScalarValue($val, $sealed_array)]);
$types[$key] = new Union([self::getLiteralTypeFromScalarValue($val)]);
}
return new TKeyedArray($types, null, $sealed_array);
return new TKeyedArray($types, null);
}

if (is_string($value)) {
Expand Down