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

Fix comparison of unions of multiple callables #9676

Merged
merged 3 commits into from
Apr 19, 2023
Merged

Fix comparison of unions of multiple callables #9676

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
19fab7a
Fix comparison of unions of multiple callables
danog Apr 19, 2023
e16037e
Fix tests
danog Apr 19, 2023
42fb384
Update
danog Apr 19, 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
2 changes: 1 addition & 1 deletion src/Psalm/Internal/Type/Comparator/UnionTypeComparator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ public static function isContainedBy(
if ($container_required_param_count > $input_all_param_count
|| $container_all_param_count < $input_required_param_count
) {
return false;
continue;
orklah marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
4 changes: 2 additions & 2 deletions tests/ArgTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,7 @@ function caller($callback) {}
function foo($a) {}

caller("foo");',
'error_message' => 'InvalidScalarArgument',
'error_message' => 'PossiblyInvalidArgument',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure why this could be considered possibly invalid

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This seems to be caused by a call to UnionTypeComparator::canBeContainedBy, which doesn't have the same callable parameter comparison logic that UnionTypeComparator::isContainedBy has.

Actually, that logic should probably be moved to the CallableTypeComparator...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah I see now, it's to allow things like https://psalm.dev/r/3cc38f5a0c

],
'callableArgsCountMismatch' => [
'code' => '<?php
Expand All @@ -823,7 +823,7 @@ function caller($callback) {}
function foo($a, $b) {}

caller("foo");',
'error_message' => 'InvalidScalarArgument',
'error_message' => 'InvalidArgument',
],
];
}
Expand Down
4 changes: 4 additions & 0 deletions tests/TypeComparatorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ public function getSuccessfulComparisons(): array
'lowercase-string',
'callable-string',
],
'callableUnionAcceptsCallableUnion' => [
'callable(int,string[]): void|callable(int): void',
danog marked this conversation as resolved.
Show resolved Hide resolved
'callable(int): void|callable(int,string[]): void',
],
];
}

Expand Down