When usinc `arc diff` in PHP 8.2 this exception can happen:
```
strlen(): Passing null to parameter #1 ($string) of type string is deprecated
```
```
$ arc diff --trace
Running unit tests...
>>> [57] (+50,812) <connect> phabricator_config
<<< [57] (+50,813) <connect> 571 us
>>> [58] (+50,813) <query> SELECT * FROM `phabricator_config`.`config_entry` WHERE namespace = 'default' AND isDeleted = 0
<<< [58] (+50,813) <query> 165 us
[2023-03-24 19:51:00] EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=9e1bb955fac9), phorge(head=arcpatch-D25066_workboard_refactor, ref.master=c6f56b8221ba, ref.arcpatch-D25066_workboard_refactor=6c38649ba830)
#0 PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/infrastructure/env/PhabricatorEnv.php:128]
#1 PhabricatorEnv::initializeCommonEnvironment(boolean) called at [<phorge>/src/infrastructure/env/PhabricatorEnv.php:75]
#2 PhabricatorEnv::initializeScriptEnvironment(boolean) called at [<phorge>/scripts/init/lib.php:26]
#3 init_phabricator_script(array) called at [<phorge>/scripts/init/init-script.php:7]
#4 require_once(string) called at [<phorge>/scripts/__init_script__.php:3]
#5 require_once(string) called at [<phorge>/src/infrastructure/testing/PhabricatorTestCase.php:62]
#6 PhabricatorTestCase::willRunTestCases(array) called at [<arcanist>/src/unit/engine/PhutilUnitTestEngine.php:64]
#7 PhutilUnitTestEngine::run() called at [<arcanist>/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php:148]
#8 ArcanistConfigurationDrivenUnitTestEngine::run() called at [<arcanist>/src/workflow/ArcanistUnitWorkflow.php:170]
#9 ArcanistUnitWorkflow::run() called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:1231]
#10 ArcanistDiffWorkflow::runUnit() called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:1133]
#11 ArcanistDiffWorkflow::runLintUnit() called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:363]
#12 ArcanistDiffWorkflow::run() called at [<arcanist>/scripts/arcanist.php:427]
<<< [1] (+51,023) <exec> 51,023,728 us
```
== What to Avoid (1) ==
Suppressing the warning is not much feasible:
```lang=diff
- if (strlen($test)) {
+ if (@strlen($test)) {
```
If something is deprecated we should avoid using it, not avoid knowing the problem.
== General Fix ==
This is the literal needed fix for some cases:
```lang=diff
- if (strlen($test)) {
+ if ($test !== null && strlen($test)) {
```
So strlen never receives NULL.
But, this probably does not fix the root problem. That is: probably we should avoid to use `strlen()` in some cases.
NOTE: Also Evan had not a strong opinion about this: https://we.phorge.it/book/flavor/article/php_pitfalls/ «A better test is __probably__ strlen()»
For example, we should avoid to use `strlen()` if we are not interested in knowing the exact length of the string in bytes.
For example, we should adopt other methods to check whenever a string is empty. For example this is the literal check that is more efficient and not deprecated:
```lang=diff
- if (strlen($test)) {
+ if (is_string($test) && $test !== '') {
```
The above also fixes the deprecation, and it does in a way that is the literal solution: assure to have a populated string (not null, not an array, etc.).
The above solution is also more efficient because it does not calculate the length of the whole string just to understand if it's a string and if it's empty.
https://www.php.net/manual/en/function.is-string.php
Truth table:
| Value | `if()` | `empty()` | `isset()` | is_string($v) && $v !== '' |
|--------|------------|-----------|-----------|----------|
| `null` | `false` | `true` | `false` | false |
| `0` | `false` | `true` | `true` | false |
| `0.0` | `false` | `true` | `true` | false |
| `"0"` | `false` | `true` | `true` | true |
| `""` | `false` | `true` | `true` | false |
| `false`| `false` | `true` | `true` | false |
| `array()` | `false` | `true` | `true` | false |
| Everything else |`true`|`false` | `true` | if it's a string → true |
== About PhabricatorEnv::getEnvConfig() ==
This deserves a dedicated strategy since it's very frequent.
NOTE: See {T15199}