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.

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

sort(), rsort() and usort() converts an array to list #2891

Merged
merged 3 commits into from Jan 28, 2024

Conversation

takaram
Copy link
Contributor

@takaram takaram commented Jan 27, 2024

@staabm
Copy link
Contributor

staabm commented Jan 27, 2024

Please have a look at phpstan/phpstan#3312

It seems also fixed by your PR. If so, please add a test

@takaram
Copy link
Contributor Author

takaram commented Jan 27, 2024

Yes, this PR will fix it too. I added a test.
Thank you!

Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

Otherwise perfect 👍

private function getArraySortFunctionType(Type $type): Type
{
if (!$type->isArray()->yes()) {
return new ErrorType();
Copy link
Member

Choose a reason for hiding this comment

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

Just return the same $type instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed


$isIterableAtLeastOnce = $type->isIterableAtLeastOnce();
if ($isIterableAtLeastOnce->no()) {
return new ConstantArrayType([], []);
Copy link
Member

Choose a reason for hiding this comment

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

Just return the same $type instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

$functionReflection !== null
&& (
(in_array($functionReflection->getName(), ['sort', 'rsort'], true) && count($expr->getArgs()) >= 1)
|| ($functionReflection->getName() === 'usort' && count($expr->getArgs()) >= 2)
Copy link
Member

Choose a reason for hiding this comment

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

We don't need to complicate this by checking that the number of args is >= 2. Just check that the first arg is always there and join the condition into a single in_array.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@takaram
Copy link
Contributor Author

takaram commented Jan 28, 2024

@ondrejmirtes
Thank you. I fixed the code.

@takaram
Copy link
Contributor Author

takaram commented Jan 28, 2024

https://github.com/phpstan/phpstan-src/actions/runs/7682388532/job/20936712149
This failure looks to be caused by this PR, but I'm not sure why it fails🤔
I need some more investigation

@takaram
Copy link
Contributor Author

takaram commented Jan 28, 2024

https://github.com/phpstan/phpstan-src/actions/runs/7682388532/job/20936712149

These errors were reported before, but they were in the baseline.
This PR changes the type descriptions, so baseline no longer suppresses the errors.

So I think we can ignore the CI error.

@ondrejmirtes
Copy link
Member

So I think we can ignore the CI error.

Yes we can :)

@ondrejmirtes ondrejmirtes merged commit e25e29e into phpstan:1.10.x Jan 28, 2024
427 of 428 checks passed
@ondrejmirtes
Copy link
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants