Bug 171486 – [Cocoa] Have check-webkit-style advise against use of [get…Class() alloc]

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

It’s safer to use alloc…Instance() so the style checker could suggest that.

Patch forthcoming.

Created attachment 308701 [details]
Add style check

Committed <https://trac.webkit.org/r216000>.

[Daniel Bates]

Can we add a unit test for this change? Notice that we have unit tests in <https://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py>. To run the unit tests, run Tools/Scripts/test-webkitpy.

[David Kilzer (:ddkilzer)]

The webkitpy unit test started to fail with this change:

[1068/1576] webkitpy.style.checkers.cpp_unittest.CppStyleTest.test_spacing_before_brackets failed:                      
  Traceback (most recent call last):
    File "./Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py", line 1885, in test_spacing_before_brackets
      self.assert_lint('        return [get_##framework##_##className##Class() alloc]; \\', '')
    File "./Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py", line 317, in assert_lint
      self.assertEqual(expected_message, self.perform_single_line_lint(code, file_name))
    File "./Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py", line 270, in perform_single_line_lint
      return self.perform_lint(code, filename, basic_error_rules)
    File "./Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py", line 259, in perform_lint
      self.process_file_data(filename, extension, lines, error_collector, unit_test_config)
    File "./Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py", line 253, in process_file_data
      checker.check(lines)
    File "./Tools/Scripts/webkitpy/style/checkers/cpp.py", line 3986, in check
      self.handle_style_error, self.min_confidence)
    File "./Tools/Scripts/webkitpy/style/checkers/cpp.py", line 3846, in _process_lines
      enum_state, asm_state, error)
    File "./Tools/Scripts/webkitpy/style/checkers/cpp.py", line 3791, in process_line
      check_style(clean_lines, line, file_extension, class_state, file_state, enum_state, error)
    File "./Tools/Scripts/webkitpy/style/checkers/cpp.py", line 2842, in check_style
      check_soft_link_class_alloc(clean_lines, line_number, error)
    File "./Tools/Scripts/webkitpy/style/checkers/cpp.py", line 2746, in check_soft_link_class_alloc
      'Using +alloc with a soft-linked class. Use alloc%sInstance() instead.' % matched.group(1))
    File "./Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py", line 69, in __call__
      ' which is not in CppChecker.categories' % (message, category))
  AssertionError: Message "Using +alloc with a soft-linked class. Use alloc_##framework##_##className##Instance() instead." has category "runtime/soft-linked-alloc", which is not in CppChecker.categories
  
Ran 1576 tests in 5.895s                                                                                                
FAILED (failures=1, errors=0)

And fixing that results in a false-positive match for the macro:

[1056/1576] webkitpy.style.checkers.cpp_unittest.CppStyleTest.test_spacing_before_brackets failed:                      
  Traceback (most recent call last):
    File "./Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py", line 1885, in test_spacing_before_brackets
      self.assert_lint('        return [get_##framework##_##className##Class() alloc]; \\', '')
    File "./Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py", line 317, in assert_lint
      self.assertEqual(expected_message, self.perform_single_line_lint(code, file_name))
  AssertionError: '' != 'Using +alloc with a soft-linked class. Use alloc_##framework##_##className##Instance() instead.  [runtime/soft-linked-alloc] [4]'
  
Ran 1576 tests in 5.756s                                                                                                
FAILED (failures=1, errors=0)

Fixed in r216013:  <https://trac.webkit.org/r216013>

[David Kilzer (:ddkilzer)]

And FWIW, TestWebKitAPI has a few hits left:

$ ./Tools/Scripts/check-webkit-style --filter=-,+runtime/soft-linked-alloc Tools/TestWebKitAPI
ERROR: Tools/TestWebKitAPI/Tests/WebKit/ios/AudioSessionCategoryIOS.mm:69:  Using +alloc with a soft-linked class. Use allocUIWindowInstance() instead.  [runtime/soft-linked-alloc] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKit/ios/AudioSessionCategoryIOS.mm:70:  Using +alloc with a soft-linked class. Use allocUIWebViewInstance() instead.  [runtime/soft-linked-alloc] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKit2Cocoa/RequiresUserActionForPlayback.mm:76:  Using +alloc with a soft-linked class. Use allocUIWindowInstance() instead.  [runtime/soft-linked-alloc] [4]
Total errors found: 3 in 755 files

Filed: Bug 171493: [Cocoa] Replace uses of [get…Class() alloc] with alloc…Instance() in TestWebKitAPI
<https://bugs.webkit.org/show_bug.cgi?id=171493>

[David Kilzer (:ddkilzer)]

And Source/WebCore has a couple left, too:

$ ./Tools/Scripts/check-webkit-style --filter=-,+runtime/soft-linked-alloc Source/WebCore
ERROR: Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:447:  Using +alloc with a soft-linked class. Use allocWebAVPictureInPicturePlayerLayerViewInstance() instead.  [runtime/soft-linked-alloc] [4]
ERROR: Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:616:  Using +alloc with a soft-linked class. Use allocWebAVPlayerLayerViewInstance() instead.  [runtime/soft-linked-alloc] [4]
Total errors found: 2 in 8492 files

Filed: Bug 171514: [Cocoa] Replace uses of [get…Class() alloc] in Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm