Changeset 56987 in webkit
- Timestamp:
- Apr 2, 2010 1:27:22 AM (14 years ago)
- Location:
- trunk/WebKitTools
- Files:
-
- 3 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/WebKitTools/ChangeLog
r56986 r56987 1 2010-04-02 Chris Jerdonek <cjerdonek@webkit.org> 2 3 Reviewed by Shinichiro Hamaji. 4 5 Re-wrote check-webkit-style's argument parsing code to use 6 Python's optparser module and more uniform error-handling logic. 7 8 https://bugs.webkit.org/show_bug.cgi?id=34676 9 10 * Scripts/webkitpy/style/optparser.py: 11 - Removed "option help" from check-webkit-style's usage string 12 since that is provided separately by the OptionParser class. 13 - Also changed the usage string from a function to a constant 14 string _USAGE. 15 - Added an _EPILOG string which renders after OptionParser's 16 usage string and option help. 17 - In the ArgumentParser class: 18 - Changed the constructor's stderr_write parameter to a 19 mock_stderr since the OptionParser accepts a sys.stderr 20 substitute rather than a sys.stderr.write substitute. 21 - Changed the constructor to set a _parser data attribute with 22 an OptionParser instance. 23 - Added a _create_option_parser() method which instantiates 24 the OptionParser. 25 - Updated _exit_with_help() to interact with the OptionParser's 26 help method. 27 - Updated the parse() method as necessary. Also changed the 28 raising of ValueErrors to calls to _exit_with_help(). 29 30 * Scripts/webkitpy/style/optparser_unittest.py: 31 - Removed the CreateUsageTest class since the create_usage method 32 was replaced by a constant string. 33 - Added a _MockStdErr class to the ArgumentParserTest class. 34 - Updated the unit tests as necessary. 35 36 1 37 2010-04-02 Adam Barth <abarth@webkit.org> 2 38 -
trunk/WebKitTools/Scripts/webkitpy/style/optparser.py
r56692 r56987 23 23 """Supports the parsing of command-line options for check-webkit-style.""" 24 24 25 import getopt 25 from optparse import OptionParser 26 26 import logging 27 27 import os.path … … 33 33 _log = logging.getLogger(__name__) 34 34 35 36 def _create_usage(default_options): 37 """Return the usage string to display for command help. 38 39 Args: 40 default_options: A DefaultCommandOptionValues instance. 41 42 """ 43 usage = """ 44 Syntax: %(program_name)s [--filter=-x,+y,...] [--git-commit=<SingleCommit>] 45 [--min-confidence=#] [--output=vs7] [--verbose] [file or directory] ... 46 47 The style guidelines this tries to follow are here: 48 http://webkit.org/coding/coding-style.html 49 50 Every style error is given a confidence score from 1-5, with 5 meaning 51 we are certain of the problem, and 1 meaning it could be a legitimate 52 construct. This can miss some errors and does not substitute for 53 code review. 54 55 To prevent specific lines from being linted, add a '// NOLINT' comment to the 56 end of the line. 57 58 Linted extensions are .cpp, .c and .h. Other file types are ignored. 59 60 The file parameter is optional and accepts multiple files. Leaving 61 out the file parameter applies the check to all files considered changed 62 by your source control management system. 63 64 Flags: 65 66 filter=-x,+y,... 67 A comma-separated list of boolean filter rules used to filter 68 which categories of style guidelines to check. The script checks 69 a category if the category passes the filter rules, as follows. 70 71 Any webkit category starts out passing. All filter rules are then 72 evaluated left to right, with later rules taking precedence. For 73 example, the rule "+foo" passes any category that starts with "foo", 74 and "-foo" fails any such category. The filter input "-whitespace, 75 +whitespace/braces" fails the category "whitespace/tab" and passes 76 "whitespace/braces". 77 78 Examples: --filter=-whitespace,+whitespace/braces 79 --filter=-whitespace,-runtime/printf,+runtime/printf_format 80 --filter=-,+build/include_what_you_use 81 82 Category names appear in error messages in brackets, for example 83 [whitespace/indent]. To see a list of all categories available to 84 %(program_name)s, along with which are enabled by default, pass 85 the empty filter as follows: 86 --filter= 87 88 git-commit=<SingleCommit> 89 Checks the style of everything from the given commit to the local tree. 90 91 min-confidence=# 92 An integer between 1 and 5 inclusive that is the minimum confidence 93 level of style errors to report. In particular, the value 1 displays 94 all errors. The default is %(default_min_confidence)s. 95 96 output=vs7 97 The output format, which may be one of 98 emacs : to ease emacs parsing 99 vs7 : compatible with Visual Studio 100 Defaults to "%(default_output_format)s". Other formats are unsupported. 101 102 verbose 103 Logging is verbose if this flag is present. 104 105 Path considerations: 106 35 _USAGE = """usage: %prog [--help] [options] [path1] [path2] ... 36 37 Overview: 38 Check coding style according to WebKit style guidelines: 39 40 http://webkit.org/coding/coding-style.html 41 42 Path arguments can be files and directories. If neither a git commit nor 43 paths are passed, then all changes in your source control working directory 44 are checked. 45 46 Style errors: 47 This script assigns to every style error a confidence score from 1-5 and 48 a category name. A confidence score of 5 means the error is certainly 49 a problem, and 1 means it could be fine. 50 51 Category names appear in error messages in brackets, for example 52 [whitespace/indent]. See the options section below for an option that 53 displays all available categories and which are reported by default. 54 55 Filters: 56 Use filters to configure what errors to report. Filters are specified using 57 a comma-separated list of boolean filter rules. The script reports errors 58 in a category if the category passes the filter, as described below. 59 60 All categories start out passing. Boolean filter rules are then evaluated 61 from left to right, with later rules taking precedence. For example, the 62 rule "+foo" passes any category that starts with "foo", and "-foo" fails 63 any such category. The filter input "-whitespace,+whitespace/braces" fails 64 the category "whitespace/tab" and passes "whitespace/braces". 65 66 Examples: --filter=-whitespace,+whitespace/braces 67 --filter=-whitespace,-runtime/printf,+runtime/printf_format 68 --filter=-,+build/include_what_you_use 69 70 Paths: 107 71 Certain style-checking behavior depends on the paths relative to 108 72 the WebKit source root of the files being checked. For example, … … 132 96 checking behaves correctly -- whether or not a checkout can be 133 97 detected. This is because all file paths will already be relative 134 to the source root and so will not need to be converted. 135 136 """ % {'program_name': os.path.basename(sys.argv[0]), 137 'default_min_confidence': default_options.min_confidence, 138 'default_output_format': default_options.output_format} 139 140 return usage 98 to the source root and so will not need to be converted.""" 99 100 _EPILOG = ("This script can miss errors and does not substitute for " 101 "code review.") 141 102 142 103 … … 263 224 264 225 265 # FIXME: Replace the use of getopt.getopt() with optparse.OptionParser.266 226 class ArgumentParser(object): 267 227 … … 288 248 default_options, 289 249 base_filter_rules=None, 290 create_usage=None,291 stderr_write=None):250 mock_stderr=None, 251 usage=None): 292 252 """Create an ArgumentParser instance. 293 253 … … 311 271 if base_filter_rules is None: 312 272 base_filter_rules = [] 313 if create_usage is None: 314 create_usage = _create_usage 315 if stderr_write is None: 316 stderr_write = sys.stderr.write 273 stderr = sys.stderr if mock_stderr is None else mock_stderr 274 if usage is None: 275 usage = _USAGE 317 276 318 277 self._all_categories = all_categories 319 278 self._base_filter_rules = base_filter_rules 279 320 280 # FIXME: Rename these to reflect that they are internal. 321 self.create_usage = create_usage322 281 self.default_options = default_options 323 self.stderr_write = stderr_write 324 325 def _exit_with_usage(self, error_message=''): 282 self.stderr_write = stderr.write 283 284 self._parser = self._create_option_parser(stderr=stderr, 285 usage=usage, 286 default_min_confidence=self.default_options.min_confidence, 287 default_output_format=self.default_options.output_format) 288 289 def _create_option_parser(self, stderr, usage, 290 default_min_confidence, default_output_format): 291 # Since the epilog string is short, it is not necessary to replace 292 # the epilog string with a mock epilog string when testing. 293 # For this reason, we use _EPILOG directly rather than passing it 294 # as an argument like we do for the usage string. 295 parser = OptionParser(usage=usage, epilog=_EPILOG) 296 297 filter_help = ('set a filter to control what categories of style ' 298 'errors to report. Specify a filter using a comma-' 299 'delimited list of boolean filter rules, for example ' 300 '"--filter -whitespace,+whitespace/braces". To display ' 301 'all categories and which are enabled by default, pass ' 302 """no value (e.g. '-f ""' or '--filter=').""") 303 parser.add_option("-f", "--filter-rules", metavar="RULES", 304 dest="filter_value", help=filter_help) 305 306 git_help = "check all changes after the given git commit." 307 parser.add_option("-g", "--git-commit", "--git-diff", "--git-since", 308 metavar="COMMIT", dest="git_since", help=git_help,) 309 310 min_confidence_help = ("set the minimum confidence of style errors " 311 "to report. Can be an integer 1-5, with 1 " 312 "displaying all errors. Defaults to %default.") 313 parser.add_option("-m", "--min-confidence", metavar="INT", 314 type="int", dest="min_confidence", 315 default=default_min_confidence, 316 help=min_confidence_help) 317 318 output_format_help = ('set the output format, which can be "emacs" ' 319 'or "vs7" (for Visual Studio). ' 320 'Defaults to "%default".') 321 parser.add_option("-o", "--output-format", metavar="FORMAT", 322 choices=["emacs", "vs7"], 323 dest="output_format", default=default_output_format, 324 help=output_format_help) 325 326 verbose_help = "enable verbose logging." 327 parser.add_option("-v", "--verbose", dest="is_verbose", default=False, 328 action="store_true", help=verbose_help) 329 330 # Override OptionParser's error() method so that option help will 331 # also display when an error occurs. Normally, just the usage 332 # string displays and not option help. 333 parser.error = self._exit_with_help 334 335 # Override OptionParser's print_help() method so that help output 336 # does not render to the screen while running unit tests. 337 print_help = parser.print_help 338 parser.print_help = lambda: print_help(file=stderr) 339 340 return parser 341 342 def _exit_with_help(self, error_message=None): 326 343 """Exit and print a usage string with an optional error message. 327 344 … … 330 347 331 348 """ 332 usage = self.create_usage(self.default_options) 333 self.stderr_write(usage) 349 # The method format_help() includes both the usage string and 350 # the flag options. 351 help = self._parser.format_help() 352 # Separate help from the error message with a single blank line. 353 self.stderr_write(help + "\n") 334 354 if error_message: 335 sys.exit('\nFATAL ERROR: ' + error_message) 336 else: 337 sys.exit(1) 355 _log.error(error_message) 356 357 # Since we are using this method to replace/override the Python 358 # module optparse's OptionParser.error() method, we match its 359 # behavior and exit with status code 2. 360 # 361 # As additional background, Python documentation says-- 362 # 363 # "Unix programs generally use 2 for command line syntax errors 364 # and 1 for all other kind of errors." 365 # 366 # (from http://docs.python.org/library/sys.html#sys.exit ) 367 sys.exit(2) 338 368 339 369 def _exit_with_categories(self): … … 382 412 383 413 """ 384 is_verbose = False 385 min_confidence = self.default_options.min_confidence 386 output_format = self.default_options.output_format 387 388 # The flags that the CommandOptionValues class supports. 389 flags = ['filter=', 'git-commit=', 'help', 'min-confidence=', 390 'output=', 'verbose'] 391 392 try: 393 (opts, paths) = getopt.getopt(args, '', flags) 394 except getopt.GetoptError, err: 395 # FIXME: Settle on an error handling approach: come up 396 # with a consistent guideline as to when and whether 397 # a ValueError should be raised versus calling 398 # sys.exit when needing to interrupt execution. 399 self._exit_with_usage('Invalid arguments: %s' % err) 400 401 git_commit = None 402 filter_rules = [] 403 404 for (opt, val) in opts: 405 # Process --help first (out of alphabetical order). 406 if opt == '--help': 407 self._exit_with_usage() 408 elif opt == '--filter': 409 if not val: 410 self._exit_with_categories() 411 filter_rules = self._parse_filter_flag(val) 412 elif opt == '--git-commit': 413 git_commit = val 414 elif opt == '--min-confidence': 415 min_confidence = val 416 elif opt == '--output': 417 output_format = val 418 elif opt == "--verbose": 419 is_verbose = True 420 else: 421 # We should never get here because getopt.getopt() 422 # raises an error in this case. 423 raise ValueError('Invalid option: "%s"' % opt) 424 425 # Check validity of resulting values. 414 (options, paths) = self._parser.parse_args(args=args) 415 416 filter_value = options.filter_value 417 git_commit = options.git_since 418 is_verbose = options.is_verbose 419 min_confidence = options.min_confidence 420 output_format = options.output_format 421 422 if filter_value is not None and not filter_value: 423 # Then the user explicitly passed no filter, for 424 # example "-f ''" or "--filter=". 425 self._exit_with_categories() 426 427 # Validate user-provided values. 428 429 # FIXME: Move the checkout error outside of this module. The parse 430 # method should not need to know whether a checkout was found. 426 431 if not found_checkout and not paths: 427 432 _log.error("WebKit checkout not found: You must run this script " … … 430 435 sys.exit(1) 431 436 432 if paths and (git_commit != None): 433 self._exit_with_usage('It is not possible to check files and a ' 434 'specific commit at the same time.') 435 436 if output_format not in ('emacs', 'vs7'): 437 raise ValueError('Invalid --output value "%s": The only ' 438 'allowed output formats are emacs and vs7.' % 439 output_format) 440 441 validate_filter_rules(filter_rules, self._all_categories) 437 if paths and git_commit: 438 self._exit_with_help('You cannot provide both paths and a git ' 439 'commit at the same time.') 442 440 443 441 min_confidence = int(min_confidence) 444 442 if (min_confidence < 1) or (min_confidence > 5): 445 raise ValueError('Invalid --min-confidence value %s: value must ' 446 'be between 1 and 5 inclusive.' % min_confidence) 443 self._exit_with_help('option --min-confidence: invalid integer: ' 444 '%s: value must be between 1 and 5' 445 % min_confidence) 446 447 if filter_value: 448 filter_rules = self._parse_filter_flag(filter_value) 449 else: 450 filter_rules = [] 451 452 try: 453 validate_filter_rules(filter_rules, self._all_categories) 454 except ValueError, err: 455 self._exit_with_help(err) 447 456 448 457 options = CommandOptionValues(filter_rules=filter_rules, -
trunk/WebKitTools/Scripts/webkitpy/style/optparser_unittest.py
r56692 r56987 25 25 import unittest 26 26 27 from optparser import _create_usage28 27 from optparser import ArgumentParser 29 28 from optparser import ArgumentPrinter … … 31 30 from optparser import DefaultCommandOptionValues 32 31 from webkitpy.style_references import LogTesting 33 34 35 class CreateUsageTest(unittest.TestCase):36 37 """Tests the _create_usage() function."""38 39 def test_create_usage(self):40 default_options = DefaultCommandOptionValues(min_confidence=3,41 output_format="vs7")42 # Exercise the code path to make sure the function does not error out.43 _create_usage(default_options)44 32 45 33 … … 77 65 """Test the ArgumentParser class.""" 78 66 67 class _MockStdErr(object): 68 69 def write(self, message): 70 # We do not want the usage string or style categories 71 # to print during unit tests, so print nothing. 72 return 73 79 74 def setUp(self): 80 75 self._log = LogTesting.setUp(self) … … 97 92 def _create_parser(self): 98 93 """Return an ArgumentParser instance for testing.""" 99 def stderr_write(message):100 # We do not want the usage string or style categories101 # to print during unit tests, so print nothing.102 return103 104 94 default_options = self._create_defaults() 105 95 106 96 all_categories = ["build" ,"whitespace"] 97 98 mock_stderr = self._MockStdErr() 99 107 100 return ArgumentParser(all_categories=all_categories, 108 101 base_filter_rules=[], 109 102 default_options=default_options, 110 stderr_write=stderr_write) 103 mock_stderr=mock_stderr, 104 usage="test usage") 111 105 112 106 def test_parse_documentation(self): … … 126 120 # Pass an unsupported argument. 127 121 self.assertRaises(SystemExit, parse, ['--bad']) 128 129 self.assertRaises(ValueError, parse, ['--min-confidence=bad']) 130 self.assertRaises(ValueError, parse, ['--min-confidence=0']) 131 self.assertRaises(ValueError, parse, ['--min-confidence=6']) 122 self._log.assertMessages(['ERROR: no such option: --bad\n']) 123 124 self.assertRaises(SystemExit, parse, ['--min-confidence=bad']) 125 self._log.assertMessages(["ERROR: option --min-confidence: invalid " 126 "integer value: 'bad'\n"]) 127 self.assertRaises(SystemExit, parse, ['--min-confidence=0']) 128 self._log.assertMessages(['ERROR: option --min-confidence: invalid ' 129 'integer: 0: value must be between 1 and ' 130 '5\n']) 131 self.assertRaises(SystemExit, parse, ['--min-confidence=6']) 132 self._log.assertMessages(['ERROR: option --min-confidence: invalid ' 133 'integer: 6: value must be between 1 and ' 134 '5\n']) 132 135 parse(['--min-confidence=1']) # works 133 136 parse(['--min-confidence=5']) # works 134 137 135 self.assertRaises(ValueError, parse, ['--output=bad']) 138 self.assertRaises(SystemExit, parse, ['--output=bad']) 139 self._log.assertMessages(["ERROR: option --output-format: invalid " 140 "choice: 'bad' (choose from 'emacs', " 141 "'vs7')\n"]) 136 142 parse(['--output=vs7']) # works 137 143 138 144 # Pass a filter rule not beginning with + or -. 139 self.assertRaises(ValueError, parse, ['--filter=build']) 145 self.assertRaises(SystemExit, parse, ['--filter=build']) 146 self._log.assertMessages(['ERROR: Invalid filter rule "build": every ' 147 'rule must start with + or -.\n']) 140 148 parse(['--filter=+build']) # works 141 149 # Pass files and git-commit at the same time. 142 self.assertRaises(SystemExit, parse, ['--git-commit=', 'file.txt']) 150 self.assertRaises(SystemExit, parse, ['--git-commit=committish', 151 'file.txt']) 152 self._log.assertMessages(['ERROR: You cannot provide both paths and ' 153 'a git commit at the same time.\n']) 154 155 def test_parse_no_paths_without_scm(self): 156 parse = self._parse 143 157 144 158 # Paths must be passed when found_scm is False. … … 148 162 "passing specific paths to check.\n"] 149 163 self._log.assertMessages(messages) 150 self.assertRaises(SystemExit, parse, ['--verbose=3'], found_scm=False) 164 # Test one more variant. 165 self.assertRaises(SystemExit, parse, ['--output=vs7'], found_scm=False) 166 self._log.assertMessages(messages) 151 167 152 168 def test_parse_default_arguments(self):
Note: See TracChangeset
for help on using the changeset viewer.