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 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
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
Is it possible relax the error for tmp variables?
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
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]
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
I would like to propose reclassifying our few existing
E_STRICT
notices andremoving 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.
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
On Tue, Feb 24, 2015 at 8:20 AM, Stanislav Malyshev smalyshev@gmail.com
wrote:
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.
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
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
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.htmlIn 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
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
On Mon, Mar 2, 2015 at 7:37 PM, Stanislav Malyshev smalyshev@gmail.com
wrote:
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.
Doing $obj::$property is possible as well, it does the same as
get_class($obj)::$property.
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.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.htmlIn 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.
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
Hi Nikita,
Le dim. 22 févr. 2015 à 23:31, Nikita Popov nikita.ppv@gmail.com 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
Le ven. 13 mars 2015 à 10:33, Patrick ALLAERT patrickallaert@php.net a
écrit :
Hi Nikita,
Le dim. 22 févr. 2015 à 23:31, Nikita Popov nikita.ppv@gmail.com 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,
NikitaI 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 ofE_ALL
and it encourages people to give attention toE_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 havingE_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
(withoutE_STRICT
then).For more errors verbosity (new application development):
One can activateE_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
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
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.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