[SCM] WebKit Debian packaging branch, webkit-1.1, updated. upstream/1.1.19-706-ge5415e9
cjerdonek at webkit.org
cjerdonek at webkit.org
Thu Feb 4 21:35:04 UTC 2010
The following commit has been merged in the webkit-1.1 branch:
commit a4404ffd4127f72554ff4be7cc50fa41a502a7b0
Author: cjerdonek at webkit.org <cjerdonek at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Mon Feb 1 07:05:45 2010 +0000
2010-01-31 Chris Jerdonek <cjerdonek at webkit.org>
Reviewed by Shinichiro Hamaji.
Moved style error handler code to their own classes, and
related refactoring. Increased unit test code coverage of
style error handling.
https://bugs.webkit.org/show_bug.cgi?id=34379
* Scripts/check-webkit-style:
- Minor change: added error_count variable.
* Scripts/webkitpy/style/checker.py:
- Renamed ProcessorOptions.should_report_error() to is_reportable().
- In the StyleChecker class--
- Removed _default_style_error_handler().
- Added _increment_error_count().
- Refactored to use DefaultStyleErrorHandler and
PatchStyleErrorHandler constructors.
* Scripts/webkitpy/style/checker_unittest.py:
- In the StyleStyleCheckerTest class--
- Removed write_sample_error().
- Removed test_default_style_error_handler().
* Scripts/webkitpy/style/error_handlers.py: Added.
- Added DefaultStyleErrorHandler class.
- Added PatchStyleErrorHandler class.
* Scripts/webkitpy/style/error_handlers_unittest.py: Added.
- Added unit tests for DefaultStyleErrorHandler and
PatchStyleErrorHandler.
* Scripts/webkitpy/style/unittests.py:
- Added error_handlers unit tests.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@54126 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index cba480d..38cd459 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,40 @@
+2010-01-31 Chris Jerdonek <cjerdonek at webkit.org>
+
+ Reviewed by Shinichiro Hamaji.
+
+ Moved style error handler code to their own classes, and
+ related refactoring. Increased unit test code coverage of
+ style error handling.
+
+ https://bugs.webkit.org/show_bug.cgi?id=34379
+
+ * Scripts/check-webkit-style:
+ - Minor change: added error_count variable.
+
+ * Scripts/webkitpy/style/checker.py:
+ - Renamed ProcessorOptions.should_report_error() to is_reportable().
+ - In the StyleChecker class--
+ - Removed _default_style_error_handler().
+ - Added _increment_error_count().
+ - Refactored to use DefaultStyleErrorHandler and
+ PatchStyleErrorHandler constructors.
+
+ * Scripts/webkitpy/style/checker_unittest.py:
+ - In the StyleStyleCheckerTest class--
+ - Removed write_sample_error().
+ - Removed test_default_style_error_handler().
+
+ * Scripts/webkitpy/style/error_handlers.py: Added.
+ - Added DefaultStyleErrorHandler class.
+ - Added PatchStyleErrorHandler class.
+
+ * Scripts/webkitpy/style/error_handlers_unittest.py: Added.
+ - Added unit tests for DefaultStyleErrorHandler and
+ PatchStyleErrorHandler.
+
+ * Scripts/webkitpy/style/unittests.py:
+ - Added error_handlers unit tests.
+
2010-01-29 Mark Rowe <mrowe at apple.com>
Rubber-stamped by Stephanie Lewis.
diff --git a/WebKitTools/Scripts/check-webkit-style b/WebKitTools/Scripts/check-webkit-style
index 245d834..501264b 100755
--- a/WebKitTools/Scripts/check-webkit-style
+++ b/WebKitTools/Scripts/check-webkit-style
@@ -86,8 +86,9 @@ def main():
patch = scm.create_patch()
style_checker.check_patch(patch)
- sys.stderr.write('Total errors found: %d\n' % style_checker.error_count)
- sys.exit(style_checker.error_count > 0)
+ error_count = style_checker.error_count
+ sys.stderr.write('Total errors found: %d\n' % error_count)
+ sys.exit(error_count > 0)
if __name__ == "__main__":
diff --git a/WebKitTools/Scripts/webkitpy/style/checker.py b/WebKitTools/Scripts/webkitpy/style/checker.py
index 6a07ada..faf954f 100644
--- a/WebKitTools/Scripts/webkitpy/style/checker.py
+++ b/WebKitTools/Scripts/webkitpy/style/checker.py
@@ -35,6 +35,8 @@ import os.path
import sys
from .. style_references import parse_patch
+from error_handlers import DefaultStyleErrorHandler
+from error_handlers import PatchStyleErrorHandler
from processors.cpp import CppProcessor
from processors.text import TextProcessor
@@ -332,10 +334,10 @@ class ProcessorOptions(object):
# Python does not automatically deduce from __eq__().
return not (self == other)
- def should_report_error(self, category, confidence_in_error):
- """Return whether an error should be reported.
+ def is_reportable(self, category, confidence_in_error):
+ """Return whether an error is reportable.
- An error should be reported if the confidence in the error
+ An error is reportable if the confidence in the error
is at least the current verbosity level, and if the current
filter says that the category should be checked.
@@ -701,49 +703,9 @@ class StyleChecker(object):
self.error_count = 0
self.options = options
- def _default_style_error_handler(self, file_path):
- """Return a default style error handler for the given path.
-
- Args:
- file_path: The path to the file containing the error. This
- is meant for reporting to the user.
-
- """
- def handle_style_error(line_number, category, confidence, message):
- """Handle the occurrence of a style error.
-
- Check whether an error should be reported. If so, increment the
- global error count and report the error details.
-
- Args:
- line_number: The number of the line containing the error.
- category: A string used to describe the "category" this bug
- falls under: "whitespace", say, or "runtime".
- Categories may have a hierarchy separated by slashes:
- "whitespace/indent".
- confidence: A number from 1-5 representing a confidence score
- for the error, with 5 meaning that we are certain
- of the problem, and 1 meaning that it could be a
- legitimate construct.
- message: The error message.
-
- """
- if not self.options.should_report_error(category, confidence):
- return
-
- self.error_count += 1
-
- if self.options.output_format == 'vs7':
- format_string = "%s(%s): %s [%s] [%d]\n"
- else:
- format_string = "%s:%s: %s [%s] [%d]\n"
-
- self._stderr_write(format_string % (file_path,
- line_number,
- message,
- category,
- confidence))
- return handle_style_error
+ def _increment_error_count(self):
+ """Increment the total count of reported errors."""
+ self.error_count += 1
def _process_file(self, processor, file_path, handle_style_error):
"""Process the file using the given processor."""
@@ -795,16 +757,18 @@ class StyleChecker(object):
file_path: A string that is the path of the file to process.
handle_style_error: The function to call when a style error
occurs. This parameter is meant for internal
- use within this class. Defaults to the
- style error handling method of this class.
+ use within this class. Defaults to a
+ DefaultStyleErrorHandler instance.
process_file: The function to call to process the file. This
parameter should be used only for unit tests.
Defaults to the file processing method of this class.
"""
if handle_style_error is None:
- handle_style_error = self._default_style_error_handler(file_path)
-
+ handle_style_error = DefaultStyleErrorHandler(file_path,
+ self.options,
+ self._increment_error_count,
+ self._stderr_write)
if process_file is None:
process_file = self._process_file
@@ -835,29 +799,11 @@ class StyleChecker(object):
"""
patch_files = parse_patch(patch_string)
for file_path, diff in patch_files.iteritems():
- line_numbers = set()
-
- handle_style_error = self._default_style_error_handler(file_path)
-
- def handle_patch_style_error(line_number, category, confidence,
- message):
- """Wrapper function of cpp_style.error for patches.
-
- This function outputs errors only if the line number
- corresponds to lines which are modified or added.
-
- """
- if not line_numbers:
- for line in diff.lines:
- # When deleted line is not set, it means that
- # the line is newly added.
- if not line[0]:
- line_numbers.add(line[1])
-
- # FIXME: Make sure errors having line number zero are
- # logged -- like carriage-return errors.
- if line_number in line_numbers:
- handle_style_error(line_number, category, confidence, message)
+ style_error_handler = PatchStyleErrorHandler(diff,
+ file_path,
+ self.options,
+ self._increment_error_count,
+ self._stderr_write)
- self.check_file(file_path, handle_patch_style_error)
+ self.check_file(file_path, style_error_handler)
diff --git a/WebKitTools/Scripts/webkitpy/style/checker_unittest.py b/WebKitTools/Scripts/webkitpy/style/checker_unittest.py
index 8aabeff..4d6b2e7 100755
--- a/WebKitTools/Scripts/webkitpy/style/checker_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/style/checker_unittest.py
@@ -159,18 +159,18 @@ class ProcessorOptionsTest(unittest.TestCase):
# code defines __ne__.
self.assertFalse(ProcessorOptions() != ProcessorOptions())
- def test_should_report_error(self):
- """Test should_report_error()."""
+ def test_is_reportable(self):
+ """Test is_reportable()."""
filter = CategoryFilter(["-xyz"])
options = ProcessorOptions(filter=filter, verbosity=3)
# Test verbosity
- self.assertTrue(options.should_report_error("abc", 3))
- self.assertFalse(options.should_report_error("abc", 2))
+ self.assertTrue(options.is_reportable("abc", 3))
+ self.assertFalse(options.is_reportable("abc", 2))
# Test filter
- self.assertTrue(options.should_report_error("xy", 3))
- self.assertFalse(options.should_report_error("xyz", 3))
+ self.assertTrue(options.is_reportable("xy", 3))
+ self.assertFalse(options.is_reportable("xyz", 3))
class WebKitArgumentDefaultsTest(unittest.TestCase):
@@ -526,66 +526,20 @@ class StyleCheckerTest(unittest.TestCase):
"""
- def setUp(self):
- self.error_messages = ""
-
- def mock_stderr_write(self, error_message):
- """A mock sys.stderr.write."""
- self.error_messages = error_message
- pass
-
- def mock_handle_style_error(self):
+ def _mock_stderr_write(self, message):
pass
- def style_checker(self, options):
- return StyleChecker(options, self.mock_stderr_write)
+ def _style_checker(self, options):
+ return StyleChecker(options, self._mock_stderr_write)
def test_init(self):
"""Test __init__ constructor."""
options = ProcessorOptions()
- style_checker = self.style_checker(options)
+ style_checker = self._style_checker(options)
self.assertEquals(style_checker.error_count, 0)
self.assertEquals(style_checker.options, options)
- def write_sample_error(self, style_checker, error_confidence):
- """Write an error to the given style checker."""
- handle_style_error = (
- style_checker._default_style_error_handler("filename"))
-
- handle_style_error(line_number=1,
- category="category",
- confidence=error_confidence,
- message="message")
-
- def test_default_style_error_handler(self):
- """Test _handle_style_error() function."""
- options = ProcessorOptions(output_format="emacs",
- verbosity=3)
- style_checker = self.style_checker(options)
-
- # Verify initialized properly.
- self.assertEquals(style_checker.error_count, 0)
- self.assertEquals(self.error_messages, "")
-
- # Check that should_print_error is getting called appropriately.
- self.write_sample_error(style_checker, 2)
- self.assertEquals(style_checker.error_count, 0) # Error confidence too low.
- self.assertEquals(self.error_messages, "")
-
- self.write_sample_error(style_checker, 3)
- self.assertEquals(style_checker.error_count, 1) # Error confidence just high enough.
- self.assertEquals(self.error_messages, "filename:1: message [category] [3]\n")
-
- # Clear previous errors.
- self.error_messages = ""
-
- # Check "vs7" output format.
- style_checker.options.output_format = "vs7"
- self.write_sample_error(style_checker, 3)
- self.assertEquals(style_checker.error_count, 2) # Error confidence just high enough.
- self.assertEquals(self.error_messages, "filename(1): message [category] [3]\n")
-
class StyleCheckerCheckFileTest(unittest.TestCase):
diff --git a/WebKitTools/Scripts/webkitpy/style/error_handlers.py b/WebKitTools/Scripts/webkitpy/style/error_handlers.py
new file mode 100644
index 0000000..54b1d76
--- /dev/null
+++ b/WebKitTools/Scripts/webkitpy/style/error_handlers.py
@@ -0,0 +1,154 @@
+# Copyright (C) 2010 Chris Jerdonek (cjerdonek at webkit.org)
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+# 1. Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+# 2. Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+#
+# THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' AND ANY
+# EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+# DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS BE LIABLE FOR ANY
+# DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+# ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+"""Defines style error handler classes.
+
+A style error handler is a function to call when a style error is
+found. Style error handlers can also have state. A class that represents
+a style error handler should implement the following methods.
+
+Methods:
+
+ __call__(self, line_number, category, confidence, message):
+
+ Handle the occurrence of a style error.
+
+ Check whether the error is reportable. If so, report the details.
+
+ Args:
+ line_number: The integer line number of the line containing the error.
+ category: The name of the category of the error, for example
+ "whitespace/newline".
+ confidence: An integer between 1-5 that represents the level of
+ confidence in the error. The value 5 means that we are
+ certain of the problem, and the value 1 means that it
+ could be a legitimate construct.
+ message: The error message to report.
+
+"""
+
+
+import sys
+
+
+class DefaultStyleErrorHandler(object):
+
+ """The default style error handler."""
+
+ def __init__(self, file_path, options, increment_error_count,
+ stderr_write=None):
+ """Create a default style error handler.
+
+ Args:
+ file_path: The path to the file containing the error. This
+ is used for reporting to the user.
+ options: A ProcessorOptions instance.
+ increment_error_count: A function that takes no arguments and
+ increments the total count of reportable
+ errors.
+ stderr_write: A function that takes a string as a parameter
+ and that is called when a style error occurs.
+ Defaults to sys.stderr.write. This should be
+ used only for unit tests.
+
+ """
+ if stderr_write is None:
+ stderr_write = sys.stderr.write
+
+ self._file_path = file_path
+ self._increment_error_count = increment_error_count
+ self._options = options
+ self._stderr_write = stderr_write
+
+ def __call__(self, line_number, category, confidence, message):
+ """Handle the occurrence of a style error.
+
+ See the docstring of this module for more information.
+
+ """
+ if not self._options.is_reportable(category, confidence):
+ return
+
+ self._increment_error_count()
+
+ if self._options.output_format == 'vs7':
+ format_string = "%s(%s): %s [%s] [%d]\n"
+ else:
+ format_string = "%s:%s: %s [%s] [%d]\n"
+
+ self._stderr_write(format_string % (self._file_path,
+ line_number,
+ message,
+ category,
+ confidence))
+
+
+class PatchStyleErrorHandler(object):
+
+ """The style error function for patch files."""
+
+ def __init__(self, diff, file_path, options, increment_error_count,
+ stderr_write):
+ """Create a patch style error handler for the given path.
+
+ Args:
+ diff: A DiffFile instance.
+ Other arguments: see the DefaultStyleErrorHandler.__init__()
+ documentation for the other arguments.
+
+ """
+ self._diff = diff
+ self._default_error_handler = DefaultStyleErrorHandler(file_path,
+ options,
+ increment_error_count,
+ stderr_write)
+
+ # The line numbers of the modified lines. This is set lazily.
+ self._line_numbers = set()
+
+ def _get_line_numbers(self):
+ """Return the line numbers of the modified lines."""
+ if not self._line_numbers:
+ for line in self._diff.lines:
+ # When deleted line is not set, it means that
+ # the line is newly added.
+ if not line[0]:
+ self._line_numbers.add(line[1])
+
+ return self._line_numbers
+
+ def __call__(self, line_number, category, confidence, message):
+ """Handle the occurrence of a style error.
+
+ This function does not report errors occurring in lines not
+ modified or added.
+
+ Args: see the DefaultStyleErrorHandler.__call__() documentation.
+
+ """
+ if line_number not in self._get_line_numbers():
+ # Then the error is not reportable.
+ return
+
+ self._default_error_handler(line_number, category, confidence,
+ message)
+
diff --git a/WebKitTools/Scripts/webkitpy/style/error_handlers_unittest.py b/WebKitTools/Scripts/webkitpy/style/error_handlers_unittest.py
new file mode 100644
index 0000000..6a91ff2
--- /dev/null
+++ b/WebKitTools/Scripts/webkitpy/style/error_handlers_unittest.py
@@ -0,0 +1,163 @@
+# Copyright (C) 2010 Chris Jerdonek (cjerdonek at webkit.org)
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+# 1. Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+# 2. Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+#
+# THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' AND ANY
+# EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+# DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS BE LIABLE FOR ANY
+# DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+# ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+"""Unit tests for error_handlers.py."""
+
+
+import unittest
+
+from .. style_references import parse_patch
+from checker import ProcessorOptions
+from error_handlers import DefaultStyleErrorHandler
+from error_handlers import PatchStyleErrorHandler
+
+
+class StyleErrorHandlerTestBase(unittest.TestCase):
+
+ def setUp(self):
+ self._error_messages = ""
+ self._error_count = 0
+
+ def _mock_increment_error_count(self):
+ self._error_count += 1
+
+ def _mock_stderr_write(self, message):
+ self._error_messages += message
+
+
+class DefaultStyleErrorHandlerTest(StyleErrorHandlerTestBase):
+
+ """Tests DefaultStyleErrorHandler class."""
+
+ _category = "whitespace/tab"
+
+ def _options(self, output_format):
+ return ProcessorOptions(verbosity=3, output_format=output_format)
+
+ def _error_handler(self, options):
+ file_path = "foo.h"
+ return DefaultStyleErrorHandler(file_path,
+ options,
+ self._mock_increment_error_count,
+ self._mock_stderr_write)
+
+ def _prepare_call(self, output_format="emacs"):
+ """Return options after initializing."""
+ options = self._options(output_format)
+
+ # Test that count is initialized to zero.
+ self.assertEquals(0, self._error_count)
+ self.assertEquals("", self._error_messages)
+
+ return options
+
+ def _call_error_handler(self, options, confidence):
+ """Handle an error with given confidence."""
+ handle_error = self._error_handler(options)
+
+ line_number = 100
+ message = "message"
+
+ handle_error(line_number, self._category, confidence, message)
+
+ def test_call_non_reportable(self):
+ """Test __call__() method with a non-reportable error."""
+ confidence = 1
+ options = self._prepare_call()
+
+ # Confirm the error is not reportable.
+ self.assertFalse(options.is_reportable(self._category, confidence))
+
+ self._call_error_handler(options, confidence)
+
+ self.assertEquals(0, self._error_count)
+ self.assertEquals("", self._error_messages)
+
+ def test_call_reportable_emacs(self):
+ """Test __call__() method with a reportable error and emacs format."""
+ confidence = 5
+ options = self._prepare_call("emacs")
+
+ self._call_error_handler(options, confidence)
+
+ self.assertEquals(1, self._error_count)
+ self.assertEquals(self._error_messages,
+ "foo.h:100: message [whitespace/tab] [5]\n")
+
+ def test_call_reportable_vs7(self):
+ """Test __call__() method with a reportable error and vs7 format."""
+ confidence = 5
+ options = self._prepare_call("vs7")
+
+ self._call_error_handler(options, confidence)
+
+ self.assertEquals(1, self._error_count)
+ self.assertEquals(self._error_messages,
+ "foo.h(100): message [whitespace/tab] [5]\n")
+
+
+class PatchStyleErrorHandlerTest(StyleErrorHandlerTestBase):
+
+ """Tests PatchStyleErrorHandler class."""
+
+ file_path = "__init__.py"
+
+ patch_string = """diff --git a/__init__.py b/__init__.py
+index ef65bee..e3db70e 100644
+--- a/__init__.py
++++ b/__init__.py
+@@ -1 +1,2 @@
+ # Required for Python to search this directory for module files
++# New line
+
+"""
+
+ def test_call(self):
+ patch_files = parse_patch(self.patch_string)
+ diff = patch_files[self.file_path]
+
+ options = ProcessorOptions(verbosity=3)
+
+ handle_error = PatchStyleErrorHandler(diff,
+ self.file_path,
+ options,
+ self._mock_increment_error_count,
+ self._mock_stderr_write)
+
+ category = "whitespace/tab"
+ confidence = 5
+ message = "message"
+
+ # Confirm error is reportable.
+ self.assertTrue(options.is_reportable(category, confidence))
+
+ # Confirm error count initialized to zero.
+ self.assertEquals(0, self._error_count)
+
+ # Test error in unmodified line (error count does not increment).
+ handle_error(1, category, confidence, message)
+ self.assertEquals(0, self._error_count)
+
+ # Test error in modified line (error count increments).
+ handle_error(2, category, confidence, message)
+ self.assertEquals(1, self._error_count)
+
diff --git a/WebKitTools/Scripts/webkitpy/style/unittests.py b/WebKitTools/Scripts/webkitpy/style/unittests.py
index aa54f03..11c10e7 100644
--- a/WebKitTools/Scripts/webkitpy/style/unittests.py
+++ b/WebKitTools/Scripts/webkitpy/style/unittests.py
@@ -36,5 +36,6 @@ import sys
import unittest
from checker_unittest import *
+from error_handlers_unittest import *
from processors.cpp_unittest import *
from processors.text_unittest import *
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list