Changeset 265096 in webkit
- Timestamp:
- Jul 30, 2020 2:44:33 PM (4 years ago)
- Location:
- trunk/Tools
- Files:
-
- 4 edited
-
ChangeLog (modified) (1 diff)
-
DumpRenderTree/mac/DumpRenderTree.mm (modified) (3 diffs)
-
Scripts/webkitpy/style/checkers/cpp.py (modified) (5 diffs)
-
Scripts/webkitpy/style/checkers/cpp_unittest.py (modified) (1 diff)
Legend:
- Unmodified
- Added
- Removed
-
trunk/Tools/ChangeLog
r265094 r265096 1 2020-07-30 David Kilzer <ddkilzer@apple.com> 2 3 check-webkit-style should enforce acronym capitalization at start/end of an identifier 4 <https://webkit.org/b/214954> 5 6 Reviewed by Jonathan Bedard. 7 8 * DumpRenderTree/mac/DumpRenderTree.mm: 9 (shouldIgnoreWebCoreNodeLeaks): 10 (changeWindowScaleIfNeeded): 11 - Fix case issues in variables using URL acronym. 12 13 * Scripts/webkitpy/style/checkers/cpp.py: 14 (_check_identifier_name_for_acronyms): Add. 15 - This contains the logic to report acronyms with invalid case 16 at the start and at the end of an identifer. 17 (check_identifier_name_in_declaration): 18 - Keep track of whether the identifer came from a class, 19 namespace or struct. 20 - Fix bug where `auto` variables would not be checked because 21 `auto` was removed with other non-type keywords like `inline`, 22 leaving no type for the variable. Add a comment to describe 23 what this line is doing. 24 - Fix bug when removing "new" that changed an identifier named 25 "newURL" to "URL". 26 - Add call to _check_identifier_name_for_acronyms() to implement 27 the check. 28 (CppChecker): 29 - Add 'readability/naming/acronym' to the list of active 30 checkers. 31 32 * Scripts/webkitpy/style/checkers/cpp_unittest.py: 33 (WebKitStyleTest.test_identifier_names_with_acronyms): 34 - Add tests. About half of the tests were taken from actual 35 code that initially caused false positives during development. 36 1 37 2020-07-30 Beth Dakin <bdakin@apple.com> 2 38 -
trunk/Tools/DumpRenderTree/mac/DumpRenderTree.mm
r264957 r265096 295 295 } 296 296 297 static bool shouldIgnoreWebCoreNodeLeaks(const string& URLString)297 static bool shouldIgnoreWebCoreNodeLeaks(const string& urlString) 298 298 { 299 299 static char* const ignoreSet[] = { … … 305 305 // FIXME: ignore case 306 306 string curIgnore(ignoreSet[i]); 307 // Match at the end of the URLString308 if (! URLString.compare(URLString.length() - curIgnore.length(), curIgnore.length(), curIgnore))307 // Match at the end of the urlString. 308 if (!urlString.compare(urlString.length() - curIgnore.length(), curIgnore.length(), curIgnore)) 309 309 return true; 310 310 } … … 1646 1646 1647 1647 #if !PLATFORM(IOS_FAMILY) 1648 static void changeWindowScaleIfNeeded(const char* testPathOrU rl)1649 { 1650 auto localPathOrU rl = String(testPathOrUrl);1648 static void changeWindowScaleIfNeeded(const char* testPathOrURL) 1649 { 1650 auto localPathOrURL = String(testPathOrURL); 1651 1651 float currentScaleFactor = [[[mainFrame webView] window] backingScaleFactor]; 1652 1652 float requiredScaleFactor = 1; 1653 if (localPathOrU rl.containsIgnoringASCIICase("/hidpi-3x-"))1653 if (localPathOrURL.containsIgnoringASCIICase("/hidpi-3x-")) 1654 1654 requiredScaleFactor = 3; 1655 else if (localPathOrU rl.containsIgnoringASCIICase("/hidpi-"))1655 else if (localPathOrURL.containsIgnoringASCIICase("/hidpi-")) 1656 1656 requiredScaleFactor = 2; 1657 1657 if (currentScaleFactor == requiredScaleFactor) -
trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py
r264949 r265096 1794 1794 'The parameter name "%s" adds no information, so it should be removed.' % parameter.name) 1795 1795 return False 1796 return True 1797 1798 1799 def _check_identifier_name_for_acronyms(identifier, line_number, is_class_or_namespace_or_struct_name, error): 1800 """Checks to see if the identifier name contains an acronym with improper case. 1801 1802 Using "url" is okay at the start of an identifier name, and "URL" is okay in the 1803 middle or at the end of an identifier name, but "Url" is never okay. 1804 """ 1805 acronyms = '|'.join(['MIME', 'URL']) 1806 1807 start_re = re.compile('^(%s)(|$)' % acronyms, re.IGNORECASE) 1808 start_expected_re = re.compile('^(%s)([^:]|$)' % acronyms.lower()) 1809 # Identifiers that start with an acronym must be all lowercase, except for class/namespace/struct names. 1810 if start_re.search(identifier) and not start_expected_re.search(identifier): 1811 start_uppercase_re = re.compile('^(%s)' % acronyms) 1812 # Ignore class/namespace/struct names that start with all-uppercase acronyms. 1813 if start_uppercase_re.search(identifier) and \ 1814 (is_class_or_namespace_or_struct_name or identifier.find('::') != -1): 1815 return True 1816 error(line_number, 'readability/naming/acronym', 5, 1817 'The identifier name "%s" starts with a acronym that is not all lowercase.' % identifier) 1818 return False 1819 1820 # FIXME: Hard to check middle words without knowing that the word to the left doesn't end with an acronym. 1821 1822 # Identifiers that end with an acronym must be all uppercase, except for variables like 'm_url' and 'Class::url()'. 1823 end_re = re.compile('[^_:](%s)$' % acronyms, re.IGNORECASE) 1824 end_expected_re = re.compile('[^_:](%s)$' % acronyms) 1825 if end_re.search(identifier) and not end_expected_re.search(identifier): 1826 error(line_number, 'readability/naming/acronym', 5, 1827 'The identifier name "%s" ends with a acronym that is not all uppercase.' % identifier) 1828 return False 1829 1796 1830 return True 1797 1831 … … 3701 3735 3702 3736 def check_identifier_name_in_declaration(filename, line_number, line, file_state, error): 3703 """Checks if identifier names contain any underscores.3737 """Checks if identifier names contain any anti-patterns like underscores. 3704 3738 3705 3739 As identifiers in libraries we are using have a bunch of … … 3746 3780 # Convert unsigned/signed types to simple types, too. 3747 3781 line = sub(r'(unsigned|signed) (?=char|short|int|long)', '', line) 3748 line = sub(r'\b(inline|using|static|const|volatile|auto|register|extern|typedef|restrict|struct|class|virtual)(?=\W)', '', line) 3782 3783 is_class_or_namespace_or_struct_name = False 3784 class_namespace_struct_name_re = re.compile('\\b(class|explicit|namespace|struct|%s)(?=\\W)' % '|'.join(_EXPORT_MACROS)) 3785 if class_namespace_struct_name_re.search(line): 3786 is_class_or_namespace_or_struct_name = True 3787 3788 # Remove keywords that aren't types. 3789 line = sub(r'\b(inline|using|static|const|volatile|register|extern|typedef|restrict|struct|class|virtual)(?=\W)', '', line) 3749 3790 3750 3791 # Remove "new" and "new (expr)" to simplify, too. 3751 line = sub(r'new\s*(\([^)]*\))?', '', line) 3792 line = sub(r'new\s+', '', line) 3793 line = sub(r'new\s*(\([^)]*\))', '', line) 3752 3794 3753 3795 # Remove all template parameters by removing matching < and >. … … 3838 3880 if modified_identifier == 'l': 3839 3881 error(line_number, 'readability/naming', 4, identifier + " is incorrectly named. Don't use the single letter 'l' as an identifier name.") 3882 3883 _check_identifier_name_for_acronyms(identifier, line_number, is_class_or_namespace_or_struct_name, error) 3840 3884 3841 3885 # There can be only one declaration in non-for-control statements. … … 4322 4366 'readability/parameter_name', 4323 4367 'readability/naming', 4368 'readability/naming/acronym', 4324 4369 'readability/naming/protected', 4325 4370 'readability/naming/underscores', -
trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py
r264949 r265096 5950 5950 '}\n', 'test.cpp', parameter_error_rules)) 5951 5951 5952 def test_identifier_names_with_acronyms(self): 5953 identifier_error_rules = ('-', 5954 '+readability/naming/acronym') 5955 5956 # Start of parameter name. 5957 error_start = 'The identifier name "%s" starts with a acronym that is not all lowercase.'\ 5958 ' [readability/naming/acronym] [5]' 5959 5960 self.assertEqual('', 5961 self.perform_lint('void load(URL url);', 'test.cpp', identifier_error_rules)) 5962 self.assertEqual(error_start % 'Url', 5963 self.perform_lint('void load(URL Url);', 'test.cpp', identifier_error_rules)) 5964 self.assertEqual(error_start % 'URL', 5965 self.perform_lint('void load(URL URL);', 'test.cpp', identifier_error_rules)) 5966 self.assertEqual('', 5967 self.perform_lint('void load(URL urlToLoad);', 'test.cpp', identifier_error_rules)) 5968 self.assertEqual(error_start % 'UrlToLoad', 5969 self.perform_lint('void load(URL UrlToLoad);', 'test.cpp', identifier_error_rules)) 5970 self.assertEqual(error_start % 'URLToLoad', 5971 self.perform_lint('void load(URL URLToLoad);', 'test.cpp', identifier_error_rules)) 5972 5973 self.assertEqual('', 5974 self.perform_lint('friend class URL;', 'test.cpp', identifier_error_rules)) 5975 self.assertEqual('', 5976 self.perform_lint('enum class URLPart;', 'test.cpp', identifier_error_rules)) 5977 self.assertEqual('', 5978 self.perform_lint('using namespace URLHelpers;', 'test.cpp', identifier_error_rules)) 5979 5980 self.assertEqual('', 5981 self.perform_lint('explicit URL(HashTableDeletedValueType);', 'test.cpp', identifier_error_rules)) 5982 self.assertEqual('', 5983 self.perform_lint('WTF_EXPORT_PRIVATE URL(CFURLRef);', 'test.cpp', identifier_error_rules)) 5984 self.assertEqual('', 5985 self.perform_lint('void URL::invalidate()', 'test.cpp', identifier_error_rules)) 5986 self.assertEqual('', 5987 self.perform_lint('void URL::URL(const URL& base, const String& relative, const URLTextEncoding* encoding)', 'test.cpp', identifier_error_rules)) 5988 self.assertEqual('', 5989 self.perform_lint('URL URL::isolatedCopy() const', 'test.cpp', identifier_error_rules)) 5990 self.assertEqual('', 5991 self.perform_lint('URLParser::URLParser(const String& input, const URL& base, const URLTextEncoding* nonUTF8QueryEncoding)', 'test.cpp', identifier_error_rules)) 5992 self.assertEqual('', 5993 self.perform_lint('bool URLParser::internalValuesConsistent(const URL& url)', 'test.cpp', identifier_error_rules)) 5994 5995 self.assertEqual('', 5996 self.perform_lint('String m_url;', 'test.cpp', identifier_error_rules)) 5997 self.assertEqual('', 5998 self.perform_lint('JSRetainPtr<JSStringRef> AccessibilityUIElement::url()', 'test.cpp', identifier_error_rules)) 5999 6000 self.assertEqual(error_start % 'Url::Url', 6001 self.perform_lint('void Url::Url()', 'test.cpp', identifier_error_rules)) 6002 self.assertEqual(error_start % 'UrlParse::UrlParse', 6003 self.perform_lint('void UrlParse::UrlParse()', 'test.cpp', identifier_error_rules)) 6004 6005 # FIXME: Hard to check middle words without knowing that the word to the left doesn't end with an acronym. 6006 6007 error_end = 'The identifier name "%s" ends with a acronym that is not all uppercase.'\ 6008 ' [readability/naming/acronym] [5]' 6009 6010 # End of parameter name. 6011 self.assertEqual(error_end % 'loadurl', 6012 self.perform_lint('void load(URL loadurl);', 'test.cpp', identifier_error_rules)) 6013 self.assertEqual(error_end % 'loadUrl', 6014 self.perform_lint('void load(URL loadUrl);', 'test.cpp', identifier_error_rules)) 6015 self.assertEqual('', 6016 self.perform_lint('void load(URL loadURL);', 'test.cpp', identifier_error_rules)) 6017 6018 self.assertEqual('', 6019 self.perform_lint('void InspectorFrontendHost::inspectedURLChanged(const String& newURL)', 'test.cpp', identifier_error_rules)) 6020 self.assertEqual(error_end % 'testPathOrUrl', 6021 self.perform_lint('static void changeWindowScaleIfNeeded(const char* testPathOrUrl)', 'test.cpp', identifier_error_rules)) 6022 self.assertEqual(error_end % 'localPathOrUrl', 6023 self.perform_lint('auto localPathOrUrl = String(testPathOrURL);', 'test.cpp', identifier_error_rules)) 6024 5952 6025 def test_comments(self): 5953 6026 # A comment at the beginning of a line is ok.
Note: See TracChangeset
for help on using the changeset viewer.
