Add a linter rule to detect the use of blacklisted functions

Summary:
As mentioned in the [[https://secure.phabricator.com/book/phabcontrib/article/php_coding_standards/ | Phabricator PHP coding standards]], the `eval` function should be avoided. There is some good discussion on [[http://stackoverflow.com/questions/951373/when-is-eval-evil-in-php | StackOverflow]] as well.

Having said that, instead of hardcoding `eval()`, I have generalised this enough to allow a set of "blacklisted" functions to be defined with `xhpast.blacklisted.function` in the `.arclint` file.

Test Plan: Added a test case.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

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

.arclint

@@ -41,7 +41,19 @@
       "exclude": "(resources/spelling/.*\\.json$)"
     },
     "text": {
-      "type": "text"
+      "type": "text",
+      "exclude": [
+        "(^\\.arclint$)"
+      ]
+    },
+    "text-without-length": {
+      "type": "text",
+      "severity": {
+        "3": "disabled"
+      },
+      "include": [
+        "(^\\.arclint$)"
+      ]
     },
     "xhpast": {
       "type": "xhpast",
@@ -50,6 +62,9 @@
         "16": "advice",
         "34": "error"
       },
+      "xhpast.blacklisted.function": {
+        "eval": "The eval() function should be avoided. It is potentially unsafe and makes debugging more difficult."
+      },
       "xhpast.php-version": "5.2.3",
       "xhpast.php-version.windows": "5.3.0"
     }

src/lint/linter/ArcanistXHPASTLinter.php

@@ -50,7 +50,9 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
   const LINT_ARRAY_SEPARATOR           = 48;
   const LINT_CONSTRUCTOR_PARENTHESES   = 49;
   const LINT_DUPLICATE_SWITCH_CASE     = 50;
+  const LINT_BLACKLISTED_FUNCTION      = 50;
 
+  private $blacklistedFunctions = array();
   private $naminghook;
   private $switchhook;
   private $version;
@@ -111,6 +113,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
       self::LINT_ARRAY_SEPARATOR           => 'Array Separator',
       self::LINT_CONSTRUCTOR_PARENTHESES   => 'Constructor Parentheses',
       self::LINT_DUPLICATE_SWITCH_CASE     => 'Duplicate Case Statements',
+      self::LINT_BLACKLISTED_FUNCTION      => 'Use of Blacklisted Function',
     );
   }
 
@@ -157,6 +160,10 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
 
   public function getLinterConfigurationOptions() {
     return parent::getLinterConfigurationOptions() + array(
+      'xhpast.blacklisted.function' => array(
+        'type' => 'optional map<string, string>',
+        'help' => pht('Blacklisted functions which should not be used.'),
+      ),
       'xhpast.naminghook' => array(
         'type' => 'optional string',
         'help' => pht(
@@ -182,6 +189,9 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
 
   public function setLinterConfigurationValue($key, $value) {
     switch ($key) {
+      case 'xhpast.blacklisted.function':
+        $this->blacklistedFunctions = $value;
+        return;
       case 'xhpast.naminghook':
         $this->naminghook = $value;
         return;
@@ -201,7 +211,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
 
   public function getVersion() {
     // The version number should be incremented whenever a new rule is added.
-    return '12';
+    return '13';
   }
 
   protected function resolveFuture($path, Future $future) {
@@ -274,6 +284,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
       'lintArraySeparator' => self::LINT_ARRAY_SEPARATOR,
       'lintConstructorParentheses' => self::LINT_CONSTRUCTOR_PARENTHESES,
       'lintSwitchStatements' => self::LINT_DUPLICATE_SWITCH_CASE,
+      'lintBlacklistedFunction' => self::LINT_BLACKLISTED_FUNCTION,
     );
 
     foreach ($method_codes as $method => $codes) {
@@ -2969,6 +2980,24 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
     }
   }
 
+  private function lintBlacklistedFunction(XHPASTNode $root) {
+    $calls = $root->selectDescendantsOfType('n_FUNCTION_CALL');
+
+    foreach ($calls as $call) {
+      $node = $call->getChildByIndex(0);
+      $name = $node->getConcreteString();
+
+      $reason = idx($this->blacklistedFunctions, $name);
+
+      if ($reason) {
+        $this->raiseLintAtNode(
+          $node,
+          self::LINT_BLACKLISTED_FUNCTION,
+          $reason);
+      }
+    }
+  }
+
   public function getSuperGlobalNames() {
     return array(
       '$GLOBALS',

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

@@ -0,0 +1,7 @@
+<?php
+eval('evil code');
+~~~~~~~~~~
+error:2:1
+~~~~~~~~~~
+~~~~~~~~~~
+{"config": {"xhpast.blacklisted.function": {"eval": "Evil function"}}}