[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373
hamaji at chromium.org
hamaji at chromium.org
Thu Apr 8 01:03:28 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit ea42cb909fc74f286ec9c8dcf98f0d858b2a0e2e
Author: hamaji at chromium.org <hamaji at chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Wed Jan 13 15:52:51 2010 +0000
2010-01-13 Chris Jerdonek <chris.jerdonek at gmail.com>
Reviewed by Shinichiro Hamaji.
Created a CategoryFilter class to encapsulate the logic of
filter rules.
https://bugs.webkit.org/show_bug.cgi?id=33454
* Scripts/webkitpy/style/checker.py:
- Added CategoryFilter class.
* Scripts/webkitpy/style/checker_unittest.py:
- Added CategoryFilter unit tests.
* Scripts/webkitpy/style/cpp_style.py:
- Updated filter methods to use CategoryFilter.
* Scripts/webkitpy/style/cpp_style_unittest.py:
- Updated references to filters.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@53185 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index e603516..3a15475 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,24 @@
+2010-01-13 Chris Jerdonek <chris.jerdonek at gmail.com>
+
+ Reviewed by Shinichiro Hamaji.
+
+ Created a CategoryFilter class to encapsulate the logic of
+ filter rules.
+
+ https://bugs.webkit.org/show_bug.cgi?id=33454
+
+ * Scripts/webkitpy/style/checker.py:
+ - Added CategoryFilter class.
+
+ * Scripts/webkitpy/style/checker_unittest.py:
+ - Added CategoryFilter unit tests.
+
+ * Scripts/webkitpy/style/cpp_style.py:
+ - Updated filter methods to use CategoryFilter.
+
+ * Scripts/webkitpy/style/cpp_style_unittest.py:
+ - Updated references to filters.
+
2010-01-12 Shinichiro Hamaji <hamaji at chromium.org>
Unreviewed. Now I can review :)
diff --git a/WebKitTools/Scripts/webkitpy/style/checker.py b/WebKitTools/Scripts/webkitpy/style/checker.py
index 20ebe25..aa84308 100644
--- a/WebKitTools/Scripts/webkitpy/style/checker.py
+++ b/WebKitTools/Scripts/webkitpy/style/checker.py
@@ -230,6 +230,71 @@ Syntax: %(program_name)s [--verbose=#] [--git-commit=<SingleCommit>] [--output=v
return usage
+class CategoryFilter(object):
+
+ """Filters whether to check style categories."""
+
+ def __init__(self, filter_rules):
+ """Create a category filter.
+
+ This method performs argument validation but does not strip
+ leading or trailing white space.
+
+ Args:
+ filter_rules: A list of strings that are filter rules, which
+ are strings beginning with the plus or minus
+ symbol (+/-). The list should include any
+ default filter rules at the beginning.
+
+ Raises:
+ ValueError: Invalid filter rule if a rule does not start with
+ plus ("+") or minus ("-").
+
+ """
+ for rule in filter_rules:
+ if not (rule.startswith('+') or rule.startswith('-')):
+ raise ValueError('Invalid filter rule "%s": every rule '
+ 'rule in the --filter flag must start '
+ 'with + or -.' % rule)
+
+ self._filter_rules = filter_rules
+ self._should_check_category = {} # Cached dictionary of category to True/False
+
+ def __str__(self):
+ return ",".join(self._filter_rules)
+
+ 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))
+
+ def should_check(self, category):
+ """Return whether the category should be checked.
+
+ The rules for determining whether a category should be checked
+ are as follows. By default all categories should be checked.
+ Then apply the filter rules in order from first to last, with
+ later flags taking precedence.
+
+ A filter rule applies to a category if the string after the
+ leading plus/minus (+/-) matches the beginning of the category
+ name. A plus (+) means the category should be checked, while a
+ minus (-) means the category should not be checked.
+
+ """
+ if category in self._should_check_category:
+ return self._should_check_category[category]
+
+ should_check = True # All categories checked by default.
+ for rule in self._filter_rules:
+ if not category.startswith(rule[1:]):
+ continue
+ should_check = rule.startswith('+')
+ self._should_check_category[category] = should_check # Update cache.
+ return should_check
+
+
# This class should not have knowledge of the flag key names.
class ProcessorOptions(object):
@@ -244,12 +309,8 @@ class ProcessorOptions(object):
confidence score at or above this value.
The default is 1, which displays all errors.
- filter_rules: A list of strings that are boolean filter rules used
- to determine whether a style category should be checked.
- Each string should start with + or -. An example
- string is "+whitespace/indent". The list includes any
- prepended default filter rules. The default is the
- empty list, which includes all categories.
+ filter: A CategoryFilter instance. The default is the empty filter,
+ which means that all categories should be checked.
git_commit: A string representing the git commit to check.
The default is None.
@@ -259,16 +320,16 @@ class ProcessorOptions(object):
class. The default is the empty dictionary.
"""
- def __init__(self, output_format, verbosity=1, filter_rules=None,
+ def __init__(self, output_format, verbosity=1, filter=None,
git_commit=None, extra_flag_values=None):
- if filter_rules is None:
- filter_rules = []
+ if filter is None:
+ filter = CategoryFilter([])
if extra_flag_values is None:
extra_flag_values = {}
self.output_format = output_format
self.verbosity = verbosity
- self.filter_rules = filter_rules
+ self.filter = filter
self.git_commit = git_commit
self.extra_flag_values = extra_flag_values
@@ -285,7 +346,7 @@ def set_options(options):
"""
cpp_style._set_output_format(options.output_format)
cpp_style._set_verbose_level(options.verbosity)
- cpp_style._set_filters(options.filter_rules)
+ cpp_style._set_filter(options.filter)
# This class should not have knowledge of the flag key names.
@@ -328,8 +389,11 @@ class ArgumentPrinter(object):
flags['output'] = options.output_format
flags['verbose'] = options.verbosity
- if options.filter_rules:
- flags['filter'] = ','.join(options.filter_rules)
+ if options.filter:
+ # Only include the filter flag if rules are present.
+ filter_string = str(options.filter)
+ if filter_string:
+ flags['filter'] = filter_string
if options.git_commit:
flags['git-commit'] = options.git_commit
@@ -491,13 +555,9 @@ class ArgumentParser(object):
raise ValueError('Invalid --verbose value %s: value must '
'be between 1-5.' % verbosity)
- for rule in filter_rules:
- if not (rule.startswith('+') or rule.startswith('-')):
- raise ValueError('Invalid filter rule "%s": every rule '
- 'rule in the --filter flag must start '
- 'with + or -.' % rule)
+ filter = CategoryFilter(filter_rules)
- options = ProcessorOptions(output_format, verbosity, filter_rules,
+ options = ProcessorOptions(output_format, verbosity, filter,
git_commit, extra_flag_values)
return (filenames, options)
diff --git a/WebKitTools/Scripts/webkitpy/style/checker_unittest.py b/WebKitTools/Scripts/webkitpy/style/checker_unittest.py
index 6ff89af..bb0cb5c 100644
--- a/WebKitTools/Scripts/webkitpy/style/checker_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/style/checker_unittest.py
@@ -37,6 +37,52 @@
import unittest
import checker as style
+from checker import CategoryFilter
+
+
+class CategoryFilterTest(unittest.TestCase):
+
+ """Tests CategoryFilter class."""
+
+ 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
+
+ def test_str(self):
+ """Test __str__ "to string" operator."""
+ filter = CategoryFilter(["+a", "-b"])
+ self.assertEquals(str(filter), "+a,-b")
+
+ def test_eq(self):
+ """Test __eq__ equality operator."""
+ filter1 = CategoryFilter(["+a", "+b"])
+ filter2 = CategoryFilter(["+a", "+b"])
+ filter3 = CategoryFilter(["+b", "+a"])
+ self.assertEquals(filter1, filter2)
+ self.assertNotEquals(filter1, filter3)
+
+ def test_should_check(self):
+ """Test should_check() method."""
+ filter = CategoryFilter([])
+ self.assertTrue(filter.should_check("everything"))
+ # Check a second time to exercise cache.
+ self.assertTrue(filter.should_check("everything"))
+
+ filter = CategoryFilter(["-"])
+ self.assertFalse(filter.should_check("anything"))
+ # Check a second time to exercise cache.
+ self.assertFalse(filter.should_check("anything"))
+
+ filter = CategoryFilter(["-", "+ab"])
+ self.assertTrue(filter.should_check("abc"))
+ self.assertFalse(filter.should_check("a"))
+
+ filter = CategoryFilter(["+", "-ab"])
+ self.assertFalse(filter.should_check("abc"))
+ self.assertTrue(filter.should_check("a"))
class DefaultArgumentsTest(unittest.TestCase):
@@ -46,6 +92,8 @@ class DefaultArgumentsTest(unittest.TestCase):
def test_filter_rules(self):
already_seen = []
for rule in style.WEBKIT_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('-'))
@@ -75,7 +123,8 @@ class ArgumentPrinterTest(unittest.TestCase):
def _create_options(self, output_format='emacs', verbosity=3,
filter_rules=[], git_commit=None,
extra_flag_values={}):
- return style.ProcessorOptions(output_format, verbosity, filter_rules,
+ filter = CategoryFilter(filter_rules)
+ return style.ProcessorOptions(output_format, verbosity, filter,
git_commit, extra_flag_values)
def test_to_flag_string(self):
@@ -173,7 +222,8 @@ class ArgumentParserTest(unittest.TestCase):
self.assertEquals(options.output_format, 'vs7')
self.assertEquals(options.verbosity, 3)
- self.assertEquals(options.filter_rules, ['-', '+whitespace'])
+ self.assertEquals(options.filter,
+ CategoryFilter(["-", "+whitespace"]))
self.assertEquals(options.git_commit, None)
def test_parse_explicit_arguments(self):
@@ -187,8 +237,8 @@ class ArgumentParserTest(unittest.TestCase):
(files, options) = parse(['--git-commit=commit'])
self.assertEquals(options.git_commit, 'commit')
(files, options) = parse(['--filter=+foo,-bar'])
- self.assertEquals(options.filter_rules,
- ['-', '+whitespace', '+foo', '-bar'])
+ self.assertEquals(options.filter,
+ CategoryFilter(["-", "+whitespace", "+foo", "-bar"]))
# Pass extra flag values.
(files, options) = parse(['--extra'], ['extra'])
diff --git a/WebKitTools/Scripts/webkitpy/style/cpp_style.py b/WebKitTools/Scripts/webkitpy/style/cpp_style.py
index 8857572..6999a7b 100644
--- a/WebKitTools/Scripts/webkitpy/style/cpp_style.py
+++ b/WebKitTools/Scripts/webkitpy/style/cpp_style.py
@@ -248,8 +248,8 @@ class _CppStyleState(object):
def __init__(self):
self.verbose_level = 1 # global setting.
self.error_count = 0 # global count of reported errors
- # filters to apply when emitting error messages
- self.filters = []
+ # filter to apply when emitting error messages
+ self.filter = None
# output format:
# "emacs" - format that emacs can parse (default)
@@ -266,32 +266,14 @@ class _CppStyleState(object):
self.verbose_level = level
return last_verbose_level
- def set_filters(self, filters):
- """Sets the error-message filters.
-
- These filters are applied when deciding whether to emit a given
- error message.
+ def set_filter(self, filter):
+ """Sets the error-message filter.
Args:
- filters: A list of strings that are boolean filter rules used
- to determine whether a style category should be checked.
- Each string should start with + or -. An example
- string is "+whitespace/indent". The list includes any
- prepended default filter rules.
-
- Raises:
- ValueError: Not all filters started with '+' or '-'. For example,
- "-,+whitespace,-whitespace/indent,whitespace/badfilter"
+ filter: A CategoryFilter instance.
+
"""
- self.filters = []
- for filter in filters:
- clean_filter = filter.strip()
- if clean_filter:
- self.filters.append(clean_filter)
- for filter in self.filters:
- if not (filter.startswith('+') or filter.startswith('-')):
- raise ValueError('Every filter in --filter must start with '
- '+ or - (%s does not)' % filter)
+ self.filter = filter
def reset_error_count(self):
"""Sets the module's error statistic back to zero."""
@@ -325,25 +307,22 @@ def _set_verbose_level(level):
return _cpp_style_state.set_verbose_level(level)
-def _filters():
- """Returns the module's list of output filters, as a list."""
- return _cpp_style_state.filters
+def _filter():
+ """Returns the module's CategoryFilter instance."""
+ return _cpp_style_state.filter
-def _set_filters(filters):
- """Sets the module's error-message filters.
+def _set_filter(filter):
+ """Sets the module's error-message filter.
- These filters are applied when deciding whether to emit a given
+ The filter is applied when deciding whether to emit a given
error message.
Args:
- filters: A list of strings that are boolean filter rules used
- to determine whether a style category should be checked.
- Each string should start with + or -. An example
- string is "+whitespace/indent". The list includes any
- prepended default filter rules.
+ filter: A CategoryFilter instance.
+
"""
- _cpp_style_state.set_filters(filters)
+ _cpp_style_state.set_filter(filter)
def error_count():
@@ -505,20 +484,12 @@ def _should_print_error(category, confidence):
if confidence < _cpp_style_state.verbose_level:
return False
- is_filtered = False
- for one_filter in _filters():
- if one_filter.startswith('-'):
- if category.startswith(one_filter[1:]):
- is_filtered = True
- elif one_filter.startswith('+'):
- if category.startswith(one_filter[1:]):
- is_filtered = False
- else:
- assert False # should have been checked for in set_filter.
- if is_filtered:
- return False
+ filter = _filter()
- return True
+ if filter is None:
+ return True # All categories should be checked by default.
+
+ return filter.should_check(category)
def error(filename, line_number, category, confidence, message):
diff --git a/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py b/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py
index fe358ef..6bb5ae9 100644
--- a/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py
@@ -41,10 +41,11 @@ import random
import re
import unittest
import cpp_style
-# FIXME: Remove the need to import something from checker. See the
+# FIXME: Remove the need to import anything from checker. See the
# FIXME notes near the STYLE_CATEGORIES definition for a
# suggestion on how to best do this.
from checker import STYLE_CATEGORIES
+from checker import CategoryFilter
# This class works as an error collector and replaces cpp_style.Error
# function for the unit tests. We also verify each category we see
@@ -1571,9 +1572,9 @@ class CppStyleTest(CppStyleTestBase):
'Tab found; better to use spaces [whitespace/tab] [1]')
def test_filter(self):
- old_filters = cpp_style._cpp_style_state.filters
+ old_filter = cpp_style._cpp_style_state.filter
try:
- cpp_style._cpp_style_state.set_filters(['-', '+whitespace', '-whitespace/indent'])
+ cpp_style._cpp_style_state.set_filter(CategoryFilter(['-', '+whitespace', '-whitespace/indent']))
self.assert_lint(
'// Hello there ',
'Line ends in whitespace. Consider deleting these extra spaces.'
@@ -1581,22 +1582,22 @@ class CppStyleTest(CppStyleTestBase):
self.assert_lint('int a = (int)1.0;', '')
self.assert_lint(' weird opening space', '')
finally:
- cpp_style._cpp_style_state.filters = old_filters
+ cpp_style._cpp_style_state.filter = old_filter
def test_filter_appending(self):
- old_filters = cpp_style._cpp_style_state.filters
+ old_filter = cpp_style._cpp_style_state.filter
try:
# Reset filters
- cpp_style._cpp_style_state.set_filters(['-whitespace'])
+ cpp_style._cpp_style_state.set_filter(CategoryFilter(['-whitespace']))
self.assert_lint('// Hello there ', '')
- cpp_style._cpp_style_state.set_filters(['-whitespace', '+whitespace/end_of_line'])
+ cpp_style._cpp_style_state.set_filter(CategoryFilter(['-whitespace', '+whitespace/end_of_line']))
self.assert_lint(
'// Hello there ',
'Line ends in whitespace. Consider deleting these extra spaces.'
' [whitespace/end_of_line] [4]')
self.assert_lint(' weird opening space', '')
finally:
- cpp_style._cpp_style_state.filters = old_filters
+ cpp_style._cpp_style_state.filter = old_filter
def test_unnamed_namespaces_in_headers(self):
self.assert_language_rules_check(
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list