> On 17 Jul 2024, at 20:16, Juliette Reinders Folmer > <php-internals_nos...@adviesenzo.nl> wrote: > > Hi all, > > I recently discovered a change was made to the Tokenizer in PHP 8.3.0 which > now allows for a comment to exist between the `yield` and `from` keyword from > the `yield from` keyword. > Before PHP 8.3, this was a parse error. > > This change was not documented (anywhere) nor publicized, which is why I only > recently came across it and opened a bug report on GitHub about it [1]. > > This change is a breaking change for projects consuming the token stream, but > more than that, it introduces an inconsistency in the Tokenizer as there is > no other single token in PHP which can contain a comment (aside from comments > tokens of course). > Comments were even explicitly forbidden in the PHP 8.0 RFC which changed the > tokenization of namespaced names [2]. > Some more detailed explanation and examples can be found in the GH thread [3] > and a detailed analysis of the tokenization change can be found in a ticket > in the PHP_CodeSniffer repo [4] (details in a fold out in the comment). > > In my opinion the change is a bug and should be reverted, but opinions on > this are divided. > After a discussion in the ticket on GitHub, it was suggested to ask for a > third/fourth/fifth opinion here on the list. > > Pros for reverting it: > - Consistency with other tokens in the token stream, none of which allow > comments within the token. > - Consistency with the 7 years before in which this was a parse error. > - The change was never documented, not in the changelog, news, nor in the > upgrading guide. > > Cons against reverting it: > - The change has been in PHP 8.3 releases for a little over seven months. > - The odd few people may have accidentally discovered the bug and taken > advantage by introducing code containing `yield /*comment*/ from` into their > project, which with a revert would become a parse error again. > > Opinions ? > > Of course, the ticket yielded discussion on whether the tokenization of > `yield from` should be changed anyhow, but please consider that discussion > out of scope. > The current question is only about reverting the bug versus regarding it as a > new feature and properly documenting it. > > Smile, > Juliette > > > 1: https://github.com/php/php-src/issues/14926 > 2: > https://wiki.php.net/rfc/namespaced_names_as_token#backward_incompatible_changes > 3: > https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/529#issuecomment-2209337419 > 4: https://github.com/php/php-src/issues/14926#issuecomment-2227152061 >
My opinion would be to revert it. Chances of someone accidentally discovering and actually using it seems unlikely at best. Even then, the parser error could be easily resolved by moving the comment out of the middle of the token with insignificant readability change. Forcing all tooling that uses token_get_all() to handle this unintentional change seems to generate more unnecessary and real busywork for something only theoretical possible to break.
