diff --git a/src/lint/linter/ArcanistPhutilLibraryLinter.php b/src/lint/linter/ArcanistPhutilLibraryLinter.php index 7c71625f..ba89e0c0 100644 --- a/src/lint/linter/ArcanistPhutilLibraryLinter.php +++ b/src/lint/linter/ArcanistPhutilLibraryLinter.php @@ -1,229 +1,335 @@ pht('Unknown Symbol'), self::LINT_DUPLICATE_SYMBOL => pht('Duplicate Symbol'), self::LINT_ONE_CLASS_PER_FILE => pht('One Class Per File'), + self::LINT_NONCANONICAL_SYMBOL => pht('Noncanonical Symbol'), ); } public function getLinterPriority() { return 2.0; } public function willLintPaths(array $paths) { // 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 // caches. $bootloader = PhutilBootloader::getInstance(); $libraries = $bootloader->getAllLibraries(); // 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. $symbols = array(); foreach ($libraries as $library) { $root = phutil_get_library_root($library); try { $symbols[$library] = id(new PhutilLibraryMapBuilder($root)) ->buildFileSymbolMap(); } catch (XHPASTSyntaxErrorException $ex) { // If the library contains a syntax error then there isn't much that we // can do. continue; } } $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 // must isolate autoloadable symbols to one per file so the autoloader // can't end up in an unresolvable cycle. foreach ($map as $file => $spec) { $have = idx($spec, 'have', array()); $have_classes = idx($have, 'class', array()) + idx($have, 'interface', array()); $have_functions = idx($have, 'function'); if ($have_functions && $have_classes) { $function_list = implode(', ', array_keys($have_functions)); $class_list = implode(', ', array_keys($have_classes)); $this->raiseLintInLibrary( $library, $file, end($have_functions), self::LINT_ONE_CLASS_PER_FILE, pht( "File '%s' mixes function (%s) and class/interface (%s) ". "definitions in the same file. A file which declares a class ". "or an interface MUST declare nothing else.", $file, $function_list, $class_list)); } else if (count($have_classes) > 1) { $class_list = implode(', ', array_keys($have_classes)); $this->raiseLintInLibrary( $library, $file, end($have_classes), self::LINT_ONE_CLASS_PER_FILE, pht( "File '%s' declares more than one class or interface (%s). ". "A file which declares a class or interface MUST declare ". "nothing else.", $file, $class_list)); } } + $libtype_map = array( + 'class' => 'class', + 'function' => 'function', + 'interface' => 'class', + 'class/interface' => 'class', + ); + // 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. + + $normal_symbols = array(); foreach ($map as $file => $spec) { $have = idx($spec, 'have', array()); foreach (array('class', 'function', 'interface') as $type) { - $libtype = ($type == 'interface') ? 'class' : $type; + $libtype = $libtype_map[$type]; 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( 'library' => $library, 'file' => $file, 'offset' => $offset, ); continue; } - $osrc = $all_symbols[$libtype][$symbol]['file']; - $olib = $all_symbols[$libtype][$symbol]['library']; + $old_symbol = $normal_symbols[$libtype][$normal_symbol]; + $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( - "Definition of %s '%s' in '%s' in library '%s' duplicates ". - "prior definition in '%s' in library '%s'.", - $type, + '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, - $osrc, - $olib)); + $old_src, + $old_lib)); } } } } $types = array('class', 'function', 'interface', 'class/interface'); foreach ($symbols as $library => $map) { // Check for unknown symbols: uses of classes, functions or interfaces // which are not defined anywhere. We reference the list of all symbols // we built up earlier. foreach ($map as $file => $spec) { $need = idx($spec, 'need', array()); foreach ($types as $type) { - $libtype = $type; - if ($type == 'interface' || $type == 'class/interface') { - $libtype = 'class'; - } + $libtype = $libtype_map[$type]; foreach (idx($need, $type, array()) as $symbol => $offset) { if (!empty($all_symbols[$libtype][$symbol])) { // Symbol is defined somewhere. 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( $library, $file, $offset, self::LINT_UNKNOWN_SYMBOL, - pht( - "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')); + $message); } } } } } - 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); $this->activePath = $root.'/'.$path; - $this->raiseLintAtOffset($offset, $code, $desc); + $this->raiseLintAtOffset($offset, $code, $desc, $original, $replacement); } public function lintPath($path) { return; } + private function normalizeSymbol($symbol) { + return phutil_utf8_strtolower($symbol); + } + }