Provide additional Arcanist PHP 8.1 fixes

Summary: Ref T13588. I pointed my local `php` at PHP 8.1 and this is what I've hit so far; all these cases seem very unlikely to have any subtle behavior. Test Plan: Ran various `arc` workflows under PHP 8.1. Maniphest Tasks: T13588 Differential Revision: https://secure.phabricator.com/D21742
BearerPipelineTest · Dec 10, 2021 · b50a646 · b50a646
1 parent 488f13a
commit b50a646
Show file tree

Hide file tree

Showing 21 changed files with 116 additions and 34 deletions.
diff --git a/src/configuration/ArcanistConfigurationManager.php b/src/configuration/ArcanistConfigurationManager.php
@@ -258,7 +258,7 @@ public function setUserConfigurationFileLocation($custom_arcrc) {
   }
 
   public function getUserConfigurationFileLocation() {
-    if (strlen($this->customArcrcFilename)) {
+    if ($this->customArcrcFilename !== null) {
       return $this->customArcrcFilename;
     }
 

diff --git a/src/filesystem/PhutilDirectoryFixture.php b/src/filesystem/PhutilDirectoryFixture.php
@@ -28,6 +28,8 @@ public function __destruct() {
   }
 
   public function getPath($to_file = null) {
+    $to_file = phutil_string_cast($to_file);
+
     return $this->path.'/'.ltrim($to_file, '/');
   }
 

diff --git a/src/future/exec/ExecFuture.php b/src/future/exec/ExecFuture.php
@@ -192,11 +192,18 @@ public function setReadBufferSize($read_buffer_size) {
    * @task interact
    */
   public function read() {
-    $stdout = $this->readStdout();
+    $stdout_value = $this->readStdout();
+
+    $stderr = $this->stderr;
+    if ($stderr === null) {
+      $stderr_value = '';
+    } else {
+      $stderr_value = substr($stderr, $this->stderrPos);
+    }
 
     $result = array(
-      $stdout,
-      (string)substr($this->stderr, $this->stderrPos),
+      $stdout_value,
+      $stderr_value,
     );
 
     $this->stderrPos = $this->getStderrBufferLength();
@@ -209,7 +216,14 @@ public function readStdout() {
       $this->updateFuture(); // Sync
     }
 
-    $result = (string)substr($this->stdout, $this->stdoutPos);
+    $stdout = $this->stdout;
+
+    if ($stdout === null) {
+      $result = '';
+    } else {
+      $result = substr($stdout, $this->stdoutPos);
+    }
+
     $this->stdoutPos = $this->getStdoutBufferLength();
 
     return $result;

diff --git a/src/lint/linter/ArcanistXMLLinter.php b/src/lint/linter/ArcanistXMLLinter.php
@@ -27,7 +27,11 @@ public function canRun() {
   }
 
   public function getCacheVersion() {
-    return LIBXML_VERSION;
+    if (defined('LIBXML_VERSION')) {
+      return LIBXML_VERSION;
+    } else {
+      return 'unavailable';
+    }
   }
 
   public function lintPath($path) {

diff --git a/src/lint/linter/__tests__/ArcanistLinterTestCase.php b/src/lint/linter/__tests__/ArcanistLinterTestCase.php
@@ -51,6 +51,13 @@ protected function executeTestsInDirectory($root) {
   private function lintFile($file, ArcanistLinter $linter) {
     $linter = clone $linter;
 
+    if (!$linter->canRun()) {
+      $this->assertSkipped(
+        pht(
+          'Linter "%s" can not run.',
+          get_class($linter)));
+    }
+
     $contents = Filesystem::readFile($file);
     $contents = preg_split('/^~{4,}\n/m', $contents);
     if (count($contents) < 2) {
@@ -283,9 +290,12 @@ private function compareLint($file, $expect, ArcanistLintResult $results) {
   }
 
   private function compareTransform($expected, $actual) {
+    $expected = phutil_string_cast($expected);
+
     if (!strlen($expected)) {
       return;
     }
+
     $this->assertEqual(
       $expected,
       $actual,

diff --git a/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php
@@ -155,7 +155,9 @@ public function process(XHPASTNode $root) {
       if ($this->windowsVersion) {
         $windows = idx($compat_info['functions_windows'], $name);
 
-        if ($windows === false) {
+        if ($windows === null) {
+          // This function has no special Windows considerations.
+        } else if ($windows === false) {
           $this->raiseLintAtNode(
             $node,
             pht(

diff --git a/src/lint/linter/xhpast/rules/ArcanistParentMemberReferenceXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistParentMemberReferenceXHPASTLinterRule.php
@@ -55,7 +55,12 @@ public function process(XHPASTNode $root) {
             }
           }
 
-          if (version_compare($this->version, '5.4.0', '>=') || !$in_closure) {
+          $version_target = $this->version;
+          if ($version_target === null) {
+            $version_target = phpversion();
+          }
+
+          if (version_compare($version_target, '5.4.0', '>=') || !$in_closure) {
             $this->raiseLintAtNode(
               $class_ref,
               pht(

diff --git a/src/lint/linter/xhpast/rules/ArcanistSelfMemberReferenceXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistSelfMemberReferenceXHPASTLinterRule.php
@@ -42,7 +42,12 @@ public function process(XHPASTNode $root) {
             }
           }
 
-          if (version_compare($this->version, '5.4.0', '>=') || !$in_closure) {
+          $version_target = $this->version;
+          if (!$version_target) {
+            $version_target = phpversion();
+          }
+
+          if (version_compare($version_target, '5.4.0', '>=') || !$in_closure) {
             $this->raiseLintAtNode(
               $class_ref,
               pht(

diff --git a/src/parser/ArcanistBundle.php b/src/parser/ArcanistBundle.php
@@ -639,8 +639,7 @@ private function getOldPath(ArcanistDiffChange $change) {
     $old_path = $change->getOldPath();
     $type = $change->getType();
 
-    if (!strlen($old_path) ||
-        $type == ArcanistDiffChangeType::TYPE_ADD) {
+    if ($old_path === '' || $type == ArcanistDiffChangeType::TYPE_ADD) {
       $old_path = null;
     }
 
@@ -1023,7 +1022,7 @@ public static function newBase85Data($data, $eol, $mode = null) {
         if ($is_64bit) {
           for ($count = 4; $count >= 0; $count--) {
             $val = $accum % 85;
-            $accum = $accum / 85;
+            $accum = (int)($accum / 85);
             $slice .= $map[$val];
           }
         } else {

diff --git a/src/parser/ArcanistDiffParser.php b/src/parser/ArcanistDiffParser.php
@@ -217,7 +217,7 @@ public function parseDiff($diff) {
       $line = $this->nextLine();
     }
 
-    if (strlen($message)) {
+    if ($message !== null && strlen($message)) {
       // If we found a message during pre-parse steps, add it to the resulting
       // changes here.
       $change = $this->buildChange(null)
@@ -582,10 +582,13 @@ protected function parseIndexHunk(ArcanistDiffChange $change) {
 
         $ok = false;
         $match = null;
-        foreach ($patterns as $pattern) {
-          $ok = preg_match('@^'.$pattern.'@', $line, $match);
-          if ($ok) {
-            break;
+
+        if ($line !== null) {
+          foreach ($patterns as $pattern) {
+            $ok = preg_match('@^'.$pattern.'@', $line, $match);
+            if ($ok) {
+              break;
+            }
           }
         }
 
@@ -774,7 +777,7 @@ protected function parseIndexHunk(ArcanistDiffChange $change) {
       $this->nextLine();
       $this->parseGitBinaryPatch();
       $line = $this->getLine();
-      if (preg_match('/^literal/', $line)) {
+      if ($line !== null && preg_match('/^literal/', $line)) {
         // We may have old/new binaries (change) or just a new binary (hg add).
         // If there are two blocks, parse both.
         $this->parseGitBinaryPatch();
@@ -920,11 +923,11 @@ protected function parseChangeset(ArcanistDiffChange $change) {
       $hunk->setNewOffset($matches[3]);
 
       // Cover for the cases where length wasn't present (implying one line).
-      $old_len = idx($matches, 2);
+      $old_len = idx($matches, 2, '');
       if (!strlen($old_len)) {
         $old_len = 1;
       }
-      $new_len = idx($matches, 4);
+      $new_len = idx($matches, 4, '');
       if (!strlen($new_len)) {
         $new_len = 1;
       }
@@ -1041,7 +1044,7 @@ protected function parseChangeset(ArcanistDiffChange $change) {
         $line = $this->nextNonemptyLine();
       }
 
-    } while (preg_match('/^@@ /', $line));
+    } while (($line !== null) && preg_match('/^@@ /', $line));
   }
 
   protected function buildChange($path = null) {

diff --git a/src/parser/PhutilBugtraqParser.php b/src/parser/PhutilBugtraqParser.php
@@ -85,7 +85,7 @@ public function processCorpus($corpus) {
       $captured_text = $capture['text'];
       $captured_offset = $capture['at'];
 
-      if (strlen($select_regexp)) {
+      if ($select_regexp !== null) {
         $selections = null;
         preg_match_all(
           $select_regexp,

diff --git a/src/parser/PhutilEmailAddress.php b/src/parser/PhutilEmailAddress.php
@@ -13,6 +13,7 @@ final class PhutilEmailAddress extends Phobject {
   private $domainName;
 
   public function __construct($email_address = null) {
+    $email_address = phutil_string_cast($email_address);
     $email_address = trim($email_address);
 
     $matches = null;
@@ -89,7 +90,7 @@ public function setAddress($address) {
 
   public function getAddress() {
     $address = $this->localPart;
-    if (strlen($this->domainName)) {
+    if ($this->domainName !== null && strlen($this->domainName)) {
       $address .= '@'.$this->domainName;
     }
     return $address;

diff --git a/src/parser/PhutilURI.php b/src/parser/PhutilURI.php
@@ -171,14 +171,19 @@ public function __toString() {
       }
     }
 
-    if (strlen($protocol) || strlen($auth) || strlen($domain)) {
+    $has_protocol = ($protocol !== null) && strlen($protocol);
+    $has_auth = ($auth !== null);
+    $has_domain = ($domain !== null) && strlen($domain);
+    $has_port = ($port !== null) && strlen($port);
+
+    if ($has_protocol || $has_auth || $has_domain) {
       if ($this->isGitURI()) {
         $prefix = "{$auth}{$domain}";
       } else {
         $prefix = "{$protocol}://{$auth}{$domain}";
       }
 
-      if (strlen($port)) {
+      if ($has_port) {
         $prefix .= ':'.$port;
       }
     }

diff --git a/src/parser/aast/api/AASTNodeList.php b/src/parser/aast/api/AASTNodeList.php
@@ -80,29 +80,35 @@ public static function newFromTree(AASTTree $tree) {
 
 /* -(  Countable  )---------------------------------------------------------- */
 
+  #[\ReturnTypeWillChange]
   public function count() {
     return count($this->ids);
   }
 
 
 /* -(  Iterator  )----------------------------------------------------------- */
 
+  #[\ReturnTypeWillChange]
   public function current() {
     return $this->list[$this->key()];
   }
 
+  #[\ReturnTypeWillChange]
   public function key() {
     return $this->ids[$this->pos];
   }
 
+  #[\ReturnTypeWillChange]
   public function next() {
     $this->pos++;
   }
 
+  #[\ReturnTypeWillChange]
   public function rewind() {
     $this->pos = 0;
   }
 
+  #[\ReturnTypeWillChange]
   public function valid() {
     return $this->pos < count($this->ids);
   }

diff --git a/src/serviceprofiler/PhutilServiceProfiler.php b/src/serviceprofiler/PhutilServiceProfiler.php
@@ -137,7 +137,7 @@ public static function echoListener($type, $id, $data) {
 
           $uri = phutil_censor_credentials($data['uri']);
 
-          if (strlen($proxy)) {
+          if ($proxy !== null) {
             $desc = "{$proxy} >> {$uri}";
           } else {
             $desc = $uri;
@@ -203,6 +203,10 @@ public static function echoListener($type, $id, $data) {
   }
 
   private static function escapeProfilerStringForDisplay($string) {
+    if ($string === null) {
+      return '';
+    }
+
     // Convert tabs and newlines to spaces and collapse blocks of whitespace,
     // most often formatting in queries.
     $string = preg_replace('/\s{2,}/', ' ', $string);

diff --git a/src/unit/renderer/ArcanistUnitConsoleRenderer.php b/src/unit/renderer/ArcanistUnitConsoleRenderer.php
@@ -65,17 +65,19 @@ private function formatTestDuration($seconds) {
       50   => "<fg:green>%s</fg><fg:yellow>{$star}</fg> ",
       200  => '<fg:green>%s</fg>  ',
       500  => '<fg:yellow>%s</fg>  ',
-      INF  => '<fg:red>%s</fg>  ',
     );
 
+    $least_acceptable = '<fg:red>%s</fg>  ';
+
     $milliseconds = $seconds * 1000;
     $duration = $this->formatTime($seconds);
     foreach ($acceptableness as $upper_bound => $formatting) {
       if ($milliseconds <= $upper_bound) {
         return phutil_console_format($formatting, $duration);
       }
     }
-    return phutil_console_format(end($acceptableness), $duration);
+
+    return phutil_console_format($least_acceptable, $duration);
   }
 
   private function formatTime($seconds) {

diff --git a/src/utils/PhutilArray.php b/src/utils/PhutilArray.php
@@ -29,6 +29,7 @@ public function toArray() {
 /* -(  Countable Interface  )------------------------------------------------ */
 
 
+  #[\ReturnTypeWillChange]
   public function count() {
     return count($this->data);
   }
@@ -37,22 +38,27 @@ public function count() {
 /* -(  Iterator Interface  )------------------------------------------------- */
 
 
+  #[\ReturnTypeWillChange]
   public function current() {
     return current($this->data);
   }
 
+  #[\ReturnTypeWillChange]
   public function key() {
     return key($this->data);
   }
 
+  #[\ReturnTypeWillChange]
   public function next() {
     return next($this->data);
   }
 
+  #[\ReturnTypeWillChange]
   public function rewind() {
     reset($this->data);
   }
 
+  #[\ReturnTypeWillChange]
   public function valid() {
     return (key($this->data) !== null);
   }
@@ -61,18 +67,22 @@ public function valid() {
 /* -(  ArrayAccess Interface  )---------------------------------------------- */
 
 
+  #[\ReturnTypeWillChange]
   public function offsetExists($key) {
     return array_key_exists($key, $this->data);
   }
 
+  #[\ReturnTypeWillChange]
   public function offsetGet($key) {
     return $this->data[$key];
   }
 
+  #[\ReturnTypeWillChange]
   public function offsetSet($key, $value) {
     $this->data[$key] = $value;
   }
 
+  #[\ReturnTypeWillChange]
   public function offsetUnset($key) {
     unset($this->data[$key]);
   }
-Original file line number
+Diff line change
@@ Expand Up / @@ -28,6 +28,8 @@ public function __destruct() { @@
       }
       public function getPath($to_file = null) {
+        $to_file = phutil_string_cast($to_file);
         return $this->path.'/'.ltrim($to_file, '/');
       }
@@ Expand Down @@