diff --git a/src/lint/linter/__tests__/ArcanistLinterTestCase.php b/src/lint/linter/__tests__/ArcanistLinterTestCase.php index b86a43b2..a6dc69b5 100644 --- a/src/lint/linter/__tests__/ArcanistLinterTestCase.php +++ b/src/lint/linter/__tests__/ArcanistLinterTestCase.php @@ -1,262 +1,326 @@ getLinter(); $files = id(new FileFinder($root)) ->withType('f') ->withSuffix('lint-test') ->find(); $test_count = 0; foreach ($files as $file) { $this->lintFile($root.$file, $linter); $test_count++; } $this->assertTrue( ($test_count > 0), pht( 'Expected to find some %s tests in directory %s!', '.lint-test', $root)); } private function lintFile($file, ArcanistLinter $linter) { $linter = clone $linter; $contents = Filesystem::readFile($file); $contents = preg_split('/^~{4,}\n/m', $contents); if (count($contents) < 2) { throw new Exception( pht( "Expected '%s' separating test case and results.", '~~~~~~~~~~')); } list($data, $expect, $xform, $config) = array_merge( $contents, array(null, null)); $basename = basename($file); if ($config) { $config = phutil_json_decode($config); } else { $config = array(); } PhutilTypeSpec::checkMap( $config, array( 'config' => 'optional map', - 'path' => 'optional string', 'mode' => 'optional string', + 'path' => 'optional string', 'stopped' => 'optional bool', )); $exception = null; $after_lint = null; $messages = null; $exception_message = false; $caught_exception = false; try { $tmp = new TempFile($basename); Filesystem::writeFile($tmp, $data); $full_path = (string)$tmp; $mode = idx($config, 'mode'); if ($mode) { Filesystem::changePermissions($tmp, octdec($mode)); } $dir = dirname($full_path); $path = basename($full_path); $working_copy = ArcanistWorkingCopyIdentity::newFromRootAndConfigFile( $dir, null, pht('Unit Test')); $configuration_manager = new ArcanistConfigurationManager(); $configuration_manager->setWorkingCopyIdentity($working_copy); $engine = new ArcanistUnitTestableLintEngine(); $engine->setWorkingCopy($working_copy); $engine->setConfigurationManager($configuration_manager); $path_name = idx($config, 'path', $path); $engine->setPaths(array($path_name)); $linter->setEngine($engine); $linter->addPath($path_name); $linter->addData($path_name, $data); foreach (idx($config, 'config', array()) as $key => $value) { $linter->setLinterConfigurationValue($key, $value); } $engine->addLinter($linter); $engine->addFileData($path_name, $data); $results = $engine->run(); $this->assertEqual( 1, count($results), pht('Expect one result returned by linter.')); $assert_stopped = idx($config, 'stopped'); if ($assert_stopped !== null) { $this->assertEqual( $assert_stopped, $linter->didStopAllLinters(), $assert_stopped ? pht('Expect linter to be stopped.') : pht('Expect linter to not be stopped.')); } $result = reset($results); $patcher = ArcanistLintPatcher::newFromArcanistLintResult($result); $after_lint = $patcher->getModifiedFileContent(); } catch (PhutilTestTerminatedException $ex) { throw $ex; } catch (Exception $exception) { $caught_exception = true; if ($exception instanceof PhutilAggregateException) { $caught_exception = false; foreach ($exception->getExceptions() as $ex) { if ($ex instanceof ArcanistUsageException || $ex instanceof ArcanistMissingLinterException) { $this->assertSkipped($ex->getMessage()); } else { $caught_exception = true; } } } else if ($exception instanceof ArcanistUsageException || $exception instanceof ArcanistMissingLinterException) { $this->assertSkipped($exception->getMessage()); } $exception_message = $exception->getMessage()."\n\n". $exception->getTraceAsString(); } $this->assertEqual(false, $caught_exception, $exception_message); $this->compareLint($basename, $expect, $result); $this->compareTransform($xform, $after_lint); } - private function compareLint($file, $expect, ArcanistLintResult $result) { - $seen = array(); - $raised = array(); - $message_map = array(); - - foreach ($result->getMessages() as $message) { - $sev = $message->getSeverity(); - $line = $message->getLine(); - $char = $message->getChar(); - $code = $message->getCode(); - $name = $message->getName(); - $message_key = $sev.':'.$line.':'.$char; - $message_map[$message_key] = $message; - $seen[] = $message_key; - $raised[] = sprintf( - ' %s: %s %s', - pht('%s at line %d, char %d', $sev, $line, $char), - $code, - $name); - } + private function compareLint($file, $expect, ArcanistLintResult $results) { + $expected_results = new ArcanistLintResult(); + $expect = trim($expect); if ($expect) { $expect = explode("\n", $expect); } else { $expect = array(); } - foreach ($expect as $key => $expected) { - $expect[$key] = head(explode(' ', $expected)); - } - $expect = array_fill_keys($expect, true); - $seen = array_fill_keys($seen, true); + foreach ($expect as $result) { + $parts = explode(':', $result); + + $message = new ArcanistLintMessage(); + + $severity = idx($parts, 0); + $line = idx($parts, 1); + $char = idx($parts, 2); + $code = idx($parts, 3); + + if ($severity !== null) { + $message->setSeverity($severity); + } + + if ($line !== null) { + $message->setLine($line); + } + + if ($char !== null) { + $message->setChar($char); + } + + if ($code !== null) { + $message->setCode($code); + } - if (!$raised) { - $raised = array(pht('No messages.')); + $expected_results->addMessage($message); } - $raised = sprintf( - "%s:\n%s", - pht('Actually raised'), - implode("\n", $raised)); - foreach (array_diff_key($expect, $seen) as $missing => $ignored) { - $missing = explode(':', $missing); - $sev = array_shift($missing); - $pos = $missing; + $missing = array(); + $surprising = $results->getMessages(); - $this->assertFailure( - pht( - "In '%s', expected lint to raise %s on line %d at char %d, ". - "but no %s was raised. %s", - $file, - $sev, - idx($pos, 0), - idx($pos, 1), - $sev, - $raised)); + // TODO: Make this more efficient. + foreach ($expected_results->getMessages() as $expected_message) { + $found = false; + + foreach ($results->getMessages() as $ii => $actual_message) { + if (!self::compareLintMessageProperty( + $expected_message->getSeverity(), + $actual_message->getSeverity())) { + + continue; + } + + if (!self::compareLintMessageProperty( + $expected_message->getLine(), + $actual_message->getLine())) { + + continue; + } + + if (!self::compareLintMessageProperty( + $expected_message->getChar(), + $actual_message->getChar())) { + + continue; + } + + if (!self::compareLintMessageProperty( + $expected_message->getCode(), + $actual_message->getCode())) { + + continue; + } + + $found = true; + unset($surprising[$ii]); + } + + if (!$found) { + $missing[] = $expected_message; + } } - foreach (array_diff_key($seen, $expect) as $surprising => $ignored) { - $message = $message_map[$surprising]; - $message_info = $message->getDescription(); + if ($missing || $surprising) { + $expected = pht('EXPECTED MESSAGES'); + if ($missing) { + foreach ($missing as $message) { + $expected .= sprintf( + "\n %s: %s %s", + pht( + '%s at line %d, char %d', + $message->getSeverity(), + $message->getLine(), + $message->getChar()), + $message->getCode(), + $message->getName()); + } + } else { + $expected .= "\n ".pht('No messages'); + } + + $actual = pht('UNEXPECTED MESSAGES'); + if ($surprising) { + foreach ($surprising as $message) { + $actual .= sprintf( + "\n %s: %s %s", + pht( + '%s at line %d, char %d', + $message->getSeverity(), + $message->getLine(), + $message->getChar()), + $message->getCode(), + $message->getName()); + } + } else { + $actual .= "\n ".pht('No messages'); + } - list($sev, $line, $char) = explode(':', $surprising); $this->assertFailure( sprintf( - "%s:\n\n%s\n\n%s", - pht( - "In '%s', lint raised %s on line %d at char %d, ". - "but nothing was expected", - $file, - $sev, - $line, - $char), - $message_info, - $raised)); + "%s\n\n%s\n\n%s", + pht("Lint failed for '%s'.", $file), + $expected, + $actual)); } } private function compareTransform($expected, $actual) { if (!strlen($expected)) { return; } $this->assertEqual( $expected, $actual, pht('File as patched by lint did not match the expected patched file.')); } + /** + * Compare properties of @{class:ArcanistLintMessage} instances. + * + * The expectation is that if one (or both) of the properties is null, then + * we don't care about its value. + * + * @param wild + * @param wild + * @return bool + */ + private static function compareLintMessageProperty($x, $y) { + return $x === null || $y === null || $x === $y; + } + } diff --git a/src/lint/linter/__tests__/chmod/empty_executable.lint-test b/src/lint/linter/__tests__/chmod/empty_executable.lint-test index 54e5a85c..cf8980b6 100644 --- a/src/lint/linter/__tests__/chmod/empty_executable.lint-test +++ b/src/lint/linter/__tests__/chmod/empty_executable.lint-test @@ -1,5 +1,5 @@ ~~~~~~~~~~ -warning:: +warning:0:0:CHMOD1:Invalid Executable ~~~~~~~~~~ ~~~~~~~~~~ {"mode": "0755"} diff --git a/src/lint/linter/__tests__/coffeelint/no_trailing_whitespace.lint-test b/src/lint/linter/__tests__/coffeelint/no_trailing_whitespace.lint-test index 6b43ad21..87351e90 100644 --- a/src/lint/linter/__tests__/coffeelint/no_trailing_whitespace.lint-test +++ b/src/lint/linter/__tests__/coffeelint/no_trailing_whitespace.lint-test @@ -1,4 +1,4 @@ x = 1234 y = 1 ~~~~~~~~~~ -error:1: +error:1:0 diff --git a/src/lint/linter/__tests__/filename/bad_filename.lint-test b/src/lint/linter/__tests__/filename/bad_filename.lint-test index 7eb7c745..12ec5757 100644 --- a/src/lint/linter/__tests__/filename/bad_filename.lint-test +++ b/src/lint/linter/__tests__/filename/bad_filename.lint-test @@ -1,5 +1,5 @@ ~~~~~~~~~~ -error:: +error:0:0:NAME1:Bad Filename ~~~~~~~~~~ ~~~~~~~~~~ {"path": "bad@filename"} diff --git a/src/lint/linter/__tests__/flake8/undefined.lint-test b/src/lint/linter/__tests__/flake8/undefined.lint-test index 2f8a2548..72a09df0 100644 --- a/src/lint/linter/__tests__/flake8/undefined.lint-test +++ b/src/lint/linter/__tests__/flake8/undefined.lint-test @@ -1,7 +1,7 @@ x = 'y' def hello(): return foo ~~~~~~~~~~ -error:3:1 -error:4:12 +error:3:1:E302 +error:4:12:F821 diff --git a/src/lint/linter/__tests__/jshint/dot-notation.lint-test b/src/lint/linter/__tests__/jshint/dot-notation.lint-test index 681e3505..53cf0f91 100644 --- a/src/lint/linter/__tests__/jshint/dot-notation.lint-test +++ b/src/lint/linter/__tests__/jshint/dot-notation.lint-test @@ -1,6 +1,6 @@ var args = {}; args['foo'] = 'bar'; args['bar'] = 'baz'; ~~~~~~~~~~ -warning:2:5 -warning:3:5 +warning:2:5:W069 +warning:3:5:W069 diff --git a/src/lint/linter/__tests__/jshint/expected-conditional.lint-test b/src/lint/linter/__tests__/jshint/expected-conditional.lint-test index 2b229273..42b75257 100644 --- a/src/lint/linter/__tests__/jshint/expected-conditional.lint-test +++ b/src/lint/linter/__tests__/jshint/expected-conditional.lint-test @@ -1,6 +1,6 @@ var foo; if (foo = 'bar') { return true; } ~~~~~~~~~~ -warning:2:16 +warning:2:16:W084 diff --git a/src/lint/linter/__tests__/jshint/jshint.lint-test b/src/lint/linter/__tests__/jshint/jshint.lint-test index edb1c779..954e1033 100644 --- a/src/lint/linter/__tests__/jshint/jshint.lint-test +++ b/src/lint/linter/__tests__/jshint/jshint.lint-test @@ -1,12 +1,12 @@ function f() { for (ii = 0; ii < 3; ii++) { g() } } { ~~~~~~~~~~ -warning:3:8 -error:7:1 -error:9:1 +warning:3:8:W033 +error:7:1:E019 +error:9:1:E041 diff --git a/src/lint/linter/__tests__/jshint/missing-semicolon.lint-test b/src/lint/linter/__tests__/jshint/missing-semicolon.lint-test index a6669698..23ba765b 100644 --- a/src/lint/linter/__tests__/jshint/missing-semicolon.lint-test +++ b/src/lint/linter/__tests__/jshint/missing-semicolon.lint-test @@ -1,3 +1,3 @@ console.log('foobar') ~~~~~~~~~~ -warning:1:22 +warning:1:22:W033 diff --git a/src/lint/linter/__tests__/jshint/too-many-errors.lint-test b/src/lint/linter/__tests__/jshint/too-many-errors.lint-test index 6dbea1f9..e7b936dd 100644 --- a/src/lint/linter/__tests__/jshint/too-many-errors.lint-test +++ b/src/lint/linter/__tests__/jshint/too-many-errors.lint-test @@ -1,5 +1,5 @@ /* jshint maxerr: 1 */ console.log('foobar') ~~~~~~~~~~ -disabled:2:22 -warning:2:22 +disabled:2:22:E043 +warning:2:22:W033 diff --git a/src/lint/linter/__tests__/jshint/unnecessary-semicolon.lint-test b/src/lint/linter/__tests__/jshint/unnecessary-semicolon.lint-test index 14eafbdf..04c627ca 100644 --- a/src/lint/linter/__tests__/jshint/unnecessary-semicolon.lint-test +++ b/src/lint/linter/__tests__/jshint/unnecessary-semicolon.lint-test @@ -1,5 +1,5 @@ function main() { return 'Hello, World!'; }; ~~~~~~~~~~ -warning:3:2 +warning:3:2:W032 diff --git a/src/lint/linter/__tests__/mergeconflict/mergeconflict.lint-test b/src/lint/linter/__tests__/mergeconflict/mergeconflict.lint-test index 3e30be52..71b7d4aa 100644 --- a/src/lint/linter/__tests__/mergeconflict/mergeconflict.lint-test +++ b/src/lint/linter/__tests__/mergeconflict/mergeconflict.lint-test @@ -1,10 +1,10 @@ { "foo": "bar", <<<<<<< HEAD "bar": "baz" ======= "baz": "foo" >>>>>>> branch2 } ~~~~~~~~~~ -error:5:1 +error:5:1:MERGECONFLICT1:Unresolved merge conflict diff --git a/src/lint/linter/__tests__/pep8/imports.lint-test b/src/lint/linter/__tests__/pep8/imports.lint-test index cd9b2287..71f0ad21 100644 --- a/src/lint/linter/__tests__/pep8/imports.lint-test +++ b/src/lint/linter/__tests__/pep8/imports.lint-test @@ -1,3 +1,3 @@ import os, sys ~~~~~~~~~~ -error:1:10 +error:1:10:E401 diff --git a/src/lint/linter/__tests__/php/fatal.lint-test b/src/lint/linter/__tests__/php/fatal.lint-test index cf13b0cd..6d3c7b82 100644 --- a/src/lint/linter/__tests__/php/fatal.lint-test +++ b/src/lint/linter/__tests__/php/fatal.lint-test @@ -1,7 +1,7 @@ recieveData(); for (i=0; i This shouldn't fatal the parser. ~~~~~~~~~~ -disabled:2:1 +disabled:2:1:XHP78:Inline HTML diff --git a/src/lint/linter/__tests__/xhpast/empty.lint-test b/src/lint/linter/__tests__/xhpast/empty.lint-test index ddbbaebd..22cca1cd 100644 --- a/src/lint/linter/__tests__/xhpast/empty.lint-test +++ b/src/lint/linter/__tests__/xhpast/empty.lint-test @@ -1,4 +1,4 @@ ~~~~~~~~~~ -error:2:1 +error:2:1:XML4:LibXML Error diff --git a/src/lint/linter/__tests__/xml/char-ref.lint-test b/src/lint/linter/__tests__/xml/char-ref.lint-test index a142bbe4..6f4cca9a 100644 --- a/src/lint/linter/__tests__/xml/char-ref.lint-test +++ b/src/lint/linter/__tests__/xml/char-ref.lint-test @@ -1,3 +1,3 @@ ~~~~~~~~~~ -error:1:63 +error:1:63:XML9:LibXML Error diff --git a/src/lint/linter/__tests__/xml/languages-5.lint-test b/src/lint/linter/__tests__/xml/languages-5.lint-test index cf469965..508b4f1a 100644 --- a/src/lint/linter/__tests__/xml/languages-5.lint-test +++ b/src/lint/linter/__tests__/xml/languages-5.lint-test @@ -1,6 +1,6 @@ <> ~~~~~~~~~~ -error:3:6 +error:3:6:XML68:LibXML Error diff --git a/src/lint/linter/__tests__/xml/languages-6.lint-test b/src/lint/linter/__tests__/xml/languages-6.lint-test index 1cdb9519..f652fbb8 100644 --- a/src/lint/linter/__tests__/xml/languages-6.lint-test +++ b/src/lint/linter/__tests__/xml/languages-6.lint-test @@ -1,7 +1,7 @@ ~~~~~~~~~~ -error:3:16 -error:4:1 +error:3:16:XML76:LibXML Error +error:4:1:XML5:LibXML Error diff --git a/src/lint/linter/xhpast/rules/__tests__/__lambda_func-function/lamba-func-function.lint-test b/src/lint/linter/xhpast/rules/__tests__/__lambda_func-function/lamba-func-function.lint-test index 554a4034..a65ee01f 100644 --- a/src/lint/linter/xhpast/rules/__tests__/__lambda_func-function/lamba-func-function.lint-test +++ b/src/lint/linter/xhpast/rules/__tests__/__lambda_func-function/lamba-func-function.lint-test @@ -1,5 +1,5 @@ << << 'bar', 'bar' => 'baz', ); array( 1, /* quack */ 2, /* quack */3, ); array( /* OPEN */ 1, /* CLOSED */ 2, ); array('quack', ); ~~~~~~~~~~ -warning:4:5 -warning:4:8 -warning:8:18 -warning:12:17 -warning:12:32 -warning:20:7 +warning:4:5:XHP76:Array Element +warning:4:8:XHP76:Array Element +warning:8:18:XHP76:Array Element +warning:12:17:XHP76:Array Element +warning:12:32:XHP76:Array Element +warning:20:7:XHP76:Array Element ~~~~~~~~~~ 'bar', 'bar' => 'baz', ); array( 1, /* quack */ 2, /* quack */ 3, ); array( /* OPEN */ 1, /* CLOSED */ 2, ); array( 'quack', ); diff --git a/src/lint/linter/xhpast/rules/__tests__/binary-expression-spacing/array.lint-test b/src/lint/linter/xhpast/rules/__tests__/binary-expression-spacing/array.lint-test index 8e0857c9..46fc5b8a 100644 --- a/src/lint/linter/xhpast/rules/__tests__/binary-expression-spacing/array.lint-test +++ b/src/lint/linter/xhpast/rules/__tests__/binary-expression-spacing/array.lint-test @@ -1,40 +1,40 @@ $y); array($x=>$y); array($x =>$y); array($x=> $y); array( $x => $y, ); array( $x => $y, ); array( $x=>$y, ); ~~~~~~~~~~ -warning:4:9 -warning:5:10 -warning:6:9 -warning:16:5 +warning:4:9:XHP27:Space Around Binary Operator +warning:5:10:XHP27:Space Around Binary Operator +warning:6:9:XHP27:Space Around Binary Operator +warning:16:5:XHP27:Space Around Binary Operator ~~~~~~~~~~ $y); array($x => $y); array($x => $y); array($x => $y); array( $x => $y, ); array( $x => $y, ); array( $x => $y, ); diff --git a/src/lint/linter/xhpast/rules/__tests__/binary-expression-spacing/binary-expression-spacing.lint-test b/src/lint/linter/xhpast/rules/__tests__/binary-expression-spacing/binary-expression-spacing.lint-test index 571a5648..3caaab1c 100644 --- a/src/lint/linter/xhpast/rules/__tests__/binary-expression-spacing/binary-expression-spacing.lint-test +++ b/src/lint/linter/xhpast/rules/__tests__/binary-expression-spacing/binary-expression-spacing.lint-test @@ -1,65 +1,65 @@ method(); Dog::bark(); $prev = ($total == 1) ? $navids[0] : $navids[$total-1]; $next = ($total == 1) ? $navids[0] : $navids[$current+1]; if ($x instanceof z &&$w) {} if ($x instanceof z && $w) {} f(1,2); ~~~~~~~~~~ -warning:4:3 -warning:5:4 -warning:6:3 -warning:8:3 -warning:9:3 -warning:10:4 -warning:11:3 -warning:12:3 -warning:14:14 -warning:21:52 -warning:22:54 -warning:23:21 -warning:25:4 +warning:4:3:XHP27:Space Around Binary Operator +warning:5:4:XHP27:Space Around Binary Operator +warning:6:3:XHP27:Space Around Binary Operator +warning:8:3:XHP27:Space Around Binary Operator +warning:9:3:XHP27:Space Around Binary Operator +warning:10:4:XHP27:Space Around Binary Operator +warning:11:3:XHP27:Space Around Binary Operator +warning:12:3:XHP27:Space Around Binary Operator +warning:14:14:XHP27:Space Around Binary Operator +warning:21:52:XHP27:Space Around Binary Operator +warning:22:54:XHP27:Space Around Binary Operator +warning:23:21:XHP27:Space Around Binary Operator +warning:25:4:XHP27:Space Around Binary Operator ~~~~~~~~~~ method(); Dog::bark(); $prev = ($total == 1) ? $navids[0] : $navids[$total - 1]; $next = ($total == 1) ? $navids[0] : $navids[$current + 1]; if ($x instanceof z && $w) {} if ($x instanceof z && $w) {} f(1, 2); diff --git a/src/lint/linter/xhpast/rules/__tests__/binary-numeric-scalar-casing/binary.lint-test b/src/lint/linter/xhpast/rules/__tests__/binary-numeric-scalar-casing/binary.lint-test index 406fe97c..8297e01f 100644 --- a/src/lint/linter/xhpast/rules/__tests__/binary-numeric-scalar-casing/binary.lint-test +++ b/src/lint/linter/xhpast/rules/__tests__/binary-numeric-scalar-casing/binary.lint-test @@ -1,5 +1,5 @@ myfunc(&$myvar); $this->myfunc($myvar); MyClass::myfunc(&$myvar); MyClass::myfunc($myvar); while (testfunc($var1, &$var2, $var3, &$var4) === false) {} sprintf('0%o', 0777 & $p); $foo(&$myvar); array_walk(array(), function () use (&$x) {}); MyClass::myfunc(array(&$x, &$y)); ~~~~~~~~~~ -error:10:8 -error:13:15 -error:16:17 -error:19:24 -error:19:39 -error:23:6 +error:10:8:XHP53:Call-Time Pass-By-Reference +error:13:15:XHP53:Call-Time Pass-By-Reference +error:16:17:XHP53:Call-Time Pass-By-Reference +error:19:24:XHP53:Call-Time Pass-By-Reference +error:19:39:XHP53:Call-Time Pass-By-Reference +error:23:6:XHP53:Call-Time Pass-By-Reference diff --git a/src/lint/linter/xhpast/rules/__tests__/cast-spacing/cast-spacing.lint-test b/src/lint/linter/xhpast/rules/__tests__/cast-spacing/cast-spacing.lint-test index bc70f302..0662c526 100644 --- a/src/lint/linter/xhpast/rules/__tests__/cast-spacing/cast-spacing.lint-test +++ b/src/lint/linter/xhpast/rules/__tests__/cast-spacing/cast-spacing.lint-test @@ -1,14 +1,14 @@ 'val1', $a => 'val2', 'a' => val3(), 1 => val4(), '1' => val5(), 1 => $val6, 'b' => 'val7', b() => 'val8', ); static $b = array( 1 => 'one', 2 => 'two', 2 => 'TWO', ); $c = array(); $d = array( A => 'ay', A => 'aye', 'A' => 'AYE', ); $e = array( self::B => 'bee', self::B => 'bea', self::b() => 'ehh', self::$b => 'weh', B => 'b', C::B => 'cb', ); $f = array( $a => 'var', 'a' => 'lit', $a => 'var2', ); ~~~~~~~~~~ -error:6:3 -error:9:3 -error:16:3 -error:21:3 -error:26:3 -error:35:3 +error:6:3:XHP22:Duplicate Keys in Array +error:9:3:XHP22:Duplicate Keys in Array +error:16:3:XHP22:Duplicate Keys in Array +error:21:3:XHP22:Duplicate Keys in Array +error:26:3:XHP22:Duplicate Keys in Array +error:35:3:XHP22:Duplicate Keys in Array diff --git a/src/lint/linter/xhpast/rules/__tests__/duplicate-switch-case/duplicate-switch-case.lint-test b/src/lint/linter/xhpast/rules/__tests__/duplicate-switch-case/duplicate-switch-case.lint-test index 2c8e5c5c..2d95b899 100644 --- a/src/lint/linter/xhpast/rules/__tests__/duplicate-switch-case/duplicate-switch-case.lint-test +++ b/src/lint/linter/xhpast/rules/__tests__/duplicate-switch-case/duplicate-switch-case.lint-test @@ -1,29 +1,29 @@ ~~~~~~~~~~ -disabled:2:1 +disabled:2:1:XHP78:Inline HTML diff --git a/src/lint/linter/xhpast/rules/__tests__/inner-function/inner-function.lint-test b/src/lint/linter/xhpast/rules/__tests__/inner-function/inner-function.lint-test index 6294eb0a..d4f73fc1 100644 --- a/src/lint/linter/xhpast/rules/__tests__/inner-function/inner-function.lint-test +++ b/src/lint/linter/xhpast/rules/__tests__/inner-function/inner-function.lint-test @@ -1,14 +1,14 @@ doSomething(); id(new Something()) ->doSomething(); ~~~~~~~~~~ -warning:3:3 -warning:3:6 +warning:3:3:XHP74:Object Operator Spacing +warning:3:6:XHP74:Object Operator Spacing ~~~~~~~~~~ doSomething(); id(new Something()) ->doSomething(); diff --git a/src/lint/linter/xhpast/rules/__tests__/paamayim-nekudotayim-spacing/paamayim-nekudotayim-spacing.lint-test b/src/lint/linter/xhpast/rules/__tests__/paamayim-nekudotayim-spacing/paamayim-nekudotayim-spacing.lint-test index 7ee65d7b..67a6c11d 100644 --- a/src/lint/linter/xhpast/rules/__tests__/paamayim-nekudotayim-spacing/paamayim-nekudotayim-spacing.lint-test +++ b/src/lint/linter/xhpast/rules/__tests__/paamayim-nekudotayim-spacing/paamayim-nekudotayim-spacing.lint-test @@ -1,36 +1,36 @@ m( $x, $y, $z); for ( $ii = 0; $ii < 1; $ii++ ) {} foreach ( $x as $y ) {} function q( $x ) {} final class X { public function f( $y ) {} } foreach ( $z as $k => $v ) {} some_call( /* respect authorial intent */ $b, $a // if comments are present ); ~~~~~~~~~~ -warning:3:5 -warning:3:8 -warning:4:3 -warning:5:3 -warning:7:21 -warning:7:24 -warning:13:6 -warning:13:30 -warning:14:10 -warning:14:19 -warning:15:12 -warning:15:15 -warning:17:21 -warning:17:24 -warning:19:10 -warning:19:25 +warning:3:5:XHP25:Spaces Inside Parentheses +warning:3:8:XHP25:Spaces Inside Parentheses +warning:4:3:XHP25:Spaces Inside Parentheses +warning:5:3:XHP25:Spaces Inside Parentheses +warning:7:21:XHP25:Spaces Inside Parentheses +warning:7:24:XHP25:Spaces Inside Parentheses +warning:13:6:XHP25:Spaces Inside Parentheses +warning:13:30:XHP25:Spaces Inside Parentheses +warning:14:10:XHP25:Spaces Inside Parentheses +warning:14:19:XHP25:Spaces Inside Parentheses +warning:15:12:XHP25:Spaces Inside Parentheses +warning:15:15:XHP25:Spaces Inside Parentheses +warning:17:21:XHP25:Spaces Inside Parentheses +warning:17:24:XHP25:Spaces Inside Parentheses +warning:19:10:XHP25:Spaces Inside Parentheses +warning:19:25:XHP25:Spaces Inside Parentheses ~~~~~~~~~~ m( $x, $y, $z); for ($ii = 0; $ii < 1; $ii++) {} foreach ($x as $y) {} function q($x) {} final class X { public function f($y) {} } foreach ($z as $k => $v) {} some_call( /* respect authorial intent */ $b, $a // if comments are present ); diff --git a/src/lint/linter/xhpast/rules/__tests__/parse_str-use/parse_str-use.lint-test b/src/lint/linter/xhpast/rules/__tests__/parse_str-use/parse_str-use.lint-test index 21afe525..0e7541d6 100644 --- a/src/lint/linter/xhpast/rules/__tests__/parse_str-use/parse_str-use.lint-test +++ b/src/lint/linter/xhpast/rules/__tests__/parse_str-use/parse_str-use.lint-test @@ -1,7 +1,7 @@ ~~~~~~~~~~ -error:3:1 +error:3:1:XHP8:Use of Close Tag `?>` diff --git a/src/lint/linter/xhpast/rules/__tests__/php-compatibility/conditional-usage.lint-test b/src/lint/linter/xhpast/rules/__tests__/php-compatibility/conditional-usage.lint-test index 6b8a50de..ba34f703 100644 --- a/src/lint/linter/xhpast/rules/__tests__/php-compatibility/conditional-usage.lint-test +++ b/src/lint/linter/xhpast/rules/__tests__/php-compatibility/conditional-usage.lint-test @@ -1,32 +1,32 @@ m()[0]; final class SomeClass extends Phobject { public function someMethod() { return function () { $this->someOtherMethod(); }; } public static function someStaticMethod() { return function () { self::someOtherMethod(); }; } } 0b1; []; ~~~~~~~~~~ -error:3:5 -error:4:9 -error:12:7 -error:18:7 -error:23:1 -error:25:1 +error:3:5:XHP45:PHP Compatibility +error:4:9:XHP45:PHP Compatibility +error:12:7:XHP45:PHP Compatibility +error:18:7:XHP45:PHP Compatibility +error:23:1:XHP45:PHP Compatibility +error:25:1:XHP45:PHP Compatibility ~~~~~~~~~~ ~~~~~~~~~~ { "config": { "xhpast.php-version": "5.3.0" } } diff --git a/src/lint/linter/xhpast/rules/__tests__/php-compatibility/php54-incompat.lint-test b/src/lint/linter/xhpast/rules/__tests__/php-compatibility/php54-incompat.lint-test index fb6e3ac6..208adbfa 100644 --- a/src/lint/linter/xhpast/rules/__tests__/php-compatibility/php54-incompat.lint-test +++ b/src/lint/linter/xhpast/rules/__tests__/php-compatibility/php54-incompat.lint-test @@ -1,18 +1,18 @@ ~~~~~~~~~~ -error:1:1 +error:1:1:XHP15:Expected Open Tag ~~~~~~~~~~ garbage garbage diff --git a/src/lint/linter/xhpast/rules/__tests__/php-short-tag/php-short-tag.lint-test b/src/lint/linter/xhpast/rules/__tests__/php-short-tag/php-short-tag.lint-test index 9c5fb0c5..1e756cda 100644 --- a/src/lint/linter/xhpast/rules/__tests__/php-short-tag/php-short-tag.lint-test +++ b/src/lint/linter/xhpast/rules/__tests__/php-short-tag/php-short-tag.lint-test @@ -1,6 +1,6 @@ $val) {} $key = 1; $thing = 1; $thing = $key; $key = $thing; } function i($stuff, $thing) { foreach ($stuff as $thing) { $thing++; } // OK: Used afterwards but inside loop. foreach ($stuff as $thing) { $thing++; } } function j($stuff, $thing) { foreach ($stuff as $thing) { break; } // ERROR: Clobbers $thing; probably not what the author intended. f($thing); } function k($stuff, $thing) { foreach ($stuff as $thing) { break; } // ERROR: Clobbers $thing. Test case to cover some errors of implementation // where subsequent legitimate foreach()es threw a wrench in the gears. f($thing); $other = array(); foreach ($other as $item) {} } ~~~~~~~~~~ -error:43:22 -error:53:22 +error:43:22:XHP32:Variable Reused As Iterator +error:53:22:XHP32:Variable Reused As Iterator diff --git a/src/lint/linter/xhpast/rules/__tests__/reused-iterator-reference/reused-iterator-reference.lint-test b/src/lint/linter/xhpast/rules/__tests__/reused-iterator-reference/reused-iterator-reference.lint-test index 0bc4c345..be42a429 100644 --- a/src/lint/linter/xhpast/rules/__tests__/reused-iterator-reference/reused-iterator-reference.lint-test +++ b/src/lint/linter/xhpast/rules/__tests__/reused-iterator-reference/reused-iterator-reference.lint-test @@ -1,130 +1,130 @@ &$v) {} $v++; // Reuse of $v } function key_value2() { $ar = array(); foreach ($ar as $k => $v) {} $v++; } function unset() { $ar = array(); foreach ($ar as &$a) {} unset($a); $a++; } function unset2() { $ar = array(); foreach ($ar as &$a) {} $a++; // Reuse of $a unset($a); } function twice_ref() { $ar1 = array(); $ar2 = array(); foreach ($ar1 as &$b) {} foreach ($ar2 as &$b) {} } function assign_ref(&$a) { $ar = array(); foreach ($ar as &$b) {} $b = &$a; } function assign_ref2(&$a) { $ar = array(); foreach ($ar as &$b) {} $b = &$a; $c = $b; } function use_inside() { $ar = array(); foreach ($ar as &$a) { $a++; } } function variable_variable() { $ar = array(); foreach ($ar as &$$a) {} $a++; $$a++; } function closure1() { $ar = array(); foreach ($ar as &$a) {} function ($a) { $a++; }; } function closure2() { function () { $ar = array(); foreach ($ar as &$a) {} }; $a++; } function closure3() { function () { $ar = array(); foreach ($ar as &$a) {} $a++; // Reuse of $a }; } function closure4() { $ar = array(); foreach ($ar as &$a) {} function ($a) { unset($a); }; $a++; // Reuse of $a } ~~~~~~~~~~ -warning:6:3 -warning:12:8 -warning:18:10 -warning:27:20 -warning:33:3 -warning:52:3 -warning:110:5 -warning:120:3 +warning:6:3:XHP39:Reuse of Iterator References +warning:12:8:XHP39:Reuse of Iterator References +warning:18:10:XHP39:Reuse of Iterator References +warning:27:20:XHP39:Reuse of Iterator References +warning:33:3:XHP39:Reuse of Iterator References +warning:52:3:XHP39:Reuse of Iterator References +warning:110:5:XHP39:Reuse of Iterator References +warning:120:3:XHP39:Reuse of Iterator References diff --git a/src/lint/linter/xhpast/rules/__tests__/reused-iterator/reused-iterator.lint-test b/src/lint/linter/xhpast/rules/__tests__/reused-iterator/reused-iterator.lint-test index 29d31b72..bfe122ce 100644 --- a/src/lint/linter/xhpast/rules/__tests__/reused-iterator/reused-iterator.lint-test +++ b/src/lint/linter/xhpast/rules/__tests__/reused-iterator/reused-iterator.lint-test @@ -1,58 +1,58 @@ $jj) { // Reuse of $ii. } } for ($jj = 0; $jj < $len; $jj++) { foreach ($list as $jj) { // Reuse of $jj. } } for ($ii = 0; $ii < $len; $ii++) { foreach ($list as $ii => &$jj) { // Reuse of $ii. } } for ($jj = 0; $jj < $len; $jj++) { foreach ($list as &$jj) { // Reuse of $jj (by reference). } } for ($ii = 0; $ii < $len; $ii++) { for ($jj = 0; $jj < $len; $jj++) { foreach ($list as $kk) { // No reuse. } } } for ($ii = 0; $ii < $len; $ii++) { for ($ii = 0; $ii < $len; $ii++) { // Reuse of $ii, pure for loops. } } for ($ii = 0; $ii < $len; $ii++) { for ($jj = $ii; $jj < $jjlen; $jj++) { // No reuse. } } foreach ($list as $thing) { for ($thing = 0; $thing < $len; $thing++) { // Reuse of $thing, for within foreach. } } ~~~~~~~~~~ -error:4:11 -error:10:11 -error:16:11 -error:22:11 -error:36:7 -error:48:7 +error:4:11:XHP23:Reuse of Iterator Variable +error:10:11:XHP23:Reuse of Iterator Variable +error:16:11:XHP23:Reuse of Iterator Variable +error:22:11:XHP23:Reuse of Iterator Variable +error:36:7:XHP23:Reuse of Iterator Variable +error:48:7:XHP23:Reuse of Iterator Variable diff --git a/src/lint/linter/xhpast/rules/__tests__/self-class-reference/self-class-references.lint-test b/src/lint/linter/xhpast/rules/__tests__/self-class-reference/self-class-references.lint-test index 33c30f4d..8cf29ac3 100644 --- a/src/lint/linter/xhpast/rules/__tests__/self-class-reference/self-class-references.lint-test +++ b/src/lint/linter/xhpast/rules/__tests__/self-class-reference/self-class-references.lint-test @@ -1,37 +1,37 @@ f(); } public static function v() { $this->f(); } } ~~~~~~~~~~ -error:8:5 +error:8:5:XHP13:Use of `$this` in Static Context diff --git a/src/lint/linter/xhpast/rules/__tests__/tautological-expression/tautological-expression.lint-test b/src/lint/linter/xhpast/rules/__tests__/tautological-expression/tautological-expression.lint-test index fa1a7d8e..c9200a3a 100644 --- a/src/lint/linter/xhpast/rules/__tests__/tautological-expression/tautological-expression.lint-test +++ b/src/lint/linter/xhpast/rules/__tests__/tautological-expression/tautological-expression.lint-test @@ -1,23 +1,23 @@ m(3) < $x->m(3)) {} if ($y[2] - $y[2]) {} if ($x == $y) {} // See xhpast 0.54 -> 0.55. return $a->sub->value < $b->sub->value; $skip_cache = true || $some_complicated_expression; $skip_cache = $a || $b; $skip_cache = false && something(); $skip_cache = f(); ~~~~~~~~~~ -error:3:5 -error:5:5 -error:7:5 -error:14:15 -error:16:15 +error:3:5:XHP20:Tautological Expression +error:5:5:XHP20:Tautological Expression +error:7:5:XHP20:Tautological Expression +error:14:15:XHP20:Tautological Expression +error:16:15:XHP20:Tautological Expression diff --git a/src/lint/linter/xhpast/rules/__tests__/this-reassignment/class.lint-test b/src/lint/linter/xhpast/rules/__tests__/this-reassignment/class.lint-test index e4081b0d..a6c96f18 100644 --- a/src/lint/linter/xhpast/rules/__tests__/this-reassignment/class.lint-test +++ b/src/lint/linter/xhpast/rules/__tests__/this-reassignment/class.lint-test @@ -1,8 +1,8 @@ $m) {} $a++; $b++; $c++; $d++; $e++; $f++; $g++; $h++; $i++; $j++; $k++; $l++; $m++; $this++; $n++; // Only one that isn't declared. extract(z()); $o++; } function g($q) { $$q = x(); $r = y(); } final class C { public function m() { $a++; x($b); $c[] = 3; $d->v = 4; $a = $f; } } function worst() { global $$x; $y++; } function superglobals() { $GLOBALS[$_FILES[$_POST[$this]]]++; } function ref_foreach($x) { foreach ($x as &$z) {} $z++; } function has_default($x = 0) { $x++; } function declparse( $a, Q $b, Q &$c, Q $d = null, Q &$e = null, $f = null, $g = null, &$h = null, &$i = null) { $a++; $b++; $c++; $d++; $e++; $f++; $g++; $h++; $i++; $j++; } function declparse_a(Q $a) { $a++; } function declparse_b(Q &$a) { $a++; } function declparse_c(Q $a = null) { $a++; } function declparse_d(Q &$a = null) { $a++; } function declparse_e($a) { $a++; } function declparse_f(&$a) { $a++; } function declparse_g($a = null) { $a++; } function declparse_h(&$a = null) { $a++; } function static_class() { SomeClass::$x; } function instance_class() { $a = $this->$x; } function exception_vars() { try { // This is intentionally left blank. } catch (Exception $y) { $y++; } } function nonuse() { isset($x); empty($y); $x++; } function twice() { $y++; $y++; } function more_exceptions() { try { // This is intentionally left blank. } catch (Exception $a) { $a++; } catch (Exception $b) { $b++; } } abstract class P { abstract public function q(); } function x() { $lib = $_SERVER['PHP_ROOT'].'/lib/titan/display/read/init.php'; require_once $lib; f(((($lib)))); // Tests for paren expressions. f(((($lub)))); } final class A { public function foo($a) { $im_service = foo($a); if ($im_servce === false) { return 1; } return 2; } } function arrow($o, $x) { echo $o->{$x->{$x->{$x.$x->{$x}}.$x}}; } function strings() { $a = 1; echo "$a"; echo "$b"; } function catchy() { try { dangerous(); } catch (Exception $ex) { $y->z(); } } function some_func($x, $y) { $func = function ($z) use ($x) { echo $x; echo $y; echo $z; }; } function some_func($x, $y) { $func = function ($z) use ($x) { echo "$x/$y/$z"; }; } ~~~~~~~~~~ -error:28:3 -error:42:5 -error:43:7 -error:44:5 -error:45:5 -error:46:10 -error:87:3 This stuff is basically testing the lexer/parser for function decls. -error:104:15 Variables in instance derefs should be checked, static should not. -error:118:3 isset() and empty() should not trigger errors. -error:122:3 Should only warn once in this function. -error:144:8 -error:150:9 -error:164:9 -error:171:5 -error:178:10 -error:185:14 +error:28:3:XHP5:Use of Undeclared Variable +error:42:5:XHP5:Use of Undeclared Variable +error:43:7:XHP5:Use of Undeclared Variable +error:44:5:XHP5:Use of Undeclared Variable +error:45:5:XHP5:Use of Undeclared Variable +error:46:10:XHP5:Use of Undeclared Variable +error:87:3:XHP5:Use of Undeclared Variable +error:104:15:XHP5:Use of Undeclared Variable +error:118:3:XHP5:Use of Undeclared Variable +error:122:3:XHP5:Use of Undeclared Variable +error:144:8:XHP5:Use of Undeclared Variable +error:150:9:XHP5:Use of Undeclared Variable +error:164:9:XHP5:Use of Undeclared Variable +error:171:5:XHP5:Use of Undeclared Variable +error:178:10:XHP5:Use of Undeclared Variable +error:185:14:XHP5:Use of Undeclared Variable diff --git a/src/lint/linter/xhpast/rules/__tests__/unexpected-return-value/construct.lint-test b/src/lint/linter/xhpast/rules/__tests__/unexpected-return-value/construct.lint-test index 894c3f6b..e1173e7d 100644 --- a/src/lint/linter/xhpast/rules/__tests__/unexpected-return-value/construct.lint-test +++ b/src/lint/linter/xhpast/rules/__tests__/unexpected-return-value/construct.lint-test @@ -1,8 +1,8 @@ $bar; // okay ~~~~~~~~~~ -error:3:1 +error:3:1:XHP3:Use of Variable diff --git a/src/workflow/ArcanistLandWorkflow.php b/src/workflow/ArcanistLandWorkflow.php index 7ca0028e..28edf0f0 100644 --- a/src/workflow/ArcanistLandWorkflow.php +++ b/src/workflow/ArcanistLandWorkflow.php @@ -1,1681 +1,1708 @@ revision; } public function getWorkflowName() { return 'land'; } public function getCommandSynopses() { return phutil_console_format(<< array( 'param' => 'master', 'help' => pht( "Land feature branch onto a branch other than the default ". "('master' in git, 'default' in hg). You can change the default ". "by setting '%s' with `%s` or for the entire project in %s.", 'arc.land.onto.default', 'arc set-config', '.arcconfig'), ), 'hold' => array( 'help' => pht( 'Prepare the change to be pushed, but do not actually push it.'), ), 'keep-branch' => array( 'help' => pht( 'Keep the feature branch after pushing changes to the '. 'remote (by default, it is deleted).'), ), 'remote' => array( 'param' => 'origin', 'help' => pht( "Push to a remote other than the default ('origin' in git)."), ), 'merge' => array( 'help' => pht( 'Perform a %s merge, not a %s merge. If the project '. 'is marked as having an immutable history, this is the default '. 'behavior.', '--no-ff', '--squash'), 'supports' => array( 'git', ), 'nosupport' => array( 'hg' => pht( 'Use the %s strategy when landing in mercurial.', '--squash'), ), ), 'squash' => array( 'help' => pht( 'Perform a %s merge, not a %s merge. If the project is '. 'marked as having a mutable history, this is the default behavior.', '--squash', '--no-ff'), 'conflicts' => array( 'merge' => pht( '%s and %s are conflicting merge strategies.', '--merge', '--squash'), ), ), 'delete-remote' => array( 'help' => pht( 'Delete the feature branch in the remote after landing it.'), 'conflicts' => array( 'keep-branch' => true, ), 'supports' => array( 'hg', ), ), 'revision' => array( 'param' => 'id', 'help' => pht( 'Use the message from a specific revision, rather than '. 'inferring the revision based on branch content.'), ), 'preview' => array( 'help' => pht( 'Prints the commits that would be landed. Does not '. 'actually modify or land the commits.'), ), '*' => 'branch', ); } public function run() { $this->readArguments(); $engine = null; if ($this->isGit && !$this->isGitSvn) { $engine = new ArcanistGitLandEngine(); } if ($engine) { $this->readEngineArguments(); $this->requireCleanWorkingCopy(); $should_hold = $this->getArgument('hold'); $engine ->setWorkflow($this) ->setRepositoryAPI($this->getRepositoryAPI()) ->setSourceRef($this->branch) ->setTargetRemote($this->remote) ->setTargetOnto($this->onto) ->setShouldHold($should_hold) ->setShouldKeep($this->keepBranch) ->setShouldSquash($this->useSquash) ->setShouldPreview($this->preview) ->setBuildMessageCallback(array($this, 'buildEngineMessage')); $engine->execute(); if (!$should_hold && !$this->preview) { $this->didPush(); } return 0; } $this->validate(); try { $this->pullFromRemote(); } catch (Exception $ex) { $this->restoreBranch(); throw $ex; } $this->printPendingCommits(); if ($this->preview) { $this->restoreBranch(); return 0; } $this->checkoutBranch(); $this->findRevision(); if ($this->useSquash) { $this->rebase(); $this->squash(); } else { $this->merge(); } $this->push(); if (!$this->keepBranch) { $this->cleanupBranch(); } if ($this->oldBranch != $this->onto) { // If we were on some branch A and the user ran "arc land B", // switch back to A. if ($this->keepBranch || $this->oldBranch != $this->branch) { $this->restoreBranch(); } } echo pht('Done.'), "\n"; return 0; } private function getUpstreamMatching($branch, $pattern) { if ($this->isGit) { $repository_api = $this->getRepositoryAPI(); list($err, $fullname) = $repository_api->execManualLocal( 'rev-parse --symbolic-full-name %s@{upstream}', $branch); if (!$err) { $matches = null; if (preg_match($pattern, $fullname, $matches)) { return last($matches); } } } return null; } + private function getGitSvnTrunk() { + if (!$this->isGitSvn) { + return null; + } + + // See T13293, this depends on the options passed when cloning. + // On any error we return `trunk`, which was the previous default. + + $repository_api = $this->getRepositoryAPI(); + list($err, $refspec) = $repository_api->execManualLocal( + 'config svn-remote.svn.fetch'); + + if ($err) { + return 'trunk'; + } + + $refspec = rtrim(substr($refspec, strrpos($refspec, ':') + 1)); + + $prefix = 'refs/remotes/'; + if (substr($refspec, 0, strlen($prefix)) !== $prefix) { + return 'trunk'; + } + + $refspec = substr($refspec, strlen($prefix)); + return $refspec; + } + private function readEngineArguments() { // NOTE: This is hard-coded for Git right now. // TODO: Clean this up and move it into LandEngines. $onto = $this->getEngineOnto(); $remote = $this->getEngineRemote(); // This just overwrites work we did earlier, but it has to be up in this // class for now because other parts of the workflow still depend on it. $this->onto = $onto; $this->remote = $remote; $this->ontoRemoteBranch = $this->remote.'/'.$onto; } private function getEngineOnto() { $onto = $this->getArgument('onto'); if ($onto !== null) { $this->writeInfo( pht('TARGET'), pht( 'Landing onto "%s", selected by the --onto flag.', $onto)); return $onto; } $api = $this->getRepositoryAPI(); $path = $api->getPathToUpstream($this->branch); if ($path->getLength()) { $cycle = $path->getCycle(); if ($cycle) { $this->writeWarn( pht('LOCAL CYCLE'), pht( 'Local branch tracks an upstream, but following it leads to a '. 'local cycle; ignoring branch upstream.')); echo tsprintf( "\n %s\n\n", implode(' -> ', $cycle)); } else { if ($path->isConnectedToRemote()) { $onto = $path->getRemoteBranchName(); $this->writeInfo( pht('TARGET'), pht( 'Landing onto "%s", selected by following tracking branches '. 'upstream to the closest remote.', $onto)); return $onto; } else { $this->writeInfo( pht('NO PATH TO UPSTREAM'), pht( 'Local branch tracks an upstream, but there is no path '. 'to a remote; ignoring branch upstream.')); } } } $config_key = 'arc.land.onto.default'; $onto = $this->getConfigFromAnySource($config_key); if ($onto !== null) { $this->writeInfo( pht('TARGET'), pht( 'Landing onto "%s", selected by "%s" configuration.', $onto, $config_key)); return $onto; } $onto = 'master'; $this->writeInfo( pht('TARGET'), pht( 'Landing onto "%s", the default target under git.', $onto)); return $onto; } private function getEngineRemote() { $remote = $this->getArgument('remote'); if ($remote !== null) { $this->writeInfo( pht('REMOTE'), pht( 'Using remote "%s", selected by the --remote flag.', $remote)); return $remote; } $api = $this->getRepositoryAPI(); $path = $api->getPathToUpstream($this->branch); $remote = $path->getRemoteRemoteName(); if ($remote !== null) { $this->writeInfo( pht('REMOTE'), pht( 'Using remote "%s", selected by following tracking branches '. 'upstream to the closest remote.', $remote)); return $remote; } $remote = 'origin'; $this->writeInfo( pht('REMOTE'), pht( 'Using remote "%s", the default remote under git.', $remote)); return $remote; } private function readArguments() { $repository_api = $this->getRepositoryAPI(); $this->isGit = $repository_api instanceof ArcanistGitAPI; $this->isHg = $repository_api instanceof ArcanistMercurialAPI; if ($this->isGit) { $repository = $this->loadProjectRepository(); $this->isGitSvn = (idx($repository, 'vcs') == 'svn'); } if ($this->isHg) { $this->isHgSvn = $repository_api->isHgSubversionRepo(); } $branch = $this->getArgument('branch'); if (empty($branch)) { $branch = $this->getBranchOrBookmark(); if ($branch !== null) { $this->branchType = $this->getBranchType($branch); // TODO: This message is misleading when landing a detached head or // a tag in Git. echo pht("Landing current %s '%s'.", $this->branchType, $branch), "\n"; $branch = array($branch); } } if (count($branch) !== 1) { throw new ArcanistUsageException( pht('Specify exactly one branch or bookmark to land changes from.')); } $this->branch = head($branch); $this->keepBranch = $this->getArgument('keep-branch'); $this->preview = $this->getArgument('preview'); if (!$this->branchType) { $this->branchType = $this->getBranchType($this->branch); } $onto_default = $this->isGit ? 'master' : 'default'; $onto_default = nonempty( $this->getConfigFromAnySource('arc.land.onto.default'), $onto_default); $onto_default = coalesce( $this->getUpstreamMatching($this->branch, '/^refs\/heads\/(.+)$/'), $onto_default); $this->onto = $this->getArgument('onto', $onto_default); $this->ontoType = $this->getBranchType($this->onto); $remote_default = $this->isGit ? 'origin' : ''; $remote_default = coalesce( $this->getUpstreamMatching($this->onto, '/^refs\/remotes\/(.+?)\//'), $remote_default); $this->remote = $this->getArgument('remote', $remote_default); if ($this->getArgument('merge')) { $this->useSquash = false; } else if ($this->getArgument('squash')) { $this->useSquash = true; } else { $this->useSquash = !$this->isHistoryImmutable(); } $this->ontoRemoteBranch = $this->onto; if ($this->isGitSvn) { - $this->ontoRemoteBranch = 'trunk'; + $this->ontoRemoteBranch = $this->getGitSvnTrunk(); } else if ($this->isGit) { $this->ontoRemoteBranch = $this->remote.'/'.$this->onto; } $this->oldBranch = $this->getBranchOrBookmark(); } private function validate() { $repository_api = $this->getRepositoryAPI(); if ($this->onto == $this->branch) { $message = pht( "You can not land a %s onto itself -- you are trying ". "to land '%s' onto '%s'. For more information on how to push ". "changes, see 'Pushing and Closing Revisions' in 'Arcanist User ". "Guide: arc diff' in the documentation.", $this->branchType, $this->branch, $this->onto); if (!$this->isHistoryImmutable()) { $message .= ' '.pht("You may be able to '%s' instead.", 'arc amend'); } throw new ArcanistUsageException($message); } if ($this->isHg) { if ($this->useSquash) { if (!$repository_api->supportsRebase()) { throw new ArcanistUsageException( pht( 'You must enable the rebase extension to use the %s strategy.', '--squash')); } } if ($this->branchType != $this->ontoType) { throw new ArcanistUsageException(pht( 'Source %s is a %s but destination %s is a %s. When landing a '. '%s, the destination must also be a %s. Use %s to specify a %s, '. 'or set %s in %s.', $this->branch, $this->branchType, $this->onto, $this->ontoType, $this->branchType, $this->branchType, '--onto', $this->branchType, 'arc.land.onto.default', '.arcconfig')); } } if ($this->isGit) { list($err) = $repository_api->execManualLocal( 'rev-parse --verify %s', $this->branch); if ($err) { throw new ArcanistUsageException( pht("Branch '%s' does not exist.", $this->branch)); } } $this->requireCleanWorkingCopy(); } private function checkoutBranch() { $repository_api = $this->getRepositoryAPI(); if ($this->getBranchOrBookmark() != $this->branch) { $repository_api->execxLocal('checkout %s', $this->branch); } switch ($this->branchType) { case self::REFTYPE_BOOKMARK: $message = pht( 'Switched to bookmark **%s**. Identifying and merging...', $this->branch); break; case self::REFTYPE_BRANCH: default: $message = pht( 'Switched to branch **%s**. Identifying and merging...', $this->branch); break; } echo phutil_console_format($message."\n"); } private function printPendingCommits() { $repository_api = $this->getRepositoryAPI(); if ($repository_api instanceof ArcanistGitAPI) { list($out) = $repository_api->execxLocal( 'log --oneline %s %s --', $this->branch, '^'.$this->onto); } else if ($repository_api instanceof ArcanistMercurialAPI) { $common_ancestor = $repository_api->getCanonicalRevisionName( hgsprintf('ancestor(%s,%s)', $this->onto, $this->branch)); $branch_range = hgsprintf( 'reverse((%s::%s) - %s)', $common_ancestor, $this->branch, $common_ancestor); list($out) = $repository_api->execxLocal( 'log -r %s --template %s', $branch_range, '{node|short} {desc|firstline}\n'); } if (!trim($out)) { $this->restoreBranch(); throw new ArcanistUsageException( pht('No commits to land from %s.', $this->branch)); } echo pht("The following commit(s) will be landed:\n\n%s", $out), "\n"; } private function findRevision() { $repository_api = $this->getRepositoryAPI(); $this->parseBaseCommitArgument(array($this->ontoRemoteBranch)); $revision_id = $this->getArgument('revision'); if ($revision_id) { $revision_id = $this->normalizeRevisionID($revision_id); $revisions = $this->getConduit()->callMethodSynchronous( 'differential.query', array( 'ids' => array($revision_id), )); if (!$revisions) { throw new ArcanistUsageException(pht( "No such revision '%s'!", "D{$revision_id}")); } } else { $revisions = $repository_api->loadWorkingCopyDifferentialRevisions( $this->getConduit(), array()); } if (!count($revisions)) { throw new ArcanistUsageException(pht( "arc can not identify which revision exists on %s '%s'. Update the ". "revision with recent changes to synchronize the %s name and hashes, ". "or use '%s' to amend the commit message at HEAD, or use ". "'%s' to select a revision explicitly.", $this->branchType, $this->branch, $this->branchType, 'arc amend', '--revision ')); } else if (count($revisions) > 1) { switch ($this->branchType) { case self::REFTYPE_BOOKMARK: $message = pht( "There are multiple revisions on feature bookmark '%s' which are ". "not present on '%s':\n\n". "%s\n". 'Separate these revisions onto different bookmarks, or use '. '--revision to use the commit message from '. 'and land them all.', $this->branch, $this->onto, $this->renderRevisionList($revisions)); break; case self::REFTYPE_BRANCH: default: $message = pht( "There are multiple revisions on feature branch '%s' which are ". "not present on '%s':\n\n". "%s\n". 'Separate these revisions onto different branches, or use '. '--revision to use the commit message from '. 'and land them all.', $this->branch, $this->onto, $this->renderRevisionList($revisions)); break; } throw new ArcanistUsageException($message); } $this->revision = head($revisions); $rev_status = $this->revision['status']; $rev_id = $this->revision['id']; $rev_title = $this->revision['title']; $rev_auxiliary = idx($this->revision, 'auxiliary', array()); $full_name = pht('D%d: %s', $rev_id, $rev_title); if ($this->revision['authorPHID'] != $this->getUserPHID()) { $other_author = $this->getConduit()->callMethodSynchronous( 'user.query', array( 'phids' => array($this->revision['authorPHID']), )); $other_author = ipull($other_author, 'userName', 'phid'); $other_author = $other_author[$this->revision['authorPHID']]; $ok = phutil_console_confirm(pht( "This %s has revision '%s' but you are not the author. Land this ". "revision by %s?", $this->branchType, $full_name, $other_author)); if (!$ok) { throw new ArcanistUserAbortException(); } } $state_warning = null; $state_header = null; if ($rev_status == ArcanistDifferentialRevisionStatus::CHANGES_PLANNED) { $state_header = pht('REVISION HAS CHANGES PLANNED'); $state_warning = pht( 'The revision you are landing ("%s") is currently in the "%s" state, '. 'indicating that you expect to revise it before moving forward.'. "\n\n". 'Normally, you should resubmit it for review and wait until it is '. '"%s" by reviewers before you continue.'. "\n\n". 'To resubmit the revision for review, either: update the revision '. 'with revised changes; or use "Request Review" from the web interface.', $full_name, pht('Changes Planned'), pht('Accepted')); } else if ($rev_status != ArcanistDifferentialRevisionStatus::ACCEPTED) { $state_header = pht('REVISION HAS NOT BEEN ACCEPTED'); $state_warning = pht( 'The revision you are landing ("%s") has not been "%s" by reviewers.', $full_name, pht('Accepted')); } if ($state_warning !== null) { $prompt = pht('Land revision in the wrong state?'); id(new PhutilConsoleBlock()) ->addParagraph(tsprintf('** %s **', $state_header)) ->addParagraph(tsprintf('%B', $state_warning)) ->draw(); $ok = phutil_console_confirm($prompt); if (!$ok) { throw new ArcanistUserAbortException(); } } if ($rev_auxiliary) { $phids = idx($rev_auxiliary, 'phabricator:depends-on', array()); if ($phids) { $dep_on_revs = $this->getConduit()->callMethodSynchronous( 'differential.query', array( 'phids' => $phids, 'status' => 'status-open', )); $open_dep_revs = array(); foreach ($dep_on_revs as $dep_on_rev) { $dep_on_rev_id = $dep_on_rev['id']; $dep_on_rev_title = $dep_on_rev['title']; $dep_on_rev_status = $dep_on_rev['status']; $open_dep_revs[$dep_on_rev_id] = $dep_on_rev_title; } if (!empty($open_dep_revs)) { $open_revs = array(); foreach ($open_dep_revs as $id => $title) { $open_revs[] = ' - D'.$id.': '.$title; } $open_revs = implode("\n", $open_revs); echo pht( "Revision '%s' depends on open revisions:\n\n%s", "D{$rev_id}: {$rev_title}", $open_revs); $ok = phutil_console_confirm(pht('Continue anyway?')); if (!$ok) { throw new ArcanistUserAbortException(); } } } } $message = $this->getConduit()->callMethodSynchronous( 'differential.getcommitmessage', array( 'revision_id' => $rev_id, )); $this->messageFile = new TempFile(); Filesystem::writeFile($this->messageFile, $message); echo pht( "Landing revision '%s'...", "D{$rev_id}: {$rev_title}")."\n"; $diff_phid = idx($this->revision, 'activeDiffPHID'); if ($diff_phid) { $this->checkForBuildables($diff_phid); } } private function pullFromRemote() { $repository_api = $this->getRepositoryAPI(); $local_ahead_of_remote = false; if ($this->isGit) { $repository_api->execxLocal('checkout %s', $this->onto); echo phutil_console_format(pht( "Switched to branch **%s**. Updating branch...\n", $this->onto)); try { $repository_api->execxLocal('pull --ff-only --no-stat'); } catch (CommandException $ex) { if (!$this->isGitSvn) { throw $ex; } } list($out) = $repository_api->execxLocal( 'log %s..%s', $this->ontoRemoteBranch, $this->onto); if (strlen(trim($out))) { $local_ahead_of_remote = true; } else if ($this->isGitSvn) { $repository_api->execxLocal('svn rebase'); } } else if ($this->isHg) { echo phutil_console_format(pht('Updating **%s**...', $this->onto)."\n"); try { list($out, $err) = $repository_api->execxLocal('pull'); $divergedbookmark = $this->onto.'@'.$repository_api->getBranchName(); if (strpos($err, $divergedbookmark) !== false) { throw new ArcanistUsageException(phutil_console_format(pht( "Local bookmark **%s** has diverged from the server's **%s** ". "(now labeled **%s**). Please resolve this divergence and run ". "'%s' again.", $this->onto, $this->onto, $divergedbookmark, 'arc land'))); } } catch (CommandException $ex) { $err = $ex->getError(); $stdout = $ex->getStdout(); // Copied from: PhabricatorRepositoryPullLocalDaemon.php // NOTE: Between versions 2.1 and 2.1.1, Mercurial changed the // behavior of "hg pull" to return 1 in case of a successful pull // with no changes. This behavior has been reverted, but users who // updated between Feb 1, 2012 and Mar 1, 2012 will have the // erroring version. Do a dumb test against stdout to check for this // possibility. // See: https://github.com/phacility/phabricator/issues/101/ // NOTE: Mercurial has translated versions, which translate this error // string. In a translated version, the string will be something else, // like "aucun changement trouve". There didn't seem to be an easy way // to handle this (there are hard ways but this is not a common // problem and only creates log spam, not application failures). // Assume English. // TODO: Remove this once we're far enough in the future that // deployment of 2.1 is exceedingly rare? if ($err != 1 || !preg_match('/no changes found/', $stdout)) { throw $ex; } } // Pull succeeded. Now make sure master is not on an outgoing change if ($repository_api->supportsPhases()) { list($out) = $repository_api->execxLocal( 'log -r %s --template %s', $this->onto, '{phase}'); if ($out != 'public') { $local_ahead_of_remote = true; } } else { // execManual instead of execx because outgoing returns // code 1 when there is nothing outgoing list($err, $out) = $repository_api->execManualLocal( 'outgoing -r %s', $this->onto); // $err === 0 means something is outgoing if ($err === 0) { $local_ahead_of_remote = true; } } } if ($local_ahead_of_remote) { throw new ArcanistUsageException(pht( "Local %s '%s' is ahead of remote %s '%s', so landing a feature ". "%s would push additional changes. Push or reset the changes in '%s' ". "before running '%s'.", $this->ontoType, $this->onto, $this->ontoType, $this->ontoRemoteBranch, $this->ontoType, $this->onto, 'arc land')); } } private function rebase() { $repository_api = $this->getRepositoryAPI(); chdir($repository_api->getPath()); if ($this->isHg) { $onto_tip = $repository_api->getCanonicalRevisionName($this->onto); $common_ancestor = $repository_api->getCanonicalRevisionName( hgsprintf('ancestor(%s, %s)', $this->onto, $this->branch)); // Only rebase if the local branch is not at the tip of the onto branch. if ($onto_tip != $common_ancestor) { // keep branch here so later we can decide whether to remove it $err = $repository_api->execPassthru( 'rebase -d %s --keepbranches', $this->onto); if ($err) { echo phutil_console_format("%s\n", pht('Aborting rebase')); $repository_api->execManualLocal('rebase --abort'); $this->restoreBranch(); throw new ArcanistUsageException(pht( "'%s' failed and the rebase was aborted. This is most ". "likely due to conflicts. Manually rebase %s onto %s, resolve ". "the conflicts, then run '%s' again.", sprintf('hg rebase %s', $this->onto), $this->branch, $this->onto, 'arc land')); } } } $repository_api->reloadWorkingCopy(); } private function squash() { $repository_api = $this->getRepositoryAPI(); if ($this->isGit) { $repository_api->execxLocal('checkout %s', $this->onto); $repository_api->execxLocal( 'merge --no-stat --squash --ff-only %s', $this->branch); } else if ($this->isHg) { // The hg code is a little more complex than git's because we // need to handle the case where the landing branch has child branches: // -a--------b master // \ // w--x mybranch // \--y subbranch1 // \--z subbranch2 // // arc land --branch mybranch --onto master : // -a--b--wx master // \--y subbranch1 // \--z subbranch2 $branch_rev_id = $repository_api->getCanonicalRevisionName($this->branch); // At this point $this->onto has been pulled from remote and // $this->branch has been rebased on top of onto(by the rebase() // function). So we're guaranteed to have onto as an ancestor of branch // when we use first((onto::branch)-onto) below. $branch_root = $repository_api->getCanonicalRevisionName( hgsprintf('first((%s::%s)-%s)', $this->onto, $this->branch, $this->onto)); $branch_range = hgsprintf( '(%s::%s)', $branch_root, $this->branch); if (!$this->keepBranch) { $this->handleAlternateBranches($branch_root, $branch_range); } // Collapse just the landing branch onto master. // Leave its children on the original branch. $err = $repository_api->execPassthru( 'rebase --collapse --keep --logfile %s -r %s -d %s', $this->messageFile, $branch_range, $this->onto); if ($err) { $repository_api->execManualLocal('rebase --abort'); $this->restoreBranch(); throw new ArcanistUsageException( pht( "Squashing the commits under %s failed. ". "Manually squash your commits and run '%s' again.", $this->branch, 'arc land')); } if ($repository_api->isBookmark($this->branch)) { // a bug in mercurial means bookmarks end up on the revision prior // to the collapse when using --collapse with --keep, // so we manually move them to the correct spots // see: http://bz.selenic.com/show_bug.cgi?id=3716 $repository_api->execxLocal( 'bookmark -f %s', $this->onto); $repository_api->execxLocal( 'bookmark -f %s -r %s', $this->branch, $branch_rev_id); } // check if the branch had children list($output) = $repository_api->execxLocal( 'log -r %s --template %s', hgsprintf('children(%s)', $this->branch), '{node}\n'); $child_branch_roots = phutil_split_lines($output, false); $child_branch_roots = array_filter($child_branch_roots); if ($child_branch_roots) { // move the branch's children onto the collapsed commit foreach ($child_branch_roots as $child_root) { $repository_api->execxLocal( 'rebase -d %s -s %s --keep --keepbranches', $this->onto, $child_root); } } // All the rebases may have moved us to another branch // so we move back. $repository_api->execxLocal('checkout %s', $this->onto); } } /** * Detect alternate branches and prompt the user for how to handle * them. An alternate branch is a branch that forks from the landing * branch prior to the landing branch tip. * * In a situation like this: * -a--------b master * \ * w--x landingbranch * \ \-- g subbranch * \--y altbranch1 * \--z altbranch2 * * y and z are alternate branches and will get deleted by the squash, * so we need to detect them and ask the user what they want to do. * * @param string The revision id of the landing branch's root commit. * @param string The revset specifying all the commits in the landing branch. * @return void */ private function handleAlternateBranches($branch_root, $branch_range) { $repository_api = $this->getRepositoryAPI(); // Using the tree in the doccomment, the revset below resolves as follows: // 1. roots(descendants(w) - descendants(x) - (w::x)) // 2. roots({x,g,y,z} - {g} - {w,x}) // 3. roots({y,z}) // 4. {y,z} $alt_branch_revset = hgsprintf( 'roots(descendants(%s)-descendants(%s)-%R)', $branch_root, $this->branch, $branch_range); list($alt_branches) = $repository_api->execxLocal( 'log --template %s -r %s', '{node}\n', $alt_branch_revset); $alt_branches = phutil_split_lines($alt_branches, false); $alt_branches = array_filter($alt_branches); $alt_count = count($alt_branches); if ($alt_count > 0) { $input = phutil_console_prompt(pht( "%s '%s' has %s %s(s) forking off of it that would be deleted ". "during a squash. Would you like to keep a non-squashed copy, rebase ". "them on top of '%s', or abort and deal with them yourself? ". "(k)eep, (r)ebase, (a)bort:", ucfirst($this->branchType), $this->branch, $alt_count, $this->branchType, $this->branch)); if ($input == 'k' || $input == 'keep') { $this->keepBranch = true; } else if ($input == 'r' || $input == 'rebase') { foreach ($alt_branches as $alt_branch) { $repository_api->execxLocal( 'rebase --keep --keepbranches -d %s -s %s', $this->branch, $alt_branch); } } else if ($input == 'a' || $input == 'abort') { $branch_string = implode("\n", $alt_branches); echo "\n", pht( "Remove the %s starting at these revisions and run %s again:\n%s", $this->branchType.'s', $branch_string, 'arc land'), "\n\n"; throw new ArcanistUserAbortException(); } else { throw new ArcanistUsageException( pht('Invalid choice. Aborting arc land.')); } } } private function merge() { $repository_api = $this->getRepositoryAPI(); // In immutable histories, do a --no-ff merge to force a merge commit with // the right message. $repository_api->execxLocal('checkout %s', $this->onto); chdir($repository_api->getPath()); if ($this->isGit) { $err = phutil_passthru( 'git merge --no-stat --no-ff --no-commit %s', $this->branch); if ($err) { throw new ArcanistUsageException(pht( "'%s' failed. Your working copy has been left in a partially ". "merged state. You can: abort with '%s'; or follow the ". "instructions to complete the merge.", 'git merge', 'git merge --abort')); } } else if ($this->isHg) { // HG arc land currently doesn't support --merge. // When merging a bookmark branch to a master branch that // hasn't changed since the fork, mercurial fails to merge. // Instead of only working in some cases, we just disable --merge // until there is a demand for it. // The user should never reach this line, since --merge is // forbidden at the command line argument level. throw new ArcanistUsageException( pht('%s is not currently supported for hg repos.', '--merge')); } } private function push() { $repository_api = $this->getRepositoryAPI(); // These commands can fail legitimately (e.g. commit hooks) try { if ($this->isGit) { $repository_api->execxLocal('commit -F %s', $this->messageFile); if (phutil_is_windows()) { // Occasionally on large repositories on Windows, Git can exit with // an unclean working copy here. This prevents reverts from being // pushed to the remote when this occurs. $this->requireCleanWorkingCopy(); } } else if ($this->isHg) { // hg rebase produces a commit earlier as part of rebase if (!$this->useSquash) { $repository_api->execxLocal( 'commit --logfile %s', $this->messageFile); } } // We dispatch this event so we can run checks on the merged revision, // right before it gets pushed out. It's easier to do this in arc land // than to try to hook into git/hg. $this->didCommitMerge(); } catch (Exception $ex) { $this->executeCleanupAfterFailedPush(); throw $ex; } if ($this->getArgument('hold')) { echo phutil_console_format(pht( 'Holding change in **%s**: it has NOT been pushed yet.', $this->onto)."\n"); } else { echo pht('Pushing change...'), "\n\n"; chdir($repository_api->getPath()); if ($this->isGitSvn) { $err = phutil_passthru('git svn dcommit'); $cmd = 'git svn dcommit'; } else if ($this->isGit) { $err = phutil_passthru('git push %s %s', $this->remote, $this->onto); $cmd = 'git push'; } else if ($this->isHgSvn) { // hg-svn doesn't support 'push -r', so we do a normal push // which hg-svn modifies to only push the current branch and // ancestors. $err = $repository_api->execPassthru('push %s', $this->remote); $cmd = 'hg push'; } else if ($this->isHg) { if (strlen($this->remote)) { $err = $repository_api->execPassthru( 'push -r %s %s', $this->onto, $this->remote); } else { $err = $repository_api->execPassthru( 'push -r %s', $this->onto); } $cmd = 'hg push'; } if ($err) { echo phutil_console_format( "** %s **\n", pht('PUSH FAILED!')); $this->executeCleanupAfterFailedPush(); if ($this->isGit) { throw new ArcanistUsageException(pht( "'%s' failed! Fix the error and run '%s' again.", $cmd, 'arc land')); } throw new ArcanistUsageException(pht( "'%s' failed! Fix the error and push this change manually.", $cmd)); } $this->didPush(); echo "\n"; } } private function executeCleanupAfterFailedPush() { $repository_api = $this->getRepositoryAPI(); if ($this->isGit) { $repository_api->execxLocal('reset --hard HEAD^'); $this->restoreBranch(); } else if ($this->isHg) { $repository_api->execxLocal( '--config extensions.mq= strip %s', $this->onto); $this->restoreBranch(); } } private function cleanupBranch() { $repository_api = $this->getRepositoryAPI(); echo pht('Cleaning up feature %s...', $this->branchType), "\n"; if ($this->isGit) { list($ref) = $repository_api->execxLocal( 'rev-parse --verify %s', $this->branch); $ref = trim($ref); $recovery_command = csprintf( 'git checkout -b %s %s', $this->branch, $ref); echo pht('(Use `%s` if you want it back.)', $recovery_command), "\n"; $repository_api->execxLocal('branch -D %s', $this->branch); } else if ($this->isHg) { $common_ancestor = $repository_api->getCanonicalRevisionName( hgsprintf('ancestor(%s,%s)', $this->onto, $this->branch)); $branch_root = $repository_api->getCanonicalRevisionName( hgsprintf('first((%s::%s)-%s)', $common_ancestor, $this->branch, $common_ancestor)); $repository_api->execxLocal( '--config extensions.mq= strip -r %s', $branch_root); if ($repository_api->isBookmark($this->branch)) { $repository_api->execxLocal('bookmark -d %s', $this->branch); } } if ($this->getArgument('delete-remote')) { if ($this->isHg) { // named branches were closed as part of the earlier commit // so only worry about bookmarks if ($repository_api->isBookmark($this->branch)) { $repository_api->execxLocal( 'push -B %s %s', $this->branch, $this->remote); } } } } public function getSupportedRevisionControlSystems() { return array('git', 'hg'); } private function getBranchOrBookmark() { $repository_api = $this->getRepositoryAPI(); if ($this->isGit) { $branch = $repository_api->getBranchName(); // If we don't have a branch name, just use whatever's at HEAD. if (!strlen($branch) && !$this->isGitSvn) { $branch = $repository_api->getWorkingCopyRevision(); } } else if ($this->isHg) { $branch = $repository_api->getActiveBookmark(); if (!$branch) { $branch = $repository_api->getBranchName(); } } return $branch; } private function getBranchType($branch) { $repository_api = $this->getRepositoryAPI(); if ($this->isHg && $repository_api->isBookmark($branch)) { return 'bookmark'; } return 'branch'; } /** * Restore the original branch, e.g. after a successful land or a failed * pull. */ private function restoreBranch() { $repository_api = $this->getRepositoryAPI(); $repository_api->execxLocal('checkout %s', $this->oldBranch); if ($this->isGit) { $repository_api->execxLocal('submodule update --init --recursive'); } echo pht( "Switched back to %s %s.\n", $this->branchType, phutil_console_format('**%s**', $this->oldBranch)); } /** * Check if a diff has a running or failed buildable, and prompt the user * before landing if it does. */ private function checkForBuildables($diff_phid) { // Try to use the more modern check which respects the "Warn on Land" // behavioral flag on build plans if we can. This newer check won't work // unless the server is running code from March 2019 or newer since the // API methods we need won't exist yet. We'll fall back to the older check // if this one doesn't work out. try { $this->checkForBuildablesWithPlanBehaviors($diff_phid); return; } catch (ArcanistUserAbortException $abort_ex) { throw $abort_ex; } catch (Exception $ex) { // Continue with the older approach, below. } // NOTE: Since Harbormaster is still beta and this stuff all got added // recently, just bail if we can't find a buildable. This is just an // advisory check intended to prevent human error. try { $buildables = $this->getConduit()->callMethodSynchronous( 'harbormaster.querybuildables', array( 'buildablePHIDs' => array($diff_phid), 'manualBuildables' => false, )); } catch (ConduitClientException $ex) { return; } if (!$buildables['data']) { // If there's no corresponding buildable, we're done. return; } $console = PhutilConsole::getConsole(); $buildable = head($buildables['data']); if ($buildable['buildableStatus'] == 'passed') { $console->writeOut( "** %s ** %s\n", pht('BUILDS PASSED'), pht('Harbormaster builds for the active diff completed successfully.')); return; } switch ($buildable['buildableStatus']) { case 'building': $message = pht( 'Harbormaster is still building the active diff for this revision.'); $prompt = pht('Land revision anyway, despite ongoing build?'); break; case 'failed': $message = pht( 'Harbormaster failed to build the active diff for this revision.'); $prompt = pht('Land revision anyway, despite build failures?'); break; default: // If we don't recognize the status, just bail. return; } $builds = $this->queryBuilds( array( 'buildablePHIDs' => array($buildable['phid']), )); $console->writeOut($message."\n\n"); $builds = msort($builds, 'getStatusSortVector'); foreach ($builds as $build) { $ansi_color = $build->getStatusANSIColor(); $status_name = $build->getStatusName(); $object_name = $build->getObjectName(); $build_name = $build->getName(); echo tsprintf( " ** %s ** %s: %s\n", $status_name, $object_name, $build_name); } $console->writeOut( "\n%s\n\n **%s**: __%s__", pht('You can review build details here:'), pht('Harbormaster URI'), $buildable['uri']); if (!phutil_console_confirm($prompt)) { throw new ArcanistUserAbortException(); } } private function checkForBuildablesWithPlanBehaviors($diff_phid) { // TODO: These queries should page through all results instead of fetching // only the first page, but we don't have good primitives to support that // in "master" yet. $this->writeInfo( pht('BUILDS'), pht('Checking build status...')); $raw_buildables = $this->getConduit()->callMethodSynchronous( 'harbormaster.buildable.search', array( 'constraints' => array( 'objectPHIDs' => array( $diff_phid, ), 'manual' => false, ), )); if (!$raw_buildables['data']) { return; } $buildables = $raw_buildables['data']; $buildable_phids = ipull($buildables, 'phid'); $raw_builds = $this->getConduit()->callMethodSynchronous( 'harbormaster.build.search', array( 'constraints' => array( 'buildables' => $buildable_phids, ), )); if (!$raw_builds['data']) { return; } $builds = array(); foreach ($raw_builds['data'] as $raw_build) { $build_ref = ArcanistBuildRef::newFromConduit($raw_build); $build_phid = $build_ref->getPHID(); $builds[$build_phid] = $build_ref; } $plan_phids = mpull($builds, 'getBuildPlanPHID'); $plan_phids = array_values($plan_phids); $raw_plans = $this->getConduit()->callMethodSynchronous( 'harbormaster.buildplan.search', array( 'constraints' => array( 'phids' => $plan_phids, ), )); $plans = array(); foreach ($raw_plans['data'] as $raw_plan) { $plan_ref = ArcanistBuildPlanRef::newFromConduit($raw_plan); $plan_phid = $plan_ref->getPHID(); $plans[$plan_phid] = $plan_ref; } $ongoing_builds = array(); $failed_builds = array(); $builds = msort($builds, 'getStatusSortVector'); foreach ($builds as $build_ref) { $plan = idx($plans, $build_ref->getBuildPlanPHID()); if (!$plan) { continue; } $plan_behavior = $plan->getBehavior('arc-land', 'always'); $if_building = ($plan_behavior == 'building'); $if_complete = ($plan_behavior == 'complete'); $if_never = ($plan_behavior == 'never'); // If the build plan "Never" warns when landing, skip it. if ($if_never) { continue; } // If the build plan warns when landing "If Complete" but the build is // not complete, skip it. if ($if_complete && !$build_ref->isComplete()) { continue; } // If the build plan warns when landing "If Building" but the build is // complete, skip it. if ($if_building && $build_ref->isComplete()) { continue; } // Ignore passing builds. if ($build_ref->isPassed()) { continue; } if (!$build_ref->isComplete()) { $ongoing_builds[] = $build_ref; } else { $failed_builds[] = $build_ref; } } if (!$ongoing_builds && !$failed_builds) { return; } if ($failed_builds) { $this->writeWarn( pht('BUILD FAILURES'), pht( 'Harbormaster failed to build the active diff for this revision:')); $prompt = pht('Land revision anyway, despite build failures?'); } else if ($ongoing_builds) { $this->writeWarn( pht('ONGOING BUILDS'), pht( 'Harbormaster is still building the active diff for this revision:')); $prompt = pht('Land revision anyway, despite ongoing build?'); } $show_builds = array_merge($failed_builds, $ongoing_builds); echo "\n"; foreach ($show_builds as $build_ref) { $ansi_color = $build_ref->getStatusANSIColor(); $status_name = $build_ref->getStatusName(); $object_name = $build_ref->getObjectName(); $build_name = $build_ref->getName(); echo tsprintf( " ** %s ** %s: %s\n", $status_name, $object_name, $build_name); } echo tsprintf( "\n%s\n\n", pht('You can review build details here:')); foreach ($buildables as $buildable) { $buildable_uri = id(new PhutilURI($this->getConduitURI())) ->setPath(sprintf('/B%d', $buildable['id'])); echo tsprintf( " **%s**: __%s__\n", pht('Buildable %d', $buildable['id']), $buildable_uri); } if (!phutil_console_confirm($prompt)) { throw new ArcanistUserAbortException(); } } public function buildEngineMessage(ArcanistLandEngine $engine) { // TODO: This is oh-so-gross. $this->findRevision(); $engine->setCommitMessageFile($this->messageFile); } public function didCommitMerge() { $this->dispatchEvent( ArcanistEventType::TYPE_LAND_WILLPUSHREVISION, array()); } public function didPush() { $this->askForRepositoryUpdate(); $mark_workflow = $this->buildChildWorkflow( 'close-revision', array( '--finalize', '--quiet', $this->revision['id'], )); $mark_workflow->run(); } private function queryBuilds(array $constraints) { $conduit = $this->getConduit(); // NOTE: This method only loads the 100 most recent builds. It's rare for // a revision to have more builds than that and there's currently no paging // wrapper for "*.search" Conduit API calls available in Arcanist. try { $raw_result = $conduit->callMethodSynchronous( 'harbormaster.build.search', array( 'constraints' => $constraints, )); } catch (Exception $ex) { // If the server doesn't have "harbormaster.build.search" yet (Aug 2016), // try the older "harbormaster.querybuilds" instead. $raw_result = $conduit->callMethodSynchronous( 'harbormaster.querybuilds', $constraints); } $refs = array(); foreach ($raw_result['data'] as $raw_data) { $refs[] = ArcanistBuildRef::newFromConduit($raw_data); } return $refs; } }