PHP: rfc:engine_warnings (error level re-classification)
wiki.php.net/rfc/engine_warningsPosted Aug 28, 2019, 9:54 AM UTC
9044
wiki.php.net
The current low classification of this error is likely a legacy from the Dark Ages
I feel like we are upgrading to the Imperial Age skipping Feudal and Castle ones with this RFC.
33
Unexpected AoE2 in r/php, have my upvote sir.
12
That mailing list lol. Can we get a declare(technical_debt=1) that runs 7.2 forever on the background for these savages? Move the language forward with PHP 8, so it doesn't smell like wordpress and php4 forever.
21
It is actually rediculous, and the arguments are just sooo garbage.
11
[deleted]
1
I read the initial few messages and thought “this seems reasonable.”
We are now what? 70 replies deep? It’s been bickering and nonsense since this morning.
6
Especially this gem: https://externals.io/message/106713#106785
It's really awkward that anybody would be under the illusion that the way the language always behaved, consistently and well-documented pretty much from the its inception, is somehow a bug that everybody agrees on that's just waiting for someone to come over and fix it.
I just realized I fucking hate Zeev and most of the BC crowd. It's great that there are multiple opinions, but it seems that they would be happy if PHP stayed in the current state forever and that is really hurting the language. If this RFC doesn't pass, I'm strongly considering switching to another language that doesn't have so much drama and is nicer to work with.
9
Ah, good old "if it's documented it's not a bug"
3
Yeah, that one in particular just sounds completely insane. How the fuck is something that generates a notice not a bug? Why would it even generate a notice then?
2
"But muh BC tho"
2
Yes please! This could help improve the code quality of projects so much...
19
Yep, Nikita is capitain of this ship now. Worth mention there are a bunch of good guys there also like Sara Goleman, Antony Ferrara, Joe Watkins, and many others i can't remember the names now.
18
Dmitry Stogov is the name as important as Nikita Popov for sure. There are others who are less visible but doing their job really well: https://github.com/php/php-src/graphs/contributors?from=2017-12-29&to=2019-08-28&type=c. So nope, while Nikita is really moving PHP forward, it's certainly no one man show.
18
Yess, sure! How could i forgot?
Reading the internals some recurrent names catch our eyes, whether by a "balanced" opinion or by their work on RFCs.
3
Your link made me realize something important: https://twitter.com/CiPHPerCoder/status/1166842293183295490
2
One of my favorite activities is deleting code.
1
[deleted]
1
This is great and I hope it lands in 8.0!
Code like what was posted here or here is an abomination and your program should absolutely blow up if you write shit like that. If people want to keep using their terrible legacy code, fine, just stick with 7.x or 5.x or whatever. I'd like to see PHP actually improve in 8.0 instead of being held back by bad code someone wrote 20 years ago.
14
Real world usages of undefined variables dont look like this bad code example. They are deep hidden in some structure
-3
About time they get discovered...
17
Exactly. Backwards-compatibility should not encompass code that has always been broken.
7
Trouble is, it's not necessarily broken. Imagine this code to calculate a discount:
private function calculateDiscountPct(User $user) : float{
if ($user->isPartner()){
$discount = self::PARTNER_BASE_DISCOUNT_PCT;
}
return $discount + $user->getLoyaltyDiscount();
}
Meanwhile, in bootstrap.php, which has existed since the dawn of time:
ini_set('error_reporting', E_ALL & ~E_NOTICE);
The function may or may not access an undeclared variable, and was written with the best of intentions and no warning suppression in it, and yet it will work just fine without error.
Not voicing an opinion one way or the other about the rfc, btw.
-3†
That's... even worse. Makes it even more important to find and fix those cases.
12
We promote notices and warnings to fatal errors at my work. It has never been an issue really.
12
This is the way to go.
This has resulted in a virtually empty error log for us.
7
I recently had to do some emergency debugging on a site that ignored every notion of errors possible. The site was on a shared host where you had to ask for the logs to get them sent to your ftp folder. I requested it and it was just shy of 1gb. Thought they had no log rotation. Turns out it was the log for the day, at 11 in the morning.
3
Yeah... I know those systems, it's absolutely horrific to work on those.
1
This is a really well written RFC. Hats off to Nikita. I am 100% in favor of this and hope they don't split it out as it moves the PHP language forward in the right direction.
11
Wait a second... this RFC wants to throw an exception if I access a variable that hasn't been initialized yet?
9
Yes please
11
I understand they're not ideal, but throwing an exception seems extreme.
0
That’s one of the proposals, yes.
3
What's a valid use case for directly accessing a variable that doesn't exist?
2
Some people just write $foo++; as a sort of "increment foo if it exists, otherwise set it to 1" or echo($foo); as a "print foo if it exists, otherwise print nothing".
Instead of writing decent code that makes sure $foo is initialized, they rely on PHP to just ignore their mistake and carry on while generating a notice (which they ignore).
There is no actual, valid use case.
7
$number = $_GET['number'];
1
$number = $_GET['number'] ?? null;
This makes the intent clear and doesn't depend on _magic_ when the query parameter is not present. Bonus is that you can use null to indicate a missing value or you can just directly set a default. You example doesn't rise to the level of valid use case IMHO. It's just a result of lazy (yet very common) programming.
2
$number = filter_input(INPUT_GET, 'number');
1
I agree with the code you have here. This is definitely the correct way to write that.
My concern is that an uninitialized variable isn't severe enough to warrant throwing an exception. I'd prefer to keep it as an E_NOTICE.
1
The only good thing about php-internals is nikita. I wonder what keeps him going in there.
5
Well he works for Jetbrains now :-) He's paid to develop PHP.
https://blog.jetbrains.com/phpstorm/2019/01/nikita-popov-joins-phpstorm-team/
20
I would not say the only, but the most prominent for sure.
14
I have been following the internals for more than 3 years now. Don't get me wrong, the amount of stupidity that gets entertained there is pathetic. And I am pretty sure that without nikics work we will be at least couple of years behind (wrt feature set) .
Most of the ones in there are like. Lots of talk (mostly rubbish) and no contributions (or trivial contributions, which too should be valued, but WTF) but this guy on the other hand works instead of just talks. If you don't agree with me.. Feel free to check out the commits in php-src.
I guess most ones in internals just want to showoff this in their resume. From their conversations I don't think any employer should keep them around, if they read those.
10
Im only guessing. But i guess jetbrains pays for some sort of php development? The most other people arent professionals.
4
[deleted]
-3
The % operator operators on integers.
Haha funny typo
That said, thanks for all the work Nikita, this is great!
5
Would it be valid to recycle declare(strict_types=1) for this? The strict type declaration is fairly new and code written using it has much less chance of having negative side effects during upgrade.
4
Oh fuck yes.
3
I expect that the undefined variable case won't get up, but the rest seems very sensible.
2
I'm really pleased to see PHP becoming a very robust scripting language.
2
I hope someone can follow my thought process on this.
Assume a piece of code. The code runs, and even tough it encounters such a notice/warning it's result is not harmed in any way. This is possible. Now the notice/warning is upgraded to an Exception which halts the code. If i have high test coverage i might be fine with running that once in the new PHP version. If i don't have a test coverage that high i might try static analysis. If no analysis tool supports finding this problem i have no more solutions but to search the needle in the haystack. Else my production code would halt somewhere i can not know about beforhand of upgrading to the new PHP version. Even tough my production code worked fine.
So i hope someone can write static analyses rules to find those upgrades to exceptions.
Case in point: https://3v4l.org/cha97 works flawlessly and silent until 7.0, then halts from 7.1 onwards. It was a one time thing i found in our 200k lines of code. The dev just started and wanted a string, but due to some changes he had to collect some parts for it so he simply forgot about the initializer. No tests, so it was a little surprise in prod.
1
Indisputable.
0
The “Undefined index: %s“ promotion is going to be disasterous for a ton of legacy code. Because traditionally, PHP was and is using warnings for what users of other languages would consider exceptional problems (like not being able to open a file), in production, you should treat warnings as errors.
And because notices are often extremely benign, you can ignore those in production and even in development.
This means that one of the most often depended-on leniency es in PHP is going to move from something usually hidden and easily hideable to something that is going to be fatal in production.
Please don’t do this.
And if you do, please, please give us a way how to handle warnings and notices without string-comparing error messages.
-1
Perhaps you shouldn't update to 8.0 (if all this makes it in) before fixing your code?
5
No shit. What I'm afraid of is that fixing could take longer than 7.4 remains supported.
1
[deleted]
1
I love this RFC, it would avoid so much bad code end up in production and cause silent yet sometime very serious bugs, Nikita Popov is definitely a very smart man.