From 19d41faddd132b74d18840853931516dd9385255 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 20 Sep 2017 11:59:18 -0700
Subject: [PATCH] (stable) Correct lint rendering when patching trailing
 whitespace in files

Summary: Ref PHI48. If a patch removes all of the lines at the end of a file, we can get some array index errors.

Test Plan: Added failing test, made it pass.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18631
---
 src/lint/linter/ArcanistTextLinter.php              |  4 +---
 src/lint/renderer/ArcanistConsoleLintRenderer.php   | 13 +++++++------
 .../ArcanistConsoleLintRendererTestCase.php         |  9 +++++++++
 .../renderer/__tests__/data/extrawhitespace.expect  |  8 ++++++++
 .../renderer/__tests__/data/extrawhitespace.txt     |  3 +++
 5 files changed, 28 insertions(+), 9 deletions(-)
 create mode 100644 src/lint/renderer/__tests__/data/extrawhitespace.expect
 create mode 100644 src/lint/renderer/__tests__/data/extrawhitespace.txt

diff --git a/src/lint/linter/ArcanistTextLinter.php b/src/lint/linter/ArcanistTextLinter.php
index 87a2d8a7..c7c4af52 100644
--- a/src/lint/linter/ArcanistTextLinter.php
+++ b/src/lint/linter/ArcanistTextLinter.php
@@ -311,9 +311,7 @@ final class ArcanistTextLinter extends ArcanistLinter {
     $this->raiseLintAtOffset(
       $offset,
       self::LINT_EOF_WHITESPACE,
-      pht(
-        'This file contains trailing whitespace at the end of the file. '.
-        'This is unnecessary and should be avoided when possible.'),
+      pht('This file contains unnecessary trailing whitespace.'),
       $string,
       '');
   }
diff --git a/src/lint/renderer/ArcanistConsoleLintRenderer.php b/src/lint/renderer/ArcanistConsoleLintRenderer.php
index 12ef4e69..ea233e3d 100644
--- a/src/lint/renderer/ArcanistConsoleLintRenderer.php
+++ b/src/lint/renderer/ArcanistConsoleLintRenderer.php
@@ -118,7 +118,6 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer {
       // or removing entire lines, but we'll adjust things below.
       $new_impact = substr_count($replacement, "\n") + 1;
 
-
       // If this is a change on a single line, we'll try to highlight the
       // changed character range to make it easier to pick out.
       if ($old_impact === 1 && $new_impact === 1) {
@@ -135,11 +134,13 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer {
           strlen($replacement));
       }
 
-
       // If lines at the beginning of the changed line range are actually the
       // same, shrink the range. This happens when a patch just adds a line.
       do {
-        if ($old_lines[$start - 1] != $new_lines[$start - 1]) {
+        $old_line = idx($old_lines, $start - 1, null);
+        $new_line = idx($new_lines, $start - 1, null);
+
+        if ($old_line !== $new_line) {
           break;
         }
 
@@ -161,10 +162,10 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer {
       // same, shrink the range. This happens when a patch just removes a
       // line.
       do {
-        $old_suffix = $old_lines[$start + $old_impact - 2];
-        $new_suffix = $new_lines[$start + $new_impact - 2];
+        $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;
         }
 
diff --git a/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php b/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php
index f36b30e7..9014288b 100644
--- a/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php
+++ b/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php
@@ -113,6 +113,13 @@ EOTEXT;
         'original' => $remline_original,
         'replacement' => $remline_replacement,
       ),
+
+      'extrawhitespace' => array(
+        'line' => 2,
+        'char' => 1,
+        'original' => "\n",
+        'replacement' => '',
+      ),
     );
 
     $defaults = array(
@@ -125,6 +132,8 @@ 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;
diff --git a/src/lint/renderer/__tests__/data/extrawhitespace.expect b/src/lint/renderer/__tests__/data/extrawhitespace.expect
new file mode 100644
index 00000000..f083852d
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/extrawhitespace.expect
@@ -0,0 +1,8 @@
+>>> Lint for path/to/example.c:
+
+
+   Warning  (WARN123) Lint Warning
+    Consider this.
+
+               1 Adrift upon the sea.
+    >>> -      2 ~
diff --git a/src/lint/renderer/__tests__/data/extrawhitespace.txt b/src/lint/renderer/__tests__/data/extrawhitespace.txt
new file mode 100644
index 00000000..ba587d4c
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/extrawhitespace.txt
@@ -0,0 +1,3 @@
+Adrift upon the sea.
+
+~