== 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
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 |
| `object 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))`|
|---------|-----------------|-----------------|-----------------|
| `true` | `true` | `true` | `true` |
| `false` | `true` | `false` | `false` |
So in bold the proposed change:
| Value | `phutil_nonempty_scalar()` |
|------------------|-----------------------------|
| `"foo"` | true |
| `""` | false |
| `null` | false |
| `0` | true |
| `0.5` | true |
| `true` | ~~Exception~~ **true** |
| `false` | ~~Exception~~ **false** |
| `array()` | Exception |
| `new stdclass()` | Exception |
| `object with tostring` | true |