Skip to content
Permalink
Browse files
2010-01-13 Chris Jerdonek <chris.jerdonek@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.

Canonical link: https://commits.webkit.org/44593@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@53185 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Shinichiro Hamaji committed Jan 13, 2010
1 parent f8b5195 commit 4f7d2a3c630539b7f392d1753fe6c302fbf30b8a
@@ -1,3 +1,24 @@
2010-01-13 Chris Jerdonek <chris.jerdonek@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@chromium.org>

Unreviewed. Now I can review :)
@@ -230,6 +230,71 @@ def _create_usage(defaults):
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 @@ def to_flag_string(self, options):

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 @@ def parse(self, args, extra_flags=None):
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)
@@ -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 @@ def test_parse_default_arguments(self):

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 @@ def test_parse_explicit_arguments(self):
(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'])
@@ -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 @@ def set_verbose_level(self, level):
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):

0 comments on commit 4f7d2a3

Please sign in to comment.