Skip to content

Conversation

phil-davis
Copy link
Contributor

@phil-davis phil-davis commented Aug 25, 2023

Description

The callback function to usort is supposed to return an int
https://www.php.net/manual/en/function.usort.php
The comparison function must return an integer less than, equal to, or greater than zero if the first argument is considered to be respectively less than, equal to, or greater than the second.
callback([mixed](https://www.php.net/manual/en/language.types.declarations.php#language.types.declarations.mixed) $a, [mixed](https://www.php.net/manual/en/language.types.declarations.php#language.types.declarations.mixed) $b): int

Fix usort calls in core so that they do return an int. Some of them were returning boolean true/false.

I don't think that there is any real behavior change. Returning true is just like returning 1 and so in reality the sorter will make the same decision in this case. Returning false is just like returning 0 - so the sorter will think that the items have the same sort "priority", but actually it might be the case that -1 should have been returned. So items might end up out-of-order.

https://www.php.net/manual/en/function.usort.php

How Has This Been Tested?

CI

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@phil-davis phil-davis self-assigned this Aug 25, 2023
@owncloud owncloud deleted a comment from update-docs bot Aug 25, 2023
@phil-davis phil-davis changed the title [WIP] Fix usort callback functions to return integers Fix usort callback functions to return integers Aug 25, 2023
@phil-davis phil-davis requested a review from jvillafanez August 25, 2023 10:39
@phil-davis phil-davis marked this pull request as ready for review August 25, 2023 10:40
@phil-davis
Copy link
Contributor Author

I need to look at the PHP unit tests...

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

75.0% 75.0% Coverage
0.0% 0.0% Duplication

@phil-davis
Copy link
Contributor Author

I need to look at the PHP unit tests...

done. @jvillafanez please review again.

@phil-davis phil-davis merged commit f5e9fbc into master Aug 28, 2023
@delete-merged-branch delete-merged-branch bot deleted the fix-usort-calls branch August 28, 2023 14:50
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.

2 participants