Fix some lint rendering issues when lines prior to other identical lines are removed

Summary: See PHI191. This is a rehash of an earlier fix, but we didn't have a test case for this half yet.

Test Plan:
  - Added a failing test, made it pass.
  - Added a linter like the one in PHI191, ran it, got a valid lint result instead of an exception.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18759

src/lint/renderer/ArcanistConsoleLintRenderer.php

@@ -148,36 +148,35 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer {
         $old_impact--;
         $new_impact--;
 
-        if ($old_impact < 0 || $new_impact < 0) {
+        // We can end up here if a patch removes a line which occurs before
-          throw new Exception(
+        // another identical line.
-            pht(
+        if ($old_impact <= 0 || $new_impact <= 0) {
-              'Modified prefix line range has become negative '.
+          break;
-              '(old = %d, new = %d).',
-              $old_impact,
-              $new_impact));
         }
       } while (true);
 
       // If the lines at the end of the changed line range are actually the
       // same, shrink the range. This happens when a patch just removes a
       // line.
-      do {
+      if ($old_impact > 0 && $new_impact > 0) {
-        $old_suffix = idx($old_lines, $start + $old_impact - 2, null);
+        do {
-        $new_suffix = idx($new_lines, $start + $new_impact - 2, null);
+          $old_suffix = idx($old_lines, $start + $old_impact - 2, null);
+          $new_suffix = idx($new_lines, $start + $new_impact - 2, null);
 
-        if ($old_suffix !== $new_suffix) {
+          if ($old_suffix !== $new_suffix) {
-          break;
+            break;
-        }
+          }
 
-        $old_impact--;
+          $old_impact--;
-        $new_impact--;
+          $new_impact--;
 
-        // We can end up here if a patch removes a line which occurs after
+          // We can end up here if a patch removes a line which occurs after
-        // another identical line.
+          // another identical line.
-        if ($old_impact <= 0 || $new_impact <= 0) {
+          if ($old_impact <= 0 || $new_impact <= 0) {
-          break;
+            break;
-        }
+          }
-      } while (true);
+        } while (true);
+      }
 
     } else {
 

src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php

@@ -142,6 +142,20 @@ EOTEXT;
         'replacement' => "\nX\nY\n",
       ),
 
+      'rmmulti' => array(
+        'line' => 2,
+        'char' => 1,
+        'original' => "\n",
+        'replacement' => '',
+      ),
+
+      'rmmulti2' => array(
+        'line' => 1,
+        'char' => 2,
+        'original' => "\n",
+        'replacement' => '',
+      ),
+
     );
 
     $defaults = array(

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

@@ -0,0 +1,10 @@
+>>> Lint for path/to/example.c:
+
+
+   Warning  (WARN123) Lint Warning
+    Consider this.
+
+               1 A
+               2 ~
+    >>> -      3 ~
+               4 B

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

@@ -0,0 +1,4 @@
+A
+
+
+B

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

@@ -0,0 +1,10 @@
+>>> Lint for path/to/example.c:
+
+
+   Warning  (WARN123) Lint Warning
+    Consider this.
+
+               1 A
+    >>> -      2 ~
+               3 ~
+               4 B

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

@@ -0,0 +1,4 @@
+A
+
+
+B