Allow doctrine/lexer 2 #460
Conversation
98d1200 to
83fc5d1
Compare
|
SA is broken, I will fix it once doctrine/lexer 2 is removed, but I just wanted to show here that things are OK according to PHPUnit. |
| } | ||
|
|
||
| /** @return array{value: int|string, type:string|int|null, position:int} */ | ||
| public function peek(): array |
There was a problem hiding this comment.
No need to also override glimpse since it returns whatever peek() returns.
It is still possible to avoid a BC-break while doing so.
83fc5d1 to
2286f7b
Compare
derrabus
left a comment
There was a problem hiding this comment.
Sacrificing the Liskov substitution principle on the altar of backwards compatibility. 🤞🏻
| { | ||
| $token = parent::peek(); | ||
|
|
||
| return (array) $token; |
There was a problem hiding this comment.
Should the array type always be forced here? In v1.13.x that was not the case because the method was inherited from the parent class. The docblock does not state that an empty array could be returned, but that is exactly what will be returned when the parent returns null right now.
There was a problem hiding this comment.
No, we should not cast null to an array. Up for a PR?
It is still possible to avoid a BC-break while doing so.