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

Wrong/Too strict type annotation for Collection::unique #40897

Closed
NiclasvanEyk opened this issue Feb 9, 2022 · 1 comment
Closed

Wrong/Too strict type annotation for Collection::unique #40897

NiclasvanEyk opened this issue Feb 9, 2022 · 1 comment

Comments

@NiclasvanEyk
Copy link
Contributor

  • Laravel Version: 9.0.0
  • PHP Version: 8.1.1
  • Database Driver & Version: /

Description:

Currently the $key parameter of the Collection::unique method has the following type annotation:

    /**
     * Return only unique items from the collection array.
     *
     * @param  (callable(TValue, TKey): bool)|string|null  $key
     * @param  bool  $strict
     * @return static
     */
    public function unique($key = null, $strict = false)

So basically it can be

  • a function that returns bool
  • a string
  • nothing at all

I think the first one is a little bit too strict. Looking at https://laravel.com/docs/9.x/collections#method-unique I should be able to return any value to determine uniqueness:

Finally, you may also pass your own closure to the unique method to specify which value should determine an item's uniqueness:

$unique = $collection->unique(function ($item) {
    return $item['brand'].$item['type'];
});
 
$unique->values()->all();
 
/*
    [
        ['name' => 'iPhone 6', 'brand' => 'Apple', 'type' => 'phone'],
        ['name' => 'Apple Watch', 'brand' => 'Apple', 'type' => 'watch'],
        ['name' => 'Galaxy S6', 'brand' => 'Samsung', 'type' => 'phone'],
        ['name' => 'Galaxy Gear', 'brand' => 'Samsung', 'type' => 'watch'],
    ]
*/

Suggestions

Not sure what makes the most sense here, I guess it would make sense for the function to return mixed instead of bool, but I am sure @nunomaduro knows more, since he made the adjustments in #39323?

@NiclasvanEyk NiclasvanEyk changed the title Wrong type annotation for Collection::unique Wrong/Too strict type annotation for Collection::unique Feb 9, 2022
@nunomaduro
Copy link
Member

@NiclasvanEyk Can you pull request the return mixed instead? Also, don't forget to adjust the tests on the types folder.

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

No branches or pull requests

2 participants