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

Fixes #9827 #9903

Merged
merged 2 commits into from Jun 15, 2023
Merged

Fixes #9827 #9903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
40ce0cf
Fixes #9827
ygottschalk Jun 12, 2023
d11bb90
Style fixes
ygottschalk Jun 12, 2023
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
4 changes: 3 additions & 1 deletion src/Psalm/Internal/Type/TypeCombiner.php
View file
Open in desktop
Expand Up @@ -130,7 +130,9 @@ public static function combine(
if (count($combination->value_types) === 1
&& !count($combination->objectlike_entries)
&& (!$combination->array_type_params
|| $combination->array_type_params[1]->isNever()
|| ( $overwrite_empty_array
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like $overwrite_empty_array should also apply to !$combination->array_type_params

So it would end up as

&& ($overwrite_empty_array && (!$combination->array_type_params || $combination->array_type_params[1]->isNever()))

or something

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought ((bool) $combination->array_type_params) === false means, there was no (unsealed/generic) array type given to combine with. So that case was for me "no array" instead of "empty array". Thats why I decided to not include that in the check...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put another PR with the version you suggested (#9909) but (for reasons given above) I feel like this is the right version. Merge which ever you think is better. Close the other one. Thanks <3

&& $combination->array_type_params[1]->isNever()
)
)
&& !$combination->builtin_type_params
&& !$combination->object_type_params
Expand Down
22 changes: 22 additions & 0 deletions tests/TypeCombinationTest.php
View file
Open in desktop
Expand Up @@ -338,6 +338,28 @@ public function providerTestValidTypeCombination(): array
'ArrayObject<int, string>',
],
],
'emptyArrayAndFalse' => [
'array<never, never>|false',
[
'array<never, never>',
'false',
],
],
'emptyArrayAndTrue' => [
'array<never, never>|true',
[
'array<never, never>',
'true',
],
],
'emptyArrayWithTrueAndFalse' => [
'array<never, never>|bool',
[
'array<never, never>',
'true',
'false',
],
],
'falseDestruction' => [
'bool',
[
Expand Down