(stable) Promote 2017 Week 36

src/lint/ArcanistLintMessage.php

@@ -302,8 +302,13 @@ final class ArcanistLintMessage extends Phobject {
     // "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 @@ final class ArcanistLintMessage extends Phobject {
     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

src/lint/__tests__/ArcanistLintMessageTestCase.php

@@ -45,6 +45,14 @@ final class ArcanistLintMessageTestCase
         '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) {

src/lint/renderer/ArcanistConsoleLintRenderer.php

@@ -171,13 +171,10 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer {
         $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));
+        // 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);
 

src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php

@@ -4,6 +4,38 @@ 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,
@@ -67,6 +99,20 @@ final class ArcanistConsoleLintRendererTestCase
         '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,
+      ),
     );
 
     $defaults = array(

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;

src/lint/renderer/__tests__/data/midline.txt

@@ -0,0 +1,4 @@
+import apple;
+import banana;
+import cat;
+import dog;

src/lint/renderer/__tests__/data/remline.expect

@@ -0,0 +1,12 @@
+>>> Lint for path/to/example.c:
+
+
+   Warning  (WARN123) Lint Warning
+    Consider this.
+
+               1 import apple;
+               2 import banana;
+               3 ~
+    >>> -      4 ~
+               5 import cat;
+               6 import dog;

src/lint/renderer/__tests__/data/remline.txt

@@ -0,0 +1,6 @@
+import apple;
+import banana;
+
+
+import cat;
+import dog;

src/workflow/ArcanistVersionWorkflow.php

@@ -51,8 +51,14 @@ EOTEXT
           pht('%s is not a git working copy.', $lib));
       }
 
-      list($stdout) = $repository->execxLocal('log -1 --format=%s', '%H %ct');
-      list($commit, $timestamp) = explode(' ', $stdout);
+      // NOTE: Carefully execute these commands in a way that works on Windows
+      // until T8298 is properly fixed. See PHI52.
+
+      list($commit) = $repository->execxLocal('log -1 --format=%%H');
+      $commit = trim($commit);
+
+      list($timestamp) = $repository->execxLocal('log -1 --format=%%ct');
+      $timestamp = trim($timestamp);
 
       $console->writeOut(
         "%s %s (%s)\n",