Fix an issue with highlighting PHP snippets with leading whitespace using XHPAST

Summary:
Ref T10576. In the test case there, XHPAST has incorrectly emitted an extra newline at the top of the output.

This cascades downward and throws every line off by one, highlighting the wrong lines in the output.

Fix this issue by stripping the prefix more carefully, so the input source is preserved accurately.

Test Plan:
  - Added tests; ran tests.
  - Verified that the snippet now highlights correctly.

{F1173155}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10576

Differential Revision: https://secure.phabricator.com/D15469

Author: epriestley

.arclint

@@ -42,7 +42,19 @@
       "type": "spelling"
     },
     "text": {
-      "type": "text"
+      "type": "text",
+      "exclude": [
+        "(^src/(.*/)?__tests__/[^/]+/.*\\.(txt|json|expect))"
+      ]
+    },
+    "text-without-length": {
+      "type": "text",
+      "include": [
+        "(^src/(.*/)?__tests__/[^/]+/.*\\.(txt|json|expect))"
+      ],
+      "severity": {
+        "3": "disabled"
+      }
     },
     "xhpast": {
       "type": "xhpast",

src/markup/syntax/highlighter/PhutilXHPASTSyntaxHighlighter.php

@@ -5,7 +5,7 @@ final class PhutilXHPASTSyntaxHighlighter extends Phobject {
   public function getHighlightFuture($source) {
     $scrub = false;
     if (strpos($source, '<?') === false) {
-      $source = "<?php\n".$source."\n";
+      $source = "<?php\n".$source;
       $scrub = true;
     }
 

src/markup/syntax/highlighter/__tests__/PhutilXHPASTSyntaxHighlighterTestCase.php

@@ -26,6 +26,14 @@ public function testBuiltinClassnames() {
       $this->read('multiline-token.expect'),
       (string)$this->highlight($this->read('multiline-token.source')),
       pht('Multi-line tokens should be split across lines.'));
+    $this->assertEqual(
+      $this->read('leading-whitespace.expect'),
+      (string)$this->highlight($this->read('leading-whitespace.source')),
+      pht('Snippets with leading whitespace should be preserved.'));
+    $this->assertEqual(
+      $this->read('no-leading-whitespace.expect'),
+      (string)$this->highlight($this->read('no-leading-whitespace.source')),
+      pht('Snippets with no leading whitespace should be preserved.'));
   }
 
 }

src/markup/syntax/highlighter/__tests__/phpfragment/leading-whitespace.expect

@@ -0,0 +1,3 @@
+    <span class="k">foreach</span> <span class="o">(</span><span class="nv">$x</span> <span class="k">as</span> <span class="nv">$y</span><span class="o">)</span> <span class="o">{</span>
+      <span class="nf" data-symbol-name="z">z</span><span class="o">();</span>
+    <span class="o">}</span>

src/markup/syntax/highlighter/__tests__/phpfragment/leading-whitespace.test

@@ -0,0 +1,3 @@
+    foreach ($x as $y) {
+      z();
+    }

src/markup/syntax/highlighter/__tests__/phpfragment/no-leading-whitespace.expect

@@ -0,0 +1,3 @@
+<span class="k">foreach</span> <span class="o">(</span><span class="nv">$x</span> <span class="k">as</span> <span class="nv">$y</span><span class="o">)</span> <span class="o">{</span>
+  <span class="nf" data-symbol-name="z">z</span><span class="o">();</span>
+<span class="o">}</span>

src/markup/syntax/highlighter/__tests__/phpfragment/no-leading-whitespace.test

@@ -0,0 +1,3 @@
+foreach ($x as $y) {
+  z();
+}

src/markup/syntax/highlighter/__tests__/xhpast/leading-whitespace.expect

@@ -0,0 +1,3 @@
+    <span class="k">foreach</span> <span class="k">(</span><span class="nv">$x</span> <span class="k">as</span> <span class="nv">$y</span><span class="k">)</span> <span class="k">{</span>
+      <span data-symbol-name="z" class="nf">z</span><span class="k">(</span><span class="k">)</span><span class="k">;</span>
+    <span class="k">}</span>

src/markup/syntax/highlighter/__tests__/xhpast/leading-whitespace.source

@@ -0,0 +1,3 @@
+    foreach ($x as $y) {
+      z();
+    }

src/markup/syntax/highlighter/__tests__/xhpast/no-leading-whitespace.expect

@@ -0,0 +1,3 @@
+<span class="k">foreach</span> <span class="k">(</span><span class="nv">$x</span> <span class="k">as</span> <span class="nv">$y</span><span class="k">)</span> <span class="k">{</span>
+  <span data-symbol-name="z" class="nf">z</span><span class="k">(</span><span class="k">)</span><span class="k">;</span>
+<span class="k">}</span>

src/markup/syntax/highlighter/__tests__/xhpast/no-leading-whitespace.source

@@ -0,0 +1,3 @@
+foreach ($x as $y) {
+  z();
+}

src/markup/syntax/highlighter/xhpast/PhutilXHPASTSyntaxHighlighterFuture.php

@@ -45,6 +45,42 @@ private function applyXHPHighlight($result) {
     $tokens = $root->getTokens();
     $interesting_symbols = $this->findInterestingSymbols($root);
 
+
+    if ($this->scrub) {
+      // If we're scrubbing, we prepended "<?php\n" to the text to force the
+      // highlighter to treat it as PHP source. Now, we need to remove that.
+
+      $ok = false;
+      if (count($tokens) >= 2) {
+        if ($tokens[0]->getTypeName() === 'T_OPEN_TAG') {
+          if ($tokens[1]->getTypeName() === 'T_WHITESPACE') {
+            $ok = true;
+          }
+        }
+      }
+
+      if (!$ok) {
+        throw new Exception(
+          pht(
+            'Expected T_OPEN_TAG, T_WHITESPACE tokens at head of results '.
+            'for highlighting parse of PHP snippet.'));
+      }
+
+      // Remove the "<?php".
+      unset($tokens[0]);
+
+      $value = $tokens[1]->getValue();
+      if ((strlen($value) < 1) || ($value[0] != "\n")) {
+        throw new Exception(
+          pht(
+            'Expected "\\n" at beginning of T_WHITESPACE token at head of '.
+            'tokens for highlighting parse of PHP snippet.'));
+      }
+
+      $value = substr($value, 1);
+      $tokens[1]->overwriteValue($value);
+    }
+
     $out = array();
     foreach ($tokens as $key => $token) {
       $value = $token->getValue();
@@ -122,10 +158,6 @@ private function applyXHPHighlight($result) {
       }
     }
 
-    if ($this->scrub) {
-      array_shift($out);
-    }
-
     return phutil_implode_html('', $out);
   }
 

src/parser/aast/api/AASTToken.php

@@ -36,6 +36,11 @@ final public function getValue() {
     return $this->value;
   }
 
+  final public function overwriteValue($value) {
+    $this->value = $value;
+    return $this;
+  }
+
   final public function getOffset() {
     return $this->offset;
   }