Addressing PHP8 incompatibilities - Miscellaneous

neandrake · neandrake · commit 40b272fa51ea · 2023-05-30T12:06:16.000-04:00
Summary: More perusing through Phabricator to find and address incompatibilities. Refs T13588 Test Plan: - Creating macros, changing their text. - Upload to use existing SSH key instead of generate one. - Modify alt text of an uploaded file. - Create phame post - Delete phriction document - Move tasks around on workboard, assign story points - Generate Diviner docs - Bulk modify tasks Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T13588 Differential Revision: https://secure.phabricator.com/D21869
diff --git a/scripts/repository/commit_hook.php b/scripts/repository/commit_hook.php
@@ -119,7 +119,7 @@
   exit($err);
 } else if ($repository->isGit() || $repository->isHg()) {
   $username = getenv(DiffusionCommitHookEngine::ENV_USER);
-  if (!strlen($username)) {
+  if ($username === null || !strlen($username)) {
     throw new Exception(
       pht(
         'No Direct Pushes: You are pushing directly to a hosted repository. '.
@@ -181,17 +181,17 @@
 $engine->setOriginalArgv(array_slice($argv, 2));
 
 $remote_address = getenv(DiffusionCommitHookEngine::ENV_REMOTE_ADDRESS);
-if (strlen($remote_address)) {
+if ($remote_address !== false && strlen($remote_address)) {
   $engine->setRemoteAddress($remote_address);
 }
 
 $remote_protocol = getenv(DiffusionCommitHookEngine::ENV_REMOTE_PROTOCOL);
-if (strlen($remote_protocol)) {
+if ($remote_protocol !== false && strlen($remote_protocol)) {
   $engine->setRemoteProtocol($remote_protocol);
 }
 
 $request_identifier = getenv(DiffusionCommitHookEngine::ENV_REQUEST);
-if (strlen($request_identifier)) {
+if ($request_identifier !== false && strlen($request_identifier)) {
   $engine->setRequestIdentifier($request_identifier);
 }
 
diff --git a/scripts/ssh/ssh-exec.php b/scripts/ssh/ssh-exec.php
@@ -103,7 +103,7 @@
         '--phabricator-ssh-device',
         $user_name,
         $device_name));
-  } else if (strlen($user_name)) {
+  } else if ($user_name !== null && strlen($user_name)) {
     $user = id(new PhabricatorPeopleQuery())
       ->setViewer(PhabricatorUser::getOmnipotentUser())
       ->withUsernames(array($user_name))
@@ -117,7 +117,7 @@
 
     id(new PhabricatorAuthSessionEngine())
       ->willServeRequestForUser($user);
-  } else if (strlen($device_name)) {
+  } else if ($device_name !== null && strlen($device_name)) {
     if (!$remote_address) {
       throw new Exception(
         pht(
diff --git a/src/aphront/response/AphrontAjaxResponse.php b/src/aphront/response/AphrontAjaxResponse.php
@@ -64,7 +64,7 @@ public function buildResponseString() {
       if ($viewer) {
         $postprocessor_key = $viewer->getUserSetting(
           PhabricatorAccessibilitySetting::SETTINGKEY);
-        if (strlen($postprocessor_key)) {
+        if ($postprocessor_key !== null && strlen($postprocessor_key)) {
           $response->setPostprocessorKey($postprocessor_key);
         }
       }
diff --git a/src/applications/auth/controller/PhabricatorAuthSSHKeyEditController.php b/src/applications/auth/controller/PhabricatorAuthSSHKeyEditController.php
@@ -54,10 +54,10 @@ public function handleRequest(AphrontRequest $request) {
       $cancel_uri);
 
     $v_name = $key->getName();
-    $e_name = strlen($v_name) ? null : true;
+    $e_name = $v_name !== null && strlen($v_name) ? null : true;
 
     $v_key = $key->getEntireKey();
-    $e_key = strlen($v_key) ? null : true;
+    $e_key = $v_key !== null && strlen($v_key) ? null : true;
 
     $validation_exception = null;
     if ($request->isFormPost()) {
diff --git a/src/applications/conduit/controller/PhabricatorConduitAPIController.php b/src/applications/conduit/controller/PhabricatorConduitAPIController.php
@@ -290,7 +290,7 @@ private function authenticateUser(
     }
 
     $token_string = idx($metadata, 'token');
-    if (strlen($token_string)) {
+    if ($token_string !== null && strlen($token_string)) {
 
       if (strlen($token_string) != 32) {
         return array(
@@ -683,7 +683,7 @@ private function decodeConduitParams(
     // Otherwise, look for a single parameter called 'params' which has the
     // entire param dictionary JSON encoded.
     $params_json = $request->getStr('params');
-    if (strlen($params_json)) {
+    if (phutil_nonempty_string($params_json)) {
       $params = null;
       try {
         $params = phutil_json_decode($params_json);
diff --git a/src/applications/diviner/atom/DivinerAtom.php b/src/applications/diviner/atom/DivinerAtom.php
@@ -81,10 +81,15 @@ public function getWarnings() {
   public function setDocblockRaw($docblock_raw) {
     $this->docblockRaw = $docblock_raw;
 
-    $parser = new PhutilDocblockParser();
-    list($text, $meta) = $parser->parse($docblock_raw);
-    $this->docblockText = $text;
-    $this->docblockMeta = $meta;
+    if ($docblock_raw !== null) {
+      $parser = new PhutilDocblockParser();
+      list($text, $meta) = $parser->parse($docblock_raw);
+      $this->docblockText = $text;
+      $this->docblockMeta = $meta;
+    } else {
+      $this->docblockText = null;
+      $this->docblockMeta = null;
+    }
 
     return $this;
   }
diff --git a/src/applications/diviner/storage/DivinerLiveSymbol.php b/src/applications/diviner/storage/DivinerLiveSymbol.php
@@ -182,7 +182,7 @@ public function getTitle() {
   public function setTitle($value) {
     $this->writeField('title', $value);
 
-    if (strlen($value)) {
+    if ($value !== null && strlen($value)) {
       $slug = DivinerAtomRef::normalizeTitleString($value);
       $hash = PhabricatorHash::digestForIndex($slug);
       $this->titleSlugHash = $hash;
diff --git a/src/applications/diviner/workflow/DivinerGenerateWorkflow.php b/src/applications/diviner/workflow/DivinerGenerateWorkflow.php
@@ -194,7 +194,7 @@ private function generateBook($book, PhutilArgumentParser $args) {
 
     $identifier = $args->getArg('repository');
     $repository = null;
-    if (strlen($identifier)) {
+    if ($identifier !== null && strlen($identifier)) {
       $repository = id(new PhabricatorRepositoryQuery())
         ->setViewer(PhabricatorUser::getOmnipotentUser())
         ->withIdentifiers(array($identifier))
diff --git a/src/applications/files/xaction/PhabricatorFileAltTextTransaction.php b/src/applications/files/xaction/PhabricatorFileAltTextTransaction.php
@@ -27,12 +27,12 @@ public function getTitle() {
     $old_value = $this->getOldValue();
     $new_value = $this->getNewValue();
 
-    if (!strlen($old_value)) {
+    if ($old_value == null || !strlen($old_value)) {
       return pht(
         '%s set the alternate text for this file to %s.',
         $this->renderAuthor(),
         $this->renderNewValue());
-    } else if (!strlen($new_value)) {
+    } else if ($new_value === null || !strlen($new_value)) {
       return pht(
         '%s removed the alternate text for this file (was %s).',
         $this->renderAuthor(),
@@ -50,13 +50,13 @@ public function getTitleForFeed() {
     $old_value = $this->getOldValue();
     $new_value = $this->getNewValue();
 
-    if (!strlen($old_value)) {
+    if ($old_value === null || !strlen($old_value)) {
       return pht(
         '%s set the alternate text for %s to %s.',
         $this->renderAuthor(),
         $this->renderObject(),
         $this->renderNewValue());
-    } else if (!strlen($new_value)) {
+    } else if ($new_value === null || !strlen($new_value)) {
       return pht(
         '%s removed the alternate text for %s (was %s).',
         $this->renderAuthor(),
diff --git a/src/applications/herald/typeahead/HeraldRuleDatasource.php b/src/applications/herald/typeahead/HeraldRuleDatasource.php
@@ -22,12 +22,14 @@ public function loadResults() {
     $query = id(new HeraldRuleQuery())
       ->setViewer($viewer);
 
-    if (preg_match('/^[hH]\d+\z/', $raw_query)) {
-      $id = trim($raw_query, 'hH');
-      $id = (int)$id;
-      $query->withIDs(array($id));
-    } else {
-      $query->withDatasourceQuery($raw_query);
+    if ($raw_query !== null && strlen($raw_query)) {
+      if (preg_match('/^[hH]\d+\z/', $raw_query)) {
+        $id = trim($raw_query, 'hH');
+        $id = (int)$id;
+        $query->withIDs(array($id));
+      } else {
+        $query->withDatasourceQuery($raw_query);
+      }
     }
 
     $rules = $query->execute();
diff --git a/src/applications/macro/engine/PhabricatorMemeEngine.php b/src/applications/macro/engine/PhabricatorMemeEngine.php
@@ -180,8 +180,8 @@ private function newAssetData(PhabricatorFile $template) {
     // When we aren't adding text, just return the data unmodified. This saves
     // us from doing expensive stitching when we aren't actually making any
     // changes to the image.
-    $above_text = $this->getAboveText();
-    $below_text = $this->getBelowText();
+    $above_text = coalesce($this->getAboveText(), '');
+    $below_text = coalesce($this->getBelowText(), '');
     if (!strlen(trim($above_text)) && !strlen(trim($below_text))) {
       return $template_data;
     }
@@ -271,15 +271,15 @@ private function newGDAsset(PhabricatorFile $template, $data) {
     $font = $this->getFont();
     $size = $metrics['size'];
 
-    $above = $this->getAboveText();
+    $above = coalesce($this->getAboveText(), '');
     if (strlen($above)) {
       $x = (int)floor(($dx - $metrics['text']['above']['width']) / 2);
       $y = $metrics['text']['above']['height'] + 12;
 
       $this->drawText($img, $font, $metrics['size'], $x, $y, $above);
     }
 
-    $below = $this->getBelowText();
+    $below = coalesce($this->getBelowText(), '');
     if (strlen($below)) {
       $x = (int)floor(($dx - $metrics['text']['below']['width']) / 2);
       $y = $dy - 12 - $metrics['text']['below']['descend'];
@@ -331,6 +331,7 @@ private function getMetrics($dim_x, $dim_y) {
         $all_fit = true;
         $text_metrics = array();
         foreach ($texts as $key => $text) {
+          $text = coalesce($text, '');
           $box = imagettfbbox($cursor, 0, $font, $text);
           $height = abs($box[3] - $box[5]);
           $width = abs($box[0] - $box[2]);
diff --git a/src/applications/maniphest/xaction/ManiphestTaskPointsTransaction.php b/src/applications/maniphest/xaction/ManiphestTaskPointsTransaction.php
@@ -98,7 +98,7 @@ public function getIcon() {
   }
 
   private function getValueForPoints($value) {
-    if (!strlen($value)) {
+    if ($value !== null && !strlen($value)) {
       $value = null;
     }
     if ($value !== null) {
diff --git a/src/applications/metamta/controller/PhabricatorMetaMTAMailViewController.php b/src/applications/metamta/controller/PhabricatorMetaMTAMailViewController.php
@@ -421,7 +421,7 @@ private function buildMetadataProperties(PhabricatorMetaMTAMail $mail) {
     $properties->addProperty(pht('Message PHID'), $mail->getPHID());
 
     $details = $mail->getMessage();
-    if (!strlen($details)) {
+    if ($details === null || !strlen($details)) {
       $details = phutil_tag('em', array(), pht('None'));
     }
     $properties->addProperty(pht('Status Details'), $details);
diff --git a/src/applications/metamta/management/PhabricatorMailManagementSendTestWorkflow.php b/src/applications/metamta/management/PhabricatorMailManagementSendTestWorkflow.php
@@ -74,7 +74,7 @@ public function execute(PhutilArgumentParser $args) {
     $viewer = $this->getViewer();
 
     $type = $args->getArg('type');
-    if (!strlen($type)) {
+    if ($type === null || !strlen($type)) {
       $type = PhabricatorMailEmailMessage::MESSAGETYPE;
     }
 
diff --git a/src/applications/owners/typeahead/PhabricatorOwnersPackageDatasource.php b/src/applications/owners/typeahead/PhabricatorOwnersPackageDatasource.php
@@ -24,14 +24,16 @@ public function loadResults() {
     $query = id(new PhabricatorOwnersPackageQuery())
       ->setOrder('name');
 
-    // If the user is querying by monogram explicitly, like "O123", do an ID
-    // search. Otherwise, do an ngram substring search.
-    if (preg_match('/^[oO]\d+\z/', $raw_query)) {
-      $id = trim($raw_query, 'oO');
-      $id = (int)$id;
-      $query->withIDs(array($id));
-    } else {
-      $query->withNameNgrams($raw_query);
+    if ($raw_query !== null && strlen($raw_query)) {
+      // If the user is querying by monogram explicitly, like "O123", do an ID
+      // search. Otherwise, do an ngram substring search.
+      if (preg_match('/^[oO]\d+\z/', $raw_query)) {
+        $id = trim($raw_query, 'oO');
+        $id = (int)$id;
+        $query->withIDs(array($id));
+      } else {
+        $query->withNameNgrams($raw_query);
+      }
     }
 
     $packages = $this->executeQuery($query);
diff --git a/src/applications/people/mail/PhabricatorPeopleEmailLoginMailEngine.php b/src/applications/people/mail/PhabricatorPeopleEmailLoginMailEngine.php
@@ -64,7 +64,7 @@ protected function newMail() {
     $message_body = PhabricatorAuthMessage::loadMessageText(
       $recipient,
       $message_key);
-    if (strlen($message_body)) {
+    if ($message_body !== null && strlen($message_body)) {
       $body[] = $this->newRemarkupText($message_body);
     }
 
diff --git a/src/applications/people/mail/PhabricatorPeopleWelcomeMailEngine.php b/src/applications/people/mail/PhabricatorPeopleWelcomeMailEngine.php
@@ -118,14 +118,14 @@ private function newBody() {
     $recipient = $this->getRecipient();
 
     $custom_body = $this->getWelcomeMessage();
-    if (strlen($custom_body)) {
+    if ($custom_body !== null && strlen($custom_body)) {
       return $this->newRemarkupText($custom_body);
     }
 
     $default_body = PhabricatorAuthMessage::loadMessageText(
       $recipient,
       PhabricatorAuthWelcomeMailMessageType::MESSAGEKEY);
-    if (strlen($default_body)) {
+    if ($default_body !== null && strlen($default_body)) {
       return $this->newRemarkupText($default_body);
     }
 
diff --git a/src/applications/phame/controller/PhameLiveController.php b/src/applications/phame/controller/PhameLiveController.php
@@ -87,7 +87,7 @@ protected function setupLiveEnvironment() {
     $this->isExternal = $is_external;
     $this->isLive = $is_live;
 
-    if (strlen($post_id)) {
+    if ($post_id !== null && strlen($post_id)) {
       $post_query = id(new PhamePostQuery())
         ->setViewer($viewer)
         ->needHeaderImage(true)
diff --git a/src/applications/phame/storage/PhamePost.php b/src/applications/phame/storage/PhamePost.php
@@ -67,7 +67,8 @@ public function getLiveURI() {
     $blog = $this->getBlog();
     $is_draft = $this->isDraft();
     $is_archived = $this->isArchived();
-    if (strlen($blog->getDomain()) && !$is_draft && !$is_archived) {
+    $is_external = $blog->getDomain() !== null && strlen($blog->getDomain());
+    if ($is_external && !$is_draft && !$is_archived) {
       return $this->getExternalLiveURI();
     } else {
       return $this->getInternalLiveURI();
diff --git a/src/applications/phriction/editor/PhrictionTransactionEditor.php b/src/applications/phriction/editor/PhrictionTransactionEditor.php
@@ -307,7 +307,7 @@ protected function buildMailBody(
     }
 
     $description = $object->getContent()->getDescription();
-    if (strlen($description)) {
+    if ($description !== null && strlen($description)) {
       $body->addTextSection(
         pht('EDIT NOTES'),
         $description);
@@ -556,7 +556,7 @@ private function newDocumentContent(PhrictionDocument $document) {
       ->setContent($this->getOldContent()->getContent())
       ->setDescription('');
 
-    if (strlen($this->getDescription())) {
+    if ($this->getDescription() !== null && strlen($this->getDescription())) {
       $content->setDescription($this->getDescription());
     }
 
diff --git a/src/applications/project/controller/PhabricatorProjectMoveController.php b/src/applications/project/controller/PhabricatorProjectMoveController.php
@@ -16,7 +16,7 @@ public function handleRequest(AphrontRequest $request) {
     $before_phids = $request->getStrList('beforePHIDs');
 
     $order = $request->getStr('order');
-    if (!strlen($order)) {
+    if ($order === null || !strlen($order)) {
       $order = PhabricatorProjectColumnNaturalOrder::ORDERKEY;
     }
 
@@ -26,7 +26,7 @@ public function handleRequest(AphrontRequest $request) {
 
     $edit_header = null;
     $raw_header = $request->getStr('header');
-    if (strlen($raw_header)) {
+    if ($raw_header !== null && strlen($raw_header)) {
       $edit_header = phutil_json_decode($raw_header);
     } else {
       $edit_header = array();
diff --git a/src/applications/project/state/PhabricatorWorkboardViewState.php b/src/applications/project/state/PhabricatorWorkboardViewState.php
@@ -41,7 +41,8 @@ public function readFromRequest(AphrontRequest $request) {
       $this->requestState['filter'] = $request->getStr('filter');
     }
 
-    if (strlen($request->getURIData('queryKey'))) {
+    $query_key = $request->getURIData('queryKey');
+    if ($query_key !== null && strlen($query_key)) {
       $this->requestState['filter'] = $request->getURIData('queryKey');
     }
 
@@ -101,7 +102,7 @@ private function newSearchEngine() {
       ->setIsBoardView(true);
   }
 
-  public function newWorkboardURI($path = null) {
+  public function newWorkboardURI($path = '') {
     $project = $this->getProject();
     $uri = urisprintf('%s%s', $project->getWorkboardURI(), $path);
     return $this->newURI($uri);
@@ -169,7 +170,7 @@ public function getOrder() {
 
   public function getQueryKey() {
     $request_query = idx($this->requestState, 'filter');
-    if (strlen($request_query)) {
+    if ($request_query !== null && strlen($request_query)) {
       return $request_query;
     }
 
@@ -203,7 +204,7 @@ private function getDefaultQueryKey() {
 
     $default_query = $project->getDefaultWorkboardFilter();
 
-    if (strlen($default_query)) {
+    if ($default_query !== null && strlen($default_query)) {
       return $default_query;
     }
 
diff --git a/src/applications/project/typeahead/PhabricatorProjectDatasource.php b/src/applications/project/typeahead/PhabricatorProjectDatasource.php
diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php
diff --git a/src/applications/search/controller/PhabricatorSearchController.php b/src/applications/search/controller/PhabricatorSearchController.php
diff --git a/src/applications/settings/panel/PhabricatorEmailAddressesSettingsPanel.php b/src/applications/settings/panel/PhabricatorEmailAddressesSettingsPanel.php
diff --git a/src/applications/transactions/bulk/PhabricatorBulkEngine.php b/src/applications/transactions/bulk/PhabricatorBulkEngine.php
diff --git a/src/infrastructure/log/PhabricatorSSHLog.php b/src/infrastructure/log/PhabricatorSSHLog.php
diff --git a/support/startup/preamble-utils.php b/support/startup/preamble-utils.php

Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,7 @@ public function buildResponseString() {`
`64`	`64`	`if ($viewer) {`
`65`	`65`	`$postprocessor_key = $viewer->getUserSetting(`
`66`	`66`	`PhabricatorAccessibilitySetting::SETTINGKEY);`
`67`		`- if (strlen($postprocessor_key)) {`
	`67`	`+ if ($postprocessor_key !== null && strlen($postprocessor_key)) {`
`68`	`68`	`$response->setPostprocessorKey($postprocessor_key);`
`69`	`69`	`}`
`70`	`70`	`}`