Re: [VOTE] Deprecate dynamic properties

  116519
November 25, 2021 23:16 internals@lists.php.net ("Brady Wetherington via internals")
Sorry for the 11th hour chime-in, but I wanted to add some context
about this proposed change, and how it would likely affect our project
and many others. I know the prospect of opt-in was already addressed,
but I wanted to talk about what our experience would be here. (We make
Snipe-IT, which is open-source but also a paid-for-hosting or
pay-for-support project).

Many modern PHP projects (like ours) have 10's or 100's of
dependencies, often installed via Composer. With this change, many of
those will start to throw deprecation warnings - eventually failing in
PHP v9. Not every package is equally modern, or equally
well-maintained. I suspect there are plenty of newer packages that
will "Just Work™" and that's great! But there are probably many older
packages, or more poorly-maintained ones, that will not.

Packagist has around 3 million packages. Figure half are "not modern"
(needing updates to be compatible with the proposed change) - that'd
be 1.5 million packages. (And that's probably pretty conservative).
Figure it takes an hour, each, to fix them all (again, that's
conservative). That's 1.5 million hours, which is 171 developer-years.
That's effectively 2 or 3 developer lifetimes (probably more, if we
try to include work-life balance and whatnot :P )

And what are 9 out of 10 maintainers going to do? They'll just plaster
#[AllowDynamicProperties] in front of every class, and be done with
it. Voila! Compatibility. So the bulk of those 171 developer-years
will be wasted.

I think opt-in would be a better approach - per-class, certainly
(#[FixedProperties] or #[StaticProperties] maybe?), but maybe per-file
as well, or maybe even in php.ini if we want to go per-project.

Different language front-ends to the php back-end engine could
certainly by default jam the attribute in on every class; that'd be
fine. But for PHP itself, one of its massive selling points has been
that it tries very hard not to break compatibility - I have written
code for PHP v4 that I have been able to drag up to PHP v7.x without
too much work. I'd hate to lose that, or make it much harder -
especially for dependencies that I don't have direct control over.

By no means am I suggesting that PHP is a 'dead language' - but let's
not cram ourselves into a Python2/Python3 compatibility hole where we
make upgrades into a nightmare for people. Adding types for variables
and parameters has been optional and is gradually becoming the
standard thing to do; I'm sure this would end up the same.

Anyways, hoping to just add some more context for those who are voting
on the proposal. Feel free to ask any questions you'd like and I'll be
happy to answer as best I can.
  116520
November 26, 2021 00:09 tekiela246@gmail.com (Kamil Tekiela)
Hi Brady,

This is a little bit overly dramatic. This isn't such a huge change that
would affect 50% of existing projects. It's likely to affect a small number
of projects in a very limited way.
It's also not true that developers will slap #[AllowDynamicProperties] on
every class. That would imply that every class in their project is using
dynamic properties. That would be absurd. If it's truly the case that every
class in 50% of the projects uses dynamic properties then we should not
consider this RFC at all. The whole premise of it is that it's highly
unlikely that many people are using dynamic properties on purpose. It's
true that many old unmaintained projects might use it as a feature, but the
keyword is "unmaintained". I would not trust dependencies that haven't been
kept up to date for a few years.

PHP has had many breaking changes over the years. PHP 8.1 introduced a much
more annoying deprecation: Deprecate passing null to non-nullable arguments
of internal functions. This one I would have no trouble believing that it
impacts 50% or even the majority of PHP projects. Dynamic properties
compared to this should be a piece of cake.

I doubt that having to add a single attribute to a selected few classes
would be such a major issue that projects would decide to stay on PHP 8.1
rather than upgrade. It's not comparable to Python 2/3 situation IMHO.

I would be interested to know some numbers from Snipe-IT. Have you done a
preliminary analysis of your codebase of how many classes are actually
using dynamic properties? From a brief look, I can see that usually most
properties are declared.

Regards,
Kamil
  116521
November 26, 2021 00:55 internals@lists.php.net ("Brady Wetherington via internals")
> This is a little bit overly dramatic. This isn't such a huge change that would affect 50% of existing projects. It's likely to affect a small number of projects in a very limited way. > It's also not true that developers will slap #[AllowDynamicProperties] on every class. That would imply that every class in their project is using dynamic properties. That would be absurd. If it's truly the case that every class in 50% of the projects uses dynamic properties then we should not consider this RFC at all. The whole premise of it is that it's highly unlikely that many people are using dynamic properties on purpose. It's true that many old unmaintained projects might use it as a feature, but the keyword is "unmaintained". I would not trust dependencies that haven't been kept up to date for a few years. > > I doubt that having to add a single attribute to a selected few classes would be such a major issue that projects would decide to stay on PHP 8.1 rather than upgrade. It's not comparable to Python 2/3 situation IMHO. > > I would be interested to know some numbers from Snipe-IT. Have you done a preliminary analysis of your codebase of how many classes are actually using dynamic properties? From a brief look, I can see that usually most properties are declared.
Sorry, I didn't mean to sound dramatic, just trying to share my concerns. I suspect that you're very right, and *very very few* projects use dynamic properties on purpose (in our own code, we probably don't ever _mean_ to, except maybe for our Custom Fields feature, but we could do the attribute for that one or so class(es)), but that's not my point. I suspect many of our _dependencies_ might be using it (by accident, to your point!). But I can't control every package in my composer.json, which is where I'm worried. I can't force those developers to do what I want, and I don't want to be stuck in a 'version sandwich' because I'm waiting for one of them to change something. And maybe when they *do* try and change something, they might also change the interface (a violation of semver, but, it happens in the wild, more often than we would like). We have to support old and new versions of php and that can make our Composer situation a real nightmare (I just dragged us forward to be able to work in PHPv8, and it was far harder than it needed to be). I'm not worried about our code at **all** - we write it! We can change it (and we'd be good citizens, I promise, we would!) And that attribute is ignored on versions of PHP that don't support it; which'd be fine for us. That's a fair ask, and we'd do it. What I'm scared about is about our 42 dependencies in composer.json, and the ~400k bytes of dependencies described by our composer.lock. And I'm worried about other projects that have even more dependencies, or might be stuck on a framework or something that's no longer being updated, but they can't migrate off of for whatever reason. I wouldn't wish that on anyone. Thank you for considering, and for your extremely well-written response, -B.
  116522
November 26, 2021 03:23 Danack@basereality.com (Dan Ackroyd)
On Fri, 26 Nov 2021 at 00:55, Brady Wetherington via internals
<internals@lists.php.net> wrote:
> > That's 1.5 million hours, which is 171 developer-years.
If we're going to imagine numbers; there are 6 million PHP developers in the world*. If on average they each lose just 1 hour per year by making typos and accidentally creating a properties dynamically, that's 6 million hours, or 684.93 years! So the value delivered by this change would be 4 times the cost just in the first year. And then every year after that it's pure benefit.
> What I'm scared about is about our 42 dependencies in composer.json,
How about sponsoring each of your dependencies some money, to encourage them to check if their code is compatible after this change, and fix it if it isn't. If a dependency is used by even just 1000 companies, and each of those companies chips in $50, then $50,000 will fund many months of work on that dependency.
> probably more, if we try to include work-life balance and whatnot :P
Most open source is done by people in their free time. Because companies keep refusing to fund open source.
> might be stuck on a framework or something that's no longer being updated,
Having companies sponsor open source projects makes it less likely they will be abandoned. cheers Dan Ack * https://www.theregister.com/2021/04/26/report_developers_slashdata/
  116524
November 26, 2021 06:16 internals@lists.php.net ("Brady Wetherington via internals")
> > That's 1.5 million hours, which is 171 developer-years. > > If we're going to imagine numbers; there are 6 million PHP developers > in the world*. If on average they each lose just 1 hour per year by > making typos and accidentally creating a properties dynamically, > that's 6 million hours, or 684.93 years! > > So the value delivered by this change would be 4 times the cost just > in the first year. And then every year after that it's pure benefit.
And as many of those who wish to, can opt-in to the attribute/feature can save themselves that time (and probably should!). We probably will for our code, tbh! But forcing this issue is going to be massively destructive to the ecosystem. Not everything is brand-spanking new. Some things just work well and don't get updated that often.
> > What I'm scared about is about our 42 dependencies in composer.json, > > How about sponsoring each of your dependencies some money, to > encourage them to check if their code is compatible after this change, > and fix it if it isn't.
You are trying to solve a technological problem with an economic solution. And one that requires that the entire ocean be boiled before it is solved. We already do give money to several places, and would like to give more. And we will. But that's not going to fix this, unless everyone does. And not everyone will. We happen to make money; not a lot of other projects do. We ourselves are open source. We don't get more than a handful of donations per year. This doesn't seem to be the right way to solve this problem.
> If a dependency is used by even just 1000 companies, and each of those > companies chips in $50, then $50,000 will fund many months of work on > that dependency. > > Most open source is done by people in their free time. Because > companies keep refusing to fund open source. > > > might be stuck on a framework or something that's no longer being updated, > > Having companies sponsor open source projects makes it less likely > they will be abandoned.
Maybe, but not always. Sometimes people just lose interest. But again, you're proposing to potentially break a big chunk of packages in hopes that an economic model that has not successfully worked anywhere else is going to work here. That seems cavalier. Right now, to get a collection of packages that work between php 7.4 and 8.0, we've had to spend tremendous amounts of work just to get there - and we had to pull 7.3 support in the process. I didn't want to do that; I want our software to be available to as many people as possible - not everyone has the freedom to install their own version of PHP - some are on crappy shared hosting or have to deal with some creaky IT department. Every single backwards-incompatible change shrinks our version-sandwich down further, especially in the context of packages. Again, I can add the new attributes to our own software today, and that means we're OK - but I can't speak to every single package we're dependent on or *they* are dependent on. -B.
  116532
November 26, 2021 16:46 pollita@php.net (Sara Golemon)
On Fri, Nov 26, 2021 at 12:16 AM Brady Wetherington via internals <
internals@lists.php.net> wrote:

> > > That's 1.5 million hours, which is 171 developer-years. > > > > If we're going to imagine numbers; there are 6 million PHP developers > > in the world*. If on average they each lose just 1 hour per year by > > making typos and accidentally creating a properties dynamically, > > that's 6 million hours, or 684.93 years! > > > > So the value delivered by this change would be 4 times the cost just > > in the first year. And then every year after that it's pure benefit. > > The point of this counter-argument which may have gotten lost in the snark
is that nobody has done the legwork to know definitively what the impact of this will be. You're both making up numbers, and this is part of the reason the deprecation warnings exist, so that concerned product developers with a stake in making sure these sorts of things go smoothly have time to test such changes out on your own codebases. It's been merged to master, so you could stand up a build now and point to the many deprecation warnings you're expecting. I'm not saying send PRs to fix them all, just show real impact rather than theoretical guessing. If it's as bad as you fear, maybe we run another RFC to remove the change before it ever sees the light of day and you become a super hero. Call you Uber Brady. Or maybe not a single deprecation warning shows up. Or just a couple that are easily fixed. Let's make PHP better, together. -Sara
  116533
November 26, 2021 22:37 internals@lists.php.net ("Brady Wetherington via internals")
> > …It's been merged to master, so you could stand up a build now and point >> to the many deprecation warnings you're expecting. I'm not saying send PRs >> to fix them all, just show real impact rather than theoretical guessing. … > > That’s a totally fair ask, and I’ll try and get that done and report back.
I think I’m probably going to run into some Laravel problems here, as they only _just_ came up with a version that will run under php 8.1. I’ll get as far as I can and let you know - even if I determine that my suspicions were completely incorrect :)
>
  116545
December 1, 2021 04:17 internals@lists.php.net ("Brady Wetherington via internals")
>>> …It's been merged to master, so you could stand up a build now and point to the many deprecation warnings you're expecting. I'm not saying send PRs to fix them all, just show real impact rather than theoretical guessing. … > > > That’s a totally fair ask, and I’ll try and get that done and report back. I think I’m probably going to run into some Laravel problems here, as they only _just_ came up with a version that will run under php 8.1. I’ll get as far as I can and let you know - even if I determine that my suspicions were completely incorrect :)
Okay - good news/bad news/good news - Good News: I managed to snag an 8.2 build and ran Snipe-IT on it, and it worked completely fine. I cheated a little bit - I didn't try to make a composer.json that would support 7.4 through 8.2, I just did a 'composer install' from PHPv8, and then switched PHP versions out from under it. Literally nothing broke at all - I was completely shocked! I was so worried that I wasn't testing it right that I had to make a quick little test script with dynamic properties in order to force the deprecation warning to show up - and it did, so I *do* have the latest. Bad News: That's because later versions of Laravel dump all deprecation warnings straight to /dev/null by default. If you configure a log 'channel' for 'deprecations', *then* you get actual results. One page load (our dashboard page) generated 267KB of deprecation warnings. Clicking through most of the main pages of our app generated around 3.5MB of them. And these aren't full stack-traces, they're just messages like: "[2021-12-01 03:48:23] local.WARNING: Creation of dynamic property Illuminate\Database\MySqlConnection::$readWriteType is deprecated in /Users/uberbrady/Documents/grokability/snipe-it/vendor/laravel/framework/src/Illuminate/Database/Connection.php on line 1368" Good News: *Many many many* of those deprecation warnings were repeats of the same deprecated thing from the exact same place. And *very very few* were from the dynamic properties thing. *None* of the dynamic properties deprecations were from our code. Everything was from composer dependencies, and only a small handful - so, probably pretty easy to remediate when it comes to that. So the _other_ deprecation notices are a whole other thing, and having so many is definitely a drag, but that's a completely separate issue from this dynamic properties thing, which doesn't seem too bad so far, for us at least. Other folks might not get off so easily, and /dev/null-ing deprecation warnings seems like it's kicking the can down the road, but, well, *we're* OK for 8.2. I should note that a lot of my PHP-based CLI tools are spewing deprecations everywhere as well, but they at least still work so I can perhaps live with that. -B.
  116551
December 1, 2021 19:06 larry@garfieldtech.com ("Larry Garfield")
On Tue, Nov 30, 2021, at 10:17 PM, Brady Wetherington via internals wrote:
>>>> …It's been merged to master, so you could stand up a build now and point to the many deprecation warnings you're expecting. I'm not saying send PRs to fix them all, just show real impact rather than theoretical guessing. … >> >> >> That’s a totally fair ask, and I’ll try and get that done and report back. I think I’m probably going to run into some Laravel problems here, as they only _just_ came up with a version that will run under php 8.1. I’ll get as far as I can and let you know - even if I determine that my suspicions were completely incorrect :) > > Okay - good news/bad news/good news - > > Good News: I managed to snag an 8.2 build and ran Snipe-IT on it, and > it worked completely fine. I cheated a little bit - I didn't try to > make a composer.json that would support 7.4 through 8.2, I just did a > 'composer install' from PHPv8, and then switched PHP versions out from > under it. Literally nothing broke at all - I was completely shocked! I > was so worried that I wasn't testing it right that I had to make a > quick little test script with dynamic properties in order to force the > deprecation warning to show up - and it did, so I *do* have the > latest. > > Bad News: That's because later versions of Laravel dump all > deprecation warnings straight to /dev/null by default. If you > configure a log 'channel' for 'deprecations', *then* you get actual > results. One page load (our dashboard page) generated 267KB of > deprecation warnings. Clicking through most of the main pages of our > app generated around 3.5MB of them. And these aren't full > stack-traces, they're just messages like: > > "[2021-12-01 03:48:23] local.WARNING: Creation of dynamic property > Illuminate\Database\MySqlConnection::$readWriteType is deprecated in > /Users/uberbrady/Documents/grokability/snipe-it/vendor/laravel/framework/src/Illuminate/Database/Connection.php > on line 1368" > > Good News: *Many many many* of those deprecation warnings were repeats > of the same deprecated thing from the exact same place. And *very very > few* were from the dynamic properties thing. *None* of the dynamic > properties deprecations were from our code. Everything was from > composer dependencies, and only a small handful - so, probably pretty > easy to remediate when it comes to that. > > So the _other_ deprecation notices are a whole other thing, and having > so many is definitely a drag, but that's a completely separate issue > from this dynamic properties thing, which doesn't seem too bad so far, > for us at least. Other folks might not get off so easily, and > /dev/null-ing deprecation warnings seems like it's kicking the can > down the road, but, well, *we're* OK for 8.2. > > I should note that a lot of my PHP-based CLI tools are spewing > deprecations everywhere as well, but they at least still work so I can > perhaps live with that. > > -B.
Thanks for the follow up! Out of curiosity, since it sounds like the dynamic props deprecation rates a "meh" on the problem scale for you, what is the most common other deprecation you're hitting? It would be interesting to see how many *unique* deprecation warnings a large, real-world application has and compare with other projects. --Larry Garfield
  116553
December 2, 2021 05:16 pollita@php.net (Sara Golemon)
On Tue, Nov 30, 2021 at 10:18 PM Brady Wetherington <
bwetherington@grokability.com> wrote:

> >>> …It's been merged to master, so you could stand up a build now and > point to the many deprecation warnings you're expecting. I'm not saying > send PRs to fix them all, just show real impact rather than theoretical > guessing. … > Okay - good news/bad news/good news - > > Bad News: That's because later versions of Laravel dump all > deprecation warnings straight to /dev/null by default. If you > configure a log 'channel' for 'deprecations', *then* you get actual > results. One page load (our dashboard page) generated 267KB of > deprecation warnings. Clicking through most of the main pages of our > app generated around 3.5MB of them. And these aren't full > stack-traces, they're just messages like: > > "[2021-12-01 03:48:23] local.WARNING: Creation of dynamic property > Illuminate\Database\MySqlConnection::$readWriteType is deprecated in > > /Users/uberbrady/Documents/grokability/snipe-it/vendor/laravel/framework/src/Illuminate/Database/Connection.php > on line 1368" > > Good News: *Many many many* of those deprecation warnings were repeats > of the same deprecated thing from the exact same place. And *very very > few* were from the dynamic properties thing. *None* of the dynamic > properties deprecations were from our code. Everything was from > composer dependencies, and only a small handful - so, probably pretty > easy to remediate when it comes to that. > > Does it look like there's any sensitive information in these log entries?
You mentioned they were only in external dependencies so I'm thinking/hoping not. What I'm getting at is: Would you mind sharing those raw logs somewhere. I'm sure someone will find it a fun opportunity to make some pull requests in a variety of projects. -Sara
  116554
December 2, 2021 07:02 internals@lists.php.net ("Brady Wetherington via internals")
On Wed, Dec 1, 2021 at 9:16 PM Sara Golemon <pollita@php.net> wrote:
> > On Tue, Nov 30, 2021 at 10:18 PM Brady Wetherington <bwetherington@grokability.com> wrote: >> >> >>> …It's been merged to master, so you could stand up a build now and point to the many deprecation warnings you're expecting. I'm not saying send PRs to fix them all, just show real impact rather than theoretical guessing. … > ... > Does it look like there's any sensitive information in these log entries? You mentioned they were only in external dependencies so I'm thinking/hoping not. What I'm getting at is: Would you mind sharing those raw logs somewhere. I'm sure someone will find it a fun opportunity to make some pull requests in a variety of projects.
Nope, not at all. Here I extracted out just the specific deprecations and added counts so anyone can see how many times a deprecation warning came up. Please do note that it is very likely that newer versions of these libraries may have already resolved the issues, I haven't bothered yet to try and upgrade to the latest versions. uberbrady@GrokBook-Pro-Black snipe-it % cut -d " " -f 4- storage/logs/deprecations.log|sort |uniq -c 10 Creation of dynamic property Barryvdh\Debugbar\DataCollector\ViewCollector::$name is deprecated in /Users/uberbrady/Documents/grokability/snipe-it/vendor/barryvdh/laravel-debugbar/src/DataCollector/ViewCollector.php on line 24 10 Creation of dynamic property Barryvdh\Debugbar\DataFormatter\QueryFormatter::$cloner is deprecated in /Users/uberbrady/Documents/grokability/snipe-it/vendor/maximebf/debugbar/src/DebugBar/DataFormatter/DataFormatter.php on line 23 10 Creation of dynamic property Barryvdh\Debugbar\DataFormatter\QueryFormatter::$dumper is deprecated in /Users/uberbrady/Documents/grokability/snipe-it/vendor/maximebf/debugbar/src/DebugBar/DataFormatter/DataFormatter.php on line 24 30 Creation of dynamic property Barryvdh\Debugbar\DataFormatter\SimpleFormatter::$cloner is deprecated in /Users/uberbrady/Documents/grokability/snipe-it/vendor/maximebf/debugbar/src/DebugBar/DataFormatter/DataFormatter.php on line 23 30 Creation of dynamic property Barryvdh\Debugbar\DataFormatter\SimpleFormatter::$dumper is deprecated in /Users/uberbrady/Documents/grokability/snipe-it/vendor/maximebf/debugbar/src/DebugBar/DataFormatter/DataFormatter.php on line 24 10 Creation of dynamic property DebugBar\DataFormatter\DataFormatter::$cloner is deprecated in /Users/uberbrady/Documents/grokability/snipe-it/vendor/maximebf/debugbar/src/DebugBar/DataFormatter/DataFormatter.php on line 23 10 Creation of dynamic property DebugBar\DataFormatter\DataFormatter::$dumper is deprecated in /Users/uberbrady/Documents/grokability/snipe-it/vendor/maximebf/debugbar/src/DebugBar/DataFormatter/DataFormatter.php on line 24 12 Creation of dynamic property Illuminate\Database\MySqlConnection::$readWriteType is deprecated in /Users/uberbrady/Documents/grokability/snipe-it/vendor/laravel/framework/src/Illuminate/Database/Connection.php on line 1368 4 class_exists(): Passing null to parameter #1 ($class) of type string is deprecated in /Users/uberbrady/Documents/grokability/snipe-it/vendor/adldap2/adldap2/src/Configuration/DomainConfiguration.php on line 153 626 mb_convert_encoding(): Handling HTML entities via mbstring is deprecated; use htmlspecialchars, htmlentities, or mb_encode_numericentity/mb_decode_numericentity instead in /Users/uberbrady/Documents/grokability/snipe-it/vendor/symfony/var-dumper/Dumper/HtmlDumper.php on line 963 84 preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated in /Users/uberbrady/Documents/grokability/snipe-it/vendor/barryvdh/laravel-debugbar/src/JavascriptRenderer.php on line 150 4 str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated in /Users/uberbrady/Documents/grokability/snipe-it/vendor/laravel/passport/src/PassportServiceProvider.php on line 268 84 substr(): Passing null to parameter #1 ($string) of type string is deprecated in /Users/uberbrady/Documents/grokability/snipe-it/vendor/barryvdh/laravel-debugbar/src/JavascriptRenderer.php on line 150
  116556
December 2, 2021 14:48 craig@craigfrancis.co.uk (Craig Francis)
On Fri, 26 Nov 2021 at 16:47, Sara Golemon <pollita@php.net> wrote:

> I'm not saying send PRs to fix them all... Let's make PHP better, > together.
On a similar theme, trying to avoid too much work for developers upgrading to later versions of PHP. Is there any value in me proposing an RFC to update *some* internal functions so they can accept NULL? I see developers using their framework of choice for GET/POST/COOKIE/etc values (where they receive NULL to represent unset values), or simply doing `$q = ($_GET['q'] ?? NULL)`, and other sources... where they will now get deprecation messages whenever they use functions like `htmlspecialchars()`, `trim()`, `strpos()`, `strtoupper()`, `strlen()`. For example, a search page, where the search term is defined in the URL (e.g. "/search/?q=abc"), and that value is shown on the page, often in an `` for the user to edit, and sometimes repeated in the page content (where they may use `strtoupper()` for styling purposes, instead of doing that via CSS `text-transform: uppercase`). Craig https://externals.io/message/116076
  116557
December 2, 2021 15:19 pollita@php.net (Sara Golemon)
On Thu, Dec 2, 2021 at 8:48 AM Craig Francis <craig@craigfrancis.co.uk>
wrote:

> On Fri, 26 Nov 2021 at 16:47, Sara Golemon <pollita@php.net> wrote: > > > I'm not saying send PRs to fix them all... Let's make PHP better, > > together. > > > > On a similar theme, trying to avoid too much work for developers upgrading > to later versions of PHP. > > Is there any value in me proposing an RFC to update *some* internal > functions so they can accept NULL? > > I see developers using their framework of choice for GET/POST/COOKIE/etc > values (where they receive NULL to represent unset values), or simply doing > `$q = ($_GET['q'] ?? NULL)`, and other sources... where they will now get > deprecation messages whenever they use functions like `htmlspecialchars()`, > `trim()`, `strpos()`, `strtoupper()`, `strlen()`. > > I'm not hard against this idea. The interpretation of null in these
contexts as being equivalent to empty string isn't unreasonable. I guess the only objection I could have would be an academic one and I can't really defend that. So yeah, sure... why not? I would say that such applications should consider unifying their own types. $a = $_GET['q'] ?? ''; Is there a place in the application where empty string and null would have been distinct? i.e. Is a search for nothing different from not searching? -Sara
  116559
December 2, 2021 17:14 cschneid@cschneid.com (Christian Schneider)
Am 02.12.2021 um 16:19 schrieb Sara Golemon <pollita@php.net>:
> I would say that such applications should consider unifying their own > types. $a = $_GET['q'] ?? ''; Is there a place in the application where > empty string and null would have been distinct? i.e. Is a search for > nothing different from not searching?
Being able to detect if a parameter was present and empty or it it was not present at all can be useful. That is the whole point of null, even though some people dislike null :-) In fact we ran into the same problem and are silencing certain notices, warnings and deprecations because they get in the way of our code base. (Side-note: We are patching it instead of suppressing it in an error handler as we saw a noticeable performance difference.) For the curious, here is our patch: perl -0777 -i.bak -pe 's#zend_error.*E_(NOTICE|WARNING|DEPRECATED).*(\n.*)?(Undefined|Attempt to read property|Uninitialized string offset|Trying to access array offset on value of type|Passing null to parameter)#if (0) $&#g; s#zend_type_error.*must be of type Countable#if(0) $&#g' Zend/*.[ch] ext/spl/*.[ch] ext/opcache/jit/*.[ch] I am *not* advocating that other people should do this, I'm just corroborating ;-) Regards, - Chris
  116560
December 2, 2021 17:28 craig@craigfrancis.co.uk (Craig Francis)
On Thu, 2 Dec 2021 at 15:19, Sara Golemon <pollita@php.net> wrote:

> I'm not hard against this idea. The interpretation of null in these > contexts as being equivalent to empty string isn't unreasonable. I guess > the only objection I could have would be an academic one and I can't really > defend that. So yeah, sure... why not? >
Thanks, I'll start by creating a list of functions I think apply (anyone who has any suggestions, please let me know... I'm just going to look at some log files to get started... or try to work out if `Z_PARAM_STRING_OR_NULL` vs `Z_PARAM_STR` / `Z_PARAM_STRING` can give me a list of functions to look though). I would say that such applications should consider unifying their own
> types. $a = $_GET['q'] ?? ''; Is there a place in the application where > empty string and null would have been distinct? i.e. Is a search for > nothing different from not searching? >
While I think unifying can help in some cases, there is a lot of existing code where developers have used the null coalesce operator to return NULL (either out of habit, or because they need to determine the difference between an empty string vs an unset value)... the main source being the functions frameworks have provided to return GPC values, e.g. - Laravel: $request->input('name'); - Symfony: $request->get('name'); - CakePHP: $this->request->getQuery('name'); - CodeIgniter: $request->getGet('name'); As to the search example... I've seen one project where the search pages would only show the form at first (when null); then, when the user did a search, it showed the search form again, the results table (where an empty string would show all records, useful to see the total number of records), and present additional filtering fields (done like this for performance reasons). That's not to say all of these examples can't be updated to work, but there will be a lot to change... like one project which had simply used `trim()` to remove whitespace from some user inputs, where the fields/values are normally present, but because NULL can happen (even via odd things, like the user being on a slow connection and the form not loading completely), it's now a problem. Thanks again, Craig
  116561
December 2, 2021 17:32 pierre.php@gmail.com (Pierre Joye)
On Thu, Dec 2, 2021, 10:20 PM Sara Golemon <pollita@php.net> wrote:

I'm not hard against this idea.  The interpretation of null in these
> contexts as being equivalent to empty string isn't unreasonable. I guess > the only objection I could have would be an academic one and I can't really > defend that. So yeah, sure... why not? > > I would say that such applications should consider unifying their own > types. $a = $_GET['q'] ?? ''; Is there a place in the application where > empty string and null would have been distinct? i.e. Is a search for > nothing different from not searching? >
I wonder what is the technical benefit from that? I do feel we are moving to a strict typed language. If that is what is desired, then let decide it clearly and move forward to rhst direction. best, Pierre
>