Skip to content

Improve sorting based on a second array with Arr::sort() - #790

Merged
giuscris merged 1 commit into
2.xfrom
improve-arr-sort
Nov 18, 2025
Merged

Improve sorting based on a second array with Arr::sort()#790
giuscris merged 1 commit into
2.xfrom
improve-arr-sort

Conversation

@giuscris

Copy link
Copy Markdown
Member

This pull request improves the type annotations and logic for sorting methods in the codebase, with a focus on clarifying parameters and enhancing array sorting behavior. The main updates include more precise PHPDoc types for sorting options, better handling of the $sortBy parameter, and improved error checking in the array sorting utility.

Type annotation improvements:

  • Updated PHPDoc comments in AbstractCollection.php and Arr.php to specify accepted types for sorting parameters such as $direction, $type, and $sortBy, making the documentation clearer and more accurate. [1] [2] [3]

Array sorting logic enhancements:

  • Refactored Arr::sort() to handle the $sortBy parameter more robustly, including:
    • Returning early when a callable is used.
    • Throwing an exception if $sortBy and the array to sort have mismatched counts.
    • Throwing an exception if a key in $sortBy is not present in the array.
    • Simplifying the sorting logic and result construction to ensure keys are preserved or not based on the $preserveKeys flag.

@giuscris giuscris added this to the 2.2.0 milestone Nov 18, 2025
@giuscris
giuscris requested a review from Copilot November 18, 2025 22:00
@giuscris giuscris self-assigned this Nov 18, 2025
@giuscris giuscris added the enhancement New feature or request label Nov 18, 2025

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the array sorting logic in Arr::sort() and improves PHPDoc type annotations for sorting methods across the codebase. The changes replace the complex array_multisort() implementation with a more explicit approach that includes better error handling and validation.

  • Refactored Arr::sort() to use explicit sorting logic with early returns for different $sortBy scenarios
  • Added validation to ensure $sortBy array matches the target array in count and keys
  • Updated PHPDoc annotations to use more precise union types for sorting constants

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
formwork/src/Utils/Arr.php Refactored sorting logic to replace array_multisort() with explicit handling of different $sortBy scenarios; added validation for array count and key matching; updated PHPDoc with precise type annotations
formwork/src/Data/AbstractCollection.php Updated PHPDoc annotations for sort() and sortBy() methods with precise union types for sorting constants
Comments suppressed due to low confidence (1)

formwork/src/Utils/Arr.php:1

  • The parameter order in the PHPDoc doesn't match the actual function signature. In the PHPDoc, $sortBy is listed before $caseSensitive (lines 643-644), but in the function signature (lines 655-661), the order is $direction, $type, $sortBy, $caseSensitive, $preserveKeys. The PHPDoc should list parameters in the same order as the function signature for clarity.
<?php

馃挕 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread formwork/src/Utils/Arr.php
Comment thread formwork/src/Data/AbstractCollection.php
Comment thread formwork/src/Utils/Arr.php Outdated
@giuscris
giuscris marked this pull request as ready for review November 18, 2025 22:10
@giuscris
giuscris merged commit cf88d62 into 2.x Nov 18, 2025
1 check passed
@giuscris
giuscris deleted the improve-arr-sort branch November 18, 2025 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants