Skip to content

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

Merged
ondrejmirtes merged 3 commits intophpstan:1.10.xfrom
takaram:sort-type
Jan 28, 2024
Merged

sort(), rsort() and usort() converts an array to list#2891
ondrejmirtes merged 3 commits intophpstan:1.10.xfrom
takaram:sort-type

Conversation

@takaram
Copy link
Copy Markdown
Contributor

@takaram takaram commented Jan 27, 2024

@staabm
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

takaram commented Jan 27, 2024

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

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

takaram commented Jan 28, 2024

@ondrejmirtes
Thank you. I fixed the code.

@takaram
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
@ondrejmirtes
Copy link
Copy Markdown
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

Development

Successfully merging this pull request may close these issues.

3 participants