Page MenuHomePhabricator
Files F13186376

D21539.diff
No OneTemporary
Actions

D21539.diff
View Options

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
+~~~~~~~~~~
+<?php
+
+class SomeClass {
+ public $a;
+ public public $b;
+ public protected $c;
+ private abstract $d;
+ private final $e;
+
+ public function foo() {}
+ public protected function bar() {}
+ abstract final public function baz() {}
+ private function wuff() {}
+ private function fizz() {}
+}
diff --git a/src/parser/aast/api/AASTToken.php b/src/parser/aast/api/AASTToken.php
--- a/src/parser/aast/api/AASTToken.php
+++ b/src/parser/aast/api/AASTToken.php
@@ -84,6 +84,17 @@
return $result;
}
+ public function getWhitespaceTokensAfter() {
+ $tokens = $this->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());
}

File Metadata

Mime Type
text/plain
Expires
Sun, May 12, 3:36 AM (2 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6288210
Default Alt Text
D21539.diff (6 KB)

Event Timeline

Phabricator · Built by Phacility