Changeset 53251 in webkit
- Timestamp:
- Jan 14, 2010, 2:19:13 AM (15 years ago)
- Location:
- trunk/WebKitTools
- Files:
-
- 7 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/WebKitTools/ChangeLog
r53249 r53251 1 2010-01-14 Chris Jerdonek <chris.jerdonek@gmail.com> 2 3 Reviewed by Shinichiro Hamaji. 4 5 Moved error() from cpp_style.py to checker.py. 6 7 https://bugs.webkit.org/show_bug.cgi?id=33620 8 9 * Scripts/check-webkit-style: 10 - Addressed FIXME to not set global state. 11 12 * Scripts/webkitpy/style/checker.py: 13 - Added argument validation to ProcessorOptions constructor. 14 - Added should_report_error() to ProcessorOptions class. 15 - Removed set_options(). 16 - Added StyleChecker class. 17 18 * Scripts/webkitpy/style/checker_unittest.py: 19 - Added unit test class for ProcessorOptions class. 20 - Added unit test to check that parse() strips white space. 21 22 * Scripts/webkitpy/style/cpp_style.py: 23 - Removed "filter" and "output_format" methods. 24 - Removed should_print_error() and error() functions. 25 - Removed default parameter value from process_file(). 26 27 * Scripts/webkitpy/style/cpp_style_unittest.py: 28 - Removed call to cpp_style._should_print_error(). 29 - Removed test_filter() and test_filter_appending(). 30 31 * Scripts/webkitpy/style/text_style.py: 32 - Removed default parameter value from process_file(). 33 1 34 2010-01-14 Diego Gonzalez <diego.gonzalez@openbossa.org> 2 35 -
trunk/WebKitTools/Scripts/check-webkit-style
r52906 r53251 67 67 (files, options) = parser.parse(sys.argv[1:]) 68 68 69 # FIXME: Eliminate the need to call this function. 70 # Options should be passed into process_file instead. 71 checker.set_options(options) 69 styleChecker = checker.StyleChecker(options) 72 70 73 71 if files: 74 72 for filename in files: 75 checker.process_file(filename)73 styleChecker.process_file(filename) 76 74 77 75 else: … … 89 87 else: 90 88 patch = scm.create_patch() 91 checker.process_patch(patch)89 styleChecker.process_patch(patch) 92 90 93 91 sys.stderr.write('Total errors found: %d\n' % checker.error_count()) -
trunk/WebKitTools/Scripts/webkitpy/style/checker.py
r53185 r53251 306 306 and "vs7" which Microsoft Visual Studio 7 can parse. 307 307 308 verbosity: An integer 1-5 that restricts output to errors with a309 confidence score at or above this value.308 verbosity: An integer between 1-5 inclusive that restricts output 309 to errors with a confidence score at or above this value. 310 310 The default is 1, which displays all errors. 311 311 … … 319 319 pairs that are not otherwise represented by this 320 320 class. The default is the empty dictionary. 321 321 322 """ 322 323 323 def __init__(self, output_format , verbosity=1, filter=None,324 def __init__(self, output_format="emacs", verbosity=1, filter=None, 324 325 git_commit=None, extra_flag_values=None): 325 326 if filter is None: … … 328 329 extra_flag_values = {} 329 330 331 if output_format not in ("emacs", "vs7"): 332 raise ValueError('Invalid "output_format" parameter: ' 333 'value must be "emacs" or "vs7". ' 334 'Value given: "%s".' % output_format) 335 336 if (verbosity < 1) or (verbosity > 5): 337 raise ValueError('Invalid "verbosity" parameter: ' 338 "value must be an integer between 1-5 inclusive. " 339 'Value given: "%s".' % verbosity) 340 330 341 self.output_format = output_format 331 342 self.verbosity = verbosity … … 334 345 self.extra_flag_values = extra_flag_values 335 346 336 337 # FIXME: Eliminate the need for this function. 338 # Options should be passed into process_file instead. 339 def set_options(options): 340 """Initialize global _CppStyleState instance. 341 342 This needs to be called before calling process_file. 343 344 Args: 345 options: A ProcessorOptions instance. 346 """ 347 cpp_style._set_output_format(options.output_format) 348 cpp_style._set_verbose_level(options.verbosity) 349 cpp_style._set_filter(options.filter) 347 def should_report_error(self, category, confidence_in_error): 348 """Return whether an error should be reported. 349 350 An error should be reported if the confidence in the error 351 is at least the current verbosity level, and if the current 352 filter says that the category should be checked. 353 354 Args: 355 category: A string that is a style category. 356 confidence_in_error: An integer between 1 and 5, inclusive, that 357 represents the application's confidence in 358 the error. A higher number signifies greater 359 confidence. 360 361 """ 362 if confidence_in_error < self.verbosity: 363 return False 364 365 if self.filter is None: 366 return True # All categories should be checked by default. 367 368 return self.filter.should_check(category) 350 369 351 370 … … 437 456 Args: 438 457 error_message: A string that is an error message to print. 458 439 459 """ 440 460 usage = self.create_usage(self.defaults) … … 552 572 553 573 verbosity = int(verbosity) 554 if ( (verbosity < 1) or (verbosity > 5)):574 if (verbosity < 1) or (verbosity > 5): 555 575 raise ValueError('Invalid --verbose value %s: value must ' 556 576 'be between 1-5.' % verbosity) … … 564 584 565 585 566 def process_file(filename): 567 """Checks style for the specified file. 568 569 If the specified filename is '-', applies cpp_style to the standard input. 570 """ 571 if cpp_style.can_handle(filename) or filename == '-': 572 cpp_style.process_file(filename) 573 elif text_style.can_handle(filename): 574 text_style.process_file(filename) 575 576 577 def process_patch(patch_string): 578 """Does lint on a single patch. 579 580 Args: 581 patch_string: A string of a patch. 582 """ 583 patch = DiffParser(patch_string.splitlines()) 584 for filename, diff in patch.files.iteritems(): 585 file_extension = os.path.splitext(filename)[1] 586 line_numbers = set() 587 588 def error_for_patch(filename, line_number, category, confidence, message): 589 """Wrapper function of cpp_style.error for patches. 590 591 This function outputs errors only if the line number 592 corresponds to lines which are modified or added. 593 """ 594 if not line_numbers: 595 for line in diff.lines: 596 # When deleted line is not set, it means that 597 # the line is newly added. 598 if not line[0]: 599 line_numbers.add(line[1]) 600 601 if line_number in line_numbers: 602 cpp_style.error(filename, line_number, category, confidence, message) 603 604 if cpp_style.can_handle(filename): 605 cpp_style.process_file(filename, error=error_for_patch) 586 class StyleChecker(object): 587 588 """Supports checking style in files and patches.""" 589 590 def __init__(self, options): 591 """Create a StyleChecker instance. 592 593 Args: 594 options: A ProcessorOptions instance. 595 596 """ 597 self.options = options 598 599 # FIXME: Eliminate the need to set global state here. 600 cpp_style._set_verbose_level(options.verbosity) 601 602 def _handle_error(self, filename, line_number, category, confidence, message): 603 """Logs the fact we've found a lint error. 604 605 We log the error location and our confidence in the error, i.e. 606 how certain we are the error is a legitimate style regression 607 versus a misidentification or justified use. 608 609 Args: 610 filename: The name of the file containing the error. 611 line_number: The number of the line containing the error. 612 category: A string used to describe the "category" this bug 613 falls under: "whitespace", say, or "runtime". 614 Categories may have a hierarchy separated by slashes: 615 "whitespace/indent". 616 confidence: A number from 1-5 representing a confidence score 617 for the error, with 5 meaning that we are certain 618 of the problem, and 1 meaning that it could be a 619 legitimate construct. 620 message: The error message. 621 622 """ 623 if not self.options.should_report_error(category, confidence): 624 return 625 626 # FIXME: Eliminate the need to reference cpp_style here. 627 cpp_style._cpp_style_state.increment_error_count() 628 629 if self.options.output_format == 'vs7': 630 sys.stderr.write('%s(%s): %s [%s] [%d]\n' % ( 631 filename, line_number, message, category, confidence)) 632 else: 633 sys.stderr.write('%s:%s: %s [%s] [%d]\n' % ( 634 filename, line_number, message, category, confidence)) 635 636 def process_file(self, filename): 637 """Checks style for the specified file. 638 639 If the specified filename is '-', applies cpp_style to the standard input. 640 641 """ 642 if cpp_style.can_handle(filename) or filename == '-': 643 cpp_style.process_file(filename, self._handle_error) 606 644 elif text_style.can_handle(filename): 607 text_style.process_file(filename, error=error_for_patch) 645 text_style.process_file(filename, self._handle_error) 646 647 def process_patch(self, patch_string): 648 """Does lint on a single patch. 649 650 Args: 651 patch_string: A string of a patch. 652 653 """ 654 patch = DiffParser(patch_string.splitlines()) 655 for filename, diff in patch.files.iteritems(): 656 file_extension = os.path.splitext(filename)[1] 657 line_numbers = set() 658 659 def error_for_patch(filename, line_number, category, confidence, message): 660 """Wrapper function of cpp_style.error for patches. 661 662 This function outputs errors only if the line number 663 corresponds to lines which are modified or added. 664 """ 665 if not line_numbers: 666 for line in diff.lines: 667 # When deleted line is not set, it means that 668 # the line is newly added. 669 if not line[0]: 670 line_numbers.add(line[1]) 671 672 if line_number in line_numbers: 673 self._handle_error(filename, line_number, category, confidence, message) 674 675 if cpp_style.can_handle(filename): 676 cpp_style.process_file(filename, error_for_patch) 677 elif text_style.can_handle(filename): 678 text_style.process_file(filename, error_for_patch) 608 679 609 680 -
trunk/WebKitTools/Scripts/webkitpy/style/checker_unittest.py
r53186 r53251 39 39 import checker as style 40 40 from checker import CategoryFilter 41 41 from checker import ProcessorOptions 42 42 43 43 class CategoryFilterTest(unittest.TestCase): … … 84 84 self.assertFalse(filter.should_check("abc")) 85 85 self.assertTrue(filter.should_check("a")) 86 87 88 class ProcessorOptionsTest(unittest.TestCase): 89 90 """Tests ProcessorOptions class.""" 91 92 def test_init(self): 93 """Test __init__ constructor.""" 94 # Check default parameters. 95 options = ProcessorOptions() 96 self.assertEquals(options.extra_flag_values, {}) 97 self.assertEquals(options.filter, CategoryFilter([])) 98 self.assertEquals(options.git_commit, None) 99 self.assertEquals(options.output_format, "emacs") 100 self.assertEquals(options.verbosity, 1) 101 102 self.assertRaises(ValueError, ProcessorOptions, output_format="bad") 103 ProcessorOptions(output_format="emacs") # No ValueError: works 104 ProcessorOptions(output_format="vs7") # works 105 self.assertRaises(ValueError, ProcessorOptions, verbosity=0) 106 self.assertRaises(ValueError, ProcessorOptions, verbosity=6) 107 ProcessorOptions(verbosity=1) # works 108 ProcessorOptions(verbosity=5) # works 109 110 # Check attributes. 111 options = ProcessorOptions(extra_flag_values={"extra_value" : 2}, 112 filter=CategoryFilter(["+"]), 113 git_commit="commit", 114 output_format="vs7", 115 verbosity=3) 116 self.assertEquals(options.extra_flag_values, {"extra_value" : 2}) 117 self.assertEquals(options.filter, CategoryFilter(["+"])) 118 self.assertEquals(options.git_commit, "commit") 119 self.assertEquals(options.output_format, "vs7") 120 self.assertEquals(options.verbosity, 3) 121 122 def test_should_report_error(self): 123 """Test should_report_error().""" 124 filter = CategoryFilter(["-xyz"]) 125 options = ProcessorOptions(filter=filter, verbosity=3) 126 127 # Test verbosity 128 self.assertTrue(options.should_report_error("abc", 3)) 129 self.assertFalse(options.should_report_error("abc", 2)) 130 131 # Test filter 132 self.assertTrue(options.should_report_error("xy", 3)) 133 self.assertFalse(options.should_report_error("xyz", 3)) 86 134 87 135 … … 240 288 self.assertEquals(options.filter, 241 289 CategoryFilter(["-", "+whitespace", "+foo", "-bar"])) 290 # Spurious white space in filter rules. 291 (files, options) = parse(['--filter=+foo ,-bar']) 292 self.assertEquals(options.filter, 293 CategoryFilter(["-", "+whitespace", "+foo", "-bar"])) 242 294 243 295 # Pass extra flag values. -
trunk/WebKitTools/Scripts/webkitpy/style/cpp_style.py
r53185 r53251 249 249 self.verbose_level = 1 # global setting. 250 250 self.error_count = 0 # global count of reported errors 251 # filter to apply when emitting error messages252 self.filter = None253 254 # output format:255 # "emacs" - format that emacs can parse (default)256 # "vs7" - format that Microsoft Visual Studio 7 can parse257 self.output_format = 'emacs'258 259 def set_output_format(self, output_format):260 """Sets the output format for errors."""261 self.output_format = output_format262 251 263 252 def set_verbose_level(self, level): … … 267 256 return last_verbose_level 268 257 269 def set_filter(self, filter):270 """Sets the error-message filter.271 272 Args:273 filter: A CategoryFilter instance.274 275 """276 self.filter = filter277 278 258 def reset_error_count(self): 279 259 """Sets the module's error statistic back to zero.""" … … 288 268 289 269 290 def _output_format():291 """Gets the module's output format."""292 return _cpp_style_state.output_format293 294 295 def _set_output_format(output_format):296 """Sets the module's output format."""297 _cpp_style_state.set_output_format(output_format)298 299 300 270 def _verbose_level(): 301 271 """Returns the module's verbosity setting.""" … … 306 276 """Sets the module's verbosity, and returns the previous setting.""" 307 277 return _cpp_style_state.set_verbose_level(level) 308 309 310 def _filter():311 """Returns the module's CategoryFilter instance."""312 return _cpp_style_state.filter313 314 315 def _set_filter(filter):316 """Sets the module's error-message filter.317 318 The filter is applied when deciding whether to emit a given319 error message.320 321 Args:322 filter: A CategoryFilter instance.323 324 """325 _cpp_style_state.set_filter(filter)326 278 327 279 … … 476 428 """File has a source file extension.""" 477 429 return self.extension()[1:] in ('c', 'cc', 'cpp', 'cxx') 478 479 480 def _should_print_error(category, confidence):481 """Returns true iff confidence >= verbose, and category passes filter."""482 # There are two ways we might decide not to print an error message:483 # the verbosity level isn't high enough, or the filters filter it out.484 if confidence < _cpp_style_state.verbose_level:485 return False486 487 filter = _filter()488 489 if filter is None:490 return True # All categories should be checked by default.491 492 return filter.should_check(category)493 494 495 def error(filename, line_number, category, confidence, message):496 """Logs the fact we've found a lint error.497 498 We log where the error was found, and also our confidence in the error,499 that is, how certain we are this is a legitimate style regression, and500 not a misidentification or a use that's sometimes justified.501 502 Args:503 filename: The name of the file containing the error.504 line_number: The number of the line containing the error.505 category: A string used to describe the "category" this bug506 falls under: "whitespace", say, or "runtime". Categories507 may have a hierarchy separated by slashes: "whitespace/indent".508 confidence: A number from 1-5 representing a confidence score for509 the error, with 5 meaning that we are certain of the problem,510 and 1 meaning that it could be a legitimate construct.511 message: The error message.512 """513 # There are two ways we might decide not to print an error message:514 # the verbosity level isn't high enough, or the filters filter it out.515 if _should_print_error(category, confidence):516 _cpp_style_state.increment_error_count()517 if _cpp_style_state.output_format == 'vs7':518 sys.stderr.write('%s(%s): %s [%s] [%d]\n' % (519 filename, line_number, message, category, confidence))520 else:521 sys.stderr.write('%s:%s: %s [%s] [%d]\n' % (522 filename, line_number, message, category, confidence))523 430 524 431 … … 2970 2877 2971 2878 2972 def process_file(filename, error =error):2879 def process_file(filename, error): 2973 2880 """Performs cpp_style on a single file. 2974 2881 -
trunk/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py
r53185 r53251 46 46 # suggestion on how to best do this. 47 47 from checker import STYLE_CATEGORIES 48 from checker import CategoryFilter49 48 50 49 # This class works as an error collector and replaces cpp_style.Error … … 67 66 ' which is not in STYLE_CATEGORIES' % (message, category)) 68 67 self._seen_style_categories[category] = 1 69 if cpp_style._should_print_error(category, confidence): 70 self._errors.append('%s [%s] [%d]' % (message, category, confidence)) 68 self._errors.append('%s [%s] [%d]' % (message, category, confidence)) 71 69 72 70 def results(self): … … 1572 1570 'Tab found; better to use spaces [whitespace/tab] [1]') 1573 1571 1574 def test_filter(self):1575 old_filter = cpp_style._cpp_style_state.filter1576 try:1577 cpp_style._cpp_style_state.set_filter(CategoryFilter(['-', '+whitespace', '-whitespace/indent']))1578 self.assert_lint(1579 '// Hello there ',1580 'Line ends in whitespace. Consider deleting these extra spaces.'1581 ' [whitespace/end_of_line] [4]')1582 self.assert_lint('int a = (int)1.0;', '')1583 self.assert_lint(' weird opening space', '')1584 finally:1585 cpp_style._cpp_style_state.filter = old_filter1586 1587 def test_filter_appending(self):1588 old_filter = cpp_style._cpp_style_state.filter1589 try:1590 # Reset filters1591 cpp_style._cpp_style_state.set_filter(CategoryFilter(['-whitespace']))1592 self.assert_lint('// Hello there ', '')1593 cpp_style._cpp_style_state.set_filter(CategoryFilter(['-whitespace', '+whitespace/end_of_line']))1594 self.assert_lint(1595 '// Hello there ',1596 'Line ends in whitespace. Consider deleting these extra spaces.'1597 ' [whitespace/end_of_line] [4]')1598 self.assert_lint(' weird opening space', '')1599 finally:1600 cpp_style._cpp_style_state.filter = old_filter1601 1602 1572 def test_unnamed_namespaces_in_headers(self): 1603 1573 self.assert_language_rules_check( -
trunk/WebKitTools/Scripts/webkitpy/style/text_style.py
r52906 r53251 52 52 53 53 54 def process_file(filename, error =cpp_style.error):54 def process_file(filename, error): 55 55 """Performs lint check for text on a single file.""" 56 56 if (not can_handle(filename)):
Note:
See TracChangeset
for help on using the changeset viewer.