From b996b4799b024de19796404d085650abd24a8797 Mon Sep 17 00:00:00 2001
From: Valerio Bozzolan <nsa+gitspam@succhia.cz>
Date: Tue, 18 Jul 2023 12:39:36 +0200
Subject: [PATCH] phutil_nonempty_scalar(): don't throw when receiving a
 boolean scalar

Summary:
Note that booleans are scalars. Full stop.

| is_scalar($v)    | Result |
|------------------|--------|
| `"foo"`          | true   |
| `""`             | true   |
| `null`           | true   |
| `0`              | true   |
| `0.5`            | true   |
| `true`           | true   |
| `false`          | true   |
| `new stdclass()` | false  |
| `array()`        | false  |

Note that phutil_nonempty_scalar() was designed just to tell
whenever a scalar is "empty" or not. So it must not explode
when receiving a valid scalar.

So the question is not whenever a boolean is a scalar or not,
but whenever is empty or not. But also this is a clear fact:

| `$v`    | `is_scalar($v)` | `!is_empty($v)` | `if(strlen($v))`|
|---------|-----------------|-----------------|-----------------|
| `true`  | `true`          | `true`          | `true`          |
| `false` | `true`          | `false`         | `false`         |

In short, now the function does not explode anymore with bool
values. Instead, it says whenever are empty or not.

In bold the exact changes:

| Value             |`phutil_nonempty_scalar($v)`|
|-------------------|----------------------------|
| `"foo"`           | true                       |
| `""`              | false                      |
| `null`            | false                      |
| `0`               | true                       |
| `0.5`             | true                       |
|`obj` with tostring| true                       |
|`obj` withno tostr.| Exception                  |
| `array()`         | Exception                  |
| `true`            | ~~Exception~~ **true**     |
| `false`           | ~~Exception~~ **false**    |

Closes T15239

Test Plan:
- check if it makes sense to you
- check the few usages

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15239

Differential Revision: https://we.phorge.it/D25117
---
 src/utils/__tests__/PhutilUtilsTestCase.php |  9 ++++++++-
 src/utils/utils.php                         | 11 +++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/src/utils/__tests__/PhutilUtilsTestCase.php b/src/utils/__tests__/PhutilUtilsTestCase.php
index d7213685..1e088f5c 100644
--- a/src/utils/__tests__/PhutilUtilsTestCase.php
+++ b/src/utils/__tests__/PhutilUtilsTestCase.php
@@ -1004,11 +1004,18 @@ final class PhutilUtilsTestCase extends PhutilTestCase {
 
     $uri = new PhutilURI('http://example.org/');
 
+    // Each test is defined in this way:
+    //   0: subject $value
+    //   1: expected result from phutil_nonempty_string($value)
+    //   2: expected result from phutil_nonempty_stringlike($value)
+    //   3: expected result from phutil_nonempty_scalar($value)
+    //   4: human test name
     $map = array(
       array(null, false, false, false, 'literal null'),
       array('', false, false, false, 'empty string'),
       array('x', true, true, true, 'nonempty string'),
-      array(false, null, null, null, 'bool'),
+      array(false, null, null, false, 'false bool'),
+      array(true, null, null, true, 'true bool'),
       array(1, null, null, true, 'integer'),
       array($uri, null, true, true, 'uri object'),
       array(2.5, null, null, true, 'float'),
diff --git a/src/utils/utils.php b/src/utils/utils.php
index 0d6dc2a1..0b86cd59 100644
--- a/src/utils/utils.php
+++ b/src/utils/utils.php
@@ -2187,6 +2187,9 @@ function phutil_nonempty_stringlike($value) {
  * string other than the empty string, integers, and floats are considered
  * scalar.
  *
+ * Note that booleans are also valid scalars, where false is considered empty,
+ * and true is non-empty since if you cast true to string, it's non-empty.
+ *
  * This method raises an exception if passed any other value.
  *
  * @param Value to test.
@@ -2205,6 +2208,14 @@ function phutil_nonempty_scalar($value) {
     return true;
   }
 
+  // Booleans are also valid scalars by PHP. Inventing the opposite can be
+  // too much esoteric and problematic.
+  //   false: empty,     because casted to string becomes ''  (empty)
+  //   true:  non-empty, because casted to string becomes '1' (non-empty)
+  if ($value === false || $value === true) {
+    return $value;
+  }
+
   if (is_object($value)) {
     try {
       $string = phutil_string_cast($value);