# [VOTE] Reclassify E_STRICT notices - Thread: https://externals.io/message/84838 - Original archive: https://news-web.php.net/php.internals/84838 - Started by Nikita Popov on 2015-03-15T15:46:08+00:00 - Messages: 17 - 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 --- ## #84838 — Nikita Popov — 2015-03-15T15:46:08+00:00 URL: https://externals.io/message/84838#84838 Hi internals! To ensure we have no shortage of new RFC votes... https://wiki.php.net/rfc/reclassify_e_strict#vote Voting is open for ten days :) Thanks, Nikita --- ## #84853 — Markus Fischer — 2015-03-15T17:32:29+00:00 URL: https://externals.io/message/84838#84853 · In reply to: #84838 > To ensure we have no shortage of new RFC votes... > > https://wiki.php.net/rfc/reclassify_e_strict#vote > > Voting is open for ten days :) From the RFC: "Signature mismatch during inheritance ... Possible alternative: Convert to E_DEPRECATED, if we intend to make this a fatal error in the future. " Has there been any decision made which course to follow? I ask since I don't see voting option for it (not saying it needs one!), it just would be nice to carve in stone what's going to be done about it. Otherwise good cleanup RFC IMHO, +1 - Markus --- ## #84854 — Nikita Popov — 2015-03-15T17:35:13+00:00 URL: https://externals.io/message/84838#84854 · In reply to: #84853 > > > […] > > From the RFC: > > "Signature mismatch during inheritance > ... > Possible alternative: Convert to E_DEPRECATED, if we intend to make this > a fatal error in the future. > " > > Has there been any decision made which course to follow? I ask since I > don't see voting option for it (not saying it needs one!), it just would > be nice to carve in stone what's going to be done about it. > > Otherwise good cleanup RFC IMHO, +1 > Thanks, I forgot to drop that before starting the vote. The note is gone now :) Nikita --- ## #84866 — Matteo Beccati — 2015-03-15T18:30:42+00:00 URL: https://externals.io/message/84838#84866 · In reply to: #84838 Hi Nikita, > Hi internals! > > To ensure we have no shortage of new RFC votes... > > https://wiki.php.net/rfc/reclassify_e_strict#vote > > Voting is open for ten days :) I know I'm late, but with I just have found the required time to test the branch with my own legacy app. The result of the test suite run is pretty bad because of the new warnings. More than half of the tests fail with a message similar to: https://revive.beccati.com/bamboo/browse/REV-EXP-NIK-1/test/case/19183062 Now, some of them would be fairly easy to fix. But, once again the problem lies in the bundled PEAR libs. For example the raiseError() method has been redefined all over the place with a custom signature. Fixing is certainly possible, but fixing will require a fairly big refactoring. In PHP4 times it was in fact quite common to change inherited method signatures to bend them to one's will and/or remove parameters and hardcode them in the parent constructor call. We now know it is bad practice, but I bet there's lot of code using these practices in controlled situations. I'm going to attempt fixing the app code (including the bundled pear libs) and report back. Cheers -- Matteo Beccati Development & Consulting - http://www.beccati.com/ --- ## #84962 — Pierre Joye — 2015-03-16T01:34:40+00:00 URL: https://externals.io/message/84838#84962 · In reply to: #84838 hi Nikita, > Hi internals! > > To ensure we have no shortage of new RFC votes... > > https://wiki.php.net/rfc/reclassify_e_strict#vote > > Voting is open for ten days :) I am late on that but I need one clarification about: "Some of the strict standards notices are converted to an error category that is considered more severe. As such error handlers might treat it more severely, resulting in BC breakage." Does it mean that some errors may end on recoverable error? Aka, an actual break, or simply a higher error level, a warning in the worst case? If it is the sooner, only for few cases, then I will vote no as I think it is not a good move. If the latter, I am all in. Cheers, -- Pierre @pierrejoye | http://www.libgd.org --- ## #84968 — Dennis Birkholz — 2015-03-16T02:17:19+00:00 URL: https://externals.io/message/84838#84968 · In reply to: #84962 Hello Pierre, Am 16.03.2015 um 02:34 schrieb Pierre Joye: > "Some of the strict standards notices are converted to an error > category that is considered more severe. As such error handlers might > treat it more severely, resulting in BC breakage." > > Does it mean that some errors may end on recoverable error? Aka, an > actual break, or simply a higher error level, a warning in the worst > case? in the "Proposal" section the RFC states: "The following section lists all strict standards notices currently in use (with code sample and error message) as well as the proposed resolution and the reasoning behind it." And then follow the actual changes: Indexing by a resource => `E_NOTICE` Abstract static methods => Remove notice "Redefining" a constructor => obsolete Signature mismatch during inheritance => `E_WARNING` Same (compatible) property in two used traits => Remove notice Accessing static property non-statically => `E_NOTICE` Only variables should be assigned by reference => `E_NOTICE` Only variables should be passed by reference => `E_NOTICE` Calling non-static methods statically => `E_DEPRECATED` So the RFC should not introduce a fatal error or something not catchable by a custom error handler. Greets Dennis --- ## #85014 — Peter Cowburn — 2015-03-16T09:24:22+00:00 URL: https://externals.io/message/84838#85014 · In reply to: #84838 > Hi internals! > > To ensure we have no shortage of new RFC votes... > > https://wiki.php.net/rfc/reclassify_e_strict#vote > > Voting is open for ten days :) > Nikita, don't forget to start a new thread with the tag [VOTE] in the subject line as per normal procedure. Thanks in advance. > > Thanks, > Nikita --- ## #85030 — Matteo Beccati — 2015-03-16T11:43:41+00:00 URL: https://externals.io/message/84838#85030 · In reply to: #84866 Hi Nikic, > In PHP4 times it was in fact quite common to change inherited method > signatures to bend them to one's will and/or remove parameters and > hardcode them in the parent constructor call. We now know it is bad > practice, but I bet there's lot of code using these practices in > controlled situations. > > I'm going to attempt fixing the app code (including the bundled pear > libs) and report back. So... I've spent a few hours on the cleanup and I'm still far from getting a green build. As you mentioned in the RFC, turning `E_STRICT` into `E_WARNING` is going to be a BC-break for someone, and fixing this one in particular requires far more effort than a simple search/replace. For some applications it means heavy refactorings to ensure that all the inherited methods have a compatible signature. That's enforced even if the classes were never designed nor are expected to be interchangeable (e.g. PEAR and its packages, many of which extend PEAR itself), or if some "shortcuts" have been made (e.g. function foo() { parent::foo("BAR"); } ). I agree that it's a wrong usage of inheritance and we shouldn't encourage it, but maybe an `E_NOTICE` would be good enough to avoid such design mistakes. Cheers -- Matteo Beccati Development & Consulting - http://www.beccati.com/ --- ## #85043 — Markus Fischer — 2015-03-16T12:28:59+00:00 URL: https://externals.io/message/84838#85043 · In reply to: #85030 Hi Matteo, > > > […] > > So... I've spent a few hours on the cleanup and I'm still far from > getting a green build. > > As you mentioned in the RFC, turning `E_STRICT` into `E_WARNING` is going to > be a BC-break for someone, and fixing this one in particular requires > far more effort than a simple search/replace. am I correct assuming that your existing test suite was running with `E_STRICT` excluded from error_reporting ? thank you, - Markus --- ## #85049 — Matteo Beccati — 2015-03-16T12:53:47+00:00 URL: https://externals.io/message/84838#85049 · In reply to: #85043 > am I correct assuming that your existing test suite was running with > `E_STRICT` excluded from error_reporting ? Of course. Cheers -- Matteo Beccati Development & Consulting - http://www.beccati.com/ --- ## #85454 — Pascal MARTIN — 2015-03-24T19:58:42+00:00 URL: https://externals.io/message/84838#85454 · In reply to: #84838 Le 15/03/2015 16:46, Nikita Popov a écrit : > https://wiki.php.net/rfc/reclassify_e_strict#vote > Voting is open for ten days :) Hi, We talked about this RFC with other people at AFUP, and it seems we are on the +1 side. Basically, with "strict" warnings, one might wonder whether or not his code is OK. With warnings and/or notices, it will be more clear that things are not just OK. Thanks! -- Pascal MARTIN http://blog.pascal-martin.fr/ @pascal_martin --- ## #85469 — Nikita Popov — 2015-03-25T16:14:59+00:00 URL: https://externals.io/message/84838#85469 · In reply to: #84838 > Hi internals! > > To ensure we have no shortage of new RFC votes... > > https://wiki.php.net/rfc/reclassify_e_strict#vote > > Voting is open for ten days :) > RFC is accepted with 28 votes in favor and 4 against. Nikita --- ## #85636 — Nikita Popov — 2015-04-01T14:57:59+00:00 URL: https://externals.io/message/84838#85636 · In reply to: #85469 > On Sun, Mar 15, 2015 at 4:46 PM, Nikita Popov > wrote: > > > […] > > RFC is accepted with 28 votes in favor and 4 against. > The RFC is now implemented. However while landing the patch I noticed that I missed a number of `E_STRICT` notices in libraries. In particular: * `mktime()` without arguments throws "You should be using the `time()` function instead" * `htmlentities()` with some encodings like EUC-JP throws "Only basic entities substitution is supported for multi-byte encodings other than UTF-8; functionality is equivalent to htmlspecialchars" * mysqli::next_result() if there is no next result throws "There is no next result set" While the first one sounds like something that should be deprecated, I couldn't say what to do with the other two. Nikita --- ## #85637 — Alex Bowers — 2015-04-01T15:24:58+00:00 URL: https://externals.io/message/84838#85637 · In reply to: #85636 Is the last one really a strict? Sounds like it should be a warning to me. Similar to when you for each over something not an array > On Wed, Mar 25, 2015 at 5:14 PM, Nikita Popov > wrote: > > > […] > > The RFC is now implemented. However while landing the patch I noticed that > I missed a number of `E_STRICT` notices in libraries. In particular: > > * `mktime()` without arguments throws "You should be using the `time()` > function instead" > > * `htmlentities()` with some encodings like EUC-JP throws "Only basic > entities substitution is supported for multi-byte encodings other than > UTF-8; functionality is equivalent to htmlspecialchars" > > * mysqli::next_result() if there is no next result throws "There is no next > result set" > > While the first one sounds like something that should be deprecated, I > couldn't say what to do with the other two. > > Nikita --- ## #85676 — Stanislav Malyshev — 2015-04-01T21:11:10+00:00 URL: https://externals.io/message/84838#85676 · In reply to: #85636 Hi! > * `htmlentities()` with some encodings like EUC-JP throws "Only basic > entities substitution is supported for multi-byte encodings other than > UTF-8; functionality is equivalent to htmlspecialchars" This looks like notice. > * mysqli::next_result() if there is no next result throws "There is no next > result set" This I'm not sure should produce anything at all - it has the return value of false, should be good enough. -- Stas Malyshev smalyshev@gmail.com --- ## #85679 — Yasuo Ohgaki — 2015-04-01T23:15:22+00:00 URL: https://externals.io/message/84838#85679 · In reply to: #85676 Hi Nikita, On Thu, Apr 2, 2015 at 6:11 AM, Stanislav Malyshev wrote: > > […] > > This looks like notice. > `htmlentities()` does not work as it should for multibyte encodings other than UTF-8. It should be converted to E_NOTICE. > * mysqli::next_result() if there is no next result throws "There is no > next > > […] > > This I'm not sure should produce anything at all - it has the return > value of false, should be good enough. Same here, too. Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #85684 — Lester Caine — 2015-04-02T08:36:54+00:00 URL: https://externals.io/message/84838#85684 · In reply to: #85676 > > […] > This I'm not sure should produce anything at all - it has the return > value of false, should be good enough. 'Traditional' scanning of a result set on most of the drivers simply takes the false/null as 'end of file' so any exception generated should be able to be switched off. But it's the same discussion as throwing exceptions when an object can't be created. One workflow simply needs the 'null', while an exception based workflow needs the exception? -- 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