diff --git a/src/lint/linter/ArcanistPhutilXHPASTLinter.php b/src/lint/linter/ArcanistPhutilXHPASTLinter.php --- a/src/lint/linter/ArcanistPhutilXHPASTLinter.php +++ b/src/lint/linter/ArcanistPhutilXHPASTLinter.php @@ -2,13 +2,14 @@ final class ArcanistPhutilXHPASTLinter extends ArcanistBaseXHPASTLinter { - const LINT_ARRAY_COMBINE = 2; - const LINT_DEPRECATED_FUNCTION = 3; - const LINT_UNSAFE_DYNAMIC_STRING = 4; + const LINT_ARRAY_COMBINE = 2; + const LINT_DEPRECATED_FUNCTION = 3; + 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 @@ public function getLintNameMap() { return array( - self::LINT_ARRAY_COMBINE => 'array_combine() Unreliable', - self::LINT_DEPRECATED_FUNCTION => 'Use of Deprecated Function', - self::LINT_UNSAFE_DYNAMIC_STRING => 'Unsafe Usage of Dynamic String', + self::LINT_ARRAY_COMBINE => 'array_combine() Unreliable', + self::LINT_DEPRECATED_FUNCTION => 'Use of Deprecated Function', + 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_DEPRECATED_FUNCTION => $warning, - self::LINT_UNSAFE_DYNAMIC_STRING => $warning, + self::LINT_ARRAY_COMBINE => $warning, + self::LINT_DEPRECATED_FUNCTION => $warning, + self::LINT_UNSAFE_DYNAMIC_STRING => $warning, + self::LINT_RAGGED_CLASSTREE_EDGE => $warning, ); } @@ -62,7 +65,7 @@ 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 @@ '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 @@ } } - private function lintUnsafeDynamicString(XHPASTNode $root) { $safe = $this->dynamicStringFunctions + array( 'pht' => 0, @@ -192,12 +195,11 @@ $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 @@ } } + 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'."); + } + } + } + } diff --git a/src/lint/linter/ArcanistXHPASTLinter.php b/src/lint/linter/ArcanistXHPASTLinter.php --- a/src/lint/linter/ArcanistXHPASTLinter.php +++ b/src/lint/linter/ArcanistXHPASTLinter.php @@ -32,7 +32,6 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ } } - 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')); diff --git a/src/lint/linter/__tests__/ArcanistXHPASTLinterTestCase.php b/src/lint/linter/__tests__/ArcanistXHPASTLinterTestCase.php --- a/src/lint/linter/__tests__/ArcanistXHPASTLinterTestCase.php +++ b/src/lint/linter/__tests__/ArcanistXHPASTLinterTestCase.php @@ -4,17 +4,9 @@ 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()); } } diff --git a/src/lint/linter/__tests__/xhpast/ragged-classtree-edges.lint-test b/src/lint/linter/__tests__/phlxhp/ragged-classtree-edges.lint-test rename from src/lint/linter/__tests__/xhpast/ragged-classtree-edges.lint-test rename to src/lint/linter/__tests__/phlxhp/ragged-classtree-edges.lint-test