diff --git a/src/lint/renderer/ArcanistConsoleLintRenderer.php b/src/lint/renderer/ArcanistConsoleLintRenderer.php
--- a/src/lint/renderer/ArcanistConsoleLintRenderer.php
+++ b/src/lint/renderer/ArcanistConsoleLintRenderer.php
@@ -15,11 +15,11 @@
public function renderLintResult(ArcanistLintResult $result) {
$messages = $result->getMessages();
$path = $result->getPath();
+ $data = $result->getData();
- $lines = explode("\n", $result->getData());
+ $line_map = $this->newOffsetMap($data);
$text = array();
-
foreach ($messages as $message) {
if (!$this->showAutofixPatches && $message->isAutofix()) {
continue;
@@ -57,7 +57,7 @@
phutil_console_wrap($description, 4));
if ($message->hasFileContext()) {
- $text[] = $this->renderContext($message, $lines);
+ $text[] = $this->renderContext($message, $data, $line_map);
}
}
@@ -75,153 +75,148 @@
protected function renderContext(
ArcanistLintMessage $message,
- array $line_data) {
+ $data,
+ array $line_map) {
- $lines_of_context = 3;
- $out = array();
+ $context = 3;
- $num_lines = count($line_data);
- // make line numbers line up with array indexes
- array_unshift($line_data, '');
+ $message = $message->newTrimmedMessage();
- $line_num = min($message->getLine(), $num_lines);
- $line_num = max(1, $line_num);
+ $original = $message->getOriginalText();
+ $replacement = $message->getReplacementText();
- // Print out preceding context before the impacted region.
- $cursor = max(1, $line_num - $lines_of_context);
- for (; $cursor < $line_num; $cursor++) {
- $out[] = $this->renderLine($cursor, $line_data[$cursor]);
- }
+ $line = $message->getLine();
+ $char = $message->getChar();
+
+ $old = $data;
+ $old_lines = phutil_split_lines($old);
- $text = $message->getOriginalText();
- $start = $message->getChar() - 1;
- $patch = '';
- // Refine original and replacement text to eliminate start and end in common
if ($message->isPatchable()) {
- $patch = $message->getReplacementText();
- $text_strlen = strlen($text);
- $patch_strlen = strlen($patch);
- $min_length = min($text_strlen, $patch_strlen);
-
- $same_at_front = 0;
- for ($ii = 0; $ii < $min_length; $ii++) {
- if ($text[$ii] !== $patch[$ii]) {
+ $patch_offset = $line_map[$line] + ($char - 1);
+
+ $new = substr_replace(
+ $old,
+ $replacement,
+ $patch_offset,
+ strlen($original));
+ $new_lines = phutil_split_lines($new);
+
+ // Figure out how many "-" and "+" lines we have by counting the newlines
+ // for the relevant patches. This may overestimate things if we are adding
+ // or removing entire lines, but we'll adjust things below.
+ $old_impact = substr_count($original, "\n") + 1;
+ $new_impact = substr_count($replacement, "\n") + 1;
+
+ $start = $line;
+
+ // 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]) {
break;
}
- $same_at_front++;
+
$start++;
- if ($text[$ii] == "\n") {
- $out[] = $this->renderLine($cursor, $line_data[$cursor]);
- $cursor++;
- $start = 0;
- $line_num++;
+ $old_impact--;
+ $new_impact--;
+
+ if ($old_impact < 0 || $new_impact < 0) {
+ throw new Exception(
+ pht(
+ 'Modified prefix line range has become negative '.
+ '(old = %d, new = %d).',
+ $old_impact,
+ $new_impact));
}
- }
- // deal with shorter string ' ' longer string ' a '
- $min_length -= $same_at_front;
+ } while (true);
- // And check the end of the string
- $same_at_end = 0;
- for ($ii = 1; $ii <= $min_length; $ii++) {
- if ($text[$text_strlen - $ii] !== $patch[$patch_strlen - $ii]) {
+ // 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 {
+ $old_suffix = $old_lines[$start + $old_impact - 2];
+ $new_suffix = $new_lines[$start + $new_impact - 2];
+
+ if ($old_suffix != $new_suffix) {
break;
}
- $same_at_end++;
- }
- $text = substr(
- $text,
- $same_at_front,
- $text_strlen - $same_at_end - $same_at_front);
- $patch = substr(
- $patch,
- $same_at_front,
- $patch_strlen - $same_at_end - $same_at_front);
- }
- // Print out the impacted region itself.
- $diff = $message->isPatchable() ? '-' : null;
-
- $text_lines = explode("\n", $text);
- $text_length = count($text_lines);
-
- $intraline = ($text != '' || $start || !preg_match('/\n$/', $patch));
-
- if ($intraline) {
- for (; $cursor < $line_num + $text_length; $cursor++) {
- $chevron = ($cursor == $line_num);
- // We may not have any data if, e.g., the old file does not exist.
- $data = idx($line_data, $cursor, null);
-
- // Highlight the problem substring.
- $text_line = $text_lines[$cursor - $line_num];
- if (strlen($text_line)) {
- $data = substr_replace(
- $data,
- phutil_console_format('##%s##', $text_line),
- ($cursor == $line_num ? ($start > 0 ? $start : null) : 0),
- strlen($text_line));
+ $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));
}
+ } while (true);
- $out[] = $this->renderLine($cursor, $data, $chevron, $diff);
- }
+ } else {
+ $old_impact = 0;
+ $new_impact = 0;
}
- // Print out replacement text.
- if ($message->isPatchable()) {
- // Strip trailing newlines, since "explode" will create an extra patch
- // line for these.
- if (strlen($patch) && ($patch[strlen($patch) - 1] === "\n")) {
- $patch = substr($patch, 0, -1);
- }
- $patch_lines = explode("\n", $patch);
- $patch_length = count($patch_lines);
-
- $patch_line = $patch_lines[0];
-
- $len = isset($text_lines[0]) ? strlen($text_lines[0]) : 0;
+ $out = array();
- $patched = phutil_console_format('##%s##', $patch_line);
+ $head = max(1, $start - $context);
+ for ($ii = $head; $ii < $start; $ii++) {
+ $out[] = array(
+ 'text' => $old_lines[$ii - 1],
+ 'number' => $ii,
+ );
+ }
- if ($intraline) {
- $patched = substr_replace(
- $line_data[$line_num],
- $patched,
- $start,
- $len);
- }
+ for ($ii = $start; $ii < $start + $old_impact; $ii++) {
+ $out[] = array(
+ 'text' => $old_lines[$ii - 1],
+ 'number' => $ii,
+ 'type' => '-',
+ 'chevron' => ($ii == $start),
+ );
+ }
- $out[] = $this->renderLine(null, $patched, false, '+');
+ for ($ii = $start; $ii < $start + $new_impact; $ii++) {
+ $out[] = array(
+ 'text' => $new_lines[$ii - 1],
+ 'type' => '+',
+ 'chevron' => ($ii == $start),
+ );
+ }
- foreach (array_slice($patch_lines, 1) as $patch_line) {
- $out[] = $this->renderLine(
- null,
- phutil_console_format('##%s##', $patch_line), false, '+');
- }
+ $cursor = $start + $old_impact;
+ $foot = min(count($old_lines), $cursor + $context);
+ for ($ii = $cursor; $ii <= $foot; $ii++) {
+ $out[] = array(
+ 'text' => $old_lines[$ii - 1],
+ 'number' => $ii,
+ 'chevron' => ($ii == $cursor),
+ );
}
- $end = min($num_lines, $cursor + $lines_of_context);
- for (; $cursor < $end; $cursor++) {
- // If there is no original text, we didn't print out a chevron or any
- // highlighted text above, so print it out here. This allows messages
- // which don't have any original/replacement information to still
- // render with indicator chevrons.
- if ($text || $message->isPatchable()) {
+ $result = array();
+
+ $seen_chevron = false;
+ foreach ($out as $spec) {
+ if ($seen_chevron) {
$chevron = false;
} else {
- $chevron = ($cursor == $line_num);
+ $chevron = !empty($spec['chevron']);
+ if ($chevron) {
+ $seen_chevron = true;
+ }
}
- $out[] = $this->renderLine($cursor, $line_data[$cursor], $chevron);
- // With original text, we'll render the text highlighted above. If the
- // lint message only has a line/char offset there's nothing to
- // highlight, so print out a caret on the next line instead.
- if ($chevron && $message->getChar()) {
- $out[] = $this->renderCaret($message->getChar());
- }
+ $result[] = $this->renderLine(
+ idx($spec, 'number'),
+ $spec['text'],
+ $chevron,
+ idx($spec, 'type'));
}
- $out[] = null;
- return implode("\n", $out);
+ return implode('', $result);
}
private function renderCaret($pos) {
@@ -245,4 +240,20 @@
pht('No lint warnings.'));
}
+ private function newOffsetMap($data) {
+ $lines = phutil_split_lines($data);
+
+ $line_map = array();
+
+ $number = 1;
+ $offset = 0;
+ foreach ($lines as $line) {
+ $line_map[$number] = $offset;
+ $number++;
+ $offset += strlen($line);
+ }
+
+ return $line_map;
+ }
+
}
diff --git a/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php b/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php
--- a/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php
+++ b/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php
@@ -26,6 +26,34 @@
'original' => 'tantawount',
'replacement' => 'tantamount',
),
+
+ 'newline' => array(
+ 'line' => 6,
+ 'char' => 1,
+ 'original' => "\n",
+ 'replacement' => '',
+ ),
+
+ 'addline' => array(
+ 'line' => 3,
+ 'char' => 1,
+ 'original' => '',
+ 'replacement' => "cherry\n",
+ ),
+
+ 'addlinesuffix' => array(
+ 'line' => 2,
+ 'char' => 7,
+ 'original' => '',
+ 'replacement' => "\ncherry",
+ ),
+
+ 'xml' => array(
+ 'line' => 3,
+ 'char' => 6,
+ 'original' => '',
+ 'replacement' => "\n",
+ ),
);
$defaults = array(
@@ -81,6 +109,11 @@
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,
diff --git a/src/lint/renderer/__tests__/data/addline.expect b/src/lint/renderer/__tests__/data/addline.expect
new file mode 100644
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/addline.expect
@@ -0,0 +1,12 @@
+>>> Lint for path/to/example.c:
+
+
+ Warning (WARN123) Lint Warning
+ Consider this.
+
+ 1 apple
+ 2 banana
+ >>> + cherry
+ 3 date
+ 4 eclaire
+ 5 fig
diff --git a/src/lint/renderer/__tests__/data/addline.txt b/src/lint/renderer/__tests__/data/addline.txt
new file mode 100644
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/addline.txt
@@ -0,0 +1,5 @@
+apple
+banana
+date
+eclaire
+fig
diff --git a/src/lint/renderer/__tests__/data/addlinesuffix.expect b/src/lint/renderer/__tests__/data/addlinesuffix.expect
new file mode 100644
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/addlinesuffix.expect
@@ -0,0 +1,12 @@
+>>> Lint for path/to/example.c:
+
+
+ Warning (WARN123) Lint Warning
+ Consider this.
+
+ 1 apple
+ 2 banana
+ >>> + cherry
+ 3 date
+ 4 eclaire
+ 5 fig
diff --git a/src/lint/renderer/__tests__/data/addlinesuffix.txt b/src/lint/renderer/__tests__/data/addlinesuffix.txt
new file mode 100644
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/addlinesuffix.txt
@@ -0,0 +1,5 @@
+apple
+banana
+date
+eclaire
+fig
diff --git a/src/lint/renderer/__tests__/data/newline.expect b/src/lint/renderer/__tests__/data/newline.expect
new file mode 100644
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/newline.expect
@@ -0,0 +1,14 @@
+>>> Lint for path/to/example.c:
+
+
+ Warning (WARN123) Lint Warning
+ Consider this.
+
+ 3 ccc
+ 4 ddd
+ 5 eee
+ >>> - 6 ~
+ 7 fff
+ 8 ggg
+ 9 hhh
+ 10 iii
diff --git a/src/lint/renderer/__tests__/data/newline.txt b/src/lint/renderer/__tests__/data/newline.txt
new file mode 100644
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/newline.txt
@@ -0,0 +1,11 @@
+aaa
+bbb
+ccc
+ddd
+eee
+
+fff
+ggg
+hhh
+iii
+jjj
diff --git a/src/lint/renderer/__tests__/data/xml.expect b/src/lint/renderer/__tests__/data/xml.expect
new file mode 100644
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/xml.expect
@@ -0,0 +1,12 @@
+>>> Lint for path/to/example.c:
+
+
+ Warning (WARN123) Lint Warning
+ Consider this.
+
+ 1 <
+ 2 wow
+ >>> - 3 xml>
+ + xml
+ + >
+ 4
diff --git a/src/lint/renderer/__tests__/data/xml.txt b/src/lint/renderer/__tests__/data/xml.txt
new file mode 100644
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/xml.txt
@@ -0,0 +1,4 @@
+<
+ wow
+ xml>
+