Move the "ragged classtree edge" linter rule to ArcanistPhutilXHPASTLinter.

Summary: This linter rule is very specific to the Phabricator and it is unlikely that it is being used elsewhere. We should move it to `ArcanistPhutilXHPASTLinter`, where other Phabricator-specific linting code lives.

Test Plan: Moved the corresponding test case.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

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

src/lint/linter/ArcanistPhutilXHPASTLinter.php

@@ -2,13 +2,14 @@
 
 final class ArcanistPhutilXHPASTLinter extends ArcanistBaseXHPASTLinter {
 
-  const LINT_ARRAY_COMBINE           = 2;
+  const LINT_ARRAY_COMBINE          = 2;
-  const LINT_DEPRECATED_FUNCTION     = 3;
+  const LINT_DEPRECATED_FUNCTION    = 3;
-  const LINT_UNSAFE_DYNAMIC_STRING   = 4;
+  const LINT_UNSAFE_DYNAMIC_STRING  = 4;
+  const LINT_RAGGED_CLASSTREE_EDGE  = 5;
 
-  private $deprecatedFunctions = array();
+  private $deprecatedFunctions    = array();
   private $dynamicStringFunctions = array();
-  private $dynamicStringClasses = array();
+  private $dynamicStringClasses   = array();
 
   public function getInfoName() {
     return 'XHPAST/libphutil Lint';
@@ -37,18 +38,20 @@ final class ArcanistPhutilXHPASTLinter extends ArcanistBaseXHPASTLinter {
 
   public function getLintNameMap() {
     return array(
-      self::LINT_ARRAY_COMBINE           => 'array_combine() Unreliable',
+      self::LINT_ARRAY_COMBINE          => 'array_combine() Unreliable',
-      self::LINT_DEPRECATED_FUNCTION     => 'Use of Deprecated Function',
+      self::LINT_DEPRECATED_FUNCTION    => 'Use of Deprecated Function',
-      self::LINT_UNSAFE_DYNAMIC_STRING   => 'Unsafe Usage of Dynamic String',
+      self::LINT_UNSAFE_DYNAMIC_STRING  => 'Unsafe Usage of Dynamic String',
+      self::LINT_RAGGED_CLASSTREE_EDGE  => 'Class Not abstract Or final',
     );
   }
 
   public function getLintSeverityMap() {
     $warning = ArcanistLintSeverity::SEVERITY_WARNING;
     return array(
-      self::LINT_ARRAY_COMBINE           => $warning,
+      self::LINT_ARRAY_COMBINE          => $warning,
-      self::LINT_DEPRECATED_FUNCTION     => $warning,
+      self::LINT_DEPRECATED_FUNCTION    => $warning,
-      self::LINT_UNSAFE_DYNAMIC_STRING   => $warning,
+      self::LINT_UNSAFE_DYNAMIC_STRING  => $warning,
+      self::LINT_RAGGED_CLASSTREE_EDGE  => $warning,
     );
   }
 
@@ -62,7 +65,7 @@ final class ArcanistPhutilXHPASTLinter extends ArcanistBaseXHPASTLinter {
 
   public function getVersion() {
     // The version number should be incremented whenever a new rule is added.
-    return '2';
+    return '3';
   }
 
   public function getLinterConfigurationOptions() {
@@ -117,6 +120,7 @@ final class ArcanistPhutilXHPASTLinter extends ArcanistBaseXHPASTLinter {
       'lintArrayCombine' => self::LINT_ARRAY_COMBINE,
       'lintUnsafeDynamicString' => self::LINT_UNSAFE_DYNAMIC_STRING,
       'lintDeprecatedFunctions' => self::LINT_DEPRECATED_FUNCTION,
+      'lintRaggedClasstreeEdges' => self::LINT_RAGGED_CLASSTREE_EDGE,
     );
 
     foreach ($method_codes as $method => $codes) {
@@ -129,7 +133,6 @@ final class ArcanistPhutilXHPASTLinter extends ArcanistBaseXHPASTLinter {
     }
   }
 
-
   private function lintUnsafeDynamicString(XHPASTNode $root) {
     $safe = $this->dynamicStringFunctions + array(
       'pht' => 0,
@@ -192,12 +195,11 @@ final class ArcanistPhutilXHPASTLinter extends ArcanistBaseXHPASTLinter {
           $call,
           self::LINT_UNSAFE_DYNAMIC_STRING,
           "Parameter ".($param + 1)." of {$name}() should be a scalar string, ".
-            "otherwise it's not safe.");
+          "otherwise it's not safe.");
       }
     }
   }
 
-
   private function lintArrayCombine(XHPASTNode $root) {
     $function_calls = $root->selectDescendantsOfType('n_FUNCTION_CALL');
     foreach ($function_calls as $call) {
@@ -242,4 +244,39 @@ final class ArcanistPhutilXHPASTLinter extends ArcanistBaseXHPASTLinter {
     }
   }
 
+  private function lintRaggedClasstreeEdges(XHPASTNode $root) {
+    $parser = new PhutilDocblockParser();
+
+    $classes = $root->selectDescendantsOfType('n_CLASS_DECLARATION');
+    foreach ($classes as $class) {
+      $is_final = false;
+      $is_abstract = false;
+      $is_concrete_extensible = false;
+
+      $attributes = $class->getChildOfType(0, 'n_CLASS_ATTRIBUTES');
+      foreach ($attributes->getChildren() as $child) {
+        if ($child->getConcreteString() == 'final') {
+          $is_final = true;
+        }
+        if ($child->getConcreteString() == 'abstract') {
+          $is_abstract = true;
+        }
+      }
+
+      $docblock = $class->getDocblockToken();
+      if ($docblock) {
+        list($text, $specials) = $parser->parse($docblock->getValue());
+        $is_concrete_extensible = idx($specials, 'concrete-extensible');
+      }
+
+      if (!$is_final && !$is_abstract && !$is_concrete_extensible) {
+        $this->raiseLintAtNode(
+          $class->getChildOfType(1, 'n_CLASS_NAME'),
+          self::LINT_RAGGED_CLASSTREE_EDGE,
+          "This class is neither 'final' nor 'abstract', and does not have ".
+          "a docblock marking it '@concrete-extensible'.");
+      }
+    }
+  }
+
 }

src/lint/linter/ArcanistXHPASTLinter.php

@@ -32,7 +32,6 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
   const LINT_CONTROL_STATEMENT_SPACING = 26;
   const LINT_BINARY_EXPRESSION_SPACING = 27;
   const LINT_ARRAY_INDEX_SPACING       = 28;
-  const LINT_RAGGED_CLASSTREE_EDGE     = 29;
   const LINT_IMPLICIT_FALLTHROUGH      = 30;
   const LINT_PHP_53_FEATURES           = 31;
   const LINT_REUSED_AS_ITERATOR        = 32;
@@ -92,7 +91,6 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
       self::LINT_CONTROL_STATEMENT_SPACING => 'Space After Control Statement',
       self::LINT_BINARY_EXPRESSION_SPACING => 'Space Around Binary Operator',
       self::LINT_ARRAY_INDEX_SPACING       => 'Spacing Before Array Index',
-      self::LINT_RAGGED_CLASSTREE_EDGE     => 'Class Not abstract Or final',
       self::LINT_IMPLICIT_FALLTHROUGH      => 'Implicit Fallthrough',
       self::LINT_PHP_53_FEATURES           => 'Use Of PHP 5.3 Features',
       self::LINT_PHP_54_FEATURES           => 'Use Of PHP 5.4 Features',
@@ -145,10 +143,6 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
       self::LINT_SEMICOLON_SPACING         => $advice,
       self::LINT_CONCATENATION_OPERATOR    => $warning,
 
-      // This is disabled by default because it implies a very strict policy
-      // which isn't necessary in the general case.
-      self::LINT_RAGGED_CLASSTREE_EDGE     => $disabled,
-
       // This is disabled by default because projects don't necessarily target
       // a specific minimum version.
       self::LINT_PHP_53_FEATURES           => $disabled,
@@ -188,7 +182,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
 
   public function getVersion() {
     // The version number should be incremented whenever a new rule is added.
-    return '6';
+    return '7';
   }
 
   protected function resolveFuture($path, Future $future) {
@@ -249,7 +243,6 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
         self::LINT_CLASS_FILENAME_MISMATCH,
       'lintPlusOperatorOnStrings' => self::LINT_PLUS_OPERATOR_ON_STRINGS,
       'lintDuplicateKeysInArray' => self::LINT_DUPLICATE_KEYS_IN_ARRAY,
-      'lintRaggedClasstreeEdges' => self::LINT_RAGGED_CLASSTREE_EDGE,
       'lintClosingCallParen' => self::LINT_CLOSING_CALL_PAREN,
       'lintClosingDeclarationParen' => self::LINT_CLOSING_DECL_PAREN,
       'lintKeywordCasing' => self::LINT_KEYWORD_CASING,
@@ -2290,42 +2283,6 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
     }
   }
 
-  private function lintRaggedClasstreeEdges(XHPASTNode $root) {
-    $parser = new PhutilDocblockParser();
-
-    $classes = $root->selectDescendantsOfType('n_CLASS_DECLARATION');
-    foreach ($classes as $class) {
-
-      $is_final = false;
-      $is_abstract = false;
-      $is_concrete_extensible = false;
-
-      $attributes = $class->getChildOfType(0, 'n_CLASS_ATTRIBUTES');
-      foreach ($attributes->getChildren() as $child) {
-        if ($child->getConcreteString() == 'final') {
-          $is_final = true;
-        }
-        if ($child->getConcreteString() == 'abstract') {
-          $is_abstract = true;
-        }
-      }
-
-      $docblock = $class->getDocblockToken();
-      if ($docblock) {
-        list($text, $specials) = $parser->parse($docblock->getValue());
-        $is_concrete_extensible = idx($specials, 'concrete-extensible');
-      }
-
-      if (!$is_final && !$is_abstract && !$is_concrete_extensible) {
-        $this->raiseLintAtNode(
-          $class->getChildOfType(1, 'n_CLASS_NAME'),
-          self::LINT_RAGGED_CLASSTREE_EDGE,
-          "This class is neither 'final' nor 'abstract', and does not have ".
-          "a docblock marking it '@concrete-extensible'.");
-      }
-    }
-  }
-
   private function lintClosingCallParen(XHPASTNode $root) {
     $calls = $root->selectDescendantsOfType('n_FUNCTION_CALL');
     $calls = $calls->add($root->selectDescendantsOfType('n_METHOD_CALL'));

src/lint/linter/__tests__/ArcanistXHPASTLinterTestCase.php

@@ -4,17 +4,9 @@ final class ArcanistXHPASTLinterTestCase
   extends ArcanistArcanistLinterTestCase {
 
   public function testXHPASTLint() {
-    $linter = new ArcanistXHPASTLinter();
-
-    $linter->setCustomSeverityMap(
-      array(
-        ArcanistXHPASTLinter::LINT_RAGGED_CLASSTREE_EDGE
-          => ArcanistLintSeverity::SEVERITY_WARNING,
-      ));
-
     $this->executeTestsInDirectory(
       dirname(__FILE__).'/xhpast/',
-      $linter);
+      new ArcanistXHPASTLinter());
   }
 
 }