mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-10 00:42:40 +01:00
Allow lint to correct the spelling of builtin symbols
Summary: Ref T13598. This builds on D21537 and adds support for correcting the capitalization of builtin systems. Test Plan: - Linted a file that uses "ExCePtIoN", got a lint correction. Maniphest Tasks: T13598 Differential Revision: https://secure.phabricator.com/D21538
This commit is contained in:
parent
08dbbbba5a
commit
e2b6439a73
3 changed files with 110 additions and 33 deletions
|
@ -53,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
|
||||||
|
@ -61,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.
|
||||||
|
@ -80,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
|
||||||
|
@ -125,20 +152,12 @@ final class ArcanistPhutilLibraryLinter extends ArcanistLinter {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
$libtype_map = array(
|
|
||||||
'class' => 'class',
|
|
||||||
'function' => 'function',
|
|
||||||
'interface' => 'class',
|
|
||||||
'class/interface' => 'class',
|
|
||||||
);
|
|
||||||
|
|
||||||
// Check for duplicate symbols: two files providing the same class or
|
// Check for duplicate symbols: two files providing the same class or
|
||||||
// function. While doing this, we also build a map of normalized symbol
|
// function. While doing this, we also build a map of normalized symbol
|
||||||
// names to original symbol names: we want a definition of "idx()" to
|
// names to original symbol names: we want a definition of "idx()" to
|
||||||
// collide with a definition of "IdX()", and want to perform spelling
|
// collide with a definition of "IdX()", and want to perform spelling
|
||||||
// corrections later.
|
// corrections later.
|
||||||
|
|
||||||
$normal_symbols = array();
|
|
||||||
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) {
|
||||||
|
@ -160,12 +179,20 @@ final class ArcanistPhutilLibraryLinter extends ArcanistLinter {
|
||||||
$old_src = $all_symbols[$libtype][$old_symbol]['file'];
|
$old_src = $all_symbols[$libtype][$old_symbol]['file'];
|
||||||
$old_lib = $all_symbols[$libtype][$old_symbol]['library'];
|
$old_lib = $all_symbols[$libtype][$old_symbol]['library'];
|
||||||
|
|
||||||
$this->raiseLintInLibrary(
|
// If these values are "null", it means that the symbol is a
|
||||||
$library,
|
// 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,
|
$file,
|
||||||
$offset,
|
$library);
|
||||||
self::LINT_DUPLICATE_SYMBOL,
|
} else {
|
||||||
pht(
|
$message = pht(
|
||||||
'Definition of symbol "%s" (of type "%s") in file "%s" in '.
|
'Definition of symbol "%s" (of type "%s") in file "%s" in '.
|
||||||
'library "%s" duplicates prior definition in file "%s" in '.
|
'library "%s" duplicates prior definition in file "%s" in '.
|
||||||
'library "%s".',
|
'library "%s".',
|
||||||
|
@ -174,7 +201,15 @@ final class ArcanistPhutilLibraryLinter extends ArcanistLinter {
|
||||||
$file,
|
$file,
|
||||||
$library,
|
$library,
|
||||||
$old_src,
|
$old_src,
|
||||||
$old_lib));
|
$old_lib);
|
||||||
|
}
|
||||||
|
|
||||||
|
$this->raiseLintInLibrary(
|
||||||
|
$library,
|
||||||
|
$file,
|
||||||
|
$offset,
|
||||||
|
self::LINT_DUPLICATE_SYMBOL,
|
||||||
|
$message);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -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);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|
|
@ -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');
|
||||||
|
|
||||||
|
$show_all = $args->getArg('all');
|
||||||
|
$show_builtins = $args->getArg('builtins');
|
||||||
|
|
||||||
|
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) {
|
if (count($paths) !== 1) {
|
||||||
throw new Exception(pht('Specify exactly one path!'));
|
throw new Exception(pht('Specify exactly one path!'));
|
||||||
}
|
}
|
||||||
$path = Filesystem::resolvePath(head($paths));
|
$path = Filesystem::resolvePath(head($paths));
|
||||||
|
|
||||||
$show_all = $args->getArg('all');
|
|
||||||
|
|
||||||
$source_code = Filesystem::readFile($path);
|
$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