diff --git a/src/filesystem/Filesystem.php b/src/filesystem/Filesystem.php index 981d4feb..dd6e984a 100644 --- a/src/filesystem/Filesystem.php +++ b/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; } diff --git a/src/future/http/status/HTTPFutureHTTPResponseStatus.php b/src/future/http/status/HTTPFutureHTTPResponseStatus.php index 469f9a0c..44636dca 100644 --- a/src/future/http/status/HTTPFutureHTTPResponseStatus.php +++ b/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); diff --git a/src/init/lib/PhutilMissingSymbolException.php b/src/init/lib/PhutilMissingSymbolException.php index 68926550..43434728 100644 --- a/src/init/lib/PhutilMissingSymbolException.php +++ b/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); diff --git a/src/moduleutils/PhutilLibraryMapBuilder.php b/src/moduleutils/PhutilLibraryMapBuilder.php index df05050a..5d359bbc 100644 --- a/src/moduleutils/PhutilLibraryMapBuilder.php +++ b/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( diff --git a/src/parser/ArcanistDiffParser.php b/src/parser/ArcanistDiffParser.php index 53a46a7a..f80917ed 100644 --- a/src/parser/ArcanistDiffParser.php +++ b/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!")); } diff --git a/src/runtime/ArcanistRuntime.php b/src/runtime/ArcanistRuntime.php index 8e3d220f..93f205cc 100644 --- a/src/runtime/ArcanistRuntime.php +++ b/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.')); diff --git a/src/symbols/PhutilSymbolLoader.php b/src/symbols/PhutilSymbolLoader.php index a5851a5e..7f733ee7 100644 --- a/src/symbols/PhutilSymbolLoader.php +++ b/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'); } } diff --git a/src/unit/engine/PhpunitTestEngine.php b/src/unit/engine/PhpunitTestEngine.php index 8206b787..4cb89006 100644 --- a/src/unit/engine/PhpunitTestEngine.php +++ b/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) diff --git a/src/unit/parser/ArcanistPhpunitTestResultParser.php b/src/unit/parser/ArcanistPhpunitTestResultParser.php index 5ccff970..0a33fa55 100644 --- a/src/unit/parser/ArcanistPhpunitTestResultParser.php +++ b/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); - } - } diff --git a/src/utils/PhutilSortVector.php b/src/utils/PhutilSortVector.php index 00146070..1bddd22e 100644 --- a/src/utils/PhutilSortVector.php +++ b/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.')); diff --git a/src/utils/utils.php b/src/utils/utils.php index 0b86cd59..7043d9b8 100644 --- a/src/utils/utils.php +++ b/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; diff --git a/src/workflow/ArcanistWorkflow.php b/src/workflow/ArcanistWorkflow.php index d0863772..3b8242cb 100644 --- a/src/workflow/ArcanistWorkflow.php +++ b/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); } diff --git a/src/workingcopyidentity/ArcanistWorkingCopyIdentity.php b/src/workingcopyidentity/ArcanistWorkingCopyIdentity.php index d2c86fd0..82a98cbb 100644 --- a/src/workingcopyidentity/ArcanistWorkingCopyIdentity.php +++ b/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(); diff --git a/support/lib/extract-symbols.php b/support/lib/extract-symbols.php index a1cd28fa..6744cd90 100755 --- a/support/lib/extract-symbols.php +++ b/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: //