Promote 2024.19 to stable

src/filesystem/Filesystem.php

@@ -275,7 +275,7 @@ final class Filesystem extends Phobject {
       $trap->destroy();
 
       if (!$ok) {
-        if (strlen($err)) {
+        if ($err !== null && strlen($err)) {
           throw new FilesystemException(
             $to,
             pht(
@@ -307,7 +307,7 @@ final class Filesystem extends Phobject {
    * @task   file
    */
   public static function remove($path) {
-    if (!strlen($path)) {
+    if ($path == null || !strlen($path)) {
       // Avoid removing PWD.
       throw new Exception(
         pht(
@@ -673,7 +673,7 @@ final class Filesystem extends Phobject {
     }
 
     // If we come back with an encoding, strip it off.
-    if (strpos($mime_type, ';') !== false) {
+    if ($mime_type !== null && strpos($mime_type, ';') !== false) {
       list($type, $encoding) = explode(';', $mime_type, 2);
       $mime_type = $type;
     }

src/future/http/status/HTTPFutureHTTPResponseStatus.php

@@ -21,7 +21,8 @@ final class HTTPFutureHTTPResponseStatus extends HTTPFutureResponseStatus {
 
     $content_type = BaseHTTPFuture::getHeader($headers, 'Content-Type');
     $match = null;
-    if (preg_match('/;\s*charset=([^;]+)/', $content_type, $match)) {
+    if (phutil_nonempty_string($content_type) &&
+      preg_match('/;\s*charset=([^;]+)/', $content_type, $match)) {
       $encoding = trim($match[1], "\"'");
       try {
         $excerpt = phutil_utf8_convert($excerpt, 'UTF-8', $encoding);

src/init/lib/PhutilMissingSymbolException.php

@@ -18,7 +18,8 @@ final class PhutilMissingSymbolException extends Exception {
       'moved, your library map may need to be rebuilt. You can rebuild '.
       'the map by running "arc liberate".'.
       "\n\n".
-      'For more information, see: https://phurl.io/u/newclasses',
+      'For more information, see: '.
+      'https://we.phorge.it/book/contrib/article/adding_new_classes/',
       $symbol,
       $type,
       $reason);

src/moduleutils/PhutilLibraryMapBuilder.php

@@ -332,11 +332,16 @@ final class PhutilLibraryMapBuilder extends Phobject {
       'xmap'      => array(),
     );
 
+    $type_translation = array(
+      'interface' => 'class',
+      'trait' => 'class',
+    );
+
     // Detect duplicate symbols within the library.
     foreach ($symbol_map as $file => $info) {
       foreach ($info['have'] as $type => $symbols) {
         foreach ($symbols as $symbol => $declaration) {
-          $lib_type = ($type == 'interface') ? 'class' : $type;
+          $lib_type = idx($type_translation, $type, $type);
           if (!empty($library_map[$lib_type][$symbol])) {
             $prior = $library_map[$lib_type][$symbol];
             throw new Exception(

src/parser/ArcanistDiffParser.php

@@ -187,6 +187,11 @@ final class ArcanistDiffParser extends Phobject {
   }
 
   public function parseDiff($diff) {
+    // Remove leading UTF-8 Byte Order Mark (BOM)
+    if (substr($diff, 0, 3) == pack('CCC', 0xEF, 0xBB, 0xBF)) {
+      $diff = substr($diff, 3);
+    }
+
     if (!strlen(trim($diff))) {
       throw new Exception(pht("Can't parse an empty diff!"));
     }

src/runtime/ArcanistRuntime.php

@@ -188,7 +188,7 @@ final class ArcanistRuntime {
         tsprintf(
           '%s <__%s__>',
           pht('Learn More:'),
-          'https://phurl.io/u/noninteractive'));
+          'https://secure.phabricator.com/T13491'));
 
       throw new PhutilArgumentUsageException(
         pht('Missing required "--" in argument list.'));

src/symbols/PhutilSymbolLoader.php

@@ -398,6 +398,12 @@ final class PhutilSymbolLoader {
   }
 
 
+  private static function classLikeExists($name) {
+    return class_exists($name, false) ||
+           interface_exists($name, false) ||
+           trait_exists($name, false);
+  }
+
   /**
    * @task internal
    */
@@ -411,7 +417,7 @@ final class PhutilSymbolLoader {
         return;
       }
     } else {
-      if (class_exists($name, false) || interface_exists($name, false)) {
+      if (self::classLikeExists($name)) {
         return;
       }
     }
@@ -431,7 +437,7 @@ final class PhutilSymbolLoader {
         $load_failed = pht('function');
       }
     } else {
-      if (!class_exists($name, false) && !interface_exists($name, false)) {
+      if (!self::classLikeExists($name)) {
         $load_failed = pht('class or interface');
       }
     }

src/unit/engine/PhpunitTestEngine.php

@@ -52,7 +52,7 @@ final class PhpunitTestEngine extends ArcanistUnitTestEngine {
       if (!Filesystem::pathExists($test_path)) {
         continue;
       }
-      $json_tmp = new TempFile();
+      $xml_tmp = new TempFile();
       $clover_tmp = null;
       $clover = null;
       if ($this->getEnableCoverage() !== false) {
@@ -64,10 +64,10 @@ final class PhpunitTestEngine extends ArcanistUnitTestEngine {
 
       $stderr = '-d display_errors=stderr';
 
-      $futures[$test_path] = new ExecFuture('%C %C %C --log-json %s %C %s',
-        $this->phpunitBinary, $config, $stderr, $json_tmp, $clover, $test_path);
+      $futures[$test_path] = new ExecFuture('%C %C %C --log-junit %s %C %s',
+        $this->phpunitBinary, $config, $stderr, $xml_tmp, $clover, $test_path);
       $tmpfiles[$test_path] = array(
-        'json' => $json_tmp,
+        'xml' => $xml_tmp,
         'clover' => $clover_tmp,
       );
     }
@@ -81,7 +81,7 @@ final class PhpunitTestEngine extends ArcanistUnitTestEngine {
 
       $results[] = $this->parseTestResults(
         $test,
-        $tmpfiles[$test]['json'],
+        $tmpfiles[$test]['xml'],
         $tmpfiles[$test]['clover'],
         $stderr);
     }
@@ -99,8 +99,8 @@ final class PhpunitTestEngine extends ArcanistUnitTestEngine {
    *
    * @return array
    */
-  private function parseTestResults($path, $json_tmp, $clover_tmp, $stderr) {
-    $test_results = Filesystem::readFile($json_tmp);
+  private function parseTestResults($path, $xml_tmp, $clover_tmp, $stderr) {
+    $test_results = Filesystem::readFile($xml_tmp);
     return id(new ArcanistPhpunitTestResultParser())
       ->setEnableCoverage($this->getEnableCoverage())
       ->setProjectRoot($this->projectRoot)

src/unit/parser/ArcanistPhpunitTestResultParser.php

@@ -25,80 +25,19 @@ final class ArcanistPhpunitTestResultParser extends ArcanistTestResultParser {
       return array($result);
     }
 
-    $report = $this->getJsonReport($test_results);
-
     // coverage is for all testcases in the executed $path
     $coverage = array();
     if ($this->enableCoverage !== false) {
       $coverage = $this->readCoverage();
     }
 
-    $last_test_finished = true;
+    $xunit_result_parser = new ArcanistXUnitTestResultParser();
+    $results = $xunit_result_parser->parseTestResults($test_results);
 
-    $results = array();
-    foreach ($report as $event) {
-      switch (idx($event, 'event')) {
-      case 'test':
-        break;
-      case 'testStart':
-        $last_test_finished = false;
-        // fall through
-        default:
-          continue 2; // switch + loop
-      }
-
-      $status = ArcanistUnitTestResult::RESULT_PASS;
-      $user_data = '';
-
-      if ('fail' == idx($event, 'status')) {
-        $status = ArcanistUnitTestResult::RESULT_FAIL;
-        $user_data  .= idx($event, 'message')."\n";
-        foreach (idx($event, 'trace') as $trace) {
-          $user_data .= sprintf(
-            "\n%s:%s",
-            idx($trace, 'file'),
-            idx($trace, 'line'));
-        }
-      } else if ('error' == idx($event, 'status')) {
-        if (strpos(idx($event, 'message'), 'Skipped Test') !== false) {
-          $status = ArcanistUnitTestResult::RESULT_SKIP;
-          $user_data .= idx($event, 'message');
-        } else if (strpos(
-            idx($event, 'message'),
-            'Incomplete Test') !== false) {
-          $status = ArcanistUnitTestResult::RESULT_SKIP;
-          $user_data .= idx($event, 'message');
-        } else {
-          $status = ArcanistUnitTestResult::RESULT_BROKEN;
-          $user_data  .= idx($event, 'message');
-          foreach (idx($event, 'trace') as $trace) {
-            $user_data .= sprintf(
-              "\n%s:%s",
-              idx($trace, 'file'),
-              idx($trace, 'line'));
-          }
-        }
-      }
-
-      $name = preg_replace('/ \(.*\)/s', '', idx($event, 'test'));
-
-      $result = new ArcanistUnitTestResult();
-      $result->setName($name);
-      $result->setResult($status);
-      $result->setDuration(idx($event, 'time'));
+    foreach ($results as $result) {
       $result->setCoverage($coverage);
-      $result->setUserData($user_data);
-
-      $results[] = $result;
-      $last_test_finished = true;
     }
 
-    if (!$last_test_finished) {
-      $results[] = id(new ArcanistUnitTestResult())
-        ->setName(idx($event, 'test')) // use last event
-        ->setUserData($this->stderr)
-        ->setResult(ArcanistUnitTestResult::RESULT_BROKEN);
-    }
     return $results;
   }
 
@@ -161,28 +100,4 @@ final class ArcanistPhpunitTestResultParser extends ArcanistTestResultParser {
     return $reports;
   }
 
-  /**
-   * We need this non-sense to make json generated by phpunit
-   * valid.
-   *
-   * @param string $json String containing JSON report
-   * @return array JSON decoded array
-   */
-  private function getJsonReport($json) {
-
-    if (empty($json)) {
-      throw new Exception(
-        pht(
-          'JSON report file is empty, it probably means that phpunit '.
-          'failed to run tests. Try running %s with %s option and then run '.
-          'generated phpunit command yourself, you might get the answer.',
-          'arc unit',
-          '--trace'));
-    }
-
-    $json = preg_replace('/}{\s*"/', '},{"', $json);
-    $json = '['.$json.']';
-    return phutil_json_decode($json);
-  }
-
 }

src/utils/PhutilSortVector.php

@@ -30,7 +30,7 @@ final class PhutilSortVector
   }
 
   public function addString($value) {
-    if (strlen($value) && (strpos("\0", $value) !== false)) {
+    if (strlen($value) && (strpos($value, "\0") !== false)) {
       throw new Exception(
         pht(
           'String components of a sort vector must not contain NULL bytes.'));

src/utils/utils.php

@@ -13,8 +13,9 @@
  *
  *    id(new Thing())->doStuff();
  *
- * @param   wild Anything.
- * @return  wild Unmodified argument.
+ * @template T
+ * @param   T $x Anything
+ * @return  T Unmodified argument.
  */
 function id($x) {
   return $x;

src/workflow/ArcanistWorkflow.php

@@ -1516,7 +1516,14 @@ abstract class ArcanistWorkflow extends Phobject {
     }
   }
 
+  /**
+   * @param string|null $revision_id
+   * @return string
+   */
   final protected function normalizeRevisionID($revision_id) {
+    if ($revision_id === null) {
+      return '';
+    }
     return preg_replace('/^D/i', '', $revision_id);
   }
 

src/workingcopyidentity/ArcanistWorkingCopyIdentity.php

@@ -289,7 +289,7 @@ final class ArcanistWorkingCopyIdentity extends Phobject {
   }
 
   public function readLocalArcConfig() {
-    if (strlen($this->localMetaDir)) {
+    if ($this->localMetaDir !== null && strlen($this->localMetaDir)) {
       $local_path = Filesystem::resolvePath('arc/config', $this->localMetaDir);
 
       $console = PhutilConsole::getConsole();

support/lib/extract-symbols.php

@@ -112,18 +112,6 @@ foreach ($namespaces as $namespace) {
     $namespace, $path, pht('namespace `%s` statements', 'use'));
 }
 
-$possible_traits = $root->selectDescendantsOfType('n_CLASS_DECLARATION');
-foreach ($possible_traits as $possible_trait) {
-  $attributes = $possible_trait->getChildByIndex(0);
-  // Can't use getChildByIndex here because not all classes have attributes
-  foreach ($attributes->getChildren() as $attribute) {
-    if (strtolower($attribute->getConcreteString()) === 'trait') {
-      phutil_fail_on_unsupported_feature($possible_trait, $path, pht('traits'));
-    }
-  }
-}
-
-
 // -(  Marked Externals  )------------------------------------------------------
 
 
@@ -256,17 +244,29 @@ foreach ($calls as $call) {
 
 // Find classes declared by this file.
 
-
 // This is "class X ... { ... }".
-$classes = $root->selectDescendantsOfType('n_CLASS_DECLARATION');
-foreach ($classes as $class) {
-  $class_name = $class->getChildByIndex(1);
-  $have[] = array(
-    'type'    => 'class',
+function build_have_element_for_class_declaration(XHPASTNode $class_node) {
+  $class_name = $class_node->getChildByIndex(1);
+
+  $type = 'class';
+  $attributes = $class_node->getChildByIndex(0);
+  foreach ($attributes->getChildren() as $attribute) {
+    if (strtolower($attribute->getConcreteString()) === 'trait') {
+      $type = 'trait';
+    }
+  }
+
+  return array(
+    'type'    => $type,
     'symbol'  => $class_name,
   );
 }
 
+$classes = $root->selectDescendantsOfType('n_CLASS_DECLARATION');
+foreach ($classes as $class) {
+  $have[] = build_have_element_for_class_declaration($class);
+}
+
 
 // Find classes used by this file. We identify these:
 //