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 = ''; + $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.