Changeset 52849 in webkit


Ignore:
Timestamp:
Jan 5, 2010 10:37:56 PM (14 years ago)
Author:
eric@webkit.org
Message:

2010-01-05 Chris Jerdonek <​chris.jerdonek@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.
Location:
trunk/WebKitTools
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebKitTools/ChangeLog

    r52829 r52849  
     12010-01-05  Chris Jerdonek  <chris.jerdonek@gmail.com>
     2
     3        Reviewed by David Levin.
     4
     5        Refactored check-webkit-style's argument parser to not rely
     6        on global state, and improved its error handling and unit
     7        test coverage.
     8
     9        https://bugs.webkit.org/show_bug.cgi?id=32966
     10
     11        * Scripts/check-webkit-style:
     12          - Adjusted to use new argument parser.
     13
     14        * Scripts/webkitpy/cpp_style.py:
     15          - Changed _CppStyleState to accept an array of filter rules
     16            instead of a comma-delimited string.
     17          - Eliminated cpp_style._DEFAULT_FILTER_RULES.
     18          - Eliminated cpp_style._USAGE.
     19
     20        * Scripts/webkitpy/cpp_style_unittest.py:
     21          - Updated test_filter() and test_default_filter().
     22
     23        * Scripts/webkitpy/style.py:
     24          - Converted style._USAGE to create_usage().
     25          - Corrected usage instructions by removing 0 as a valid
     26            --verbose flag value.
     27          - Removed use_webkit_styles().
     28          - Added ProcessorOptions class.
     29          - Added ArgumentDefaults class.
     30          - Added ArgumentPrinter class.
     31          - Removed parse_arguments and added ArgumentParser class.
     32          - Moved exit_with_usage() and exit_with_categories() into
     33            ArgumentParser.
     34          - Refactored parse_arguments() as ArgumentParser.parse().
     35          - Improved parser error handling.
     36
     37        * Scripts/webkitpy/style_unittest.py:
     38          - Added DefaultArgumentsTest class.
     39          - Addressed FIXME to check style.WEBKIT_FILTER_RULES
     40            against style.STYLE_CATEGORIES.
     41          - Added ArgumentPrinterTest class.
     42          - Added ArgumentParserTest class and rewrote parser unit tests.
     43
    1442010-01-05  Adam Roben  <aroben@apple.com>
    245

  • trunk/WebKitTools/Scripts/check-webkit-style

    r52703 r52849  
    22#
    33# Copyright (C) 2009 Google Inc. All rights reserved.
     4# Copyright (C) 2010 Chris Jerdonek (chris.jerdonek@gmail.com)
    45#
    56# Redistribution and use in source and binary forms, with or without
    … …  
    5253
    5354def main():
    54     style.use_webkit_styles()
    55 
    56     (files, flags) = style.parse_arguments(sys.argv[1:], ["git-commit="],
    57                                            display_help=True)
    58 
    5955    # Change stderr to write with replacement characters so we don't die
    6056    # if we try to print something containing non-ASCII characters.
    … …  
    6460                                           'replace')
    6561
    66     if files and "--git-commit" in flags:
    67         style.exit_with_usage('It is not possible to check files and a '
    68                               'specific commit at the same time.',
    69                               display_help=True)
     62    defaults = style.ArgumentDefaults(style.DEFAULT_OUTPUT_FORMAT,
     63                                      style.DEFAULT_VERBOSITY,
     64                                      style.WEBKIT_FILTER_RULES)
     65
     66    parser = style.ArgumentParser(defaults)
     67    (files, options) = parser.parse(sys.argv[1:])
     68
     69    # FIXME: Eliminate the need to call this function.
     70    #        Options should be passed into process_file instead.
     71    style.set_options(options)
    7072
    7173    if files:
    … …  
    7779        scm = detect_scm_system(cwd)
    7880
    79         if "--git-commit" in flags:
    80             commit = flags["--git-commit"]
     81        if options.git_commit:
     82            commit = options.git_commit
    8183            if '..' in commit:
    8284                # FIXME: If the range is a "...", the code should find the common ancestor and

  • trunk/WebKitTools/Scripts/webkitpy/cpp_style.py

    r52808 r52849  
    4848
    4949
    50 _USAGE = ''
    51 
    52 
    53 # The default state of the category filter. This is overrided by the --filter=
    54 # flag. By default all errors are on, so only add here categories that should be
    55 # off by default (i.e., categories that must be enabled by the --filter= flags).
    56 # All entries here should start with a '-' or '+', as in the --filter= flag.
    57 _DEFAULT_FILTER_RULES = []
    58 
    5950# Headers that we consider STL headers.
    6051_STL_HEADERS = frozenset([
    … …  
    259250        self.error_count = 0    # global count of reported errors
    260251        # filters to apply when emitting error messages
    261         self.filters = _DEFAULT_FILTER_RULES[:]
     252        self.filters = []
    262253
    263254        # output format:
    … …  
    283274
    284275        Args:
    285           filters: A string of comma-separated filters (eg "+whitespace/indent").
    286                    Each filter should start with + or -; else we die.
     276          filters: A list of strings that are boolean filter rules used
     277                   to determine whether a style category should be checked.
     278                   Each string should start with + or -. An example
     279                   string is "+whitespace/indent". The list includes any
     280                   prepended default filter rules.
    287281
    288282        Raises:
    289           ValueError: The comma-separated filters did not all start with '+' or '-'.
    290                       E.g. "-,+whitespace,-whitespace/indent,whitespace/badfilter"
     283          ValueError: Not all filters started with '+' or '-'. For example,
     284                      "-,+whitespace,-whitespace/indent,whitespace/badfilter"
    291285        """
    292         # Default filters always have less priority than the flag ones.
    293         self.filters = _DEFAULT_FILTER_RULES[:]
    294         for filter in filters.split(','):
     286        self.filters = []
     287        for filter in filters:
    295288            clean_filter = filter.strip()
    296289            if clean_filter:
    … …  
    345338
    346339    Args:
    347       filters: A string of comma-separated filters (eg "whitespace/indent").
    348                Each filter should start with + or -; else we die.
     340      filters: A list of strings that are boolean filter rules used
     341               to determine whether a style category should be checked.
     342               Each string should start with + or -. An example
     343               string is "+whitespace/indent". The list includes any
     344               prepended default filter rules.
    349345    """
    350346    _cpp_style_state.set_filters(filters)

  • trunk/WebKitTools/Scripts/webkitpy/cpp_style_unittest.py

    r52808 r52849  
    4242import unittest
    4343import cpp_style
    44 # FIXME: Remove the need to import something from style.
    45 from style import _STYLE_CATEGORIES
     44# FIXME: Remove the need to import something from style. See the
     45#        FIXME notes near the STYLE_CATEGORIES definition for a
     46#        suggestion on how to best do this.
     47from style import STYLE_CATEGORIES
    4648
    4749# This class works as an error collector and replaces cpp_style.Error
    4850# function for the unit tests.  We also verify each category we see
    49 # is in cpp_style._STYLE_CATEGORIES, to help keep that list up to date.
     51# is in STYLE_CATEGORIES, to help keep that list up to date.
    5052class ErrorCollector:
    51     _all_style_categories = _STYLE_CATEGORIES
     53    _all_style_categories = STYLE_CATEGORIES
    5254    # This a list including all categories seen in any unit test.
    5355    _seen_style_categories = {}
    … …  
    6264        self._assert_fn(category in self._all_style_categories,
    6365                        'Message "%s" has category "%s",'
    64                         ' which is not in _STYLE_CATEGORIES' % (message, category))
     66                        ' which is not in STYLE_CATEGORIES' % (message, category))
    6567        self._seen_style_categories[category] = 1
    6668        if cpp_style._should_print_error(category, confidence):
    … …  
    15721574        old_filters = cpp_style._cpp_style_state.filters
    15731575        try:
    1574             cpp_style._cpp_style_state.set_filters('-,+whitespace,-whitespace/indent')
     1576            cpp_style._cpp_style_state.set_filters(['-', '+whitespace', '-whitespace/indent'])
    15751577            self.assert_lint(
    15761578                '// Hello there ',
    … …  
    15821584            cpp_style._cpp_style_state.filters = old_filters
    15831585
    1584     def test_default_filter(self):
    1585         default_filter_rules = cpp_style._DEFAULT_FILTER_RULES
     1586    def test_filter_appending(self):
    15861587        old_filters = cpp_style._cpp_style_state.filters
    1587         cpp_style._DEFAULT_FILTER_RULES = [ '-whitespace' ]
    15881588        try:
    15891589            # Reset filters
    1590             cpp_style._cpp_style_state.set_filters('')
     1590            cpp_style._cpp_style_state.set_filters(['-whitespace'])
    15911591            self.assert_lint('// Hello there ', '')
    1592             cpp_style._cpp_style_state.set_filters('+whitespace/end_of_line')
     1592            cpp_style._cpp_style_state.set_filters(['-whitespace', '+whitespace/end_of_line'])
    15931593            self.assert_lint(
    15941594                '// Hello there ',
    … …  
    15981598        finally:
    15991599            cpp_style._cpp_style_state.filters = old_filters
    1600             cpp_style._DEFAULT_FILTER_RULES = default_filter_rules
    16011600
    16021601    def test_unnamed_namespaces_in_headers(self):

  • trunk/WebKitTools/Scripts/webkitpy/style.py

    r52541 r52849  
    11# Copyright (C) 2009 Google Inc. All rights reserved.
     2# Copyright (C) 2010 Chris Jerdonek (chris.jerdonek@gmail.com)
    23#
    34# Redistribution and use in source and binary forms, with or without
    … …  
    3031
    3132# FIXME: Move more code from cpp_style to here.
    32 # check-webkit-style should not refer cpp_style directly.
    3333
    3434import getopt
    … …  
    4141
    4242
    43 # Default options
    44 _DEFAULT_VERBOSITY = 1
    45 _DEFAULT_OUTPUT_FORMAT = 'emacs'
     43# These defaults are used by check-webkit-style.
     44DEFAULT_VERBOSITY = 1
     45DEFAULT_OUTPUT_FORMAT = 'emacs'
    4646
    4747
    … …  
    4949#        For categories for which we want to have similar functionality,
    5050#        modify the implementation and enable them.
    51 # FIXME: Add a unit test to ensure the corresponding categories
    52 #        are elements of _STYLE_CATEGORIES.
    5351#
    54 # For unambiguous terminology, we use "filter rule" rather than "filter"
    55 # for an individual boolean filter flag like "+foo". This allows us to
    56 # reserve "filter" for what one gets by collectively applying all of
    57 # the filter rules as specified by a --filter flag.
    58 _WEBKIT_FILTER_RULES = [
     52# Throughout this module, we use "filter rule" rather than "filter"
     53# for an individual boolean filter flag like "+foo". This allows us to
     54# reserve "filter" for what one gets by collectively applying all of
     55# the filter rules.
     56#
     57# The _WEBKIT_FILTER_RULES are prepended to any user-specified filter
     58# rules. Since by default all errors are on, only include rules that
     59# begin with a - sign.
     60WEBKIT_FILTER_RULES = [
    5961    '-build/endif_comment',
    6062    '-build/include_what_you_use',  # <string> for std::string
    … …  
    8082
    8183
     84# FIXME: The STYLE_CATEGORIES show up in both file types cpp_style.py
     85#        and text_style.py. Break this list into possibly overlapping
     86#        sub-lists, and store each sub-list in the corresponding .py
     87#        file. The file style.py can obtain the master list by taking
     88#        the union. This will allow the unit tests for each file type
     89#        to check that all of their respective style categories are
     90#        represented -- without having to reference style.py or be
     91#        aware of the existence of other file types.
     92#
    8293# We categorize each style rule we print.  Here are the categories.
    83 # We want an explicit list so we can list them all in cpp_style --filter=.
     94# We want an explicit list so we can display a full list to the user.
    8495# If you add a new error message with a new category, add it to the list
    8596# here!  cpp_style_unittest.py should tell you if you forget to do this.
    86 _STYLE_CATEGORIES = [
     97STYLE_CATEGORIES = [
    8798    'build/class',
    8899    'build/deprecated',
    … …  
    148159
    149160
    150 _USAGE = """
     161def _create_usage(defaults):
     162    """Return the usage string to display for command help.
     163
     164    Args:
     165      defaults: An ArgumentDefaults instance.
     166
     167    """
     168    usage = """
    151169Syntax: %(program_name)s [--verbose=#] [--git-commit=<SingleCommit>] [--output=vs7]
    152170        [--filter=-x,+y,...] [file] ...
    … …  
    172190
    173191    verbose=#
    174       A number 0-5 to restrict errors to certain verbosity levels.
    175       Defaults to %(default_verbosity)s.
     192      A number 1-5 that restricts output to errors with a confidence
     193      score at or above this value. In particular, the value 1 displays
     194      all errors. The default is %(default_verbosity)s.
    176195
    177196    git-commit=<SingleCommit>
    … …  
    205224      the empty filter as follows:
    206225         --filter=
    207 """ % {
    208     'program_name': os.path.basename(sys.argv[0]),
    209     'default_verbosity': _DEFAULT_VERBOSITY,
    210     'default_output_format': _DEFAULT_OUTPUT_FORMAT
    211     }
    212 
    213 
    214 def use_webkit_styles():
    215     """Configures this module for WebKit style."""
    216     cpp_style._DEFAULT_FILTER_RULES = _WEBKIT_FILTER_RULES
    217 
    218 
    219 def exit_with_usage(error_message, display_help=False):
    220     """Exit and print a usage string with an optional error message.
     226""" % {'program_name': os.path.basename(sys.argv[0]),
     227       'default_verbosity': defaults.verbosity,
     228       'default_output_format': defaults.output_format}
     229
     230    return usage
     231
     232
     233# This class should not have knowledge of the flag key names.
     234class ProcessorOptions(object):
     235
     236    """A container to store options to use when checking style.
     237
     238    Attributes:
     239      output_format: A string that is the output format. The supported
     240                     output formats are "emacs" which emacs can parse
     241                     and "vs7" which Microsoft Visual Studio 7 can parse.
     242
     243      verbosity: An integer 1-5 that restricts output to errors with a
     244                 confidence score at or above this value.
     245                 The default is 1, which displays all errors.
     246
     247      filter_rules: A list of strings that are boolean filter rules used
     248                    to determine whether a style category should be checked.
     249                    Each string should start with + or -. An example
     250                    string is "+whitespace/indent". The list includes any
     251                    prepended default filter rules. The default is the
     252                    empty list, which includes all categories.
     253
     254      git_commit: A string representing the git commit to check.
     255                  The default is None.
     256
     257      extra_flag_values: A string-string dictionary of all flag key-value
     258                         pairs that are not otherwise represented by this
     259                         class. The default is the empty dictionary.
     260    """
     261
     262    def __init__(self, output_format, verbosity=1, filter_rules=None,
     263                 git_commit=None, extra_flag_values=None):
     264        if filter_rules is None:
     265            filter_rules = []
     266        if extra_flag_values is None:
     267            extra_flag_values = {}
     268
     269        self.output_format = output_format
     270        self.verbosity = verbosity
     271        self.filter_rules = filter_rules
     272        self.git_commit = git_commit
     273        self.extra_flag_values = extra_flag_values
     274
     275
     276# FIXME: Eliminate the need for this function.
     277#        Options should be passed into process_file instead.
     278def set_options(options):
     279    """Initialize global _CppStyleState instance.
     280
     281    This needs to be called before calling process_file.
    221282
    222283    Args:
    223       error_message: The optional error message.
    224       display_help: Whether to display help output. Suppressing help
    225                     output is useful for unit tests.
    226     """
    227     if display_help:
    228         sys.stderr.write(_USAGE)
    229     if error_message:
    230         sys.exit('\nFATAL ERROR: ' + error_message)
    231     else:
    232         sys.exit(1)
    233 
    234 
    235 def exit_with_categories(display_help=False):
    236     """Exit and print all style categories, along with the default filter.
    237 
    238     These category names appear in error messages.  They can be filtered
    239     using the --filter flag.
    240 
    241     Args:
    242       display_help: Whether to display help output. Suppressing help
    243                     output is useful for unit tests.
    244     """
    245     if display_help:
    246         sys.stderr.write('\nAll categories:\n')
    247         for category in sorted(_STYLE_CATEGORIES):
    248             sys.stderr.write('    ' + category + '\n')
    249 
    250         sys.stderr.write('\nDefault filter rules**:\n')
    251         for filter_rule in sorted(_WEBKIT_FILTER_RULES):
    252             sys.stderr.write('    ' + filter_rule + '\n')
    253         sys.stderr.write('\n**The command always evaluates the above '
    254                          'rules, and before any --filter flag.\n\n')
    255 
    256     sys.exit(0)
    257 
    258 
    259 def parse_arguments(args, additional_flags=[], display_help=False):
    260     """Parses the command line arguments.
    261 
    262     This may set the output format and verbosity level as side-effects.
    263 
    264     Args:
    265       args: The command line arguments:
    266       additional_flags: A list of strings which specifies flags we allow.
    267       display_help: Whether to display help output. Suppressing help
    268                     output is useful for unit tests.
    269 
    270     Returns:
    271       A tuple of (filenames, flags)
    272 
    273       filenames: The list of filenames to lint.
    274       flags: The dict of the flag names and the flag values.
    275     """
    276     flags = ['help', 'output=', 'verbose=', 'filter='] + additional_flags
    277     additional_flag_values = {}
    278     try:
    279         (opts, filenames) = getopt.getopt(args, '', flags)
    280     except getopt.GetoptError:
    281         exit_with_usage('Invalid arguments.', display_help)
    282 
    283     verbosity = _DEFAULT_VERBOSITY
    284     output_format = _DEFAULT_OUTPUT_FORMAT
    285     filters = ''
    286 
    287     for (opt, val) in opts:
    288         if opt == '--help':
    289             exit_with_usage(None, display_help)
    290         elif opt == '--output':
    291             if not val in ('emacs', 'vs7'):
    292                 exit_with_usage('The only allowed output formats are emacs and vs7.',
    293                                 display_help)
    294             output_format = val
    295         elif opt == '--verbose':
    296             verbosity = int(val)
    297         elif opt == '--filter':
    298             filters = val
    299             if not filters:
    300                 exit_with_categories(display_help)
     284      options: A ProcessorOptions instance.
     285    """
     286    cpp_style._set_output_format(options.output_format)
     287    cpp_style._set_verbose_level(options.verbosity)
     288    cpp_style._set_filters(options.filter_rules)
     289
     290
     291# This class should not have knowledge of the flag key names.
     292class ArgumentDefaults(object):
     293
     294    """A container to store default argument values.
     295
     296    Attributes:
     297      output_format: A string that is the default output format.
     298      verbosity: An integer that is the default verbosity level.
     299      filter_rules: A list of strings that are boolean filter rules
     300                    to prepend to any user-specified rules.
     301
     302    """
     303
     304    def __init__(self, default_output_format, default_verbosity,
     305                 default_filter_rules):
     306        self.output_format = default_output_format
     307        self.verbosity = default_verbosity
     308        self.filter_rules = default_filter_rules
     309
     310
     311class ArgumentPrinter(object):
     312
     313    """Supports the printing of check-webkit-style command arguments."""
     314
     315    def _flag_pair_to_string(self, flag_key, flag_value):
     316        return '--%(key)s=%(val)s' % {'key': flag_key, 'val': flag_value }
     317
     318    def to_flag_string(self, options):
     319        """Return a flag string yielding the given ProcessorOptions instance.
     320
     321        This method orders the flag values alphabetically by the flag key.
     322
     323        Args:
     324          options: A ProcessorOptions instance.
     325
     326        """
     327        flags = options.extra_flag_values.copy()
     328
     329        flags['output'] = options.output_format
     330        flags['verbose'] = options.verbosity
     331        if options.filter_rules:
     332            flags['filter'] = ','.join(options.filter_rules)
     333        if options.git_commit:
     334            flags['git-commit'] = options.git_commit
     335
     336        flag_string = ''
     337        # Alphabetizing lets us unit test this method.
     338        for key in sorted(flags.keys()):
     339            flag_string += self._flag_pair_to_string(key, flags[key]) + ' '
     340
     341        return flag_string.strip()
     342
     343
     344class ArgumentParser(object):
     345
     346    """Supports the parsing of check-webkit-style command arguments.
     347
     348    Attributes:
     349      defaults: An ArgumentDefaults instance.
     350      create_usage: A function that accepts an ArgumentDefaults instance
     351                    and returns a string of usage instructions.
     352                    This defaults to the function used to generate the
     353                    usage string for check-webkit-style.
     354      doc_print: A function that accepts a string parameter and that is
     355                 called to display help messages. This defaults to
     356                 sys.stderr.write().
     357
     358    """
     359
     360    def __init__(self, argument_defaults, create_usage=None, doc_print=None):
     361        if create_usage is None:
     362            create_usage = _create_usage
     363        if doc_print is None:
     364            doc_print = sys.stderr.write
     365
     366        self.defaults = argument_defaults
     367        self.create_usage = create_usage
     368        self.doc_print = doc_print
     369
     370    def _exit_with_usage(self, error_message=''):
     371        """Exit and print a usage string with an optional error message.
     372
     373        Args:
     374          error_message: A string that is an error message to print.
     375        """
     376        usage = self.create_usage(self.defaults)
     377        self.doc_print(usage)
     378        if error_message:
     379            sys.exit('\nFATAL ERROR: ' + error_message)
    301380        else:
    302             additional_flag_values[opt] = val
    303 
    304     cpp_style._set_output_format(output_format)
    305     cpp_style._set_verbose_level(verbosity)
    306     cpp_style._set_filters(filters)
    307 
    308     return (filenames, additional_flag_values)
     381            sys.exit(1)
     382
     383    def _exit_with_categories(self):
     384        """Exit and print the style categories and default filter rules."""
     385        self.doc_print('\nAll categories:\n')
     386        for category in sorted(STYLE_CATEGORIES):
     387            self.doc_print('    ' + category + '\n')
     388
     389        self.doc_print('\nDefault filter rules**:\n')
     390        for filter_rule in sorted(self.defaults.filter_rules):
     391            self.doc_print('    ' + filter_rule + '\n')
     392        self.doc_print('\n**The command always evaluates the above rules, '
     393                       'and before any --filter flag.\n\n')
     394
     395        sys.exit(0)
     396
     397    def _parse_filter_flag(self, flag_value):
     398        """Parse the value of the --filter flag.
     399
     400        These filters are applied when deciding whether to emit a given
     401        error message.
     402
     403        Args:
     404          flag_value: A string of comma-separated filter rules, for
     405                      example "-whitespace,+whitespace/indent".
     406
     407        """
     408        filters = []
     409        for uncleaned_filter in flag_value.split(','):
     410            filter = uncleaned_filter.strip()
     411            if not filter:
     412                continue
     413            filters.append(filter)
     414        return filters
     415
     416    def parse(self, args, extra_flags=None):
     417        """Parse the command line arguments to check-webkit-style.
     418
     419        Args:
     420          args: A list of command-line arguments as returned by sys.argv[1:].
     421          extra_flags: A list of flags whose values we want to extract, but
     422                       are not supported by the ProcessorOptions class.
     423                       An example flag "new_flag=". This defaults to the
     424                       empty list.
     425
     426        Returns:
     427          A tuple of (filenames, options)
     428
     429          filenames: The list of filenames to check.
     430          options: A ProcessorOptions instance.
     431
     432        """
     433        if extra_flags is None:
     434            extra_flags = []
     435
     436        output_format = self.defaults.output_format
     437        verbosity = self.defaults.verbosity
     438        filter_rules = self.defaults.filter_rules
     439
     440        # The flags already supported by the ProcessorOptions class.
     441        flags = ['help', 'output=', 'verbose=', 'filter=', 'git-commit=']
     442
     443        for extra_flag in extra_flags:
     444            if extra_flag in flags:
     445                raise ValueError('Flag \'%(extra_flag)s is duplicated '
     446                                 'or already supported.' %
     447                                 {'extra_flag': extra_flag})
     448            flags.append(extra_flag)
     449
     450        try:
     451            (opts, filenames) = getopt.getopt(args, '', flags)
     452        except getopt.GetoptError:
     453            # FIXME: Settle on an error handling approach: come up
     454            #        with a consistent guideline as to when and whether
     455            #        a ValueError should be raised versus calling
     456            #        sys.exit when needing to interrupt execution.
     457            self._exit_with_usage('Invalid arguments.')
     458
     459        extra_flag_values = {}
     460        git_commit = None
     461
     462        for (opt, val) in opts:
     463            if opt == '--help':
     464                self._exit_with_usage()
     465            elif opt == '--output':
     466                output_format = val
     467            elif opt == '--verbose':
     468                verbosity = val
     469            elif opt == '--git-commit':
     470                git_commit = val
     471            elif opt == '--filter':
     472                if not val:
     473                    self._exit_with_categories()
     474                # Prepend the defaults.
     475                filter_rules = filter_rules + self._parse_filter_flag(val)
     476            else:
     477                extra_flag_values[opt] = val
     478
     479        # Check validity of resulting values.
     480        if filenames and (git_commit != None):
     481            self._exit_with_usage('It is not possible to check files and a '
     482                                  'specific commit at the same time.')
     483
     484        if output_format not in ('emacs', 'vs7'):
     485            raise ValueError('Invalid --output value "%s": The only '
     486                             'allowed output formats are emacs and vs7.' %
     487                             output_format)
     488
     489        verbosity = int(verbosity)
     490        if ((verbosity < 1) or (verbosity > 5)):
     491            raise ValueError('Invalid --verbose value %s: value must '
     492                             'be between 1-5.' % verbosity)
     493
     494        for rule in filter_rules:
     495            if not (rule.startswith('+') or rule.startswith('-')):
     496                raise ValueError('Invalid filter rule "%s": every rule '
     497                                 'rule in the --filter flag must start '
     498                                 'with + or -.' % rule)
     499
     500        options = ProcessorOptions(output_format, verbosity, filter_rules,
     501                                   git_commit, extra_flag_values)
     502
     503        return (filenames, options)
    309504
    310505

  • trunk/WebKitTools/Scripts/webkitpy/style_unittest.py

    r52583 r52849  
    55# Copyright (C) 2009 Torch Mobile Inc.
    66# Copyright (C) 2009 Apple Inc. All rights reserved.
     7# Copyright (C) 2010 Chris Jerdonek (chris.jerdonek@gmail.com)
    78#
    89# Redistribution and use in source and binary forms, with or without
    … …  
    3738
    3839import style
    39 # FIXME: Eliminate cpp_style dependency.
    40 import cpp_style
     40
     41
     42class DefaultArgumentsTest(unittest.TestCase):
     43
     44    """Tests validity of default arguments used by check-webkit-style."""
     45
     46    def test_filter_rules(self):
     47        already_seen = []
     48        for rule in style.WEBKIT_FILTER_RULES:
     49            # All categories are on by default, so defaults should
     50            # begin with -.
     51            self.assertTrue(rule.startswith('-'))
     52            self.assertTrue(rule[1:] in style.STYLE_CATEGORIES)
     53            self.assertFalse(rule in already_seen)
     54            already_seen.append(rule)
     55
     56    def test_defaults(self):
     57        """Check that default arguments are valid."""
     58        defaults = style.ArgumentDefaults(style.DEFAULT_OUTPUT_FORMAT,
     59                                          style.DEFAULT_VERBOSITY,
     60                                          style.WEBKIT_FILTER_RULES)
     61        # FIXME: We should not need to call parse() to determine
     62        #        whether the default arguments are valid.
     63        parser = style.ArgumentParser(defaults)
     64        # No need to test the return value here since we test parse()
     65        # on valid arguments elsewhere.
     66        parser.parse([]) # arguments valid: no error or SystemExit
     67
     68
     69class ArgumentPrinterTest(unittest.TestCase):
     70
     71    """Tests the ArgumentPrinter class."""
     72
     73    _printer = style.ArgumentPrinter()
     74
     75    def _create_options(self, output_format='emacs', verbosity=3,
     76                        filter_rules=[], git_commit=None,
     77                        extra_flag_values={}):
     78        return style.ProcessorOptions(output_format, verbosity, filter_rules,
     79                                      git_commit, extra_flag_values)
     80
     81    def test_to_flag_string(self):
     82        options = self._create_options('vs7', 5, ['+foo', '-bar'], 'git',
     83                                       {'a': 0, 'z': 1})
     84        self.assertEquals('--a=0 --filter=+foo,-bar --git-commit=git '
     85                          '--output=vs7 --verbose=5 --z=1',
     86                          self._printer.to_flag_string(options))
     87
     88        # This is to check that --filter and --git-commit do not
     89        # show up when not user-specified.
     90        options = self._create_options()
     91        self.assertEquals('--output=emacs --verbose=3',
     92                          self._printer.to_flag_string(options))
    4193
    4294
    4395class ArgumentParserTest(unittest.TestCase):
    44     """Tests argument parsing."""
    45     def test_parse_arguments(self):
    46         old_usage = style._USAGE
    47         old_style_categories = style._STYLE_CATEGORIES
    48         old_webkit_filter_rules = style._WEBKIT_FILTER_RULES
    49         old_output_format = cpp_style._cpp_style_state.output_format
    50         old_verbose_level = cpp_style._cpp_style_state.verbose_level
    51         old_filters = cpp_style._cpp_style_state.filters
    52         try:
    53             # Don't print usage during the tests, or filter categories
    54             style._USAGE = ''
    55             style._STYLE_CATEGORIES = []
    56             style._WEBKIT_FILTER_RULES = []
    57 
    58             self.assertRaises(SystemExit, style.parse_arguments, ['--badopt'])
    59             self.assertRaises(SystemExit, style.parse_arguments, ['--help'])
    60             self.assertRaises(SystemExit, style.parse_arguments, ['--filter='])
    61             # This is illegal because all filters must start with + or -
    62             self.assertRaises(ValueError, style.parse_arguments, ['--filter=foo'])
    63             self.assertRaises(ValueError, style.parse_arguments,
    64                               ['--filter=+a,b,-c'])
    65 
    66             self.assertEquals((['foo.cpp'], {}), style.parse_arguments(['foo.cpp']))
    67             self.assertEquals(old_output_format, cpp_style._cpp_style_state.output_format)
    68             self.assertEquals(old_verbose_level, cpp_style._cpp_style_state.verbose_level)
    69 
    70             self.assertEquals(([], {}), style.parse_arguments([]))
    71             self.assertEquals(([], {}), style.parse_arguments(['--v=0']))
    72 
    73             self.assertEquals((['foo.cpp'], {}),
    74                               style.parse_arguments(['--v=1', 'foo.cpp']))
    75             self.assertEquals(1, cpp_style._cpp_style_state.verbose_level)
    76             self.assertEquals((['foo.h'], {}),
    77                               style.parse_arguments(['--v=3', 'foo.h']))
    78             self.assertEquals(3, cpp_style._cpp_style_state.verbose_level)
    79             self.assertEquals((['foo.cpp'], {}),
    80                               style.parse_arguments(['--verbose=5', 'foo.cpp']))
    81             self.assertEquals(5, cpp_style._cpp_style_state.verbose_level)
    82             self.assertRaises(ValueError,
    83                               style.parse_arguments, ['--v=f', 'foo.cpp'])
    84 
    85             self.assertEquals((['foo.cpp'], {}),
    86                               style.parse_arguments(['--output=emacs', 'foo.cpp']))
    87             self.assertEquals('emacs', cpp_style._cpp_style_state.output_format)
    88             self.assertEquals((['foo.h'], {}),
    89                               style.parse_arguments(['--output=vs7', 'foo.h']))
    90             self.assertEquals('vs7', cpp_style._cpp_style_state.output_format)
    91             self.assertRaises(SystemExit,
    92                               style.parse_arguments, ['--output=blah', 'foo.cpp'])
    93 
    94             filt = '-,+whitespace,-whitespace/indent'
    95             self.assertEquals((['foo.h'], {}),
    96                               style.parse_arguments(['--filter='+filt, 'foo.h']))
    97             self.assertEquals(['-', '+whitespace', '-whitespace/indent'],
    98                               cpp_style._cpp_style_state.filters)
    99 
    100             self.assertEquals((['foo.cpp', 'foo.h'], {}),
    101                               style.parse_arguments(['foo.cpp', 'foo.h']))
    102 
    103             self.assertEquals((['foo.cpp'], {'--foo': ''}),
    104                               style.parse_arguments(['--foo', 'foo.cpp'], ['foo']))
    105             self.assertEquals((['foo.cpp'], {'--foo': 'bar'}),
    106                               style.parse_arguments(['--foo=bar', 'foo.cpp'], ['foo=']))
    107             self.assertEquals((['foo.cpp'], {}),
    108                               style.parse_arguments(['foo.cpp'], ['foo=']))
    109             self.assertRaises(SystemExit,
    110                               style.parse_arguments,
    111                               ['--footypo=bar', 'foo.cpp'], ['foo='])
    112         finally:
    113             style._USAGE = old_usage
    114             style._STYLE_CATEGORIES = old_style_categories
    115             style._WEBKIT_FILTER_RULES = old_webkit_filter_rules
    116             cpp_style._cpp_style_state.output_format = old_output_format
    117             cpp_style._cpp_style_state.verbose_level = old_verbose_level
    118             cpp_style._cpp_style_state.filters = old_filters
     96
     97    """Test the ArgumentParser class."""
     98
     99    def _parse(self):
     100        """Return a default parse() function for testing."""
     101        return self._create_parser().parse
     102
     103    def _create_defaults(self, default_output_format='vs7',
     104                         default_verbosity=3,
     105                         default_filter_rules=['-', '+whitespace']):
     106        """"Return a default ArgumentDefaults instance for testing."""
     107        return style.ArgumentDefaults(default_output_format,
     108                                      default_verbosity,
     109                                      default_filter_rules)
     110
     111    def _create_parser(self, defaults=None):
     112        """"Return an ArgumentParser instance for testing."""
     113        def create_usage(_defaults):
     114            """Return a usage string for testing."""
     115            return "usage"
     116
     117        def doc_print(message):
     118            # We do not want the usage string or style categories
     119            # to print during unit tests, so print nothing.
     120            return
     121
     122        if defaults is None:
     123            defaults = self._create_defaults()
     124
     125        return style.ArgumentParser(defaults, create_usage, doc_print)
     126
     127    def test_parse_documentation(self):
     128        parse = self._parse()
     129
     130        # FIXME: Test both the printing of the usage string and the
     131        #        filter categories help.
     132
     133        # Request the usage string.
     134        self.assertRaises(SystemExit, parse, ['--help'])
     135        # Request default filter rules and available style categories.
     136        self.assertRaises(SystemExit, parse, ['--filter='])
     137
     138    def test_parse_bad_values(self):
     139        parse = self._parse()
     140
     141        # Pass an unsupported argument.
     142        self.assertRaises(SystemExit, parse, ['--bad'])
     143
     144        self.assertRaises(ValueError, parse, ['--verbose=bad'])
     145        self.assertRaises(ValueError, parse, ['--verbose=0'])
     146        self.assertRaises(ValueError, parse, ['--verbose=6'])
     147        parse(['--verbose=1']) # works
     148        parse(['--verbose=5']) # works
     149
     150        self.assertRaises(ValueError, parse, ['--output=bad'])
     151        parse(['--output=vs7']) # works
     152
     153        # Pass a filter rule not beginning with + or -.
     154        self.assertRaises(ValueError, parse, ['--filter=foo'])
     155        parse(['--filter=+foo']) # works
     156        # Pass files and git-commit at the same time.
     157        self.assertRaises(SystemExit, parse, ['--git-commit=', 'file.txt'])
     158        # Pass an extra flag already supported.
     159        self.assertRaises(ValueError, parse, [], ['filter='])
     160        parse([], ['extra=']) # works
     161        # Pass an extra flag with typo.
     162        self.assertRaises(SystemExit, parse, ['--extratypo='], ['extra='])
     163        parse(['--extra='], ['extra=']) # works
     164        self.assertRaises(ValueError, parse, [], ['extra=', 'extra='])
     165
     166
     167    def test_parse_default_arguments(self):
     168        parse = self._parse()
     169
     170        (files, options) = parse([])
     171
     172        self.assertEquals(files, [])
     173
     174        self.assertEquals(options.output_format, 'vs7')
     175        self.assertEquals(options.verbosity, 3)
     176        self.assertEquals(options.filter_rules, ['-', '+whitespace'])
     177        self.assertEquals(options.git_commit, None)
     178
     179    def test_parse_explicit_arguments(self):
     180        parse = self._parse()
     181
     182        # Pass non-default explicit values.
     183        (files, options) = parse(['--output=emacs'])
     184        self.assertEquals(options.output_format, 'emacs')
     185        (files, options) = parse(['--verbose=4'])
     186        self.assertEquals(options.verbosity, 4)
     187        (files, options) = parse(['--git-commit=commit'])
     188        self.assertEquals(options.git_commit, 'commit')
     189        (files, options) = parse(['--filter=+foo,-bar'])
     190        self.assertEquals(options.filter_rules,
     191                          ['-', '+whitespace', '+foo', '-bar'])
     192
     193        # Pass extra flag values.
     194        (files, options) = parse(['--extra'], ['extra'])
     195        self.assertEquals(options.extra_flag_values, {'--extra': ''})
     196        (files, options) = parse(['--extra='], ['extra='])
     197        self.assertEquals(options.extra_flag_values, {'--extra': ''})
     198        (files, options) = parse(['--extra=x'], ['extra='])
     199        self.assertEquals(options.extra_flag_values, {'--extra': 'x'})
     200
     201    def test_parse_files(self):
     202        parse = self._parse()
     203
     204        (files, options) = parse(['foo.cpp'])
     205        self.assertEquals(files, ['foo.cpp'])
     206
     207        # Pass multiple files.
     208        (files, options) = parse(['--output=emacs', 'foo.cpp', 'bar.cpp'])
     209        self.assertEquals(files, ['foo.cpp', 'bar.cpp'])
    119210
    120211
Note: See TracChangeset for help on using the changeset viewer.