# Missing warning for stats(), etc. when invoked with empty string - Thread: https://externals.io/message/112396 - Original archive: https://news-web.php.net/php.internals/112396 - Started by Claude Pache on 2020-12-03T10:20:07+00:00 - Messages: 8 - 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 --- ## #112396 — Claude Pache — 2020-12-03T10:20:07+00:00 URL: https://externals.io/message/112396#112396 Hi, `stats()` and friends invoked with empty string or null as argument, return `false` (indicating failure), but do not emit the Warning that is expected on failure. See: https://3v4l.org/jXC2N That discrepancy between the two ways of indicating failure is problematic. Indeed, whenever I use one of those functions without prefixing it by @, I expect that it will either return a value of expected type (`array` in case of `stats()`), or run the panic procedure of my error handler. I propose to ensure that those functions always emit a Warning in each and every situation they return false/null (provided, of course, that those functions are not supposed to return `false/null` in absence of failure, and that their purpose is not to check failure condition as in `file_exists()`). —Claude --- ## #112397 — Claude Pache — 2020-12-03T13:30:15+00:00 URL: https://externals.io/message/112396#112397 · In reply to: #112396 [Of course, I meant `stat()`, not `stats()`. Resending the message with correct spelling, including in subject line. Sorry.] Hi, `stat()` and friends invoked with empty string or null as argument, return `false` (indicating failure), but do not emit the Warning that is expected on failure. See: https://3v4l.org/jXC2N That discrepancy between the two ways of indicating failure is problematic. Indeed, whenever I use one of those functions without prefixing it by @, I expect that it will either return a value of expected type (`array` in case of `stat()`), or run the panic procedure of my error handler. I propose to ensure that those functions always emit a Warning in each and every situation they return false/null (provided, of course, that those functions are not supposed to return `false/null` in absence of failure, and that their purpose is not to check for failure condition as in `file_exists()`). —Claude --- ## #112400 — Levi Morrison via internals — 2020-12-03T21:05:50+00:00 URL: https://externals.io/message/112396#112400 · In reply to: #112397 > > [Of course, I meant `stat()`, not `stats()`. Resending the message with correct spelling, including in subject line. Sorry.] > > > Hi, > > `stat()` and friends invoked with empty string or null as argument, return `false` (indicating failure), but do not emit the Warning that is expected on failure. See: https://3v4l.org/jXC2N > > That discrepancy between the two ways of indicating failure is problematic. Indeed, whenever I use one of those functions without prefixing it by @, I expect that it will either return a value of expected type (`array` in case of `stat()`), or run the panic procedure of my error handler. > > I propose to ensure that those functions always emit a Warning in each and every situation they return false/null (provided, of course, that those functions are not supposed to return `false/null` in absence of failure, and that their purpose is not to check for failure condition as in `file_exists()`). > > —Claude > This is a long-standing behavior. Further, I don't know many people who want _more_ warnings in their codebase. In other words, I disagree with this proposal. --- ## #112401 — Nikita Popov — 2020-12-03T21:13:08+00:00 URL: https://externals.io/message/112396#112401 · In reply to: #112400 On Thu, Dec 3, 2020 at 10:06 PM Levi Morrison via internals < internals@lists.php.net> wrote: > On Thu, Dec 3, 2020 at 6:30 AM Claude Pache > wrote: > > […] > correct spelling, including in subject line. Sorry.] > > […] > return `false` (indicating failure), but do not emit the Warning that is > expected on failure. See: https://3v4l.org/jXC2N > > […] > problematic. Indeed, whenever I use one of those functions without > prefixing it by @, I expect that it will either return a value of expected > type (`array` in case of `stat()`), or run the panic procedure of my error > handler. > > […] > and every situation they return false/null (provided, of course, that those > functions are not supposed to return `false/null` in absence of failure, > and that their purpose is not to check for failure condition as in > `file_exists()`). > > […] > > This is a long-standing behavior. Further, I don't know many people > who want _more_ warnings in their codebase. > > In other words, I disagree with this proposal. This seems like a pretty odd position to take. I think there's two ways it can reasonably behave: 1. Always generate a warning if `stat()` fails. 2. Never generate a warning if `stat()` fails. While the current option is: 3. Always generate a warning if `stat()` fails ... unless an empty string was passed. I'm okay with either 1. or 2., but I don't think that 3. constitutes sensible behavior. Regards, Nikita --- ## #112403 — Christian Schneider — 2020-12-04T07:12:31+00:00 URL: https://externals.io/message/112396#112403 · In reply to: #112401 Am 03.12.2020 um 22:13 schrieb Nikita Popov : > On Thu, Dec 3, 2020 at 10:06 PM Levi Morrison via internals < > internals@lists.php.net > wrote: > > […] > > > This seems like a pretty odd position to take. I think there's two ways it > can reasonably behave: > > 1. Always generate a warning if `stat()` fails. > 2. Never generate a warning if `stat()` fails. > > While the current option is: > > 3. Always generate a warning if `stat()` fails ... unless an empty string was > passed. > > I'm okay with either 1. or 2., but I don't think that 3. constitutes > sensible behavior. Wel,, there is actually a third case: null (which is also the reason for the empty string behavior) Some people might use it like "I have a filename but there is no such file, so `stat()` should issue a warning because something unexpected happened" and "I have no filename so the warning is not necessary". So 3 might not be completely useless. That being said: I'm not against changing 3 to 1, I just wanted to point out the null-case. - Chris --- ## #112404 — Claude Pache — 2020-12-04T08:32:35+00:00 URL: https://externals.io/message/112396#112404 · In reply to: #112400 > Le 3 déc. 2020 à 22:05, Levi Morrison a écrit : > > This is a long-standing behavior. Further, I don't know many people > who want _more_ warnings in their codebase. My guess is that this position was already discussed in the occasion of the several past RFC whose main purpose was to introduce more warnings/errors/exceptions? [1], [2], [3], etc. Apparently, many people do want more warnings... But my main point is not exactly that. It is inconsistency in behaviour, that leads to wrong assumptions, that leads to incorrect code. As another example, see the faulty `@is_file( $data ) === false` check mentioned in the other thread [4]. Precisely (and I realise that maybe I wasn’t explicit enough in my message), the issue is that `stat("non/existent/file")` raises a warning and returns false, that leads to the wrong assumption that `stat($random_string)` raises a warning whenever it returns `false` instead of an array, that leads to the deceptive confidence that the `false` case will be always handled by the error handler. —Claude [1]: https://wiki.php.net/rfc/counting_non_countables [2]: https://wiki.php.net/rfc/notice-for-non-valid-array-container [3]: https://wiki.php.net/rfc/magic-methods-signature [4]: https://externals.io/message/112333#112350 > > > […] > écrit : > > […] > > > My guess is that this position was already discussed in the occasion of > the several past RFC whose main purpose was to introduce more > warnings/errors/exceptions? [1], [2], [3], etc. Apparently, many people do > want more warnings... > This is getting a bit off-topic, but I think it's necessary to clarify: What many of us actually want is *less* warnings. Having less warnings can take the form of converting them to exceptions, or take the form of removing them entirely. Both of these things happen, but the conversions to exceptions tend to be the more controversial and thus take the spotlight. I'm personally not a fan of warnings thrown by stream/FS functions (especially when backed by socket streams), as robust code just ends up having to suppress them all. Unfortunately I can't say that these warnings are entirely useless, as they *are* helpful for debugging. It would be nice if PHP had a stronger separation between "this is an error" warnings and "this is informational" warnings. Things are slowly moving in this direction with the first category being partially converted into exceptions... But well, that's a global problem. The current stance of stream/FS functions is very much that all error conditions should warn. Regards, Nikita --- ## #112409 — Christoph M. Becker — 2020-12-04T11:52:11+00:00 URL: https://externals.io/message/112396#112409 · In reply to: #112396 > `stats()` and friends invoked with empty string or null as argument, return `false` (indicating failure), but do not emit the Warning that is expected on failure. See: https://3v4l.org/jXC2N > > That discrepancy between the two ways of indicating failure is problematic. Indeed, whenever I use one of those functions without prefixing it by @, I expect that it will either return a value of expected type (`array` in case of `stats()`), or run the panic procedure of my error handler. > > I propose to ensure that those functions always emit a Warning in each and every situation they return false/null (provided, of course, that those functions are not supposed to return `false/null` in absence of failure, and that their purpose is not to check failure condition as in `file_exists()`). Returning false without raising a warning has been introduced[1] to fix bug #21410[2]. The problem back then was that without that early return, on Windows the empty string gave erroneous `stat()` results, and filetype("") returned "dir". This is no longer the case at least as of PHP 7.3.0 (I didn't test older versions); instead after reverting 560e33968d I now get: Warning: `stat()`: stat failed for in %s on line %d bool(false) So, yes, I think we should revert that commit. [1] [2] Regards, Christoph