PHP 8.2: fixes for strlen() not accepting NULL anymore, part 1

Summary:
This change avoids some unnecessary uses of the strlen() function,
actually fixing some deprecation warnings in PHP 8.2.

In short, this is the suggested universal replace:

    -if(strlen($v))
    +if(phutil_nonempty_string($v))

And, if you know PHP, this is also another adoptable replace, but
only for cases where you are sure that the string "0" is not useful:

    -if(strlen($v))
    +if($v))

As usual the optimal solution depends on the contest.

Other similar patches will probably follow.

Closes T15222
Ref T15190

Test Plan:
- for the first time in my life, with this change, the unit tests are passed in PHP 8.2
- check with your big eyes that there are no obvious typos

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15199, T15190, T15222

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

src/applications/config/check/PhabricatorStorageSetupCheck.php

@@ -151,19 +151,19 @@ final class PhabricatorStorageSetupCheck extends PhabricatorSetupCheck {
 
     $how_many = 0;
 
-    if (strlen($access_key)) {
+    if (phutil_nonempty_string($access_key)) {
       $how_many++;
     }
 
-    if (strlen($secret_key)) {
+    if (phutil_nonempty_string($secret_key)) {
       $how_many++;
     }
 
-    if (strlen($region)) {
+    if (phutil_nonempty_string($region)) {
       $how_many++;
     }
 
-    if (strlen($endpoint)) {
+    if (phutil_nonempty_string($endpoint)) {
       $how_many++;
     }
 

src/applications/files/engine/PhabricatorS3FileStorageEngine.php

@@ -31,11 +31,11 @@ final class PhabricatorS3FileStorageEngine
     $endpoint = PhabricatorEnv::getEnvConfig('amazon-s3.endpoint');
     $region = PhabricatorEnv::getEnvConfig('amazon-s3.region');
 
-    return (strlen($bucket) &&
+    return phutil_nonempty_string($bucket) &&
-      strlen($access_key) &&
+      phutil_nonempty_string($access_key) &&
-      strlen($secret_key) &&
+      phutil_nonempty_string($secret_key) &&
-      strlen($endpoint) &&
+      phutil_nonempty_string($endpoint) &&
-      strlen($region));
+      phutil_nonempty_string($region);
   }
 
 
@@ -57,7 +57,7 @@ final class PhabricatorS3FileStorageEngine
     $parts[] = 'phabricator';
 
     $instance_name = PhabricatorEnv::getEnvConfig('cluster.instance');
-    if (strlen($instance_name)) {
+    if (phutil_nonempty_string($instance_name)) {
       $parts[] = $instance_name;
     }
 

src/applications/files/storage/PhabricatorFile.php

@@ -856,7 +856,7 @@ final class PhabricatorFile extends PhabricatorFileDAO
     // instance identity in the path allows us to distinguish between requests
     // originating from different instances but served through the same CDN.
     $instance = PhabricatorEnv::getEnvConfig('cluster.instance');
-    if (strlen($instance)) {
+    if (phutil_nonempty_string($instance)) {
       $parts[] = '@'.$instance;
     }
 
@@ -903,7 +903,7 @@ final class PhabricatorFile extends PhabricatorFileDAO
     $parts[] = 'xform';
 
     $instance = PhabricatorEnv::getEnvConfig('cluster.instance');
-    if (strlen($instance)) {
+    if (phutil_nonempty_string($instance)) {
       $parts[] = '@'.$instance;
     }
 

src/applications/metamta/engine/PhabricatorMailEmailEngine.php

@@ -507,7 +507,7 @@ final class PhabricatorMailEmailEngine
   public function newDefaultEmailAddress() {
     $raw_address = PhabricatorEnv::getEnvConfig('metamta.default-address');
 
-    if (!strlen($raw_address)) {
+    if (!$raw_address) {
       $domain = $this->newMailDomain();
       $raw_address = "noreply@{$domain}";
     }
@@ -527,7 +527,7 @@ final class PhabricatorMailEmailEngine
 
   private function newMailDomain() {
     $domain = PhabricatorEnv::getEnvConfig('metamta.reply-handler-domain');
-    if (strlen($domain)) {
+    if ($domain) {
       return $domain;
     }
 

src/applications/notification/client/PhabricatorNotificationServerRef.php

@@ -152,7 +152,7 @@ final class PhabricatorNotificationServerRef
       ->setPath($full_path);
 
     $instance = PhabricatorEnv::getEnvConfig('cluster.instance');
-    if (strlen($instance)) {
+    if (phutil_nonempty_string($instance)) {
       $uri->replaceQueryParam('instance', $instance);
     }
 

src/applications/repository/engine/PhabricatorRepositoryPullEngine.php

@@ -238,7 +238,7 @@ final class PhabricatorRepositoryPullEngine
     $identifier = $repository->getPHID();
 
     $instance = PhabricatorEnv::getEnvConfig('cluster.instance');
-    if (strlen($instance)) {
+    if (phutil_nonempty_string($instance)) {
       $identifier = "{$identifier}:{$instance}";
     }
 

src/applications/repository/storage/PhabricatorRepository.php

@@ -2480,7 +2480,8 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
       $has_https = false;
     }
 
-    $has_ssh = (bool)strlen(PhabricatorEnv::getEnvConfig('phd.user'));
+    $phd_user = PhabricatorEnv::getEnvConfig('phd.user');
+    $has_ssh = phutil_nonempty_string($phd_user);
 
     $protocol_map = array(
       PhabricatorRepositoryURI::BUILTIN_PROTOCOL_SSH => $has_ssh,

src/infrastructure/cluster/PhabricatorDatabaseRef.php

@@ -229,7 +229,7 @@ final class PhabricatorDatabaseRef
     $host = $this->getHost();
 
     $port = $this->getPort();
-    if (strlen($port)) {
+    if ($port) {
       return "{$host}:{$port}";
     }
 

src/infrastructure/env/PhabricatorEnv.php

@@ -125,7 +125,7 @@ final class PhabricatorEnv extends Phobject {
     // If an instance identifier is defined, write it into the environment so
     // it's available to subprocesses.
     $instance = self::getEnvConfig('cluster.instance');
-    if (strlen($instance)) {
+    if (phutil_nonempty_string($instance)) {
       putenv('PHABRICATOR_INSTANCE='.$instance);
       $_ENV['PHABRICATOR_INSTANCE'] = $instance;
     }
@@ -432,7 +432,7 @@ final class PhabricatorEnv extends Phobject {
     $uri = new PhutilURI($raw_uri);
 
     $host = $uri->getDomain();
-    if (!strlen($host)) {
+    if (!phutil_nonempty_string($host)) {
       return false;
     }
 
@@ -455,7 +455,7 @@ final class PhabricatorEnv extends Phobject {
     $self_map = array();
     foreach ($self_uris as $self_uri) {
       $host = id(new PhutilURI($self_uri))->getDomain();
-      if (!strlen($host)) {
+      if (!phutil_nonempty_string($host)) {
         continue;
       }
 
@@ -661,7 +661,7 @@ final class PhabricatorEnv extends Phobject {
   public static function isValidLocalURIForLink($uri) {
     $uri = (string)$uri;
 
-    if (!strlen($uri)) {
+    if (!phutil_nonempty_string($uri)) {
       return false;
     }
 
@@ -726,7 +726,7 @@ final class PhabricatorEnv extends Phobject {
     $uri = new PhutilURI($raw_uri);
 
     $proto = $uri->getProtocol();
-    if (!strlen($proto)) {
+    if (!$proto) {
       throw new Exception(
         pht(
           'URI "%s" is not a valid linkable resource. A valid linkable '.
@@ -745,7 +745,7 @@ final class PhabricatorEnv extends Phobject {
     }
 
     $domain = $uri->getDomain();
-    if (!strlen($domain)) {
+    if (!$domain) {
       throw new Exception(
         pht(
           'URI "%s" is not a valid linkable resource. A valid linkable '.
@@ -793,7 +793,7 @@ final class PhabricatorEnv extends Phobject {
     $uri = new PhutilURI($raw_uri);
 
     $proto = $uri->getProtocol();
-    if (!strlen($proto)) {
+    if (!$proto) {
       throw new Exception(
         pht(
           'URI "%s" is not a valid fetchable resource. A valid fetchable '.
@@ -812,7 +812,7 @@ final class PhabricatorEnv extends Phobject {
     }
 
     $domain = $uri->getDomain();
-    if (!strlen($domain)) {
+    if (!$domain) {
       throw new Exception(
         pht(
           'URI "%s" is not a valid fetchable resource. A valid fetchable '.

src/infrastructure/log/PhabricatorSSHLog.php

@@ -24,7 +24,7 @@ final class PhabricatorSSHLog extends Phobject {
       );
 
       $sudo_user = PhabricatorEnv::getEnvConfig('phd.user');
-      if (strlen($sudo_user)) {
+      if (phutil_nonempty_string($sudo_user)) {
         $data['S'] = $sudo_user;
       }