Page MenuHomePhorge
Files F2890195

No OneTemporary
Actions

View Options

diff --git a/src/lint/renderer/ArcanistConsoleLintRenderer.php b/src/lint/renderer/ArcanistConsoleLintRenderer.php
index 6f3476b0..20e79b2f 100644
--- a/src/lint/renderer/ArcanistConsoleLintRenderer.php
+++ b/src/lint/renderer/ArcanistConsoleLintRenderer.php
@@ -1,248 +1,259 @@
<?php
/**
* Shows lint messages to the user.
*/
final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer {
private $showAutofixPatches = false;
public function setShowAutofixPatches($show_autofix_patches) {
$this->showAutofixPatches = $show_autofix_patches;
return $this;
}
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;
}
if ($message->isError()) {
$color = 'red';
} else {
$color = 'yellow';
}
$severity = ArcanistLintSeverity::getStringForSeverity(
$message->getSeverity());
$code = $message->getCode();
$name = $message->getName();
$description = $message->getDescription();
if ($message->getOtherLocations()) {
$locations = array();
foreach ($message->getOtherLocations() as $location) {
$locations[] =
idx($location, 'path', $path).
(!empty($location['line']) ? ":{$location['line']}" : '');
}
$description .= "\n".pht(
'Other locations: %s',
implode(', ', $locations));
}
$text[] = phutil_console_format(
" **<bg:{$color}> %s </bg>** (%s) __%s__\n%s\n",
$severity,
$code,
$name,
phutil_console_wrap($description, 4));
if ($message->hasFileContext()) {
- $text[] = $this->renderContext($message, $lines);
+ $text[] = $this->renderContext($message, $data, $line_map);
}
}
if ($text) {
$prefix = phutil_console_format(
"**>>>** %s\n\n\n",
pht(
'Lint for %s:',
phutil_console_format('__%s__', $path)));
return $prefix.implode("\n", $text);
} else {
return null;
}
}
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) {
return str_repeat(' ', 16 + $pos).'^';
}
protected function renderLine($line, $data, $chevron = false, $diff = null) {
$chevron = $chevron ? '>>>' : '';
return sprintf(
' %3s %1s %6s %s',
$chevron,
$diff,
$line,
$data);
}
public function renderOkayResult() {
return phutil_console_format(
"<bg:green>** %s **</bg> %s\n",
pht('OKAY'),
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
index 320c00fb..7f39d352 100644
--- a/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php
+++ b/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php
@@ -1,98 +1,131 @@
<?php
final class ArcanistConsoleLintRendererTestCase
extends PhutilTestCase {
public function testRendering() {
$map = array(
'simple' => array(
'line' => 1,
'char' => 1,
'original' => 'a',
'replacement' => 'z',
),
'inline' => array(
'line' => 1,
'char' => 7,
'original' => 'cat',
'replacement' => 'dog',
),
// In this test, the original and replacement texts have a large
// amount of overlap.
'overlap' => array(
'line' => 1,
'char' => 1,
'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(
'severity' => ArcanistLintSeverity::SEVERITY_WARNING,
'name' => 'Lint Warning',
'path' => 'path/to/example.c',
'description' => 'Consider this.',
'code' => 'WARN123',
);
foreach ($map as $key => $test_case) {
$data = $this->readTestData("{$key}.txt");
$expect = $this->readTestData("{$key}.expect");
$test_case = $test_case + $defaults;
$path = $test_case['path'];
$severity = $test_case['severity'];
$name = $test_case['name'];
$description = $test_case['description'];
$code = $test_case['code'];
$line = $test_case['line'];
$char = $test_case['char'];
$original = idx($test_case, 'original');
$replacement = idx($test_case, 'replacement');
$message = id(new ArcanistLintMessage())
->setPath($path)
->setSeverity($severity)
->setName($name)
->setDescription($description)
->setCode($code)
->setLine($line)
->setChar($char)
->setOriginalText($original)
->setReplacementText($replacement);
$result = id(new ArcanistLintResult())
->setPath($path)
->setData($data)
->addMessage($message);
$renderer = new ArcanistConsoleLintRenderer();
try {
PhutilConsoleFormatter::disableANSI(true);
$actual = $renderer->renderLintResult($result);
PhutilConsoleFormatter::disableANSI(false);
} catch (Exception $ex) {
PhutilConsoleFormatter::disableANSI(false);
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,
pht(
'Lint rendering for "%s".',
$key));
}
}
private function readTestData($filename) {
$path = dirname(__FILE__).'/data/'.$filename;
return Filesystem::readFile($path);
}
}
diff --git a/src/lint/renderer/__tests__/data/addline.expect b/src/lint/renderer/__tests__/data/addline.expect
new file mode 100644
index 00000000..44aa945c
--- /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
index 00000000..07147513
--- /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
index 00000000..44aa945c
--- /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
index 00000000..07147513
--- /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
index 00000000..0533707c
--- /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
index 00000000..830c73a1
--- /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
index 00000000..767ea655
--- /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 <xml />
diff --git a/src/lint/renderer/__tests__/data/xml.txt b/src/lint/renderer/__tests__/data/xml.txt
new file mode 100644
index 00000000..136a688e
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/xml.txt
@@ -0,0 +1,4 @@
+<
+ wow
+ xml>
+<xml />

File Metadata

Mime Type
text/x-diff
Expires
Sun, Jan 19, 13:11 (3 w, 3 d ago)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
1124942
Default Alt Text
(18 KB)

Event Timeline

Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0