From fe4856277c6c88470ffee144a69e81405cbd167e Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Mon, 15 Jun 2015 20:00:54 +1000
Subject: [PATCH] Add some tests for subclasses

Summary: Add some tests to ensure that `ArcanistXHPASTLinterRule` subclasses are properly implemented. This should catch issues such as two linter rules having the same `ID` value. See D13272 for a similar change.

Test Plan: `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D13274
---
 src/__phutil_library_map__.php                |  2 ++
 src/lint/linter/ArcanistXHPASTLinter.php      | 28 +------------------
 .../xhpast/ArcanistXHPASTLinterRule.php       | 26 +++++++++++++++++
 .../ArcanistXHPASTLinterRuleTestCase.php      | 10 +++++++
 4 files changed, 39 insertions(+), 27 deletions(-)
 create mode 100644 src/lint/linter/xhpast/__tests__/ArcanistXHPASTLinterRuleTestCase.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 6c4fe32c..e5e0583d 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -261,6 +261,7 @@ phutil_register_library_map(array(
     'ArcanistXHPASTLintSwitchHook' => 'lint/linter/xhpast/ArcanistXHPASTLintSwitchHook.php',
     'ArcanistXHPASTLinter' => 'lint/linter/ArcanistXHPASTLinter.php',
     'ArcanistXHPASTLinterRule' => 'lint/linter/xhpast/ArcanistXHPASTLinterRule.php',
+    'ArcanistXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/__tests__/ArcanistXHPASTLinterRuleTestCase.php',
     'ArcanistXHPASTLinterTestCase' => 'lint/linter/__tests__/ArcanistXHPASTLinterTestCase.php',
     'ArcanistXMLLinter' => 'lint/linter/ArcanistXMLLinter.php',
     'ArcanistXMLLinterTestCase' => 'lint/linter/__tests__/ArcanistXMLLinterTestCase.php',
@@ -533,6 +534,7 @@ phutil_register_library_map(array(
     'ArcanistXHPASTLintSwitchHook' => 'Phobject',
     'ArcanistXHPASTLinter' => 'ArcanistBaseXHPASTLinter',
     'ArcanistXHPASTLinterRule' => 'Phobject',
+    'ArcanistXHPASTLinterRuleTestCase' => 'PhutilTestCase',
     'ArcanistXHPASTLinterTestCase' => 'ArcanistLinterTestCase',
     'ArcanistXMLLinter' => 'ArcanistLinter',
     'ArcanistXMLLinterTestCase' => 'ArcanistLinterTestCase',
diff --git a/src/lint/linter/ArcanistXHPASTLinter.php b/src/lint/linter/ArcanistXHPASTLinter.php
index 0d1b593d..668542f8 100644
--- a/src/lint/linter/ArcanistXHPASTLinter.php
+++ b/src/lint/linter/ArcanistXHPASTLinter.php
@@ -8,7 +8,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
   private $rules = array();
 
   public function __construct() {
-    $this->rules = $this->getLinterRules();
+    $this->rules = ArcanistXHPASTLinterRule::loadAllRules();
   }
 
   public function __clone() {
@@ -66,32 +66,6 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
     return count($this->rules);
   }
 
-  public function getLinterRules() {
-    $rules = array();
-
-    $symbols = id(new PhutilSymbolLoader())
-      ->setAncestorClass('ArcanistXHPASTLinterRule')
-      ->loadObjects();
-
-    foreach ($symbols as $class => $rule) {
-      $id = $rule->getLintID();
-
-      if (isset($rules[$id])) {
-        throw new Exception(
-          pht(
-            'Two linter rules (`%s`, `%s`) share the same lint ID (%d). '.
-            'Each linter rule must have a unique ID.',
-            $class,
-            get_class($rules[$id]),
-            $id));
-      }
-
-      $rules[$id] = $rule;
-    }
-
-    return $rules;
-  }
-
   protected function resolveFuture($path, Future $future) {
     $tree = $this->getXHPASTTreeForPath($path);
     if (!$tree) {
diff --git a/src/lint/linter/xhpast/ArcanistXHPASTLinterRule.php b/src/lint/linter/xhpast/ArcanistXHPASTLinterRule.php
index 35bf5516..00c44194 100644
--- a/src/lint/linter/xhpast/ArcanistXHPASTLinterRule.php
+++ b/src/lint/linter/xhpast/ArcanistXHPASTLinterRule.php
@@ -4,6 +4,32 @@ abstract class ArcanistXHPASTLinterRule extends Phobject {
 
   private $linter = null;
 
+  final public static function loadAllRules() {
+    $rules = array();
+
+    $symbols = id(new PhutilSymbolLoader())
+      ->setAncestorClass(__CLASS__)
+      ->loadObjects();
+
+    foreach ($symbols as $class => $rule) {
+      $id = $rule->getLintID();
+
+      if (isset($rules[$id])) {
+        throw new Exception(
+          pht(
+            'Two linter rules (`%s`, `%s`) share the same lint ID (%d). '.
+            'Each linter rule must have a unique ID.',
+            $class,
+            get_class($rules[$id]),
+            $id));
+      }
+
+      $rules[$id] = $rule;
+    }
+
+    return $rules;
+  }
+
   final public function getLintID() {
     $class = new ReflectionClass($this);
 
diff --git a/src/lint/linter/xhpast/__tests__/ArcanistXHPASTLinterRuleTestCase.php b/src/lint/linter/xhpast/__tests__/ArcanistXHPASTLinterRuleTestCase.php
new file mode 100644
index 00000000..fcb49f7c
--- /dev/null
+++ b/src/lint/linter/xhpast/__tests__/ArcanistXHPASTLinterRuleTestCase.php
@@ -0,0 +1,10 @@
+<?php
+
+final class ArcanistXHPASTLinterRuleTestCase extends PhutilTestCase {
+
+  public function testLoadAllRules() {
+    ArcanistXHPASTLinterRule::loadAllRules();
+    $this->assertTrue(true);
+  }
+
+}