mirror of
https://we.phorge.it/source/arcanist.git
synced 2025-01-09 14:21:01 +01:00
(stable) Promote 2021 Week 6
This commit is contained in:
commit
dfc16ce41c
11 changed files with 423 additions and 87 deletions
|
@ -402,6 +402,8 @@ phutil_register_library_map(array(
|
||||||
'ArcanistParenthesesSpacingXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistParenthesesSpacingXHPASTLinterRuleTestCase.php',
|
'ArcanistParenthesesSpacingXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistParenthesesSpacingXHPASTLinterRuleTestCase.php',
|
||||||
'ArcanistParseStrUseXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistParseStrUseXHPASTLinterRule.php',
|
'ArcanistParseStrUseXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistParseStrUseXHPASTLinterRule.php',
|
||||||
'ArcanistParseStrUseXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistParseStrUseXHPASTLinterRuleTestCase.php',
|
'ArcanistParseStrUseXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistParseStrUseXHPASTLinterRuleTestCase.php',
|
||||||
|
'ArcanistPartialCatchXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPartialCatchXHPASTLinterRule.php',
|
||||||
|
'ArcanistPartialCatchXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistPartialCatchXHPASTLinterRuleTestCase.php',
|
||||||
'ArcanistPasteRef' => 'ref/paste/ArcanistPasteRef.php',
|
'ArcanistPasteRef' => 'ref/paste/ArcanistPasteRef.php',
|
||||||
'ArcanistPasteSymbolRef' => 'ref/paste/ArcanistPasteSymbolRef.php',
|
'ArcanistPasteSymbolRef' => 'ref/paste/ArcanistPasteSymbolRef.php',
|
||||||
'ArcanistPasteWorkflow' => 'workflow/ArcanistPasteWorkflow.php',
|
'ArcanistPasteWorkflow' => 'workflow/ArcanistPasteWorkflow.php',
|
||||||
|
@ -1451,6 +1453,8 @@ phutil_register_library_map(array(
|
||||||
'ArcanistParenthesesSpacingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
'ArcanistParenthesesSpacingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
||||||
'ArcanistParseStrUseXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
'ArcanistParseStrUseXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
||||||
'ArcanistParseStrUseXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
'ArcanistParseStrUseXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
||||||
|
'ArcanistPartialCatchXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
||||||
|
'ArcanistPartialCatchXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
||||||
'ArcanistPasteRef' => 'ArcanistRef',
|
'ArcanistPasteRef' => 'ArcanistRef',
|
||||||
'ArcanistPasteSymbolRef' => 'ArcanistSimpleSymbolRef',
|
'ArcanistPasteSymbolRef' => 'ArcanistSimpleSymbolRef',
|
||||||
'ArcanistPasteWorkflow' => 'ArcanistArcWorkflow',
|
'ArcanistPasteWorkflow' => 'ArcanistArcWorkflow',
|
||||||
|
|
|
@ -13,6 +13,7 @@ final class ArcanistPhutilLibraryLinter extends ArcanistLinter {
|
||||||
const LINT_UNKNOWN_SYMBOL = 1;
|
const LINT_UNKNOWN_SYMBOL = 1;
|
||||||
const LINT_DUPLICATE_SYMBOL = 2;
|
const LINT_DUPLICATE_SYMBOL = 2;
|
||||||
const LINT_ONE_CLASS_PER_FILE = 3;
|
const LINT_ONE_CLASS_PER_FILE = 3;
|
||||||
|
const LINT_NONCANONICAL_SYMBOL = 4;
|
||||||
|
|
||||||
public function getInfoName() {
|
public function getInfoName() {
|
||||||
return pht('Phutil Library Linter');
|
return pht('Phutil Library Linter');
|
||||||
|
@ -43,6 +44,7 @@ final class ArcanistPhutilLibraryLinter extends ArcanistLinter {
|
||||||
self::LINT_UNKNOWN_SYMBOL => pht('Unknown Symbol'),
|
self::LINT_UNKNOWN_SYMBOL => pht('Unknown Symbol'),
|
||||||
self::LINT_DUPLICATE_SYMBOL => pht('Duplicate Symbol'),
|
self::LINT_DUPLICATE_SYMBOL => pht('Duplicate Symbol'),
|
||||||
self::LINT_ONE_CLASS_PER_FILE => pht('One Class Per File'),
|
self::LINT_ONE_CLASS_PER_FILE => pht('One Class Per File'),
|
||||||
|
self::LINT_NONCANONICAL_SYMBOL => pht('Noncanonical Symbol'),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -51,6 +53,14 @@ final class ArcanistPhutilLibraryLinter extends ArcanistLinter {
|
||||||
}
|
}
|
||||||
|
|
||||||
public function willLintPaths(array $paths) {
|
public function willLintPaths(array $paths) {
|
||||||
|
|
||||||
|
$libtype_map = array(
|
||||||
|
'class' => 'class',
|
||||||
|
'function' => 'function',
|
||||||
|
'interface' => 'class',
|
||||||
|
'class/interface' => 'class',
|
||||||
|
);
|
||||||
|
|
||||||
// NOTE: For now, we completely ignore paths and just lint every library in
|
// NOTE: For now, we completely ignore paths and just lint every library in
|
||||||
// its entirety. This is simpler and relatively fast because we don't do any
|
// its entirety. This is simpler and relatively fast because we don't do any
|
||||||
// detailed checks and all the data we need for this comes out of module
|
// detailed checks and all the data we need for this comes out of module
|
||||||
|
@ -59,6 +69,26 @@ final class ArcanistPhutilLibraryLinter extends ArcanistLinter {
|
||||||
$bootloader = PhutilBootloader::getInstance();
|
$bootloader = PhutilBootloader::getInstance();
|
||||||
$libraries = $bootloader->getAllLibraries();
|
$libraries = $bootloader->getAllLibraries();
|
||||||
|
|
||||||
|
// Load all the builtin symbols first.
|
||||||
|
$builtin_map = PhutilLibraryMapBuilder::newBuiltinMap();
|
||||||
|
$builtin_map = $builtin_map['have'];
|
||||||
|
|
||||||
|
$normal_symbols = array();
|
||||||
|
$all_symbols = array();
|
||||||
|
foreach ($builtin_map as $type => $builtin_symbols) {
|
||||||
|
$libtype = $libtype_map[$type];
|
||||||
|
foreach ($builtin_symbols as $builtin_symbol => $ignored) {
|
||||||
|
$normal_symbol = $this->normalizeSymbol($builtin_symbol);
|
||||||
|
$normal_symbols[$type][$normal_symbol] = $builtin_symbol;
|
||||||
|
|
||||||
|
$all_symbols[$libtype][$builtin_symbol] = array(
|
||||||
|
'library' => null,
|
||||||
|
'file' => null,
|
||||||
|
'offset' => null,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Load the up-to-date map for each library, without loading the library
|
// Load the up-to-date map for each library, without loading the library
|
||||||
// itself. This means lint results will accurately reflect the state of
|
// itself. This means lint results will accurately reflect the state of
|
||||||
// the working copy.
|
// the working copy.
|
||||||
|
@ -78,7 +108,6 @@ final class ArcanistPhutilLibraryLinter extends ArcanistLinter {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
$all_symbols = array();
|
|
||||||
foreach ($symbols as $library => $map) {
|
foreach ($symbols as $library => $map) {
|
||||||
// Check for files which declare more than one class/interface in the same
|
// Check for files which declare more than one class/interface in the same
|
||||||
// file, or mix function definitions with class/interface definitions. We
|
// file, or mix function definitions with class/interface definitions. We
|
||||||
|
@ -124,13 +153,20 @@ final class ArcanistPhutilLibraryLinter extends ArcanistLinter {
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check for duplicate symbols: two files providing the same class or
|
// Check for duplicate symbols: two files providing the same class or
|
||||||
// function.
|
// function. While doing this, we also build a map of normalized symbol
|
||||||
|
// names to original symbol names: we want a definition of "idx()" to
|
||||||
|
// collide with a definition of "IdX()", and want to perform spelling
|
||||||
|
// corrections later.
|
||||||
|
|
||||||
foreach ($map as $file => $spec) {
|
foreach ($map as $file => $spec) {
|
||||||
$have = idx($spec, 'have', array());
|
$have = idx($spec, 'have', array());
|
||||||
foreach (array('class', 'function', 'interface') as $type) {
|
foreach (array('class', 'function', 'interface') as $type) {
|
||||||
$libtype = ($type == 'interface') ? 'class' : $type;
|
$libtype = $libtype_map[$type];
|
||||||
foreach (idx($have, $type, array()) as $symbol => $offset) {
|
foreach (idx($have, $type, array()) as $symbol => $offset) {
|
||||||
if (empty($all_symbols[$libtype][$symbol])) {
|
$normal_symbol = $this->normalizeSymbol($symbol);
|
||||||
|
|
||||||
|
if (empty($normal_symbols[$libtype][$normal_symbol])) {
|
||||||
|
$normal_symbols[$libtype][$normal_symbol] = $symbol;
|
||||||
$all_symbols[$libtype][$symbol] = array(
|
$all_symbols[$libtype][$symbol] = array(
|
||||||
'library' => $library,
|
'library' => $library,
|
||||||
'file' => $file,
|
'file' => $file,
|
||||||
|
@ -139,23 +175,41 @@ final class ArcanistPhutilLibraryLinter extends ArcanistLinter {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
$osrc = $all_symbols[$libtype][$symbol]['file'];
|
$old_symbol = $normal_symbols[$libtype][$normal_symbol];
|
||||||
$olib = $all_symbols[$libtype][$symbol]['library'];
|
$old_src = $all_symbols[$libtype][$old_symbol]['file'];
|
||||||
|
$old_lib = $all_symbols[$libtype][$old_symbol]['library'];
|
||||||
|
|
||||||
|
// If these values are "null", it means that the symbol is a
|
||||||
|
// builtin symbol provided by PHP or a PHP extension.
|
||||||
|
|
||||||
|
if ($old_lib === null) {
|
||||||
|
$message = pht(
|
||||||
|
'Definition of symbol "%s" (of type "%s") in file "%s" in '.
|
||||||
|
'library "%s" duplicates builtin definition of the same '.
|
||||||
|
'symbol.',
|
||||||
|
$symbol,
|
||||||
|
$type,
|
||||||
|
$file,
|
||||||
|
$library);
|
||||||
|
} else {
|
||||||
|
$message = pht(
|
||||||
|
'Definition of symbol "%s" (of type "%s") in file "%s" in '.
|
||||||
|
'library "%s" duplicates prior definition in file "%s" in '.
|
||||||
|
'library "%s".',
|
||||||
|
$symbol,
|
||||||
|
$type,
|
||||||
|
$file,
|
||||||
|
$library,
|
||||||
|
$old_src,
|
||||||
|
$old_lib);
|
||||||
|
}
|
||||||
|
|
||||||
$this->raiseLintInLibrary(
|
$this->raiseLintInLibrary(
|
||||||
$library,
|
$library,
|
||||||
$file,
|
$file,
|
||||||
$offset,
|
$offset,
|
||||||
self::LINT_DUPLICATE_SYMBOL,
|
self::LINT_DUPLICATE_SYMBOL,
|
||||||
pht(
|
$message);
|
||||||
"Definition of %s '%s' in '%s' in library '%s' duplicates ".
|
|
||||||
"prior definition in '%s' in library '%s'.",
|
|
||||||
$type,
|
|
||||||
$symbol,
|
|
||||||
$file,
|
|
||||||
$library,
|
|
||||||
$osrc,
|
|
||||||
$olib));
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -169,61 +223,148 @@ final class ArcanistPhutilLibraryLinter extends ArcanistLinter {
|
||||||
foreach ($map as $file => $spec) {
|
foreach ($map as $file => $spec) {
|
||||||
$need = idx($spec, 'need', array());
|
$need = idx($spec, 'need', array());
|
||||||
foreach ($types as $type) {
|
foreach ($types as $type) {
|
||||||
$libtype = $type;
|
$libtype = $libtype_map[$type];
|
||||||
if ($type == 'interface' || $type == 'class/interface') {
|
|
||||||
$libtype = 'class';
|
|
||||||
}
|
|
||||||
foreach (idx($need, $type, array()) as $symbol => $offset) {
|
foreach (idx($need, $type, array()) as $symbol => $offset) {
|
||||||
if (!empty($all_symbols[$libtype][$symbol])) {
|
if (!empty($all_symbols[$libtype][$symbol])) {
|
||||||
// Symbol is defined somewhere.
|
// Symbol is defined somewhere.
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
$libphutil_root = dirname(phutil_get_library_root('arcanist'));
|
$normal_symbol = $this->normalizeSymbol($symbol);
|
||||||
|
if (!empty($normal_symbols[$libtype][$normal_symbol])) {
|
||||||
|
$proper_symbol = $normal_symbols[$libtype][$normal_symbol];
|
||||||
|
|
||||||
|
switch ($type) {
|
||||||
|
case 'class':
|
||||||
|
$summary = pht(
|
||||||
|
'Class symbol "%s" should be written as "%s".',
|
||||||
|
$symbol,
|
||||||
|
$proper_symbol);
|
||||||
|
break;
|
||||||
|
case 'function':
|
||||||
|
$summary = pht(
|
||||||
|
'Function symbol "%s" should be written as "%s".',
|
||||||
|
$symbol,
|
||||||
|
$proper_symbol);
|
||||||
|
break;
|
||||||
|
case 'interface':
|
||||||
|
$summary = pht(
|
||||||
|
'Interface symbol "%s" should be written as "%s".',
|
||||||
|
$symbol,
|
||||||
|
$proper_symbol);
|
||||||
|
break;
|
||||||
|
case 'class/interface':
|
||||||
|
$summary = pht(
|
||||||
|
'Class or interface symbol "%s" should be written as "%s".',
|
||||||
|
$symbol,
|
||||||
|
$proper_symbol);
|
||||||
|
break;
|
||||||
|
default:
|
||||||
|
throw new Exception(
|
||||||
|
pht('Unknown symbol type "%s".', $type));
|
||||||
|
}
|
||||||
|
|
||||||
|
$this->raiseLintInLibrary(
|
||||||
|
$library,
|
||||||
|
$file,
|
||||||
|
$offset,
|
||||||
|
self::LINT_NONCANONICAL_SYMBOL,
|
||||||
|
$summary,
|
||||||
|
$symbol,
|
||||||
|
$proper_symbol);
|
||||||
|
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
$arcanist_root = dirname(phutil_get_library_root('arcanist'));
|
||||||
|
|
||||||
|
switch ($type) {
|
||||||
|
case 'class':
|
||||||
|
$summary = pht(
|
||||||
|
'Use of unknown class symbol "%s".',
|
||||||
|
$symbol);
|
||||||
|
break;
|
||||||
|
case 'function':
|
||||||
|
$summary = pht(
|
||||||
|
'Use of unknown function symbol "%s".',
|
||||||
|
$symbol);
|
||||||
|
break;
|
||||||
|
case 'interface':
|
||||||
|
$summary = pht(
|
||||||
|
'Use of unknown interface symbol "%s".',
|
||||||
|
$symbol);
|
||||||
|
break;
|
||||||
|
case 'class/interface':
|
||||||
|
$summary = pht(
|
||||||
|
'Use of unknown class or interface symbol "%s".',
|
||||||
|
$symbol);
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
|
||||||
|
$details = pht(
|
||||||
|
"Common causes are:\n".
|
||||||
|
"\n".
|
||||||
|
" - Your copy of Arcanist is out of date.\n".
|
||||||
|
" This is the most common cause.\n".
|
||||||
|
" Update this copy of Arcanist:\n".
|
||||||
|
"\n".
|
||||||
|
" %s\n".
|
||||||
|
"\n".
|
||||||
|
" - Some other library is out of date.\n".
|
||||||
|
" Update the library this symbol appears in.\n".
|
||||||
|
"\n".
|
||||||
|
" - The symbol is misspelled.\n".
|
||||||
|
" Spell the symbol name correctly.\n".
|
||||||
|
"\n".
|
||||||
|
" - You added the symbol recently, but have not updated\n".
|
||||||
|
" the symbol map for the library.\n".
|
||||||
|
" Run \"arc liberate\" in the library where the symbol is\n".
|
||||||
|
" defined.\n".
|
||||||
|
"\n".
|
||||||
|
" - This symbol is defined in an external library.\n".
|
||||||
|
" Use \"@phutil-external-symbol\" to annotate it.\n".
|
||||||
|
" Use \"grep\" to find examples of usage.",
|
||||||
|
$arcanist_root);
|
||||||
|
|
||||||
|
$message = implode(
|
||||||
|
"\n\n",
|
||||||
|
array(
|
||||||
|
$summary,
|
||||||
|
$details,
|
||||||
|
));
|
||||||
|
|
||||||
$this->raiseLintInLibrary(
|
$this->raiseLintInLibrary(
|
||||||
$library,
|
$library,
|
||||||
$file,
|
$file,
|
||||||
$offset,
|
$offset,
|
||||||
self::LINT_UNKNOWN_SYMBOL,
|
self::LINT_UNKNOWN_SYMBOL,
|
||||||
pht(
|
$message);
|
||||||
"Use of unknown %s '%s'. Common causes are:\n\n".
|
|
||||||
" - Your %s is out of date.\n".
|
|
||||||
" This is the most common cause.\n".
|
|
||||||
" Update this copy of libphutil: %s\n\n".
|
|
||||||
" - Some other library is out of date.\n".
|
|
||||||
" Update the library this symbol appears in.\n\n".
|
|
||||||
" - This symbol is misspelled.\n".
|
|
||||||
" Spell the symbol name correctly.\n".
|
|
||||||
" Symbol name spelling is case-sensitive.\n\n".
|
|
||||||
" - This symbol was added recently.\n".
|
|
||||||
" Run `%s` on the library it was added to.\n\n".
|
|
||||||
" - This symbol is external. Use `%s`.\n".
|
|
||||||
" Use `%s` to find usage examples of this directive.\n\n".
|
|
||||||
"*** ALTHOUGH USUALLY EASY TO FIX, THIS IS A SERIOUS ERROR.\n".
|
|
||||||
"*** THIS ERROR IS YOUR FAULT. YOU MUST RESOLVE IT.",
|
|
||||||
$type,
|
|
||||||
$symbol,
|
|
||||||
'libphutil/',
|
|
||||||
$libphutil_root,
|
|
||||||
'arc liberate',
|
|
||||||
'@phutil-external-symbol',
|
|
||||||
'grep'));
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private function raiseLintInLibrary($library, $path, $offset, $code, $desc) {
|
private function raiseLintInLibrary(
|
||||||
|
$library,
|
||||||
|
$path,
|
||||||
|
$offset,
|
||||||
|
$code,
|
||||||
|
$desc,
|
||||||
|
$original = null,
|
||||||
|
$replacement = null) {
|
||||||
$root = phutil_get_library_root($library);
|
$root = phutil_get_library_root($library);
|
||||||
|
|
||||||
$this->activePath = $root.'/'.$path;
|
$this->activePath = $root.'/'.$path;
|
||||||
$this->raiseLintAtOffset($offset, $code, $desc);
|
$this->raiseLintAtOffset($offset, $code, $desc, $original, $replacement);
|
||||||
}
|
}
|
||||||
|
|
||||||
public function lintPath($path) {
|
public function lintPath($path) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private function normalizeSymbol($symbol) {
|
||||||
|
return phutil_utf8_strtolower($symbol);
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -22,68 +22,65 @@ final class ArcanistInvalidModifiersXHPASTLinterRule
|
||||||
$is_final = false;
|
$is_final = false;
|
||||||
$is_static = false;
|
$is_static = false;
|
||||||
$visibility = null;
|
$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) {
|
foreach ($modifiers as $modifier) {
|
||||||
switch ($modifier->getConcreteString()) {
|
switch ($modifier->getConcreteString()) {
|
||||||
case 'abstract':
|
case 'abstract':
|
||||||
if ($method->getTypeName() == 'n_CLASS_MEMBER_MODIFIER_LIST') {
|
if ($is_property) {
|
||||||
$this->raiseLintAtNode(
|
$this->raiseLintAtNode(
|
||||||
$modifier,
|
$modifier,
|
||||||
pht(
|
pht(
|
||||||
'Properties cannot be declared `%s`.',
|
'Properties cannot be declared "abstract".'));
|
||||||
'abstract'));
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if ($is_abstract) {
|
if ($is_abstract) {
|
||||||
$this->raiseLintAtNode(
|
$this->raiseLintAtNode(
|
||||||
$modifier,
|
$modifier,
|
||||||
pht(
|
pht(
|
||||||
'Multiple `%s` modifiers are not allowed.',
|
'Multiple "abstract" modifiers are not allowed.'));
|
||||||
'abstract'));
|
|
||||||
}
|
|
||||||
|
|
||||||
if ($is_final) {
|
|
||||||
$this->raiseLintAtNode(
|
|
||||||
$modifier,
|
|
||||||
pht(
|
|
||||||
'Cannot use the `%s` modifier on an `%s` class member',
|
|
||||||
'final',
|
|
||||||
'abstract'));
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
$abstract_modifier = $modifier;
|
||||||
$is_abstract = true;
|
$is_abstract = true;
|
||||||
break;
|
break;
|
||||||
|
|
||||||
case 'final':
|
case 'final':
|
||||||
if ($is_abstract) {
|
if ($is_property) {
|
||||||
$this->raiseLintAtNode(
|
$this->raiseLintAtNode(
|
||||||
$modifier,
|
$modifier,
|
||||||
pht(
|
pht(
|
||||||
'Cannot use the `%s` modifier on an `%s` class member',
|
'Properties can not be declared "final".'));
|
||||||
'final',
|
|
||||||
'abstract'));
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if ($is_final) {
|
if ($is_final) {
|
||||||
$this->raiseLintAtNode(
|
$this->raiseLintAtNode(
|
||||||
$modifier,
|
$modifier,
|
||||||
pht(
|
pht(
|
||||||
'Multiple `%s` modifiers are not allowed.',
|
'Multiple "final" modifiers are not allowed.'));
|
||||||
'final'));
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
$final_modifier = $modifier;
|
||||||
$is_final = true;
|
$is_final = true;
|
||||||
break;
|
break;
|
||||||
case 'public':
|
case 'public':
|
||||||
case 'protected':
|
case 'protected':
|
||||||
case 'private':
|
case 'private':
|
||||||
if ($visibility) {
|
if ($visibility !== null) {
|
||||||
$this->raiseLintAtNode(
|
$this->raiseLintAtNode(
|
||||||
$modifier,
|
$modifier,
|
||||||
pht('Multiple access type modifiers are not allowed.'));
|
pht('Multiple access type modifiers are not allowed.'));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
$visibility_modifier = $modifier;
|
||||||
|
|
||||||
$visibility = $modifier->getConcreteString();
|
$visibility = $modifier->getConcreteString();
|
||||||
|
$visibility = phutil_utf8_strtolower($visibility);
|
||||||
break;
|
break;
|
||||||
|
|
||||||
case 'static':
|
case 'static':
|
||||||
|
@ -91,12 +88,47 @@ final class ArcanistInvalidModifiersXHPASTLinterRule
|
||||||
$this->raiseLintAtNode(
|
$this->raiseLintAtNode(
|
||||||
$modifier,
|
$modifier,
|
||||||
pht(
|
pht(
|
||||||
'Multiple `%s` modifiers are not allowed.',
|
'Multiple "static" modifiers are not allowed.'));
|
||||||
'static'));
|
|
||||||
}
|
}
|
||||||
break;
|
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.
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -0,0 +1,45 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
final class ArcanistPartialCatchXHPASTLinterRule
|
||||||
|
extends ArcanistXHPASTLinterRule {
|
||||||
|
|
||||||
|
const ID = 132;
|
||||||
|
|
||||||
|
public function getLintName() {
|
||||||
|
return pht('Partial Catch');
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getLintSeverity() {
|
||||||
|
return ArcanistLintSeverity::SEVERITY_WARNING;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function process(XHPASTNode $root) {
|
||||||
|
$catch_lists = $root->selectDescendantsOfType('n_CATCH_LIST');
|
||||||
|
|
||||||
|
foreach ($catch_lists as $catch_list) {
|
||||||
|
$catches = $catch_list->getChildrenOfType('n_CATCH');
|
||||||
|
|
||||||
|
$classes = array();
|
||||||
|
foreach ($catches as $catch) {
|
||||||
|
$class_node = $catch->getChildOfType(0, 'n_CLASS_NAME');
|
||||||
|
$class_name = $class_node->getConcreteString();
|
||||||
|
$class_name = phutil_utf8_strtolower($class_name);
|
||||||
|
|
||||||
|
$classes[$class_name] = $class_node;
|
||||||
|
}
|
||||||
|
|
||||||
|
$catches_exception = idx($classes, 'exception');
|
||||||
|
$catches_throwable = idx($classes, 'throwable');
|
||||||
|
|
||||||
|
if ($catches_exception && !$catches_throwable) {
|
||||||
|
$this->raiseLintAtNode(
|
||||||
|
$catches_exception,
|
||||||
|
pht(
|
||||||
|
'Try/catch block catches "Exception", but does not catch '.
|
||||||
|
'"Throwable". In PHP7 and newer, some runtime exceptions '.
|
||||||
|
'will escape this block.'));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
|
@ -0,0 +1,10 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
final class ArcanistPartialCatchXHPASTLinterRuleTestCase
|
||||||
|
extends ArcanistXHPASTLinterRuleTestCase {
|
||||||
|
|
||||||
|
public function testLinter() {
|
||||||
|
$this->executeTestsInDirectory(dirname(__FILE__).'/partial-catch/');
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
|
@ -5,14 +5,36 @@ class SomeClass {
|
||||||
public public $b;
|
public public $b;
|
||||||
public protected $c;
|
public protected $c;
|
||||||
private abstract $d;
|
private abstract $d;
|
||||||
|
private final $e;
|
||||||
|
|
||||||
public function foo() {}
|
public function foo() {}
|
||||||
public protected function bar() {}
|
public protected function bar() {}
|
||||||
abstract final public function baz() {}
|
abstract final public function baz() {}
|
||||||
|
final private function wuff() {}
|
||||||
|
private final function fizz() {}
|
||||||
}
|
}
|
||||||
~~~~~~~~~~
|
~~~~~~~~~~
|
||||||
error:5:10:XHP72:Invalid Modifiers
|
error:5:10:XHP72:Invalid Modifiers
|
||||||
error:6:10:XHP72:Invalid Modifiers
|
error:6:10:XHP72:Invalid Modifiers
|
||||||
error:7:11:XHP72:Invalid Modifiers
|
error:7:11:XHP72:Invalid Modifiers
|
||||||
error:10:10:XHP72:Invalid Modifiers
|
error:8:11:XHP72:Invalid Modifiers
|
||||||
error:11:12: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() {}
|
||||||
|
}
|
||||||
|
|
|
@ -0,0 +1,17 @@
|
||||||
|
<?php
|
||||||
|
try {
|
||||||
|
f();
|
||||||
|
} catch (Exception $ex) {
|
||||||
|
g();
|
||||||
|
}
|
||||||
|
|
||||||
|
try {
|
||||||
|
f();
|
||||||
|
} catch (Exception $ex) {
|
||||||
|
g();
|
||||||
|
} catch (Throwable $ex) {
|
||||||
|
h();
|
||||||
|
}
|
||||||
|
|
||||||
|
~~~~~~~~~~
|
||||||
|
warning:4:10:XHP132
|
|
@ -266,9 +266,29 @@ final class PhutilLibraryMapBuilder extends Phobject {
|
||||||
*/
|
*/
|
||||||
private function buildSymbolAnalysisFuture($file) {
|
private function buildSymbolAnalysisFuture($file) {
|
||||||
$absolute_file = $this->getPath($file);
|
$absolute_file = $this->getPath($file);
|
||||||
|
return self::newExtractSymbolsFuture(
|
||||||
|
array(),
|
||||||
|
array($absolute_file));
|
||||||
|
}
|
||||||
|
|
||||||
|
private static function newExtractSymbolsFuture(array $flags, array $paths) {
|
||||||
$bin = dirname(__FILE__).'/../../support/lib/extract-symbols.php';
|
$bin = dirname(__FILE__).'/../../support/lib/extract-symbols.php';
|
||||||
|
|
||||||
return new ExecFuture('php -f %R -- --ugly %R', $bin, $absolute_file);
|
return new ExecFuture(
|
||||||
|
'php -f %R -- --ugly %Ls -- %Ls',
|
||||||
|
$bin,
|
||||||
|
$flags,
|
||||||
|
$paths);
|
||||||
|
}
|
||||||
|
|
||||||
|
public static function newBuiltinMap() {
|
||||||
|
$future = self::newExtractSymbolsFuture(
|
||||||
|
array('--builtins'),
|
||||||
|
array());
|
||||||
|
|
||||||
|
list($json) = $future->resolvex();
|
||||||
|
|
||||||
|
return phutil_json_decode($json);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|
|
@ -84,6 +84,17 @@ abstract class AASTToken extends Phobject {
|
||||||
return $result;
|
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() {
|
final public function getLineNumber() {
|
||||||
return idx($this->tree->getOffsetToLineNumberMap(), $this->getOffset());
|
return idx($this->tree->getOffsetToLineNumberMap(), $this->getOffset());
|
||||||
}
|
}
|
||||||
|
|
|
@ -67,6 +67,18 @@ function xsprintf($callback, $userdata, array $argv) {
|
||||||
}
|
}
|
||||||
|
|
||||||
if ($callback !== null) {
|
if ($callback !== null) {
|
||||||
|
|
||||||
|
// See T13588 and D21500. This function uses "$callback()", instead
|
||||||
|
// of "call_user_func()", to simplify reference behavior: some of
|
||||||
|
// these arguments must be passed by reference.
|
||||||
|
|
||||||
|
// Prior to PHP7, this syntax will not work if "$callback" is a
|
||||||
|
// string referencing a static method, like "C::m".
|
||||||
|
|
||||||
|
// This syntax does work if "$callback" is an array referencing
|
||||||
|
// a static method, like "array('C', 'm')", in all versions of PHP
|
||||||
|
// since PHP 5.4.
|
||||||
|
|
||||||
$callback($userdata, $pattern, $pos, $argv[$arg], $len);
|
$callback($userdata, $pattern, $pos, $argv[$arg], $len);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -23,8 +23,8 @@ $args->setSynopsis(<<<EOHELP
|
||||||
|
|
||||||
Symbols are reported in JSON on stdout.
|
Symbols are reported in JSON on stdout.
|
||||||
|
|
||||||
This script is used internally by libphutil/arcanist to build maps of
|
This script is used internally by Arcanist to build maps of library
|
||||||
library symbols.
|
symbols.
|
||||||
|
|
||||||
It would be nice to eventually implement this as a C++ xhpast binary,
|
It would be nice to eventually implement this as a C++ xhpast binary,
|
||||||
as it's relatively stable and performance is currently awful
|
as it's relatively stable and performance is currently awful
|
||||||
|
@ -38,7 +38,11 @@ $args->parse(
|
||||||
array(
|
array(
|
||||||
'name' => 'all',
|
'name' => 'all',
|
||||||
'help' => pht(
|
'help' => pht(
|
||||||
'Report all symbols, including built-ins and declared externals.'),
|
'Emit all symbols, including built-ins and declared externals.'),
|
||||||
|
),
|
||||||
|
array(
|
||||||
|
'name' => 'builtins',
|
||||||
|
'help' => pht('Emit builtin symbols.'),
|
||||||
),
|
),
|
||||||
array(
|
array(
|
||||||
'name' => 'ugly',
|
'name' => 'ugly',
|
||||||
|
@ -52,14 +56,32 @@ $args->parse(
|
||||||
));
|
));
|
||||||
|
|
||||||
$paths = $args->getArg('path');
|
$paths = $args->getArg('path');
|
||||||
if (count($paths) !== 1) {
|
|
||||||
throw new Exception(pht('Specify exactly one path!'));
|
|
||||||
}
|
|
||||||
$path = Filesystem::resolvePath(head($paths));
|
|
||||||
|
|
||||||
$show_all = $args->getArg('all');
|
$show_all = $args->getArg('all');
|
||||||
|
$show_builtins = $args->getArg('builtins');
|
||||||
|
|
||||||
$source_code = Filesystem::readFile($path);
|
if ($show_all && $show_builtins) {
|
||||||
|
throw new PhutilArgumentUsageException(
|
||||||
|
pht(
|
||||||
|
'Flags "--all" and "--builtins" are not compatible.'));
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($show_builtins && $paths) {
|
||||||
|
throw new PhutilArgumentUsageException(
|
||||||
|
pht(
|
||||||
|
'Flag "--builtins" may not be used with a path.'));
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($show_builtins) {
|
||||||
|
$path = '<builtins>';
|
||||||
|
$source_code = '';
|
||||||
|
} else {
|
||||||
|
if (count($paths) !== 1) {
|
||||||
|
throw new Exception(pht('Specify exactly one path!'));
|
||||||
|
}
|
||||||
|
$path = Filesystem::resolvePath(head($paths));
|
||||||
|
$source_code = Filesystem::readFile($path);
|
||||||
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
$tree = XHPASTTree::newFromData($source_code);
|
$tree = XHPASTTree::newFromData($source_code);
|
||||||
|
@ -474,6 +496,14 @@ foreach ($need as $key => $spec) {
|
||||||
$required_symbols[$type][$name] = $spec['symbol']->getOffset();
|
$required_symbols[$type][$name] = $spec['symbol']->getOffset();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if ($show_builtins) {
|
||||||
|
foreach ($builtins as $type => $builtin_symbols) {
|
||||||
|
foreach ($builtin_symbols as $builtin_symbol => $ignored) {
|
||||||
|
$declared_symbols[$type][$builtin_symbol] = null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
$result = array(
|
$result = array(
|
||||||
'have' => $declared_symbols,
|
'have' => $declared_symbols,
|
||||||
'need' => $required_symbols,
|
'need' => $required_symbols,
|
||||||
|
@ -543,8 +573,6 @@ function phutil_symbols_get_builtins() {
|
||||||
'parent' => true,
|
'parent' => true,
|
||||||
'self' => true,
|
'self' => true,
|
||||||
|
|
||||||
'PhutilBootloader' => true,
|
|
||||||
|
|
||||||
// PHP7 defines these new parent classes of "Exception", but they do not
|
// PHP7 defines these new parent classes of "Exception", but they do not
|
||||||
// exist prior to PHP7. It's possible to use them safely in PHP5, in
|
// exist prior to PHP7. It's possible to use them safely in PHP5, in
|
||||||
// some cases, to write code which is compatible with either PHP5 or
|
// some cases, to write code which is compatible with either PHP5 or
|
||||||
|
@ -570,12 +598,6 @@ function phutil_symbols_get_builtins() {
|
||||||
'isset' => true,
|
'isset' => true,
|
||||||
'die' => true,
|
'die' => true,
|
||||||
|
|
||||||
// These are provided by libphutil but not visible in the map.
|
|
||||||
|
|
||||||
'phutil_is_windows' => true,
|
|
||||||
'phutil_load_library' => true,
|
|
||||||
'phutil_is_hiphop_runtime' => true,
|
|
||||||
|
|
||||||
// HPHP/i defines these functions as 'internal', but they are NOT
|
// HPHP/i defines these functions as 'internal', but they are NOT
|
||||||
// builtins and do not exist in vanilla PHP. Make sure we don't mark
|
// builtins and do not exist in vanilla PHP. Make sure we don't mark
|
||||||
// them as builtin since we need to add dependencies for them.
|
// them as builtin since we need to add dependencies for them.
|
||||||
|
|
Loading…
Reference in a new issue