Page MenuHomeSealhub
Files F10360382

No OneTemporary
Actions

View Options

diff --git a/src/lint/renderer/ArcanistConsoleLintRenderer.php b/src/lint/renderer/ArcanistConsoleLintRenderer.php
index a4dfffa9..4862f47f 100644
--- a/src/lint/renderer/ArcanistConsoleLintRenderer.php
+++ b/src/lint/renderer/ArcanistConsoleLintRenderer.php
@@ -1,332 +1,331 @@
<?php
/**
* Shows lint messages to the user.
*/
final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer {
private $showAutofixPatches = false;
private $testableMode;
public function setShowAutofixPatches($show_autofix_patches) {
$this->showAutofixPatches = $show_autofix_patches;
return $this;
}
public function setTestableMode($testable_mode) {
$this->testableMode = $testable_mode;
return $this;
}
public function getTestableMode() {
return $this->testableMode;
}
public function renderLintResult(ArcanistLintResult $result) {
$messages = $result->getMessages();
$path = $result->getPath();
$data = $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, $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,
$data,
array $line_map) {
$context = 3;
$message = $message->newTrimmedMessage();
$original = $message->getOriginalText();
$replacement = $message->getReplacementText();
$line = $message->getLine();
$char = $message->getChar();
$old = $data;
$old_lines = phutil_split_lines($old);
$old_impact = substr_count($original, "\n") + 1;
$start = $line;
if ($message->isPatchable()) {
$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.
$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) {
$old_lines[$start - 1] = substr_replace(
$old_lines[$start - 1],
$this->highlightText($original),
$char - 1,
strlen($original));
$new_lines[$start - 1] = substr_replace(
$new_lines[$start - 1],
$this->highlightText($replacement),
$char - 1,
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 {
$old_line = idx($old_lines, $start - 1, null);
$new_line = idx($new_lines, $start - 1, null);
if ($old_line !== $new_line) {
break;
}
$start++;
$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));
+ // We can end up here if a patch removes a line which occurs before
+ // another identical line.
+ if ($old_impact <= 0 || $new_impact <= 0) {
+ break;
}
} 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 {
- $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) {
- break;
- }
-
- $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);
+ if ($old_impact > 0 && $new_impact > 0) {
+ do {
+ $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) {
+ break;
+ }
+
+ $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);
+ }
} else {
// If we have "original" text and it is contained on a single line,
// highlight the affected area. If we don't have any text, we'll mark
// the character with a caret (below, in rendering) instead.
if ($old_impact == 1 && strlen($original)) {
$old_lines[$start - 1] = substr_replace(
$old_lines[$start - 1],
$this->highlightText($original),
$char - 1,
strlen($original));
}
$old_impact = 0;
$new_impact = 0;
}
$out = array();
$head = max(1, $start - $context);
for ($ii = $head; $ii < $start; $ii++) {
$out[] = array(
'text' => $old_lines[$ii - 1],
'number' => $ii,
);
}
for ($ii = $start; $ii < $start + $old_impact; $ii++) {
$out[] = array(
'text' => $old_lines[$ii - 1],
'number' => $ii,
'type' => '-',
'chevron' => ($ii == $start),
);
}
for ($ii = $start; $ii < $start + $new_impact; $ii++) {
// If the patch was at the end of the file and ends with a newline, we
// won't have an actual entry in the array for the last line, even though
// we want to show it in the diff.
$out[] = array(
'text' => idx($new_lines, $ii - 1, ''),
'type' => '+',
'chevron' => ($ii == $start),
);
}
$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),
);
}
$result = array();
$seen_chevron = false;
foreach ($out as $spec) {
if ($seen_chevron) {
$chevron = false;
} else {
$chevron = !empty($spec['chevron']);
if ($chevron) {
$seen_chevron = true;
}
}
// If the line doesn't actually end in a newline, add one so the layout
// doesn't mess up. This can happen when the last line of the old file
// didn't have a newline at the end.
$text = $spec['text'];
if (!preg_match('/\n\z/', $spec['text'])) {
$text .= "\n";
}
$result[] = $this->renderLine(
idx($spec, 'number'),
$text,
$chevron,
idx($spec, 'type'));
// If this is just a message and does not have a patch, put a little
// caret underneath the line to point out where the issue is.
if ($chevron) {
if (!$message->isPatchable() && !strlen($original)) {
$result[] = $this->renderCaret($char)."\n";
}
}
}
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);
}
// If the last line ends in a newline, add a virtual offset for the final
// line with no characters on it. This allows lint messages to target the
// last line of the file at character 1.
if ($lines) {
if (preg_match('/\n\z/', $line)) {
$line_map[$number] = $offset;
}
}
return $line_map;
}
private function highlightText($text) {
if ($this->getTestableMode()) {
return '>'.$text.'<';
} else {
return (string)tsprintf('##%s##', $text);
}
}
}
diff --git a/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php b/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php
index f8928229..f65e40c9 100644
--- a/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php
+++ b/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php
@@ -1,227 +1,241 @@
<?php
final class ArcanistConsoleLintRendererTestCase
extends PhutilTestCase {
public function testRendering() {
$midline_original = <<<EOTEXT
import apple;
import banana;
import cat;
import dog;
EOTEXT;
$midline_replacement = <<<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;
$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",
),
'caret' => array(
'line' => 2,
'char' => 13,
'name' => 'Fruit Misinformation',
'description' => 'Arguably untrue.',
),
'original' => array(
'line' => 1,
'char' => 4,
'original' => 'should of',
),
'midline' => array(
'line' => 1,
'char' => 1,
'original' => $midline_original,
'replacement' => $midline_replacement,
),
'remline' => array(
'line' => 1,
'char' => 1,
'original' => $remline_original,
'replacement' => $remline_replacement,
),
'extrawhitespace' => array(
'line' => 2,
'char' => 1,
'original' => "\n",
'replacement' => '',
),
'eofnewline' => array(
'line' => 1,
'char' => 7,
'original' => '',
'replacement' => "\n",
),
'eofmultilinechar' => array(
'line' => 5,
'char' => 3,
'original' => '',
'replacement' => "\nX\nY\n",
),
'eofmultilineline' => array(
'line' => 6,
'char' => 1,
'original' => '',
'replacement' => "\nX\nY\n",
),
+ 'rmmulti' => array(
+ 'line' => 2,
+ 'char' => 1,
+ 'original' => "\n",
+ 'replacement' => '',
+ ),
+
+ 'rmmulti2' => array(
+ 'line' => 1,
+ 'char' => 2,
+ 'original' => "\n",
+ 'replacement' => '',
+ ),
+
);
$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 = id(new ArcanistConsoleLintRenderer())
->setTestableMode(true);
try {
PhutilConsoleFormatter::disableANSI(true);
$actual = $renderer->renderLintResult($result);
PhutilConsoleFormatter::disableANSI(false);
} catch (Exception $ex) {
PhutilConsoleFormatter::disableANSI(false);
throw $ex;
}
$this->assertEqual(
$expect,
$actual,
pht(
'Lint rendering for "%s".',
$key));
}
}
private function readTestData($filename) {
$path = dirname(__FILE__).'/data/'.$filename;
$data = Filesystem::readFile($path);
// If we find "~~~" at the end of the file, get rid of it and any whitespace
// afterwards. This allows specifying data files with trailing empty
// lines.
$data = preg_replace('/~~~\s*\z/', '', $data);
// Trim "~" off the ends of lines. This allows the "expect" file to test
// for trailing whitespace without actually containing trailing
// whitespace.
$data = preg_replace('/~$/m', '', $data);
return $data;
}
}
diff --git a/src/lint/renderer/__tests__/data/rmmulti.expect b/src/lint/renderer/__tests__/data/rmmulti.expect
new file mode 100644
index 00000000..1e4518cf
--- /dev/null
+++ b/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
diff --git a/src/lint/renderer/__tests__/data/rmmulti.txt b/src/lint/renderer/__tests__/data/rmmulti.txt
new file mode 100644
index 00000000..82cb3214
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/rmmulti.txt
@@ -0,0 +1,4 @@
+A
+
+
+B
diff --git a/src/lint/renderer/__tests__/data/rmmulti2.expect b/src/lint/renderer/__tests__/data/rmmulti2.expect
new file mode 100644
index 00000000..cd49440f
--- /dev/null
+++ 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
diff --git a/src/lint/renderer/__tests__/data/rmmulti2.txt b/src/lint/renderer/__tests__/data/rmmulti2.txt
new file mode 100644
index 00000000..82cb3214
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/rmmulti2.txt
@@ -0,0 +1,4 @@
+A
+
+
+B

File Metadata

Mime Type
text/x-diff
Expires
Sat, Nov 8, 06:31 (1 d, 13 h)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
1034158
Default Alt Text
(17 KB)

Event Timeline