diff --git a/src/lint/linter/xhpast/rules/ArcanistInvalidModifiersXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistInvalidModifiersXHPASTLinterRule.php --- a/src/lint/linter/xhpast/rules/ArcanistInvalidModifiersXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistInvalidModifiersXHPASTLinterRule.php @@ -22,68 +22,65 @@ $is_final = false; $is_static = false; $visibility = null; + $is_property = ($method->getTypeName() == 'n_CLASS_MEMBER_MODIFIER_LIST'); + $is_method = !$is_property; + + $final_modifier = null; + $visibility_modifier = null; + $abstract_modifier = null; foreach ($modifiers as $modifier) { switch ($modifier->getConcreteString()) { case 'abstract': - if ($method->getTypeName() == 'n_CLASS_MEMBER_MODIFIER_LIST') { + if ($is_property) { $this->raiseLintAtNode( $modifier, pht( - 'Properties cannot be declared `%s`.', - 'abstract')); + 'Properties cannot be declared "abstract".')); } if ($is_abstract) { $this->raiseLintAtNode( $modifier, pht( - 'Multiple `%s` modifiers are not allowed.', - 'abstract')); - } - - if ($is_final) { - $this->raiseLintAtNode( - $modifier, - pht( - 'Cannot use the `%s` modifier on an `%s` class member', - 'final', - 'abstract')); + 'Multiple "abstract" modifiers are not allowed.')); } + $abstract_modifier = $modifier; $is_abstract = true; break; case 'final': - if ($is_abstract) { + if ($is_property) { $this->raiseLintAtNode( $modifier, pht( - 'Cannot use the `%s` modifier on an `%s` class member', - 'final', - 'abstract')); + 'Properties can not be declared "final".')); } if ($is_final) { $this->raiseLintAtNode( $modifier, pht( - 'Multiple `%s` modifiers are not allowed.', - 'final')); + 'Multiple "final" modifiers are not allowed.')); } + $final_modifier = $modifier; $is_final = true; break; case 'public': case 'protected': case 'private': - if ($visibility) { + if ($visibility !== null) { $this->raiseLintAtNode( $modifier, pht('Multiple access type modifiers are not allowed.')); } + $visibility_modifier = $modifier; + $visibility = $modifier->getConcreteString(); + $visibility = phutil_utf8_strtolower($visibility); break; case 'static': @@ -91,12 +88,47 @@ $this->raiseLintAtNode( $modifier, pht( - 'Multiple `%s` modifiers are not allowed.', - 'static')); + 'Multiple "static" modifiers are not allowed.')); } break; } } + + $is_private = ($visibility === 'private'); + + if ($is_final && $is_abstract) { + if ($is_method) { + $this->raiseLintAtNode( + $final_modifier, + pht('Methods may not be both "abstract" and "final".')); + } else { + // Properties can't be "abstract" and "final" either, but they can't + // ever be "abstract" at all, and we've already raise a message about + // that earlier. + } + } + + if ($is_private && $is_final) { + if ($is_method) { + $final_tokens = $final_modifier->getTokens(); + $space_tokens = last($final_tokens)->getWhitespaceTokensAfter(); + + $final_offset = head($final_tokens)->getOffset(); + + $final_string = array_merge($final_tokens, $space_tokens); + $final_string = mpull($final_string, 'getValue'); + $final_string = implode('', $final_string); + + $this->raiseLintAtOffset( + $final_offset, + pht('Methods may not be both "private" and "final".'), + $final_string, + ''); + } else { + // Properties can't be "final" at all, and we already raised a + // message about this. + } + } } } diff --git a/src/lint/linter/xhpast/rules/__tests__/invalid-modifiers/invalid-modifiers.lint-test b/src/lint/linter/xhpast/rules/__tests__/invalid-modifiers/invalid-modifiers.lint-test --- a/src/lint/linter/xhpast/rules/__tests__/invalid-modifiers/invalid-modifiers.lint-test +++ b/src/lint/linter/xhpast/rules/__tests__/invalid-modifiers/invalid-modifiers.lint-test @@ -5,14 +5,36 @@ public public $b; public protected $c; private abstract $d; + private final $e; public function foo() {} public protected function bar() {} abstract final public function baz() {} + final private function wuff() {} + private final function fizz() {} } ~~~~~~~~~~ error:5:10:XHP72:Invalid Modifiers error:6:10:XHP72:Invalid Modifiers error:7:11:XHP72:Invalid Modifiers -error:10:10:XHP72:Invalid Modifiers -error:11:12:XHP72:Invalid Modifiers +error:8:11:XHP72:Invalid Modifiers +error:11:10:XHP72:Invalid Modifiers +error:12:12:XHP72:Invalid Modifiers +error:13:3:XHP72:Invalid Modifiers +error:14:11:XHP72:Invalid Modifiers +~~~~~~~~~~ +tree->getRawTokenStream(); + $result = array(); + $ii = $this->id + 1; + while ($ii < count($tokens) && $tokens[$ii]->isAnyWhitespace()) { + $result[$ii] = $tokens[$ii]; + ++$ii; + } + return $result; + } + final public function getLineNumber() { return idx($this->tree->getOffsetToLineNumberMap(), $this->getOffset()); }