Document generics order for PriorityQueue and SplPriorityQueue #72
Conversation
Closes laminas#71 Previously implemented template order did not match those of native SplPriorityQueue, i.e. \SplPriorityQueue<TPriority, TValue> - instead the templates were back-to-front. - PriorityQueue order is fixed and a test case added - PriorityQueue value template renamed from `T` to `TValue` - SplPriorityQueue order fixed and test case added - Removes attempt to document internal array priority, adding resulting psalm issues to the baseline. Users will typically insert with an integer priority and shouldn't care that internally an array is used to represent priority - also, an array for \SplPriorityQueue priority is effectively an invalid argument according to stubs - this is better off in the baseline right? Signed-off-by: George Steel <[email protected]>
Signed-off-by: George Steel <[email protected]>
|
Not sure if this should target 3.12.1 or 3.13.0 ? |
|
@gsteel if this is a bugfix, |
|
This would possibly break upstream projects which already spent time in fixing their code. Yes, this looks correct to me but technically, it was not incorrect before. |
|
ah this is only released for a few hours, maybe we can Patch it 👌🏻 |
|
Oh, thats been a while. Not sure if we should annoy upstream devs who spent time to fix their static analysis. Do we have other thoughts? |
I guess it depends whether we consider the original template order a bug or not - IMO it's a bug - aligning with PHP's SplPriorityQueue and psalms stubs should be a given. Not annoying devs with this is really valid, but the longer it's left, the more people we annoy? This will already affect one or two laminas packages I think… view, maybe validator, possibly others. My bad! Sorry… |
It's also possible, if this is a bug, that devs who already implemented correct static analysis have been annoyed by this break, so would appreciate this fix. |
|
Discussed in chat: clarifying template types is OK, but swapping order will likely just lead to unnecessary downstream breakages. Yes, it is annoying that the generic types are inverted in our implementation, but it is just that: annoying. Not a major deal breaker. |
…d inference Signed-off-by: George Steel <[email protected]>
gsteel
changed the title
Reference #71
Previously implemented template order did not match those of native SplPriorityQueue, i.e. \SplPriorityQueue<TPriority, TValue> - instead the templates were back-to-front.
It's decided that 'fixing' the order of the placeholders will be inconvenient for users that have already implemented templates with the existing order.
TtoTValue