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

@@ -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',

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

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

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.

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.'));
+    }
+  }
+
+}