Page MenuHomePhorge
Files F2890106

No OneTemporary
Actions

View Options

diff --git a/src/lint/renderer/ArcanistConsoleLintRenderer.php b/src/lint/renderer/ArcanistConsoleLintRenderer.php
index 4acbb644..8880fbe8 100644
--- a/src/lint/renderer/ArcanistConsoleLintRenderer.php
+++ b/src/lint/renderer/ArcanistConsoleLintRenderer.php
@@ -1,266 +1,314 @@
<?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.
- $old_impact = substr_count($original, "\n") + 1;
$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 {
if ($old_lines[$start - 1] != $new_lines[$start - 1]) {
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));
}
} 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 = $old_lines[$start + $old_impact - 2];
$new_suffix = $new_lines[$start + $new_impact - 2];
if ($old_suffix != $new_suffix) {
break;
}
$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);
} 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++) {
$out[] = array(
'text' => $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;
}
}
$result[] = $this->renderLine(
idx($spec, 'number'),
$spec['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);
}
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 693b1788..68a35420 100644
--- a/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php
+++ b/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php
@@ -1,138 +1,145 @@
<?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",
),
'caret' => array(
'line' => 2,
'char' => 13,
'name' => 'Fruit Misinformation',
'description' => 'Arguably untrue.',
),
+
+ 'original' => array(
+ 'line' => 1,
+ 'char' => 4,
+ 'original' => 'should of',
+ ),
);
$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();
+ $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;
}
// 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/inline.expect b/src/lint/renderer/__tests__/data/inline.expect
index eba4522b..d1d63fda 100644
--- a/src/lint/renderer/__tests__/data/inline.expect
+++ b/src/lint/renderer/__tests__/data/inline.expect
@@ -1,8 +1,8 @@
>>> Lint for path/to/example.c:
Warning (WARN123) Lint Warning
Consider this.
- >>> - 1 adjudicated
- + adjudidoged
+ >>> - 1 adjudi>cat<ed
+ + adjudi>dog<ed
diff --git a/src/lint/renderer/__tests__/data/overlap.expect b/src/lint/renderer/__tests__/data/original.expect
similarity index 61%
copy from src/lint/renderer/__tests__/data/overlap.expect
copy to src/lint/renderer/__tests__/data/original.expect
index 19de1c46..f3bec302 100644
--- a/src/lint/renderer/__tests__/data/overlap.expect
+++ b/src/lint/renderer/__tests__/data/original.expect
@@ -1,8 +1,7 @@
>>> Lint for path/to/example.c:
Warning (WARN123) Lint Warning
Consider this.
- >>> - 1 tantawount
- + tantamount
+ >>> 1 He >should of< known.
diff --git a/src/lint/renderer/__tests__/data/original.txt b/src/lint/renderer/__tests__/data/original.txt
new file mode 100644
index 00000000..090fe16d
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/original.txt
@@ -0,0 +1 @@
+He should of known.
diff --git a/src/lint/renderer/__tests__/data/overlap.expect b/src/lint/renderer/__tests__/data/overlap.expect
index 19de1c46..d7756f3d 100644
--- a/src/lint/renderer/__tests__/data/overlap.expect
+++ b/src/lint/renderer/__tests__/data/overlap.expect
@@ -1,8 +1,8 @@
>>> Lint for path/to/example.c:
Warning (WARN123) Lint Warning
Consider this.
- >>> - 1 tantawount
- + tantamount
+ >>> - 1 tanta>w<ount
+ + tanta>m<ount
diff --git a/src/lint/renderer/__tests__/data/simple.expect b/src/lint/renderer/__tests__/data/simple.expect
index 43b9910f..c363ba07 100644
--- a/src/lint/renderer/__tests__/data/simple.expect
+++ b/src/lint/renderer/__tests__/data/simple.expect
@@ -1,10 +1,10 @@
>>> Lint for path/to/example.c:
Warning (WARN123) Lint Warning
Consider this.
- >>> - 1 a
- + z
+ >>> - 1 >a<
+ + >z<
2 b
3 c

File Metadata

Mime Type
text/x-diff
Expires
Sun, Jan 19, 13:04 (3 w, 3 d ago)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
1124884
Default Alt Text
(14 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