Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread David Gebler 
On Mon, Dec 12, 2022 at 10:20 PM Dan Liebner  wrote:

> As the RFC points out, "Some languages, such as JavaScript, do not consider
> accesses to undefined array keys to be an error condition at all, and allow
> such an operation to be performed silently."
>

It is by definition an error condition though. You're trying to access
something which doesn't exist.


>
> For the last 20 years, it's been my impression that PHP also embraced this
> philosophy, and that raising an E_NOTICE for "Undefined index" was more or
> less offered as a debugging tool, as a matter of practicality.
>

In the earlier days of PHP, it was more acceptable to allow garbage in
scripts. Generally, the community consensus has been that trending away
from that has been a positive.

It's strongly arguable that E_NOTICE was never appropriate for undefined
array index access, since it is not really something which can occur in
"the normal course of running a script" (since it necessarily means your
script is broken).


>
> JavaScript returns the undefined primitive for accesses to undefined array
> and object keys. PHP returns the NULL primitive respectively despite
> raising an error.
>
> Here's my biggest criticism: the change essentially forces this:
> $arr['key']
>
> to be rewritten as this (or some other intolerably bad alternative):
> isset($arr) && isset($arr['key']) && $arr['key']
>
> This is a major regression in my opinion as far as productivity and
> readability are concerned.
>

It seems strange to quibble about readability and syntax when you're
talking about a situation in your code which is by definition an error
condition. Accessing something which doesn't exist isn't the same as
accessing something which does exist and it not having a value (i.e. the
value is a null).

Null = "Can I have the address for this customer please?", "No sorry, we
don't have an address on file for them.
"Undefined = "Can I have the gooble-gabble for this customer please?", "The
what?"

But sure, we don't always have a guarantee about the structure of some
data, especially in web where input usually comes from an HTTP request
which may or may not contain anything. But in those situations we can check
instead of making assumptions.

Good = "Hey, just checking, do we have something called gooble-gabble on
file for this customer?", "Sure, here it is" for one customer, "No, sorry
that's not a thing" for another (this is your array_key_exists / isset /
null coalesce / whatever)

Bad = "Can I have the gooble-gabble for this customer please?", "Sure, here
it is" for one customer, "The what?" for another


>
> It has been proposed to make the error level of "Undefined index"
> configurable so that teams and individual developers can decide for
> themselves how they want this situation to be handled.


There's a much better way of allowing developers to decide for themselves
how to handle the gooble-gabble situation in a future major release; have
undefined index access throw an UndefinedIndexError which can be caught.

Then we can say

"Can I have the gooble-gabble for this customer please? And if
gooble-gabble isn't a thing, no problem, I can deal with that"

That's my two cents, cheers.

Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread Derick Rethans 
On 13 December 2022 15:55:41 GMT, Thomas Hruska  wrote:
>On 12/13/2022 7:15 AM, Derick Rethans wrote:
>> On Mon, 12 Dec 2022, Thomas Hruska wrote:
>> 
>>> On 12/12/2022 3:52 PM, Derick Rethans wrote:
 On 12 December 2022 22:20:27 GMT, Dan Liebner  wrote:
 
> It has been proposed to make the error level of "Undefined index"
> configurable so that teams and individual developers can decide
> for themselves how they want this situation to be handled. Given
> that:
> 
> - PHP has been treating this as an E_NOTICE for over 20 years
 
 But not in the last three years.
>>> 
>> 
>> 
>>> The type of post that the OP sent to internals is likely to become
>>> more common in the next few months as PHP 8.x starts rolling out
>>> globally.
>> 
>> PHP 8 has been rolling out for two years "globally" already.
>
>Not for those of us that run the package managed version of PHP in Ubuntu 
>Server LTS it hasn't.

That's your and their choice, and not a problem that we here should care about. 
I certainly don't. 

There are also well established third party repositories (sury and remi) to 
cater for this use case as well.

From the PHP project's point of view, PHP 8 has been out for more than two 
years. 


cheers 
Derick 

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread Dan Liebner 
> Can you expand a bit on how you think distinguishing "undefined" from
"null" would help?

First off, I would be perfectly happy if undefined array/object index
accesses went back to raising E_NOTICE. Undefined variables already resolve
to NULL so in that sense NULL is already the "undefined" primitive.

That said, if PHP were to introduce an `undefined` primitive, here's what I
think would change:
- NULL remains, and as such represents an "empty", yet defined, value.
- Undefined variables and array/object accesses now resolve to "undefined"
- `$var = undefined` is the equivalent of calling unset($var)
- Biggest breaking change would probably be `$undefinedVar === null` is no
longer true

prop; // undefined, no error

$undy->method(); // fatal error

function($param = undefined) {} // fatal error

// more difficult scenarios...
// JavaScript would throw an error
// classic PHP behavior would resolve to null, therefore now undefined
// either is arguably valid, latter would create need for optional chaining
$undy['key']; // ?
$undy->prop; // ?

Re: [PHP-DEV] Remove PHP-x.y.* git branches

2022-12-13 Thread Tim Düsterhus 

Hi

On 12/13/22 18:55, Sara Golemon wrote:

Why do you want to remove these branches?

I agree that they have minimal value especially since the tags are the
actual release commit (and for release process versions often represent a
spur off the release branch), but the cost in the repo is negligible.  What
is gained from removing information from the repository?


One benefit of removing those branches would be, that usability of 
GitHub's branch selector improves, specifically the branch selector when 
creating a new PR.


Currently one needs to know the exact naming scheme and then use the 
"Find a branch" input to efficiently find a specific target branch. With 
the extra branches removed, one can open the dropdown and simply click 
the correct branch without needing to think.


Best regards
Tim Düsterhus

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Remove PHP-x.y.* git branches

2022-12-13 Thread Sara Golemon 
On Tue, Dec 13, 2022 at 11:31 AM Michael Voříšek - ČVUT FJFI <
voris...@fjfi.cvut.cz> wrote:

> Hello everyone,
>
> I am the author of https://github.com/php/php-src/issues/10007 proposal
> and I would ask you for the green light to do so.
>
>
Why do you want to remove these branches?

I agree that they have minimal value especially since the tags are the
actual release commit (and for release process versions often represent a
spur off the release branch), but the cost in the repo is negligible.  What
is gained from removing information from the repository?

-Sara

[PHP-DEV] Re: Remove PHP-x.y.* git branches

2022-12-13 Thread Ben Ramsey 

On 12/13/22 11:30, Michael Voříšek - ČVUT FJFI wrote:

Hello everyone,
I am the author of https://github.com/php/php-src/issues/10007 proposal
and I would ask you for the green light to do so.
There are two kinds of unusefull branches:
a) the PHP-x-y-* branches - I would restrict the removal of branches
that HEAD commit matches git tag HEAD-1 commit
In theory the branches can be used somewhere, but they are pointless as
git tag HEAD-1 is a 1:1 replacement.
No data will/can be lost (and in theory, such branches can be restored
anytime locally).
b) the 10+ years old branches as mentioned in
https://externals.io/message/102982
I do not expect these branches to be used anywhere.
If 20 years old contribution is expected to be finished, the author
should have a local git/svn copy and open regular PR for it :)
Can we collectively agree?
With kind regards / Mit freundlichen Grüßen / S přátelským pozdravem,

Michael Voříšek


Release managers use these these branches for packaging RCs and 
incorporating any critical bug/security fixes between the RC and GA release.


See “Packaging a non-stable release (alpha/beta/RC),” step 3, here: 
https://github.com/php/php-src/blob/master/docs/release-process.md#packaging-a-non-stable-release-alphabetarc


To recommend this change, you will need also need to recommend changes 
to the release process.


--
Cheers,
Ben

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

[PHP-DEV] Remove PHP-x.y.* git branches

2022-12-13 Thread Michael Voříšek - ČVUT FJFI 
Hello everyone, 


I am the author of https://github.com/php/php-src/issues/10007 proposal
and I would ask you for the green light to do so. 

There are two kinds of unusefull branches: 


a) the PHP-x-y-* branches - I would restrict the removal of branches
that HEAD commit matches git tag HEAD-1 commit 


In theory the branches can be used somewhere, but they are pointless as
git tag HEAD-1 is a 1:1 replacement. 


No data will/can be lost (and in theory, such branches can be restored
anytime locally). 


b) the 10+ years old branches as mentioned in
https://externals.io/message/102982 

I do not expect these branches to be used anywhere. 


If 20 years old contribution is expected to be finished, the author
should have a local git/svn copy and open regular PR for it :) 

Can we collectively agree? 


With kind regards / Mit freundlichen Grüßen / S přátelským pozdravem,

Michael Voříšek

Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread Craig Francis 
On 13 Dec 2022, at 15:45, Rowan Tommins  wrote:
> Although presumably they return null rather than an empty string precisely so 
> that users can check if the value was provided, without providing an extra 
> method equivalent to isset($_GET['q']), e.g.
> 
> [...]
> 
> For cases where you don't need that distinction, Laravel, Symfony, and 
> CakePHP all allow a default to be passed as the second parameter.


Yep, that's right... but it's not easy to know if that distinction is needed 
for every single variable (why it's easier to work around this issue at the 
sinks, as Rector now does).

I assume the next step is for PHP is to deprecate null coercion for all 
contests; e.g. concat ('Search: ' . $search), comparisons ('' != null), 
arithmetic, and the remaining functions like print()/echo()/sprintf('%s')... 
and then deprecate all other forms of coercion (e.g. 5 + '3')... because that 
will be fun :-)

Craig

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] [RFC] Asymmetric Visibility, with readonly

2022-12-13 Thread Claude Pache 


> Le 13 déc. 2022 à 16:34, Claude Pache  a écrit :
> 
> 
> Hi,
> 
> As of today, the following declarations are rejected as syntax errors:
> 
> ```php
> class C {
> $a;
> int $b;
> }
> ```
> 
> while the following declarations are accepted with implicit `public` 
> visibility:
> 
> ```php
> class C {
> static $c;
> readonly int $d;
> }
> ```
> 
> It would be reasonable to propose to allow to consistently omit the `public` 
> keyword. But allowing to omit it in some cases (including the most 
> controversial one: `protected(set)`) and not in other cases...? Because of 
> this inconsistency, people are incited to always write explicitly `public` 
> anyway.
> 
> —Claude


However, I’m just realising that omitting `public` in declarations like `public 
$a` and `public int $b` is probably not a good idea, because it is incompatible 
with constructor property promotion, as `function __construct(public int $b) { 
}`, and `function __construct(int $b) { }` have different meanings.

—Claude

Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread Rowan Tommins 

On 13/12/2022 16:08, Dan Liebner wrote:

Can I also just point out that by current definitions the "null coalescing
operator" isn't properly named, it should be the "undefined/null coalescing
operator". The only reason it is able to get away with not raising an error
for undefined variables is that it's described as "syntactic sugar" for
isset(), which is itself the exception to the rule for not raising
undefined errors.Other languages, such as JavaScript, would raise an error
for an expression such as `undefinedvar ?? 0`. And JavaScript conveniently
offers the undefined primitive for precisely handling attempted accesses of
undefined keys, while retaining the brevity and convenience of a
truthy/falsy `obj.key` check.



Can you expand a bit on how you think distinguishing "undefined" from 
"null" would help? Let's keep the focus on possible solutions, not just 
arguing in circles.


It's always been a source of confusion to me that JS has both, and the 
syntax for working them seems far from elegant (unless things have 
improved, and you no longer need to use typeof to detect undefined?); 
but maybe I'm looking at it wrong.


Regards,

--
Rowan Tommins
[IMSoP]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread Larry Garfield 
On Tue, Dec 13, 2022, at 10:39 AM, Deleu wrote:

>> And yes, I used to work on a system that didn't do that properly.  (TYPO3,
>> ~800,000 LOC)  It took me a few weeks, but we still managed to fix all of
>> these issues in mostly one-person-month, mostly with dropping "?? null"
>> around in various places.  It's not the world-destroyer people make it out
>> to be.
>>
>
> Do you honestly think that everyone with a small/medium PHP project
> (500k~1M LOC) out there has someone as competent as you capable of
> achieving what you did? That is definitely a dream rock to live under.

In all modesty, 95% of the changes I made to get TYPO3 ready for 8.0 could have 
been made by a junior dev with a checklist and a few patterns to copy and 
paste.  "Run tests, look at logs, fix a few lines, repeat."  It was hardly the 
most intellectually stimulating work I've done.

--Larry Garfield

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread Deleu 
>
> Anyone surprised by any of the changes in recent versions that boil down
> to "know your types and know if you actually have a variable" has been
> living under a rock for the last 20 years.  None of this has been a
> secret.  The language has been providing more and more tools to
> know-your-types for years.
>

I have seen this type of comment here many times and I don't understand why
it keeps being repeated. The world is made up of many rocks and we're all
living under one of them. This whole thread is nothing but
my-rock-is-the-only-rock-everyone-else-should-live-under type of comments.


> And yes, I used to work on a system that didn't do that properly.  (TYPO3,
> ~800,000 LOC)  It took me a few weeks, but we still managed to fix all of
> these issues in mostly one-person-month, mostly with dropping "?? null"
> around in various places.  It's not the world-destroyer people make it out
> to be.
>

Do you honestly think that everyone with a small/medium PHP project
(500k~1M LOC) out there has someone as competent as you capable of
achieving what you did? That is definitely a dream rock to live under.

-- 
Marco Deleu

Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread Larry Garfield 
On Tue, Dec 13, 2022, at 9:55 AM, Thomas Hruska wrote:
> On 12/13/2022 7:15 AM, Derick Rethans wrote:
>> On Mon, 12 Dec 2022, Thomas Hruska wrote:
>> 
>>> On 12/12/2022 3:52 PM, Derick Rethans wrote:
 On 12 December 2022 22:20:27 GMT, Dan Liebner  wrote:

> It has been proposed to make the error level of "Undefined index"
> configurable so that teams and individual developers can decide
> for themselves how they want this situation to be handled. Given
> that:
>
> - PHP has been treating this as an E_NOTICE for over 20 years

 But not in the last three years.
>>>
>> 
>> 
>>> The type of post that the OP sent to internals is likely to become
>>> more common in the next few months as PHP 8.x starts rolling out
>>> globally.
>> 
>> PHP 8 has been rolling out for two years "globally" already.
>> 
>> cheers,
>> Derick
>
> Not for those of us that run the package managed version of PHP in 
> Ubuntu Server LTS it hasn't.
>
> https://wiki.ubuntu.com/Releases
>
> PHP 8.x is brand new as of *next year* from the perspective of those who 
> patiently wait for certain OS level updates because overall system 
> stability is important for critical production servers.  The x.x.2 for 
> the Ubuntu LTS series tends to be sufficiently stable for most 
> environments but 22.04.2 hasn't been released yet.  Hence my earlier 
> comment.

Lagging behind on distro LTS releases for production is one thing.  But if the 
dev team and/or their managers aren't even *looking* at PHP 8 for 2 years, and 
then getting shocked at *deprecations* or known-bad-practices like undefined 
vars becoming a louder message, that's entirely on them.

Developing under E_ALL has been the recommendation for... 15 years?  20 years?  
If someone has been doing that at any point since George W. Bush's first term, 
the undefined vars error messages would have long-since been fixed and 8.0 
would be a cakewalk.

Anyone surprised by any of the changes in recent versions that boil down to 
"know your types and know if you actually have a variable" has been living 
under a rock for the last 20 years.  None of this has been a secret.  The 
language has been providing more and more tools to know-your-types for years.

And yes, I used to work on a system that didn't do that properly.  (TYPO3, 
~800,000 LOC)  It took me a few weeks, but we still managed to fix all of these 
issues in mostly one-person-month, mostly with dropping "?? null" around in 
various places.  It's not the world-destroyer people make it out to be.

--Larry Garfield

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread Dan Liebner 
Can I also just point out that by current definitions the "null coalescing
operator" isn't properly named, it should be the "undefined/null coalescing
operator". The only reason it is able to get away with not raising an error
for undefined variables is that it's described as "syntactic sugar" for
isset(), which is itself the exception to the rule for not raising
undefined errors. Other languages, such as JavaScript, would raise an error
for an expression such as `undefinedvar ?? 0`. And JavaScript conveniently
offers the undefined primitive for precisely handling attempted accesses of
undefined keys, while retaining the brevity and convenience of a
truthy/falsy `obj.key` check.

If you're set on raising errors and eventually exceptions for undefined key
accesses, we need a concise, performant way to say `isset($varExpression) ?
$varExpression : null` that's still way more concise than `$varExpression
?? null` that's baked into the language.

On Tue, Dec 13, 2022 at 7:53 AM Dan Liebner  wrote:

> > No, code doesn't break. It now shows a warning instead of an error.
> There is no behavioural change.
> It breaks my app. Does that count? And didn't you follow up by referencing
> this as "adding settings that change behaviour"? Does it change behavior or
> not?
>
> > Adding a configuration setting to make this a notice on some
> installations and not on others, would just mean that anything that needs
> to be able to run everywhere needs to take care to not rely on the setting
> either way, making it harder to develop portable code.
> What code now relies on this raising a warning rather than a notice that
> can't consistently deal with it by using isset or the null coalescing
> operator? Or are you subtly referring to the fact that this RFC is a
> stepping stone to escalating the severity to throwing an error?
> https://wiki.php.net/rfc/undefined_variable_error_promotion -> "This RFC
> proposes that accessing an undefined variable is rendered illegal behaviour
> in the next major version of PHP, and will result in an Error exception
> being thrown if it occurs."
>
> Here's another suggestion:
> Make accesses to undefined array keys (objects, variables, etc) return a
> new `undefined` primitive. That way, developers who are focused on writing
> concise, readable code can continue to write and maintain code manageably,
> and developers who are keen on writing a lot of code in a more "strict"
> manner can handle undefined array key accesses consistently, without having
> to rely on configuration settings.
>
> > just fix your code.
> Practically speaking, I'd much more likely stay on 7.4 or migrate to Node.
>
>
> On Mon, Dec 12, 2022 at 5:52 PM Derick Rethans  wrote:
>
>> On 12 December 2022 22:20:27 GMT, Dan Liebner  wrote:
>>
>> >It has been proposed to make the error level of "Undefined index"
>> >configurable so that teams and individual developers can decide for
>> >themselves how they want this situation to be handled. Given that:
>> >
>> >   - PHP has been treating this as an E_NOTICE for over 20 years
>>
>> But not in the last three years.
>>
>> >   - the change is a breaking change affecting many large codebases
>> >   ubiquitously (unless one were to unadvisedly suppress E_WARNING
>> errors)
>>
>> No, code doesn't break. It now shows a warning instead of an error. There
>> is no behavioural change.
>>
>> >   - 7.4 is now deprecated
>>
>> Yes, as each release does 2+1 years. Which also means you've had 3 years
>> to address this in your code base(s) already.
>>
>> >I think making the error level of "Undefined index" configurable is a
>> very
>> >reasonable suggestion, and I support it.
>>
>> Adding a configuration setting to make this a notice on some
>> installations and not on others, would just mean that anything that needs
>> to be able to run everywhere needs to take care to not rely on the setting
>> either way, making it harder to develop portable code.
>>
>> We're also not generally anything near keen on adding settings that
>> change behaviour , and suggesting to add one to make a warning a notice
>> seems very far short of a bar that needs to be reached before many of us
>> would agree to add a setting to make PHP less portable.
>>
>> Alternatively you can just fix your code.
>>
>> >Are we able to revisit this topic as a community and potentially bring in
>> >more PHP developers from around the world to weigh in?
>>
>> You're always free to follow the RFC process, but I think you'll just up
>> wasting your, and everybody else's, time with it.
>>
>> I can't see this being reversed, nor a setting added for, through an RFC
>> process.
>>
>> cheers
>> Derick
>>
>>

Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread Lynn 
On Tue, Dec 13, 2022 at 4:55 PM Thomas Hruska 
wrote:

>
> Not for those of us that run the package managed version of PHP in
> Ubuntu Server LTS it hasn't.
>
> https://wiki.ubuntu.com/Releases
>
> PHP 8.x is brand new as of *next year* from the perspective of those who
> patiently wait for certain OS level updates because overall system
> stability is important for critical production servers.  The x.x.2 for
> the Ubuntu LTS series tends to be sufficiently stable for most
> environments but 22.04.2 hasn't been released yet.  Hence my earlier
> comment.
>

Lagging behind like that is a choice though.

Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread Thomas Hruska 

On 12/13/2022 7:15 AM, Derick Rethans wrote:

On Mon, 12 Dec 2022, Thomas Hruska wrote:


On 12/12/2022 3:52 PM, Derick Rethans wrote:

On 12 December 2022 22:20:27 GMT, Dan Liebner  wrote:


It has been proposed to make the error level of "Undefined index"
configurable so that teams and individual developers can decide
for themselves how they want this situation to be handled. Given
that:

- PHP has been treating this as an E_NOTICE for over 20 years


But not in the last three years.






The type of post that the OP sent to internals is likely to become
more common in the next few months as PHP 8.x starts rolling out
globally.


PHP 8 has been rolling out for two years "globally" already.

cheers,
Derick


Not for those of us that run the package managed version of PHP in 
Ubuntu Server LTS it hasn't.


https://wiki.ubuntu.com/Releases

PHP 8.x is brand new as of *next year* from the perspective of those who 
patiently wait for certain OS level updates because overall system 
stability is important for critical production servers.  The x.x.2 for 
the Ubuntu LTS series tends to be sufficiently stable for most 
environments but 22.04.2 hasn't been released yet.  Hence my earlier 
comment.


--
Thomas Hruska
CubicleSoft President

CubicleSoft has over 80 original open source projects and counting.
Plus a couple of commercial/retail products.

What software are you looking to build?

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread Rowan Tommins 

On 13/12/2022 14:32, Craig Francis wrote:

Most frameworks return NULL when the user has not provided the value:

 $search = $request->input('q'); // Laravel
 $search = $request->get('q'); // Symfony
 $search = $this->request->getQuery('q'); // CakePHP
 $search = $request->getGet('q'); // CodeIgniter



Fair enough.

Although presumably they return null rather than an empty string 
precisely so that users can check if the value was provided, without 
providing an extra method equivalent to isset($_GET['q']), e.g.


if ( $search === null ) {
    render_search_form();
}
elseif ( trim($search) === '' ) {
    show_validation_error();
}
else {
    perform_search($search);
}


For cases where you don't need that distinction, Laravel, Symfony, and 
CakePHP all allow a default to be passed as the second parameter.



Regards,

--
Rowan Tommins
[IMSoP]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] [RFC] Asymmetric Visibility, with readonly

2022-12-13 Thread Claude Pache 


> Le 11 déc. 2022 à 20:18, Larry Garfield  a écrit :
> 
> On Thu, Dec 1, 2022, at 12:31 PM, Tim Düsterhus wrote:
>> Hi
>> 
>> On 11/29/22 21:29, Larry Garfield wrote:
>>> Thank you everyone for the feedback.  Based on this thread, we've made two 
>>> changes to the RFC:
>>> 
>>> 1. We've moved readonly back to forbidden with a-viz for now.  I've added a 
>>> section to Future Scope where we really should sort this out in the future, 
>>> but we'll do that in the future when we can all focus on the various 
>>> nuances of just that piece.
>>> 
>>> 2. I rewrote the section on __set to make it clearer.  That also included 
>>> Ilija and I digging into all the nuances that are already present.  The 
>>> text may still look a bit complex, but that's because the existing logic is 
>>> already complex with readonly.  Long story short, the a-viz RFC does not 
>>> change anything in the way __set works vis a vis asymmetric visibility; it 
>>> just inherits and continues what readonly already started, so it's 
>>> consistent.
>>> 
>>> The PR should be updated in the next week or two with the latest changes.  
>>> Baring any major need for change, we expect to call a vote for it shortly 
>>> after New Years.
>>> 
>> 
>> Okay, then I'd like to officially "request" that the abbreviated form 
>> [1] is dropped:
>> 
>> I believe 'protected(set) string $foo' is easily confused with 
>> 'protected string $foo' at a simple glance.
>> 
>> Also any implicit rules are something developers will need to learn by 
>> heart, whereas an explicit 'public protected(set) string $foo' could 
>> reasonably be understood by someone without any PHP experience and some 
>> basic experience of OO concepts.
>> 
>> Having two separate explicit keywords also makes it much clearer that 
>> asymmetric visibility is involved, because it's also asymmetric in the code.
>> 
>> I believe the only benefit of the abbreviated form is saving 6 
>> keystrokes (+ one hit to the spacebar) and I don't believe it's worth 
>> the lack of clarity for an important property of the defined property.
>> 
>> Best regards
>> Tim Düsterhus
>> 
>> [1] https://wiki.php.net/rfc/asymmetric-visibility#abbreviated_form
> 
> 
> Does anyone else have feelings on this point?  IMO, the shorthand makes a lot 
> of sense when used with readonly to avoid lines getting just annoyingly long, 
> but without it I can see the argument for not allowing it; it's about a wash 
> in terms of length with readonly today.  I'm comfortable going with the 
> consensus on this one for now.
> 
> --Larry Garfield

Hi,

As of today, the following declarations are rejected as syntax errors:

``php
class C {
$a;
int $b;
}
```

while the following declarations are accepted with implicit `public` visibility:

``php
class C {
static $c;
readonly int $d;
}
```

It would be reasonable to propose to allow to consistently omit the `public` 
keyword. But allowing to omit it in some cases (including the most 
controversial one: `protected(set)`) and not in other cases...? Because of this 
inconsistency, people are incited to always write explicitly `public` anyway.

—Claude

Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread Andreas Leathley 

On 13.12.22 13:53, Dan Liebner wrote:

It breaks my app. Does that count?


This sounds like you completely ignore notices in your application yet
elevate warnings to exceptions/errors. It would be better to log both
notices and warnings without generating exceptions/errors, and look
through those logs from time to time, to see possible issues (and
nothing would currently break that way). I used to suppress notices in
my applications too, but after logging them it was my experience that
notices can usually be easily avoided and often hint at oversights /
possible improvements.


Here's another suggestion:
Make accesses to undefined array keys (objects, variables, etc) return a
new `undefined` primitive. That way, developers who are focused on writing
concise, readable code can continue to write and maintain code manageably,
and developers who are keen on writing a lot of code in a more "strict"
manner can handle undefined array key accesses consistently, without having
to rely on configuration settings.


That would likely be a major change and break in the language (which is
usually frowned upon), but if you have a clear concept, you can create
an RFC for it and start a discussion.

I do think that your view on "concise, readable code" would in my mind
be code which is more prone to bugs and unclear in its intentions. If an
array index possibly does not exist and you check its value, it would
already be unclear to me if you mean to check for its existence, or for
a null value, or for a non-empty string, or something else. Making it
clear the index might not exist gives a reader more information, and
making the expected types (and checks) clearer in general is not an
exercise to be strict for fun, but to avoid bugs because of unexpected
values.

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread Craig Francis 
On 13 Dec 2022, at 12:39, Rowan Tommins  wrote:
> On 13/12/2022 10:11, Craig Francis wrote:
>> The null value can come from many sources (e.g. GET/POST/COOKIE/databases)
> 
> 
> These two examples are interesting in conjunction: $_GET, $_POST, and 
> $_COOKIE will never contain null values unless you have code writing directly 
> to them, because HTTP has no representation for it, only empty strings.



Most frameworks return NULL when the user has not provided the value:

$search = $request->input('q'); // Laravel
$search = $request->get('q'); // Symfony
$search = $this->request->getQuery('q'); // CakePHP
$search = $request->getGet('q'); // CodeIgniter

This is also common, to avoid undefined indexes (as you suggested earlier):

$search = ($_GET['q'] ?? NULL);

And some developers use:

$search = filter_input(INPUT_GET, 'q');

That said, I'm glad you have found a positive example from this change.

Craig

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread Derick Rethans 
On Mon, 12 Dec 2022, Thomas Hruska wrote:

> On 12/12/2022 3:52 PM, Derick Rethans wrote:
> > On 12 December 2022 22:20:27 GMT, Dan Liebner  wrote:
> > 
> > > It has been proposed to make the error level of "Undefined index" 
> > > configurable so that teams and individual developers can decide 
> > > for themselves how they want this situation to be handled. Given 
> > > that:
> > > 
> > >- PHP has been treating this as an E_NOTICE for over 20 years
> > 
> > But not in the last three years.
> 


> The type of post that the OP sent to internals is likely to become 
> more common in the next few months as PHP 8.x starts rolling out 
> globally.

PHP 8 has been rolling out for two years "globally" already.

cheers,
Derick
-- 
https://derickrethans.nl | https://xdebug.org | https://dram.io

Author of Xdebug. Like it? Consider supporting me: https://xdebug.org/support
Host of PHP Internals News: https://phpinternals.news

mastodon: @derickr@phpc.social @xdebug@phpc.social
twitter: @derickr and @xdebug

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread Derick Rethans 
On Mon, 12 Dec 2022, Rowan Tommins wrote:

> On 12 December 2022 22:20:27 GMT, Dan Liebner  wrote:
> >Here's my biggest criticism: the change essentially forces this:
> >$arr['key']
> >
> >to be rewritten as this (or some other intolerably bad alternative):
> >isset($arr) && isset($arr['key']) && $arr['key']
> 
> 
> Fortunately, this is not the case, the actual replacement code is this:
> 
> $arr['key'] ?? null

Or before ?? was a thing, like:

filter_input(INPUT_GET, 'key')

cheers,
Derick

-- 
https://derickrethans.nl | https://xdebug.org | https://dram.io

Author of Xdebug. Like it? Consider supporting me: https://xdebug.org/support
Host of PHP Internals News: https://phpinternals.news

mastodon: @derickr@phpc.social @xdebug@phpc.social
twitter: @derickr and @xdebug

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread Rowan Tommins 

On 13/12/2022 12:53, Dan Liebner wrote:

It breaks my app. Does that count?



The only way a Warning can break an application is if you're running a 
custom handler which promotes it to an error. If that is your case, then 
blaming PHP is a little bit like programming your car to do an emergency 
stop at every road sign, then complaining to the highways agency when 
someone runs into the back of you.


On the other hand, in keeping with my earlier suggestion of making valid 
use cases easy, PHP could help by making it easier to filter messages in 
that callback. At the moment, you'd have to match the content of the 
message, which is not ideal; a unique code for each message type would 
be much nicer, but there's potentially quite a lot of initial work to 
assign codes to all the existing messages.


That way, you could promote some warnings to errors, and ignore others; 
or log them with your own choice of severity, possibly not even limiting 
yourself to those that PHP provides.


Of course, this won't help in the long run for things which eventually 
do become errors, but for everything below that level, it would be much 
more powerful than a one-off setting to change the severity of one 
specific message.


Regards,

--
Rowan Tommins
[IMSoP]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread Dan Liebner 
> No, code doesn't break. It now shows a warning instead of an error. There
is no behavioural change.
It breaks my app. Does that count? And didn't you follow up by referencing
this as "adding settings that change behaviour"? Does it change behavior or
not?

> Adding a configuration setting to make this a notice on some
installations and not on others, would just mean that anything that needs
to be able to run everywhere needs to take care to not rely on the setting
either way, making it harder to develop portable code.
What code now relies on this raising a warning rather than a notice that
can't consistently deal with it by using isset or the null coalescing
operator? Or are you subtly referring to the fact that this RFC is a
stepping stone to escalating the severity to throwing an error?
https://wiki.php.net/rfc/undefined_variable_error_promotion -> "This RFC
proposes that accessing an undefined variable is rendered illegal behaviour
in the next major version of PHP, and will result in an Error exception
being thrown if it occurs."

Here's another suggestion:
Make accesses to undefined array keys (objects, variables, etc) return a
new `undefined` primitive. That way, developers who are focused on writing
concise, readable code can continue to write and maintain code manageably,
and developers who are keen on writing a lot of code in a more "strict"
manner can handle undefined array key accesses consistently, without having
to rely on configuration settings.

> just fix your code.
Practically speaking, I'd much more likely stay on 7.4 or migrate to Node.


On Mon, Dec 12, 2022 at 5:52 PM Derick Rethans  wrote:

> On 12 December 2022 22:20:27 GMT, Dan Liebner  wrote:
>
> >It has been proposed to make the error level of "Undefined index"
> >configurable so that teams and individual developers can decide for
> >themselves how they want this situation to be handled. Given that:
> >
> >   - PHP has been treating this as an E_NOTICE for over 20 years
>
> But not in the last three years.
>
> >   - the change is a breaking change affecting many large codebases
> >   ubiquitously (unless one were to unadvisedly suppress E_WARNING errors)
>
> No, code doesn't break. It now shows a warning instead of an error. There
> is no behavioural change.
>
> >   - 7.4 is now deprecated
>
> Yes, as each release does 2+1 years. Which also means you've had 3 years
> to address this in your code base(s) already.
>
> >I think making the error level of "Undefined index" configurable is a very
> >reasonable suggestion, and I support it.
>
> Adding a configuration setting to make this a notice on some installations
> and not on others, would just mean that anything that needs to be able to
> run everywhere needs to take care to not rely on the setting either way,
> making it harder to develop portable code.
>
> We're also not generally anything near keen on adding settings that change
> behaviour , and suggesting to add one to make a warning a notice seems very
> far short of a bar that needs to be reached before many of us would agree
> to add a setting to make PHP less portable.
>
> Alternatively you can just fix your code.
>
> >Are we able to revisit this topic as a community and potentially bring in
> >more PHP developers from around the world to weigh in?
>
> You're always free to follow the RFC process, but I think you'll just up
> wasting your, and everybody else's, time with it.
>
> I can't see this being reversed, nor a setting added for, through an RFC
> process.
>
> cheers
> Derick
>
>

Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread Rowan Tommins 

On 13/12/2022 10:11, Craig Francis wrote:

undefined indexes are a classic, but once you've found them all (the tricky 
bit), it often makes your code better, and helps identify mistakes (similar to 
undefined variables).
[...]
The null value can come from many sources (e.g. GET/POST/COOKIE/databases)



These two examples are interesting in conjunction: $_GET, $_POST, and 
$_COOKIE will never contain null values unless you have code writing 
directly to them, because HTTP has no representation for it, only empty 
strings. So the only context where you'll get a null from them is 
because you mishandled an undefined array key, which as you say is often 
an indicator of a mistake.


Data retrieved from a database is probably the most common source of 
actual null values. Sometimes, it's important to distinguish those nulls 
from empty strings, sometimes you don't care - but that's true of 
undefined array keys as well. Which is ultimately the problem with all 
of these discussions - some people will see more benefits from catching 
extra mistakes, others will see more cost from fixing code that's lazy 
but not actually broken.


An interesting example I came across recently was a user-defined 
function for SQL string escaping (I'd like to stress legacy; I know that 
proper parameters avoid this issue). The function always expected a 
string, but didn't check that, so if given a PHP null, it would put '' 
in the SQL rather than NULL, leading to data corruption. I have 
effectively added the same notice to it as PHP has to built-in 
functions, to track down where else this might be causing problems 
without escalating to an error immediately.


Regards,

--
Rowan Tommins
[IMSoP]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread Robert Landers 
On Tue, Dec 13, 2022 at 11:16 AM Andreas Leathley  wrote:
>
> On 13.12.22 11:01, Robert Landers wrote:
> > intended: ($a['foo'] ?? null) || ($a['bar'] ?? null)
> > Further, writing code like this increases the number of opcodes needed
> > to perform relatively simple logic by ~150%, increasing end-user
> > latency and spending CPU cycles somewhat needlessly.
> I think it is quite the opposite: calling the error handler because of
> E_NOTICE (or now E_WARNING) is quite the additional overhead, and if
> projects just ignore E_NOTICE their applications will be slowed down
> because of that, while fixing the code will avoid the error handler
> altogether. Just because something is longer to write in a programming
> language also does not make it necessarily slower. But if you have any
> actual measurements why using the null coalescing operator would slow
> down code in general that would be something useful to share.
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>

Ah, that's a good point. I ran some tests and was surprised that it
was slower due to needing to call the error handler. I even disabled
E_WARNING in my php.ini to see if that made a difference (it did, but
still not the same quite as fast as using null coalesce).

Results are here:

https://gist.github.com/withinboredom/bd7276b7703663c26c12a18820747bcb

Interestingly the null coalesces used more memory. That surprised me
and was unexpected. Apparently using coalesce requires some temporary
variables to work. Perhaps there would be a benefit for a
FETCH_DIM_IS_COALESCE opcode?

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread Andreas Leathley 

On 13.12.22 11:01, Robert Landers wrote:

intended: ($a['foo'] ?? null) || ($a['bar'] ?? null)
Further, writing code like this increases the number of opcodes needed
to perform relatively simple logic by ~150%, increasing end-user
latency and spending CPU cycles somewhat needlessly.

I think it is quite the opposite: calling the error handler because of
E_NOTICE (or now E_WARNING) is quite the additional overhead, and if
projects just ignore E_NOTICE their applications will be slowed down
because of that, while fixing the code will avoid the error handler
altogether. Just because something is longer to write in a programming
language also does not make it necessarily slower. But if you have any
actual measurements why using the null coalescing operator would slow
down code in general that would be something useful to share.

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread Craig Francis 
On 12 Dec 2022, at 23:36, Thomas Hruska  wrote:
> I suspect many people are in a similar holding pattern who are currently 
> running packaged 7.4.x and are just now discovering all of the changes for 
> PHP 8.x as they are planning out their system upgrade paths in the coming 
> months.  While you probably wish everyone marched in step with PHP-latest, 
> that's simply not feasible in reality.


Hi Thomas, Dan,

I know a few teams that have started using 8.x on their development servers 
(while 7.x is still on demo/live). Each team has their difficulties, but 
fortunately most issues can be ignored for now (as in, code works, and gives 
you time to make changes)... undefined indexes are a classic, but once you've 
found them all (the tricky bit), it often makes your code better, and helps 
identify mistakes (similar to undefined variables).

But, the one change I still cannot justify, is the deprecation of null being 
passed to the ~335 functions like htmlspecialchars. The null value can come 
from many sources (e.g. GET/POST/COOKIE/databases), and while you can still use 
null coercion in contexts like string concat (empty string), arithmetic (the 
value 0), or a boolean (false); finding all the function calls is incredibly 
hard (one team I work with, who has a fairly good test coverage, is still 
finding these over a year later)... at least it can be ignored for 8.x, but 
it's going to be a nightmare for 9.0... I tried writing an RFC to revert this 
change (considering there was pretty much no discussion went into it, and I 
cannot find a benefit for breaking coercion in this one context), but it was 
met with a lot of hostility from internals, so I'm finding ways to work around 
it, because I don't like systems using old/unsupported software (and I've got 
some ugly hacks for it, like using the latest version of Rector to flood your 
code base with `(string) $var`, or to use a namespace).

Craig


https://wiki.php.net/rfc/null_coercion_consistency 

https://externals.io/message/117501 




null_coercion.php




index.php

Re: [PHP-DEV] Revisiting RFC: Engine Warnings -- Undefined array index

2022-12-13 Thread Robert Landers 
On Tue, Dec 13, 2022 at 12:36 AM Thomas Hruska  wrote:
>
> On 12/12/2022 3:52 PM, Derick Rethans wrote:
> > On 12 December 2022 22:20:27 GMT, Dan Liebner  wrote:
> >
> >> It has been proposed to make the error level of "Undefined index"
> >> configurable so that teams and individual developers can decide for
> >> themselves how they want this situation to be handled. Given that:
> >>
> >>- PHP has been treating this as an E_NOTICE for over 20 years
> >
> > But not in the last three years.
>
> PHP 8.x is only starting to roll out in Linux LTS distro packages at the
> end-user level.  I've moved to 8.x internally but I am, in particular,
> waiting for Ubuntu Server 22.04.2 to officially drop before beginning
> meticulously planned upgrades to mission critical systems.  I suspect
> many people are in a similar holding pattern who are currently running
> packaged 7.4.x and are just now discovering all of the changes for PHP
> 8.x as they are planning out their system upgrade paths in the coming
> months.  While you probably wish everyone marched in step with
> PHP-latest, that's simply not feasible in reality.
>
> The type of post that the OP sent to internals is likely to become more
> common in the next few months as PHP 8.x starts rolling out globally.
> Most businesses are waiting for at least the holidays to conclude before
> performing any major system upgrades if not longer for specific OS
> releases to drop.
>
> --
> Thomas Hruska
> CubicleSoft President
>
> CubicleSoft has over 80 original open source projects and counting.
> Plus a couple of commercial/retail products.
>
> What software are you looking to build?
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>

Hello all,

> The recent change to elevating "Undefined index" from E_NOTICE to E_WARNING
> set forth and passed by https://wiki.php.net/rfc/engine_warnings to me
> seems antithetical to what PHP has been and what has made it a choice for
> developers over the last few decades.

>From my perspective, in the last few years, it seems that PHP has been
trending towards "preventing dev mistakes" over anything else. IMHO,
this has prevented us from getting some really nice language features,
such as operator overloading (because "devs might make mistakes").

I think it is worth pointing out that PHP has always had this kind of
interesting dichotomy towards letting devs do what they want and
preventing them from doing 'bad' things. For example, even though you
could divide by zero you could never create a socket with AF_PACKET
and send raw IP packets from PHP.

Is this a bad thing? I don't know, but it's interesting to watch the
language evolve and so far the changes have revealed hidden bugs in
existing code bases, at least in my experience. However, fixing those
bugs has been a nightmare because some of them have been there for
10-15 years. Ancient silent bugs are suddenly showing up and you have
to make a hard call on whether to fix the ancient mistake or keep the
broken behavior. It isn't always a simple decision and may often be a
business decision vs. a technical one.

Interestingly, with the array access warning, you now have to write
much more boilerplate code just to do something that was once the
default behavior. Not only does this require much more typing, but it
also introduces much more cognitive load as you have to figure out
whether a key has been set before (or be defensive, which introduces
potential performance implications). Even then, using ?? on array
access can introduce more bugs than it is meant to prevent. Most devs
find out the hard way that ?? has an unintuitive order of operations:

typed: $a['foo'] ?? null || $a['bar'] ?? null

intended: ($a['foo'] ?? null) || ($a['bar'] ?? null)

Further, writing code like this increases the number of opcodes needed
to perform relatively simple logic by ~150%, increasing end-user
latency and spending CPU cycles somewhat needlessly.

Personally, I'm a fan of some of the changes made to the language, and
I look forward to PHP's future. I hope this "preventing dev mistake"
mindset is just a phase though because, IMHO, it really has held back
some fantastic proposals that people put a lot of time and effort
into. It's also caused us to have to write much more boilerplate code
for a debatable benefit.

Anyway, as an outside observer, that's my 2¢ on the matter.

Robert Landers
Software Engineer
Utrecht NL

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php