# [RFC] Reclassifying engine warnings - Thread: https://externals.io/message/106713 - Original archive: https://news-web.php.net/php.internals/106713 - Started by Nikita Popov on 2019-08-28T09:33:25+00:00 - Messages: 137 - RFC: Reclassifying engine warnings (Implemented (PHP 8.0)) — https://externals.io/rfc/engine_warnings.md - Page 1 of 6: messages 1 to 25 (25 per page, add `?page=N` to the URL) - Next page: https://externals.io/message/106713.md?page=2 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · RFCs at https://externals.io/rfc.md · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #106713 — Nikita Popov — 2019-08-28T09:33:25+00:00 URL: https://externals.io/message/106713#106713 Hi internals, I think it's time to take a look at our existing warnings & notices in the engine, and think about whether their current classification is still appropriate. Error conditions like "undefined variable" only generating a notice is really quite mind-boggling. I've prepared an RFC with some suggested classifications, though there's room for bikeshedding here... https://wiki.php.net/rfc/engine_warnings Regards, Nikita --- ## #106714 — Lynn — 2019-08-28T10:07:19+00:00 URL: https://externals.io/message/106713#106714 · In reply to: #106713 Hi Nikita, This RFC makes me very happy! I have a few points that I didn't find clarification on in the RFC. For "Undefined variable: %s" the new proposal is an Error Exception. For a starter: will it include dynamically declared variables such as $$foo? For my second point: in regards of backwards compatibility, is it possible to first trigger a deprecation for each of the cases that will abort the flow? If not, I fear I won't be able to upgrade as we still maintain a lot of legacy code that deals with dynamically declared and undefined variables, and possibly other violations that we don't know of. We are working on rewriting all this code but don't have enough developers to rewrite everything under a few years. If we can start logging these deprecations, we know where to fix what, rather than having our entire application crash. This way we can prioritize our maintenance on fixes each of these cases. The scenarios where this occurs often in our code base are dynamically included files that may or may not set a variable. The code is most likely "broken", but the behavior it produces is what's considered correct at this point. It can be solved by defining the variable with null value, but it's hard to trace without logging as we can't always set it with null value. Regards, Lynn van der Berg > Hi internals, > > I think it's time to take a look at our existing warnings & notices in the > engine, and think about whether their current classification is still > appropriate. Error conditions like "undefined variable" only generating a > notice is really quite mind-boggling. > > I've prepared an RFC with some suggested classifications, though there's > room for bikeshedding here... > > https://wiki.php.net/rfc/engine_warnings > > Regards, > Nikita --- ## #106715 — Zeev Suraski — 2019-08-28T10:40:47+00:00 URL: https://externals.io/message/106713#106715 · In reply to: #106713 > Hi internals, > > I think it's time to take a look at our existing warnings & notices in the > engine, and think about whether their current classification is still > appropriate. Error conditions like "undefined variable" only generating a > notice is really quite mind-boggling. > > I've prepared an RFC with some suggested classifications, though there's > room for bikeshedding here... > > https://wiki.php.net/rfc/engine_warnings Specifically on undefined variables, the way we deal with them has little to do with register_globals. It's behavior you can find in other dynamic languages (e.g. Perl), and allows for certain code patterns (which rely on the automatic creation of a variable whenever it's used in write context, and on a default known-in-advance value in case it's used in a read context). It's fine not to like this behavior or the code patterns that commonly rely on it (e.g., @$foo++), but it's intentional and isn't related to any historical reasons. I think many (if not all) of your proposals make sense, but most of them make sense as an opt-in - perhaps using something similar to Perl's strict mode (which incidentally, changes the way the language treats undefined variables in exactly the same way). This would also provide a future-proof solution for additional similarly-themed proposals (such as strict ops, etc.). Zeev --- ## #106716 — Mark Randall — 2019-08-28T11:07:02+00:00 URL: https://externals.io/message/106713#106716 · In reply to: #106713 > Hi internals, > https://wiki.php.net/rfc/engine_warnings This is an absolutely outstanding move and I certainly hope it gets passed unanimously. PHP as a whole has traditionally been a language that is insanely forgiving of errors, in many cases allowing code execution to continue as if they never even happened, even in utterly egregious cases, and that is certainly something that has played a part in PHP's reputational difficulties. I am extremely happy to see this and other changes underway to reverse these historic failings. It can only be a good thing for the PHP ecosystem as a whole to have the engine and core extensions enforce better protections against bad or potentially buggy code. It won't be an instant transition, and there will be a lot of hold-outs on 7.4 who just don't want to deal with it, but as people do eventually migrate to 8.0, this error-throwing behaviour will force programmers to improve their code rather than just consign the problem to the /dev/null error log. These changes are, no doubt, going to be a painful wake up call for our friends in the community who are, shall we say, not as focused on the quality of their code as we (and their customers) would perhaps like them to be. For everyone else, it's one more tool to help make us aware of problems, and prevent those problems from propagating along the stack and effecting other things. Bring it on. -- Mark Randall --- ## #106717 — Nikita Popov — 2019-08-28T11:10:21+00:00 URL: https://externals.io/message/106713#106717 · In reply to: #106715 > > > On Wed, Aug 28, 2019 at 12:33 PM Nikita Popov > wrote: > > > […] > > > Specifically on undefined variables, the way we deal with them has little > to do with register_globals. It's behavior you can find in other dynamic > languages (e.g. Perl), and allows for certain code patterns (which rely on > the automatic creation of a variable whenever it's used in write context, > and on a default known-in-advance value in case it's used in a read > context). It's fine not to like this behavior or the code patterns that > commonly rely on it (e.g., @$foo++), but it's intentional and isn't related > to any historical reasons. > This argument makes sense for arrays and objects (and I don't promote undefined index/property to exceptions for that reason), but I don't think it holds any water for simple variables. Writing @$counts[$key]++ is a lazy way to count values and avoid ugly boilerplate for if (isset($counts[$key])) { $counts[$key]++; } else { $counts[$key] = 1; }. But @$foo++ is just a really bad way of writing either $foo++ or $foo = 1. Outside of variable variables, the concept of a conditionally defined variable just doesn't make a lot of sense. > I think many (if not all) of your proposals make sense, but most of them > make sense as an opt-in - perhaps using something similar to Perl's strict > mode (which incidentally, changes the way the language treats undefined > variables in exactly the same way). This would also provide a future-proof > solution for additional similarly-themed proposals (such as strict ops, > etc.). > I don't think this is an appropriate use of an opt-in. It's a case where we can balance language cleanup with backwards compatibility concerns. Code that works after this proposal will also work before it, and as such there is no danger of ecosystem bifurcation that would need to be addressed by an opt-in. Thanks, Nikita --- ## #106718 — Rowan Tommins — 2019-08-28T11:27:58+00:00 URL: https://externals.io/message/106713#106718 · In reply to: #106713 > I think it's time to take a look at our existing warnings & notices in the > engine, and think about whether their current classification is still > appropriate. Error conditions like "undefined variable" only generating a > notice is really quite mind-boggling. > While I agree with the reasoning behind this, I think we should be very, very careful of promoting anything beyond Warning. While it's certainly true that bugs may be hiding behind Notices and Warnings, it's equally certain that there is code that is poorly written but has entirely correct behaviour; for such code, the cure will be worse than the disease, because an unhandled Error is like suddenly pulling the plug out of the server in the middle of a transaction. That's why I was very careful with my undefined constant RFC [1] to first raise the Notice to a Warning, and leave an appropriate period before raising to Error. I would be 100% behind raising undefined variable from Notice to Warning, but think the impact of raising to Error is high enough that it would risk delaying take-up of PHP 8. Is it too late to raise to Warning in 7.4, with a clear message that it will be an error in 8.0? That would give time for people to discover it in rare code paths, get changes pushed to third-party libraries, etc, before it becomes an unavoidable error. [1] https://wiki.php.net/rfc/deprecate-bareword-strings -- Rowan Collins [IMSoP] --- ## #106719 — Lynn — 2019-08-28T11:45:49+00:00 URL: https://externals.io/message/106713#106719 · In reply to: #106717 This argument makes sense for arrays and objects (and I don't promote > undefined index/property to exceptions for that reason), but I don't think > it holds any water for simple variables. Writing @$counts[$key]++ is a lazy > way to count values and avoid ugly boilerplate for if > (isset($counts[$key])) { $counts[$key]++; } else { $counts[$key] = 1; }. > But @$foo++ is just a really bad way of writing either $foo++ or $foo = 1. > Outside of variable variables, the concept of a conditionally defined > variable just doesn't make a lot of sense. > The variables are not conditionally declared on purpose, but as it's code written 15~20 years ago, maintained, copy-pasted over and over and never reviewed, there's a lot of issues that remain that my trigger an error exception if this proposal is passed without migration path. ``` // a.php if ($someCondition) { // comes from another file, never used, can't verify existance $foo = 'foo'; } // b.php $bar = 'bar'; // scenario1.php include 'a.php'; // this would crash include 'b.php'; echo $foo . $bar; // this would crash if the include didn't yet // scenario2.php $someCondition = true; include 'a.php'; // this would not crash include 'b.php'; echo $foo . $bar; // this would not crash ``` The problem with the above legacy code, which we sadly have spread through a spaghetti of thousands of files, is that if it starts throwing errors with no migration path, we simply can't update without spending possibly weeks debugging and trying to find scenarios where this might trigger. Now I'm not sure, but if I would've used `include_once` and combine this with function/class scopes (people loved to do that in our legacy code), all hell breaks loose and it will be nearly impossible to find the cases. All I'm asking is for a clear upgrade path with deprecations so that my code can be safely migrated over time, rather than have it crash with the next version. Compared to > > > This argument makes sense for arrays and objects (and I don't promote > > […] > > The variables are not conditionally declared on purpose, but as it's code > written 15~20 years ago, maintained, copy-pasted over and over and never > reviewed, there's a lot of issues that remain that my trigger an error > exception if this proposal is passed without migration path. > To be clear, this was in response to Zeev's particular argument, which I don't think is valid. I am aware that accesses to undefined variables are a thing in legacy code. However, I feel pretty strongly that converting any of these to deprecations is not a good idea. While there's certainly different views on this, I've seen it often enough deprecation warning are considered an even lower error level than notices (imagine my surprise when PEAR stopped working completely in PHP 8 because nobody ever saw the hundreds of suppressed deprecations). We could throw a deprecation in addition, but I think this will make the development experience really suck for anyone who is not actively working on a migration right now (imagine seeing lots of warnings/notices during development twice). I think it would be better to register an error handler that matches the cases that will throw and can then log those. I'd be happy to provide an implementation if you think this would be useful for your use-case. Also has the nice advantage that you can start using it right now and don't have to wait until you upgrade to a release that has deprecations. Nikita > > ``` > // a.php > if ($someCondition) { // comes from another file, never used, can't verify > existance > $foo = 'foo'; > } > > // b.php > $bar = 'bar'; > > // scenario1.php > include 'a.php'; // this would crash > include 'b.php'; > > echo $foo . $bar; // this would crash if the include didn't yet > > // scenario2.php > $someCondition = true; > > include 'a.php'; // this would not crash > include 'b.php'; > > echo $foo . $bar; // this would not crash > ``` > > The problem with the above legacy code, which we sadly have spread through > a spaghetti of thousands of files, is that if it starts throwing errors > with no migration path, we simply can't update without spending possibly > weeks debugging and trying to find scenarios where this might trigger. Now > I'm not sure, but if I would've used `include_once` and combine this with > function/class scopes (people loved to do that in our legacy code), all > hell breaks loose and it will be nearly impossible to find the cases. > > All I'm asking is for a clear upgrade path with deprecations so that my > code can be safely migrated over time, rather than have it crash with the > next version. Compared to easy to fix. By having it throw deprecations which we can easily log to a > specific file, we can gather and fix the cases when we have time to spend > on technical debt, and by the time the warning will turn into an error > exception, we'll have fixed probably 90%+ of the cases, making the upgrade > possible. > > Regards, > Lynn van der Berg --- ## #106721 — Christian Schneider — 2019-08-28T11:58:54+00:00 URL: https://externals.io/message/106713#106721 · In reply to: #106717 Am 28.08.2019 um 13:10 schrieb Nikita Popov : > > > […] > > This argument makes sense for arrays and objects (and I don't promote > undefined index/property to exceptions for that reason), but I don't think > it holds any water for simple variables. Writing @$counts[$key]++ is a lazy > way to count values and avoid ugly boilerplate for if > (isset($counts[$key])) { $counts[$key]++; } else { $counts[$key] = 1; }. > But @$foo++ is just a really bad way of writing either $foo++ or $foo = 1. > Outside of variable variables, the concept of a conditionally defined > variable just doesn't make a lot of sense. We often use the pattern $a .= "xy" or $a[] = "xy" and requiring to initialise leads to either boiler-plate code in often very simple functions or even worse to write the first one as $a = "xy" and $a = ["xy"] which both hurt symmetry and extensibility (you need to change two places if you want to add a new first entry). We are already suppressing the `E_NOTICE` for this as the pattern is too useful to fill our logs. Your simple typo in variable names can be caught well enough by a static analyser, we have a simplistic one in out git commit hook which more than does the job for us. And it does it even better in your average case as it also catches typos in rare code paths not executed by your tests. And no, don't say "make sure you have 100% code coverage", that is a myth. Summary: Promote `E_NOTICE` to E_WARNINGS, fine, make them abort code execution: Don't do it, don't break our code. For people promoting a stricter PHP mode: Just fix your code if it caused an E_NOTICE/E_WARNING, you don't need an Exception for that. And I don't buy the whole security argument, code has to be secure on a whole different level than undefined variables. - Chris --- ## #106722 — Rowan Tommins — 2019-08-28T12:18:28+00:00 URL: https://externals.io/message/106713#106722 · In reply to: #106720 > However, I feel pretty strongly that converting any of these to > deprecations is not a good idea. While there's certainly different views on > this, I've seen it often enough deprecation warning are considered an even > lower error level than notices (imagine my surprise when PEAR stopped > working completely in PHP 8 because nobody ever saw the hundreds of > suppressed deprecations). We could throw a deprecation in addition, but I > think this will make the development experience really suck for anyone who > is not actively working on a migration right now (imagine seeing lots of > warnings/notices during development twice). > Again, I considered this carefully for undefined constants and discussed it extensively on the RFC and resulting thread. In short, "deprecation notice" doesn't have to mean "E_DEPRECATED". Arguably, "severity" and "type" should be two different dimensions, and E_DEPRECATION_WARNING would be E_DEPRECATION & E_WARNING; but in practice, a Warning containing the text "this is deprecated" achieves the goal just fine. Whatever the mechanism, the point is to make people aware as far in advance as possible, so that they can start addressing the problem before it becomes a blocker to upgrading. As briefly mentioned, third-party libraries are a key case here: if a library raises extra Warnings, I can take the time to submit a patch to that library, wait for the maintainer to accept it, and make sure I can use the latest version; if the library raises extra Errors, I have to delay my upgrade, or run a patched version of the library, until it's fixed. Regards, -- Rowan Tommins [IMSoP] --- ## #106723 — Zeev Suraski — 2019-08-28T12:34:54+00:00 URL: https://externals.io/message/106713#106723 · In reply to: #106717 > > > > […] > > This argument makes sense for arrays and objects (and I don't promote > undefined index/property to exceptions for that reason), but I don't think > it holds any water for simple variables. Writing @$counts[$key]++ is a lazy > way to count values and avoid ugly boilerplate for if > (isset($counts[$key])) { $counts[$key]++; } else { $counts[$key] = 1; }. > But @$foo++ is just a really bad way of writing either $foo++ or $foo = 1. > Outside of variable variables, the concept of a conditionally defined > variable just doesn't make a lot of sense. > This example has nothing to do with arrays. There are many code patterns in which relying on this behavior makes perfect sense for folks who are a lot less strictly-minded. For example: foreach (whatever) { if (sth) { @$whCount++; } } Yes, it may be painful for many eyes that $whCount is not explicitly initialized, but the above code is perfectly legitimate, warning-free notice-compliant code since forever. Moreover - this isn't legacy - there are a lot of folks who appreciate this precise behavior, which is documented and works as expected for the last 20+ years. Or: if ($bookCount>0) { $suffix = 's'; } print "$bookCount book$suffix"; These are just two simple cases I bumped into myself recently. There's an infinite supply of more of those where these came from. > > > […] > > I don't think this is an appropriate use of an opt-in. It's a case where > we can balance language cleanup with backwards compatibility concerns. Code > that works after this proposal will also work before it, and as such there > is no danger of ecosystem bifurcation that would need to be addressed by an > opt-in. > Calling this 'cleanup' is opinionated, and avoiding bifurcation by forcing that opinion on everyone isn't a very good solution for those who have other opinions. While the opinion that variables must not be used before being initialized is obviously a valid one - it is just that, one valid opinion - and there are others. PHP never took this opinion as an axiomatic requirement (and not because of register_globals) - instead, the intent was to have a default value for uninitialized variables - a consistent, documented behavior since the dawn of the language. Can this be problematic under certain situations? Absolutely. Can it be useful in other cases? Sure (which is why it's very common). A great deal of folks both rely on this behavior and *like *it. Those who don't (and there's plenty of those as well of course) - always had a reasonable solution of enabling `E_STRICT` and enforcing E_STRICT-compliant code. I still think that having a strict mode (which can encompass strict types, strict ops, stricter error behavior, etc.) makes a lot of sense and would arguably be a superior option for the many folks who prefer a stricter language - but there's simply no way we can change one of the most fundamental behaviors of the language and force it down people's throats - not only because it breaks compatibility, but because it breaks how many people are used to write their PHP code. Perl provided stricter-liking folks with a solution in the form of 'use strict;' decades ago; JS did something similar much more recently. Neither of these created any sort of bifurcation - it's a simple, sensible solution that has virtually no downsides. Zeev --- ## #106724 — Lynn — 2019-08-28T13:29:30+00:00 URL: https://externals.io/message/106713#106724 · In reply to: #106720 However, I feel pretty strongly that converting any of these to > deprecations is not a good idea. While there's certainly different views on > this, I've seen it often enough deprecation warning are considered an even > lower error level than notices (imagine my surprise when PEAR stopped > working completely in PHP 8 because nobody ever saw the hundreds of > suppressed deprecations). We could throw a deprecation in addition, but I > think this will make the development experience really suck for anyone who > is not actively working on a migration right now (imagine seeing lots of > warnings/notices during development twice). > You got a very valid point here. As long as there is a clear migration path, albeit with user-land tools, I'll take it. > I think it would be better to register an error handler that matches the > cases that will throw and can then log those. I'd be happy to provide an > implementation if you think this would be useful for your use-case. Also > has the nice advantage that you can start using it right now and don't have > to wait until you upgrade to a release that has deprecations. > It would be great to have a package that reports all these cases, though we could try to craft something ourselves if this RFC is approved. Perhaps frameworks providing error handlers can add specific logging for these scenarios as well, saves you from putting more effort in this than required. Regards, Lynn van der Berg --- ## #106725 — Chase Peeler — 2019-08-28T13:30:32+00:00 URL: https://externals.io/message/106713#106725 · In reply to: #106723 > > > > […] > generating a > > […] > there's > > […] > little > > […] > (which > > […] > think > > […] > lazy > > […] > 1. > > […] > > This example has nothing to do with arrays. There are many code patterns > in which relying on this behavior makes perfect sense for folks who are a > lot less strictly-minded. For example: > > foreach (whatever) { > if (sth) { > @$whCount++; > } > } > > Yes, it may be painful for many eyes that $whCount is not explicitly > initialized, but the above code is perfectly legitimate, warning-free > notice-compliant code since forever. Moreover - this isn't legacy - there > are a lot of folks who appreciate this precise behavior, which is > documented and works as expected for the last 20+ years. > > Or: > > if ($bookCount>0) { > $suffix = 's'; > } > > print "$bookCount book$suffix"; > > These are just two simple cases I bumped into myself recently. There's an > infinite supply of more of those where these came from. > > > > […] > strict > > […] > future-proof > > […] > Code > > […] > there > > […] > an > > […] > > Calling this 'cleanup' is opinionated, and avoiding bifurcation by forcing > that opinion on everyone isn't a very good solution for those who have > other opinions. While the opinion that variables must not be used before > being initialized is obviously a valid one - it is just that, one valid > opinion - and there are others. PHP never took this opinion as an > axiomatic requirement (and not because of register_globals) - instead, the > intent was to have a default value for uninitialized variables - a > consistent, documented behavior since the dawn of the language. Can this > be problematic under certain situations? Absolutely. Can it be useful in > other cases? Sure (which is why it's very common). A great deal of folks > both rely on this behavior and *like *it. Those who don't (and there's > plenty of those as well of course) - always had a reasonable solution of > enabling `E_STRICT` and enforcing E_STRICT-compliant code. I still think > that having a strict mode (which can encompass strict types, strict ops, > stricter error behavior, etc.) makes a lot of sense and would arguably be a > superior option for the many folks who prefer a stricter language - but > there's simply no way we can change one of the most fundamental behaviors > of the language and force it down people's throats - not only because it > breaks compatibility, but because it breaks how many people are used to > write their PHP code. Perl provided stricter-liking folks with a solution > in the form of 'use strict;' decades ago; JS did something similar much > more recently. Neither of these created any sort of bifurcation - it's a > simple, sensible solution that has virtually no downsides. > > While I like Zeev's idea of making it opt-in, I think that a deprecation path is needed at the very least. I think this has the potential to be an even bigger issue to deal with than short tags. At least short tags have been discouraged for a long time. The first short tags RFC would have probably lead to a delay in upgrading to 8.0. This RFC would pretty much guarantee never being able to upgrade to 8.0 until we've totally retired our legacy code base... which will probably be just in time for PHP 28.0 - assuming the PHP project isn't dead at that point due to this RFC. > Zeev > -- Chase Peeler chasepeeler@gmail.com --- ## #106726 — Chase Peeler — 2019-08-28T13:48:14+00:00 URL: https://externals.io/message/106713#106726 · In reply to: #106725 > > > > > > […] > path is needed at the very least. I think this has the potential to be an > even bigger issue to deal with than short tags. At least short tags have > been discouraged for a long time. The first short tags RFC would have > probably lead to a delay in upgrading to 8.0. This RFC would pretty much > guarantee never being able to upgrade to 8.0 until we've totally retired > our legacy code base... which will probably be just in time for PHP 28.0 - > assuming the PHP project isn't dead at that point due to this RFC. > > I've been told I might not have been totally clear in my post. My position is that this should be done as an opt-in feature like Zeev as proposed. If it is not done as an opt-in feature, then I don't think it should be done at all. If it is still done, then I think a deprecation path is a must. As mentioned earlier, this doesn't necessarily mean E_DEPRECATION messages - warnings will work too. The key is that error logs with more urgency than notices are created that users can use to track down and fix issues. > Zeev > > […] > > > -- > Chase Peeler > chasepeeler@gmail.com > -- Chase Peeler chasepeeler@gmail.com --- ## #106727 — Chase Peeler — 2019-08-28T14:11:08+00:00 URL: https://externals.io/message/106713#106727 On Wed, Aug 28, 2019 at 9:55 AM Reindl Harald wrote: > > > Am 28.08.19 um 15:48 schrieb Chase Peeler: > > […] > > hell, there are notices for at least a decade > > error_log = "/var/log/php_error.log" > error_reporting = `E_ALL` > display_errors = 1 > > this is my development *as well* production config on every machine > since 16 years now and every warning/notice get fixed because every 30 > minutes the errorlog get mailed to every developer and admin > > turn logging on and start cleanup what you would have had a decade time. Am I the only one getting tired of people telling me how I need to operate just like they do? Everyone's circumstances are different. Just because something is easy for you, or works well for you, doesn't mean it's easy or even feasible for someone else. I've learned it's useless to try to explain how my situation might not be exactly like everyone else's, so I'm not going to waste the time doing it again. Why can't anyone see that taking on these myopic "My situation is the only valid situation" views are going to kill this language. "I always initialize my variables, so, it's no big deal if everyone else is forced to do so." "I was able to find and replace all the short tags in my code in 5 minutes, so it should be easy for everyone else to do as well." "We fix every single notice within 30 minutes of it popping up in a log. You should be able to do that as well." etc. etc. etc. Also, if you are going to try and tell me how I should my job, at least have the guts to reply to the entire list instead of sending it to me in a private reply. I'm obviously not afraid to turn around to make your reply public, so it'll save everyone a bit of time. -- Chase Peeler chasepeeler@gmail.com --- ## #106728 — Chase Peeler — 2019-08-28T14:15:48+00:00 URL: https://externals.io/message/106713#106728 · In reply to: #106727 Well, one reason I was so vocal about short tags wasn't a love for short tags themselves. It wasn't even to prevent the detrimental effects of removing them. Honestly, the 2nd RFC wasn't a horrible option. It was more about the precedent that it set - pushing huge BC breaks on the users (most of which are voiceless in the process, because they aren't involved in the community at all, most not really aware there is anything to be involved with) with little, if any, positive gain. Let's see how this plays out a bit longer. It's too early to tell if it's a dying attempt to keep pushing those type of changes, which are destined to be rejected, or, evidence that we are still in danger of having such a precedent set. > > > On Wed, Aug 28, 2019 at 9:55 AM Reindl Harald > wrote: > > > […] > > > Am I the only one getting tired of people telling me how I need to operate > just like they do? Everyone's circumstances are different. Just because > something is easy for you, or works well for you, doesn't mean it's easy or > even feasible for someone else. I've learned it's useless to try to explain > how my situation might not be exactly like everyone else's, so I'm not > going to waste the time doing it again. > > Why can't anyone see that taking on these myopic "My situation is the only > valid situation" views are going to kill this language. > > "I always initialize my variables, so, it's no big deal if everyone else > is forced to do so." > "I was able to find and replace all the short tags in my code in 5 > minutes, so it should be easy for everyone else to do as well." > "We fix every single notice within 30 minutes of it popping up in a log. > You should be able to do that as well." > etc. etc. etc. > > Also, if you are going to try and tell me how I should my job, at least > have the guts to reply to the entire list instead of sending it to me in a > private reply. I'm obviously not afraid to turn around to make your reply > public, so it'll save everyone a bit of time. > > -- > Chase Peeler > chasepeeler@gmail.com > -- Chase Peeler chasepeeler@gmail.com --- ## #106729 — Gert — 2019-08-28T14:20:15+00:00 URL: https://externals.io/message/106713#106729 · In reply to: #106728 Maybe i'm misunderstanding something here, but what does turning notices into deprecations achieve? Because if you have deprecation notices being logged then it shouldn't be extra work to log notices/warnings as well right? > > Well, one reason I was so vocal about short tags wasn't a love for short > tags themselves. It wasn't even to prevent the detrimental effects of > removing them. Honestly, the 2nd RFC wasn't a horrible option. It was more > about the precedent that it set - pushing huge BC breaks on the users (most > of which are voiceless in the process, because they aren't involved in the > community at all, most not really aware there is anything to be involved > with) with little, if any, positive gain. > > Let's see how this plays out a bit longer. It's too early to tell if it's a > dying attempt to keep pushing those type of changes, which are destined to > be rejected, or, evidence that we are still in danger of having such a > precedent set. > > > > > > > […] > > > -- > Chase Peeler > chasepeeler@gmail.com --- ## #106730 — Matthew Brown — 2019-08-28T14:22:22+00:00 URL: https://externals.io/message/106713#106730 · In reply to: #106713 Looking at our notice logs, I estimate (fairly roughly) that it would require about a week's worth of my time to fix these issues in vimeo.com’s 700K LOC codebase (the undefined variables are confined to our views). IMO it's akin to taking the training wheels off the language – I think the PHP ecosystem is mature enough to deal with the change. > Hi internals, > > I think it's time to take a look at our existing warnings & notices in the > engine, and think about whether their current classification is still > appropriate. Error conditions like "undefined variable" only generating a > notice is really quite mind-boggling. > > I've prepared an RFC with some suggested classifications, though there's > room for bikeshedding here... > > https://wiki.php.net/rfc/engine_warnings > > Regards, > Nikita --- ## #106731 — Mark Randall — 2019-08-28T14:24:33+00:00 URL: https://externals.io/message/106713#106731 · In reply to: #106726 > My position is that this should be done as an opt-in feature like Zeev as > proposed. If it is not done as an opt-in feature, then I don't think it > should be done at all. IMO one shouldn't have to opt-in to use common-sense error checking of engine operations. It's essential to think of the long-term future of PHP and what makes sense going forward, and to my mind that means things need to be more intuitive and less prone to mistakes. Be it Little Jonny Just-Grad who is starting his first proper PHP project, or the lead engineer of Megacorp, Inc. who is about to write the first line of the company's next multi-million dollar hit, they shouldn't need to pile-up on declares at the top of each page just to make the engine perform as intuitive common sense would dictate. By the very nature of using @ to suppress error messages, the examples given are all fully aware that the behaviour they are using is not good practice. If the engine was perfectly confident in your use of the statements that followed it, then it wouldn't display an error that you would need to suppress in the first place, now would it? Besides, we have tools available for years now to make this behaviour more defined: $counts[$key] = ($counts[$key] ?? 0) + 1; "If $counts[$key] is not set, use the value 0". Null Coalescing was explicitly designed to provide for this behaviour. -- Mark Randall --- ## #106732 — Chase Peeler — 2019-08-28T14:27:26+00:00 URL: https://externals.io/message/106713#106732 · In reply to: #106729 > Maybe i'm misunderstanding something here, but what does turning > notices into deprecations achieve? Because if you have deprecation > notices being logged then it shouldn't be extra work to log > notices/warnings as well right? > > Notices include a lot more than just undeclared variables. Turning them on in our environment would pretty much make the logs unusable for any on the spot checking for issues. They would only be useful if we were trying to parse out specific errors, and, they would be HUGE. Each web server generates about 5-10 megs of logs in a day. Our CLI servers (which runs beanstalkd jobs) generates about 80-100 megs of logs in a day. That's without notices turned on. > > > […] > more > > […] > (most > > […] > the > > […] > it's a > > […] > to > > […] > wrote: > > […] > messages > > […] > time. > > […] > operate > > […] > easy or > > […] > explain > > […] > only > > […] > else > > […] > log. > > […] > in a > > […] > reply > > […] > -- Chase Peeler chasepeeler@gmail.com --- ## #106733 — Marco Pivetta — 2019-08-28T14:39:26+00:00 URL: https://externals.io/message/106713#106733 · In reply to: #106732 > > Notices include a lot more than just undeclared variables. Turning them on > in our environment would pretty much make the logs unusable for any on the > spot checking for issues. They would only be useful if we were trying to > parse out specific errors, and, they would be HUGE. Each web server > generates about 5-10 megs of logs in a day. Our CLI servers (which runs > beanstalkd jobs) generates about 80-100 megs of logs in a day. That's > without notices turned on. I worked with clients with much more log overhead happening: the solution is working to fix these issues, and not ignoring more of them. Marco Pivetta http://twitter.com/Ocramius http://ocramius.github.com/ --- ## #106734 — Rowan Collins — 2019-08-28T14:47:17+00:00 URL: https://externals.io/message/106713#106734 >By the very nature of using @ to suppress error messages, the examples >given are all fully aware that the behaviour they are using is not good practice. I think that is a fault in the examples. I have never seen @ used to squash these messages, but have frequently worked with codebases where logging at the `E_NOTICE` level would have filled megabytes of log file in minutes. As I said earlier, the fact that something is bad practice doesn't stop it being difficult to locate and change. The requests for a deprecation period, or an opt-in mode, are about that change, not a disagreement about the principle. Regards, -- Rowan Collins [IMSoP] --- ## #106735 — Rowan Collins — 2019-08-28T14:52:24+00:00 URL: https://externals.io/message/106713#106735 · In reply to: #106733 >I worked with clients with much more log overhead happening: the >solution >is working to fix these issues, and not ignoring more of them. Being right is not the same as being easy, or being the top priority for an organisation with limited resources. Regards, -- Rowan Collins [IMSoP] --- ## #106736 — Chase Peeler — 2019-08-28T14:54:32+00:00 URL: https://externals.io/message/106713#106736 · In reply to: #106733 > On Wed, Aug 28, 2019 at 4:27 PM Chase Peeler > wrote: > > > […] > > > I worked with clients with much more log overhead happening: the solution > is working to fix these issues, and not ignoring more of them. > > You going to come and fix the issues? It's an internal application and most of those messages are coming from legacy areas of the code which are mainly "it works, so leave it alone" things. Instead of going back and spending time trying to fix those boondoggles, we invest the time of our developers (there are 2 others besides myself) into building new features that help our business grow. Time permitting, we try and update some of the legacy areas, but, we usually find it's a better investment to just rebuild them at that point. Bottom line is that we live with the not-so-good stuff so that we can focus on adding new great stuff. The not-so-good stuff isn't holding us back, and trying to fix things like undeclared variables would have absolutely ZERO positive effect on our business, which uses this application every day. If I went to our executive team and said "Can we delay that new scheduling system that will really help our business so I can go back and update code to get rid of these undeclared variable notices?" I'd get laughed at! Like I've said before - can we please stop pretending we understand everyone else's situation? Maybe my situation is unique. My gut tells me it might be unique among people on this list, but, that it's actually pretty common among the myriad of developers out there which never get involved in these discussions. > Marco Pivetta > > http://twitter.com/Ocramius > > http://ocramius.github.com/ > > -- Chase Peeler chasepeeler@gmail.com --- ## #106737 — Rowan Collins — 2019-08-28T14:56:29+00:00 URL: https://externals.io/message/106713#106737 · In reply to: #106730 >Looking at our notice logs, I estimate (fairly roughly) that it would >require about a week's worth of my time to fix these issues I honestly thought you were posting that as an argument against. A week of resource (plus the accompanying QA impact etc) is a significant investment for many organisations. That's why it has the potential to delay adoption of a new version, and why a long lead-in via deprecation or opt-in is necessary. Regards, -- Rowan Collins [IMSoP] --- Next page: https://externals.io/message/106713.md?page=2