Preamble
The function phutil_nonempty_scalar() was recently (Apr 20 2022) introduced in Phabricator to help in supporting PHP 8 support and do stricter type checks.
Note that it was introduced since this commit:
https://secure.phabricator.com/rARCf098e8d86373c0651616390a9db6bd215c9bd228
rARCf098e8d86373: Introduce PHP8.1 replacement functions for string tests which may take multiple…
The function is designed to check whenever a scalar can be considered empty or not.
What is a scalar? Uh?
A scalar is a quantity described by a single value. That is.
So, variables containing a single value (or that can be described as a single value) are scalars.
Practical examples:
| is_scalar($v) | Result |
|---|---|
| "foo" | true |
| "" | true |
| null | true |
| 0 | true |
| 0.5 | true |
| true | true |
| false | true |
| new stdclass() | false |
| array() | false |
https://www.php.net/manual/en/function.is-scalar.php
Description of the problem
The results marked in bold are problematic:
| Value | is_scalar() | phutil_nonempty_scalar() |
|---|---|---|
| "foo" | true | true |
| "" | true | false |
| null | true | false |
| 0 | true | true |
| 0.5 | true | true |
| true | true | Exception |
| false | true | Exception |
| array() | false | Exception |
| new stdclass() | false | Exception |
| obj with tostring | false | true |
I'm not very opinionated about what should happen instead, but I'm 100% sure that an exception is totally wrong in this case, since a boolean is a valid scalar. Full stop.
PHP is already full of absurd things, such as the value zero being considered empty by is_empty(). If we then we throw an exception because a method that is supposed to tell you whether a scalar is empty or not, it throws instead an exception because it receives a valid scalar, that is not acceptable and makes this function virtually not usable.
For booleans the function must not return an exception, but a clean return value.
Proposed solution
The function phutil_nonempty_scalar() is supposed to only check whenever a scalar is empty or not, and it MUST NOT throw an exception if it receives a valid scalar.
Also, it should not declare true as empty, since true is a valid scalar widely considered as non-empty. In fact:
| $v | is_scalar($v) | !is_empty($v) | if(strlen($v)) | (string)$v !== '' | (string) $v |
|---|---|---|---|---|---|
| true | true | true | true | true | '1' |
| false | true | false | false | false | '' |
In short, having said that booleans are scalars:
- true is non-empty
- false is empty
So in bold the proposed natural change:
| Value | ...nonempty_scalar() |
|---|---|
| "foo" | true |
| "" | false |
| null | false |
| 0 | true |
| 0.5 | true |
| true | |
| false | |
| array() | Exception |
| new stdclass() | Exception |
| object with tostring | true |