Fix PHT linter

Summary: Variables in strings and concatenation lists.

Test Plan:
New unit tests.

  preg_match('/^((?>[^$\\\\]*)|\\\\.)*$/s', str_repeat('a', 1e6));

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

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

src/lint/linter/ArcanistXHPASTLinter.php

@@ -156,7 +156,7 @@ final class ArcanistXHPASTLinter extends ArcanistLinter {
   }
 
   public function getCacheVersion() {
-    return '1-'.md5_file(xhpast_get_binary_path());
+    return '2-'.md5_file(xhpast_get_binary_path());
   }
 
   public function lintPath($path) {
@@ -298,25 +298,11 @@ final class ArcanistXHPASTLinter extends ArcanistLinter {
         continue;
       }
 
-      // TODO: Desceptively, n_STRING_SCALAR may include variables, mostly
-      // because I was lazy when implementing the parser. We should perform more
-      // strict checks here, and/or enhance the parser.
-
       $identifier = $parameters->getChildByIndex(0);
-      if ($identifier->getTypeName() == 'n_STRING_SCALAR' ||
+      if ($this->isConstantString($identifier)) {
-          $identifier->getTypeName() == 'n_HEREDOC') {
         continue;
       }
 
-      if ($identifier->getTypeName() == 'n_CONCATENATION_LIST') {
-        foreach ($identifier->getChildren() as $child) {
-          if ($child->getTypeName() == 'n_STRING_SCALAR' ||
-              $child->getTypeName() == 'n_OPERATOR') {
-            continue 2;
-          }
-        }
-      }
-
       $this->raiseLintAtNode(
         $call,
         self::LINT_PHT_WITH_DYNAMIC_STRING,
@@ -325,6 +311,42 @@ final class ArcanistXHPASTLinter extends ArcanistLinter {
     }
   }
 
+  private function isConstantString(XHPASTNode $node) {
+    $value = $node->getConcreteString();
+
+    switch ($node->getTypeName()) {
+      case 'n_HEREDOC':
+        if ($value[3] == "'") { // Nowdoc: <<<'EOT'
+          return true;
+        }
+        $value = preg_replace('/^.+\n|\n.*$/', '', $value);
+        break;
+
+      case 'n_STRING_SCALAR':
+        if ($value[0] == "'") {
+          return true;
+        }
+        $value = substr($value, 1, -1);
+        break;
+
+      case 'n_CONCATENATION_LIST':
+        foreach ($node->getChildren() as $child) {
+          if ($child->getTypeName() == 'n_OPERATOR') {
+            continue;
+          }
+          if (!$this->isConstantString($child)) {
+            return false;
+          }
+        }
+        return true;
+
+      default:
+        return false;
+    }
+
+    return preg_match('/^((?>[^$\\\\]*)|\\\\.)*$/s', $value);
+  }
+
   public function lintPHP53Features($root) {
 
     $functions = $root->selectTokensOfType('T_FUNCTION');

src/lint/linter/__tests__/xhpast/pht.lint-test

@@ -5,7 +5,23 @@ pht('a'.'b');
 pht(f());
 pht();
 pht($a);
+pht('a'.$a);
+pht('$a');
+pht("$a");
 pht('%s', $a);
+
+pht(<<<EOT
+a
+EOT
+);
+
+pht(<<<EOT
+$a
+EOT
+);
 ~~~~~~~~~~
 disabled:5:1
 disabled:7:1
+disabled:8:1
+disabled:10:1
+disabled:18:1