From cbc785ddce71be073f536c1faea8c231e495d5df Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 5 Sep 2017 16:50:59 -0700
Subject: [PATCH] Fix a similar lint rendering issue when trimming identical
 lines out of patches

Summary: Ref T9846. See PHI48. See D18538 for a similar fix. We can contract the suffix lines too much if, e.g, a newline after another newline is removed. Prevent contraction to fewer than 0 lines.

Test Plan: Added a failing test, made it pass.

Reviewers: chad

Reviewed By: chad

Subscribers: alexmv

Maniphest Tasks: T9846

Differential Revision: https://secure.phabricator.com/D18541
---
 .../renderer/ArcanistConsoleLintRenderer.php  | 11 ++++-----
 .../ArcanistConsoleLintRendererTestCase.php   | 24 +++++++++++++++++++
 .../renderer/__tests__/data/remline.expect    | 12 ++++++++++
 src/lint/renderer/__tests__/data/remline.txt  |  6 +++++
 4 files changed, 46 insertions(+), 7 deletions(-)
 create mode 100644 src/lint/renderer/__tests__/data/remline.expect
 create mode 100644 src/lint/renderer/__tests__/data/remline.txt

diff --git a/src/lint/renderer/ArcanistConsoleLintRenderer.php b/src/lint/renderer/ArcanistConsoleLintRenderer.php
index 8880fbe8..12ef4e69 100644
--- a/src/lint/renderer/ArcanistConsoleLintRenderer.php
+++ b/src/lint/renderer/ArcanistConsoleLintRenderer.php
@@ -171,13 +171,10 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer {
         $old_impact--;
         $new_impact--;
 
-        if ($old_impact < 0 || $new_impact < 0) {
-          throw new Exception(
-            pht(
-              'Modified suffix line range has become negative '.
-              '(old = %d, new = %d).',
-              $old_impact,
-              $new_impact));
+        // We can end up here if a patch removes a line which occurs after
+        // another identical line.
+        if ($old_impact <= 0 || $new_impact <= 0) {
+          break;
         }
       } while (true);
 
diff --git a/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php b/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php
index bf7bca4b..f36b30e7 100644
--- a/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php
+++ b/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php
@@ -15,6 +15,23 @@ EOTEXT;
 import apple;
 import banana;
 
+import cat;
+import dog;
+EOTEXT;
+
+    $remline_original = <<<EOTEXT
+import apple;
+import banana;
+
+
+import cat;
+import dog;
+EOTEXT;
+
+    $remline_replacement = <<<EOTEXT
+import apple;
+import banana;
+
 import cat;
 import dog;
 EOTEXT;
@@ -89,6 +106,13 @@ EOTEXT;
         'original' => $midline_original,
         'replacement' => $midline_replacement,
       ),
+
+      'remline' => array(
+        'line' => 1,
+        'char' => 1,
+        'original' => $remline_original,
+        'replacement' => $remline_replacement,
+      ),
     );
 
     $defaults = array(
diff --git a/src/lint/renderer/__tests__/data/remline.expect b/src/lint/renderer/__tests__/data/remline.expect
new file mode 100644
index 00000000..a4d65c95
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/remline.expect
@@ -0,0 +1,12 @@
+>>> Lint for path/to/example.c:
+
+
+   Warning  (WARN123) Lint Warning
+    Consider this.
+
+               1 import apple;
+               2 import banana;
+               3 ~
+    >>> -      4 ~
+               5 import cat;
+               6 import dog;
diff --git a/src/lint/renderer/__tests__/data/remline.txt b/src/lint/renderer/__tests__/data/remline.txt
new file mode 100644
index 00000000..4e526abe
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/remline.txt
@@ -0,0 +1,6 @@
+import apple;
+import banana;
+
+
+import cat;
+import dog;