# [RFC] E_NOTICE for invalid array container - Thread: https://externals.io/message/94768 - Original archive: https://news-web.php.net/php.internals/94768 - Started by David Walker on 2016-07-31T19:31:16+00:00 - Messages: 7 - 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 --- ## #94768 — David Walker — 2016-07-31T19:31:16+00:00 URL: https://externals.io/message/94768#94768 Hi All, I'm attempting to get involved and hack away at some internals; I figured the best method to do this would be to pick up a bug and do my best at it. So I grabbed #37676. The issue asks to implement an E_(NOTICE|WARNING) when attempting to access scalar values as arrays. cmb alerted me to a long thread that came up last year ( http://marc.info/?t=143379796900001&r=1&w=2). I've read over it and the couple of other PR's that attempted to resolve this issue. As I read, the couple of major complaints about adding an `E_NOTICE` were that multiple-accesses in a set (i.e. $a=false;$a[1][2][3];) would produce too many errors. Matt raised this issue, in addition to it only being raised for non-null issues; having `NULL` be an 'identity' of sort that any array access on a `NULL` is NULL. I agree with him in part, and oppose in others. I think that original access to a `NULL` should be treated as accessing any scalar value, however I do agree that multiple notices would be bad. Rowan has also presented his thoughts to the same, and this implementation takes into account his solution where we don't raise much. Christoph countered that: ```$o->foo->bar``` would itself raise two notices rather than one. I would present that that behavior should be pruned itself. Object access on a null should alert, but subsequent calls ought not to (I don't know how to prevent that, but then again I haven't learned). So I want to solicit for more discussion given previous talks. My first-attempt RFC for this is at: https://wiki.php.net/rfc/notice-for-non-valid-array-container -- Dave ps: this is my first attempt here at an RFC solo, and preemptively apologize if anything deviates from the norm too far, did my best to follow all them how-to's. --- ## #94769 — Niklas Keller — 2016-07-31T20:04:11+00:00 URL: https://externals.io/message/94768#94769 · In reply to: #94768 2016-07-31 21:31 GMT+02:00 David Walker : > Hi All, > > I'm attempting to get involved and hack away at some internals; I figured > the best method to do this would be to pick up a bug and do my best at it. > So I grabbed #37676. The issue asks to implement an E_(NOTICE|WARNING) > when attempting to access scalar values as arrays. cmb alerted me to a > long thread that came up last year ( > http://marc.info/?t=143379796900001&r=1&w=2). I've read over it and the > couple of other PR's that attempted to resolve this issue. > > As I read, the couple of major complaints about adding an `E_NOTICE` were > that multiple-accesses in a set (i.e. $a=false;$a[1][2][3];) would produce > too many errors. Matt > raised this > issue, in addition to it only being raised for non-null issues; having `NULL` > be an 'identity' of sort that any array access on a `NULL` is NULL. I agree > with him in part, and oppose in others. I think that original access to a > `NULL` should be treated as accessing any scalar value, however I do agree > that multiple notices would be bad. Rowan > has also > presented his thoughts to the same, and this implementation takes into > account his solution where we don't raise much. > > Christoph > countered > that: ```$o->foo->bar``` would itself raise two notices rather than one. I > would present that that behavior should be pruned itself. Object access on > a null should alert, but subsequent calls ought not to (I don't know how to > prevent that, but then again I haven't learned). > > > So I want to solicit for more discussion given previous talks. My > first-attempt RFC for this is at: > https://wiki.php.net/rfc/notice-for-non-valid-array-container > > -- > Dave > > ps: this is my first attempt here at an RFC solo, and preemptively > apologize if anything deviates from the norm too far, did my best to follow > all them how-to's. > Big +1. We came across that some days ago again. Regards, Niklas --- ## #94772 — Dan Ackroyd — 2016-08-01T07:14:15+00:00 URL: https://externals.io/message/94768#94772 · In reply to: #94768 Hi David, > So I want to solicit for more discussion given previous talks. My > first-attempt RFC for this is at: > https://wiki.php.net/rfc/notice-for-non-valid-array-container > I've very likely to support this RFC, however the section on 'Backward Incompatible Changes' needs to be filled out, as this RFC would make code that currently emits no notices, start emitting notices. cheers Dan --- ## #94774 — David Walker — 2016-08-01T09:03:27+00:00 URL: https://externals.io/message/94768#94774 · In reply to: #94772 Hi Dan, I left it blank to go along with default production ini-settings where notices would be excluded. However, you are probably correct that it should be noted for development environments as something new. I'll toss it in there in the morning Thanks -- Dave > Hi David, > > > > […] > > I've very likely to support this RFC, however the section on 'Backward > Incompatible Changes' needs to be filled out, as this RFC would make > code that currently emits no notices, start emitting notices. > > cheers > Dan --- ## #94784 — Dan Ackroyd — 2016-08-01T22:27:07+00:00 URL: https://externals.io/message/94768#94784 · In reply to: #94774 Hi David, The updated section still doesn't really describe the BC break that well. Some people will need to change their code to avoid having notices raised frequently. How difficult that will be needs to be thought about. Additionally, although the default ini files may turn off notices in production environment, I really don't believe that is best practice. For my machines, in both production and dev all warnings and notices (except deprecation notices in production) are converted into exceptions. Anything else allows mistakes in code to go undetected. cheers Dan --- ## #94805 — David Walker — 2016-08-02T18:33:04+00:00 URL: https://externals.io/message/94768#94805 · In reply to: #94784 Dan, You're right on both accounts; also, as I just notices it could easily have other BC-breaks (although expecting errors in a certain format is odd) through calls like `error_get_last()`, which would regardless of INI settings include this NOTICE. I will update the RFC with these changes. -- Dave > Hi David, > > The updated section still doesn't really describe the BC break that well. > > Some people will need to change their code to avoid having notices > raised frequently. How difficult that will be needs to be thought > about. > > Additionally, although the default ini files may turn off notices in > production environment, I really don't believe that is best practice. > For my machines, in both production and dev all warnings and notices > (except deprecation notices in production) are converted into > exceptions. Anything else allows mistakes in code to go undetected. > > cheers > Dan --- ## #94935 — David Walker — 2016-08-08T15:49:57+00:00 URL: https://externals.io/message/94768#94935 · In reply to: #94805 All, I updated the RFC, and made a couple updates to tests where this notice is raising itself (fpm & mysqli). I ended up catching a couple of oddities in how mine (and previous proposed) changes would have impacted a foreach(list() = each()), so it won't raise a warning on the last, null, element of each. Before I ask for voting, I do have a best-practices question I hope someone would have an answer to. My dev setup doesn't include all the extensions, nor do I have a copy of windows for testing. What would be the best way to ensure all tests are run so I can find other locations where this warning might be adversely impacting some of the tests? I know Travis will run tests when I create a PR, but I don't know if it is ensuring all tests are fully run either. Thanks -- Dave > Dan, > > You're right on both accounts; also, as I just notices it could easily > have other BC-breaks (although expecting errors in a certain format is odd) > through calls like `error_get_last()`, which would regardless of INI settings > include this NOTICE. I will update the RFC with these changes. > > -- > Dave > > > > > […]