Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » [VOTE] Reclassify E_STRICT notices »

[VOTE] Reclassify E_STRICT notices

php.internals

Nikita Popov

11 years ago
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

Markus Fischer

11 years ago
On 15.03.15 16:46, Nikita Popov wrote:
> 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

Nikita Popov

11 years ago
On Sun, Mar 15, 2015 at 6:32 PM, Markus Fischer <markus@fischer.name> wrote:
> On 15.03.15 16:46, Nikita Popov wrote: > > 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 >
Thanks, I forgot to drop that before starting the vote. The note is gone now :) Nikita

Matteo Beccati

11 years ago
Hi Nikita, On 15/03/2015 16:46, Nikita Popov wrote:
> 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/

Matteo Beccati

11 years ago
Hi Nikic, On 15/03/2015 19:30, Matteo Beccati wrote:
> 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/

Markus Fischer

11 years ago
Hi Matteo, On 16.03.15 12:43, Matteo Beccati wrote:
> On 15/03/2015 19:30, Matteo Beccati wrote: >> 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.
am I correct assuming that your existing test suite was running with E_STRICT excluded from error_reporting ? thank you, - Markus

Matteo Beccati

11 years ago
On 16/03/2015 13:28, Markus Fischer wrote:
> 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/

Pierre Joye

11 years ago
hi Nikita, On Mon, Mar 16, 2015 at 2:46 AM, Nikita Popov <nikita.ppv@gmail.com> wrote:
> 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

Dennis Birkholz

11 years ago
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

Peter Cowburn

11 years ago
On 15 March 2015 at 15:46, Nikita Popov <nikita.ppv@gmail.com> wrote:
> 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.

Pascal MARTIN

11 years ago
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

Nikita Popov

11 years ago
On Sun, Mar 15, 2015 at 4:46 PM, Nikita Popov <nikita.ppv@gmail.com> wrote:
> 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

Nikita Popov

11 years ago
On Wed, Mar 25, 2015 at 5:14 PM, Nikita Popov <nikita.ppv@gmail.com> wrote:
> On Sun, Mar 15, 2015 at 4:46 PM, Nikita Popov <nikita.ppv@gmail.com> > wrote: > >> 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. >
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

Alex Bowers

11 years ago
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 1 Apr 2015 15:58, "Nikita Popov" <nikita.ppv@gmail.com> wrote:

Stas Malyshev

11 years ago
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

Yasuo Ohgaki

11 years ago
Hi Nikita, On Thu, Apr 2, 2015 at 6:11 AM, Stanislav Malyshev <smalyshev@gmail.com> wrote:
> > * 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. >
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 > > result set" > > 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

Lester Caine

11 years ago
On 01/04/15 22:11, Stanislav Malyshev wrote:
>> * 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.
'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