Attachment 357320 Details for Bug 192703 – proposed patch

[patch] proposed patch

StyleCheck.txt (text/plain), 6.49 KB, created by Alexey Proskuryakov on 2018-12-14 09:48:45 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Alexey Proskuryakov

Created: 2018-12-14 09:48:45 PST

Size: 6.49 KB

patch

obsolete

>Index: Tools/ChangeLog
>===================================================================
>--- Tools/ChangeLog	(revision 239210)
>+++ Tools/ChangeLog	(working copy)
>@@ -1,3 +1,17 @@
>+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 NOBODY (OOPS!).
>+
>+        * 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>
> 
>         [PSON] WebsitePolicies are lost on process-swap
>Index: Tools/Scripts/webkitpy/style/checkers/cpp.py
>===================================================================
>--- Tools/Scripts/webkitpy/style/checkers/cpp.py	(revision 239031)
>+++ Tools/Scripts/webkitpy/style/checkers/cpp.py	(working copy)
>@@ -1093,6 +1093,43 @@ def check_invalid_increment(clean_lines,
>               '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):
>     """Stores information about a class."""
> 
>@@ -3865,6 +3902,7 @@ def process_line(filename, file_extensio
>     check_for_non_standard_constructs(clean_lines, line, class_state, error)
>     check_posix_threading(clean_lines, line, error)
>     check_invalid_increment(clean_lines, line, error)
>+    check_os_version_checks(filename, clean_lines, line, error)
> 
> 
> class _InlineASMState(object):
>@@ -3954,6 +3992,7 @@ class CppChecker(object):
>         'build/cpp_comment',
>         'build/webcore_export',
>         'build/wk_api_available',
>+        'build/version_check',
>         'legal/copyright',
>         'readability/braces',
>         'readability/casting',
>Index: Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py
>===================================================================
>--- Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py	(revision 239031)
>+++ Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py	(working copy)
>@@ -5528,6 +5528,16 @@ class WebKitStyleTest(CppStyleTestBase):
>         self.assert_lint('WK_API_AVAILABLE(macosx(WK_IOS_TBA), ios(3.4.5))', 'WK_IOS_TBA is neither a version number nor WK_MAC_TBA  [build/wk_api_available] [5]')
>         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.
>         pass

Actions: View | Formatted Diff | Diff

Attachments on bug 192703: 357320