Skip to content

Commit

Permalink
Prevent array{a: Foo} going cleanly into array<Foo> (#8691)
Browse files Browse the repository at this point in the history 
* Prevent array{a: Foo} going cleanly into array<Foo>

* Add test for new behaviour

* Fix code style issues

* Allow unions to be cloned again

* Simplify params properties
  • Loading branch information
@muglug
muglug committed Nov 10, 2022
1 parent 0cd1f13 commit d63da1f
Show file tree
Hide file tree
Showing 38 changed files with 266 additions and 259 deletions.
8 changes: 8 additions & 0 deletions phpcs.xml
View file
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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

0 comments on commit d63da1f

Please sign in to comment.