Some minor check-webkit-style code clean-ups. This revision
authorcjerdonek@webkit.org <cjerdonek@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Feb 2010 07:38:43 +0000 (07:38 +0000)
committercjerdonek@webkit.org <cjerdonek@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Feb 2010 07:38:43 +0000 (07:38 +0000)
contains no new functionality.

Reviewed by Shinichiro Hamaji.

https://bugs.webkit.org/show_bug.cgi?id=34932

* Scripts/check-webkit-style:
  - Replaced the call to webkit_argument_defaults() with a
    default parameter in the ArgumentParser constructor.

* Scripts/webkitpy/style/checker.py:
  - Removed the WEBKIT prefix from the default global variables.
  - Prefixed several of the global variables with an underscore
    to reflect that they are used internally.
  - Renamed _DEFAULT_FILTER_RULES to _BASE_FILTER_RULES.
  - Addressed a FIXME by changing the _PATH_RULES_SPECIFIER
    configuration from list-tuple pairs to list-list pairs.
  - Renamed style_categories() to _all_categories().
  - Renamed webkit_argument_defaults() to _check_webkit_style_defaults().
  - Renamed the ArgumentDefaults class to DefaultCommandOptionValues.
  - In the ArgumentParser class--
    - Renamed the doc_print attribute to stderr_write.
    - Other minor updates.

* Scripts/webkitpy/style/checker_unittest.py:
  - Updated the import statements and unit test classes as necessary.
  - Added assertions to test _PATH_RULES_SPECIFIER more fully.

* Scripts/webkitpy/style/error_handlers_unittest.py:
  - Updated the unit test classes as necessary.
  - Changed StyleErrorHandlerTestBase to store a list of error
    messages rather than just the last one.

* Scripts/webkitpy/style/filter.py:
  - Altered FilterConfiguration._path_rules_from_path()
    to convert the path_rules list to a tuple.

* Scripts/webkitpy/style/filter_unittest.py:
  - Updated the unit tests to reflect the change from tuples to
    lists in the _PATH_RULES_SPECIFIER configuration variable.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@54874 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebKitTools/ChangeLog
WebKitTools/Scripts/check-webkit-style
WebKitTools/Scripts/webkitpy/style/checker.py
WebKitTools/Scripts/webkitpy/style/checker_unittest.py
WebKitTools/Scripts/webkitpy/style/error_handlers.py
WebKitTools/Scripts/webkitpy/style/error_handlers_unittest.py
WebKitTools/Scripts/webkitpy/style/filter.py
WebKitTools/Scripts/webkitpy/style/filter_unittest.py

index 3a8b7f533bd9d61c8f3658c46c2e139b9675103a..4b65659813be5729058a189219674c1750bd8a07 100644 (file)
@@ -1,3 +1,47 @@
+2010-02-15  Chris Jerdonek  <cjerdonek@webkit.org>
+
+        Reviewed by Shinichiro Hamaji.
+
+        Some minor check-webkit-style code clean-ups.  This revision
+        contains no new functionality.
+
+        https://bugs.webkit.org/show_bug.cgi?id=34932
+
+        * Scripts/check-webkit-style:
+          - Replaced the call to webkit_argument_defaults() with a
+            default parameter in the ArgumentParser constructor.
+
+        * Scripts/webkitpy/style/checker.py:
+          - Removed the WEBKIT prefix from the default global variables.
+          - Prefixed several of the global variables with an underscore
+            to reflect that they are used internally.
+          - Renamed _DEFAULT_FILTER_RULES to _BASE_FILTER_RULES.
+          - Addressed a FIXME by changing the _PATH_RULES_SPECIFIER
+            configuration from list-tuple pairs to list-list pairs.
+          - Renamed style_categories() to _all_categories().
+          - Renamed webkit_argument_defaults() to _check_webkit_style_defaults().
+          - Renamed the ArgumentDefaults class to DefaultCommandOptionValues.
+          - In the ArgumentParser class--
+            - Renamed the doc_print attribute to stderr_write.
+            - Other minor updates.
+
+        * Scripts/webkitpy/style/checker_unittest.py:
+          - Updated the import statements and unit test classes as necessary.
+          - Added assertions to test _PATH_RULES_SPECIFIER more fully.
+
+        * Scripts/webkitpy/style/error_handlers_unittest.py:
+          - Updated the unit test classes as necessary.
+          - Changed StyleErrorHandlerTestBase to store a list of error
+            messages rather than just the last one.
+
+        * Scripts/webkitpy/style/filter.py:
+          - Altered FilterConfiguration._path_rules_from_path()
+            to convert the path_rules list to a tuple.
+
+        * Scripts/webkitpy/style/filter_unittest.py:
+          - Updated the unit tests to reflect the change from tuples to
+            lists in the _PATH_RULES_SPECIFIER configuration variable.
+
 2010-02-16  Mark Rowe  <mrowe@apple.com>
 
         Let's not check garbage in to common build scripts and hose the world now eh guys?
index 501264b4759d80585060fac84824100e37829dbb..f19fe73aedbd8084c16a3e738b36d0a5a6cee06f 100755 (executable)
@@ -58,9 +58,7 @@ def main():
                                            codecs.getwriter('utf8'),
                                            'replace')
 
-    defaults = checker.webkit_argument_defaults()
-
-    parser = checker.ArgumentParser(defaults)
+    parser = checker.ArgumentParser()
     (files, options) = parser.parse(sys.argv[1:])
 
     style_checker = checker.StyleChecker(options)
index fbda8cb2b8454a82cec180489ba6f7bd824e4f64..d455f23d4996aca2175a339848bf8e87a03f28f5 100644 (file)
@@ -45,9 +45,9 @@ from processors.cpp import CppProcessor
 from processors.text import TextProcessor
 
 
-# These defaults are used by check-webkit-style.
-WEBKIT_DEFAULT_VERBOSITY = 1
-WEBKIT_DEFAULT_OUTPUT_FORMAT = 'emacs'
+# These are default option values for the command-line option parser.
+_DEFAULT_VERBOSITY = 1
+_DEFAULT_OUTPUT_FORMAT = 'emacs'
 
 
 # FIXME: For style categories we will never want to have, remove them.
@@ -55,14 +55,16 @@ WEBKIT_DEFAULT_OUTPUT_FORMAT = 'emacs'
 #        modify the implementation and enable them.
 #
 # Throughout this module, we use "filter rule" rather than "filter"
-# for an individual boolean filter flag like "+foo". This allows us to
+# for an individual boolean filter flag like "+foo".  This allows us to
 # reserve "filter" for what one gets by collectively applying all of
 # the filter rules.
 #
-# 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_DEFAULT_FILTER_RULES = [
+# The base filter rules are the filter rules that begin the list of
+# filter rules used to check style.  For example, these rules precede
+# any user-specified filter rules.  Since by default all categories are
+# checked, this list should normally include only rules that begin
+# with a "-" sign.
+_BASE_FILTER_RULES = [
     '-build/endif_comment',
     '-build/include_what_you_use',  # <string> for std::string
     '-build/storage_class',  # const static
@@ -86,26 +88,24 @@ WEBKIT_DEFAULT_FILTER_RULES = [
     ]
 
 
-# FIXME: Change the second value of each tuple from a tuple to a list,
-#        and alter the filter code so it accepts lists instead.  (The
-#        filter code will need to convert incoming values from a list
-#        to a tuple prior to caching).  This will make this
-#        configuration setting a bit simpler since tuples have an
-#        unusual syntax case.
-#
 # The path-specific filter rules.
 #
 # This list is order sensitive.  Only the first path substring match
 # is used.  See the FilterConfiguration documentation in filter.py
 # for more information on this list.
+#
+# Each string appearing in this nested list should have at least
+# one associated unit test assertion.  These assertions are located,
+# for example, in the test_path_rules_specifier() unit test method of
+# checker_unittest.py.
 _PATH_RULES_SPECIFIER = [
     # Files in these directories are consumers of the WebKit
     # API and therefore do not follow the same header including
     # discipline as WebCore.
     (["WebKitTools/WebKitAPITest/",
       "WebKit/qt/QGVLauncher/"],
-     ("-build/include",
-      "-readability/streams")),
+     ["-build/include",
+      "-readability/streams"]),
     ([# The GTK+ APIs use GTK+ naming style, which includes
       # lower-cased, underscore-separated values.
       "WebKit/gtk/webkit/",
@@ -116,13 +116,7 @@ _PATH_RULES_SPECIFIER = [
       # QtTest module.
       "WebKit/qt/tests/",
       "JavaScriptCore/qt/tests"],
-     ("-readability/naming",)),
-    # These are test file patterns.
-    (["_test.cpp",
-      "_unittest.cpp",
-      "_regtest.cpp"],
-     ("-readability/streams",  # Many unit tests use cout.
-      "-runtime/rtti")),
+     ["-readability/naming"]),
 ]
 
 
@@ -131,7 +125,7 @@ _PATH_RULES_SPECIFIER = [
 # future merges.
 #
 # Include a warning for skipped files that are less obvious.
-SKIPPED_FILES_WITH_WARNING = [
+_SKIPPED_FILES_WITH_WARNING = [
     # The Qt API and tests do not follow WebKit style.
     # They follow Qt style. :)
     "gtk2drawing.c", # WebCore/platform/gtk/gtk2drawing.c
@@ -145,36 +139,37 @@ SKIPPED_FILES_WITH_WARNING = [
 
 # Don't include a warning for skipped files that are more common
 # and more obvious.
-SKIPPED_FILES_WITHOUT_WARNING = [
+_SKIPPED_FILES_WITHOUT_WARNING = [
     "LayoutTests/"
     ]
 
 
 # The maximum number of errors to report per file, per category.
 # If a category is not a key, then it has no maximum.
-MAX_REPORTS_PER_CATEGORY = {
+_MAX_REPORTS_PER_CATEGORY = {
     "whitespace/carriage_return": 1
 }
 
 
-def style_categories():
+def _all_categories():
     """Return the set of all categories used by check-webkit-style."""
     # Take the union across all processors.
     return CommonCategories.union(CppProcessor.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 _check_webkit_style_defaults():
+    """Return the default command-line options for check-webkit-style."""
+    return DefaultCommandOptionValues(base_filter_rules=_BASE_FILTER_RULES,
+                                      output_format=_DEFAULT_OUTPUT_FORMAT,
+                                      verbosity=_DEFAULT_VERBOSITY)
 
 
-def _create_usage(defaults):
+
+def _create_usage(default_options):
     """Return the usage string to display for command help.
 
     Args:
-      defaults: An ArgumentDefaults instance.
+      default_options: A DefaultCommandOptionValues instance.
 
     """
     usage = """
@@ -236,8 +231,8 @@ Syntax: %(program_name)s [--verbose=#] [--git-commit=<SingleCommit>] [--output=v
       the empty filter as follows:
          --filter=
 """ % {'program_name': os.path.basename(sys.argv[0]),
-       'default_verbosity': defaults.verbosity,
-       'default_output_format': defaults.output_format}
+       'default_verbosity': default_options.verbosity,
+       'default_output_format': default_options.output_format}
 
     return usage
 
@@ -255,7 +250,7 @@ Syntax: %(program_name)s [--verbose=#] [--git-commit=<SingleCommit>] [--output=v
 # This class should not have knowledge of the flag key names.
 class ProcessorOptions(object):
 
-    """A container to store options passed via the command line.
+    """Stores the option values passed by the user via the command line.
 
     Attributes:
       extra_flag_values: A string-string dictionary of all flag key-value
@@ -359,23 +354,22 @@ class ProcessorOptions(object):
 
 
 # This class should not have knowledge of the flag key names.
-class ArgumentDefaults(object):
+class DefaultCommandOptionValues(object):
 
-    """A container to store default argument values.
+    """Stores the default check-webkit-style command-line options.
 
     Attributes:
+      base_filter_rules: A list of boolean filter rule strings that begin
+                         the list of filter rules used to check style.
       output_format: A string that is the default output format.
       verbosity: An integer that is the default verbosity level.
-      base_filter_rules: A list of strings that are boolean filter rules
-                         to prepend to any user-specified rules.
 
     """
 
-    def __init__(self, default_output_format, default_verbosity,
-                 default_base_filter_rules):
-        self.output_format = default_output_format
-        self.verbosity = default_verbosity
-        self.base_filter_rules = default_base_filter_rules
+    def __init__(self, base_filter_rules, output_format, verbosity):
+        self.base_filter_rules = base_filter_rules
+        self.output_format = output_format
+        self.verbosity = verbosity
 
 
 class ArgumentPrinter(object):
@@ -413,31 +407,40 @@ class ArgumentPrinter(object):
         return flag_string.strip()
 
 
+# FIXME: Replace the use of getopt.getopt() with optparse.OptionParser.
 class ArgumentParser(object):
 
     """Supports the parsing of check-webkit-style command arguments.
 
     Attributes:
-      defaults: An ArgumentDefaults instance.
-      create_usage: A function that accepts an ArgumentDefaults instance
-                    and returns a string of usage instructions.
-                    This defaults to the function used to generate the
-                    usage string for check-webkit-style.
-      doc_print: A function that accepts a string parameter and that is
-                 called to display help messages. This defaults to
-                 sys.stderr.write().
+      create_usage: A function that accepts a DefaultCommandOptionValues
+                    instance and returns a string of usage instructions.
+                    Defaults to the function that generates the usage
+                    string for check-webkit-style.
+      default_options: A DefaultCommandOptionValues instance.  Defaults
+                       to the default command option values used by
+                       check-webkit-style.
+      stderr_write: A function that takes a string as a parameter and
+                    serves as stderr.write.  Defaults to sys.stderr.write.
+                    This parameter should be specified only for unit tests.
 
     """
 
-    def __init__(self, argument_defaults, create_usage=None, doc_print=None):
+    def __init__(self,
+                 create_usage=None,
+                 default_options=None,
+                 stderr_write=None):
         if create_usage is None:
             create_usage = _create_usage
-        if doc_print is None:
-            doc_print = sys.stderr.write
+        if default_options is None:
+            default_options = _check_webkit_style_defaults()
+        if stderr_write is None:
+            stderr_write = sys.stderr.write
 
-        self.defaults = argument_defaults
+        # FIXME: Rename these to reflect that they are internal.
         self.create_usage = create_usage
-        self.doc_print = doc_print
+        self.default_options = default_options
+        self.stderr_write = stderr_write
 
     def _exit_with_usage(self, error_message=''):
         """Exit and print a usage string with an optional error message.
@@ -446,8 +449,8 @@ class ArgumentParser(object):
           error_message: A string that is an error message to print.
 
         """
-        usage = self.create_usage(self.defaults)
-        self.doc_print(usage)
+        usage = self.create_usage(self.default_options)
+        self.stderr_write(usage)
         if error_message:
             sys.exit('\nFATAL ERROR: ' + error_message)
         else:
@@ -455,16 +458,16 @@ class ArgumentParser(object):
 
     def _exit_with_categories(self):
         """Exit and print the style categories and default filter rules."""
-        self.doc_print('\nAll categories:\n')
-        categories = style_categories()
+        self.stderr_write('\nAll categories:\n')
+        categories = _all_categories()
         for category in sorted(categories):
-            self.doc_print('    ' + category + '\n')
+            self.stderr_write('    ' + category + '\n')
 
-        self.doc_print('\nDefault filter rules**:\n')
-        for filter_rule in sorted(self.defaults.base_filter_rules):
-            self.doc_print('    ' + filter_rule + '\n')
-        self.doc_print('\n**The command always evaluates the above rules, '
-                       'and before any --filter flag.\n\n')
+        self.stderr_write('\nDefault filter rules**:\n')
+        for filter_rule in sorted(self.default_options.base_filter_rules):
+            self.stderr_write('    ' + filter_rule + '\n')
+        self.stderr_write('\n**The command always evaluates the above rules, '
+                          'and before any --filter flag.\n\n')
 
         sys.exit(0)
 
@@ -504,9 +507,9 @@ class ArgumentParser(object):
         if extra_flags is None:
             extra_flags = []
 
-        output_format = self.defaults.output_format
-        verbosity = self.defaults.verbosity
-        base_rules = self.defaults.base_filter_rules
+        output_format = self.default_options.output_format
+        verbosity = self.default_options.verbosity
+        base_rules = self.default_options.base_filter_rules
 
         # The flags already supported by the ProcessorOptions class.
         flags = ['help', 'output=', 'verbose=', 'filter=', 'git-commit=']
@@ -558,7 +561,7 @@ class ArgumentParser(object):
                              'allowed output formats are emacs and vs7.' %
                              output_format)
 
-        all_categories = style_categories()
+        all_categories = _all_categories()
         validate_filter_rules(user_rules, all_categories)
 
         verbosity = int(verbosity)
@@ -573,7 +576,7 @@ class ArgumentParser(object):
         options = ProcessorOptions(extra_flag_values=extra_flag_values,
                       filter_configuration=filter_configuration,
                       git_commit=git_commit,
-                      max_reports_per_category=MAX_REPORTS_PER_CATEGORY,
+                      max_reports_per_category=_MAX_REPORTS_PER_CATEGORY,
                       output_format=output_format,
                       verbosity=verbosity)
 
@@ -617,14 +620,14 @@ class ProcessorDispatcher(object):
 
     def should_skip_with_warning(self, file_path):
         """Return whether the given file should be skipped with a warning."""
-        for skipped_file in SKIPPED_FILES_WITH_WARNING:
+        for skipped_file in _SKIPPED_FILES_WITH_WARNING:
             if file_path.find(skipped_file) >= 0:
                 return True
         return False
 
     def should_skip_without_warning(self, file_path):
         """Return whether the given file should be skipped without a warning."""
-        for skipped_file in SKIPPED_FILES_WITHOUT_WARNING:
+        for skipped_file in _SKIPPED_FILES_WITHOUT_WARNING:
             if file_path.find(skipped_file) >= 0:
                 return True
         return False
@@ -784,10 +787,11 @@ class StyleChecker(object):
 
         """
         if handle_style_error is None:
-            handle_style_error = DefaultStyleErrorHandler(file_path,
-                                     self.options,
-                                     self._increment_error_count,
-                                     self._stderr_write)
+            handle_style_error = DefaultStyleErrorHandler(file_path=file_path,
+                                     options=self.options,
+                                     increment_error_count=
+                                         self._increment_error_count,
+                                     stderr_write=self._stderr_write)
         if process_file is None:
             process_file = self._process_file
 
index e1c9baf709866e13b7e1614440139fbea40f651b..728c106aff135d7175323f0e35fbc9018e4d703e 100755 (executable)
 import unittest
 
 import checker as style
+from checker import _BASE_FILTER_RULES
+from checker import _MAX_REPORTS_PER_CATEGORY
 from checker import _PATH_RULES_SPECIFIER as PATH_RULES_SPECIFIER
-from checker import style_categories
+from checker import _all_categories
+from checker import DefaultCommandOptionValues
 from checker import ProcessorDispatcher
 from checker import ProcessorOptions
 from checker import StyleChecker
@@ -138,18 +141,18 @@ class GlobalVariablesTest(unittest.TestCase):
     """Tests validity of the global variables."""
 
     def _all_categories(self):
-        return style.style_categories()
+        return _all_categories()
 
     def defaults(self):
-        return style.webkit_argument_defaults()
+        return style._check_webkit_style_defaults()
 
-    def test_filter_rules(self):
+    def test_webkit_base_filter_rules(self):
+        base_filter_rules = _BASE_FILTER_RULES
         defaults = self.defaults()
         already_seen = []
-        validate_filter_rules(defaults.base_filter_rules,
-                              self._all_categories())
+        validate_filter_rules(base_filter_rules, self._all_categories())
         # Also do some additional checks.
-        for rule in defaults.base_filter_rules:
+        for rule in base_filter_rules:
             # Check no leading or trailing white space.
             self.assertEquals(rule, rule.strip())
             # All categories are on by default, so defaults should
@@ -161,37 +164,60 @@ class GlobalVariablesTest(unittest.TestCase):
 
     def test_defaults(self):
         """Check that default arguments are valid."""
-        defaults = self.defaults()
+        default_options = self.defaults()
 
         # FIXME: We should not need to call parse() to determine
         #        whether the default arguments are valid.
-        parser = style.ArgumentParser(defaults)
+        parser = style.ArgumentParser(default_options=default_options)
         # No need to test the return value here since we test parse()
         # on valid arguments elsewhere.
         parser.parse([]) # arguments valid: no error or SystemExit
 
     def test_path_rules_specifier(self):
-        all_categories = style_categories()
+        all_categories = self._all_categories()
         for (sub_paths, path_rules) in PATH_RULES_SPECIFIER:
-            self.assertTrue(isinstance(path_rules, tuple),
-                            "Checking: " + str(path_rules))
             validate_filter_rules(path_rules, self._all_categories())
 
-        # Try using the path specifier (as an "end-to-end" check).
         config = FilterConfiguration(path_specific=PATH_RULES_SPECIFIER)
-        self.assertTrue(config.should_check("xxx_any_category",
-                                            "xxx_non_matching_path"))
-        self.assertTrue(config.should_check("xxx_any_category",
-                                            "WebKitTools/WebKitAPITest/"))
-        self.assertFalse(config.should_check("build/include",
-                                             "WebKitTools/WebKitAPITest/"))
-        self.assertFalse(config.should_check("readability/naming",
-                             "WebKit/qt/tests/qwebelement/tst_qwebelement.cpp"))
+
+        def assertCheck(path, category):
+            """Assert that the given category should be checked."""
+            message = ('Should check category "%s" for path "%s".'
+                       % (category, path))
+            self.assertTrue(config.should_check(category, path))
+
+        def assertNoCheck(path, category):
+            """Assert that the given category should not be checked."""
+            message = ('Should not check category "%s" for path "%s".'
+                       % (category, path))
+            self.assertFalse(config.should_check(category, path), message)
+
+        assertCheck("random_path.cpp",
+                    "build/include")
+        assertNoCheck("WebKitTools/WebKitAPITest/main.cpp",
+                      "build/include")
+        assertNoCheck("WebKit/qt/QGVLauncher/main.cpp",
+                      "build/include")
+        assertNoCheck("WebKit/qt/QGVLauncher/main.cpp",
+                      "readability/streams")
+
+        assertCheck("random_path.cpp",
+                    "readability/naming")
+        assertNoCheck("WebKit/gtk/webkit/webkit.h",
+                      "readability/naming")
+        assertNoCheck("WebCore/css/CSSParser.cpp",
+                      "readability/naming")
+        assertNoCheck("WebKit/qt/tests/qwebelement/tst_qwebelement.cpp",
+                      "readability/naming")
+        assertNoCheck(
+            "JavaScriptCore/qt/tests/qscriptengine/tst_qscriptengine.cpp",
+            "readability/naming")
+
 
     def test_max_reports_per_category(self):
-        """Check that MAX_REPORTS_PER_CATEGORY is valid."""
+        """Check that _MAX_REPORTS_PER_CATEGORY is valid."""
         all_categories = self._all_categories()
-        for category in style.MAX_REPORTS_PER_CATEGORY.iterkeys():
+        for category in _MAX_REPORTS_PER_CATEGORY.iterkeys():
             self.assertTrue(category in all_categories,
                             'Key "%s" is not a category' % category)
 
@@ -237,29 +263,29 @@ class ArgumentParserTest(unittest.TestCase):
         """Return a default parse() function for testing."""
         return self._create_parser().parse
 
-    def _create_defaults(self, default_output_format='vs7',
-                         default_verbosity=3,
-                         default_filter_rules=['-', '+whitespace']):
-        """Return a default ArgumentDefaults instance for testing."""
-        return style.ArgumentDefaults(default_output_format,
-                                      default_verbosity,
-                                      default_filter_rules)
+    def _create_defaults(self):
+        """Return a DefaultCommandOptionValues instance for testing."""
+        base_filter_rules = ["-", "+whitespace"]
+        return DefaultCommandOptionValues(base_filter_rules=base_filter_rules,
+                                          output_format="vs7",
+                                          verbosity=3)
 
-    def _create_parser(self, defaults=None):
+    def _create_parser(self):
         """Return an ArgumentParser instance for testing."""
-        def create_usage(_defaults):
+        def create_usage(_default_options):
             """Return a usage string for testing."""
             return "usage"
 
-        def doc_print(message):
+        def stderr_write(message):
             # We do not want the usage string or style categories
             # to print during unit tests, so print nothing.
             return
 
-        if defaults is None:
-            defaults = self._create_defaults()
+        default_options = self._create_defaults()
 
-        return style.ArgumentParser(defaults, create_usage, doc_print)
+        return style.ArgumentParser(create_usage=create_usage,
+                                    default_options=default_options,
+                                    stderr_write=stderr_write)
 
     def test_parse_documentation(self):
         parse = self._parse()
index 1940e03d2bbea03ef35be00d8f00dcf4a733f76a..d87df716277ef2dcf5fd765de6e74c96a68515ea 100644 (file)
@@ -153,10 +153,13 @@ class PatchStyleErrorHandler(object):
 
         """
         self._diff = diff
-        self._default_error_handler = DefaultStyleErrorHandler(file_path,
-                                          options,
-                                          increment_error_count,
-                                          stderr_write)
+
+        self._default_error_handler = DefaultStyleErrorHandler(
+                                          file_path=file_path,
+                                          increment_error_count=
+                                              increment_error_count,
+                                          options=options,
+                                          stderr_write=stderr_write)
 
         # The line numbers of the modified lines. This is set lazily.
         self._line_numbers = set()
index 1d7e998c2d5d4cff6cc7426ffdb0883a42060390..43a11fe5bf4be94a491f26404e7456a5d670c01d 100644 (file)
@@ -34,46 +34,44 @@ from error_handlers import PatchStyleErrorHandler
 class StyleErrorHandlerTestBase(unittest.TestCase):
 
     def setUp(self):
-        self._error_messages = ""
+        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
+        self._error_messages.append(message)
 
 
 class DefaultStyleErrorHandlerTest(StyleErrorHandlerTestBase):
 
     """Tests DefaultStyleErrorHandler class."""
 
-    _file_path = "foo.h"
-
     _category = "whitespace/tab"
+    """The category name for the tests in this class."""
 
-    def _error_handler(self, options):
-        return DefaultStyleErrorHandler(self._file_path,
-                                        options,
-                                        self._mock_increment_error_count,
-                                        self._mock_stderr_write)
+    _file_path = "foo.h"
+    """The file path for the tests in this class."""
 
     def _check_initialized(self):
         """Check that count and error messages are initialized."""
         self.assertEquals(0, self._error_count)
-        self.assertEquals("", self._error_messages)
-
-    def _call(self, handle_error, options, confidence):
-        """Handle an error with the given error handler."""
-        line_number = 100
-        message = "message"
+        self.assertEquals(0, len(self._error_messages))
 
-        handle_error(line_number, self._category, confidence, message)
-
-    def _call_error_handler(self, options, confidence):
-        """Handle an error using a new error handler."""
-        handle_error = self._error_handler(options)
-        self._call(handle_error, options, confidence)
+    def _error_handler(self, options):
+        return DefaultStyleErrorHandler(file_path=self._file_path,
+                                        increment_error_count=
+                                            self._mock_increment_error_count,
+                                        options=options,
+                                        stderr_write=self._mock_stderr_write)
+
+    def _call_error_handler(self, handle_error, confidence):
+        """Call the given error handler with a test error."""
+        handle_error(line_number=100,
+                     category=self._category,
+                     confidence=confidence,
+                     message="message")
 
     def test_call_non_reportable(self):
         """Test __call__() method with a non-reportable error."""
@@ -86,10 +84,11 @@ class DefaultStyleErrorHandlerTest(StyleErrorHandlerTestBase):
                                                confidence,
                                                self._file_path))
 
-        self._call_error_handler(options, confidence)
+        error_handler = self._error_handler(options)
+        self._call_error_handler(error_handler, confidence)
 
         self.assertEquals(0, self._error_count)
-        self.assertEquals("", self._error_messages)
+        self.assertEquals([], self._error_messages)
 
     def test_call_reportable_emacs(self):
         """Test __call__() method with a reportable error and emacs format."""
@@ -97,10 +96,11 @@ class DefaultStyleErrorHandlerTest(StyleErrorHandlerTestBase):
         options = ProcessorOptions(verbosity=3, output_format="emacs")
         self._check_initialized()
 
-        self._call_error_handler(options, confidence)
+        error_handler = self._error_handler(options)
+        self._call_error_handler(error_handler, confidence)
 
         self.assertEquals(1, self._error_count)
-        self.assertEquals(self._error_messages,
+        self.assertEquals(self._error_messages[-1],
                           "foo.h:100:  message  [whitespace/tab] [5]\n")
 
     def test_call_reportable_vs7(self):
@@ -109,10 +109,11 @@ class DefaultStyleErrorHandlerTest(StyleErrorHandlerTestBase):
         options = ProcessorOptions(verbosity=3, output_format="vs7")
         self._check_initialized()
 
-        self._call_error_handler(options, confidence)
+        error_handler = self._error_handler(options)
+        self._call_error_handler(error_handler, confidence)
 
         self.assertEquals(1, self._error_count)
-        self.assertEquals(self._error_messages,
+        self.assertEquals(self._error_messages[-1],
                           "foo.h(100):  message  [whitespace/tab] [5]\n")
 
     def test_call_max_reports_per_category(self):
@@ -125,25 +126,25 @@ class DefaultStyleErrorHandlerTest(StyleErrorHandlerTestBase):
         self._check_initialized()
 
         # First call: usual reporting.
-        self._call(error_handler, options, confidence)
+        self._call_error_handler(error_handler, confidence)
         self.assertEquals(1, self._error_count)
-        self.assertEquals(self._error_messages,
+        self.assertEquals(1, len(self._error_messages))
+        self.assertEquals(self._error_messages[-1],
                           "foo.h:100:  message  [whitespace/tab] [5]\n")
 
         # Second call: suppression message reported.
-        self._error_messages = ""
-        self._call(error_handler, options, confidence)
+        self._call_error_handler(error_handler, confidence)
         self.assertEquals(2, self._error_count)
-        self.assertEquals(self._error_messages,
+        self.assertEquals(2, len(self._error_messages))
+        self.assertEquals(self._error_messages[-1],
                           "foo.h:100:  message  [whitespace/tab] [5]\n"
                           "Suppressing further [%s] reports for this file.\n"
                           % self._category)
 
         # Third call: no report.
-        self._error_messages = ""
-        self._call(error_handler, options, confidence)
+        self._call_error_handler(error_handler, confidence)
         self.assertEquals(3, self._error_count)
-        self.assertEquals(self._error_messages, "")
+        self.assertEquals(2, len(self._error_messages))
 
 
 class PatchStyleErrorHandlerTest(StyleErrorHandlerTestBase):
index 19c2f4d4fc619a29d28b0d7f5fa1db1a31eec25f..2bd1ad750397ef0c110709a79672fec683d3fe0c 100644 (file)
@@ -139,12 +139,8 @@ class FilterConfiguration(object):
                          are appended.  The first substring match takes
                          precedence, i.e. only the first match triggers
                          an append.
-                             The "path_rules" value is the tuple of filter
+                             The "path_rules" value is a list of filter
                          rules that can be appended to the base rules.
-                         The value is a tuple rather than a list so it
-                         can be used as a dictionary key.  The dictionary
-                         is for caching purposes in the implementation of
-                         this class.
 
           user_rules: A list of filter rules that is always appended
                       to the base rules and any path rules.  In other
@@ -210,16 +206,28 @@ class FilterConfiguration(object):
         return self._path_specific_lower
 
     def _path_rules_from_path(self, path):
-        """Determine the path-specific rules to use, and return as a tuple."""
+        """Determine the path-specific rules to use, and return as a tuple.
+
+         This method returns a tuple rather than a list so the return
+         value can be passed to _filter_from_path_rules() without change.
+
+        """
         path = path.lower()
         for (sub_paths, path_rules) in self._get_path_specific_lower():
             for sub_path in sub_paths:
                 if path.find(sub_path) > -1:
-                    return path_rules
+                    return tuple(path_rules)
         return () # Default to the empty tuple.
 
     def _filter_from_path_rules(self, path_rules):
-        """Return the CategoryFilter associated to a path rules tuple."""
+        """Return the CategoryFilter associated to the given path rules.
+
+        Args:
+          path_rules: A tuple of path rules.  We require a tuple rather
+                      than a list so the value can be used as a dictionary
+                      key in self._path_rules_to_filter.
+
+        """
         # We reuse the same CategoryFilter where possible to take
         # advantage of the caching they do.
         if path_rules not in self._path_rules_to_filter:
index 84760a580acd6a63ff0ed553d17e48fb94d3c945..76f25db447052e26182db08f314e9285ed1aba3f 100644 (file)
@@ -157,7 +157,7 @@ class FilterConfigurationTest(unittest.TestCase):
         # Test that the attributes are getting set correctly.
         # We use parameter values that are different from the defaults.
         base_rules = ["-"]
-        path_specific = [(["path"], ("+a",))]
+        path_specific = [(["path"], ["+a"])]
         user_rules = ["+"]
 
         config = self._config(base_rules, path_specific, user_rules)
@@ -185,7 +185,7 @@ class FilterConfigurationTest(unittest.TestCase):
 
         # These parameter values are different from the defaults.
         base_rules = ["-"]
-        path_specific = [(["path"], ("+a",))]
+        path_specific = [(["path"], ["+a"])]
         user_rules = ["+"]
 
         self.assertFalse(config.__eq__(FilterConfiguration(
@@ -219,8 +219,8 @@ class FilterConfigurationTest(unittest.TestCase):
     def test_path_specific(self):
         """Test effect of path_rules_specifier on should_check()."""
         base_rules = ["-"]
-        path_specific = [(["path1"], ("+b",)),
-                         (["path2"], ("+c",))]
+        path_specific = [(["path1"], ["+b"]),
+                         (["path2"], ["+c"])]
         user_rules = []
 
         config = self._config(base_rules, path_specific, user_rules)
@@ -233,7 +233,7 @@ class FilterConfigurationTest(unittest.TestCase):
     def test_path_with_different_case(self):
         """Test a path that differs only in case."""
         base_rules = ["-"]
-        path_specific = [(["Foo/"], ("+whitespace",))]
+        path_specific = [(["Foo/"], ["+whitespace"])]
         user_rules = []
 
         config = self._config(base_rules, path_specific, user_rules)