@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator

PHP Pitfalls: mention strlen() deprecation since PHP 8.1

Summary:
- expand documentation about PHP Pitfalls to mention strlen() in PHP 8.1
- mention phutil_string_cast()
- mention phutil_nonempty_string()
- add a commodity link from PHP Contributors Manual

Ref T15190

Test Plan: * check with your big eyes for typos

Reviewers: O1 Blessed Committers, Cigaryno, avivey

Reviewed By: O1 Blessed Committers, Cigaryno, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Tags: #documentation

Maniphest Tasks: T15190

Differential Revision: https://we.phorge.it/D25108

+73 -22
+4
src/docs/contributor/php_coding_standards.diviner
··· 176 176 return $this->favoriteFood; 177 177 } 178 178 } 179 + 180 + # Extra Readings 181 + 182 + * @{article:PHP Pitfalls}
+69 -22
src/docs/flavor/php_pitfalls.diviner
··· 49 49 false // boolean 50 50 array() // empty array 51 51 52 - Disregarding some bizarre edge cases, all other values are truthy. Note that 53 - because "0" is falsey, this sort of thing (intended to prevent users from making 54 - empty comments) is wrong in PHP: 55 - 56 - COUNTEREXAMPLE 57 - if ($comment_text) { 58 - make_comment($comment_text); 59 - } 60 - 61 - This is wrong because it prevents users from making the comment "0". //THIS 62 - COMMENT IS TOTALLY AWESOME AND I MAKE IT ALL THE TIME SO YOU HAD BETTER NOT 63 - BREAK IT!!!// A better test is probably `strlen()`. 52 + Disregarding some bizarre edge cases, all other values are truthy. 64 53 65 54 In addition to truth tests with `if`, PHP has two special truthiness operators 66 55 which look like functions but aren't: `empty()` and `isset()`. These operators ··· 92 81 `empty()` evaluates truthiness exactly opposite of `if()`. `isset()` returns 93 82 `true` for everything except `null`. This is the truth table: 94 83 95 - | Value | `if()` | `empty()` | `isset()` | 96 - |-------|--------|-----------|-----------| 97 - | `null` | `false` | `true` | `false` | 98 - | `0` | `false` | `true` | `true` | 99 - | `0.0` | `false` | `true` | `true` | 100 - | `"0"` | `false` | `true` | `true` | 101 - | `""` | `false` | `true` | `true` | 102 - | `false` | `false` | `true` | `true` | 103 - | `array()` | `false` | `true` | `true` | 104 - | Everything else | `true` | `false` | `true` | 84 + | Value | `if()` | `empty()` | `isset()` | 85 + |---------------|--------|-----------|-----------| 86 + | `null` | `false`| `true` | `false` | 87 + | `0` | `false`| `true` | `true` | 88 + | `0.0` | `false`| `true` | `true` | 89 + | `"0"` | `false`| `true` | `true` | 90 + | `""` | `false`| `true` | `true` | 91 + |`false` | `false`| `true` | `true` | 92 + |`array()` | `false`| `true` | `true` | 93 + |Everything else| `true` | `false` | `true` | 105 94 106 95 The value of these operators is that they accept undeclared variables and do 107 96 not issue a warning. Specifically, if you try to do this you get a warning: ··· 137 126 Put another way, use `isset()` if you want to type `if ($value !== null)` but 138 127 are testing something that may not be declared. Use `empty()` if you want to 139 128 type `if (!$value)` but you are testing something that may not be declared. 129 + 130 + = Check for non-empty strings = 131 + 132 + As already mentioned, note that you cannot just use an `if` or `empty()` to 133 + check for a non-empty string, mostly because "0" is falsey, so you cannot rely 134 + on this sort of thing to prevent users from making empty comments: 135 + 136 + COUNTEREXAMPLE 137 + if ($comment_text) { 138 + make_comment($comment_text); 139 + } 140 + 141 + This is wrong because it prevents users from making the comment "0". 142 + 143 + //THE COMMENT "0" IS TOTALLY AWESOME AND I MAKE IT ALL THE TIME SO YOU HAD 144 + BETTER NOT BREAK IT!!!// 145 + 146 + Another way //was// also `strlen()`: 147 + 148 + COUNTEREXAMPLE 149 + if (strlen($comment_text)) { 150 + make_comment($comment_text); 151 + } 152 + 153 + But using `strlen(null)` causes a deprecation warning since PHP 8.1. Also, 154 + using `strlen()` uses too many CPU cycles to just check of a non-empty. 155 + 156 + In short, outside Phorge, this is a general way to check for non-empty strings 157 + for most wild input types: 158 + 159 + ```lang=php 160 + $value_str = (string) $value; 161 + if ($value_str !== '') { 162 + // do something 163 + } 164 + ``` 165 + 166 + To do the same thing in Phorge, use this better and safer approach: 167 + 168 + ```lang=php 169 + $value_str = phutil_string_cast($value); 170 + if ($value_str !== '') { 171 + // do something 172 + } 173 + ``` 174 + 175 + And, if you are 100% sure that you are __only__ working with string and 176 + null, evaluate this instead: 177 + 178 + ```lang=php 179 + if (phutil_nonempty_string($value)) { 180 + // do something 181 + } 182 + ``` 183 + 184 + WARNING: The function `phutil_nonempty_string()` is designed to throw a nice 185 + exception if it receives `true`, `false`, an array, an object or anything 186 + alien that is not a string and not null. Do your evaluations. 140 187 141 188 = usort(), uksort(), and uasort() are Slow = 142 189