(experimental) Merge branch 'master' into experimental

src/lint/ArcanistLintMessage.php

@@ -25,8 +25,15 @@ final class ArcanistLintMessage extends Phobject {
     $message = new ArcanistLintMessage();
 
     $message->setPath($dict['path']);
-    $message->setLine($dict['line']);
+
-    $message->setChar($dict['char']);
+    if (isset($dict['line'])) {
+      $message->setLine($dict['line']);
+    }
+
+    if (isset($dict['char'])) {
+      $message->setChar($dict['char']);
+    }
+
     $message->setCode($dict['code']);
     $message->setSeverity($dict['severity']);
     $message->setName($dict['name']);

src/lint/renderer/ArcanistConsoleLintRenderer.php

@@ -235,8 +235,11 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer {
     }
 
     for ($ii = $start; $ii < $start + $new_impact; $ii++) {
+      // If the patch was at the end of the file and ends with a newline, we
+      // won't have an actual entry in the array for the last line, even though
+      // we want to show it in the diff.
       $out[] = array(
-        'text' => $new_lines[$ii - 1],
+        'text' => idx($new_lines, $ii - 1, ''),
         'type' => '+',
         'chevron' => ($ii == $start),
       );
@@ -265,9 +268,17 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer {
         }
       }
 
+      // If the line doesn't actually end in a newline, add one so the layout
+      // doesn't mess up. This can happen when the last line of the old file
+      // didn't have a newline at the end.
+      $text = $spec['text'];
+      if (!preg_match('/\n\z/', $spec['text'])) {
+        $text .= "\n";
+      }
+
       $result[] = $this->renderLine(
         idx($spec, 'number'),
-        $spec['text'],
+        $text,
         $chevron,
         idx($spec, 'type'));
 
@@ -310,6 +321,15 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer {
       $offset += strlen($line);
     }
 
+    // If the last line ends in a newline, add a virtual offset for the final
+    // line with no characters on it. This allows lint messages to target the
+    // last line of the file at character 1.
+    if ($lines) {
+      if (preg_match('/\n\z/', $line)) {
+        $line_map[$number] = $offset;
+      }
+    }
+
     return $line_map;
   }
 

src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php

@@ -120,6 +120,28 @@ EOTEXT;
         'original' => "\n",
         'replacement' => '',
       ),
+
+      'eofnewline' => array(
+        'line' => 1,
+        'char' => 7,
+        'original' => '',
+        'replacement' => "\n",
+      ),
+
+      'eofmultilinechar' => array(
+        'line' => 5,
+        'char' => 3,
+        'original' => '',
+        'replacement' => "\nX\nY\n",
+      ),
+
+      'eofmultilineline' => array(
+        'line' => 6,
+        'char' => 1,
+        'original' => '',
+        'replacement' => "\nX\nY\n",
+      ),
+
     );
 
     $defaults = array(
@@ -132,8 +154,6 @@ EOTEXT;
 
     foreach ($map as $key => $test_case) {
       $data = $this->readTestData("{$key}.txt");
-      $data = preg_replace('/~+\s*$/m', '', $data);
-
       $expect = $this->readTestData("{$key}.expect");
 
       $test_case = $test_case + $defaults;
@@ -184,11 +204,6 @@ EOTEXT;
         throw $ex;
       }
 
-      // Trim "~" off the ends of lines. This allows the "expect" file to test
-      // for trailing whitespace without actually containing trailing
-      // whitespace.
-      $expect = preg_replace('/~+$/m', '', $expect);
-
       $this->assertEqual(
         $expect,
         $actual,
@@ -200,7 +215,19 @@ EOTEXT;
 
   private function readTestData($filename) {
     $path = dirname(__FILE__).'/data/'.$filename;
-    return Filesystem::readFile($path);
+    $data = Filesystem::readFile($path);
+
+    // If we find "~~~" at the end of the file, get rid of it and any whitespace
+    // afterwards. This allows specifying data files with trailing empty
+    // lines.
+    $data = preg_replace('/~~~\s*\z/', '', $data);
+
+    // Trim "~" off the ends of lines. This allows the "expect" file to test
+    // for trailing whitespace without actually containing trailing
+    // whitespace.
+    $data = preg_replace('/~$/m', '', $data);
+
+    return $data;
   }
 
 }

src/lint/renderer/__tests__/data/eofmultilinechar.expect

@@ -0,0 +1,12 @@
+>>> Lint for path/to/example.c:
+
+
+   Warning  (WARN123) Lint Warning
+    Consider this.
+
+               3 3
+               4 4
+               5 5
+    >>> +        ~
+        +        X
+        +        Y

src/lint/renderer/__tests__/data/eofmultilinechar.txt

@@ -0,0 +1,5 @@
+1
+2
+3
+4
+5

src/lint/renderer/__tests__/data/eofmultilineline.expect

@@ -0,0 +1,12 @@
+>>> Lint for path/to/example.c:
+
+
+   Warning  (WARN123) Lint Warning
+    Consider this.
+
+               3 3
+               4 4
+               5 5
+    >>> +        ~
+        +        X
+        +        Y

src/lint/renderer/__tests__/data/eofmultilineline.txt

@@ -0,0 +1,5 @@
+1
+2
+3
+4
+5

src/lint/renderer/__tests__/data/eofnewline.expect

@@ -0,0 +1,9 @@
+>>> Lint for path/to/example.c:
+
+
+   Warning  (WARN123) Lint Warning
+    Consider this.
+
+    >>> -      1 abcdef
+        +        abcdef
+        +        ~

src/lint/renderer/__tests__/data/eofnewline.txt

@@ -0,0 +1 @@
+abcdef~~~

src/lint/renderer/__tests__/data/extrawhitespace.txt

@@ -1,3 +1,3 @@
 Adrift upon the sea.
 
-~
+~~~

src/repository/api/ArcanistGitAPI.php

@@ -497,6 +497,10 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
       return null;
     }
 
+    if (!strlen($branch)) {
+      return null;
+    }
+
     return $branch;
   }
 
@@ -509,7 +513,7 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
       // Verify this, and strip it.
       $ref = rtrim($stdout);
       $branch = $this->getBranchNameFromRef($ref);
-      if (!$branch) {
+      if ($branch === null) {
         throw new Exception(
           pht('Failed to parse %s output!', 'git symbolic-ref'));
       }
@@ -1015,7 +1019,7 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
       list($ref, $hash, $epoch, $tree, $desc, $text) = $fields;
 
       $branch = $this->getBranchNameFromRef($ref);
-      if ($branch) {
+      if ($branch !== null) {
         $result[] = array(
           'current' => ($branch === $current),
           'name' => $branch,

src/workflow/ArcanistLandWorkflow.php

@@ -444,7 +444,7 @@ EOTEXT
     $branch = $this->getArgument('branch');
     if (empty($branch)) {
       $branch = $this->getBranchOrBookmark();
-      if ($branch) {
+      if ($branch !== null) {
         $this->branchType = $this->getBranchType($branch);
 
         // TODO: This message is misleading when landing a detached head or