From 08dbbbba5ade68d463df3098569dd27eb21d3833 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 3 Feb 2021 08:23:46 -0800 Subject: [PATCH] When lint identifies an unknown symbol, attempt to correct it if it is miscapitalized Summary: Ref T13598. If you spell a symbol like "Polygon" as "PoLyGoN", you currently get an "unknown symbol" lint message. However, provided "Polygon" is a valid symbol, we can unambiguously correct the spelling of the symbol. Note that this patch can only correct the spelling of application symbols, not builtin symbols (since none of the library maps contain builtin symbols). Test Plan: {F8374599} Maniphest Tasks: T13598 Differential Revision: https://secure.phabricator.com/D21537 --- .../linter/ArcanistPhutilLibraryLinter.php | 186 ++++++++++++++---- 1 file changed, 146 insertions(+), 40 deletions(-) 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 @@ -13,6 +13,7 @@ final class ArcanistPhutilLibraryLinter extends ArcanistLinter { const LINT_UNKNOWN_SYMBOL = 1; const LINT_DUPLICATE_SYMBOL = 2; const LINT_ONE_CLASS_PER_FILE = 3; + const LINT_NONCANONICAL_SYMBOL = 4; public function getInfoName() { return pht('Phutil Library Linter'); @@ -43,6 +44,7 @@ final class ArcanistPhutilLibraryLinter extends ArcanistLinter { self::LINT_UNKNOWN_SYMBOL => 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'), ); } @@ -123,14 +125,29 @@ 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 - // 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, @@ -139,8 +156,9 @@ final class ArcanistPhutilLibraryLinter extends ArcanistLinter { 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, @@ -148,14 +166,15 @@ final class ArcanistPhutilLibraryLinter extends ArcanistLinter { $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)); } } } @@ -169,61 +188,148 @@ final class ArcanistPhutilLibraryLinter extends ArcanistLinter { 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); + } + }