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

Clarification on the return type of Collection::getKeys() and Collection::getValues() #242

Open
weirdan opened this issue Jun 21, 2020 · 3 comments

Comments

@weirdan
Copy link
Contributor

weirdan commented Jun 21, 2020

/**
* Gets all keys/indices of the collection.
*
* @return array<int, int|string> The keys/indices of the collection, in the order of the corresponding
* elements in the collection.
*
* @psalm-return TKey[]
*/
public function getKeys() : array;
/**
* Gets all values of the collection.
*
* @return array The values of all elements in the collection, in the order they
* appear in the collection.
*
* @psalm-return T[]
*/
public function getValues() : array;

The above specifies that those method return T[]. However, ArrayCollection actually returns list<T> (see documentation on list type). Is it unintentional, or are implementations actually allowed to return sparse arrays from those methods?

@drupol
Copy link

drupol commented Aug 3, 2020

Hi,

I don't see anything wrong in there.

Keep in mind that array<int, int|string> is equivalent to list<int|string> which is also equivalent to TKey[] so... for me this is good.

@someniatko
Copy link
Contributor

someniatko commented Aug 3, 2020

@drupol they are not equivalent. list implies that indices go from 0 to count($list) - 1 with the step 1 (0, 1, 2, 3 ... $count -1) with no gaps and shuffling. In array there might be gaps and any order.

Example:
[ 0 => 'zero', 2 => 2 ] is a valid array<int, int|string>, but invalid list<int|string>
[ 0 => 'zero', 1 => 1 ] is both a valid array<int, int|string> and list<int|string>

There is indeed nothing "wrong" in existing annotations, but list would be more precise and therefore is better - because the retval of these methods could be used as arguments to functions type-hinted against list.

@drupol
Copy link

drupol commented Aug 3, 2020

Ooh ! Nice catch, thanks for the clarification, I didn't know :-)

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

3 participants