Page MenuHomePhabricator
Files F15338489

D18538.diff
No OneTemporary
Actions

D18538.diff
View Options

diff --git a/src/lint/ArcanistLintMessage.php b/src/lint/ArcanistLintMessage.php
--- a/src/lint/ArcanistLintMessage.php
+++ b/src/lint/ArcanistLintMessage.php
@@ -302,8 +302,13 @@
// "patchable" if they are, so we don't need a special check for the case
// where the entire string is a shared prefix.
+ // However, if the two strings are in the form "ABC" and "ABBC", we may
+ // find a prefix and a suffix with a combined length greater than the
+ // total size of the smaller string if we don't limit the search.
+ $max_suffix = ($minimum_length - $prefix_length);
+
$suffix_length = 0;
- for ($ii = 1; $ii <= $minimum_length; $ii++) {
+ for ($ii = 1; $ii <= $max_suffix; $ii++) {
$original_char = $original[$original_length - $ii];
$replacement_char = $replacement[$replacement_length - $ii];
if ($original_char !== $replacement_char) {
@@ -323,8 +328,11 @@
if ($prefix_length) {
$prefix = substr($original, 0, $prefix_length);
- $original = substr($original, $prefix_length);
- $replacement = substr($replacement, $prefix_length);
+ // NOTE: Prior to PHP7, `substr("a", 1)` returned false instead of
+ // the empty string. Cast these to force the PHP7-ish behavior we
+ // expect.
+ $original = (string)substr($original, $prefix_length);
+ $replacement = (string)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
diff --git a/src/lint/__tests__/ArcanistLintMessageTestCase.php b/src/lint/__tests__/ArcanistLintMessageTestCase.php
--- a/src/lint/__tests__/ArcanistLintMessageTestCase.php
+++ b/src/lint/__tests__/ArcanistLintMessageTestCase.php
@@ -45,6 +45,14 @@
'line' => 2,
'char' => 5,
),
+ 'mid-newline' => array(
+ 'old' => 'ABA',
+ 'new' => 'ABBA',
+ 'old.expect' => '',
+ 'new.expect' => 'B',
+ 'line' => 1,
+ 'char' => 3,
+ ),
);
foreach ($map as $key => $test_case) {
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
@@ -4,6 +4,21 @@
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;
+
$map = array(
'simple' => array(
'line' => 1,
@@ -67,6 +82,13 @@
'char' => 4,
'original' => 'should of',
),
+
+ 'midline' => array(
+ 'line' => 1,
+ 'char' => 1,
+ 'original' => $midline_original,
+ 'replacement' => $midline_replacement,
+ ),
);
$defaults = array(
diff --git a/src/lint/renderer/__tests__/data/midline.expect b/src/lint/renderer/__tests__/data/midline.expect
new file mode 100644
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/midline.expect
@@ -0,0 +1,11 @@
+>>> Lint for path/to/example.c:
+
+
+ Warning (WARN123) Lint Warning
+ Consider this.
+
+ 1 import apple;
+ 2 import banana;
+ >>> + ~
+ 3 import cat;
+ 4 import dog;
diff --git a/src/lint/renderer/__tests__/data/midline.txt b/src/lint/renderer/__tests__/data/midline.txt
new file mode 100644
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/midline.txt
@@ -0,0 +1,4 @@
+import apple;
+import banana;
+import cat;
+import dog;

File Metadata

Mime Type
text/plain
Expires
Mar 10 2025, 11:03 AM (4 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7431698
Default Alt Text
D18538.diff (3 KB)

Event Timeline

Phabricator · Built by Phacility