Page MenuHomePhabricator
Files F15422340

D21538.diff
No OneTemporary
Actions

D21538.diff
View Options

diff --git a/src/lint/linter/ArcanistPhutilLibraryLinter.php b/src/lint/linter/ArcanistPhutilLibraryLinter.php
--- a/src/lint/linter/ArcanistPhutilLibraryLinter.php
+++ b/src/lint/linter/ArcanistPhutilLibraryLinter.php
@@ -53,6 +53,14 @@
}
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
// 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
@@ -61,6 +69,26 @@
$bootloader = PhutilBootloader::getInstance();
$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
// itself. This means lint results will accurately reflect the state of
// the working copy.
@@ -80,7 +108,6 @@
}
}
- $all_symbols = array();
foreach ($symbols as $library => $map) {
// Check for files which declare more than one class/interface in the same
// file, or mix function definitions with class/interface definitions. We
@@ -125,20 +152,12 @@
}
}
- $libtype_map = array(
- 'class' => 'class',
- 'function' => 'function',
- 'interface' => 'class',
- 'class/interface' => 'class',
- );
-
// Check for duplicate symbols: two files providing the same class or
// 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.
- $normal_symbols = array();
foreach ($map as $file => $spec) {
$have = idx($spec, 'have', array());
foreach (array('class', 'function', 'interface') as $type) {
@@ -160,12 +179,20 @@
$old_src = $all_symbols[$libtype][$old_symbol]['file'];
$old_lib = $all_symbols[$libtype][$old_symbol]['library'];
- $this->raiseLintInLibrary(
- $library,
- $file,
- $offset,
- self::LINT_DUPLICATE_SYMBOL,
- pht(
+ // 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".',
@@ -174,7 +201,15 @@
$file,
$library,
$old_src,
- $old_lib));
+ $old_lib);
+ }
+
+ $this->raiseLintInLibrary(
+ $library,
+ $file,
+ $offset,
+ self::LINT_DUPLICATE_SYMBOL,
+ $message);
}
}
}
diff --git a/src/moduleutils/PhutilLibraryMapBuilder.php b/src/moduleutils/PhutilLibraryMapBuilder.php
--- a/src/moduleutils/PhutilLibraryMapBuilder.php
+++ b/src/moduleutils/PhutilLibraryMapBuilder.php
@@ -266,9 +266,29 @@
*/
private function buildSymbolAnalysisFuture($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';
- 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);
}
diff --git a/support/lib/extract-symbols.php b/support/lib/extract-symbols.php
--- a/support/lib/extract-symbols.php
+++ b/support/lib/extract-symbols.php
@@ -23,8 +23,8 @@
Symbols are reported in JSON on stdout.
- This script is used internally by libphutil/arcanist to build maps of
- library symbols.
+ This script is used internally by Arcanist to build maps of library
+ symbols.
It would be nice to eventually implement this as a C++ xhpast binary,
as it's relatively stable and performance is currently awful
@@ -38,7 +38,11 @@
array(
'name' => 'all',
'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(
'name' => 'ugly',
@@ -52,14 +56,32 @@
));
$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_builtins = $args->getArg('builtins');
+
+if ($show_all && $show_builtins) {
+ throw new PhutilArgumentUsageException(
+ pht(
+ 'Flags "--all" and "--builtins" are not compatible.'));
+}
-$source_code = Filesystem::readFile($path);
+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 {
$tree = XHPASTTree::newFromData($source_code);
@@ -474,6 +496,14 @@
$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(
'have' => $declared_symbols,
'need' => $required_symbols,
@@ -543,8 +573,6 @@
'parent' => true,
'self' => true,
- 'PhutilBootloader' => true,
-
// 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
// some cases, to write code which is compatible with either PHP5 or
@@ -570,12 +598,6 @@
'isset' => 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
// 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.

File Metadata

Mime Type
text/plain
Expires
Sun, Mar 23, 7:24 AM (1 d, 18 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7228206
Default Alt Text
D21538.diff (8 KB)

Event Timeline

Phabricator · Built by Phacility