# Reclassify E_STRICT notices - Thread: https://externals.io/message/83498 - Original archive: https://news-web.php.net/php.internals/83498 - Started by Nikita Popov on 2015-02-22T22:30:47+00:00 - Messages: 16 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #83498 — Nikita Popov — 2015-02-22T22:30:47+00:00 URL: https://externals.io/message/83498#83498 Hi internals! I would like to propose reclassifying our few existing `E_STRICT` notices and removing this error category: https://wiki.php.net/rfc/reclassify_e_strict As we don't really have good guidelines on when which type of error should be thrown, I'm mainly going by what category other similar errors use. I'm open to suggestions, but hope this will not deteriorate into total bikeshed. Thanks, Nikita --- ## #83503 — Lester Caine — 2015-02-22T23:12:37+00:00 URL: https://externals.io/message/83498#83503 · In reply to: #83498 > I would like to propose reclassifying our few existing `E_STRICT` notices and > removing this error category: > > https://wiki.php.net/rfc/reclassify_e_strict > > As we don't really have good guidelines on when which type of error should > be thrown, I'm mainly going by what category other similar errors use. I'm > open to suggestions, but hope this will not deteriorate into total bikeshed. At last something which fits my roadmap ... Only problem is '"Redefining" a constructor' which I certainly accept while upgrading code, but I also appreciate why 'Remove PHP4 Constructors' might not be accepted leaving a difficulty. I think one ends up with still needing a 'mode' switch if the legacy constructors are retained? -- Lester Caine - G8HFL ----------------------------- Contact - http://lsces.co.uk/wiki/?page=contact L.S.Caine Electronic Services - http://lsces.co.uk EnquirySolve - http://enquirysolve.com/ Model Engineers Digital Workshop - http://medw.co.uk Rainbow Digital Media - http://rainbowdigitalmedia.co.uk --- ## #83543 — Yasuo Ohgaki — 2015-02-23T05:32:15+00:00 URL: https://externals.io/message/83498#83543 · In reply to: #83498 Hi Nikita, > I would like to propose reclassifying our few existing `E_STRICT` notices and > removing this error category: > > https://wiki.php.net/rfc/reclassify_e_strict > > As we don't really have good guidelines on when which type of error should > be thrown, I'm mainly going by what category other similar errors use. I'm > open to suggestions, but hope this will not deteriorate into total > bikeshed. > +1 overall. Regarding "Only variables should be assigned by reference" Most of errors are appropriate, but some of them may be removed. For example, literals do not make sense so current behavior is good. $ php -r 'array_pop([1,2,3]);' PHP Fatal error: Only variables can be passed by reference in Command line code on line 1 However, emitting "Only variables should be assigned by reference" for this $top = array_pop(some_func_returns_array()); // Code needs only top element seems too strict, for example. I would rather PHP behaves like HHVM - http://3v4l.org/5AIrb. - http://3v4l.org/O0SXE Is it possible relax the error for tmp variables? Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #83605 — Rowan Collins — 2015-02-23T20:39:13+00:00 URL: https://externals.io/message/83498#83605 · In reply to: #83498 Nikita Popov wrote on 22/02/2015 22:30: > Hi internals! > > I would like to propose reclassifying our few existing `E_STRICT` notices and > removing this error category: > > https://wiki.php.net/rfc/reclassify_e_strict > > As we don't really have good guidelines on when which type of error should > be thrown, I'm mainly going by what category other similar errors use. I'm > open to suggestions, but hope this will not deteriorate into total bikeshed. I think my preferred approach would be to first come up with some defined criteria for different levels of warning, and then look at how many we need, and how well they map to the current situation. At the moment we have 3 severity levels: E_STRICT, E_NOTICE, and E_WARNING. Many people will run with only `E_WARNING` turned on, because of the number of messages in the lower levels. There have been a couple of requests recently to raise messages between these severities, which I mentioned before leads to a kind of "inflation" unless we also lower the severity of other messages. It may be that we only need two different severities, but as you say, we don't have any guidelines for how to use those two, so dumping a few extra things in `E_NOTICE` and a few in `E_WARNING` doesn't really make things any clearer to the user. It's also possible that we need more than two *categories* of warning, but not as a strict hierarchy of *severities* - the aim of the classification being to allow the user to make decisions about which warnings they want to see or handle. It's already hard to say whether `E_DEPRECATED` has a relative severity, or is actually orthogonal; perhaps there should be combinations like E_IMPORTANT_DEPRECATED = `E_DEPRECATED` + E_WARNING... Obviously, this is a tougher job than what you have proposed, and since I keep mentioning it, I should probably stop talking and start doing, but I'm not convinced of the value of tinkering around the edges without tackling the underlying problem. Regards, -- Rowan Collins [IMSoP] --- ## #83635 — Stanislav Malyshev — 2015-02-24T07:20:13+00:00 URL: https://externals.io/message/83498#83635 · In reply to: #83498 Hi! > > I would like to propose reclassifying our few existing `E_STRICT` notices and > removing this error category: > > https://wiki.php.net/rfc/reclassify_e_strict Could you add some more explanation about why it is a good thing? I.e., `E_STRICT` has pretty clear place and hurts no one, why remove it? What we are trying to accomplish by that? I'm just not seeing how it would be better without E_STRICT. -- Stas Malyshev smalyshev@gmail.com --- ## #83642 — Lester Caine — 2015-02-24T08:09:38+00:00 URL: https://externals.io/message/83498#83642 · In reply to: #83635 > > […] > Could you add some more explanation about why it is a good thing? I.e., > `E_STRICT` has pretty clear place and hurts no one, why remove it? What we > are trying to accomplish by that? I'm just not seeing how it would be > better without E_STRICT. I don't think that it is necessary to remove the error itself, but some of the coding style elements it currently 'allows' need to be reviewed to see if it is time they were deprecated, allowing some of the new 'strict' elements to be tagged properly in PHP7 ? -- Lester Caine - G8HFL ----------------------------- Contact - http://lsces.co.uk/wiki/?page=contact L.S.Caine Electronic Services - http://lsces.co.uk EnquirySolve - http://enquirysolve.com/ Model Engineers Digital Workshop - http://medw.co.uk Rainbow Digital Media - http://rainbowdigitalmedia.co.uk --- ## #83669 — Nikita Popov — 2015-02-24T15:02:47+00:00 URL: https://externals.io/message/83498#83669 · In reply to: #83635 On Tue, Feb 24, 2015 at 8:20 AM, Stanislav Malyshev wrote: > Hi! > > > […] > and > > […] > > Could you add some more explanation about why it is a good thing? I.e., > `E_STRICT` has pretty clear place and hurts no one, why remove it? What we > are trying to accomplish by that? I'm just not seeing how it would be > better without E_STRICT. > The motivation behind this is pretty ad-hoc. I have noticed that we have a number of `E_STRICT` notices that seem pretty bogus and should be dropped. I've also noticed that we throw `E_STRICT` for some imho relatively severe errors (like signature mismatch, which fatals in other circumstances), in which case the level should be increased. So in the end it's less about "lets drop E_STRICT" and more about "after reclassifying some `E_STRICT` notices, there isn't really anything left in this category". In addition to that, I don't like the semantics that "strict standards" notices convey. Many (professional) PHP developers target notice-free code (which does not throw any kind of warnings/notices that are not explicitly suppressed) and from this perspective "strict standards" notices are a weird concept. Either the code is okay to write or it isn't, don't throw a notice to tell me to "consider using an accessor" (because that informational notice will present use in notice-free code). So this RFC drops the portion of `E_STRICT` that appears bogus/informational to me (the three mentioned in the RFC and the by-ref cases discussed in a separate thread) and moves the rest to proper notices / warnings / deprecations. Nikita --- ## #83752 — Derick Rethans — 2015-02-25T09:32:42+00:00 URL: https://externals.io/message/83498#83752 · In reply to: #83498 > I would like to propose reclassifying our few existing `E_STRICT` > notices and removing this error category: > > https://wiki.php.net/rfc/reclassify_e_strict > > As we don't really have good guidelines on when which type of error > should be thrown, I'm mainly going by what category other similar > errors use. I'm open to suggestions, but hope this will not > deteriorate into total bikeshed. Those guidelines where part of the original proposal though: http://grokbase.com/t/php/php-internals/06aq0a1vzx/rfc-e-deprecated Which interestingly mentions your "Abstract static methods" case. And I did write something up (with my opinions of it): http://derickrethans.nl/erecoverableerror.html In any case, some comments on a few of the cases: "Redefining" a constructor - I think that should be retained (or an E_NOTICE) as it's something that might catch people out. I think it helps enough to warrant it. "Same (compatible) property in two used traits" - I think that should be changed to an E_NOTICE, or not at all, if it's already an E_NOTICE. For a similar reason as above. "Accessing static property non-statically" - I think this should stay E_STRICT, as it falls in the original proposal's category of "any rule that reflects common strict standards, like OOP theory that is considered harmless if not followed" cheers, Derick --- ## #84176 — Julien Pauli — 2015-03-02T18:13:16+00:00 URL: https://externals.io/message/83498#84176 · In reply to: #83752 > > > > […] > > Those guidelines where part of the original proposal though: > http://grokbase.com/t/php/php-internals/06aq0a1vzx/rfc-e-deprecated > Which interestingly mentions your "Abstract static methods" case. > > And I did write something up (with my opinions of it): > http://derickrethans.nl/erecoverableerror.html > > In any case, some comments on a few of the cases: > > "Redefining" a constructor > - I think that should be retained (or an E_NOTICE) as it's something > that might catch people out. I think it helps enough to warrant it. > > "Same (compatible) property in two used traits" > - I think that should be changed to an E_NOTICE, or not at all, if it's > already an E_NOTICE. For a similar reason as above. > > "Accessing static property non-statically" > - I think this should stay E_STRICT, as it falls in the original > proposal's category of "any rule that reflects common strict > standards, like OOP theory that is considered harmless if not > followed" > > I'm not against removing `E_STRICT` and reclassifying the errors, however, like Derick , I'd like to keep the trait and redefining constructor ones as they may really help to spot problems. About the accessing static property non statically, I would have thrown an `E_ERROR` : the property simply doesn't exist. Class properties should be accessed using the class, not an instance of it , IMO. Julien.P --- ## #84177 — Stanislav Malyshev — 2015-03-02T18:37:33+00:00 URL: https://externals.io/message/83498#84177 · In reply to: #84176 Hi! > About the accessing static property non statically, I would have thrown an > `E_ERROR` : the property simply doesn't exist. Class properties should be > accessed using the class, not an instance of it , IMO. Sometimes, there's no fixed class but there's an object of this class. Doing something like $klass = get_class($foo); $klass::property is possible, but $foo->property is easier. -- Stas Malyshev smalyshev@gmail.com --- ## #84221 — Nikita Popov — 2015-03-03T09:55:33+00:00 URL: https://externals.io/message/83498#84221 · In reply to: #84177 On Mon, Mar 2, 2015 at 7:37 PM, Stanislav Malyshev wrote: > Hi! > > > […] > an > > […] > > Sometimes, there's no fixed class but there's an object of this class. > Doing something like $klass = get_class($foo); $klass::property is > possible, but $foo->property is easier. > Doing $obj::$property is possible as well, it does the same as get_class($obj)::$property. Nikita --- ## #84670 — Patrick ALLAERT — 2015-03-13T09:33:39+00:00 URL: https://externals.io/message/83498#84670 · In reply to: #83498 Hi Nikita, Le dim. 22 févr. 2015 à 23:31, Nikita Popov a écrit : Hi internals! > > I would like to propose reclassifying our few existing `E_STRICT` notices and > removing this error category: > > https://wiki.php.net/rfc/reclassify_e_strict > > As we don't really have good guidelines on when which type of error should > be thrown, I'm mainly going by what category other similar errors use. I'm > open to suggestions, but hope this will not deteriorate into total > bikeshed. > > Thanks, > Nikita > I am *very* glad you bring this subject on the table and I fully agree with the overall picture, I yet have to see the details as soon as I have some time. Just a remark regarding E_STRICT: since its introduction in PHP 5.0, it provided a way for people migrating from 4.x to 5.x with error_reporting = `E_ALL` to focus on the conversion of existing (legacy) application, while not forcing people to take care of changing things that "worked before but are now considered not so good" (yaaaaayyy, BC!). At the same time, people developing new applications could (via opt-in!) enjoy some more strictness thanks to this flag by adding it to the error_reporting. A few versions/years later, PHP 5.4 integrated `E_STRICT` as part of `E_ALL` and it encourages people to give attention to `E_STRICT` in addition to the other levels. However, we are again a bit in the same situation now: how could we provide extra information to PHP 7.x users (via opt-in) about things that we consider bad practice/behaviour abuse/side effect, but that will continue to work? `E_DEPRECATED` may sound fine, but not always as it *imply* the feature will be gone some day. Could you think about this as this is somewhat related to `E_STRICT` history? A possible option (to investigate) would be to combine the various error levels with a new flag (E_NEW?): zend_error(E_WARNING | E_NEW, "This is a new warning as of 7.0!"); For BC: People having `E_WARNING` as part of their error_reporting without activating E_NEW would not see it. Think about this as: it's PHP 5.0 with error_reporting = `E_ALL` (without `E_STRICT` then). For more errors verbosity (new application development): One can activate `E_ALL` reporting (error_reporting = `E_ALL` | E_NEW or something else which doesn't expose the E_NEW flag ?) in order to see all new notices/warnings/... Later, instead of activating E_NEW by default (like including `E_STRICT` in E_ALL), the idea would be to remove the flag where appropriate so that: zend_error(E_WARNING, "This is a new warning as of 7.0!"); What's your opinion about this? (as a feature, not focusing on the details). Thanks, Patrick --- ## #84671 — Patrick ALLAERT — 2015-03-13T09:36:25+00:00 URL: https://externals.io/message/83498#84671 · In reply to: #84670 Le ven. 13 mars 2015 à 10:33, Patrick ALLAERT a écrit : > Hi Nikita, > > Le dim. 22 févr. 2015 à 23:31, Nikita Popov a > écrit : > > Hi internals! > > […] > > I am *very* glad you bring this subject on the table and I fully agree > with the overall picture, I yet have to see the details as soon as I have > some time. > > Just a remark regarding E_STRICT: since its introduction in PHP 5.0, it > provided a way for people migrating from 4.x to 5.x with error_reporting = > `E_ALL` to focus on the conversion of existing (legacy) application, while > not forcing people to take care of changing things that "worked before but > are now considered not so good" (yaaaaayyy, BC!). > At the same time, people developing new applications could (via opt-in!) > enjoy some more strictness thanks to this flag by adding it to the > error_reporting. > > A few versions/years later, PHP 5.4 integrated `E_STRICT` as part of `E_ALL` > and it encourages people to give attention to `E_STRICT` in addition to the > other levels. > > However, we are again a bit in the same situation now: how could we > provide extra information to PHP 7.x users (via opt-in) about things that > we consider bad practice/behaviour abuse/side effect, but that will > continue to work? `E_DEPRECATED` may sound fine, but not always as it *imply* > the feature will be gone some day. > > Could you think about this as this is somewhat related to `E_STRICT` history? > > A possible option (to investigate) would be to combine the various error > levels with a new flag (E_NEW?): > > zend_error(E_WARNING | E_NEW, "This is a new warning as of 7.0!"); > > For BC: > People having `E_WARNING` as part of their error_reporting without > activating E_NEW would not see it. Think about this as: it's PHP 5.0 with > error_reporting = `E_ALL` (without `E_STRICT` then). > > For more errors verbosity (new application development): > One can activate `E_ALL` reporting > FIX: s/activate `E_ALL` reporting/activate E_NEW reporting/ > (error_reporting = `E_ALL` | E_NEW or something else which doesn't expose > the E_NEW flag ?) in order to see all new notices/warnings/... > > Later, instead of activating E_NEW by default (like including `E_STRICT` in > E_ALL), the idea would be to remove the flag where appropriate so that: > > zend_error(E_WARNING, "This is a new warning as of 7.0!"); > > What's your opinion about this? (as a feature, not focusing on the > details). > > Thanks, > Patrick --- ## #84807 — Nikita Popov — 2015-03-15T08:34:54+00:00 URL: https://externals.io/message/83498#84807 · In reply to: #84176 > > > > […] > I'm not against removing `E_STRICT` and reclassifying the errors, however, > like Derick , I'd like to keep the trait and redefining constructor ones as > they may really help to spot problems. > The redefining constructor notice is already gone with the ctors RFC, so I won't comment on that. Regarding the trait warning: There is a tradeoff between a) this notice can spot problems and b) this notice completely disallows using this feature in professional code. I don't think having the same property in two traits is in any way fundamentally wrong if the declarations are compatible, so I don't see why we should (effectively) completely forbid it. Nikita --- ## #87387 — Ferenc Kovacs — 2015-07-30T09:24:12+00:00 URL: https://externals.io/message/83498#87387 · In reply to: #83498 > Hi internals! > > I would like to propose reclassifying our few existing `E_STRICT` notices and > removing this error category: > > https://wiki.php.net/rfc/reclassify_e_strict > > As we don't really have good guidelines on when which type of error should > be thrown, I'm mainly going by what category other similar errors use. I'm > open to suggestions, but hope this will not deteriorate into total > bikeshed. > > Thanks, > Nikita > hi, this RFC got accepted, but there are 4 more E_STRICTs in the core which were kept/missed: http://lxr.php.net/xref/PHP_TRUNK/ext/date/php_date.c#1544 http://lxr.php.net/xref/PHP_TRUNK/ext/standard/html.c#1241 http://lxr.php.net/xref/PHP_TRUNK/ext/mysqli/mysqli_api.c#1606 http://lxr.php.net/xref/PHP_TRUNK/ext/mysqli/mysqli_api.c#1644 for the sake of consistency I would like these to be removed/changed into some other error levels so that we emit no `E_STRICT` from the core. I cc'ed Andrey and Derick to this mail as I'm curious about their opinion on the mysql and date related ones. -- Ferenc Kovács @Tyr43l - http://tyrael.hu --- ## #87390 — Derick Rethans — 2015-07-30T11:38:53+00:00 URL: https://externals.io/message/83498#87390 · In reply to: #87387 > > > > […] > > this RFC got accepted, but there are 4 more E_STRICTs in the core > which were kept/missed: > http://lxr.php.net/xref/PHP_TRUNK/ext/date/php_date.c#1544 I think `E_DEPRECATED` works for this as well - which I am perfectly happy with. cheers, Derick