Page MenuHomePhabricator
Files F13198498

D18509.id.diff
No OneTemporary
Actions

D18509.id.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -222,6 +222,7 @@
'ArcanistLibraryTestCase' => '__tests__/ArcanistLibraryTestCase.php',
'ArcanistLintEngine' => 'lint/engine/ArcanistLintEngine.php',
'ArcanistLintMessage' => 'lint/ArcanistLintMessage.php',
+ 'ArcanistLintMessageTestCase' => 'lint/__tests__/ArcanistLintMessageTestCase.php',
'ArcanistLintPatcher' => 'lint/ArcanistLintPatcher.php',
'ArcanistLintRenderer' => 'lint/renderer/ArcanistLintRenderer.php',
'ArcanistLintResult' => 'lint/ArcanistLintResult.php',
@@ -638,6 +639,7 @@
'ArcanistLibraryTestCase' => 'PhutilLibraryTestCase',
'ArcanistLintEngine' => 'Phobject',
'ArcanistLintMessage' => 'Phobject',
+ 'ArcanistLintMessageTestCase' => 'PhutilTestCase',
'ArcanistLintPatcher' => 'Phobject',
'ArcanistLintRenderer' => 'Phobject',
'ArcanistLintResult' => 'Phobject',
diff --git a/src/lint/ArcanistLintMessage.php b/src/lint/ArcanistLintMessage.php
--- a/src/lint/ArcanistLintMessage.php
+++ b/src/lint/ArcanistLintMessage.php
@@ -274,4 +274,75 @@
return $value;
}
+ public function newTrimmedMessage() {
+ if (!$this->isPatchable()) {
+ return clone $this;
+ }
+
+ // If the original and replacement text have a similar prefix or suffix,
+ // we trim it to reduce the size of the diff we show to the user.
+
+ $replacement = $this->getReplacementText();
+ $original = $this->getOriginalText();
+
+ $replacement_length = strlen($replacement);
+ $original_length = strlen($original);
+
+ $minimum_length = min($original_length, $replacement_length);
+
+ $prefix_length = 0;
+ for ($ii = 0; $ii < $minimum_length; $ii++) {
+ if ($original[$ii] !== $replacement[$ii]) {
+ break;
+ }
+ $prefix_length++;
+ }
+
+ // NOTE: The two strings can't be the same because the message won't be
+ // "patchable" if they are, so we don't need a special check for the case
+ // where the entire string is a shared prefix.
+
+ $suffix_length = 0;
+ for ($ii = 1; $ii <= $minimum_length; $ii++) {
+ $original_char = $original[$original_length - $ii];
+ $replacement_char = $replacement[$replacement_length - $ii];
+ if ($original_char !== $replacement_char) {
+ break;
+ }
+ $suffix_length++;
+ }
+
+ if ($suffix_length) {
+ $original = substr($original, 0, -$suffix_length);
+ $replacement = substr($replacement, 0, -$suffix_length);
+ }
+
+ $line = $this->getLine();
+ $char = $this->getChar();
+
+ if ($prefix_length) {
+ $prefix = substr($original, 0, $prefix_length);
+
+ $original = substr($original, $prefix_length);
+ $replacement = substr($replacement, $prefix_length);
+
+ // If we've removed a prefix, we need to push the character and line
+ // number for the warning forward to account for the characters we threw
+ // away.
+ for ($ii = 0; $ii < $prefix_length; $ii++) {
+ $char++;
+ if ($prefix[$ii] == "\n") {
+ $line++;
+ $char = 1;
+ }
+ }
+ }
+
+ return id(clone $this)
+ ->setOriginalText($original)
+ ->setReplacementText($replacement)
+ ->setLine($line)
+ ->setChar($char);
+ }
+
}
diff --git a/src/lint/__tests__/ArcanistLintMessageTestCase.php b/src/lint/__tests__/ArcanistLintMessageTestCase.php
new file mode 100644
--- /dev/null
+++ b/src/lint/__tests__/ArcanistLintMessageTestCase.php
@@ -0,0 +1,80 @@
+<?php
+
+final class ArcanistLintMessageTestCase
+ extends PhutilTestCase {
+
+ public function testMessageTrimming() {
+ $map = array(
+ 'simple' => array(
+ 'old' => 'a',
+ 'new' => 'b',
+ 'old.expect' => 'a',
+ 'new.expect' => 'b',
+ 'line' => 1,
+ 'char' => 1,
+ ),
+ 'prefix' => array(
+ 'old' => 'ever after',
+ 'new' => 'evermore',
+ 'old.expect' => ' after',
+ 'new.expect' => 'more',
+ 'line' => 1,
+ 'char' => 5,
+ ),
+ 'suffix' => array(
+ 'old' => 'arcane archaeology',
+ 'new' => 'mythic archaeology',
+ 'old.expect' => 'arcane',
+ 'new.expect' => 'mythic',
+ 'line' => 1,
+ 'char' => 1,
+ ),
+ 'both' => array(
+ 'old' => 'large red apple',
+ 'new' => 'large blue apple',
+ 'old.expect' => 'red',
+ 'new.expect' => 'blue',
+ 'line' => 1,
+ 'char' => 7,
+ ),
+ 'prefix-newline' => array(
+ 'old' => "four score\nand five years ago",
+ 'new' => "four score\nand seven years ago",
+ 'old.expect' => 'five',
+ 'new.expect' => 'seven',
+ 'line' => 2,
+ 'char' => 5,
+ ),
+ );
+
+ foreach ($map as $key => $test_case) {
+ $message = id(new ArcanistLintMessage())
+ ->setOriginalText($test_case['old'])
+ ->setReplacementText($test_case['new'])
+ ->setLine(1)
+ ->setChar(1);
+
+ $actual = $message->newTrimmedMessage();
+
+ $this->assertEqual(
+ $test_case['old.expect'],
+ $actual->getOriginalText(),
+ pht('Original text for "%s".', $key));
+
+ $this->assertEqual(
+ $test_case['new.expect'],
+ $actual->getReplacementText(),
+ pht('Replacement text for "%s".', $key));
+
+ $this->assertEqual(
+ $test_case['line'],
+ $actual->getLine(),
+ pht('Line for "%s".', $key));
+
+ $this->assertEqual(
+ $test_case['char'],
+ $actual->getChar(),
+ pht('Char for "%s".', $key));
+ }
+ }
+}
diff --git a/src/lint/linter/__tests__/xml/attr2.lint-test b/src/lint/linter/__tests__/xml/attr2.lint-test
deleted file mode 100644
--- a/src/lint/linter/__tests__/xml/attr2.lint-test
+++ /dev/null
@@ -1,4 +0,0 @@
-<foo foo=">oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo
-~~~~~~~~~~
-error:1:2
-error:2:1
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
@@ -17,6 +17,15 @@
'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',
+ ),
);
$defaults = array(
diff --git a/src/lint/renderer/__tests__/data/overlap.expect b/src/lint/renderer/__tests__/data/overlap.expect
new file mode 100644
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/overlap.expect
@@ -0,0 +1,8 @@
+>>> Lint for path/to/example.c:
+
+
+ Warning (WARN123) Lint Warning
+ Consider this.
+
+ >>> - 1 tantawount
+ + tantamount
diff --git a/src/lint/renderer/__tests__/data/overlap.txt b/src/lint/renderer/__tests__/data/overlap.txt
new file mode 100644
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/overlap.txt
@@ -0,0 +1 @@
+tantawount

File Metadata

Mime Type
text/plain
Expires
Tue, May 14, 7:17 AM (2 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6277628
Default Alt Text
D18509.id.diff (12 KB)

Event Timeline

Phabricator · Built by Phacility