Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:124538
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>
 <6699F817.8070806@adviesenzo.nl>
 <9571bb82-9873-4319-9bd1-0361748335be@bastelstu.be>
 <669BDB00.70507@adviesenzo.nl>
 <fa7db246-1577-43b7-a117-c18a73140091@bastelstu.be>
 <669BE870.2050908@adviesenzo.nl>
 <CAPyj-LB538drkCdq2jbrAWuU5uTq_izW97ppMvN=X=xDPfCZUQ@mail.gmail.com>
Message-ID: <669D3A65.8020603@adviesenzo.nl>
Date: Sun, 21 Jul 2024 18:42:13 +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: <CAPyj-LB538drkCdq2jbrAWuU5uTq_izW97ppMvN=X=xDPfCZUQ@mail.gmail.com>
Content-Type: multipart/alternative;
 boundary="------------000906060505030007080509"
From: php-internals_nospam@adviesenzo.nl (Juliette Reinders Folmer)

This is a multi-part message in MIME format.
--------------000906060505030007080509
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit

On 21-7-2024 13:28, Ilija Tovilo wrote:

Hi Ilja,

>
> I agree that it's not very likely that many people started placing
> comments between yield and from, but certainly not impossible. The
> main problem with reverting this change is that such code will
> completely stop working. Patch updates are routinely installed on
> servers without additional testing, so it's very much possible that we
> would brick peoples production environments.
That can be said about any bug fix though. The space bar heating 
anecdote comes to mind.

> To compare the impact on PHP_CodeSniffer: From what I understand
> (please correct me if I'm wrong), there are really only two possible
> scenarios when encountering comments between yield and from:
>
> * PHP_CodeSniffer doesn't see the comment and acts as if it weren't
> there, and potentially removes it when fixing errors automatically.
> * PHP_CodeSniffer thinks this is a syntax error and thus reports it.

Actually, the impact is a bit larger than that (false 
positives/negatives for indentation checks, annotations being ignored, 
comments not being checked etc), but that's a different discussion.

The more problematic impact is that this undocumented change in 
tokenization can currently cause the autofixer of PHPCS to **create** 
parse errors in code.

> While that's not optimal, it is limited to a subset of the already
> small set of people using this new feature, namely the ones also using
> PHP_CodeSniffer. It is also limited to development environments, where
> PHP_CodeSniffer is executed.

While I understand the interest in minimizing the impact, there are 
millions of people using PHP_CodeSniffer every day.

> In the likely case of not finding consensus on this issue: Setting up
> a vote doesn't seem straight-forward to me either. Would we consider
> 8.3 the baseline, with reverting the change requiring the 2/3
> majority, or do we consider 8.2 the baseline?

For a potential vote, I would think more along the lines of consistency 
rules, i.e. "what is allowed in a token" (whitespace, new lines, 
comments) ? And then, depending on the proposal, what tokens would be 
impacted if this is changed (namespaced names, cast tokens, 
heredoc/nowdoc openers, yield from) and how will this impact these tokens ?

Smile,
Juliette

--------------000906060505030007080509
Content-Type: text/html; charset=utf-8
Content-Transfer-Encoding: 7bit

<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 21-7-2024 13:28, Ilija Tovilo wrote:<br>
      <br>
      Hi Ilja,<br>
      <br>
    </div>
    <blockquote
cite="mid:CAPyj-LB538drkCdq2jbrAWuU5uTq_izW97ppMvN=X=xDPfCZUQ@mail.gmail.com"
      type="cite"><br>
      <pre wrap="">I agree that it's not very likely that many people started placing
comments between yield and from, but certainly not impossible. The
main problem with reverting this change is that such code will
completely stop working. Patch updates are routinely installed on
servers without additional testing, so it's very much possible that we
would brick peoples production environments.</pre>
    </blockquote>
    That can be said about any bug fix though. The space bar heating
    anecdote comes to mind.<br>
    <br>
    <blockquote
cite="mid:CAPyj-LB538drkCdq2jbrAWuU5uTq_izW97ppMvN=X=xDPfCZUQ@mail.gmail.com"
      type="cite">
      <pre wrap="">To compare the impact on PHP_CodeSniffer: From what I understand
(please correct me if I'm wrong), there are really only two possible
scenarios when encountering comments between yield and from:

* PHP_CodeSniffer doesn't see the comment and acts as if it weren't
there, and potentially removes it when fixing errors automatically.
* PHP_CodeSniffer thinks this is a syntax error and thus reports it.</pre>
    </blockquote>
    <br>
    Actually, the impact is a bit larger than that (false
    positives/negatives for indentation checks, annotations being
    ignored, comments not being checked etc), but that's a different
    discussion.<br>
    <br>
    The more problematic impact is that this undocumented change in
    tokenization can currently cause the autofixer of PHPCS to
    **create** parse errors in code.<br>
    <br>
    <blockquote
cite="mid:CAPyj-LB538drkCdq2jbrAWuU5uTq_izW97ppMvN=X=xDPfCZUQ@mail.gmail.com"
      type="cite">
      <pre wrap="">While that's not optimal, it is limited to a subset of the already
small set of people using this new feature, namely the ones also using
PHP_CodeSniffer. It is also limited to development environments, where
PHP_CodeSniffer is executed.</pre>
    </blockquote>
    <br>
    While I understand the interest in minimizing the impact, there are
    millions of people using PHP_CodeSniffer every day.<br>
    <br>
    <blockquote
cite="mid:CAPyj-LB538drkCdq2jbrAWuU5uTq_izW97ppMvN=X=xDPfCZUQ@mail.gmail.com"
      type="cite">
      <pre wrap="">In the likely case of not finding consensus on this issue: Setting up
a vote doesn't seem straight-forward to me either. Would we consider
8.3 the baseline, with reverting the change requiring the 2/3
majority, or do we consider 8.2 the baseline?</pre>
    </blockquote>
    <br>
    For a potential vote, I would think more along the lines of
    consistency rules, i.e. "what is allowed in a token" (whitespace,
    new lines, comments) ? And then, depending on the proposal, what
    tokens would be impacted if this is changed (namespaced names, cast
    tokens, heredoc/nowdoc openers, yield from) and how will this impact
    these tokens ?<br>
    <br>
    Smile,<br>
    Juliette<br>
  </body>
</html>

--------------000906060505030007080509--