Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:124505
Subject: Re: [PHP-DEV] Request for opinions: bug vs feature - change
 intokenization of yield from
To: internals@lists.php.net
References: <66984FD0.5090805@adviesenzo.nl>
 <AM8P250MB0170FFCA0014FB9EC272DB4FE2AC2@AM8P250MB0170.EURP250.PROD.OUTLOOK.COM>
 <c9736385-94b7-4064-911f-e5fc1df7e2bd@gmx.de>
 <AM8P250MB0170B4C59B4313A565CD029AE2AC2@AM8P250MB0170.EURP250.PROD.OUTLOOK.COM>
Message-ID: <6699F817.8070806@adviesenzo.nl>
Date: Fri, 19 Jul 2024 07:22:31 +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
In-Reply-To: <AM8P250MB0170B4C59B4313A565CD029AE2AC2@AM8P250MB0170.EURP250.PROD.OUTLOOK.COM>
Content-Type: multipart/alternative;
 boundary="------------060003050400090207020206"
From: php-internals_nospam@adviesenzo.nl (Juliette Reinders Folmer)

This is a multi-part message in MIME format.
--------------060003050400090207020206
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit

On 19-7-2024 1:09, Bob Weinand wrote:
> Hey Christoph,
>
>> Am 19.07.2024 um 00:51 schrieb Christoph M. Becker <cmbecker69@gmx.de>:
>>
>> Hi Bob!
>>
>> On 18.07.2024 at 15:41, Bob Weinand wrote:
>>
>>> Moreover, it can - at least - be worked around in tooling by special 
>>> casing the T_YIELD_FROM token and extracting the comment from the 
>>> raw parsed string:
>>>
>>> var_dump(token_get_all('<?php yield /* comment */ from $foo;'));
>>>
>>> will contain:
>>>
>>> [1]=> array(3) { [0]=> int(270) [1]=> string(24) "yield /* comment 
>>> */ from" [2]=> int(1) }
>>>
>>> It's not optimal, but probably the least bad solution to leave it 
>>> unchanged in PHP 8.3, have tooling special case it and properly fix 
>>> it in PHP 8.4.
>>
>> And what about "code" like <https://3v4l.org/4CLhM>?  Is Codesniffer
>> supposed to scan the result of <https://3v4l.org/dKDcs> for possible CS
>> violations?
>>
>> Cheers,
>> Christoph
>
> I suppose you mean https://3v4l.org/IMi8Y, (you missed the <?php tag).
> If you want to scan that, it's quite easy to strip the leading yield 
> and trailing from, and tokenize that again to extract all comments.
>
> Sure, it's a hack, but it'll work: https://3v4l.org/8eAiV.
>
> Bob
>

Hi Bob,

Of course, everything can be hacked around, but that still leaves the 
question what should be the "proper tokenization". Having this change in 
PHP 8.3 and then - as you suggest - yet another in PHP 8.4, makes it 
mighty hard to have a consistent token stream in tooling, especially as 
it is unclear what the "proper tokenization" should/would be.

More than anything, I find it concerning that this change sets a 
precedent for tokens to include comments.

Just as an example: what does this mean for the PHP 8.0 nullsafe object 
operator ? Should we now suddenly allow that to be written as `? 
/*comment*/ ->` ?
Or what about a cast token ? Should that be allowed to be `(string /*for 
reasons*/)` ?

Allowing this change to stay in, without having the discussion about 
what the "proper tokenization" should be, feels off and random to me and 
opens the door for more random changes.

As for the impact on tooling: a change in the tokenization of any token 
has an impact not only on tooling like PHPCS itself, but also on every 
single external standard build on top of it and is a breaking change.
To give you some perspective - for PHPCS we even went as far as to 
"undo" the PHP 8.0 tokenization of namespaced names for the time being 
(in the PHPCS 3.x releases) and we'll only change the PHPCS tokenizer to 
use the PHP 8.0 tokenization in the PHPCS 4.0 release as it would 
otherwise break too many existing sniffs. [1]

Smile,
Juliette

1: https://github.com/squizlabs/PHP_CodeSniffer/issues/3041

--------------060003050400090207020206
Content-Type: text/html; charset=utf-8
Content-Transfer-Encoding: 8bit

<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <div class="moz-cite-prefix">On 19-7-2024 1:09, Bob Weinand wrote:<br>
    </div>
    <blockquote
cite="mid:AM8P250MB0170B4C59B4313A565CD029AE2AC2@AM8P250MB0170.EURP250.PROD.OUTLOOK.COM"
      type="cite">
      <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
      <div dir="ltr">Hey Christoph,</div>
      <div dir="ltr"><br>
        <blockquote type="cite">Am 19.07.2024 um 00:51 schrieb Christoph
          M. Becker <a class="moz-txt-link-rfc2396E" href="mailto:cmbecker69@gmx.de">&lt;cmbecker69@gmx.de&gt;</a>:<br>
          <br>
        </blockquote>
      </div>
      <blockquote type="cite">
        <div dir="ltr"><span>Hi Bob!</span><br>
          <span></span><br>
          <span>On 18.07.2024 at 15:41, Bob Weinand wrote:</span><br>
          <span></span><br>
          <blockquote type="cite"><span>Moreover, it can - at least - be
              worked around in tooling by special casing the
              T_YIELD_FROM token and extracting the comment from the raw
              parsed string:</span><br>
          </blockquote>
          <blockquote type="cite"><span></span><br>
          </blockquote>
          <blockquote type="cite"><span>var_dump(token_get_all('&lt;?php
              yield /* comment */ from $foo;'));</span><br>
          </blockquote>
          <blockquote type="cite"><span></span><br>
          </blockquote>
          <blockquote type="cite"><span>will contain:</span><br>
          </blockquote>
          <blockquote type="cite"><span></span><br>
          </blockquote>
          <blockquote type="cite"><span>[1]=&gt; array(3) { [0]=&gt;
              int(270) [1]=&gt; string(24) "yield /* comment */ from"
              [2]=&gt; int(1) }</span><br>
          </blockquote>
          <blockquote type="cite"><span></span><br>
          </blockquote>
          <blockquote type="cite"><span>It's not optimal, but probably
              the least bad solution to leave it unchanged in PHP 8.3,
              have tooling special case it and properly fix it in PHP
              8.4.</span><br>
          </blockquote>
          <span></span><br>
          <span>And what about "code" like
            <a class="moz-txt-link-rfc2396E" href="https://3v4l.org/4CLhM">&lt;https://3v4l.org/4CLhM&gt;</a>?  Is Codesniffer</span><br>
          <span>supposed to scan the result of
            <a class="moz-txt-link-rfc2396E" href="https://3v4l.org/dKDcs">&lt;https://3v4l.org/dKDcs&gt;</a> for possible CS</span><br>
          <span>violations?</span><br>
          <span></span><br>
          <span>Cheers,</span><br>
          <span>Christoph</span><br>
        </div>
      </blockquote>
      <br>
      <div>I suppose you mean <a moz-do-not-send="true"
          href="https://3v4l.org/IMi8Y">https://3v4l.org/IMi8Y</a>, (you
        missed the &lt;?php tag).</div>
      <div>If you want to scan that, it's quite easy to strip the
        leading yield and trailing from, and tokenize that again to
        extract all comments.</div>
      <div><br>
      </div>
      <div>Sure, it's a hack, but it'll work: <a moz-do-not-send="true"
          href="https://3v4l.org/8eAiV">https://3v4l.org/8eAiV</a>.</div>
      <div><br>
      </div>
      <div>Bob</div>
      <div><br>
      </div>
    </blockquote>
    <br>
    Hi Bob,<br>
    <br>
    Of course, everything can be hacked around, but that still leaves
    the question what should be the "proper tokenization". Having this
    change in PHP 8.3 and then - as you suggest - yet another in PHP
    8.4, makes it mighty hard to have a consistent token stream in
    tooling, especially as it is unclear what the "proper tokenization"
    should/would be.<br>
    <br>
    More than anything, I find it concerning that this change sets a
    precedent for tokens to include comments.<br>
    <br>
    Just as an example: what does this mean for the PHP 8.0 nullsafe
    object operator ? Should we now suddenly allow that to be written as
    `? /*comment*/ -&gt;` ?<br>
    Or what about a cast token ? Should that be allowed to be `(string
    /*for reasons*/)` ?<br>
    <br>
    Allowing this change to stay in, without having the discussion about
    what the "proper tokenization" should be, feels off and random to me
    and opens the door for more random changes.<br>
    <br>
    As for the impact on tooling: a change in the tokenization of any
    token has an impact not only on tooling like PHPCS itself, but also
    on every single external standard build on top of it and is a
    breaking change.<br>
    To give you some perspective - for PHPCS we even went as far as to
    "undo" the PHP 8.0 tokenization of namespaced names for the time
    being (in the PHPCS 3.x releases) and we'll only change the PHPCS
    tokenizer to use the PHP 8.0 tokenization in the PHPCS 4.0 release
    as it would otherwise break too many existing sniffs. [1]<br>
    <br>
    Smile,<br>
    Juliette<br>
    <br>
    1: <a class="moz-txt-link-freetext" href="https://github.com/squizlabs/PHP_CodeSniffer/issues/3041">https://github.com/squizlabs/PHP_CodeSniffer/issues/3041</a><br>
  </body>
</html>

--------------060003050400090207020206--