- Revision
- 96866
- Author
- commit-qu...@webkit.org
- Date
- 2011-10-06 15:44:45 -0700 (Thu, 06 Oct 2011)
Log Message
The GTK+ WebKit2 headers produce a lot of style warnings
https://bugs.webkit.org/show_bug.cgi?id=69481
Patch by Martin Robinson <mrobin...@igalia.com> on 2011-10-06
Reviewed by David Levin.
Prevent emitting so many style warnings for GTK+ API. We skip header
files in the WebKit2 GTK+ API directory and also avoid warnings about
identifier names that begin with "webkit_" in files that contain the
string "gtk".
* Scripts/webkitpy/style/checker.py: Do not check header files in
Source/WebKit2/UIProcess/API/gtk that do not end in Private.h. This required
adding the ability to specify a regular _expression_ in the skip list. Remove
a few files from the skipped list that no longer exist.
* Scripts/webkitpy/style/checker_unittest.py: Added a test for this behavior.
* Scripts/webkitpy/style/checkers/cpp.py: If a path contains "gtk" don't warn
about identifiers that begin with "webkit_".
* Scripts/webkitpy/style/checkers/cpp_unittest.py: Added a test for this behavior.
Modified Paths
Diff
Modified: trunk/Tools/ChangeLog (96865 => 96866)
--- trunk/Tools/ChangeLog 2011-10-06 22:43:40 UTC (rev 96865)
+++ trunk/Tools/ChangeLog 2011-10-06 22:44:45 UTC (rev 96866)
@@ -1,3 +1,24 @@
+2011-10-06 Martin Robinson <mrobin...@igalia.com>
+
+ The GTK+ WebKit2 headers produce a lot of style warnings
+ https://bugs.webkit.org/show_bug.cgi?id=69481
+
+ Reviewed by David Levin.
+
+ Prevent emitting so many style warnings for GTK+ API. We skip header
+ files in the WebKit2 GTK+ API directory and also avoid warnings about
+ identifier names that begin with "webkit_" in files that contain the
+ string "gtk".
+
+ * Scripts/webkitpy/style/checker.py: Do not check header files in
+ Source/WebKit2/UIProcess/API/gtk that do not end in Private.h. This required
+ adding the ability to specify a regular _expression_ in the skip list. Remove
+ a few files from the skipped list that no longer exist.
+ * Scripts/webkitpy/style/checker_unittest.py: Added a test for this behavior.
+ * Scripts/webkitpy/style/checkers/cpp.py: If a path contains "gtk" don't warn
+ about identifiers that begin with "webkit_".
+ * Scripts/webkitpy/style/checkers/cpp_unittest.py: Added a test for this behavior.
+
2011-10-06 Brent Fulgham <bfulg...@webkit.org>
[WinCairo] Correct config.json for WinCairo Test builds.
Modified: trunk/Tools/Scripts/webkitpy/style/checker.py (96865 => 96866)
--- trunk/Tools/Scripts/webkitpy/style/checker.py 2011-10-06 22:43:40 UTC (rev 96865)
+++ trunk/Tools/Scripts/webkitpy/style/checker.py 2011-10-06 22:44:45 UTC (rev 96866)
@@ -32,6 +32,7 @@
import logging
import os.path
+import re
import sys
from checkers.common import categories as CommonCategories
@@ -277,16 +278,13 @@
# WebKit maintains some files in Mozilla style on purpose to ease
# future merges.
_SKIPPED_FILES_WITH_WARNING = [
- "gtk2drawing.c", # WebCore/platform/gtk/gtk2drawing.c
- "gtkdrawing.h", # WebCore/platform/gtk/gtkdrawing.h
"Source/WebKit/gtk/tests/",
- # Soup API that is still being cooked, will be removed from WebKit
- # in a few months when it is merged into soup proper. The style
- # follows the libsoup style completely.
- "Source/WebCore/platform/network/soup/cache/",
- ]
+ # All WebKit*.h files in Source/WebKit2/UIProcess/API/gtk,
+ # except those ending in ...Private.h are GTK+ API headers,
+ # which differ greatly from WebKit coding style.
+ re.compile(r'Source/WebKit2/UIProcess/API/gtk/WebKit(?!.*Private\.h).*\.h$'),
+ 'Source/WebKit2/UIProcess/API/gtk/webkit2.h']
-
# Files to skip that are more common or obvious.
#
# This list should be in addition to files with FileType.NONE. Files
@@ -467,10 +465,18 @@
"""Return the file extension without the leading dot."""
return os.path.splitext(file_path)[1].lstrip(".")
+ def _should_skip_file_path(self, file_path, skip_array_entry):
+ if isinstance(skip_array_entry, str):
+ if file_path.find(skip_array_entry) >= 0:
+ return True
+ elif skip_array_entry.match(file_path):
+ return True
+ return False
+
def should_skip_with_warning(self, file_path):
"""Return whether the given file should be skipped with a warning."""
for skipped_file in _SKIPPED_FILES_WITH_WARNING:
- if file_path.find(skipped_file) >= 0:
+ if self._should_skip_file_path(file_path, skipped_file):
return True
return False
@@ -492,7 +498,7 @@
elif basename == 'test_expectations.txt' or basename == 'drt_expectations.txt':
return False
for skipped_file in _SKIPPED_FILES_WITHOUT_WARNING:
- if file_path.find(skipped_file) >= 0:
+ if self._should_skip_file_path(file_path, skipped_file):
return True
return False
Modified: trunk/Tools/Scripts/webkitpy/style/checker_unittest.py (96865 => 96866)
--- trunk/Tools/Scripts/webkitpy/style/checker_unittest.py 2011-10-06 22:43:40 UTC (rev 96865)
+++ trunk/Tools/Scripts/webkitpy/style/checker_unittest.py 2011-10-06 22:44:45 UTC (rev 96866)
@@ -302,22 +302,30 @@
def test_should_skip_with_warning(self):
"""Test should_skip_with_warning()."""
- # Check a non-skipped file.
- self.assertFalse(self._dispatcher.should_skip_with_warning("foo.txt"))
-
# Check skipped files.
paths_to_skip = [
- "gtk2drawing.c",
- "gtkdrawing.h",
- "Source/WebCore/platform/gtk/gtk2drawing.c",
- "Source/WebCore/platform/gtk/gtkdrawing.h",
"Source/WebKit/gtk/tests/testatk.c",
+ "Source/WebKit2/UIProcess/API/gtk/webkit2.h",
+ "Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h",
+ "Source/WebKit2/UIProcess/API/gtk/WebKitLoader.h",
]
for path in paths_to_skip:
self.assertTrue(self._dispatcher.should_skip_with_warning(path),
"Checking: " + path)
+ # Verify that some files are not skipped.
+ paths_not_to_skip = [
+ "foo.txt",
+ "Source/WebKit2/UIProcess/API/gtk/HelperClass.cpp",
+ "Source/WebKit2/UIProcess/API/gtk/HelperClass.h",
+ "Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp",
+ "Source/WebKit2/UIProcess/API/gtk/WebKitWebViewPrivate.h",
+ ]
+
+ for path in paths_not_to_skip:
+ self.assertFalse(self._dispatcher.should_skip_with_warning(path))
+
def _assert_should_skip_without_warning(self, path, is_checker_none,
expected):
# Check the file type before asserting the return value.
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py (96865 => 96866)
--- trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py 2011-10-06 22:43:40 UTC (rev 96865)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py 2011-10-06 22:44:45 UTC (rev 96866)
@@ -3056,6 +3056,7 @@
if not file_state.is_objective_c() and modified_identifier.find('_') >= 0:
# Various exceptions to the rule: _javascript_ op codes functions, const_iterator.
if (not (filename.find('_javascript_Core') >= 0 and modified_identifier.find('op_') >= 0)
+ and not (filename.find('gtk') >= 0 and modified_identifier.startswith('webkit_') >= 0)
and not modified_identifier.startswith('tst_')
and not modified_identifier.startswith('webkit_dom_object_')
and not modified_identifier.startswith('NPN_')
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py (96865 => 96866)
--- trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py 2011-10-06 22:43:40 UTC (rev 96865)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py 2011-10-06 22:44:45 UTC (rev 96866)
@@ -4399,6 +4399,19 @@
self.assert_lint('void webkit_dom_object_init();', '')
self.assert_lint('void webkit_dom_object_class_init();', '')
+ # There is an exception for GTK+ API.
+ self.assert_lint('void webkit_web_view_load(int var1, int var2)', '', 'Source/Webkit/gtk/webkit/foo.cpp')
+ self.assert_lint('void webkit_web_view_load(int var1, int var2)', '', 'Source/Webkit2/UIProcess/gtk/foo.cpp')
+
+ # Test that this doesn't also apply to files not in a 'gtk' directory.
+ self.assert_lint('void webkit_web_view_load(int var1, int var2)',
+ 'webkit_web_view_load is incorrectly named. Don\'t use underscores in your identifier names.'
+ ' [readability/naming] [4]', 'Source/Webkit/webkit/foo.cpp')
+ # Test that this doesn't also apply to names that don't start with 'webkit_'.
+ self.assert_lint_one_of_many_errors_re('void otherkit_web_view_load(int var1, int var2)',
+ 'otherkit_web_view_load is incorrectly named. Don\'t use underscores in your identifier names.'
+ ' [readability/naming] [4]', 'Source/Webkit/webkit/foo.cpp')
+
# There is an exception for some unit tests that begin with "tst_".
self.assert_lint('void tst_QWebFrame::arrayObjectEnumerable(int var1, int var2)', '')
