Changeset 239250 in webkit

Timestamp:

Dec 14, 2018 11:51:56 PM (5 years ago)

Author:

ap@apple.com

Message:

Add a style checker rule for Xcode version macros use
​https://bugs.webkit.org/show_bug.cgi?id=192703

Reviewed by Alex Christensen.

• Scripts/webkitpy/style/checkers/cpp.py:

(check_os_version_checks):
(process_line):
(CppChecker):

• Scripts/webkitpy/style/checkers/cpp_unittest.py:

(WebKitStyleTest.test_os_version_checks):

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/style/checkers/cpp.py (modified) (3 diffs)
• Scripts/webkitpy/style/checkers/cpp_unittest.py (modified) (1 diff)

trunk/Tools/ChangeLog

@@ +1 @@
+2018-12-14  Alexey Proskuryakov  <ap@apple.com>
+
+        Add a style checker rule for Xcode version macros use
+        https://bugs.webkit.org/show_bug.cgi?id=192703
+
+        Reviewed by Alex Christensen.
+
+        * Scripts/webkitpy/style/checkers/cpp.py:
+        (check_os_version_checks):
+        (process_line):
+        (CppChecker):
+        * Scripts/webkitpy/style/checkers/cpp_unittest.py:
+        (WebKitStyleTest.test_os_version_checks):
+
 2018-12-14  Chris Dumez  <cdumez@apple.com>
 

trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py

@@ -1093 +1093 @@
               'Changing pointer instead of value (or unused value of operator*).')
 
+
+# Matches Xcode *VERSION_MIN_REQUIRED and *VERSION_MAX_ALLOWED macros.
+_RE_PATTERN_XCODE_VERSION_MACRO = re.compile(
+    r'.+(VERSION_MIN_REQUIRED|VERSION_MAX_ALLOWED)')
+
+_RE_PATTERN_XCODE_MIN_REQUIRED_MACRO = re.compile(
+    r'.+?([A-Z_]+)_VERSION_MIN_REQUIRED [><=]+ (\d+)')
+
+
+def check_os_version_checks(filename, clean_lines, line_number, error):
+    """ Checks for mistakes using VERSION_MIN_REQUIRED and VERSION_MAX_ALLOWED macros:
+    1. These should only be used centrally to defined named HAVE, USE or ENABLE style macros.
+    2. VERSION_MIN_REQUIRED never changes for a minor OS version.
+
+    These should be centralized in wtf/Platform.h and wtf/FeatureDefines.h.
+
+    Args:
+      filename: Name of the file that is being processed.
+      clean_lines: A CleansedLines instance containing the file.
+      line_number: The number of the line to check.
+      error: The function to call with any errors found.
+    """
+
+    line = clean_lines.elided[line_number]
+
+    for version_match in _RE_PATTERN_XCODE_MIN_REQUIRED_MACRO.finditer(line):
+        os_prefix = version_match.group(1)
+        version_number = int(version_match.group(2))
+        if os_prefix == '__MAC_OS_X' and version_number % 100 != 0 or os_prefix != '__MAC_OS_X' and version_number % 10000 != 0:
+            error(line_number, 'build/version_check', 5, 'Incorrect OS version check. VERSION_MIN_REQUIRED values never include a minor version. You may be looking for a combination of VERSION_MIN_REQUIRED for target OS version check and VERSION_MAX_ALLOWED for SDK check.')
+            break
+
+    if filename == 'Source/WTF/wtf/Platform.h' or filename == 'Source/WTF/wtf/FeatureDefines.h':
+        return
+
+    if _RE_PATTERN_XCODE_VERSION_MACRO.match(line):
+        error(line_number, 'build/version_check', 5, 'Misplaced OS version check. Please use a named macro in wtf/Platform.h or wtf/FeatureDefines.h.')
 
 class _ClassInfo(object):
@@ -3866 +3903 @@
     check_posix_threading(clean_lines, line, error)
     check_invalid_increment(clean_lines, line, error)
+    check_os_version_checks(filename, clean_lines, line, error)
 
 
@@ -3955 +3993 @@
         'build/webcore_export',
         'build/wk_api_available',
+        'build/version_check',
         'legal/copyright',
         'readability/braces',

trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py

@@ -5529 +5529 @@
         self.assert_lint('WK_API_AVAILABLE(macosx(1.2.3), ios(WK_MAC_TBA))', 'WK_MAC_TBA is neither a version number nor WK_IOS_TBA  [build/wk_api_available] [5]')
 
+    def test_os_version_checks(self):
+        self.assert_lint('#if PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED < 110000', 'Misplaced OS version check. Please use a named macro in wtf/Platform.h or wtf/FeatureDefines.h.  [build/version_check] [5]')
+        self.assert_lint('#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MAX_ALLOWED >= 101300', 'Misplaced OS version check. Please use a named macro in wtf/Platform.h or wtf/FeatureDefines.h.  [build/version_check] [5]')
+        self.assert_lint('#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED < 101300', '', 'Source/WTF/wtf/Platform.h')
+        self.assert_lint('#if PLATFORM(MAC) && __IPHONE_OS_VERSION_MIN_REQUIRED > 120000', '', 'Source/WTF/wtf/Platform.h')
+        self.assert_lint('#if PLATFORM(MAC) && __IPHONE_OS_VERSION_MIN_REQUIRED > 120400', 'Incorrect OS version check. VERSION_MIN_REQUIRED values never include a minor version. You may be looking for a combination of VERSION_MIN_REQUIRED for target OS version check and VERSION_MAX_ALLOWED for SDK check.  [build/version_check] [5]', 'Source/WTF/wtf/Platform.h')
+        self.assert_lint('#if !TARGET_OS_SIMULATOR && __WATCH_OS_VERSION_MIN_REQUIRED < 50000', 'Misplaced OS version check. Please use a named macro in wtf/Platform.h or wtf/FeatureDefines.h.  [build/version_check] [5]')
+        self.assert_lint('#if (PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED < 110000) || (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED < 101304)', ['Incorrect OS version check. VERSION_MIN_REQUIRED values never include a minor version. You may be looking for a combination of VERSION_MIN_REQUIRED for target OS version check and VERSION_MAX_ALLOWED for SDK check.  [build/version_check] [5]', 'Misplaced OS version check. Please use a named macro in wtf/Platform.h or wtf/FeatureDefines.h.  [build/version_check] [5]'])
+        self.assert_lint('#define FOO ((PLATFORM(MAC) && __MAC_OS_X_VERSION_MAX_ALLOWED >= 101302 && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101300))', 'Misplaced OS version check. Please use a named macro in wtf/Platform.h or wtf/FeatureDefines.h.  [build/version_check] [5]')
+
     def test_other(self):
         # FIXME: Implement this.