From 9e65fc65164a3fc82e6027f77b111fd8e9362afc Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Sat, 8 Aug 2015 23:30:24 +1000
Subject: [PATCH 01/20] Improve declaration formatting linter rule for
 anonymous functions

Summary: Ref T8742. Anonymous functions should have a space after the `function` keyword. Additionally, ensure that there is a space before and after the `use` token.

Test Plan: Modified existing test cases.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Maniphest Tasks: T8742

Differential Revision: https://secure.phabricator.com/D13804
---
 .../call-time-pass-by-reference.lint-test     |  8 +--
 .../xhpast/decl-parens-hug-closing.lint-test  | 20 ++++---
 .../__tests__/xhpast/inner-function.lint-test |  2 +-
 .../xhpast/naming-conventions.lint-test       |  2 +-
 .../__tests__/xhpast/php53-features.lint-test | 12 ++--
 .../reused-iterator-reference.lint-test       |  8 +--
 .../__tests__/xhpast/switches.lint-test       |  2 +-
 .../xhpast/undeclared-variables.lint-test     |  4 +-
 ...DeclarationParenthesesXHPASTLinterRule.php | 58 ++++++++++++++++---
 9 files changed, 79 insertions(+), 37 deletions(-)

diff --git a/src/lint/linter/__tests__/xhpast/call-time-pass-by-reference.lint-test b/src/lint/linter/__tests__/xhpast/call-time-pass-by-reference.lint-test
index 6aacffdb..0f03a774 100644
--- a/src/lint/linter/__tests__/xhpast/call-time-pass-by-reference.lint-test
+++ b/src/lint/linter/__tests__/xhpast/call-time-pass-by-reference.lint-test
@@ -1,8 +1,8 @@
 <?php
 class MyClass {
-    public function myfunc($var) {
-        echo $var;
-    }
+  public function myfunc($var) {
+    echo $var;
+  }
 }
 
 $myvar = true;
@@ -21,7 +21,7 @@ sprintf('0%o', 0777 & $p);
 
 $foo(&$myvar);
 
-array_walk(array(), function() use (&$x) {});
+array_walk(array(), function () use (&$x) {});
 MyClass::myfunc(array(&$x, &$y));
 ~~~~~~~~~~
 error:2:7 XHP19
diff --git a/src/lint/linter/__tests__/xhpast/decl-parens-hug-closing.lint-test b/src/lint/linter/__tests__/xhpast/decl-parens-hug-closing.lint-test
index c6accb15..be9438e3 100644
--- a/src/lint/linter/__tests__/xhpast/decl-parens-hug-closing.lint-test
+++ b/src/lint/linter/__tests__/xhpast/decl-parens-hug-closing.lint-test
@@ -20,9 +20,10 @@ final class X {
 
 }
 
-f(function($x) {});
-f(function($x ) {});
-f(function($x ) use ($z) {});
+f(function ($x) {});
+f(function ($x ) {});
+f(function ($x ) use ($z) {});
+f(function ($x)use($z) {});
 ~~~~~~~~~~
 warning:4:14
 warning:5:11
@@ -31,8 +32,10 @@ error:10:13
 warning:13:23
 warning:16:31
 warning:19:33
-warning:24:14
-warning:25:14
+warning:24:15
+warning:25:15
+warning:26:16
+warning:26:19
 ~~~~~~~~~~
 <?php
 
@@ -56,6 +59,7 @@ final class X {
 
 }
 
-f(function($x) {});
-f(function($x) {});
-f(function($x) use ($z) {});
+f(function ($x) {});
+f(function ($x) {});
+f(function ($x) use ($z) {});
+f(function ($x) use ($z) {});
diff --git a/src/lint/linter/__tests__/xhpast/inner-function.lint-test b/src/lint/linter/__tests__/xhpast/inner-function.lint-test
index 38d0ca15..6294eb0a 100644
--- a/src/lint/linter/__tests__/xhpast/inner-function.lint-test
+++ b/src/lint/linter/__tests__/xhpast/inner-function.lint-test
@@ -8,7 +8,7 @@ function outer() {
 
 // Closures are allowed.
 function my_func($foo) {
-  function() {};
+  function () {};
 }
 ~~~~~~~~~~
 warning:5:5
diff --git a/src/lint/linter/__tests__/xhpast/naming-conventions.lint-test b/src/lint/linter/__tests__/xhpast/naming-conventions.lint-test
index 3bf1a71a..cc78e403 100644
--- a/src/lint/linter/__tests__/xhpast/naming-conventions.lint-test
+++ b/src/lint/linter/__tests__/xhpast/naming-conventions.lint-test
@@ -21,7 +21,7 @@ final class Quack {
 
 
 
-function() use ($this_is_a_closure) {};
+function () use ($this_is_a_closure) {};
 
 function f(&$YY) {}
 
diff --git a/src/lint/linter/__tests__/xhpast/php53-features.lint-test b/src/lint/linter/__tests__/xhpast/php53-features.lint-test
index 6417a56d..a2237293 100644
--- a/src/lint/linter/__tests__/xhpast/php53-features.lint-test
+++ b/src/lint/linter/__tests__/xhpast/php53-features.lint-test
@@ -2,9 +2,8 @@
 
 namespace a;
 use b, c;
-f(function() {});
-g(function() use ($c) {});
-h(function /* ! */ () use ($c) {});
+f(function () {});
+g(function () use ($c) {});
 static::m();
 1 ? 1 : 2;
 1 ?: 2;
@@ -16,10 +15,9 @@ error:3:1
 error:4:5
 error:5:3
 error:6:3
-error:7:3
-error:8:1
-error:10:1
-error:13:6
+error:7:1
+error:9:1
+error:12:6
 ~~~~~~~~~~
 ~~~~~~~~~~
 {
diff --git a/src/lint/linter/__tests__/xhpast/reused-iterator-reference.lint-test b/src/lint/linter/__tests__/xhpast/reused-iterator-reference.lint-test
index cbfa13b1..ac65ff57 100644
--- a/src/lint/linter/__tests__/xhpast/reused-iterator-reference.lint-test
+++ b/src/lint/linter/__tests__/xhpast/reused-iterator-reference.lint-test
@@ -90,13 +90,13 @@ function variable_variable() {
 function closure1() {
   $ar = array();
   foreach ($ar as &$a) {}
-  function($a) {
+  function ($a) {
     $a++;
   };
 }
 
 function closure2() {
-  function() {
+  function () {
     $ar = array();
     foreach ($ar as &$a) {}
   };
@@ -104,7 +104,7 @@ function closure2() {
 }
 
 function closure3() {
-  function() {
+  function () {
     $ar = array();
     foreach ($ar as &$a) {}
     $a++; // Reuse of $a
@@ -114,7 +114,7 @@ function closure3() {
 function closure4() {
   $ar = array();
   foreach ($ar as &$a) {}
-  function($a) {
+  function ($a) {
     unset($a);
   };
   $a++; // Reuse of $a
diff --git a/src/lint/linter/__tests__/xhpast/switches.lint-test b/src/lint/linter/__tests__/xhpast/switches.lint-test
index 89e97069..6c6fa944 100644
--- a/src/lint/linter/__tests__/xhpast/switches.lint-test
+++ b/src/lint/linter/__tests__/xhpast/switches.lint-test
@@ -65,7 +65,7 @@ switch ($x) {
     }
   case 4:
     function f() { throw new Exception(); }
-    g(function() { return; });
+    g(function () { return; });
     final class C { public function m() { return; } }
     interface I {}
   case 5:
diff --git a/src/lint/linter/__tests__/xhpast/undeclared-variables.lint-test b/src/lint/linter/__tests__/xhpast/undeclared-variables.lint-test
index 903e620a..c6cd1b40 100644
--- a/src/lint/linter/__tests__/xhpast/undeclared-variables.lint-test
+++ b/src/lint/linter/__tests__/xhpast/undeclared-variables.lint-test
@@ -1,6 +1,6 @@
 <?php
 
-function() use ($c) {
+function () use ($c) {
   $c++;
 };
 
@@ -173,7 +173,7 @@ function catchy() {
 }
 
 function some_func($x, $y) {
-  $func = function($z) use ($x) {
+  $func = function ($z) use ($x) {
     echo $x;
     echo $y;
     echo $z;
diff --git a/src/lint/linter/xhpast/rules/ArcanistDeclarationParenthesesXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistDeclarationParenthesesXHPASTLinterRule.php
index 357ac77f..6606b0b3 100644
--- a/src/lint/linter/xhpast/rules/ArcanistDeclarationParenthesesXHPASTLinterRule.php
+++ b/src/lint/linter/xhpast/rules/ArcanistDeclarationParenthesesXHPASTLinterRule.php
@@ -24,7 +24,7 @@ final class ArcanistDeclarationParenthesesXHPASTLinterRule
       $tokens = $params->getTokens();
 
       $first = head($tokens);
-      $last = last($tokens);
+      $last  = last($tokens);
 
       $leading = $first->getNonsemanticTokensBefore();
       $leading_text = implode('', mpull($leading, 'getValue'));
@@ -32,14 +32,27 @@ final class ArcanistDeclarationParenthesesXHPASTLinterRule
       $trailing = $last->getNonsemanticTokensBefore();
       $trailing_text = implode('', mpull($trailing, 'getValue'));
 
-      if (preg_match('/^\s+$/', $leading_text)) {
-        $this->raiseLintAtOffset(
-          $first->getOffset() - strlen($leading_text),
-          pht(
-            'Convention: no spaces before opening parenthesis in '.
-            'function and method declarations.'),
-          $leading_text,
-          '');
+      if ($dec->getChildByIndex(2)->getTypeName() == 'n_EMPTY') {
+        // Anonymous functions.
+        if ($leading_text != ' ') {
+          $this->raiseLintAtOffset(
+            $first->getOffset() - strlen($leading_text),
+            pht(
+              'Convention: space before opening parenthesis in '.
+              'anonymous function declarations.'),
+            $leading_text,
+            ' ');
+        }
+      } else {
+        if (preg_match('/^\s+$/', $leading_text)) {
+          $this->raiseLintAtOffset(
+            $first->getOffset() - strlen($leading_text),
+            pht(
+              'Convention: no spaces before opening parenthesis in '.
+              'function and method declarations.'),
+            $leading_text,
+            '');
+        }
       }
 
       if (preg_match('/^\s+$/', $trailing_text)) {
@@ -51,6 +64,33 @@ final class ArcanistDeclarationParenthesesXHPASTLinterRule
           $trailing_text,
           '');
       }
+
+      $use_list = $dec->getChildByIndex(4);
+      if ($use_list->getTypeName() == 'n_EMPTY') {
+        continue;
+      }
+      $use_token = $use_list->selectTokensOfType('T_USE');
+
+      foreach ($use_token as $use) {
+        $before = $use->getNonsemanticTokensBefore();
+        $after  = $use->getNonsemanticTokensAfter();
+
+        if (!$before) {
+          $this->raiseLintAtOffset(
+            $use->getOffset(),
+            pht('Convention: space before `%s` token.', 'use'),
+            '',
+            ' ');
+        }
+
+        if (!$after) {
+          $this->raiseLintAtOffset(
+            $use->getOffset() + strlen($use->getValue()),
+            pht('Convention: space after `%s` token.', 'use'),
+            '',
+            ' ');
+        }
+      }
     }
   }
 

From f43b74c6052f10b54bb5c97215fdc02d8935226f Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Tue, 11 Aug 2015 06:48:56 +1000
Subject: [PATCH 02/20] Improve PHP compatibility linter

Summary: Ref T8674. Adds to `ArcanistPHPCompatibilityXHPASTLinterRule` such that an error is raised whenever `self` or `$this` is used in an anonymous closure prior to PHP 5.4.

Test Plan: Added test cases.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin

Maniphest Tasks: T8674

Differential Revision: https://secure.phabricator.com/D13841
---
 .../__tests__/xhpast/php54-features.lint-test | 17 +++++++
 ...canistPHPCompatibilityXHPASTLinterRule.php | 46 +++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/src/lint/linter/__tests__/xhpast/php54-features.lint-test b/src/lint/linter/__tests__/xhpast/php54-features.lint-test
index bdeff107..91c112c5 100644
--- a/src/lint/linter/__tests__/xhpast/php54-features.lint-test
+++ b/src/lint/linter/__tests__/xhpast/php54-features.lint-test
@@ -6,9 +6,26 @@ f()[0];
 // The check above should be this, see T4334.
 // $o->m()[0];
 
+final class SomeClass extends Phobject {
+  public function someMethod() {
+    return function () {
+      $this->someOtherMethod();
+    };
+  }
+
+  public static function someStaticMethod() {
+    return function () {
+      self::someOtherMethod();
+    };
+  }
+}
+
 ~~~~~~~~~~
 error:3:5
 error:4:9
+error:9:13
+error:12:7
+error:18:7
 ~~~~~~~~~~
 ~~~~~~~~~~
 {
diff --git a/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php
index 3f118b43..7697356e 100644
--- a/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php
+++ b/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php
@@ -402,6 +402,52 @@ final class ArcanistPHPCompatibilityXHPASTLinterRule
           break;
       }
     }
+
+    $closures = $this->getAnonymousClosures($root);
+    foreach ($closures as $closure) {
+      $static_accesses = $closure
+        ->selectDescendantsOfType('n_CLASS_STATIC_ACCESS');
+
+      foreach ($static_accesses as $static_access) {
+        $class = $static_access->getChildByIndex(0);
+
+        if ($class->getTypeName() != 'n_CLASS_NAME') {
+          continue;
+        }
+
+        if (strtolower($class->getConcreteString()) != 'self') {
+          continue;
+        }
+
+        $this->raiseLintAtNode(
+          $class,
+          pht(
+            'The use of `%s` in an anonymous closure is not '.
+            'available before PHP 5.4.',
+            'self'));
+      }
+
+      $property_accesses = $closure
+        ->selectDescendantsOfType('n_OBJECT_PROPERTY_ACCESS');
+      foreach ($property_accesses as $property_access) {
+        $variable = $property_access->getChildByIndex(0);
+
+        if ($variable->getTypeName() != 'n_VARIABLE') {
+          continue;
+        }
+
+        if ($variable->getConcreteString() != '$this') {
+          continue;
+        }
+
+        $this->raiseLintAtNode(
+          $variable,
+          pht(
+            'The use of `%s` in an anonymous closure is not '.
+            'available before PHP 5.4.',
+            '$this'));
+      }
+    }
   }
 
   private function lintPHP54Incompatibilities(XHPASTNode $root) {

From 6c759ae34396a96b3ec94a269f0fe635a4a47879 Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Tue, 11 Aug 2015 06:49:20 +1000
Subject: [PATCH 03/20] Improve useless overriding method linter rule

Summary:
Improve `ArcanistUselessOverridingMethodXHPASTLinterRule` by allowing overriding methods which set default values. For example, the following scenario is perfectly valid:

```lang=php

class SomeClass {
  public function __construct($x) {}
}

class SomeOtherClass extends Class {
  public function __construct($x = null) {
    parent::__construct($x);
  }
}
```

Test Plan: Added test case.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D13840
---
 .../__tests__/xhpast/useless-overriding-method.lint-test     | 4 ++++
 .../ArcanistUselessOverridingMethodXHPASTLinterRule.php      | 5 +++++
 2 files changed, 9 insertions(+)

diff --git a/src/lint/linter/__tests__/xhpast/useless-overriding-method.lint-test b/src/lint/linter/__tests__/xhpast/useless-overriding-method.lint-test
index 308bf76f..40771407 100644
--- a/src/lint/linter/__tests__/xhpast/useless-overriding-method.lint-test
+++ b/src/lint/linter/__tests__/xhpast/useless-overriding-method.lint-test
@@ -16,6 +16,10 @@ final class MyClass extends SomeOtherClass {
   public function anotherMethod($x, &$y) {
     return parent::anotherMethod($x, $y);
   }
+
+  public function usefulMethodWithDefaultValue($x = null) {
+    return parent::usefulMethodWithDefaultValue($x);
+  }
 }
 ~~~~~~~~~~
 error:3:13
diff --git a/src/lint/linter/xhpast/rules/ArcanistUselessOverridingMethodXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistUselessOverridingMethodXHPASTLinterRule.php
index 8a4859e5..475ed25f 100644
--- a/src/lint/linter/xhpast/rules/ArcanistUselessOverridingMethodXHPASTLinterRule.php
+++ b/src/lint/linter/xhpast/rules/ArcanistUselessOverridingMethodXHPASTLinterRule.php
@@ -26,12 +26,17 @@ final class ArcanistUselessOverridingMethodXHPASTLinterRule
       $parameters = array();
 
       foreach ($parameter_list->getChildren() as $parameter) {
+        $default   = $parameter->getChildByIndex(2);
         $parameter = $parameter->getChildByIndex(1);
 
         if ($parameter->getTypeName() == 'n_VARIABLE_REFERENCE') {
           $parameter = $parameter->getChildOfType(0, 'n_VARIABLE');
         }
 
+        if ($default->getTypeName() != 'n_EMPTY') {
+          continue 2;
+        }
+
         $parameters[] = $parameter->getConcreteString();
       }
 

From 587f7440e3f9fc164f6969ca8dc008231d6c07aa Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Tue, 11 Aug 2015 06:49:54 +1000
Subject: [PATCH 04/20] Fix self member reference rule for PHP 5.3

Summary:
Fixes T8674. Currently, `ArcanistSelfMemberReferenceXHPASTLinterRule` warns if a fully-qualified class name is used in place of `self`. This is fine in most cases, but in some specific scenarios fails for PHP 5.3 because `self` (and also `$this`) cannot be used in an anonymous closure (see [[http://php.net/manual/en/functions.anonymous.php | anonymous functions]] and [[https://wiki.php.net/rfc/closures/removal-of-this | removal of `$this` in closures]]).

In order to do this, I also had to modify the manner in which configuration was passed to individual linter rule. Previously, it was only possible or an individual linter rule to be set with a configuration value. Once the linter rule had set this value, there was no means by which to allow this value to be shared amongst linter rules.

Depends on D13819.

Test Plan: Added unit tests.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin

Maniphest Tasks: T8674

Differential Revision: https://secure.phabricator.com/D13820
---
 src/lint/linter/ArcanistXHPASTLinter.php      |  9 +++-
 .../self-member-references-php53.lint-test    | 19 ++++++++
 .../self-member-references-php54.lint-test    | 30 ++++++++++++
 .../xhpast/ArcanistXHPASTLinterRule.php       | 48 ++++++++++++++++++-
 ...canistPHPCompatibilityXHPASTLinterRule.php | 31 ------------
 ...istSelfMemberReferenceXHPASTLinterRule.php | 24 +++++++---
 6 files changed, 121 insertions(+), 40 deletions(-)
 create mode 100644 src/lint/linter/__tests__/xhpast/self-member-references-php53.lint-test
 create mode 100644 src/lint/linter/__tests__/xhpast/self-member-references-php54.lint-test

diff --git a/src/lint/linter/ArcanistXHPASTLinter.php b/src/lint/linter/ArcanistXHPASTLinter.php
index 3a036f2f..985f0950 100644
--- a/src/lint/linter/ArcanistXHPASTLinter.php
+++ b/src/lint/linter/ArcanistXHPASTLinter.php
@@ -67,14 +67,21 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
   }
 
   public function setLinterConfigurationValue($key, $value) {
+    $matched = false;
+
     foreach ($this->rules as $rule) {
       foreach ($rule->getLinterConfigurationOptions() as $k => $spec) {
         if ($k == $key) {
-          return $rule->setLinterConfigurationValue($key, $value);
+          $matched = true;
+          $rule->setLinterConfigurationValue($key, $value);
         }
       }
     }
 
+    if ($matched) {
+      return;
+    }
+
     return parent::setLinterConfigurationValue($key, $value);
   }
 
diff --git a/src/lint/linter/__tests__/xhpast/self-member-references-php53.lint-test b/src/lint/linter/__tests__/xhpast/self-member-references-php53.lint-test
new file mode 100644
index 00000000..5d06f638
--- /dev/null
+++ b/src/lint/linter/__tests__/xhpast/self-member-references-php53.lint-test
@@ -0,0 +1,19 @@
+<?php
+
+final class SomeClass extends Phobject {
+  public static function someMethod() {
+    $closure = function () {
+      SomeClass::someOtherMethod();
+    };
+    $closure();
+  }
+}
+~~~~~~~~~~
+error:3:13
+~~~~~~~~~~
+~~~~~~~~~~
+{
+  "config": {
+    "xhpast.php-version": "5.3.0"
+  }
+}
diff --git a/src/lint/linter/__tests__/xhpast/self-member-references-php54.lint-test b/src/lint/linter/__tests__/xhpast/self-member-references-php54.lint-test
new file mode 100644
index 00000000..ce0ec08e
--- /dev/null
+++ b/src/lint/linter/__tests__/xhpast/self-member-references-php54.lint-test
@@ -0,0 +1,30 @@
+<?php
+
+final class SomeClass extends Phobject {
+  public static function someMethod() {
+    $closure = function () {
+      SomeClass::someOtherMethod();
+    };
+    $closure();
+  }
+}
+~~~~~~~~~~
+error:3:13
+advice:6:7
+~~~~~~~~~~
+<?php
+
+final class SomeClass extends Phobject {
+  public static function someMethod() {
+    $closure = function () {
+      self::someOtherMethod();
+    };
+    $closure();
+  }
+}
+~~~~~~~~~~
+{
+  "config": {
+    "xhpast.php-version": "5.4.0"
+  }
+}
diff --git a/src/lint/linter/xhpast/ArcanistXHPASTLinterRule.php b/src/lint/linter/xhpast/ArcanistXHPASTLinterRule.php
index bf1c2aad..da5a37b5 100644
--- a/src/lint/linter/xhpast/ArcanistXHPASTLinterRule.php
+++ b/src/lint/linter/xhpast/ArcanistXHPASTLinterRule.php
@@ -5,6 +5,9 @@ abstract class ArcanistXHPASTLinterRule extends Phobject {
   private $linter = null;
   private $lintID = null;
 
+  protected $version;
+  protected $windowsVersion;
+
   final public static function loadAllRules() {
     return id(new PhutilClassMapQuery())
       ->setAncestorClass(__CLASS__)
@@ -47,10 +50,29 @@ abstract class ArcanistXHPASTLinterRule extends Phobject {
   }
 
   public function getLinterConfigurationOptions() {
-    return array();
+    return array(
+      'xhpast.php-version' => array(
+        'type' => 'optional string',
+        'help' => pht('PHP version to target.'),
+      ),
+      'xhpast.php-version.windows' => array(
+        'type' => 'optional string',
+        'help' => pht('PHP version to target on Windows.'),
+      ),
+    );
   }
 
-  public function setLinterConfigurationValue($key, $value) {}
+  public function setLinterConfigurationValue($key, $value) {
+    switch ($key) {
+      case 'xhpast.php-version':
+        $this->version = $value;
+        return;
+
+      case 'xhpast.php-version.windows':
+        $this->windowsVersion = $value;
+        return;
+    }
+  }
 
   abstract public function process(XHPASTNode $root);
 
@@ -138,6 +160,28 @@ abstract class ArcanistXHPASTLinterRule extends Phobject {
 
 /* -(  Utility  )------------------------------------------------------------ */
 
+  /**
+   * Retrieve all anonymous closure(s).
+   *
+   * Returns all descendant nodes which represent an anonymous function
+   * declaration.
+   *
+   * @param  XHPASTNode    Root node.
+   * @return AASTNodeList
+   */
+  protected function getAnonymousClosures(XHPASTNode $root) {
+    $func_decls = $root->selectDescendantsOfType('n_FUNCTION_DECLARATION');
+    $nodes      = array();
+
+    foreach ($func_decls as $func_decl) {
+      if ($func_decl->getChildByIndex(2)->getTypeName() == 'n_EMPTY') {
+        $nodes[] = $func_decl;
+      }
+    }
+
+    return AASTNodeList::newFromTreeAndNodes($root->getTree(), $nodes);
+  }
+
   /**
    * Retrieve all calls to some specified function(s).
    *
diff --git a/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php
index 7697356e..5ee6f3fe 100644
--- a/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php
+++ b/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php
@@ -5,41 +5,10 @@ final class ArcanistPHPCompatibilityXHPASTLinterRule
 
   const ID = 45;
 
-  private $version;
-  private $windowsVersion;
-
   public function getLintName() {
     return pht('PHP Compatibility');
   }
 
-  public function getLinterConfigurationOptions() {
-    return parent::getLinterConfigurationOptions() + array(
-      'xhpast.php-version' => array(
-        'type' => 'optional string',
-        'help' => pht('PHP version to target.'),
-      ),
-      'xhpast.php-version.windows' => array(
-        'type' => 'optional string',
-        'help' => pht('PHP version to target on Windows.'),
-      ),
-    );
-  }
-
-  public function setLinterConfigurationValue($key, $value) {
-    switch ($key) {
-      case 'xhpast.php-version':
-        $this->version = $value;
-        return;
-
-      case 'xhpast.php-version.windows':
-        $this->windowsVersion = $value;
-        return;
-
-      default:
-        return parent::setLinterConfigurationValue($key, $value);
-    }
-  }
-
   public function process(XHPASTNode $root) {
     static $compat_info;
 
diff --git a/src/lint/linter/xhpast/rules/ArcanistSelfMemberReferenceXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistSelfMemberReferenceXHPASTLinterRule.php
index 4c3399b7..bfec780e 100644
--- a/src/lint/linter/xhpast/rules/ArcanistSelfMemberReferenceXHPASTLinterRule.php
+++ b/src/lint/linter/xhpast/rules/ArcanistSelfMemberReferenceXHPASTLinterRule.php
@@ -23,6 +23,7 @@ final class ArcanistSelfMemberReferenceXHPASTLinterRule
 
       $class_static_accesses = $class_declaration
         ->selectDescendantsOfType('n_CLASS_STATIC_ACCESS');
+      $closures = $this->getAnonymousClosures($class_declaration);
 
       foreach ($class_static_accesses as $class_static_access) {
         $double_colons = $class_static_access
@@ -35,12 +36,23 @@ final class ArcanistSelfMemberReferenceXHPASTLinterRule
         $class_ref_name = $class_ref->getConcreteString();
 
         if (strtolower($class_name) == strtolower($class_ref_name)) {
-          $this->raiseLintAtNode(
-            $class_ref,
-            pht(
-              'Use `%s` for local static member references.',
-              'self::'),
-            'self');
+          $in_closure = false;
+
+          foreach ($closures as $closure) {
+            if ($class_ref->isDescendantOf($closure)) {
+              $in_closure = true;
+              break;
+            }
+          }
+
+          if (version_compare($this->version, '5.4.0', '>=') || !$in_closure) {
+            $this->raiseLintAtNode(
+              $class_ref,
+              pht(
+                'Use `%s` for local static member references.',
+                'self::'),
+              'self');
+          }
         }
 
         static $self_refs = array(

From 0e0957872083daa38309204ad9774568c9fcdd5c Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Tue, 11 Aug 2015 06:50:47 +1000
Subject: [PATCH 05/20] Use PhutilClassMapQuery instead of PhutilSymbolLoader

Summary: Use `PhutilClassMapQuery` instead of `PhutilSymbolLoader`, mainly for consistency. Depends on D13572.

Test Plan: This hasn't been tested very comphrehensively as of yet.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D13573
---
 src/configuration/ArcanistConfiguration.php   | 24 ++------------
 .../ArcanistConfigurationDrivenLintEngine.php | 32 ++-----------------
 src/unit/engine/PhutilUnitTestEngine.php      |  6 ++--
 src/workflow/ArcanistLintersWorkflow.php      |  4 +--
 4 files changed, 10 insertions(+), 56 deletions(-)

diff --git a/src/configuration/ArcanistConfiguration.php b/src/configuration/ArcanistConfiguration.php
index 71152587..91d0dc65 100644
--- a/src/configuration/ArcanistConfiguration.php
+++ b/src/configuration/ArcanistConfiguration.php
@@ -35,28 +35,10 @@ class ArcanistConfiguration extends Phobject {
   }
 
   public function buildAllWorkflows() {
-    $workflows_by_name = array();
-
-    $workflows_by_class_name = id(new PhutilSymbolLoader())
+    return id(new PhutilClassMapQuery())
       ->setAncestorClass('ArcanistWorkflow')
-      ->loadObjects();
-    foreach ($workflows_by_class_name as $class => $workflow) {
-      $name = $workflow->getWorkflowName();
-
-      if (isset($workflows_by_name[$name])) {
-        $other = get_class($workflows_by_name[$name]);
-        throw new Exception(
-          pht(
-            'Workflows %s and %s both implement workflows named %s.',
-            $class,
-            $other,
-            $name));
-      }
-
-      $workflows_by_name[$name] = $workflow;
-    }
-
-    return $workflows_by_name;
+      ->setUniqueMethod('getWorkflowName')
+      ->execute();
   }
 
   final public function isValidWorkflow($workflow) {
diff --git a/src/lint/engine/ArcanistConfigurationDrivenLintEngine.php b/src/lint/engine/ArcanistConfigurationDrivenLintEngine.php
index 531ccf6c..9d8693af 100644
--- a/src/lint/engine/ArcanistConfigurationDrivenLintEngine.php
+++ b/src/lint/engine/ArcanistConfigurationDrivenLintEngine.php
@@ -137,36 +137,10 @@ final class ArcanistConfigurationDrivenLintEngine extends ArcanistLintEngine {
   }
 
   private function loadAvailableLinters() {
-    $linters = id(new PhutilSymbolLoader())
+    return id(new PhutilClassMapQuery())
       ->setAncestorClass('ArcanistLinter')
-      ->loadObjects();
-
-    $map = array();
-    foreach ($linters as $linter) {
-      $name = $linter->getLinterConfigurationName();
-
-      // This linter isn't selectable through configuration.
-      if ($name === null) {
-        continue;
-      }
-
-      if (empty($map[$name])) {
-        $map[$name] = $linter;
-        continue;
-      }
-
-      $orig_class = get_class($map[$name]);
-      $this_class = get_class($linter);
-      throw new Exception(
-        pht(
-          "Two linters (`%s`, `%s`) both have the same configuration ".
-          "name ('%s'). Linters must have unique configuration names.",
-          $orig_class,
-          $this_class,
-          $name));
-    }
-
-    return $map;
+      ->setUniqueMethod('getLinterConfigurationName', true)
+      ->execute();
   }
 
   private function matchPaths(
diff --git a/src/unit/engine/PhutilUnitTestEngine.php b/src/unit/engine/PhutilUnitTestEngine.php
index 7509b300..1dd47220 100644
--- a/src/unit/engine/PhutilUnitTestEngine.php
+++ b/src/unit/engine/PhutilUnitTestEngine.php
@@ -76,11 +76,9 @@ final class PhutilUnitTestEngine extends ArcanistUnitTestEngine {
   private function getAllTests() {
     $project_root = $this->getWorkingCopy()->getProjectRoot();
 
-    $symbols = id(new PhutilSymbolLoader())
-      ->setType('class')
+    $symbols = id(new PhutilClassMapQuery())
       ->setAncestorClass('PhutilTestCase')
-      ->setConcreteOnly(true)
-      ->selectSymbolsWithoutLoading();
+      ->execute();
 
     $in_working_copy = array();
 
diff --git a/src/workflow/ArcanistLintersWorkflow.php b/src/workflow/ArcanistLintersWorkflow.php
index b872e300..6d70b4d6 100644
--- a/src/workflow/ArcanistLintersWorkflow.php
+++ b/src/workflow/ArcanistLintersWorkflow.php
@@ -36,9 +36,9 @@ EOTEXT
   public function run() {
     $console = PhutilConsole::getConsole();
 
-    $linters = id(new PhutilSymbolLoader())
+    $linters = id(new PhutilClassMapQuery())
       ->setAncestorClass('ArcanistLinter')
-      ->loadObjects();
+      ->execute();
 
     try {
       $built = $this->newLintEngine()->buildLinters();

From 59698df856ddaff7440c8b244043d2a1f598d522 Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Tue, 11 Aug 2015 06:54:15 +1000
Subject: [PATCH 06/20] Rough version of configuration driven unit test engine

Summary: Ref T5568. As discussed in IRC. This is very rough and not widely useable, but represents a solid first step.

Test Plan: Ran `arc unit` with a bunch of flags.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: rfreebern, aripringle, jaydiablo, BYK, tycho.tatitscheff, epriestley, Korvin

Maniphest Tasks: T5568

Differential Revision: https://secure.phabricator.com/D13579
---
 .arcconfig                                    |   2 +-
 .arcunit                                      |   8 +
 src/__phutil_library_map__.php                |   2 +
 ...anistConfigurationDrivenUnitTestEngine.php | 199 ++++++++++++++++++
 src/unit/engine/ArcanistUnitTestEngine.php    |   4 +
 src/unit/engine/PhutilUnitTestEngine.php      |   4 +
 6 files changed, 218 insertions(+), 1 deletion(-)
 create mode 100644 .arcunit
 create mode 100644 src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php

diff --git a/.arcconfig b/.arcconfig
index 28823294..1309022c 100644
--- a/.arcconfig
+++ b/.arcconfig
@@ -1,6 +1,6 @@
 {
   "phabricator.uri": "https://secure.phabricator.com/",
-  "unit.engine": "PhutilUnitTestEngine",
+  "unit.engine": "ArcanistConfigurationDrivenUnitTestEngine",
   "load": [
     "src/"
   ]
diff --git a/.arcunit b/.arcunit
new file mode 100644
index 00000000..860ee1ae
--- /dev/null
+++ b/.arcunit
@@ -0,0 +1,8 @@
+{
+  "engines": {
+    "phutil": {
+      "type": "phutil",
+      "include": "(\\.php$)"
+    }
+  }
+}
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 4783a8d6..841b4e84 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -57,6 +57,7 @@ phutil_register_library_map(array(
     'ArcanistConcatenationOperatorXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistConcatenationOperatorXHPASTLinterRule.php',
     'ArcanistConfiguration' => 'configuration/ArcanistConfiguration.php',
     'ArcanistConfigurationDrivenLintEngine' => 'lint/engine/ArcanistConfigurationDrivenLintEngine.php',
+    'ArcanistConfigurationDrivenUnitTestEngine' => 'unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php',
     'ArcanistConfigurationManager' => 'configuration/ArcanistConfigurationManager.php',
     'ArcanistConsoleLintRenderer' => 'lint/renderer/ArcanistConsoleLintRenderer.php',
     'ArcanistConstructorParenthesesXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistConstructorParenthesesXHPASTLinterRule.php',
@@ -333,6 +334,7 @@ phutil_register_library_map(array(
     'ArcanistConcatenationOperatorXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistConfiguration' => 'Phobject',
     'ArcanistConfigurationDrivenLintEngine' => 'ArcanistLintEngine',
+    'ArcanistConfigurationDrivenUnitTestEngine' => 'ArcanistUnitTestEngine',
     'ArcanistConfigurationManager' => 'Phobject',
     'ArcanistConsoleLintRenderer' => 'ArcanistLintRenderer',
     'ArcanistConstructorParenthesesXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
diff --git a/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php b/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php
new file mode 100644
index 00000000..9c5ed8ab
--- /dev/null
+++ b/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php
@@ -0,0 +1,199 @@
+<?php
+
+final class ArcanistConfigurationDrivenUnitTestEngine
+  extends ArcanistUnitTestEngine {
+
+  protected function supportsRunAllTests() {
+    $engines = $this->buildTestEngines();
+
+    foreach ($engines as $engine) {
+      if ($engine->supportsRunAllTests()) {
+        return true;
+      }
+    }
+
+    return false;
+  }
+
+  public function buildTestEngines() {
+    $working_copy = $this->getWorkingCopy();
+    $config_path  = $working_copy->getProjectPath('.arcunit');
+
+    if (!Filesystem::pathExists($config_path)) {
+      throw new ArcanistUsageException(
+        pht(
+          "Unable to find '%s' file to configure test engines. Create an ".
+          "'%s' file in the root directory of the working copy.",
+          '.arcunit',
+          '.arcunit'));
+    }
+
+    $data = Filesystem::readFile($config_path);
+    $config = null;
+    try {
+      $config = phutil_json_decode($data);
+    } catch (PhutilJSONParserException $ex) {
+      throw new PhutilProxyException(
+        pht(
+          "Expected '%s' file to be a valid JSON file, but ".
+          "failed to decode '%s'.",
+          '.arcunit',
+          $config_path),
+        $ex);
+    }
+
+    $test_engines = $this->loadAvailableTestEngines();
+
+    try {
+      PhutilTypeSpec::checkMap(
+        $config,
+        array(
+          'engines' => 'map<string, map<string, wild>>',
+        ));
+    } catch (PhutilTypeCheckException $ex) {
+      throw new PhutilProxyException(
+        pht("Error in parsing '%s' file.", $config_path),
+        $ex);
+    }
+
+    $built_test_engines = array();
+    $all_paths = $this->getPaths();
+
+    foreach ($config['engines'] as $name => $spec) {
+      $type = idx($spec, 'type');
+
+      if ($type !== null) {
+        if (empty($test_engines[$type])) {
+          throw new ArcanistUsageException(
+            pht(
+              "Test engine '%s' specifies invalid type '%s'. ".
+              "Available test engines are: %s.",
+              $name,
+              $type,
+              implode(', ', array_keys($test_engines))));
+        }
+
+        $test_engine = clone $test_engines[$type];
+      } else {
+        // We'll raise an error below about the invalid "type" key.
+        // TODO: Can we just do the type check first, and simplify this a bit?
+        $test_engine = null;
+      }
+
+      try {
+        PhutilTypeSpec::checkMap(
+          $spec,
+          array(
+            'type' => 'string',
+            'include' => 'optional regex | list<regex>',
+            'exclude' => 'optional regex | list<regex>',
+          ));
+      } catch (PhutilTypeCheckException $ex) {
+        throw new PhutilProxyException(
+          pht(
+            "Error in parsing '%s' file, for test engine '%s'.",
+            '.arcunit',
+            $name),
+          $ex);
+      }
+
+      $include = (array)idx($spec, 'include', array());
+      $exclude = (array)idx($spec, 'exclude', array());
+      $paths = $this->matchPaths(
+        $all_paths,
+        $include,
+        $exclude);
+
+      $test_engine->setPaths($paths);
+      $built_test_engines[] = $test_engine;
+    }
+
+    return $built_test_engines;
+  }
+
+  public function run() {
+    $paths = $this->getPaths();
+
+    // If we are running with `--everything` then `$paths` will be `null`.
+    if (!$paths) {
+      $paths = array();
+    }
+
+    $engines    = $this->buildTestEngines();
+    $results    = array();
+    $exceptions = array();
+
+    foreach ($engines as $engine) {
+      $engine
+        ->setWorkingCopy($this->getWorkingCopy())
+        ->setEnableCoverage($this->getEnableCoverage());
+
+      // TODO: At some point, maybe we should emit a warning here if an engine
+      // doesn't support `--everything`, to reduce surprise when `--everything`
+      // does not really mean `--everything`.
+      if ($engine->supportsRunAllTests()) {
+        $engine->setRunAllTests($this->getRunAllTests());
+      }
+
+      try {
+        // TODO: Type check the results.
+        $results[] = $engine->run();
+      } catch (ArcanistNoEffectException $ex) {
+        $exceptions[] = $ex;
+      }
+    }
+
+    if (!$results) {
+      // If all engines throw an `ArcanistNoEffectException`, then we should
+      // preserve this behavior.
+      throw new ArcanistNoEffectException(pht('No tests to run.'));
+    }
+
+    return array_mergev($results);
+  }
+
+  private function loadAvailableTestEngines() {
+    return id(new PhutilClassMapQuery())
+      ->setAncestorClass('ArcanistUnitTestEngine')
+      ->setUniqueMethod('getEngineConfigurationName', true)
+      ->execute();
+  }
+
+  /**
+   * TODO: This is copied from @{class:ArcanistConfigurationDrivenLintEngine}.
+   */
+  private function matchPaths(array $paths, array $include, array $exclude) {
+    $match = array();
+
+    foreach ($paths as $path) {
+      $keep = false;
+      if (!$include) {
+        $keep = true;
+      } else {
+        foreach ($include as $rule) {
+          if (preg_match($rule, $path)) {
+            $keep = true;
+            break;
+          }
+        }
+      }
+
+      if (!$keep) {
+        continue;
+      }
+
+      if ($exclude) {
+        foreach ($exclude as $rule) {
+          if (preg_match($rule, $path)) {
+            continue 2;
+          }
+        }
+      }
+
+      $match[] = $path;
+    }
+
+    return $match;
+  }
+
+}
diff --git a/src/unit/engine/ArcanistUnitTestEngine.php b/src/unit/engine/ArcanistUnitTestEngine.php
index 9783ce26..ad66bb90 100644
--- a/src/unit/engine/ArcanistUnitTestEngine.php
+++ b/src/unit/engine/ArcanistUnitTestEngine.php
@@ -16,6 +16,10 @@ abstract class ArcanistUnitTestEngine extends Phobject {
 
   final public function __construct() {}
 
+  public function getEngineConfigurationName() {
+    return null;
+  }
+
   final public function setRunAllTests($run_all_tests) {
     if (!$this->supportsRunAllTests() && $run_all_tests) {
       throw new Exception(
diff --git a/src/unit/engine/PhutilUnitTestEngine.php b/src/unit/engine/PhutilUnitTestEngine.php
index 1dd47220..eb00f79c 100644
--- a/src/unit/engine/PhutilUnitTestEngine.php
+++ b/src/unit/engine/PhutilUnitTestEngine.php
@@ -5,6 +5,10 @@
  */
 final class PhutilUnitTestEngine extends ArcanistUnitTestEngine {
 
+  public function getEngineConfigurationName() {
+    return 'phutil';
+  }
+
   protected function supportsRunAllTests() {
     return true;
   }

From 830bcbc2a5e9ccc724e80ba878f61d71c6590ae0 Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Tue, 11 Aug 2015 06:58:28 +1000
Subject: [PATCH 07/20] Add a linter rule for array elements

Summary: Add a linter rule to ensure that array elements occupy a single line each.

Test Plan: Added test cases.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D13842
---
 src/__phutil_library_map__.php                |  2 +
 .../__tests__/xhpast/array-value.lint-test    | 48 ++++++++++++++++
 .../ArcanistArrayValueXHPASTLinterRule.php    | 56 +++++++++++++++++++
 3 files changed, 106 insertions(+)
 create mode 100644 src/lint/linter/__tests__/xhpast/array-value.lint-test
 create mode 100644 src/lint/linter/xhpast/rules/ArcanistArrayValueXHPASTLinterRule.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 841b4e84..422c792f 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -15,6 +15,7 @@ phutil_register_library_map(array(
     'ArcanistAnoidWorkflow' => 'workflow/ArcanistAnoidWorkflow.php',
     'ArcanistArrayIndexSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistArrayIndexSpacingXHPASTLinterRule.php',
     'ArcanistArraySeparatorXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistArraySeparatorXHPASTLinterRule.php',
+    'ArcanistArrayValueXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistArrayValueXHPASTLinterRule.php',
     'ArcanistBackoutWorkflow' => 'workflow/ArcanistBackoutWorkflow.php',
     'ArcanistBaseCommitParser' => 'parser/ArcanistBaseCommitParser.php',
     'ArcanistBaseCommitParserTestCase' => 'parser/__tests__/ArcanistBaseCommitParserTestCase.php',
@@ -292,6 +293,7 @@ phutil_register_library_map(array(
     'ArcanistAnoidWorkflow' => 'ArcanistWorkflow',
     'ArcanistArrayIndexSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistArraySeparatorXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
+    'ArcanistArrayValueXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistBackoutWorkflow' => 'ArcanistWorkflow',
     'ArcanistBaseCommitParser' => 'Phobject',
     'ArcanistBaseCommitParserTestCase' => 'PhutilTestCase',
diff --git a/src/lint/linter/__tests__/xhpast/array-value.lint-test b/src/lint/linter/__tests__/xhpast/array-value.lint-test
new file mode 100644
index 00000000..a1339798
--- /dev/null
+++ b/src/lint/linter/__tests__/xhpast/array-value.lint-test
@@ -0,0 +1,48 @@
+<?php
+
+array(
+  1, 2, 3,
+);
+
+array(
+  'foo' => 'bar', 'bar' => 'baz',
+);
+
+array(
+  1, /* quack */ 2, /* quack */3,
+);
+
+array(
+  /* OPEN   */ 1,
+  /* CLOSED */ 2,
+);
+~~~~~~~~~~
+warning:4:5
+warning:4:8
+warning:8:18
+warning:12:17
+warning:12:32
+~~~~~~~~~~
+<?php
+
+array(
+  1,
+  2,
+  3,
+);
+
+array(
+  'foo' => 'bar',
+  'bar' => 'baz',
+);
+
+array(
+  1, /* quack */
+  2, /* quack */
+  3,
+);
+
+array(
+  /* OPEN   */ 1,
+  /* CLOSED */ 2,
+);
diff --git a/src/lint/linter/xhpast/rules/ArcanistArrayValueXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistArrayValueXHPASTLinterRule.php
new file mode 100644
index 00000000..21f50928
--- /dev/null
+++ b/src/lint/linter/xhpast/rules/ArcanistArrayValueXHPASTLinterRule.php
@@ -0,0 +1,56 @@
+<?php
+
+final class ArcanistArrayValueXHPASTLinterRule
+  extends ArcanistXHPASTLinterRule {
+
+  const ID = 76;
+
+  public function getLintName() {
+    return pht('Array Element');
+  }
+
+  public function getLintSeverity() {
+    return ArcanistLintSeverity::SEVERITY_WARNING;
+  }
+
+  public function process(XHPASTNode $root) {
+    $arrays = $root->selectDescendantsOfType('n_ARRAY_LITERAL');
+
+    foreach ($arrays as $array) {
+      $array_values = $array
+        ->getChildOfType(0, 'n_ARRAY_VALUE_LIST')
+        ->getChildrenOfType('n_ARRAY_VALUE');
+
+      if (!$array_values) {
+        // There is no need to check an empty array.
+        continue;
+      }
+
+      $multiline = $array->getLineNumber() != $array->getEndLineNumber();
+
+      if (!$multiline) {
+        continue;
+      }
+
+      foreach ($array_values as $value) {
+        list($before, $after) = $value->getSurroundingNonsemanticTokens();
+
+        if (strpos(implode('', mpull($before, 'getValue')), "\n") === false) {
+          if (last($before)->isAnyWhitespace()) {
+            $token = last($before);
+            $replacement = "\n".$value->getIndentation();
+          } else {
+            $token = head($value->getTokens());
+            $replacement = "\n".$value->getIndentation().$token->getValue();
+          }
+
+          $this->raiseLintAtToken(
+            $token,
+            pht('Array elements should each occupy a single line.'),
+            $replacement);
+        }
+      }
+    }
+  }
+
+}

From 504ff426818898a25c13a597392c7451e73a447e Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Tue, 11 Aug 2015 07:55:11 +1000
Subject: [PATCH 08/20] Minor improvement for array value linter rule

Summary:
This linter rule fails on multi-line arrays with no whitespace before the first array value. Specifically, the following exception is thrown:

```
Fatal error: Call to a member function isAnyWhitespace() on boolean in arcanist/src/lint/linter/xhpast/rules/ArcanistArrayValueXHPASTLinterRule.php on line 40
```

Test Plan: Added another test case.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D13856
---
 src/lint/linter/__tests__/xhpast/array-value.lint-test    | 8 ++++++++
 .../xhpast/rules/ArcanistArrayValueXHPASTLinterRule.php   | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/lint/linter/__tests__/xhpast/array-value.lint-test b/src/lint/linter/__tests__/xhpast/array-value.lint-test
index a1339798..9ab10ad7 100644
--- a/src/lint/linter/__tests__/xhpast/array-value.lint-test
+++ b/src/lint/linter/__tests__/xhpast/array-value.lint-test
@@ -16,12 +16,16 @@ array(
   /* OPEN   */ 1,
   /* CLOSED */ 2,
 );
+
+array('quack',
+);
 ~~~~~~~~~~
 warning:4:5
 warning:4:8
 warning:8:18
 warning:12:17
 warning:12:32
+warning:20:7
 ~~~~~~~~~~
 <?php
 
@@ -46,3 +50,7 @@ array(
   /* OPEN   */ 1,
   /* CLOSED */ 2,
 );
+
+array(
+'quack',
+);
diff --git a/src/lint/linter/xhpast/rules/ArcanistArrayValueXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistArrayValueXHPASTLinterRule.php
index 21f50928..6b960d74 100644
--- a/src/lint/linter/xhpast/rules/ArcanistArrayValueXHPASTLinterRule.php
+++ b/src/lint/linter/xhpast/rules/ArcanistArrayValueXHPASTLinterRule.php
@@ -36,7 +36,7 @@ final class ArcanistArrayValueXHPASTLinterRule
         list($before, $after) = $value->getSurroundingNonsemanticTokens();
 
         if (strpos(implode('', mpull($before, 'getValue')), "\n") === false) {
-          if (last($before)->isAnyWhitespace()) {
+          if (last($before) && last($before)->isAnyWhitespace()) {
             $token = last($before);
             $replacement = "\n".$value->getIndentation();
           } else {

From e4caf1a7d94f72718aa7908ade73d29c895f48df Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Tue, 11 Aug 2015 07:55:38 +1000
Subject: [PATCH 09/20] Automatically use the configuration driven unit test
 engine

Summary: Ref T5568. Use the `ArcanistConfigurationDrivenUnitTestEngine` automatically, if an `.arcunit` file exists. This behavior mimics the auto-detection for the configuration driven lint engine.

Test Plan:
Ran through the following scenarios:

  - Ran `arc unit` and saw unit tests execute.
  - Ran `arc unit --engine PhutilUnitTestEngine`
  - Remove the `.arcunit` file and ran `arc unit`... got an exception.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Maniphest Tasks: T5568

Differential Revision: https://secure.phabricator.com/D13855
---
 .arcconfig                            |  1 -
 src/workflow/ArcanistUnitWorkflow.php | 26 +------------
 src/workflow/ArcanistWorkflow.php     | 53 +++++++++++++++++++++++++++
 3 files changed, 54 insertions(+), 26 deletions(-)

diff --git a/.arcconfig b/.arcconfig
index 1309022c..66841c44 100644
--- a/.arcconfig
+++ b/.arcconfig
@@ -1,6 +1,5 @@
 {
   "phabricator.uri": "https://secure.phabricator.com/",
-  "unit.engine": "ArcanistConfigurationDrivenUnitTestEngine",
   "load": [
     "src/"
   ]
diff --git a/src/workflow/ArcanistUnitWorkflow.php b/src/workflow/ArcanistUnitWorkflow.php
index 567b772e..a97bbdeb 100644
--- a/src/workflow/ArcanistUnitWorkflow.php
+++ b/src/workflow/ArcanistUnitWorkflow.php
@@ -113,21 +113,8 @@ EOTEXT
   }
 
   public function run() {
-
     $working_copy = $this->getWorkingCopy();
 
-    $engine_class = $this->getArgument(
-      'engine',
-      $this->getConfigurationManager()->getConfigFromAnySource('unit.engine'));
-
-    if (!$engine_class) {
-      throw new ArcanistNoEngineException(
-        pht(
-          'No unit test engine is configured for this project. Edit %s '.
-          'to specify a unit test engine.',
-          '.arcconfig'));
-    }
-
     $paths = $this->getArgument('paths');
     $rev = $this->getArgument('rev');
     $everything = $this->getArgument('everything');
@@ -145,18 +132,7 @@ EOTEXT
       $paths = $this->selectPathsForWorkflow($paths, $rev);
     }
 
-    if (!class_exists($engine_class) ||
-        !is_subclass_of($engine_class, 'ArcanistUnitTestEngine')) {
-      throw new ArcanistUsageException(
-        pht(
-          "Configured unit test engine '%s' is not a subclass of '%s'.",
-          $engine_class,
-          'ArcanistUnitTestEngine'));
-    }
-
-    $this->engine = newv($engine_class, array());
-    $this->engine->setWorkingCopy($working_copy);
-    $this->engine->setConfigurationManager($this->getConfigurationManager());
+    $this->engine = $this->newUnitTestEngine($this->getArgument('engine'));
     if ($everything) {
       $this->engine->setRunAllTests(true);
     } else {
diff --git a/src/workflow/ArcanistWorkflow.php b/src/workflow/ArcanistWorkflow.php
index dc4913e2..87b37605 100644
--- a/src/workflow/ArcanistWorkflow.php
+++ b/src/workflow/ArcanistWorkflow.php
@@ -1878,6 +1878,59 @@ abstract class ArcanistWorkflow extends Phobject {
     return $engine;
   }
 
+  /**
+   * Build a new unit test engine for the current working copy.
+   *
+   * Optionally, you can pass an explicit engine class name to build an engine
+   * of a particular class. Normally this is used to implement an `--engine`
+   * flag from the CLI.
+   *
+   * @param string Optional explicit engine class name.
+   * @return ArcanistUnitTestEngine Constructed engine.
+   */
+  protected function newUnitTestEngine($engine_class = null) {
+    $working_copy = $this->getWorkingCopy();
+    $config = $this->getConfigurationManager();
+
+    if (!$engine_class) {
+      $engine_class = $config->getConfigFromAnySource('unit.engine');
+    }
+
+    if (!$engine_class) {
+      if (Filesystem::pathExists($working_copy->getProjectPath('.arcunit'))) {
+        $engine_class = 'ArcanistConfigurationDrivenUnitTestEngine';
+      }
+    }
+
+    if (!$engine_class) {
+      throw new ArcanistNoEngineException(
+        pht(
+          "No unit test engine is configured for this project. Create an ".
+          "'%s' file, or configure an advanced engine with '%s' in '%s'.",
+          '.arcunit',
+          'unit.engine',
+          '.arcconfig'));
+    }
+
+    $base_class = 'ArcanistUnitTestEngine';
+    if (!class_exists($engine_class) ||
+        !is_subclass_of($engine_class, $base_class)) {
+      throw new ArcanistUsageException(
+        pht(
+          'Configured unit test engine "%s" is not a subclass of "%s", '.
+          'but must be.',
+          $engine_class,
+          $base_class));
+    }
+
+    $engine = newv($engine_class, array())
+      ->setWorkingCopy($working_copy)
+      ->setConfigurationManager($config);
+
+    return $engine;
+  }
+
+
   protected function openURIsInBrowser(array $uris) {
     $browser = $this->getBrowserCommand();
     foreach ($uris as $uri) {

From de58fc809e378995a19019f5842dad89d348dd60 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 10 Aug 2015 15:35:15 -0700
Subject: [PATCH 10/20] Preserve more information when merging coverage

Summary:
Ref T8096. Each test reports coverage information, which we sometimes merge into a combined coverage report.

Usually, each test will report results for every line in the file, so if the file is 30 lines long, coverage is usually 30 characters long.

However, for whatever reason, tests might report results for only the first part of the file. This is allowed and we handle it properly.

Right now, if one test reports 10 lines of results and another reports 30 lines of results, we only use the first 10 lines of results. Instead, extend the merged coverage to include the extra 20 lines of results.

(This is an uncommon case which I only hit because I was manually banging on my keyboard to generate test data, but there's no reason not to handle it better.)

Test Plan: Used web UI, added + executed unit tests.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8096

Differential Revision: https://secure.phabricator.com/D13854
---
 src/__phutil_library_map__.php                |  2 +
 src/unit/ArcanistUnitTestResult.php           | 11 ++++-
 .../ArcanistUnitTestResultTestCase.php        | 43 +++++++++++++++++++
 3 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100644 src/unit/__tests__/ArcanistUnitTestResultTestCase.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 422c792f..aa38462b 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -247,6 +247,7 @@ phutil_register_library_map(array(
     'ArcanistUnitRenderer' => 'unit/renderer/ArcanistUnitRenderer.php',
     'ArcanistUnitTestEngine' => 'unit/engine/ArcanistUnitTestEngine.php',
     'ArcanistUnitTestResult' => 'unit/ArcanistUnitTestResult.php',
+    'ArcanistUnitTestResultTestCase' => 'unit/__tests__/ArcanistUnitTestResultTestCase.php',
     'ArcanistUnitTestableLintEngine' => 'lint/engine/ArcanistUnitTestableLintEngine.php',
     'ArcanistUnitWorkflow' => 'workflow/ArcanistUnitWorkflow.php',
     'ArcanistUnnecessaryFinalModifierXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistUnnecessaryFinalModifierXHPASTLinterRule.php',
@@ -525,6 +526,7 @@ phutil_register_library_map(array(
     'ArcanistUnitRenderer' => 'Phobject',
     'ArcanistUnitTestEngine' => 'Phobject',
     'ArcanistUnitTestResult' => 'Phobject',
+    'ArcanistUnitTestResultTestCase' => 'PhutilTestCase',
     'ArcanistUnitTestableLintEngine' => 'ArcanistLintEngine',
     'ArcanistUnitWorkflow' => 'ArcanistWorkflow',
     'ArcanistUnnecessaryFinalModifierXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
diff --git a/src/unit/ArcanistUnitTestResult.php b/src/unit/ArcanistUnitTestResult.php
index 039393f8..a0dc2f72 100644
--- a/src/unit/ArcanistUnitTestResult.php
+++ b/src/unit/ArcanistUnitTestResult.php
@@ -129,13 +129,22 @@ final class ArcanistUnitTestResult extends Phobject {
 
     $base = reset($coverage);
     foreach ($coverage as $more_coverage) {
-      $len = min(strlen($base), strlen($more_coverage));
+      $base_len = strlen($base);
+      $more_len = strlen($more_coverage);
+
+      $len = min($base_len, $more_len);
       for ($ii = 0; $ii < $len; $ii++) {
         if ($more_coverage[$ii] == 'C') {
           $base[$ii] = 'C';
         }
       }
+
+      // If a secondary report has more data, copy all of it over.
+      if ($more_len > $base_len) {
+        $base .= substr($more_coverage, $base_len);
+      }
     }
+
     return $base;
   }
 
diff --git a/src/unit/__tests__/ArcanistUnitTestResultTestCase.php b/src/unit/__tests__/ArcanistUnitTestResultTestCase.php
new file mode 100644
index 00000000..7c4d43dd
--- /dev/null
+++ b/src/unit/__tests__/ArcanistUnitTestResultTestCase.php
@@ -0,0 +1,43 @@
+<?php
+
+final class ArcanistUnitTestResultTestCase extends PhutilTestCase {
+
+  public function testCoverageMerges() {
+    $cases = array(
+      array(
+        'coverage' => array(),
+        'expect' => null,
+      ),
+      array(
+        'coverage' => array(
+          'UUUNCNC',
+        ),
+        'expect' => 'UUUNCNC',
+      ),
+      array(
+        'coverage' => array(
+          'UUCUUU',
+          'UUUUCU',
+        ),
+        'expect' => 'UUCUCU',
+      ),
+      array(
+        'coverage' => array(
+          'UUCCCU',
+          'UUUCCCNNNC',
+        ),
+        'expect' => 'UUCCCCNNNC',
+      ),
+    );
+
+    foreach ($cases as $case) {
+      $input = $case['coverage'];
+      $expect = $case['expect'];
+
+      $actual = ArcanistUnitTestResult::mergeCoverage($input);
+
+      $this->assertEqual($expect, $actual);
+    }
+  }
+
+}

From b82e4cd40e1fec6fbd2e439d0e8905b10bd4f8ea Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Tue, 11 Aug 2015 22:33:12 +1000
Subject: [PATCH 11/20] Fix failing JSHint linter test

Summary: This test case is failing on JSHint v2.8.0.

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D13860
---
 src/lint/linter/__tests__/jshint/jshint.lint-test | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/lint/linter/__tests__/jshint/jshint.lint-test b/src/lint/linter/__tests__/jshint/jshint.lint-test
index fa6d2249..37c13892 100644
--- a/src/lint/linter/__tests__/jshint/jshint.lint-test
+++ b/src/lint/linter/__tests__/jshint/jshint.lint-test
@@ -9,3 +9,4 @@ function f() {
 ~~~~~~~~~~
 warning:3:8
 error:7:1
+error:9:

From 31a49196801a612ae00e241013b099f9aa0364c1 Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Tue, 11 Aug 2015 22:34:38 +1000
Subject: [PATCH 12/20] Improve linter rules for array formatting

Summary: Modify `ArcanistParenthesesSpacingXHPASTLinterRule` and `ArcanistCallParenthesesXHPASTLinterRule` to apply to `array()` and `list()` as well. Depends on D13858.

Test Plan: Added unit tests.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D13859
---
 .../phlxhp/array-formatting.lint-test         | 16 +++++++
 ...rcanistCallParenthesesXHPASTLinterRule.php | 42 ++++++++++++++-----
 ...nistParenthesesSpacingXHPASTLinterRule.php |  4 +-
 ...anistConfigurationDrivenUnitTestEngine.php | 17 ++++----
 4 files changed, 60 insertions(+), 19 deletions(-)
 create mode 100644 src/lint/linter/__tests__/phlxhp/array-formatting.lint-test

diff --git a/src/lint/linter/__tests__/phlxhp/array-formatting.lint-test b/src/lint/linter/__tests__/phlxhp/array-formatting.lint-test
new file mode 100644
index 00000000..3dda317d
--- /dev/null
+++ b/src/lint/linter/__tests__/phlxhp/array-formatting.lint-test
@@ -0,0 +1,16 @@
+<?php
+
+array ( 1, 2, 3 );
+list ( $x, $y ) = array();
+~~~~~~~~~~
+warning:3:6
+warning:3:8
+warning:3:16
+warning:4:5
+warning:4:7
+warning:4:14
+~~~~~~~~~~
+<?php
+
+array(1, 2, 3);
+list($x, $y) = array();
diff --git a/src/lint/linter/xhpast/rules/ArcanistCallParenthesesXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistCallParenthesesXHPASTLinterRule.php
index 5d6bc2cf..67cb3676 100644
--- a/src/lint/linter/xhpast/rules/ArcanistCallParenthesesXHPASTLinterRule.php
+++ b/src/lint/linter/xhpast/rules/ArcanistCallParenthesesXHPASTLinterRule.php
@@ -14,13 +14,33 @@ final class ArcanistCallParenthesesXHPASTLinterRule
   }
 
   public function process(XHPASTNode $root) {
-    $calls = $root->selectDescendantsOfTypes(array(
+    $nodes = $root->selectDescendantsOfTypes(array(
+      'n_ARRAY_LITERAL',
       'n_FUNCTION_CALL',
       'n_METHOD_CALL',
+      'n_LIST',
     ));
 
-    foreach ($calls as $call) {
-      $params = $call->getChildOfType(1, 'n_CALL_PARAMETER_LIST');
+    foreach ($nodes as $node) {
+      switch ($node->getTypeName()) {
+        case 'n_ARRAY_LITERAL':
+          $params = $node->getChildOfType(0, 'n_ARRAY_VALUE_LIST');
+          break;
+
+        case 'n_FUNCTION_CALL':
+        case 'n_METHOD_CALL':
+          $params = $node->getChildOfType(1, 'n_CALL_PARAMETER_LIST');
+          break;
+
+        case 'n_LIST':
+          $params = $node->getChildOfType(0, 'n_ASSIGNMENT_LIST');
+          break;
+
+        default:
+          throw new Exception(
+            pht("Unexpected node of type '%s'!", $node->getTypeName()));
+      }
+
       $tokens = $params->getTokens();
       $first  = head($tokens);
 
@@ -29,17 +49,13 @@ final class ArcanistCallParenthesesXHPASTLinterRule
       if (preg_match('/^\s+$/', $leading_text)) {
         $this->raiseLintAtOffset(
           $first->getOffset() - strlen($leading_text),
-          pht('Convention: no spaces before opening parenthesis in calls.'),
+          pht('Convention: no spaces before opening parentheses.'),
           $leading_text,
           '');
       }
-    }
 
-    foreach ($calls as $call) {
       // If the last parameter of a call is a HEREDOC, don't apply this rule.
-      $params = $call
-        ->getChildOfType(1, 'n_CALL_PARAMETER_LIST')
-        ->getChildren();
+      $params = $params->getChildren();
 
       if ($params) {
         $last_param = last($params);
@@ -48,15 +64,19 @@ final class ArcanistCallParenthesesXHPASTLinterRule
         }
       }
 
-      $tokens = $call->getTokens();
+      $tokens = $node->getTokens();
       $last = array_pop($tokens);
 
+      if ($node->getTypeName() == 'n_ARRAY_LITERAL') {
+        continue;
+      }
+
       $trailing = $last->getNonsemanticTokensBefore();
       $trailing_text = implode('', mpull($trailing, 'getValue'));
       if (preg_match('/^\s+$/', $trailing_text)) {
         $this->raiseLintAtOffset(
           $last->getOffset() - strlen($trailing_text),
-          pht('Convention: no spaces before closing parenthesis in calls.'),
+          pht('Convention: no spaces before closing parentheses.'),
           $trailing_text,
           '');
       }
diff --git a/src/lint/linter/xhpast/rules/ArcanistParenthesesSpacingXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistParenthesesSpacingXHPASTLinterRule.php
index b0bd8340..7ad16a44 100644
--- a/src/lint/linter/xhpast/rules/ArcanistParenthesesSpacingXHPASTLinterRule.php
+++ b/src/lint/linter/xhpast/rules/ArcanistParenthesesSpacingXHPASTLinterRule.php
@@ -15,11 +15,13 @@ final class ArcanistParenthesesSpacingXHPASTLinterRule
 
   public function process(XHPASTNode $root) {
     $all_paren_groups = $root->selectDescendantsOfTypes(array(
+      'n_ARRAY_VALUE_LIST',
+      'n_ASSIGNMENT_LIST',
       'n_CALL_PARAMETER_LIST',
+      'n_DECLARATION_PARAMETER_LIST',
       'n_CONTROL_CONDITION',
       'n_FOR_EXPRESSION',
       'n_FOREACH_EXPRESSION',
-      'n_DECLARATION_PARAMETER_LIST',
     ));
 
     foreach ($all_paren_groups as $group) {
diff --git a/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php b/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php
index 9c5ed8ab..e24b4d5f 100644
--- a/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php
+++ b/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php
@@ -97,14 +97,17 @@ final class ArcanistConfigurationDrivenUnitTestEngine
           $ex);
       }
 
-      $include = (array)idx($spec, 'include', array());
-      $exclude = (array)idx($spec, 'exclude', array());
-      $paths = $this->matchPaths(
-        $all_paths,
-        $include,
-        $exclude);
+      if ($all_paths) {
+        $include = (array)idx($spec, 'include', array());
+        $exclude = (array)idx($spec, 'exclude', array());
+        $paths = $this->matchPaths(
+          $all_paths,
+          $include,
+          $exclude);
+
+        $test_engine->setPaths($paths);
+      }
 
-      $test_engine->setPaths($paths);
       $built_test_engines[] = $test_engine;
     }
 

From e00e2939c164df85852ce2d157de4197b399c30b Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Tue, 11 Aug 2015 22:35:40 +1000
Subject: [PATCH 13/20] Various linter fixes

Summary: Self explanatory.

Test Plan: `arc lint`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D13861
---
 .../__tests__/ArcanistLinterTestCase.php      |  2 +-
 src/parser/ArcanistBundle.php                 | 95 +++++++++++++++++--
 src/repository/api/ArcanistMercurialAPI.php   |  6 +-
 3 files changed, 89 insertions(+), 14 deletions(-)

diff --git a/src/lint/linter/__tests__/ArcanistLinterTestCase.php b/src/lint/linter/__tests__/ArcanistLinterTestCase.php
index c023efd1..d4007d2e 100644
--- a/src/lint/linter/__tests__/ArcanistLinterTestCase.php
+++ b/src/lint/linter/__tests__/ArcanistLinterTestCase.php
@@ -65,7 +65,7 @@ abstract class ArcanistLinterTestCase extends PhutilTestCase {
           '~~~~~~~~~~'));
     }
 
-    list ($data, $expect, $xform, $config) = array_merge(
+    list($data, $expect, $xform, $config) = array_merge(
       $contents,
       array(null, null));
 
diff --git a/src/parser/ArcanistBundle.php b/src/parser/ArcanistBundle.php
index a4d85581..8586e281 100644
--- a/src/parser/ArcanistBundle.php
+++ b/src/parser/ArcanistBundle.php
@@ -819,16 +819,91 @@ final class ArcanistBundle extends Phobject {
     // additional casting.)
 
     static $map = array(
-      '0', '1', '2', '3', '4', '5', '6', '7', '8', '9',
-      'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J',
-      'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T',
-      'U', 'V', 'W', 'X', 'Y', 'Z',
-      'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j',
-      'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't',
-      'u', 'v', 'w', 'x', 'y', 'z',
-      '!', '#', '$', '%', '&', '(', ')', '*', '+', '-',
-      ';', '<', '=', '>', '?', '@', '^', '_', '`', '{',
-      '|', '}', '~',
+      '0',
+      '1',
+      '2',
+      '3',
+      '4',
+      '5',
+      '6',
+      '7',
+      '8',
+      '9',
+      'A',
+      'B',
+      'C',
+      'D',
+      'E',
+      'F',
+      'G',
+      'H',
+      'I',
+      'J',
+      'K',
+      'L',
+      'M',
+      'N',
+      'O',
+      'P',
+      'Q',
+      'R',
+      'S',
+      'T',
+      'U',
+      'V',
+      'W',
+      'X',
+      'Y',
+      'Z',
+      'a',
+      'b',
+      'c',
+      'd',
+      'e',
+      'f',
+      'g',
+      'h',
+      'i',
+      'j',
+      'k',
+      'l',
+      'm',
+      'n',
+      'o',
+      'p',
+      'q',
+      'r',
+      's',
+      't',
+      'u',
+      'v',
+      'w',
+      'x',
+      'y',
+      'z',
+      '!',
+      '#',
+      '$',
+      '%',
+      '&',
+      '(',
+      ')',
+      '*',
+      '+',
+      '-',
+      ';',
+      '<',
+      '=',
+      '>',
+      '?',
+      '@',
+      '^',
+      '_',
+      '`',
+      '{',
+      '|',
+      '}',
+      '~',
     );
 
     $buf = '';
diff --git a/src/repository/api/ArcanistMercurialAPI.php b/src/repository/api/ArcanistMercurialAPI.php
index 4abcea04..33105c20 100644
--- a/src/repository/api/ArcanistMercurialAPI.php
+++ b/src/repository/api/ArcanistMercurialAPI.php
@@ -250,7 +250,7 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
         list($node, $rev, $full_author, $date, $branch, $tag,
           $parents, $desc) = explode("\1", $log, 9);
 
-        list ($author, $author_email) = $this->parseFullAuthor($full_author);
+        list($author, $author_email) = $this->parseFullAuthor($full_author);
 
         // NOTE: If a commit has only one parent, {parents} returns empty.
         // If it has two parents, {parents} returns revs and short hashes, not
@@ -535,7 +535,7 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
 
   public function supportsRebase() {
     if ($this->supportsRebase === null) {
-      list ($err) = $this->execManualLocal('help rebase');
+      list($err) = $this->execManualLocal('help rebase');
       $this->supportsRebase = $err === 0;
     }
 
@@ -544,7 +544,7 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
 
   public function supportsPhases() {
     if ($this->supportsPhases === null) {
-      list ($err) = $this->execManualLocal('help phase');
+      list($err) = $this->execManualLocal('help phase');
       $this->supportsPhases = $err === 0;
     }
 

From 01c3f4120741697107316aee7effc77f2bc4cd79 Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Tue, 11 Aug 2015 22:37:28 +1000
Subject: [PATCH 14/20] Fix `arc unit --everything`

Summary: This was broken in D13573.

Test Plan: Ran `arc unit --everything` in rPHU.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D13864
---
 src/unit/engine/PhutilUnitTestEngine.php | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/unit/engine/PhutilUnitTestEngine.php b/src/unit/engine/PhutilUnitTestEngine.php
index eb00f79c..110f8e35 100644
--- a/src/unit/engine/PhutilUnitTestEngine.php
+++ b/src/unit/engine/PhutilUnitTestEngine.php
@@ -80,9 +80,11 @@ final class PhutilUnitTestEngine extends ArcanistUnitTestEngine {
   private function getAllTests() {
     $project_root = $this->getWorkingCopy()->getProjectRoot();
 
-    $symbols = id(new PhutilClassMapQuery())
+    $symbols = id(new PhutilSymbolLoader())
+      ->setType('class')
       ->setAncestorClass('PhutilTestCase')
-      ->execute();
+      ->setConcreteOnly(true)
+      ->selectSymbolsWithoutLoading();
 
     $in_working_copy = array();
 

From 423e7d2389336430110f489e411fe0d66a25ade3 Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Tue, 11 Aug 2015 22:50:57 +1000
Subject: [PATCH 15/20] Move a test file

Summary: This is a test case for `ArcanistXHPASTLinter`, not `ArcanistPhutilXHPASTLinter`.

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D13866
---
 .../__tests__/{phlxhp => xhpast}/array-formatting.lint-test       | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename src/lint/linter/__tests__/{phlxhp => xhpast}/array-formatting.lint-test (100%)

diff --git a/src/lint/linter/__tests__/phlxhp/array-formatting.lint-test b/src/lint/linter/__tests__/xhpast/array-formatting.lint-test
similarity index 100%
rename from src/lint/linter/__tests__/phlxhp/array-formatting.lint-test
rename to src/lint/linter/__tests__/xhpast/array-formatting.lint-test

From 807057087d65298e69bb8260db81bee1ae109084 Mon Sep 17 00:00:00 2001
From: Aiden Scandella <ai@uber.com>
Date: Tue, 11 Aug 2015 14:49:48 -0700
Subject: [PATCH 16/20] Restore default "yes" behavior for lint patch

Summary:
Fixes T9029

See T9029 for more details, but basically at some point phutil_console_confirm
(which takes a `$default_no` parameter) was refactored to `$console->confirm()`
which takes a `$default` parameter with semantics negated..

Test Plan:
Run `arc lint` on a repository where patch is suggested. Default
option should be "[Y/n]" to accept the patch.

Reviewers: joshuaspence, #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Maniphest Tasks: T9029

Differential Revision: https://secure.phabricator.com/D13873
---
 src/workflow/ArcanistLintWorkflow.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/workflow/ArcanistLintWorkflow.php b/src/workflow/ArcanistLintWorkflow.php
index f1578c37..27db427d 100644
--- a/src/workflow/ArcanistLintWorkflow.php
+++ b/src/workflow/ArcanistLintWorkflow.php
@@ -526,7 +526,7 @@ EOTEXT
           $prompt = pht(
             'Apply this patch to %s?',
             phutil_console_format('__%s__', $result->getPath()));
-          if (!$console->confirm($prompt, $default_no = false)) {
+          if (!$console->confirm($prompt, $default = true)) {
             continue;
           }
         }

From f4c322cb72cd479080cedb52018bcb1435043192 Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Wed, 12 Aug 2015 13:22:37 +1000
Subject: [PATCH 17/20] Add a linter rule for list assignments

Summary: Add a linter rule to prevent trailing commas in a list assignment. `list($x, $y,)` is equivalent to `list($x, $y)`, but the latter is cleaner and should be preferred.

Test Plan: Added test cases.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D13870
---
 src/__phutil_library_map__.php                |  2 +
 .../xhpast/list-assignment.lint-test          | 11 ++++++
 ...ArcanistListAssignmentXHPASTLinterRule.php | 38 +++++++++++++++++++
 3 files changed, 51 insertions(+)
 create mode 100644 src/lint/linter/__tests__/xhpast/list-assignment.lint-test
 create mode 100644 src/lint/linter/xhpast/rules/ArcanistListAssignmentXHPASTLinterRule.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index aa38462b..c4b3d8b1 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -156,6 +156,7 @@ phutil_register_library_map(array(
     'ArcanistLinter' => 'lint/linter/ArcanistLinter.php',
     'ArcanistLinterTestCase' => 'lint/linter/__tests__/ArcanistLinterTestCase.php',
     'ArcanistLintersWorkflow' => 'workflow/ArcanistLintersWorkflow.php',
+    'ArcanistListAssignmentXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistListAssignmentXHPASTLinterRule.php',
     'ArcanistListWorkflow' => 'workflow/ArcanistListWorkflow.php',
     'ArcanistLogicalOperatorsXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistLogicalOperatorsXHPASTLinterRule.php',
     'ArcanistLowercaseFunctionsXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistLowercaseFunctionsXHPASTLinterRule.php',
@@ -435,6 +436,7 @@ phutil_register_library_map(array(
     'ArcanistLinter' => 'Phobject',
     'ArcanistLinterTestCase' => 'PhutilTestCase',
     'ArcanistLintersWorkflow' => 'ArcanistWorkflow',
+    'ArcanistListAssignmentXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistListWorkflow' => 'ArcanistWorkflow',
     'ArcanistLogicalOperatorsXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistLowercaseFunctionsXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
diff --git a/src/lint/linter/__tests__/xhpast/list-assignment.lint-test b/src/lint/linter/__tests__/xhpast/list-assignment.lint-test
new file mode 100644
index 00000000..b9871b03
--- /dev/null
+++ b/src/lint/linter/__tests__/xhpast/list-assignment.lint-test
@@ -0,0 +1,11 @@
+<?php
+list($x, $y, , ,) = array();
+list($x, $y, , , $z) = array();
+~~~~~~~~~~
+warning:2:12
+warning:2:14
+warning:2:16
+~~~~~~~~~~
+<?php
+list($x, $y  ) = array();
+list($x, $y, , , $z) = array();
diff --git a/src/lint/linter/xhpast/rules/ArcanistListAssignmentXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistListAssignmentXHPASTLinterRule.php
new file mode 100644
index 00000000..152c09a6
--- /dev/null
+++ b/src/lint/linter/xhpast/rules/ArcanistListAssignmentXHPASTLinterRule.php
@@ -0,0 +1,38 @@
+<?php
+
+final class ArcanistListAssignmentXHPASTLinterRule
+  extends ArcanistXHPASTLinterRule {
+
+  const ID = 77;
+
+  public function getLintName() {
+    return pht('List Assignment');
+  }
+
+  public function getLintSeverity() {
+    return ArcanistLintSeverity::SEVERITY_WARNING;
+  }
+
+  public function process(XHPASTNode $root) {
+    $assignment_lists = $root->selectDescendantsOfType('n_ASSIGNMENT_LIST');
+
+    foreach ($assignment_lists as $assignment_list) {
+      $tokens = array_slice($assignment_list->getTokens(), 1, -1);
+
+      foreach (array_reverse($tokens) as $token) {
+        if ($token->getTypeName() == ',') {
+          $this->raiseLintAtToken(
+            $token,
+            pht('Unnecessary comma in list assignment.'),
+            '');
+          continue;
+        }
+
+        if ($token->isSemantic()) {
+          break;
+        }
+      }
+    }
+  }
+
+}

From bf88f4616cdfc8a391012f768e5e226aa0fb9a52 Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Fri, 14 Aug 2015 07:37:48 +1000
Subject: [PATCH 18/20] Add a linter rule for global variables

Summary: Ref T7409. Global variables are gross and should be avoided like the plague.

Test Plan: Added test cases.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Maniphest Tasks: T7409

Differential Revision: https://secure.phabricator.com/D13882
---
 src/__phutil_library_map__.php                |  2 ++
 .../xhpast/global-variables.lint-test         |  8 ++++++
 .../xhpast/naming-conventions.lint-test       |  1 +
 .../xhpast/undeclared-variables.lint-test     |  2 ++
 ...ArcanistGlobalVariableXHPASTLinterRule.php | 28 +++++++++++++++++++
 5 files changed, 41 insertions(+)
 create mode 100644 src/lint/linter/__tests__/xhpast/global-variables.lint-test
 create mode 100644 src/lint/linter/xhpast/rules/ArcanistGlobalVariableXHPASTLinterRule.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index c4b3d8b1..70f87173 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -110,6 +110,7 @@ phutil_register_library_map(array(
     'ArcanistGeneratedLinterTestCase' => 'lint/linter/__tests__/ArcanistGeneratedLinterTestCase.php',
     'ArcanistGetConfigWorkflow' => 'workflow/ArcanistGetConfigWorkflow.php',
     'ArcanistGitAPI' => 'repository/api/ArcanistGitAPI.php',
+    'ArcanistGlobalVariableXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistGlobalVariableXHPASTLinterRule.php',
     'ArcanistGoLintLinter' => 'lint/linter/ArcanistGoLintLinter.php',
     'ArcanistGoLintLinterTestCase' => 'lint/linter/__tests__/ArcanistGoLintLinterTestCase.php',
     'ArcanistGoTestResultParser' => 'unit/parser/ArcanistGoTestResultParser.php',
@@ -390,6 +391,7 @@ phutil_register_library_map(array(
     'ArcanistGeneratedLinterTestCase' => 'ArcanistLinterTestCase',
     'ArcanistGetConfigWorkflow' => 'ArcanistWorkflow',
     'ArcanistGitAPI' => 'ArcanistRepositoryAPI',
+    'ArcanistGlobalVariableXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistGoLintLinter' => 'ArcanistExternalLinter',
     'ArcanistGoLintLinterTestCase' => 'ArcanistExternalLinterTestCase',
     'ArcanistGoTestResultParser' => 'ArcanistTestResultParser',
diff --git a/src/lint/linter/__tests__/xhpast/global-variables.lint-test b/src/lint/linter/__tests__/xhpast/global-variables.lint-test
new file mode 100644
index 00000000..863cf5fd
--- /dev/null
+++ b/src/lint/linter/__tests__/xhpast/global-variables.lint-test
@@ -0,0 +1,8 @@
+<?php
+global $BASE_URL;
+function foo() {
+  global $x, $y;
+}
+~~~~~~~~~~
+warning:2:1
+warning:4:3
diff --git a/src/lint/linter/__tests__/xhpast/naming-conventions.lint-test b/src/lint/linter/__tests__/xhpast/naming-conventions.lint-test
index cc78e403..2cd47e36 100644
--- a/src/lint/linter/__tests__/xhpast/naming-conventions.lint-test
+++ b/src/lint/linter/__tests__/xhpast/naming-conventions.lint-test
@@ -72,4 +72,5 @@ warning:29:3
 warning:30:3
 warning:31:3
 warning:33:3
+warning:37:3
 warning:55:3
diff --git a/src/lint/linter/__tests__/xhpast/undeclared-variables.lint-test b/src/lint/linter/__tests__/xhpast/undeclared-variables.lint-test
index c6cd1b40..1bedf0fa 100644
--- a/src/lint/linter/__tests__/xhpast/undeclared-variables.lint-test
+++ b/src/lint/linter/__tests__/xhpast/undeclared-variables.lint-test
@@ -180,6 +180,7 @@ function some_func($x, $y) {
   };
 }
 ~~~~~~~~~~
+warning:9:3
 error:28:3
 error:30:3
 error:36:3
@@ -188,6 +189,7 @@ error:43:7
 error:44:5
 error:45:5
 error:46:10
+warning:51:3
 error:51:10 worst ever
 warning:61:3
 error:87:3 This stuff is basically testing the lexer/parser for function decls.
diff --git a/src/lint/linter/xhpast/rules/ArcanistGlobalVariableXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistGlobalVariableXHPASTLinterRule.php
new file mode 100644
index 00000000..7a68bf17
--- /dev/null
+++ b/src/lint/linter/xhpast/rules/ArcanistGlobalVariableXHPASTLinterRule.php
@@ -0,0 +1,28 @@
+<?php
+
+final class ArcanistGlobalVariableXHPASTLinterRule
+  extends ArcanistXHPASTLinterRule {
+
+  const ID = 79;
+
+  public function getLintName() {
+    return pht('Global Variables');
+  }
+
+  public function getLintSeverity() {
+    return ArcanistLintSeverity::SEVERITY_WARNING;
+  }
+
+  public function process(XHPASTNode $root) {
+    $nodes = $root->selectDescendantsOfType('n_GLOBAL_DECLARATION_LIST');
+
+    foreach ($nodes as $node) {
+      $this->raiseLintAtNode(
+        $node,
+        pht(
+          'Limit the use of global variables. Global variables are '.
+          'generally a bad idea and should be avoided when possible.'));
+    }
+  }
+
+}

From 2e76e2965cbee3672a2aece8c56850490f707232 Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Fri, 14 Aug 2015 07:41:41 +1000
Subject: [PATCH 19/20] Add a linter rule for inline HTML

Summary: Ref T7409. Based on [[https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Generic/Sniffs/Files/InlineHTMLSniff.php | InlineHTMLSniff]].

Test Plan: Added unit tests.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin

Maniphest Tasks: T9140, T7409

Differential Revision: https://secure.phabricator.com/D13871
---
 src/__phutil_library_map__.php                |  2 ++
 .../ArcanistInlineHTMLXHPASTLinterRule.php    | 31 +++++++++++++++++++
 .../__tests__/xhpast/inline-html.lint-test    |  4 +++
 3 files changed, 37 insertions(+)
 create mode 100644 src/lint/linter/ArcanistInlineHTMLXHPASTLinterRule.php
 create mode 100644 src/lint/linter/__tests__/xhpast/inline-html.lint-test

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 70f87173..97a653f0 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -125,6 +125,7 @@ phutil_register_library_map(array(
     'ArcanistImplicitConstructorXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistImplicitConstructorXHPASTLinterRule.php',
     'ArcanistImplicitFallthroughXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistImplicitFallthroughXHPASTLinterRule.php',
     'ArcanistImplicitVisibilityXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistImplicitVisibilityXHPASTLinterRule.php',
+    'ArcanistInlineHTMLXHPASTLinterRule' => 'lint/linter/ArcanistInlineHTMLXHPASTLinterRule.php',
     'ArcanistInnerFunctionXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistInnerFunctionXHPASTLinterRule.php',
     'ArcanistInstallCertificateWorkflow' => 'workflow/ArcanistInstallCertificateWorkflow.php',
     'ArcanistInstanceOfOperatorXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistInstanceOfOperatorXHPASTLinterRule.php',
@@ -406,6 +407,7 @@ phutil_register_library_map(array(
     'ArcanistImplicitConstructorXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistImplicitFallthroughXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistImplicitVisibilityXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
+    'ArcanistInlineHTMLXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistInnerFunctionXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistInstallCertificateWorkflow' => 'ArcanistWorkflow',
     'ArcanistInstanceOfOperatorXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
diff --git a/src/lint/linter/ArcanistInlineHTMLXHPASTLinterRule.php b/src/lint/linter/ArcanistInlineHTMLXHPASTLinterRule.php
new file mode 100644
index 00000000..3cb758ad
--- /dev/null
+++ b/src/lint/linter/ArcanistInlineHTMLXHPASTLinterRule.php
@@ -0,0 +1,31 @@
+<?php
+
+final class ArcanistInlineHTMLXHPASTLinterRule
+  extends ArcanistXHPASTLinterRule {
+
+  const ID = 78;
+
+  public function getLintName() {
+    return pht('Inline HTML');
+  }
+
+  public function getLintSeverity() {
+    return ArcanistLintSeverity::SEVERITY_DISABLED;
+  }
+
+  public function process(XHPASTNode $root) {
+    $inline_html = $root->selectTokensOfType('T_INLINE_HTML');
+
+    foreach ($inline_html as $html) {
+      if (substr($html->getValue(), 0, 2) == '#!') {
+        // Ignore shebang lines.
+        continue;
+      }
+
+      $this->raiseLintAtToken(
+        $html,
+        pht('PHP files must only contain PHP code.'));
+    }
+  }
+
+}
diff --git a/src/lint/linter/__tests__/xhpast/inline-html.lint-test b/src/lint/linter/__tests__/xhpast/inline-html.lint-test
new file mode 100644
index 00000000..13cdd0d1
--- /dev/null
+++ b/src/lint/linter/__tests__/xhpast/inline-html.lint-test
@@ -0,0 +1,4 @@
+#!/usr/bin/env php
+<html>
+~~~~~~~~~~
+disabled:2:1

From fe8ed2a6f8b09b8c56e476ed1f9624d35732b776 Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Fri, 14 Aug 2015 07:44:19 +1000
Subject: [PATCH 20/20] Add a linter rule for the use of `parse_str`

Summary: The use of the [[http://php.net/manual/en/function.parse-str.php | parse_str]] method (when called without sepcifying a second parameter) hinders static analysis. Specifically, the `parse_str('...')` behaves similarly to [[http://php.net/manual/en/function.extract.php | extract]].

Test Plan: Added unit tests.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D13857
---
 src/__phutil_library_map__.php                |  2 ++
 .../__tests__/xhpast/extract-use.lint-test    |  5 ++++
 .../__tests__/xhpast/parse_str-use.lint-test  |  7 +++++
 .../ArcanistParseStrUseXHPASTLinterRule.php   | 29 +++++++++++++++++++
 ...nistUndeclaredVariableXHPASTLinterRule.php |  2 ++
 5 files changed, 45 insertions(+)
 create mode 100644 src/lint/linter/__tests__/xhpast/extract-use.lint-test
 create mode 100644 src/lint/linter/__tests__/xhpast/parse_str-use.lint-test
 create mode 100644 src/lint/linter/xhpast/rules/ArcanistParseStrUseXHPASTLinterRule.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 97a653f0..ca5061e4 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -185,6 +185,7 @@ phutil_register_library_map(array(
     'ArcanistPHPOpenTagXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPHPOpenTagXHPASTLinterRule.php',
     'ArcanistPHPShortTagXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPHPShortTagXHPASTLinterRule.php',
     'ArcanistParenthesesSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistParenthesesSpacingXHPASTLinterRule.php',
+    'ArcanistParseStrUseXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistParseStrUseXHPASTLinterRule.php',
     'ArcanistPasteWorkflow' => 'workflow/ArcanistPasteWorkflow.php',
     'ArcanistPatchWorkflow' => 'workflow/ArcanistPatchWorkflow.php',
     'ArcanistPhpLinter' => 'lint/linter/ArcanistPhpLinter.php',
@@ -467,6 +468,7 @@ phutil_register_library_map(array(
     'ArcanistPHPOpenTagXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistPHPShortTagXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistParenthesesSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
+    'ArcanistParseStrUseXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistPasteWorkflow' => 'ArcanistWorkflow',
     'ArcanistPatchWorkflow' => 'ArcanistWorkflow',
     'ArcanistPhpLinter' => 'ArcanistExternalLinter',
diff --git a/src/lint/linter/__tests__/xhpast/extract-use.lint-test b/src/lint/linter/__tests__/xhpast/extract-use.lint-test
new file mode 100644
index 00000000..43f13e6c
--- /dev/null
+++ b/src/lint/linter/__tests__/xhpast/extract-use.lint-test
@@ -0,0 +1,5 @@
+<?php
+
+extract();
+~~~~~~~~~~
+error:3:1
diff --git a/src/lint/linter/__tests__/xhpast/parse_str-use.lint-test b/src/lint/linter/__tests__/xhpast/parse_str-use.lint-test
new file mode 100644
index 00000000..21afe525
--- /dev/null
+++ b/src/lint/linter/__tests__/xhpast/parse_str-use.lint-test
@@ -0,0 +1,7 @@
+<?php
+
+$params = array();
+parse_str('foo=bar', $params);
+parse_str('foo=bar');
+~~~~~~~~~~
+error:5:1
diff --git a/src/lint/linter/xhpast/rules/ArcanistParseStrUseXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistParseStrUseXHPASTLinterRule.php
new file mode 100644
index 00000000..9656e4a7
--- /dev/null
+++ b/src/lint/linter/xhpast/rules/ArcanistParseStrUseXHPASTLinterRule.php
@@ -0,0 +1,29 @@
+<?php
+
+final class ArcanistParseStrUseXHPASTLinterRule
+  extends ArcanistXHPASTLinterRule {
+
+  const ID = 80;
+
+  public function getLintName() {
+    return pht('Questionable Use of %s', 'parse_str()');
+  }
+
+  public function process(XHPASTNode $root) {
+    $calls = $this->getFunctionCalls($root, array('parse_str'));
+
+    foreach ($calls as $call) {
+      $call_params = $call->getChildOfType(1, 'n_CALL_PARAMETER_LIST');
+
+      if (count($call_params->getChildren()) < 2) {
+        $this->raiseLintAtNode(
+          $call,
+          pht(
+            'Avoid %s unless the second parameter is specified. '.
+            'It is confusing and hinders static analysis.',
+            'parse_str()'));
+      }
+    }
+  }
+
+}
diff --git a/src/lint/linter/xhpast/rules/ArcanistUndeclaredVariableXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistUndeclaredVariableXHPASTLinterRule.php
index c60b216c..05464d41 100644
--- a/src/lint/linter/xhpast/rules/ArcanistUndeclaredVariableXHPASTLinterRule.php
+++ b/src/lint/linter/xhpast/rules/ArcanistUndeclaredVariableXHPASTLinterRule.php
@@ -48,6 +48,8 @@ final class ArcanistUndeclaredVariableXHPASTLinterRule
     //
     // TODO: Support functions defined inside other functions which is commonly
     // used with anonymous functions.
+    //
+    // TODO: parse_str() also makes lexical scope unknowable, see D13857.
 
     $defs = $root->selectDescendantsOfTypes(array(
       'n_FUNCTION_DECLARATION',