Attachment 359977 Details for Bug 193750 – Patch v1

[patch] Patch v1

bug-193750-20190123174032.patch (text/plain), 8.23 KB, created by David Kilzer (:ddkilzer) on 2019-01-23 17:40:33 PST

(hide)

Description:

Filename:

MIME Type:

Creator: David Kilzer (:ddkilzer)

Created: 2019-01-23 17:40:33 PST

Size: 8.23 KB

patch

obsolete

>Subversion Revision: 240252
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 8852aca9bbd60bf43d22dbed9e70568d3e66197a..ab2dbffdb65fe540affd4659a4ad24298a9095f5 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,22 @@
>+2019-01-23  David Kilzer  <ddkilzer@apple.com>
>+
>+        check-webkit-style should warn when using soft-linking macros in a header
>+        <https://webkit.org/b/193750>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * Scripts/webkitpy/style/checkers/cpp.py:
>+        (check_language): Add softlink/header check by refactoring the
>+        softlink/framework check.
>+        (CppChecker): Enable softlink/header check by default.
>+        * Scripts/webkitpy/style/checkers/cpp_unittest.py:
>+        (CppStyleTest.test_softlink_framework): Rename from
>+        CppStyleTest.test_softlink() and change test framework name in
>+        case AVFoundation gets its own AVFoundationSoftLink.{h,mm}
>+        files. Set file_name for macros that only appear in headers.
>+        (CppStyleTest.test_softlink_header): Add tests for using
>+        soft-link macros in headers.
>+
> 2018-12-07  David Kilzer  <ddkilzer@apple.com>
> 
>         Ignore false-positive leaks under bmalloc::Heap::Heap
>diff --git a/Tools/Scripts/webkitpy/style/checkers/cpp.py b/Tools/Scripts/webkitpy/style/checkers/cpp.py
>index be373cf8a89ee449d97976079a362a42486f12af..8b0a8dc17f554bbc5c6a3c794240c4b85ca2ab7c 100644
>--- a/Tools/Scripts/webkitpy/style/checkers/cpp.py
>+++ b/Tools/Scripts/webkitpy/style/checkers/cpp.py
>@@ -3332,11 +3332,15 @@ def check_language(filename, clean_lines, line_number, file_extension, include_s
>                           'RetainPtr<> should never contain a type with \'*\'. Correct: RetainPtr<NSString>, RetainPtr<CFStringRef>.')
>                 break
> 
>-    frameworks_with_soft_links = ['CoreMedia', 'CoreVideo', 'DataDetectorsCore', 'MediaAccessibility', 'MediaRemote', 'PassKit', 'QuickLook', 'UIKit', 'VideoToolbox']
>-    matched = re.compile('^\s*SOFT_LINK_FRAMEWORK.*\(({})\)'.format('|'.join(frameworks_with_soft_links))).search(line)
>+    matched = re.compile('^\s*SOFT_LINK_(PRIVATE_)?FRAMEWORK.*\((\S+)\)').search(line)
>     if matched:
>-        framework_name = matched.group(1)
>-        if not re.compile('^\s*SOFT_LINK_FRAMEWORK_FOR_(HEADER|SOURCE)(_WITH_EXPORT)?\({}\)'.format(framework_name)).search(line):
>+        framework_name = matched.group(2)
>+        if file_extension == 'h' and not search(r'^\s*SOFT_LINK_(PRIVATE_)?FRAMEWORK_FOR_HEADER.*\(', line):
>+            error(line_number, 'softlink/header', 5,
>+                  'Never soft-link frameworks in headers. Put the soft-link macros in a source file, or create {framework}SoftLink.{{cpp,mm}} instead.'.format(framework=framework_name))
>+
>+        frameworks_with_soft_links = ['CoreMedia', 'CoreVideo', 'DataDetectorsCore', 'MediaAccessibility', 'MediaRemote', 'PassKit', 'QuickLook', 'UIKit', 'VideoToolbox']
>+        if framework_name in frameworks_with_soft_links and not re.compile('^\s*SOFT_LINK_(PRIVATE_)?FRAMEWORK_FOR_(HEADER|SOURCE)(_WITH_EXPORT)?\({}\)'.format(framework_name)).search(line):
>             error(line_number, 'softlink/framework', 5,
>                   'Use {framework}SoftLink.{{cpp,h,mm}} to soft-link to {framework}.framework.'.format(framework=framework_name))
> 
>@@ -4093,6 +4097,7 @@ class CppChecker(object):
>         'security/printf',
>         'security/temp_file',
>         'softlink/framework',
>+        'softlink/header',
>         'whitespace/blank_line',
>         'whitespace/braces',
>         'whitespace/brackets',
>diff --git a/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py b/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py
>index 78f2f1dc495b3161219869d3803081ef622154f6..e144d793ae824ff51c882b938e90b9ca938ebe01 100644
>--- a/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py
>+++ b/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py
>@@ -1688,24 +1688,35 @@ class CppStyleTest(CppStyleTestBase):
>             '  [runtime/retainptr] [5]')
>         self.assert_lint('''RetainPtr<NSDictionary<NSString *, NSArray<NSString *>>> dictionary;''', '')
> 
>-    def test_softlink(self):
>+    def test_softlink_framework(self):
>         self.assert_lint(
>-            '''SOFT_LINK_FRAMEWORK(AVFoundation)''',
>+            '''SOFT_LINK_FRAMEWORK(Foundation)''',
>             '')
>         self.assert_lint(
>-            '''SOFT_LINK_FRAMEWORK_OPTIONAL(AVFoundation)''',
>+            '''SOFT_LINK_FRAMEWORK_OPTIONAL(Foundation)''',
>             '')
>         self.assert_lint(
>-            '''SOFT_LINK_FRAMEWORK_OPTIONAL_PREFLIGHT(AVFoundation)''',
>+            '''SOFT_LINK_FRAMEWORK_OPTIONAL_PREFLIGHT(Foundation)''',
>             '')
>         self.assert_lint(
>-            '''SOFT_LINK_FRAMEWORK_FOR_HEADER(AVFoundation)''',
>+            '''SOFT_LINK_FRAMEWORK_FOR_HEADER(Foundation)''',
>+            '',
>+             file_name='foo.h')
>+        self.assert_lint(
>+            '''SOFT_LINK_FRAMEWORK_FOR_SOURCE(Foundation)''',
>             '')
>         self.assert_lint(
>-            '''SOFT_LINK_FRAMEWORK_FOR_SOURCE(AVFoundation)''',
>+            '''SOFT_LINK_FRAMEWORK_FOR_SOURCE_WITH_EXPORT(Foundation)''',
>             '')
>         self.assert_lint(
>-            '''SOFT_LINK_FRAMEWORK_FOR_SOURCE_WITH_EXPORT(AVFoundation)''',
>+            '''SOFT_LINK_PRIVATE_FRAMEWORK_FOR_HEADER(Foundation)''',
>+            '',
>+             file_name='foo.h')
>+        self.assert_lint(
>+            '''SOFT_LINK_PRIVATE_FRAMEWORK_FOR_SOURCE(Foundation)''',
>+            '')
>+        self.assert_lint(
>+            '''SOFT_LINK_PRIVATE_FRAMEWORK_FOR_SOURCE_WITH_EXPORT(Foundation)''',
>             '')
> 
>         self.assert_lint(
>@@ -1722,7 +1733,8 @@ class CppStyleTest(CppStyleTestBase):
>             '  [softlink/framework] [5]')
>         self.assert_lint(
>             '''SOFT_LINK_FRAMEWORK_FOR_HEADER(UIKit)''',
>-            '')
>+            '',
>+             file_name='foo.h')
>         self.assert_lint(
>             '''SOFT_LINK_FRAMEWORK_FOR_SOURCE(UIKit)''',
>             '')
>@@ -1730,6 +1742,43 @@ class CppStyleTest(CppStyleTestBase):
>             '''SOFT_LINK_FRAMEWORK_FOR_SOURCE_WITH_EXPORT(UIKit)''',
>             '')
> 
>+    def test_softlink_header(self):
>+        self.assert_lint(
>+            '''SOFT_LINK_FRAMEWORK(MyFramework)''',
>+            'Never soft-link frameworks in headers. Put the soft-link macros in a source file, or create MyFrameworkSoftLink.{cpp,mm} instead.'
>+            '  [softlink/header] [5]',
>+            file_name='foo.h')
>+        self.assert_lint(
>+            '''SOFT_LINK_FRAMEWORK_OPTIONAL(MyFramework)''',
>+            'Never soft-link frameworks in headers. Put the soft-link macros in a source file, or create MyFrameworkSoftLink.{cpp,mm} instead.'
>+            '  [softlink/header] [5]',
>+            file_name='foo.h')
>+        self.assert_lint(
>+            '''SOFT_LINK_FRAMEWORK_OPTIONAL_PREFLIGHT(MyFramework)''',
>+            'Never soft-link frameworks in headers. Put the soft-link macros in a source file, or create MyFrameworkSoftLink.{cpp,mm} instead.'
>+            '  [softlink/header] [5]',
>+            file_name='foo.h')
>+        self.assert_lint(
>+            '''SOFT_LINK_PRIVATE_FRAMEWORK(MyPrivateFramework)''',
>+            'Never soft-link frameworks in headers. Put the soft-link macros in a source file, or create MyPrivateFrameworkSoftLink.{cpp,mm} instead.'
>+            '  [softlink/header] [5]',
>+            file_name='foo.h')
>+
>+        self.assert_lint(
>+            '''SOFT_LINK_FRAMEWORK_FOR_HEADER(MyFramework)''',
>+            '',
>+            file_name='foo.h')
>+        self.assert_lint(
>+            '''SOFT_LINK_FRAMEWORK_FOR_SOURCE(MyFramework)''',
>+            'Never soft-link frameworks in headers. Put the soft-link macros in a source file, or create MyFrameworkSoftLink.{cpp,mm} instead.'
>+            '  [softlink/header] [5]',
>+            file_name='foo.h')
>+        self.assert_lint(
>+            '''SOFT_LINK_FRAMEWORK_FOR_SOURCE_WITH_EXPORT(MyFramework)''',
>+            'Never soft-link frameworks in headers. Put the soft-link macros in a source file, or create MyFrameworkSoftLink.{cpp,mm} instead.'
>+            '  [softlink/header] [5]',
>+            file_name='foo.h')
>+
>     # Variable-length arrays are not permitted.
>     def test_variable_length_array_detection(self):
>         errmsg = ('Do not use variable-length arrays.  Use an appropriately named '

Actions: View | Formatted Diff | Diff

Attachments on bug 193750: 359977