Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:124462
To: PHP internals <internals@lists.php.net>
Subject: [PHP-DEV] Request for opinions: bug vs feature - change in tokenization of yield from
Message-ID: <66984FD0.5090805@adviesenzo.nl>
Date: Thu, 18 Jul 2024 01:12:16 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:38.0) Gecko/20100101
 Thunderbird/38.7.0
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/alternative;
 boundary="------------070705070108060206080208"
From: php-internals_nospam@adviesenzo.nl (Juliette Reinders Folmer)

This is a multi-part message in MIME format.
--------------070705070108060206080208
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit

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 adetailed 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


--------------070705070108060206080208
Content-Type: text/html; charset=utf-8
Content-Transfer-Encoding: 7bit

<html>
  <head>

    <meta http-equiv="content-type" content="text/html; charset=utf-8">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <font size="-1"><font face="Verdana">Hi all,<br>
        <br>
        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.<br>
        Before PHP 8.3, this was a parse error.<br>
        <br>
        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].<br>
        <br>
        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 </font></font><font size="-1"><font
        face="Verdana"><font size="-1"><font face="Verdana">(aside from
            comments tokens of course)</font></font>.<br>
        Comments were even explicitly forbidden in the PHP 8.0 RFC which
        changed the tokenization of namespaced names [2].<br>
        Some more detailed explanation and examples can be found in the
        GH thread [3] </font></font>and a<font size="-1"><font
        face="Verdana"><font size="-1"><font face="Verdana"><font
              size="-1"><font face="Verdana"> 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).<br>
                <br>
              </font></font></font></font>In my opinion the change is a
        bug and should be reverted, but opinions on this are divided.<br>
      </font></font><font size="-1"><font face="Verdana"><font size="-1"><font
            face="Verdana">After a discussion in the ticket on GitHub,
            it was suggested to ask for a third/fourth/fifth opinion
            here on the list.<br>
          </font></font><br>
        Pros for reverting it:<br>
        - Consistency with other tokens in the token stream, none of
        which allow comments within the token.<br>
        - Consistency with the 7 years before in which this was a parse
        error.<br>
        - The change was never documented, not in the changelog, news,
        nor in the upgrading guide.<br>
        <br>
        Cons against reverting it:<br>
        - The change has been in PHP 8.3 releases for a little over
        seven months.<br>
        - 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.<br>
        <br>
        Opinions ?<br>
        <br>
        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.<br>
        The current question is only about reverting the bug versus
        regarding it as a new feature and properly documenting it.<br>
        <br>
        Smile,<br>
        Juliette</font></font><font size="-1"><font face="Verdana"><font
          size="-1"><font face="Verdana"><br>
          </font></font><br>
      </font></font><br>
    <font size="-1"><font face="Verdana">1<font size="-1"><font
            face="Verdana">: <a class="moz-txt-link-freetext" href="https://github.com/php/php-src/issues/14926">https://github.com/php/php-src/issues/14926</a><br>
            2: </font></font></font></font><font size="-1"><font
        face="Verdana"><font size="-1"><font face="Verdana"><font
              size="-1"><font face="Verdana"><a class="moz-txt-link-freetext" href="https://wiki.php.net/rfc/namespaced_names_as_token#backward_incompatible_changes">https://wiki.php.net/rfc/namespaced_names_as_token#backward_incompatible_changes</a><br>
              </font></font>3</font></font>:
<a class="moz-txt-link-freetext" href="https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/529#issuecomment-2209337419">https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/529#issuecomment-2209337419</a><br>
        4:
        <a class="moz-txt-link-freetext" href="https://github.com/php/php-src/issues/14926#issuecomment-2227152061">https://github.com/php/php-src/issues/14926#issuecomment-2227152061</a><br>
      </font></font><font size="-1"><font face="Verdana"><br>
      </font></font>
  </body>
</html>

--------------070705070108060206080208--