[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 00:55:34 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit 79fcb11bf75b75858eaf6a2f26ed44179432e7fb
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Wed Jan 6 06:37:56 2010 +0000
2010-01-05 Chris Jerdonek <chris.jerdonek at gmail.com>
Reviewed by David Levin.
Refactored check-webkit-style's argument parser to not rely
on global state, and improved its error handling and unit
test coverage.
https://bugs.webkit.org/show_bug.cgi?id=32966
* Scripts/check-webkit-style:
- Adjusted to use new argument parser.
* Scripts/webkitpy/cpp_style.py:
- Changed _CppStyleState to accept an array of filter rules
instead of a comma-delimited string.
- Eliminated cpp_style._DEFAULT_FILTER_RULES.
- Eliminated cpp_style._USAGE.
* Scripts/webkitpy/cpp_style_unittest.py:
- Updated test_filter() and test_default_filter().
* Scripts/webkitpy/style.py:
- Converted style._USAGE to create_usage().
- Corrected usage instructions by removing 0 as a valid
--verbose flag value.
- Removed use_webkit_styles().
- Added ProcessorOptions class.
- Added ArgumentDefaults class.
- Added ArgumentPrinter class.
- Removed parse_arguments and added ArgumentParser class.
- Moved exit_with_usage() and exit_with_categories() into
ArgumentParser.
- Refactored parse_arguments() as ArgumentParser.parse().
- Improved parser error handling.
* Scripts/webkitpy/style_unittest.py:
- Added DefaultArgumentsTest class.
- Addressed FIXME to check style.WEBKIT_FILTER_RULES
against style.STYLE_CATEGORIES.
- Added ArgumentPrinterTest class.
- Added ArgumentParserTest class and rewrote parser unit tests.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@52849 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index d33f4c0..8d884a8 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,46 @@
+2010-01-05 Chris Jerdonek <chris.jerdonek at gmail.com>
+
+ Reviewed by David Levin.
+
+ Refactored check-webkit-style's argument parser to not rely
+ on global state, and improved its error handling and unit
+ test coverage.
+
+ https://bugs.webkit.org/show_bug.cgi?id=32966
+
+ * Scripts/check-webkit-style:
+ - Adjusted to use new argument parser.
+
+ * Scripts/webkitpy/cpp_style.py:
+ - Changed _CppStyleState to accept an array of filter rules
+ instead of a comma-delimited string.
+ - Eliminated cpp_style._DEFAULT_FILTER_RULES.
+ - Eliminated cpp_style._USAGE.
+
+ * Scripts/webkitpy/cpp_style_unittest.py:
+ - Updated test_filter() and test_default_filter().
+
+ * Scripts/webkitpy/style.py:
+ - Converted style._USAGE to create_usage().
+ - Corrected usage instructions by removing 0 as a valid
+ --verbose flag value.
+ - Removed use_webkit_styles().
+ - Added ProcessorOptions class.
+ - Added ArgumentDefaults class.
+ - Added ArgumentPrinter class.
+ - Removed parse_arguments and added ArgumentParser class.
+ - Moved exit_with_usage() and exit_with_categories() into
+ ArgumentParser.
+ - Refactored parse_arguments() as ArgumentParser.parse().
+ - Improved parser error handling.
+
+ * Scripts/webkitpy/style_unittest.py:
+ - Added DefaultArgumentsTest class.
+ - Addressed FIXME to check style.WEBKIT_FILTER_RULES
+ against style.STYLE_CATEGORIES.
+ - Added ArgumentPrinterTest class.
+ - Added ArgumentParserTest class and rewrote parser unit tests.
+
2010-01-05 Adam Roben <aroben at apple.com>
Test that it's safe to call IWebView::close when
diff --git a/WebKitTools/Scripts/check-webkit-style b/WebKitTools/Scripts/check-webkit-style
index 08983d8..1cd0613 100755
--- a/WebKitTools/Scripts/check-webkit-style
+++ b/WebKitTools/Scripts/check-webkit-style
@@ -1,6 +1,7 @@
#!/usr/bin/env python
#
# Copyright (C) 2009 Google Inc. All rights reserved.
+# Copyright (C) 2010 Chris Jerdonek (chris.jerdonek at gmail.com)
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are
@@ -51,11 +52,6 @@ from webkitpy.scm import detect_scm_system
def main():
- style.use_webkit_styles()
-
- (files, flags) = style.parse_arguments(sys.argv[1:], ["git-commit="],
- display_help=True)
-
# Change stderr to write with replacement characters so we don't die
# if we try to print something containing non-ASCII characters.
sys.stderr = codecs.StreamReaderWriter(sys.stderr,
@@ -63,10 +59,16 @@ def main():
codecs.getwriter('utf8'),
'replace')
- if files and "--git-commit" in flags:
- style.exit_with_usage('It is not possible to check files and a '
- 'specific commit at the same time.',
- display_help=True)
+ defaults = style.ArgumentDefaults(style.DEFAULT_OUTPUT_FORMAT,
+ style.DEFAULT_VERBOSITY,
+ style.WEBKIT_FILTER_RULES)
+
+ parser = style.ArgumentParser(defaults)
+ (files, options) = parser.parse(sys.argv[1:])
+
+ # FIXME: Eliminate the need to call this function.
+ # Options should be passed into process_file instead.
+ style.set_options(options)
if files:
for filename in files:
@@ -76,8 +78,8 @@ def main():
cwd = os.path.abspath('.')
scm = detect_scm_system(cwd)
- if "--git-commit" in flags:
- commit = flags["--git-commit"]
+ if options.git_commit:
+ commit = options.git_commit
if '..' in commit:
# FIXME: If the range is a "...", the code should find the common ancestor and
# start there (see git diff --help for information about how ... usually works).
diff --git a/WebKitTools/Scripts/webkitpy/cpp_style.py b/WebKitTools/Scripts/webkitpy/cpp_style.py
index 2d95d3e..8857572 100644
--- a/WebKitTools/Scripts/webkitpy/cpp_style.py
+++ b/WebKitTools/Scripts/webkitpy/cpp_style.py
@@ -47,15 +47,6 @@ import sys
import unicodedata
-_USAGE = ''
-
-
-# The default state of the category filter. This is overrided by the --filter=
-# flag. By default all errors are on, so only add here categories that should be
-# off by default (i.e., categories that must be enabled by the --filter= flags).
-# All entries here should start with a '-' or '+', as in the --filter= flag.
-_DEFAULT_FILTER_RULES = []
-
# Headers that we consider STL headers.
_STL_HEADERS = frozenset([
'algobase.h', 'algorithm', 'alloc.h', 'bitset', 'deque', 'exception',
@@ -258,7 +249,7 @@ class _CppStyleState(object):
self.verbose_level = 1 # global setting.
self.error_count = 0 # global count of reported errors
# filters to apply when emitting error messages
- self.filters = _DEFAULT_FILTER_RULES[:]
+ self.filters = []
# output format:
# "emacs" - format that emacs can parse (default)
@@ -282,16 +273,18 @@ class _CppStyleState(object):
error message.
Args:
- filters: A string of comma-separated filters (eg "+whitespace/indent").
- Each filter should start with + or -; else we die.
+ 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: The comma-separated filters did not all start with '+' or '-'.
- E.g. "-,+whitespace,-whitespace/indent,whitespace/badfilter"
+ ValueError: Not all filters started with '+' or '-'. For example,
+ "-,+whitespace,-whitespace/indent,whitespace/badfilter"
"""
- # Default filters always have less priority than the flag ones.
- self.filters = _DEFAULT_FILTER_RULES[:]
- for filter in filters.split(','):
+ self.filters = []
+ for filter in filters:
clean_filter = filter.strip()
if clean_filter:
self.filters.append(clean_filter)
@@ -344,8 +337,11 @@ def _set_filters(filters):
error message.
Args:
- filters: A string of comma-separated filters (eg "whitespace/indent").
- Each filter should start with + or -; else we die.
+ 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.
"""
_cpp_style_state.set_filters(filters)
diff --git a/WebKitTools/Scripts/webkitpy/cpp_style_unittest.py b/WebKitTools/Scripts/webkitpy/cpp_style_unittest.py
index 0effb75..a420019 100644
--- a/WebKitTools/Scripts/webkitpy/cpp_style_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/cpp_style_unittest.py
@@ -41,14 +41,16 @@ import random
import re
import unittest
import cpp_style
-# FIXME: Remove the need to import something from style.
-from style import _STYLE_CATEGORIES
+# FIXME: Remove the need to import something from style. See the
+# FIXME notes near the STYLE_CATEGORIES definition for a
+# suggestion on how to best do this.
+from style import STYLE_CATEGORIES
# This class works as an error collector and replaces cpp_style.Error
# function for the unit tests. We also verify each category we see
-# is in cpp_style._STYLE_CATEGORIES, to help keep that list up to date.
+# is in STYLE_CATEGORIES, to help keep that list up to date.
class ErrorCollector:
- _all_style_categories = _STYLE_CATEGORIES
+ _all_style_categories = STYLE_CATEGORIES
# This a list including all categories seen in any unit test.
_seen_style_categories = {}
@@ -61,7 +63,7 @@ class ErrorCollector:
category, confidence, message):
self._assert_fn(category in self._all_style_categories,
'Message "%s" has category "%s",'
- ' which is not in _STYLE_CATEGORIES' % (message, category))
+ ' which is not in STYLE_CATEGORIES' % (message, category))
self._seen_style_categories[category] = 1
if cpp_style._should_print_error(category, confidence):
self._errors.append('%s [%s] [%d]' % (message, category, confidence))
@@ -1571,7 +1573,7 @@ class CppStyleTest(CppStyleTestBase):
def test_filter(self):
old_filters = cpp_style._cpp_style_state.filters
try:
- cpp_style._cpp_style_state.set_filters('-,+whitespace,-whitespace/indent')
+ cpp_style._cpp_style_state.set_filters(['-', '+whitespace', '-whitespace/indent'])
self.assert_lint(
'// Hello there ',
'Line ends in whitespace. Consider deleting these extra spaces.'
@@ -1581,15 +1583,13 @@ class CppStyleTest(CppStyleTestBase):
finally:
cpp_style._cpp_style_state.filters = old_filters
- def test_default_filter(self):
- default_filter_rules = cpp_style._DEFAULT_FILTER_RULES
+ def test_filter_appending(self):
old_filters = cpp_style._cpp_style_state.filters
- cpp_style._DEFAULT_FILTER_RULES = [ '-whitespace' ]
try:
# Reset filters
- cpp_style._cpp_style_state.set_filters('')
+ cpp_style._cpp_style_state.set_filters(['-whitespace'])
self.assert_lint('// Hello there ', '')
- cpp_style._cpp_style_state.set_filters('+whitespace/end_of_line')
+ cpp_style._cpp_style_state.set_filters(['-whitespace', '+whitespace/end_of_line'])
self.assert_lint(
'// Hello there ',
'Line ends in whitespace. Consider deleting these extra spaces.'
@@ -1597,7 +1597,6 @@ class CppStyleTest(CppStyleTestBase):
self.assert_lint(' weird opening space', '')
finally:
cpp_style._cpp_style_state.filters = old_filters
- cpp_style._DEFAULT_FILTER_RULES = default_filter_rules
def test_unnamed_namespaces_in_headers(self):
self.assert_language_rules_check(
diff --git a/WebKitTools/Scripts/webkitpy/style.py b/WebKitTools/Scripts/webkitpy/style.py
index 58c5163..20ebe25 100644
--- a/WebKitTools/Scripts/webkitpy/style.py
+++ b/WebKitTools/Scripts/webkitpy/style.py
@@ -1,4 +1,5 @@
# Copyright (C) 2009 Google Inc. All rights reserved.
+# Copyright (C) 2010 Chris Jerdonek (chris.jerdonek at gmail.com)
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are
@@ -29,7 +30,6 @@
"""Front end of some style-checker modules."""
# FIXME: Move more code from cpp_style to here.
-# check-webkit-style should not refer cpp_style directly.
import getopt
import os.path
@@ -40,22 +40,24 @@ import text_style
from diff_parser import DiffParser
-# Default options
-_DEFAULT_VERBOSITY = 1
-_DEFAULT_OUTPUT_FORMAT = 'emacs'
+# These defaults are used by check-webkit-style.
+DEFAULT_VERBOSITY = 1
+DEFAULT_OUTPUT_FORMAT = 'emacs'
# FIXME: For style categories we will never want to have, remove them.
# For categories for which we want to have similar functionality,
# modify the implementation and enable them.
-# FIXME: Add a unit test to ensure the corresponding categories
-# are elements of _STYLE_CATEGORIES.
#
-# For unambiguous terminology, we use "filter rule" rather than "filter"
-# 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 as specified by a --filter flag.
-_WEBKIT_FILTER_RULES = [
+# Throughout this module, we use "filter rule" rather than "filter"
+# 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_FILTER_RULES = [
'-build/endif_comment',
'-build/include_what_you_use', # <string> for std::string
'-build/storage_class', # const static
@@ -79,11 +81,20 @@ _WEBKIT_FILTER_RULES = [
]
+# FIXME: The STYLE_CATEGORIES show up in both file types cpp_style.py
+# and text_style.py. Break this list into possibly overlapping
+# sub-lists, and store each sub-list in the corresponding .py
+# file. The file style.py can obtain the master list by taking
+# the union. This will allow the unit tests for each file type
+# to check that all of their respective style categories are
+# represented -- without having to reference style.py or be
+# aware of the existence of other file types.
+#
# We categorize each style rule we print. Here are the categories.
-# We want an explicit list so we can list them all in cpp_style --filter=.
+# We want an explicit list so we can display a full list to the user.
# If you add a new error message with a new category, add it to the list
# here! cpp_style_unittest.py should tell you if you forget to do this.
-_STYLE_CATEGORIES = [
+STYLE_CATEGORIES = [
'build/class',
'build/deprecated',
'build/endif_comment',
@@ -147,7 +158,14 @@ _STYLE_CATEGORIES = [
]
-_USAGE = """
+def _create_usage(defaults):
+ """Return the usage string to display for command help.
+
+ Args:
+ defaults: An ArgumentDefaults instance.
+
+ """
+ usage = """
Syntax: %(program_name)s [--verbose=#] [--git-commit=<SingleCommit>] [--output=vs7]
[--filter=-x,+y,...] [file] ...
@@ -171,8 +189,9 @@ Syntax: %(program_name)s [--verbose=#] [--git-commit=<SingleCommit>] [--output=v
Flags:
verbose=#
- A number 0-5 to restrict errors to certain verbosity levels.
- Defaults to %(default_verbosity)s.
+ A number 1-5 that restricts output to errors with a confidence
+ score at or above this value. In particular, the value 1 displays
+ all errors. The default is %(default_verbosity)s.
git-commit=<SingleCommit>
Checks the style of everything from the given commit to the local tree.
@@ -204,108 +223,284 @@ Syntax: %(program_name)s [--verbose=#] [--git-commit=<SingleCommit>] [--output=v
%(program_name)s, along with which are enabled by default, pass
the empty filter as follows:
--filter=
-""" % {
- 'program_name': os.path.basename(sys.argv[0]),
- 'default_verbosity': _DEFAULT_VERBOSITY,
- 'default_output_format': _DEFAULT_OUTPUT_FORMAT
- }
+""" % {'program_name': os.path.basename(sys.argv[0]),
+ 'default_verbosity': defaults.verbosity,
+ 'default_output_format': defaults.output_format}
+ return usage
-def use_webkit_styles():
- """Configures this module for WebKit style."""
- cpp_style._DEFAULT_FILTER_RULES = _WEBKIT_FILTER_RULES
+# This class should not have knowledge of the flag key names.
+class ProcessorOptions(object):
-def exit_with_usage(error_message, display_help=False):
- """Exit and print a usage string with an optional error message.
+ """A container to store options to use when checking style.
- Args:
- error_message: The optional error message.
- display_help: Whether to display help output. Suppressing help
- output is useful for unit tests.
- """
- if display_help:
- sys.stderr.write(_USAGE)
- if error_message:
- sys.exit('\nFATAL ERROR: ' + error_message)
- else:
- sys.exit(1)
+ Attributes:
+ output_format: A string that is the output format. The supported
+ output formats are "emacs" which emacs can parse
+ and "vs7" which Microsoft Visual Studio 7 can parse.
+ verbosity: An integer 1-5 that restricts output to errors with a
+ confidence score at or above this value.
+ The default is 1, which displays all errors.
-def exit_with_categories(display_help=False):
- """Exit and print all style categories, along with the default filter.
+ 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.
- These category names appear in error messages. They can be filtered
- using the --filter flag.
+ git_commit: A string representing the git commit to check.
+ The default is None.
- Args:
- display_help: Whether to display help output. Suppressing help
- output is useful for unit tests.
+ extra_flag_values: A string-string dictionary of all flag key-value
+ pairs that are not otherwise represented by this
+ class. The default is the empty dictionary.
"""
- if display_help:
- sys.stderr.write('\nAll categories:\n')
- for category in sorted(_STYLE_CATEGORIES):
- sys.stderr.write(' ' + category + '\n')
- sys.stderr.write('\nDefault filter rules**:\n')
- for filter_rule in sorted(_WEBKIT_FILTER_RULES):
- sys.stderr.write(' ' + filter_rule + '\n')
- sys.stderr.write('\n**The command always evaluates the above '
- 'rules, and before any --filter flag.\n\n')
+ def __init__(self, output_format, verbosity=1, filter_rules=None,
+ git_commit=None, extra_flag_values=None):
+ if filter_rules is None:
+ filter_rules = []
+ if extra_flag_values is None:
+ extra_flag_values = {}
- sys.exit(0)
+ self.output_format = output_format
+ self.verbosity = verbosity
+ self.filter_rules = filter_rules
+ self.git_commit = git_commit
+ self.extra_flag_values = extra_flag_values
-def parse_arguments(args, additional_flags=[], display_help=False):
- """Parses the command line arguments.
+# FIXME: Eliminate the need for this function.
+# Options should be passed into process_file instead.
+def set_options(options):
+ """Initialize global _CppStyleState instance.
- This may set the output format and verbosity level as side-effects.
+ This needs to be called before calling process_file.
Args:
- args: The command line arguments:
- additional_flags: A list of strings which specifies flags we allow.
- display_help: Whether to display help output. Suppressing help
- output is useful for unit tests.
+ options: A ProcessorOptions instance.
+ """
+ cpp_style._set_output_format(options.output_format)
+ cpp_style._set_verbose_level(options.verbosity)
+ cpp_style._set_filters(options.filter_rules)
+
+
+# This class should not have knowledge of the flag key names.
+class ArgumentDefaults(object):
- Returns:
- A tuple of (filenames, flags)
+ """A container to store default argument values.
+
+ Attributes:
+ output_format: A string that is the default output format.
+ verbosity: An integer that is the default verbosity level.
+ filter_rules: A list of strings that are boolean filter rules
+ to prepend to any user-specified rules.
- filenames: The list of filenames to lint.
- flags: The dict of the flag names and the flag values.
"""
- flags = ['help', 'output=', 'verbose=', 'filter='] + additional_flags
- additional_flag_values = {}
- try:
- (opts, filenames) = getopt.getopt(args, '', flags)
- except getopt.GetoptError:
- exit_with_usage('Invalid arguments.', display_help)
-
- verbosity = _DEFAULT_VERBOSITY
- output_format = _DEFAULT_OUTPUT_FORMAT
- filters = ''
-
- for (opt, val) in opts:
- if opt == '--help':
- exit_with_usage(None, display_help)
- elif opt == '--output':
- if not val in ('emacs', 'vs7'):
- exit_with_usage('The only allowed output formats are emacs and vs7.',
- display_help)
- output_format = val
- elif opt == '--verbose':
- verbosity = int(val)
- elif opt == '--filter':
- filters = val
- if not filters:
- exit_with_categories(display_help)
- else:
- additional_flag_values[opt] = val
- cpp_style._set_output_format(output_format)
- cpp_style._set_verbose_level(verbosity)
- cpp_style._set_filters(filters)
+ def __init__(self, default_output_format, default_verbosity,
+ default_filter_rules):
+ self.output_format = default_output_format
+ self.verbosity = default_verbosity
+ self.filter_rules = default_filter_rules
+
+
+class ArgumentPrinter(object):
- return (filenames, additional_flag_values)
+ """Supports the printing of check-webkit-style command arguments."""
+
+ def _flag_pair_to_string(self, flag_key, flag_value):
+ return '--%(key)s=%(val)s' % {'key': flag_key, 'val': flag_value }
+
+ def to_flag_string(self, options):
+ """Return a flag string yielding the given ProcessorOptions instance.
+
+ This method orders the flag values alphabetically by the flag key.
+
+ Args:
+ options: A ProcessorOptions instance.
+
+ """
+ flags = options.extra_flag_values.copy()
+
+ flags['output'] = options.output_format
+ flags['verbose'] = options.verbosity
+ if options.filter_rules:
+ flags['filter'] = ','.join(options.filter_rules)
+ if options.git_commit:
+ flags['git-commit'] = options.git_commit
+
+ flag_string = ''
+ # Alphabetizing lets us unit test this method.
+ for key in sorted(flags.keys()):
+ flag_string += self._flag_pair_to_string(key, flags[key]) + ' '
+
+ return flag_string.strip()
+
+
+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().
+
+ """
+
+ def __init__(self, argument_defaults, create_usage=None, doc_print=None):
+ if create_usage is None:
+ create_usage = _create_usage
+ if doc_print is None:
+ doc_print = sys.stderr.write
+
+ self.defaults = argument_defaults
+ self.create_usage = create_usage
+ self.doc_print = doc_print
+
+ def _exit_with_usage(self, error_message=''):
+ """Exit and print a usage string with an optional error message.
+
+ Args:
+ error_message: A string that is an error message to print.
+ """
+ usage = self.create_usage(self.defaults)
+ self.doc_print(usage)
+ if error_message:
+ sys.exit('\nFATAL ERROR: ' + error_message)
+ else:
+ sys.exit(1)
+
+ def _exit_with_categories(self):
+ """Exit and print the style categories and default filter rules."""
+ self.doc_print('\nAll categories:\n')
+ for category in sorted(STYLE_CATEGORIES):
+ self.doc_print(' ' + category + '\n')
+
+ self.doc_print('\nDefault filter rules**:\n')
+ for filter_rule in sorted(self.defaults.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')
+
+ sys.exit(0)
+
+ def _parse_filter_flag(self, flag_value):
+ """Parse the value of the --filter flag.
+
+ These filters are applied when deciding whether to emit a given
+ error message.
+
+ Args:
+ flag_value: A string of comma-separated filter rules, for
+ example "-whitespace,+whitespace/indent".
+
+ """
+ filters = []
+ for uncleaned_filter in flag_value.split(','):
+ filter = uncleaned_filter.strip()
+ if not filter:
+ continue
+ filters.append(filter)
+ return filters
+
+ def parse(self, args, extra_flags=None):
+ """Parse the command line arguments to check-webkit-style.
+
+ Args:
+ args: A list of command-line arguments as returned by sys.argv[1:].
+ extra_flags: A list of flags whose values we want to extract, but
+ are not supported by the ProcessorOptions class.
+ An example flag "new_flag=". This defaults to the
+ empty list.
+
+ Returns:
+ A tuple of (filenames, options)
+
+ filenames: The list of filenames to check.
+ options: A ProcessorOptions instance.
+
+ """
+ if extra_flags is None:
+ extra_flags = []
+
+ output_format = self.defaults.output_format
+ verbosity = self.defaults.verbosity
+ filter_rules = self.defaults.filter_rules
+
+ # The flags already supported by the ProcessorOptions class.
+ flags = ['help', 'output=', 'verbose=', 'filter=', 'git-commit=']
+
+ for extra_flag in extra_flags:
+ if extra_flag in flags:
+ raise ValueError('Flag \'%(extra_flag)s is duplicated '
+ 'or already supported.' %
+ {'extra_flag': extra_flag})
+ flags.append(extra_flag)
+
+ try:
+ (opts, filenames) = getopt.getopt(args, '', flags)
+ except getopt.GetoptError:
+ # FIXME: Settle on an error handling approach: come up
+ # with a consistent guideline as to when and whether
+ # a ValueError should be raised versus calling
+ # sys.exit when needing to interrupt execution.
+ self._exit_with_usage('Invalid arguments.')
+
+ extra_flag_values = {}
+ git_commit = None
+
+ for (opt, val) in opts:
+ if opt == '--help':
+ self._exit_with_usage()
+ elif opt == '--output':
+ output_format = val
+ elif opt == '--verbose':
+ verbosity = val
+ elif opt == '--git-commit':
+ git_commit = val
+ elif opt == '--filter':
+ if not val:
+ self._exit_with_categories()
+ # Prepend the defaults.
+ filter_rules = filter_rules + self._parse_filter_flag(val)
+ else:
+ extra_flag_values[opt] = val
+
+ # Check validity of resulting values.
+ if filenames and (git_commit != None):
+ self._exit_with_usage('It is not possible to check files and a '
+ 'specific commit at the same time.')
+
+ if output_format not in ('emacs', 'vs7'):
+ raise ValueError('Invalid --output value "%s": The only '
+ 'allowed output formats are emacs and vs7.' %
+ output_format)
+
+ verbosity = int(verbosity)
+ if ((verbosity < 1) or (verbosity > 5)):
+ 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)
+
+ options = ProcessorOptions(output_format, verbosity, filter_rules,
+ git_commit, extra_flag_values)
+
+ return (filenames, options)
def process_file(filename):
diff --git a/WebKitTools/Scripts/webkitpy/style_unittest.py b/WebKitTools/Scripts/webkitpy/style_unittest.py
index d446606..119706a 100644
--- a/WebKitTools/Scripts/webkitpy/style_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/style_unittest.py
@@ -4,6 +4,7 @@
# Copyright (C) 2009 Google Inc. All rights reserved.
# Copyright (C) 2009 Torch Mobile Inc.
# Copyright (C) 2009 Apple Inc. All rights reserved.
+# Copyright (C) 2010 Chris Jerdonek (chris.jerdonek at gmail.com)
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are
@@ -36,86 +37,176 @@
import unittest
import style
-# FIXME: Eliminate cpp_style dependency.
-import cpp_style
+
+
+class DefaultArgumentsTest(unittest.TestCase):
+
+ """Tests validity of default arguments used by check-webkit-style."""
+
+ def test_filter_rules(self):
+ already_seen = []
+ for rule in style.WEBKIT_FILTER_RULES:
+ # All categories are on by default, so defaults should
+ # begin with -.
+ self.assertTrue(rule.startswith('-'))
+ self.assertTrue(rule[1:] in style.STYLE_CATEGORIES)
+ 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)
+ # FIXME: We should not need to call parse() to determine
+ # whether the default arguments are valid.
+ parser = style.ArgumentParser(defaults)
+ # No need to test the return value here since we test parse()
+ # on valid arguments elsewhere.
+ parser.parse([]) # arguments valid: no error or SystemExit
+
+
+class ArgumentPrinterTest(unittest.TestCase):
+
+ """Tests the ArgumentPrinter class."""
+
+ _printer = style.ArgumentPrinter()
+
+ 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,
+ git_commit, extra_flag_values)
+
+ def test_to_flag_string(self):
+ options = self._create_options('vs7', 5, ['+foo', '-bar'], 'git',
+ {'a': 0, 'z': 1})
+ self.assertEquals('--a=0 --filter=+foo,-bar --git-commit=git '
+ '--output=vs7 --verbose=5 --z=1',
+ self._printer.to_flag_string(options))
+
+ # This is to check that --filter and --git-commit do not
+ # show up when not user-specified.
+ options = self._create_options()
+ self.assertEquals('--output=emacs --verbose=3',
+ self._printer.to_flag_string(options))
class ArgumentParserTest(unittest.TestCase):
- """Tests argument parsing."""
- def test_parse_arguments(self):
- old_usage = style._USAGE
- old_style_categories = style._STYLE_CATEGORIES
- old_webkit_filter_rules = style._WEBKIT_FILTER_RULES
- old_output_format = cpp_style._cpp_style_state.output_format
- old_verbose_level = cpp_style._cpp_style_state.verbose_level
- old_filters = cpp_style._cpp_style_state.filters
- try:
- # Don't print usage during the tests, or filter categories
- style._USAGE = ''
- style._STYLE_CATEGORIES = []
- style._WEBKIT_FILTER_RULES = []
-
- self.assertRaises(SystemExit, style.parse_arguments, ['--badopt'])
- self.assertRaises(SystemExit, style.parse_arguments, ['--help'])
- self.assertRaises(SystemExit, style.parse_arguments, ['--filter='])
- # This is illegal because all filters must start with + or -
- self.assertRaises(ValueError, style.parse_arguments, ['--filter=foo'])
- self.assertRaises(ValueError, style.parse_arguments,
- ['--filter=+a,b,-c'])
-
- self.assertEquals((['foo.cpp'], {}), style.parse_arguments(['foo.cpp']))
- self.assertEquals(old_output_format, cpp_style._cpp_style_state.output_format)
- self.assertEquals(old_verbose_level, cpp_style._cpp_style_state.verbose_level)
-
- self.assertEquals(([], {}), style.parse_arguments([]))
- self.assertEquals(([], {}), style.parse_arguments(['--v=0']))
-
- self.assertEquals((['foo.cpp'], {}),
- style.parse_arguments(['--v=1', 'foo.cpp']))
- self.assertEquals(1, cpp_style._cpp_style_state.verbose_level)
- self.assertEquals((['foo.h'], {}),
- style.parse_arguments(['--v=3', 'foo.h']))
- self.assertEquals(3, cpp_style._cpp_style_state.verbose_level)
- self.assertEquals((['foo.cpp'], {}),
- style.parse_arguments(['--verbose=5', 'foo.cpp']))
- self.assertEquals(5, cpp_style._cpp_style_state.verbose_level)
- self.assertRaises(ValueError,
- style.parse_arguments, ['--v=f', 'foo.cpp'])
-
- self.assertEquals((['foo.cpp'], {}),
- style.parse_arguments(['--output=emacs', 'foo.cpp']))
- self.assertEquals('emacs', cpp_style._cpp_style_state.output_format)
- self.assertEquals((['foo.h'], {}),
- style.parse_arguments(['--output=vs7', 'foo.h']))
- self.assertEquals('vs7', cpp_style._cpp_style_state.output_format)
- self.assertRaises(SystemExit,
- style.parse_arguments, ['--output=blah', 'foo.cpp'])
-
- filt = '-,+whitespace,-whitespace/indent'
- self.assertEquals((['foo.h'], {}),
- style.parse_arguments(['--filter='+filt, 'foo.h']))
- self.assertEquals(['-', '+whitespace', '-whitespace/indent'],
- cpp_style._cpp_style_state.filters)
-
- self.assertEquals((['foo.cpp', 'foo.h'], {}),
- style.parse_arguments(['foo.cpp', 'foo.h']))
-
- self.assertEquals((['foo.cpp'], {'--foo': ''}),
- style.parse_arguments(['--foo', 'foo.cpp'], ['foo']))
- self.assertEquals((['foo.cpp'], {'--foo': 'bar'}),
- style.parse_arguments(['--foo=bar', 'foo.cpp'], ['foo=']))
- self.assertEquals((['foo.cpp'], {}),
- style.parse_arguments(['foo.cpp'], ['foo=']))
- self.assertRaises(SystemExit,
- style.parse_arguments,
- ['--footypo=bar', 'foo.cpp'], ['foo='])
- finally:
- style._USAGE = old_usage
- style._STYLE_CATEGORIES = old_style_categories
- style._WEBKIT_FILTER_RULES = old_webkit_filter_rules
- cpp_style._cpp_style_state.output_format = old_output_format
- cpp_style._cpp_style_state.verbose_level = old_verbose_level
- cpp_style._cpp_style_state.filters = old_filters
+
+ """Test the ArgumentParser class."""
+
+ def _parse(self):
+ """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_parser(self, defaults=None):
+ """"Return an ArgumentParser instance for testing."""
+ def create_usage(_defaults):
+ """Return a usage string for testing."""
+ return "usage"
+
+ def doc_print(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()
+
+ return style.ArgumentParser(defaults, create_usage, doc_print)
+
+ def test_parse_documentation(self):
+ parse = self._parse()
+
+ # FIXME: Test both the printing of the usage string and the
+ # filter categories help.
+
+ # Request the usage string.
+ self.assertRaises(SystemExit, parse, ['--help'])
+ # Request default filter rules and available style categories.
+ self.assertRaises(SystemExit, parse, ['--filter='])
+
+ def test_parse_bad_values(self):
+ parse = self._parse()
+
+ # Pass an unsupported argument.
+ self.assertRaises(SystemExit, parse, ['--bad'])
+
+ self.assertRaises(ValueError, parse, ['--verbose=bad'])
+ self.assertRaises(ValueError, parse, ['--verbose=0'])
+ self.assertRaises(ValueError, parse, ['--verbose=6'])
+ parse(['--verbose=1']) # works
+ parse(['--verbose=5']) # works
+
+ self.assertRaises(ValueError, parse, ['--output=bad'])
+ parse(['--output=vs7']) # works
+
+ # Pass a filter rule not beginning with + or -.
+ self.assertRaises(ValueError, parse, ['--filter=foo'])
+ parse(['--filter=+foo']) # works
+ # Pass files and git-commit at the same time.
+ self.assertRaises(SystemExit, parse, ['--git-commit=', 'file.txt'])
+ # Pass an extra flag already supported.
+ self.assertRaises(ValueError, parse, [], ['filter='])
+ parse([], ['extra=']) # works
+ # Pass an extra flag with typo.
+ self.assertRaises(SystemExit, parse, ['--extratypo='], ['extra='])
+ parse(['--extra='], ['extra=']) # works
+ self.assertRaises(ValueError, parse, [], ['extra=', 'extra='])
+
+
+ def test_parse_default_arguments(self):
+ parse = self._parse()
+
+ (files, options) = parse([])
+
+ self.assertEquals(files, [])
+
+ self.assertEquals(options.output_format, 'vs7')
+ self.assertEquals(options.verbosity, 3)
+ self.assertEquals(options.filter_rules, ['-', '+whitespace'])
+ self.assertEquals(options.git_commit, None)
+
+ def test_parse_explicit_arguments(self):
+ parse = self._parse()
+
+ # Pass non-default explicit values.
+ (files, options) = parse(['--output=emacs'])
+ self.assertEquals(options.output_format, 'emacs')
+ (files, options) = parse(['--verbose=4'])
+ self.assertEquals(options.verbosity, 4)
+ (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'])
+
+ # Pass extra flag values.
+ (files, options) = parse(['--extra'], ['extra'])
+ self.assertEquals(options.extra_flag_values, {'--extra': ''})
+ (files, options) = parse(['--extra='], ['extra='])
+ self.assertEquals(options.extra_flag_values, {'--extra': ''})
+ (files, options) = parse(['--extra=x'], ['extra='])
+ self.assertEquals(options.extra_flag_values, {'--extra': 'x'})
+
+ def test_parse_files(self):
+ parse = self._parse()
+
+ (files, options) = parse(['foo.cpp'])
+ self.assertEquals(files, ['foo.cpp'])
+
+ # Pass multiple files.
+ (files, options) = parse(['--output=emacs', 'foo.cpp', 'bar.cpp'])
+ self.assertEquals(files, ['foo.cpp', 'bar.cpp'])
if __name__ == '__main__':
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list