1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-10 00:42:40 +01:00

(stable) Promote 2021 Week 4

This commit is contained in:
epriestley 2021-01-22 20:20:29 -08:00
commit 66d1acec70
22 changed files with 212 additions and 68 deletions

View file

@ -762,6 +762,7 @@ phutil_register_library_map(array(
'PhutilGitHubResponse' => 'future/github/PhutilGitHubResponse.php',
'PhutilGitURI' => 'parser/PhutilGitURI.php',
'PhutilGitURITestCase' => 'parser/__tests__/PhutilGitURITestCase.php',
'PhutilGitsprintfTestCase' => 'xsprintf/__tests__/PhutilGitsprintfTestCase.php',
'PhutilHTMLParser' => 'parser/html/PhutilHTMLParser.php',
'PhutilHTMLParserTestCase' => 'parser/html/__tests__/PhutilHTMLParserTestCase.php',
'PhutilHTTPEngineExtension' => 'future/http/PhutilHTTPEngineExtension.php',
@ -927,6 +928,7 @@ phutil_register_library_map(array(
'csprintf' => 'xsprintf/csprintf.php',
'exec_manual' => 'future/exec/execx.php',
'execx' => 'future/exec/execx.php',
'gitsprintf' => 'xsprintf/gitsprintf.php',
'head' => 'utils/utils.php',
'head_key' => 'utils/utils.php',
'hgsprintf' => 'xsprintf/hgsprintf.php',
@ -1043,6 +1045,7 @@ phutil_register_library_map(array(
'xsprintf' => 'xsprintf/xsprintf.php',
'xsprintf_callback_example' => 'xsprintf/xsprintf.php',
'xsprintf_command' => 'xsprintf/csprintf.php',
'xsprintf_git' => 'xsprintf/gitsprintf.php',
'xsprintf_javascript' => 'xsprintf/jsprintf.php',
'xsprintf_ldap' => 'xsprintf/ldapsprintf.php',
'xsprintf_mercurial' => 'xsprintf/hgsprintf.php',
@ -1825,6 +1828,7 @@ phutil_register_library_map(array(
'PhutilGitHubResponse' => 'Phobject',
'PhutilGitURI' => 'Phobject',
'PhutilGitURITestCase' => 'PhutilTestCase',
'PhutilGitsprintfTestCase' => 'PhutilTestCase',
'PhutilHTMLParser' => 'Phobject',
'PhutilHTMLParserTestCase' => 'PhutilTestCase',
'PhutilHTTPEngineExtension' => 'Phobject',

View file

@ -180,10 +180,10 @@ final class PhutilErrorHandler extends Phobject {
* @return void
* @task internal
*/
public static function handleError($num, $str, $file, $line, $ctx) {
public static function handleError($num, $str, $file, $line, $ctx = null) {
foreach (self::$traps as $trap) {
$trap->addError($num, $str, $file, $line, $ctx);
$trap->addError($num, $str, $file, $line);
}
if ((error_reporting() & $num) == 0) {
@ -214,7 +214,6 @@ final class PhutilErrorHandler extends Phobject {
array(
'file' => $file,
'line' => $line,
'context' => $ctx,
'error_code' => $num,
'trace' => $trace,
));

View file

@ -41,13 +41,12 @@ final class PhutilErrorTrap extends Phobject {
private $destroyed;
private $errors = array();
public function addError($num, $str, $file, $line, $ctx) {
public function addError($num, $str, $file, $line) {
$this->errors[] = array(
'num' => $num,
'str' => $str,
'file' => $file,
'line' => $line,
'ctx' => $ctx,
);
return $this;
}

View file

@ -268,7 +268,8 @@ final class PhutilOAuth1Future extends FutureProxy {
throw new Exception(pht('%s failed!', 'openssl_sign()'));
}
openssl_free_key($pkey);
// Deprecated in PHP 8; key is automatically freed.
@openssl_free_key($pkey);
return base64_encode($signature);
case 'PLAINTEXT':

View file

@ -274,9 +274,11 @@ final class ArcanistGitLandEngine
// as changes.
list($changes) = $api->execxLocal(
'diff --no-ext-diff %s..%s --',
$into_commit,
$max_hash);
'diff --no-ext-diff %s --',
gitsprintf(
'%s..%s',
$into_commit,
$max_hash));
$changes = trim($changes);
if (!strlen($changes)) {
@ -762,7 +764,7 @@ final class ArcanistGitLandEngine
$api = $this->getRepositoryAPI();
list($stdout) = $api->execxLocal(
'merge-base %s %s',
'merge-base -- %s %s',
$branch,
$commit);
$merge_base = trim($stdout);
@ -781,7 +783,7 @@ final class ArcanistGitLandEngine
list($info) = $api->execxLocal(
'log -n1 --format=%s %s --',
'%aD%n%an%n%ae',
$commit);
gitsprintf('%s', $commit));
$info = trim($info);
list($date, $author, $email) = explode("\n", $info, 3);
@ -1452,19 +1454,19 @@ final class ArcanistGitLandEngine
$commit_map = array();
foreach ($symbols as $symbol) {
$symbol_commit = $symbol->getCommit();
$format = '%H%x00%P%x00%s%x00';
$format = '--format=%H%x00%P%x00%s%x00';
if ($into_commit === null) {
list($commits) = $api->execxLocal(
'log %s --format=%s',
$symbol_commit,
$format);
'log %s %s --',
$format,
gitsprintf('%s', $symbol_commit));
} else {
list($commits) = $api->execxLocal(
'log %s --not %s --format=%s',
$symbol_commit,
$into_commit,
$format);
'log %s %s --not %s --',
$format,
gitsprintf('%s', $symbol_commit),
gitsprintf('%s', $into_commit));
}
$commits = phutil_split_lines($commits, false);

View file

@ -274,7 +274,7 @@ abstract class ArcanistLintEngine extends Phobject {
return ArcanistLintSeverity::isAtLeastAsSevere($severity, $minimum);
}
final private function shouldUseCache(
private function shouldUseCache(
$cache_granularity,
$repository_version) {
@ -313,7 +313,7 @@ abstract class ArcanistLintEngine extends Phobject {
return $this;
}
final private function isRelevantMessage(ArcanistLintMessage $message) {
private function isRelevantMessage(ArcanistLintMessage $message) {
// When a user runs "arc lint", we default to raising only warnings on
// lines they have changed (errors are still raised anywhere in the
// file). The list of $changed lines may be null, to indicate that the

View file

@ -291,7 +291,7 @@ abstract class ArcanistLinter extends Phobject {
* @param list<string>
* @return list<string>
*/
final private function filterPaths(array $paths) {
private function filterPaths(array $paths) {
$engine = $this->getEngine();
$keep = array();

View file

@ -35,7 +35,8 @@ final class ArcanistPyFlakesLinter extends ArcanistExternalLinter {
list($stdout) = execx('%C --version', $this->getExecutableCommand());
$matches = array();
if (preg_match('/^(?P<version>\d+\.\d+\.\d+)$/', $stdout, $matches)) {
$pattern = '/^(?P<version>\d+\.\d+\.\d+)( Python.*)?$/';
if (preg_match($pattern, $stdout, $matches)) {
return $matches['version'];
} else {
return false;
@ -52,7 +53,8 @@ final class ArcanistPyFlakesLinter extends ArcanistExternalLinter {
$messages = array();
foreach ($lines as $line) {
$matches = null;
if (!preg_match('/^(.*?):(\d+): (.*)$/', $line, $matches)) {
$pattern = '/^(?<path>.*?):(?<line>\d+):(?<column>\d*) (?<message>.*)$/';
if (!preg_match($pattern, $line, $matches)) {
continue;
}
foreach ($matches as $key => $match) {
@ -60,7 +62,7 @@ final class ArcanistPyFlakesLinter extends ArcanistExternalLinter {
}
$severity = ArcanistLintSeverity::SEVERITY_WARNING;
$description = $matches[3];
$description = $matches['message'];
$error_regexp = '/(^undefined|^duplicate|before assignment$)/';
if (preg_match($error_regexp, $description)) {
@ -69,7 +71,10 @@ final class ArcanistPyFlakesLinter extends ArcanistExternalLinter {
$message = new ArcanistLintMessage();
$message->setPath($path);
$message->setLine($matches[2]);
$message->setLine($matches['line']);
if ($matches['column'] != '') {
$message->setChar($matches['column']);
}
$message->setCode($this->getLinterName());
$message->setName($this->getLinterName());
$message->setDescription($description);

View file

@ -3,4 +3,4 @@ if (foo = 'bar') {
return true;
}
~~~~~~~~~~
warning:2:16:W084
warning:2:9:W084

View file

@ -2,6 +2,6 @@ import sys, os
x += 1
~~~~~~~~~~
warning:1:0
warning:1:0
error:3:0
warning:1:
warning:1:
error:3:

View file

@ -94,7 +94,7 @@ final class ArcanistFormattedStringXHPASTLinterRule
$argv = array($format->evalStatic()) + array_fill(0, $argc, null);
try {
xsprintf(null, null, $argv);
xsprintf(array(__CLASS__, 'processXsprintfCallback'), null, $argv);
} catch (BadFunctionCallException $ex) {
$this->raiseLintAtNode(
$call,
@ -105,4 +105,23 @@ final class ArcanistFormattedStringXHPASTLinterRule
}
}
public static function processXsprintfCallback(
$userdata,
&$pattern,
&$pos,
&$value,
&$length) {
if ($value !== null) {
throw new Exception('Expected dummy value to be null');
}
// Turn format "%$pattern" with argument null into format "%s" with
// argument "%$pattern". This ensures we always provide valid input for
// sprintf to avoid getting a ValueError when using custom format
// specifiers.
$value = '%'.$pattern[$pos];
$pattern[$pos] = 's';
}
}

View file

@ -75,6 +75,10 @@ final class PhutilTypeSpec extends Phobject {
}
break;
case 'regex':
if (!is_string($value)) {
throw new PhutilTypeCheckException($this, $value, $name);
}
$trap = new PhutilErrorTrap();
$ok = @preg_match($value, '');
$err = $trap->getErrorsAsString();

View file

@ -27,7 +27,7 @@ final class ArcanistGitCommitMessageHardpointQuery
$futures[$hash] = $api->execFutureLocal(
'log -n1 --format=%s %s --',
'%s%n%n%b',
$hash);
gitsprintf('%s', $hash));
}
yield $this->yieldFutures($futures);

View file

@ -88,7 +88,7 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
*/
private function isDescendant($child, $parent) {
list($common_ancestor) = $this->execxLocal(
'merge-base %s %s',
'merge-base -- %s %s',
$child,
$parent);
$common_ancestor = trim($common_ancestor);
@ -214,7 +214,7 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
}
list($err, $merge_base) = $this->execManualLocal(
'merge-base %s %s',
'merge-base -- %s %s',
$symbolic_commit,
$this->getHeadCommit());
if ($err) {
@ -381,7 +381,7 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
}
list($merge_base) = $this->execxLocal(
'merge-base %s HEAD',
'merge-base -- %s HEAD',
$default_relative);
return trim($merge_base);
@ -569,22 +569,24 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
} else if ($relative == self::GIT_MAGIC_ROOT_COMMIT) {
// First commit.
list($stdout) = $this->execxLocal(
'log --format=medium HEAD');
'log --format=medium HEAD --');
} else {
// 2..N commits.
list($stdout) = $this->execxLocal(
'log --first-parent --format=medium %s..%s',
$this->getBaseCommit(),
$this->getHeadCommit());
'log --first-parent --format=medium %s --',
gitsprintf(
'%s..%s',
$this->getBaseCommit(),
$this->getHeadCommit()));
}
return $stdout;
}
public function getGitHistoryLog() {
list($stdout) = $this->execxLocal(
'log --format=medium -n%d %s',
'log --format=medium -n%d %s --',
self::SEARCH_LENGTH_FOR_PARENT_REVISIONS,
$this->getBaseCommit());
gitsprintf('%s', $this->getBaseCommit()));
return $stdout;
}
@ -721,7 +723,7 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
array(
'diff %C --raw %s --',
$diff_options,
$diff_base,
gitsprintf('%s', $diff_base),
));
$untracked_future = $this->buildLocalFuture(
@ -782,7 +784,7 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
list($stdout, $stderr) = $this->execxLocal(
'diff %C --raw %s HEAD --',
$this->getDiffBaseOptions(),
$this->getBaseCommit());
gitsprintf('%s', $this->getBaseCommit()));
return $this->parseGitRawDiff($stdout);
}
@ -935,15 +937,15 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
public function getChangedFiles($since_commit) {
list($stdout) = $this->execxLocal(
'diff --raw %s',
$since_commit);
'diff --raw %s --',
gitsprintf('%s', $since_commit));
return $this->parseGitRawDiff($stdout);
}
public function getBlame($path) {
list($stdout) = $this->execxLocal(
'blame --porcelain -w -M %s -- %s',
$this->getBaseCommit(),
gitsprintf('%s', $this->getBaseCommit()),
$path);
// the --porcelain format prints at least one header line per source line,
@ -1030,7 +1032,7 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
list($stdout) = $this->execxLocal(
'ls-tree %s -- %s',
$revision,
gitsprintf('%s', $revision),
$path);
$info = $this->parseGitTree($stdout);
@ -1046,7 +1048,7 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
}
list($stdout) = $this->execxLocal(
'cat-file blob %s',
'cat-file blob -- %s',
$info[$path]['ref']);
return $stdout;
}
@ -1171,7 +1173,7 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
list($message) = $this->execxLocal(
'log -n1 --format=%C %s --',
'%s%n%n%b',
$commit);
gitsprintf('%s', $commit));
return $message;
}
@ -1249,9 +1251,9 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
}
list($summary) = $this->execxLocal(
'log -n 1 --format=%C %s',
'%s',
$commit);
'log -n 1 %s %s --',
'--format=%s',
gitsprintf('%s', $commit));
return trim($summary);
}
@ -1268,7 +1270,7 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
$matches = null;
if (preg_match('/^merge-base\((.+)\)$/', $name, $matches)) {
list($err, $merge_base) = $this->execManualLocal(
'merge-base %s HEAD',
'merge-base -- %s HEAD',
$matches[1]);
if (!$err) {
$this->setBaseCommitExplanation(
@ -1282,7 +1284,7 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
}
} else if (preg_match('/^branch-unique\((.+)\)$/', $name, $matches)) {
list($err, $merge_base) = $this->execManualLocal(
'merge-base %s HEAD',
'merge-base -- %s HEAD',
$matches[1]);
if ($err) {
return null;
@ -1400,7 +1402,7 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
if (!$err) {
$upstream = rtrim($upstream);
list($upstream_merge_base) = $this->execxLocal(
'merge-base %s HEAD',
'merge-base -- %s HEAD',
$upstream);
$upstream_merge_base = rtrim($upstream_merge_base);
$this->setBaseCommitExplanation(

View file

@ -84,7 +84,7 @@ final class ArcanistGitCommitGraphQuery
$format = implode('%x02', $fields).'%x01';
$future = $api->newFuture(
'log --format=%s %Ls --stdin',
'log --format=%s %Ls --stdin --',
$format,
$flags);
$future->write($ref_blob);

View file

@ -408,7 +408,7 @@ abstract class PhutilTestCase extends Phobject {
*
* @task internal
*/
final private function failTest($reason) {
private function failTest($reason) {
$this->resultTest(ArcanistUnitTestResult::RESULT_FAIL, $reason);
}
@ -421,7 +421,7 @@ abstract class PhutilTestCase extends Phobject {
*
* @task internal
*/
final private function passTest($reason) {
private function passTest($reason) {
$this->resultTest(ArcanistUnitTestResult::RESULT_PASS, $reason);
}
@ -433,12 +433,12 @@ abstract class PhutilTestCase extends Phobject {
* @return void
* @task internal
*/
final private function skipTest($reason) {
private function skipTest($reason) {
$this->resultTest(ArcanistUnitTestResult::RESULT_SKIP, $reason);
}
final private function resultTest($test_result, $reason) {
private function resultTest($test_result, $reason) {
$coverage = $this->endCoverage();
$result = new ArcanistUnitTestResult();
@ -553,7 +553,7 @@ abstract class PhutilTestCase extends Phobject {
/**
* @phutil-external-symbol function xdebug_start_code_coverage
*/
final private function beginCoverage() {
private function beginCoverage() {
if (!$this->enableCoverage) {
return;
}
@ -566,7 +566,7 @@ abstract class PhutilTestCase extends Phobject {
* @phutil-external-symbol function xdebug_get_code_coverage
* @phutil-external-symbol function xdebug_stop_code_coverage
*/
final private function endCoverage() {
private function endCoverage() {
if (!$this->enableCoverage) {
return;
}
@ -618,7 +618,7 @@ abstract class PhutilTestCase extends Phobject {
return $coverage;
}
final private function assertCoverageAvailable() {
private function assertCoverageAvailable() {
if (!function_exists('xdebug_start_code_coverage')) {
throw new Exception(
pht("You've enabled code coverage but XDebug is not installed."));
@ -675,7 +675,7 @@ abstract class PhutilTestCase extends Phobject {
*
* @return map
*/
final private static function getCallerInfo() {
private static function getCallerInfo() {
$callee = array();
$caller = array();
$seen = false;

View file

@ -318,7 +318,7 @@ abstract class AbstractDirectedGraph extends Phobject {
* which cycle.
* @task cycle
*/
final private function performCycleDetection($node, array $visited) {
private function performCycleDetection($node, array $visited) {
$visited[$node] = true;
foreach ($this->knownNodes[$node] as $edge) {
if (isset($visited[$edge])) {

View file

@ -498,6 +498,8 @@ final class PhutilUTF8TestCase extends PhutilTestCase {
phutil_utf8_convert('xyz', 'moon language', 'UTF-8');
} catch (Exception $ex) {
$caught = $ex;
} catch (Throwable $ex) {
$caught = $ex;
}
$this->assertTrue((bool)$caught, pht('Conversion with bogus encoding.'));

View file

@ -728,7 +728,7 @@ abstract class ArcanistWorkflow extends Phobject {
return $this->workingDirectory;
}
final private function setParentWorkflow($parent_workflow) {
private function setParentWorkflow($parent_workflow) {
$this->parentWorkflow = $parent_workflow;
return $this;
}
@ -1377,7 +1377,7 @@ abstract class ArcanistWorkflow extends Phobject {
));
}
final private function loadBundleFromConduit(
private function loadBundleFromConduit(
ConduitClient $conduit,
$params) {

View file

@ -0,0 +1,40 @@
<?php
final class PhutilGitsprintfTestCase extends PhutilTestCase {
public function testHgsprintf() {
$selectors = array(
'HEAD' => 'HEAD',
'master' => 'master',
'a..b' => 'a..b',
'feature^' => 'feature^',
'--flag' => false,
);
foreach ($selectors as $input => $expect) {
$caught = null;
try {
$output = gitsprintf('%s', $input);
} catch (Exception $ex) {
$caught = $ex;
} catch (Throwable $ex) {
$caught = $ex;
}
if ($caught !== null) {
$actual = false;
} else {
$actual = $output;
}
$this->assertEqual(
$expect,
$actual,
pht(
'Result for input "%s".',
$input));
}
}
}

View file

@ -0,0 +1,64 @@
<?php
/**
* Format a Git ref selector. This formatting is important when executing
* commands like "git log" which can not unambiguously parse all values as
* ref selectors.
*
* Supports the following conversions:
*
* %s Ref Selector
* Escapes a Git ref selector. In particular, this will reject ref selectors
* which Git may interpret as flags.
*
* %R Raw String
* Passes text through unescaped.
*/
function gitsprintf($pattern /* , ... */) {
$args = func_get_args();
return xsprintf('xsprintf_git', null, $args);
}
/**
* @{function:xsprintf} callback for Git encoding.
*/
function xsprintf_git($userdata, &$pattern, &$pos, &$value, &$length) {
$type = $pattern[$pos];
switch ($type) {
case 's':
// See T13589. Some Git commands accept both a ref selector and a list of
// paths. For example:
// $ git log <ref> -- <path> <path> ...
// These commands disambiguate ref selectors from paths using "--", but
// have no mechanism for disambiguating ref selectors from flags.
// Thus, there appears to be no way (in the general case) to safely
// invoke these commands with an arbitrary ref selector string: ref
// selector strings like "--flag" may be interpreted as flags, not as
// ref selectors.
// To resolve this, we reject any ref selector which begins with "-".
// These selectors are never valid anyway, so there is no loss of overall
// correctness. It would be more desirable to pass them to Git in a way
// that guarantees Git inteprets the string as a ref selector, but it
// appears that no mechanism exists to allow this.
if (preg_match('(^-)', $value)) {
throw new Exception(
pht(
'Git ref selector "%s" is not a valid selector and can not be '.
'passed to the Git CLI safely in the general case.',
$value));
}
break;
case 'R':
$type = 's';
break;
}
$pattern[$pos] = $type;
}

View file

@ -94,8 +94,11 @@ function __arcanist_init_script__() {
)));
// Disable the insanely dangerous XML entity loader by default.
// PHP 8 deprecates this function and disables this by default; remove once
// PHP 7 is no longer supported or a future version has removed the function
// entirely.
if (function_exists('libxml_disable_entity_loader')) {
libxml_disable_entity_loader(true);
@libxml_disable_entity_loader(true);
}
$root = dirname(dirname(dirname(__FILE__)));