From 9090efcb40ed29166bad7a6ba5c08d29ebd4ab77 Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Wed, 13 May 2015 06:50:30 +1000
Subject: [PATCH] Add a linter rule to prevent hardcoded class names

Summary: Add a linter rule to advise against the use of hardcoded class names. Hardcoded class names make the code harder to refactor and it is generally preferable to use the `__CLASS__` magic constant instead.

Test Plan: This works, but needs some polish.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12605
---
 src/lint/linter/ArcanistXHPASTLinter.php      | 41 ++++++++++++++++++-
 .../xhpast/class-name-literal.lint-test       | 29 +++++++++++++
 2 files changed, 69 insertions(+), 1 deletion(-)
 create mode 100644 src/lint/linter/__tests__/xhpast/class-name-literal.lint-test

diff --git a/src/lint/linter/ArcanistXHPASTLinter.php b/src/lint/linter/ArcanistXHPASTLinter.php
index 84dcfa2c..4a26767e 100644
--- a/src/lint/linter/ArcanistXHPASTLinter.php
+++ b/src/lint/linter/ArcanistXHPASTLinter.php
@@ -61,6 +61,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
   const LINT_INNER_FUNCTION             = 59;
   const LINT_DEFAULT_PARAMETERS         = 60;
   const LINT_LOWERCASE_FUNCTIONS        = 61;
+  const LINT_CLASS_NAME_LITERAL         = 62;
 
   private $blacklistedFunctions = array();
   private $naminghook;
@@ -191,6 +192,8 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
         => pht('Default Parameters'),
       self::LINT_LOWERCASE_FUNCTIONS
         => pht('Lowercase Functions'),
+      self::LINT_CLASS_NAME_LITERAL
+        => pht('Class Name Literal'),
     );
   }
 
@@ -240,6 +243,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
       self::LINT_INNER_FUNCTION             => $warning,
       self::LINT_DEFAULT_PARAMETERS         => $warning,
       self::LINT_LOWERCASE_FUNCTIONS        => $advice,
+      self::LINT_CLASS_NAME_LITERAL         => $advice,
     );
   }
 
@@ -307,7 +311,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
 
   public function getVersion() {
     // The version number should be incremented whenever a new rule is added.
-    return '24';
+    return '25';
   }
 
   protected function resolveFuture($path, Future $future) {
@@ -395,6 +399,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
       'lintInnerFunctions' => self::LINT_INNER_FUNCTION,
       'lintDefaultParameters' => self::LINT_DEFAULT_PARAMETERS,
       'lintLowercaseFunctions' => self::LINT_LOWERCASE_FUNCTIONS,
+      'lintClassNameLiteral' => self::LINT_CLASS_NAME_LITERAL,
     );
 
     foreach ($method_codes as $method => $codes) {
@@ -3727,6 +3732,40 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
     }
   }
 
+  private function lintClassNameLiteral(XHPASTNode $root) {
+    $class_declarations = $root->selectDescendantsOfType('n_CLASS_DECLARATION');
+
+    foreach ($class_declarations as $class_declaration) {
+      $class_name = $class_declaration
+        ->getChildOfType(1, 'n_CLASS_NAME')
+        ->getConcreteString();
+
+      $strings = $class_declaration->selectDescendantsOfType('n_STRING_SCALAR');
+
+      foreach ($strings as $string) {
+        $contents = substr($string->getSemanticString(), 1, -1);
+        $replacement = null;
+
+        if ($contents == $class_name) {
+          $replacement = '__CLASS__';
+        }
+
+        $regex = '/\b'.preg_quote($class_name, '/').'\b/';
+        if (!preg_match($regex, $contents)) {
+          continue;
+        }
+
+        $this->raiseLintAtNode(
+          $string,
+          self::LINT_CLASS_NAME_LITERAL,
+          pht(
+            "Don't hard-code class names, use %s instead.",
+            '__CLASS__'),
+          $replacement);
+      }
+    }
+  }
+
   /**
    * Retrieve all calls to some specified function(s).
    *
diff --git a/src/lint/linter/__tests__/xhpast/class-name-literal.lint-test b/src/lint/linter/__tests__/xhpast/class-name-literal.lint-test
new file mode 100644
index 00000000..7694bcb2
--- /dev/null
+++ b/src/lint/linter/__tests__/xhpast/class-name-literal.lint-test
@@ -0,0 +1,29 @@
+<?php
+class MyClass {
+  public function someMethod() {
+    return 'MyClass';
+  }
+
+  public function someOtherMethod() {
+    $x = 'MyClass is awesome';
+    $y = 'MyClassIsAwesome';
+    return __CLASS__;
+  }
+}
+~~~~~~~~~~
+error:2:7
+advice:4:12
+advice:8:10
+~~~~~~~~~~
+<?php
+class MyClass {
+  public function someMethod() {
+    return __CLASS__;
+  }
+
+  public function someOtherMethod() {
+    $x = 'MyClass is awesome';
+    $y = 'MyClassIsAwesome';
+    return __CLASS__;
+  }
+}