[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373
eric at webkit.org
eric at webkit.org
Thu Apr 8 01:11:24 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit b8783ccde1386c7415cee1d5a7c705a57798aba5
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Mon Jan 18 00:04:35 2010 +0000
2010-01-17 Chris Jerdonek <cjerdonek at webkit.org>
Reviewed by Adam Barth.
Eliminated the error_count global variable and related
check-webkit-style refactoring.
https://bugs.webkit.org/show_bug.cgi?id=33678
* Scripts/check-webkit-style:
- Updated to use webkit_argument_defaults().
- Renamed styleChecker to style_checker.
* Scripts/webkitpy/style/checker.py:
- Prefixed the three default arguments with WEBKIT_DEFAULT.
- Added webkit_argument_defaults().
- Added default filter_rules parameter to CategoryFilter constructor.
- Added __ne__() to CategoryFilter class.
- Added __eq__() and __ne__() to ProcessorOptions class.
- Added error_count and _write_error attributes to StyleChecker class.
- Made StyleChecker._handle_error() increment the error count.
* Scripts/webkitpy/style/checker_unittest.py:
- Improved CategoryFilterTest.test_eq().
- Added CategoryFilterTest.test_ne().
- Added test_eq() and test_ne() to ProcessorOptionsTest class.
- Updated unit tests to use webkit_argument_defaults().
- Added StyleCheckerTest class.
* Scripts/webkitpy/style/cpp_style.py:
- Removed references to global error_count.
* Scripts/webkitpy/style/cpp_style_unittest.py:
- Removed CppStyleStateTest class.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@53374 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 61f5c96..9bae1f9 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,38 @@
+2010-01-17 Chris Jerdonek <cjerdonek at webkit.org>
+
+ Reviewed by Adam Barth.
+
+ Eliminated the error_count global variable and related
+ check-webkit-style refactoring.
+
+ https://bugs.webkit.org/show_bug.cgi?id=33678
+
+ * Scripts/check-webkit-style:
+ - Updated to use webkit_argument_defaults().
+ - Renamed styleChecker to style_checker.
+
+ * Scripts/webkitpy/style/checker.py:
+ - Prefixed the three default arguments with WEBKIT_DEFAULT.
+ - Added webkit_argument_defaults().
+ - Added default filter_rules parameter to CategoryFilter constructor.
+ - Added __ne__() to CategoryFilter class.
+ - Added __eq__() and __ne__() to ProcessorOptions class.
+ - Added error_count and _write_error attributes to StyleChecker class.
+ - Made StyleChecker._handle_error() increment the error count.
+
+ * Scripts/webkitpy/style/checker_unittest.py:
+ - Improved CategoryFilterTest.test_eq().
+ - Added CategoryFilterTest.test_ne().
+ - Added test_eq() and test_ne() to ProcessorOptionsTest class.
+ - Updated unit tests to use webkit_argument_defaults().
+ - Added StyleCheckerTest class.
+
+ * Scripts/webkitpy/style/cpp_style.py:
+ - Removed references to global error_count.
+
+ * Scripts/webkitpy/style/cpp_style_unittest.py:
+ - Removed CppStyleStateTest class.
+
2010-01-15 Jon Honeycutt <jhoneycutt at apple.com>
get_accParent should try to retrieve parent AccessibilityObject, before
diff --git a/WebKitTools/Scripts/check-webkit-style b/WebKitTools/Scripts/check-webkit-style
index a5ab450..1475e6a 100755
--- a/WebKitTools/Scripts/check-webkit-style
+++ b/WebKitTools/Scripts/check-webkit-style
@@ -59,18 +59,16 @@ def main():
codecs.getwriter('utf8'),
'replace')
- defaults = checker.ArgumentDefaults(checker.DEFAULT_OUTPUT_FORMAT,
- checker.DEFAULT_VERBOSITY,
- checker.WEBKIT_FILTER_RULES)
+ defaults = checker.webkit_argument_defaults()
parser = checker.ArgumentParser(defaults)
(files, options) = parser.parse(sys.argv[1:])
- styleChecker = checker.StyleChecker(options)
+ style_checker = checker.StyleChecker(options)
if files:
for filename in files:
- styleChecker.process_file(filename)
+ style_checker.process_file(filename)
else:
cwd = os.path.abspath('.')
@@ -86,10 +84,10 @@ def main():
patch = scm.create_patch_since_local_commit(commit)
else:
patch = scm.create_patch()
- styleChecker.process_patch(patch)
+ style_checker.process_patch(patch)
- sys.stderr.write('Total errors found: %d\n' % checker.error_count())
- sys.exit(checker.error_count() > 0)
+ sys.stderr.write('Total errors found: %d\n' % style_checker.error_count)
+ sys.exit(style_checker.error_count > 0)
if __name__ == "__main__":
diff --git a/WebKitTools/Scripts/webkitpy/style/checker.py b/WebKitTools/Scripts/webkitpy/style/checker.py
index a62d9a1..c46a545 100644
--- a/WebKitTools/Scripts/webkitpy/style/checker.py
+++ b/WebKitTools/Scripts/webkitpy/style/checker.py
@@ -41,8 +41,8 @@ from diff_parser import DiffParser
# These defaults are used by check-webkit-style.
-DEFAULT_VERBOSITY = 1
-DEFAULT_OUTPUT_FORMAT = 'emacs'
+WEBKIT_DEFAULT_VERBOSITY = 1
+WEBKIT_DEFAULT_OUTPUT_FORMAT = 'emacs'
# FIXME: For style categories we will never want to have, remove them.
@@ -57,7 +57,7 @@ DEFAULT_OUTPUT_FORMAT = 'emacs'
# The _WEBKIT_FILTER_RULES are prepended to any user-specified filter
# rules. Since by default all errors are on, only include rules that
# begin with a - sign.
-WEBKIT_FILTER_RULES = [
+WEBKIT_DEFAULT_FILTER_RULES = [
'-build/endif_comment',
'-build/include_what_you_use', # <string> for std::string
'-build/storage_class', # const static
@@ -158,6 +158,13 @@ STYLE_CATEGORIES = [
]
+def webkit_argument_defaults():
+ """Return the DefaultArguments instance for use by check-webkit-style."""
+ return ArgumentDefaults(WEBKIT_DEFAULT_OUTPUT_FORMAT,
+ WEBKIT_DEFAULT_VERBOSITY,
+ WEBKIT_DEFAULT_FILTER_RULES)
+
+
def _create_usage(defaults):
"""Return the usage string to display for command help.
@@ -234,7 +241,7 @@ class CategoryFilter(object):
"""Filters whether to check style categories."""
- def __init__(self, filter_rules):
+ def __init__(self, filter_rules=None):
"""Create a category filter.
This method performs argument validation but does not strip
@@ -245,12 +252,16 @@ class CategoryFilter(object):
are strings beginning with the plus or minus
symbol (+/-). The list should include any
default filter rules at the beginning.
+ Defaults to the empty list.
Raises:
ValueError: Invalid filter rule if a rule does not start with
plus ("+") or minus ("-").
"""
+ if filter_rules is None:
+ filter_rules = []
+
for rule in filter_rules:
if not (rule.startswith('+') or rule.startswith('-')):
raise ValueError('Invalid filter rule "%s": every rule '
@@ -263,11 +274,15 @@ class CategoryFilter(object):
def __str__(self):
return ",".join(self._filter_rules)
+ # Useful for unit testing.
def __eq__(self, other):
- # This is useful for unit testing.
- # Two category filters are the same if and only if their
- # constituent filter rules are the same.
- return (str(self) == str(other))
+ """Return whether this CategoryFilter instance is equal to another."""
+ return self._filter_rules == other._filter_rules
+
+ # Useful for unit testing.
+ def __ne__(self, other):
+ # Python does not automatically deduce from __eq__().
+ return not (self == other)
def should_check(self, category):
"""Return whether the category should be checked.
@@ -324,7 +339,7 @@ class ProcessorOptions(object):
def __init__(self, output_format="emacs", verbosity=1, filter=None,
git_commit=None, extra_flag_values=None):
if filter is None:
- filter = CategoryFilter([])
+ filter = CategoryFilter()
if extra_flag_values is None:
extra_flag_values = {}
@@ -344,6 +359,27 @@ class ProcessorOptions(object):
self.git_commit = git_commit
self.extra_flag_values = extra_flag_values
+ # Useful for unit testing.
+ def __eq__(self, other):
+ """Return whether this ProcessorOptions instance is equal to another."""
+ if self.output_format != other.output_format:
+ return False
+ if self.verbosity != other.verbosity:
+ return False
+ if self.filter != other.filter:
+ return False
+ if self.git_commit != other.git_commit:
+ return False
+ if self.extra_flag_values != other.extra_flag_values:
+ return False
+
+ return True
+
+ # Useful for unit testing.
+ def __ne__(self, other):
+ # 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.
@@ -585,26 +621,38 @@ class ArgumentParser(object):
class StyleChecker(object):
- """Supports checking style in files and patches."""
+ """Supports checking style in files and patches.
+
+ Attributes:
+ error_count: An integer that is the total number of reported
+ errors for the lifetime of this StyleChecker
+ instance.
+ options: A ProcessorOptions instance that controls the behavior
+ of style checking.
+
+ """
- def __init__(self, options):
+ def __init__(self, options, write_error=sys.stderr.write):
"""Create a StyleChecker instance.
Args:
- options: A ProcessorOptions instance.
+ options: See options attribute.
+ stderr_write: A function that takes a string as a parameter
+ and that is called when a style error occurs.
"""
+ self._write_error = write_error
self.options = options
+ self.error_count = 0
# FIXME: Eliminate the need to set global state here.
cpp_style._set_verbose_level(options.verbosity)
def _handle_error(self, filename, line_number, category, confidence, message):
- """Logs the fact we've found a lint error.
+ """Handle the occurrence of a style error while checking.
- We log the error location and our confidence in the error, i.e.
- how certain we are the error is a legitimate style regression
- versus a misidentification or justified use.
+ Check whether an error should be reported. If so, increment the
+ global error count and report the error details.
Args:
filename: The name of the file containing the error.
@@ -623,15 +671,15 @@ class StyleChecker(object):
if not self.options.should_report_error(category, confidence):
return
- # FIXME: Eliminate the need to reference cpp_style here.
- cpp_style._cpp_style_state.increment_error_count()
+ self.error_count += 1
if self.options.output_format == 'vs7':
- sys.stderr.write('%s(%s): %s [%s] [%d]\n' % (
- filename, line_number, message, category, confidence))
+ format_string = "%s(%s): %s [%s] [%d]\n"
else:
- sys.stderr.write('%s:%s: %s [%s] [%d]\n' % (
- filename, line_number, message, category, confidence))
+ format_string = "%s:%s: %s [%s] [%d]\n"
+
+ self._write_error(format_string % (filename, line_number, message,
+ category, confidence))
def process_file(self, filename):
"""Checks style for the specified file.
@@ -676,8 +724,3 @@ class StyleChecker(object):
cpp_style.process_file(filename, error_for_patch)
elif text_style.can_handle(filename):
text_style.process_file(filename, error_for_patch)
-
-
-def error_count():
- """Returns the total error count."""
- return cpp_style.error_count()
diff --git a/WebKitTools/Scripts/webkitpy/style/checker_unittest.py b/WebKitTools/Scripts/webkitpy/style/checker_unittest.py
index 11e0f52..5a6e650 100755
--- a/WebKitTools/Scripts/webkitpy/style/checker_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/style/checker_unittest.py
@@ -39,6 +39,7 @@ import unittest
import checker as style
from checker import CategoryFilter
from checker import ProcessorOptions
+from checker import StyleChecker
class CategoryFilterTest(unittest.TestCase):
@@ -47,7 +48,7 @@ class CategoryFilterTest(unittest.TestCase):
def test_init(self):
"""Test __init__ constructor."""
self.assertRaises(ValueError, CategoryFilter, ["no_prefix"])
- CategoryFilter([]) # No ValueError: works
+ CategoryFilter() # No ValueError: works
CategoryFilter(["+"]) # No ValueError: works
CategoryFilter(["-"]) # No ValueError: works
@@ -57,16 +58,26 @@ class CategoryFilterTest(unittest.TestCase):
self.assertEquals(str(filter), "+a,-b")
def test_eq(self):
- """Test __eq__ equality operator."""
+ """Test __eq__ equality function."""
filter1 = CategoryFilter(["+a", "+b"])
filter2 = CategoryFilter(["+a", "+b"])
filter3 = CategoryFilter(["+b", "+a"])
- self.assertEquals(filter1, filter2)
- self.assertNotEquals(filter1, filter3)
+
+ # == calls __eq__.
+ self.assertTrue(filter1 == filter2)
+ self.assertFalse(filter1 == filter3) # Cannot test with assertNotEqual.
+
+ def test_ne(self):
+ """Test __ne__ inequality function."""
+ # != calls __ne__.
+ # By default, __ne__ always returns true on different objects.
+ # Thus, just check the distinguishing case to verify that the
+ # code defines __ne__.
+ self.assertFalse(CategoryFilter() != CategoryFilter())
def test_should_check(self):
"""Test should_check() method."""
- filter = CategoryFilter([])
+ filter = CategoryFilter()
self.assertTrue(filter.should_check("everything"))
# Check a second time to exercise cache.
self.assertTrue(filter.should_check("everything"))
@@ -94,11 +105,12 @@ class ProcessorOptionsTest(unittest.TestCase):
# Check default parameters.
options = ProcessorOptions()
self.assertEquals(options.extra_flag_values, {})
- self.assertEquals(options.filter, CategoryFilter([]))
+ self.assertEquals(options.filter, CategoryFilter())
self.assertEquals(options.git_commit, None)
self.assertEquals(options.output_format, "emacs")
self.assertEquals(options.verbosity, 1)
+ # Check argument validation.
self.assertRaises(ValueError, ProcessorOptions, output_format="bad")
ProcessorOptions(output_format="emacs") # No ValueError: works
ProcessorOptions(output_format="vs7") # works
@@ -119,6 +131,31 @@ class ProcessorOptionsTest(unittest.TestCase):
self.assertEquals(options.output_format, "vs7")
self.assertEquals(options.verbosity, 3)
+ def test_eq(self):
+ """Test __eq__ equality function."""
+ # == calls __eq__.
+ self.assertTrue(ProcessorOptions() == ProcessorOptions())
+
+ # Verify that a difference in any argument cause equality to fail.
+ options = ProcessorOptions(extra_flag_values={"extra_value" : 1},
+ filter=CategoryFilter(["+"]),
+ git_commit="commit",
+ output_format="vs7",
+ verbosity=1)
+ self.assertFalse(options == ProcessorOptions(extra_flag_values={"extra_value" : 2}))
+ self.assertFalse(options == ProcessorOptions(filter=CategoryFilter(["-"])))
+ self.assertFalse(options == ProcessorOptions(git_commit="commit2"))
+ self.assertFalse(options == ProcessorOptions(output_format="emacs"))
+ self.assertFalse(options == ProcessorOptions(verbosity=2))
+
+ def test_ne(self):
+ """Test __ne__ inequality function."""
+ # != calls __ne__.
+ # By default, __ne__ always returns true on different objects.
+ # Thus, just check the distinguishing case to verify that the
+ # code defines __ne__.
+ self.assertFalse(ProcessorOptions() != ProcessorOptions())
+
def test_should_report_error(self):
"""Test should_report_error()."""
filter = CategoryFilter(["-xyz"])
@@ -133,27 +170,31 @@ class ProcessorOptionsTest(unittest.TestCase):
self.assertFalse(options.should_report_error("xyz", 3))
-class DefaultArgumentsTest(unittest.TestCase):
+class WebKitArgumentDefaultsTest(unittest.TestCase):
"""Tests validity of default arguments used by check-webkit-style."""
+ def defaults(self):
+ return style.webkit_argument_defaults()
+
def test_filter_rules(self):
+ defaults = self.defaults()
already_seen = []
- for rule in style.WEBKIT_FILTER_RULES:
+ for rule in defaults.filter_rules:
# Check no leading or trailing white space.
self.assertEquals(rule, rule.strip())
# All categories are on by default, so defaults should
# begin with -.
self.assertTrue(rule.startswith('-'))
self.assertTrue(rule[1:] in style.STYLE_CATEGORIES)
+ # Check no rule occurs twice.
self.assertFalse(rule in already_seen)
already_seen.append(rule)
def test_defaults(self):
"""Check that default arguments are valid."""
- defaults = style.ArgumentDefaults(style.DEFAULT_OUTPUT_FORMAT,
- style.DEFAULT_VERBOSITY,
- style.WEBKIT_FILTER_RULES)
+ defaults = self.defaults()
+
# FIXME: We should not need to call parse() to determine
# whether the default arguments are valid.
parser = style.ArgumentParser(defaults)
@@ -311,6 +352,68 @@ class ArgumentParserTest(unittest.TestCase):
self.assertEquals(files, ['foo.cpp', 'bar.cpp'])
+class StyleCheckerTest(unittest.TestCase):
+
+ """Test the StyleChecker class.
+
+ Attributes:
+ error_message: A string that is the last error message reported
+ by the test StyleChecker instance.
+
+ """
+
+ error_message = ""
+
+ def mock_write_error(self, error_message):
+ """Store an error message without printing it."""
+ self.error_message = error_message
+ pass
+
+ def style_checker(self, options):
+ return StyleChecker(options, self.mock_write_error)
+
+ def test_init(self):
+ """Test __init__ constructor."""
+ options = ProcessorOptions()
+ 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."""
+ style_checker._handle_error(filename="filename",
+ line_number=1,
+ category="category",
+ confidence=error_confidence,
+ message="message")
+
+ def test_handle_error(self):
+ """Test _handler_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_message, "")
+
+ # 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_message, "")
+
+ self.write_sample_error(style_checker, 3)
+ self.assertEquals(style_checker.error_count, 1) # Error confidence just high enough.
+ self.assertEquals(self.error_message, "filename:1: message [category] [3]\n")
+
+ # 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_message, "filename(1): message [category] [3]\n")
+
+
if __name__ == '__main__':
import sys
diff --git a/WebKitTools/Scripts/webkitpy/style/cpp_style.py b/WebKitTools/Scripts/webkitpy/style/cpp_style.py
index 469d2e0..3940a75 100644
--- a/WebKitTools/Scripts/webkitpy/style/cpp_style.py
+++ b/WebKitTools/Scripts/webkitpy/style/cpp_style.py
@@ -247,7 +247,6 @@ class _CppStyleState(object):
def __init__(self):
self.verbose_level = 1 # global setting.
- self.error_count = 0 # global count of reported errors
def set_verbose_level(self, level):
"""Sets the module's verbosity, and returns the previous setting."""
@@ -255,14 +254,6 @@ class _CppStyleState(object):
self.verbose_level = level
return last_verbose_level
- def reset_error_count(self):
- """Sets the module's error statistic back to zero."""
- self.error_count = 0
-
- def increment_error_count(self):
- """Bumps the module's error statistic."""
- self.error_count += 1
-
_cpp_style_state = _CppStyleState()
@@ -277,11 +268,6 @@ def _set_verbose_level(level):
return _cpp_style_state.set_verbose_level(level)
-def error_count():
- """Returns the global count of reported errors."""
- return _cpp_style_state.error_count
-
-
class _FunctionState(object):
"""Tracks current function name and the number of lines in its body."""
diff --git a/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py b/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py
index cd702d2..4d2f2e6 100644
--- a/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py
@@ -2619,16 +2619,6 @@ class NoNonVirtualDestructorsTest(CppStyleTestBase):
'virtual method(s), one declared at line 2. [runtime/virtual] [4]'])
-class CppStyleStateTest(unittest.TestCase):
- def test_error_count(self):
- self.assertEquals(0, cpp_style.error_count())
- cpp_style._cpp_style_state.increment_error_count()
- cpp_style._cpp_style_state.increment_error_count()
- self.assertEquals(2, cpp_style.error_count())
- cpp_style._cpp_style_state.reset_error_count()
- self.assertEquals(0, cpp_style.error_count())
-
-
class WebKitStyleTest(CppStyleTestBase):
# for http://webkit.org/coding/coding-style.html
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list