Diff
Modified: trunk/Tools/ChangeLog (111863 => 111864)
--- trunk/Tools/ChangeLog 2012-03-23 15:04:58 UTC (rev 111863)
+++ trunk/Tools/ChangeLog 2012-03-23 15:43:39 UTC (rev 111864)
@@ -1,3 +1,48 @@
+2012-03-22 Martin Robinson <[email protected]>
+
+ [check-webkit-style] Alphabetical sorting errors in headers are reported for the line after the first out of order header
+ https://bugs.webkit.org/show_bug.cgi?id=81986
+
+ Reviewed by David Levin.
+
+ Track whether or not errors are filtered in error handlers. If an
+ alphabetical sorting error is reported for a header and filtered, try
+ reporting it for the other potential source of the error. This ensures
+ that the style bot can still find new sorting errors in both potential
+ situations:
+
+ #include <foo.h> <-- 1. edited line
+ #include <baz.h> <-- 2. edited line
+
+ * Scripts/webkitpy/style/checkers/cpp.py:
+ (check_include_line): Potentially report the error for both lines,
+ if the first error is filtered.
+ * Scripts/webkitpy/style/checkers/cpp_unittest.py: Implement a new style of
+ test that allows only reporting errors for certain lines. This verifies that
+ when one line of an ordering error is filtered, the error is reported on the
+ other line.
+ (ErrorCollector.__init__):
+ (ErrorCollector.__call__):
+ (CppStyleTestBase.perform_lint):
+ (CppStyleTestBase.perform_language_rules_check):
+ (CppStyleTestBase.assert_language_rules_check):
+ (OrderOfIncludesTest.test_check_alphabetical_include_order_errors_reported_for_both_lines):
+ Added a new test that verifies that when one line of the two lines of an ordering
+ error occur, the error is reported on the other line.
+ * Scripts/webkitpy/style/checkers/jsonchecker_unittest.py:
+ (MockErrorHandler.__call__): Report True because the error is handled.
+ * Scripts/webkitpy/style/checkers/test_expectations_unittest.py:
+ (ErrorCollector.__call__): Ditto.
+ * Scripts/webkitpy/style/checkers/watchlist_unittest.py:
+ (MockErrorHandler.__call__): Ditto.
+ * Scripts/webkitpy/style/checkers/xcodeproj_unittest.py:
+ (TestErrorHandler.__call__): Ditto.
+ * Scripts/webkitpy/style/checkers/xml_unittest.py:
+ (MockErrorHandler.__call__): Ditto.
+ * Scripts/webkitpy/style/error_handlers.py:
+ (DefaultStyleErrorHandler.__call__): Report True if the error is handled
+ and False if it is filtered.
+
2012-03-23 Patrick Gansterer <[email protected]>
Build fix for WinCE after r111778.
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py (111863 => 111864)
--- trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py 2012-03-23 15:04:58 UTC (rev 111863)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py 2012-03-23 15:43:39 UTC (rev 111864)
@@ -2776,8 +2776,11 @@
if previous_match:
previous_header_type = include_state.header_types[previous_line_number]
if previous_header_type == _OTHER_HEADER and previous_line.strip() > line.strip():
- error(line_number, 'build/include_order', 4,
- 'Alphabetical sorting problem.')
+ # This type of error is potentially a problem with this line or the previous one,
+ # so if the error is filtered for one line, report it for the next. This is so that
+ # we properly handle patches, for which only modified lines produce errors.
+ if not error(line_number - 1, 'build/include_order', 4, 'Alphabetical sorting problem.'):
+ error(line_number, 'build/include_order', 4, 'Alphabetical sorting problem.')
if error_message:
if file_extension == 'h':
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py (111863 => 111864)
--- trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py 2012-03-23 15:04:58 UTC (rev 111863)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py 2012-03-23 15:43:39 UTC (rev 111864)
@@ -53,22 +53,28 @@
# This is a list including all categories seen in any unit test.
_seen_style_categories = {}
- def __init__(self, assert_fn, filter=None):
+ def __init__(self, assert_fn, filter=None, lines_to_check=None):
"""assert_fn: a function to call when we notice a problem.
filter: filters the errors that we are concerned about."""
self._assert_fn = assert_fn
self._errors = []
+ self._lines_to_check = lines_to_check
if not filter:
filter = FilterConfiguration()
self._filter = filter
- def __call__(self, unused_linenum, category, confidence, message):
+ def __call__(self, line_number, category, confidence, message):
self._assert_fn(category in self._all_style_categories,
'Message "%s" has category "%s",'
' which is not in STYLE_CATEGORIES' % (message, category))
+
+ if self._lines_to_check and not line_number in self._lines_to_check:
+ return False
+
if self._filter.should_check(category, ""):
self._seen_style_categories[category] = 1
self._errors.append('%s [%s] [%d]' % (message, category, confidence))
+ return True
def results(self):
if len(self._errors) < 2:
@@ -246,8 +252,8 @@
return cpp_style.process_file_data(filename, file_extension, lines,
error, self.min_confidence, unit_test_config)
- def perform_lint(self, code, filename, basic_error_rules, unit_test_config={}):
- error_collector = ErrorCollector(self.assert_, FilterConfiguration(basic_error_rules))
+ def perform_lint(self, code, filename, basic_error_rules, unit_test_config={}, lines_to_check=None):
+ error_collector = ErrorCollector(self.assert_, FilterConfiguration(basic_error_rules), lines_to_check)
lines = code.split('\n')
extension = filename.split('.')[1]
self.process_file_data(filename, extension, lines, error_collector, unit_test_config)
@@ -272,13 +278,13 @@
return self.perform_lint(code, 'test.' + file_extension, basic_error_rules)
# Only keep some errors related to includes, namespaces and rtti.
- def perform_language_rules_check(self, filename, code):
+ def perform_language_rules_check(self, filename, code, lines_to_check=None):
basic_error_rules = ('-',
'+build/include',
'+build/include_order',
'+build/namespaces',
'+runtime/rtti')
- return self.perform_lint(code, filename, basic_error_rules)
+ return self.perform_lint(code, filename, basic_error_rules, lines_to_check=lines_to_check)
# Only keep function length errors.
def perform_function_lengths_check(self, code):
@@ -327,9 +333,9 @@
if not re.search(expected_message_re, message):
self.fail('Message was:\n' + message + 'Expected match to "' + expected_message_re + '"')
- def assert_language_rules_check(self, file_name, code, expected_message):
+ def assert_language_rules_check(self, file_name, code, expected_message, lines_to_check=None):
self.assertEquals(expected_message,
- self.perform_language_rules_check(file_name, code))
+ self.perform_language_rules_check(file_name, code, lines_to_check))
def assert_include_what_you_use(self, code, expected_message):
self.assertEquals(expected_message,
@@ -2568,6 +2574,30 @@
'#include <assert.h>\n',
'')
+ def test_check_alphabetical_include_order_errors_reported_for_both_lines(self):
+ # If one of the two lines of out of order headers are filtered, the error should be
+ # reported on the other line.
+ self.assert_language_rules_check('foo.h',
+ '#include "a.h"\n'
+ '#include "c.h"\n'
+ '#include "b.h"\n',
+ 'Alphabetical sorting problem. [build/include_order] [4]',
+ lines_to_check=[2])
+
+ self.assert_language_rules_check('foo.h',
+ '#include "a.h"\n'
+ '#include "c.h"\n'
+ '#include "b.h"\n',
+ 'Alphabetical sorting problem. [build/include_order] [4]',
+ lines_to_check=[3])
+
+ # If no lines are filtered, the error should be reported only once.
+ self.assert_language_rules_check('foo.h',
+ '#include "a.h"\n'
+ '#include "c.h"\n'
+ '#include "b.h"\n',
+ 'Alphabetical sorting problem. [build/include_order] [4]')
+
def test_check_line_break_after_own_header(self):
self.assert_language_rules_check('foo.cpp',
'#include "config.h"\n'
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/jsonchecker_unittest.py (111863 => 111864)
--- trunk/Tools/Scripts/webkitpy/style/checkers/jsonchecker_unittest.py 2012-03-23 15:04:58 UTC (rev 111863)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/jsonchecker_unittest.py 2012-03-23 15:43:39 UTC (rev 111864)
@@ -39,6 +39,7 @@
def __call__(self, line_number, category, confidence, message):
self._handle_style_error(self, line_number, category, confidence, message)
+ return True
class JSONCheckerTest(unittest.TestCase):
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py (111863 => 111864)
--- trunk/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py 2012-03-23 15:04:58 UTC (rev 111863)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py 2012-03-23 15:43:39 UTC (rev 111864)
@@ -47,6 +47,7 @@
def __call__(self, lineno, category, confidence, message):
self._errors.append('%s [%s] [%d]' % (message, category, confidence))
+ return True
def get_errors(self):
return ''.join(self._errors)
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/watchlist_unittest.py (111863 => 111864)
--- trunk/Tools/Scripts/webkitpy/style/checkers/watchlist_unittest.py 2012-03-23 15:04:58 UTC (rev 111863)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/watchlist_unittest.py 2012-03-23 15:43:39 UTC (rev 111864)
@@ -49,6 +49,7 @@
def __call__(self, line_number, category, confidence, message):
self._handle_style_error(self, line_number, category, confidence, message)
+ return True
class WatchListTest(unittest.TestCase):
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/xcodeproj_unittest.py (111863 => 111864)
--- trunk/Tools/Scripts/webkitpy/style/checkers/xcodeproj_unittest.py 2012-03-23 15:04:58 UTC (rev 111863)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/xcodeproj_unittest.py 2012-03-23 15:43:39 UTC (rev 111864)
@@ -39,6 +39,7 @@
def __call__(self, line_number, category, confidence, message):
self.handler(self, line_number, category, confidence, message)
+ return True
class XcodeProjectFileCheckerTest(unittest.TestCase):
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/xml_unittest.py (111863 => 111864)
--- trunk/Tools/Scripts/webkitpy/style/checkers/xml_unittest.py 2012-03-23 15:04:58 UTC (rev 111863)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/xml_unittest.py 2012-03-23 15:43:39 UTC (rev 111864)
@@ -39,6 +39,7 @@
def __call__(self, line_number, category, confidence, message):
self._handle_style_error(self, line_number, category, confidence, message)
+ return True
class XMLCheckerTest(unittest.TestCase):
Modified: trunk/Tools/Scripts/webkitpy/style/error_handlers.py (111863 => 111864)
--- trunk/Tools/Scripts/webkitpy/style/error_handlers.py 2012-03-23 15:04:58 UTC (rev 111863)
+++ trunk/Tools/Scripts/webkitpy/style/error_handlers.py 2012-03-23 15:43:39 UTC (rev 111864)
@@ -138,12 +138,12 @@
"""
if not self.should_line_be_checked(line_number):
- return
+ return False
if not self._configuration.is_reportable(category=category,
confidence_in_error=confidence,
file_path=self._file_path):
- return
+ return False
category_total = self._add_reportable_error(category)
@@ -151,14 +151,14 @@
if (max_reports is not None) and (category_total > max_reports):
# Then suppress displaying the error.
- return
+ return False
self._configuration.write_style_error(category=category,
confidence_in_error=confidence,
file_path=self._file_path,
line_number=line_number,
message=message)
-
if category_total == max_reports:
self._configuration.stderr_write("Suppressing further [%s] reports "
"for this file.\n" % category)
+ return True
