Changeset 58401 in webkit
- Timestamp:
- Apr 28, 2010 4:19:41 AM (14 years ago)
- Location:
- trunk/WebKitTools
- Files:
-
- 3 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/WebKitTools/ChangeLog
r58397 r58401 1 2010-04-28 Chris Jerdonek <cjerdonek@webkit.org> 2 3 Reviewed by Shinichiro Hamaji. 4 5 Adjusted check-webkit-style so that files with file type NONE 6 are automatically skipped without warning. 7 8 https://bugs.webkit.org/show_bug.cgi?id=38197 9 10 This change simplifies configuring which files to skip. It also 11 addresses an issue whereby check-webkit-style was unintentionally 12 checking .vcproj files for carriage returns. 13 14 * Scripts/webkitpy/style/checker.py: 15 - Moved the C++, Python, and text file extensions to new 16 module-level configuration variables. 17 - Removed .pyc from the _SKIPPED_FILES_WITHOUT_WARNING configuration 18 variable. 19 - Changed the numeric values of the FileType enum so that 20 FileType.NONE evaluates to False. 21 - For ProcessorDispatcher.should_skip_without_warning(): 22 - Changed the method to return True for FileType.NONE files. 23 - Made ChangeLog files an exception to getting skipped. 24 - Changed the StyleProcessor.process() method to raise an 25 exception if given a file path that should not be processed. 26 27 * Scripts/webkitpy/style/checker_unittest.py: 28 - Updated the unit tests and added more test cases as necessary. 29 1 30 2010-04-28 Eric Seidel <eric@webkit.org> 2 31 -
trunk/WebKitTools/Scripts/webkitpy/style/checker.py
r58263 r58401 158 158 159 159 160 _CPP_FILE_EXTENSIONS = [ 161 'c', 162 'cpp', 163 'h', 164 ] 165 166 _PYTHON_FILE_EXTENSION = 'py' 167 168 # FIXME: Include 'vcproj' files as text files after creating a mechanism 169 # for exempting them from the carriage-return checker (since they 170 # are Windows-only files). 171 _TEXT_FILE_EXTENSIONS = [ 172 'ac', 173 'cc', 174 'cgi', 175 'css', 176 'exp', 177 'flex', 178 'gyp', 179 'gypi', 180 'html', 181 'idl', 182 'in', 183 'js', 184 'mm', 185 'php', 186 'pl', 187 'pm', 188 'pri', 189 'pro', 190 'rb', 191 'sh', 192 'txt', 193 # 'vcproj', # See FIXME above. 194 'wm', 195 'xhtml', 196 'y', 197 ] 198 199 200 # Files to skip that are less obvious. 201 # 160 202 # Some files should be skipped when checking style. For example, 161 203 # WebKit maintains some files in Mozilla style on purpose to ease 162 204 # future merges. 163 #164 # Include a warning for skipped files that are less obvious.165 205 _SKIPPED_FILES_WITH_WARNING = [ 166 206 # The Qt API and tests do not follow WebKit style. … … 175 215 176 216 177 # Don't include a warning for skipped files that are more common 178 # and more obvious. 217 # Files to skip that are more common or obvious. 218 # 219 # This list should be in addition to files with FileType.NONE. Files 220 # with FileType.NONE are automatically skipped without warning. 179 221 _SKIPPED_FILES_WITHOUT_WARNING = [ 180 222 "LayoutTests/", 181 ".pyc",182 223 ] 183 224 … … 330 371 class FileType: 331 372 332 NONE = 1373 NONE = 0 # FileType.NONE evaluates to False. 333 374 # Alphabetize remaining types 334 CPP = 2335 PYTHON = 3336 TEXT = 4375 CPP = 1 376 PYTHON = 2 377 TEXT = 3 337 378 338 379 … … 348 389 """Supports determining whether and how to check style, based on path.""" 349 390 350 cpp_file_extensions = (351 'c',352 'cpp',353 'h',354 )355 356 text_file_extensions = (357 'css',358 'html',359 'idl',360 'js',361 'mm',362 'php',363 'pm',364 'txt',365 )366 367 391 def _file_extension(self, file_path): 368 392 """Return the file extension without the leading dot.""" … … 378 402 def should_skip_without_warning(self, file_path): 379 403 """Return whether the given file should be skipped without a warning.""" 404 if not self._file_type(file_path): # FileType.NONE. 405 return True 406 # Since "LayoutTests" is in _SKIPPED_FILES_WITHOUT_WARNING, make 407 # an exception to prevent files like "LayoutTests/ChangeLog" and 408 # "LayoutTests/ChangeLog-2009-06-16" from being skipped. 409 # 410 # FIXME: Figure out a good way to avoid having to add special logic 411 # for this special case. 412 if os.path.basename(file_path).startswith('ChangeLog'): 413 return False 380 414 for skipped_file in _SKIPPED_FILES_WITHOUT_WARNING: 381 415 if file_path.find(skipped_file) >= 0: … … 387 421 file_extension = self._file_extension(file_path) 388 422 389 if (file_extension in self.cpp_file_extensions) or (file_path == '-'):423 if (file_extension in _CPP_FILE_EXTENSIONS) or (file_path == '-'): 390 424 # FIXME: Do something about the comment below and the issue it 391 425 # raises since cpp_style already relies on the extension. … … 395 429 # the extension. 396 430 return FileType.CPP 397 elif file_extension == "py":431 elif file_extension == _PYTHON_FILE_EXTENSION: 398 432 return FileType.PYTHON 399 elif ( "ChangeLog" in file_pathor433 elif (os.path.basename(file_path).startswith('ChangeLog') or 400 434 (not file_extension and "WebKitTools/Scripts/" in file_path) or 401 file_extension in self.text_file_extensions):435 file_extension in _TEXT_FILE_EXTENSIONS): 402 436 return FileType.TEXT 403 437 else: 404 # FIXME: If possible, change this method to default to405 # returning FileType.TEXT. The should_process() method406 # should really encapsulate which files not to check.407 # Currently, "skip" logic is spread between both this408 # method and should_process.409 438 return FileType.NONE 410 439 … … 624 653 def should_process(self, file_path): 625 654 """Return whether the file should be checked for style.""" 626 627 655 if self._dispatcher.should_skip_without_warning(file_path): 628 656 return False … … 672 700 673 701 if checker is None: 674 # FIXME: Should we really be skipping files that return True 675 # for should_process()? Perhaps this should be a 676 # warning or an exception so we can find out if 677 # should_process() is missing any files. 678 _log.debug('File not a recognized type to check. Skipping: "%s"' 679 % file_path) 680 return 702 raise AssertionError("File should not be checked: '%s'" % file_path) 681 703 682 704 _log.debug("Using class: " + checker.__class__.__name__) -
trunk/WebKitTools/Scripts/webkitpy/style/checker_unittest.py
r58263 r58401 273 273 """Tests the "should skip" methods of the ProcessorDispatcher class.""" 274 274 275 def setUp(self): 276 self._dispatcher = ProcessorDispatcher() 277 275 278 def test_should_skip_with_warning(self): 276 279 """Test should_skip_with_warning().""" 277 dispatcher = ProcessorDispatcher()278 279 280 # Check a non-skipped file. 280 self.assertFalse( dispatcher.should_skip_with_warning("foo.txt"))281 self.assertFalse(self._dispatcher.should_skip_with_warning("foo.txt")) 281 282 282 283 # Check skipped files. … … 293 294 294 295 for path in paths_to_skip: 295 self.assertTrue( dispatcher.should_skip_with_warning(path),296 self.assertTrue(self._dispatcher.should_skip_with_warning(path), 296 297 "Checking: " + path) 297 298 298 def test_should_skip_without_warning(self): 299 """Test should_skip_without_warning().""" 300 dispatcher = ProcessorDispatcher() 301 302 # Check a non-skipped file. 303 self.assertFalse(dispatcher.should_skip_without_warning("foo.txt")) 304 305 # Check skipped files. 306 paths_to_skip = [ 307 # LayoutTests folder 308 "LayoutTests/foo.txt", 309 ] 310 311 for path in paths_to_skip: 312 self.assertTrue(dispatcher.should_skip_without_warning(path), 313 "Checking: " + path) 299 def _assert_should_skip_without_warning(self, path, is_checker_none, 300 expected): 301 # Check the file type before asserting the return value. 302 checker = self._dispatcher.dispatch_processor(file_path=path, 303 handle_style_error=None, 304 min_confidence=3) 305 message = 'while checking: %s' % path 306 self.assertEquals(checker is None, is_checker_none, message) 307 self.assertEquals(self._dispatcher.should_skip_without_warning(path), 308 expected, message) 309 310 def test_should_skip_without_warning__true(self): 311 """Test should_skip_without_warning() for True return values.""" 312 # Check a file with NONE file type. 313 path = 'foo.asdf' # Non-sensical file extension. 314 self._assert_should_skip_without_warning(path, 315 is_checker_none=True, 316 expected=True) 317 318 # Check files with non-NONE file type. These examples must be 319 # drawn from the _SKIPPED_FILES_WITHOUT_WARNING configuration 320 # variable. 321 path = os.path.join('LayoutTests', 'foo.txt') 322 self._assert_should_skip_without_warning(path, 323 is_checker_none=False, 324 expected=True) 325 326 def test_should_skip_without_warning__false(self): 327 """Test should_skip_without_warning() for False return values.""" 328 paths = ['foo.txt', 329 os.path.join('LayoutTests', 'ChangeLog'), 330 ] 331 332 for path in paths: 333 self._assert_should_skip_without_warning(path, 334 is_checker_none=False, 335 expected=False) 314 336 315 337 … … 412 434 paths = [ 413 435 "ChangeLog", 436 "ChangeLog-2009-06-16", 437 "foo.ac", 438 "foo.cc", 439 "foo.cgi", 414 440 "foo.css", 441 "foo.exp", 442 "foo.flex", 443 "foo.gyp", 444 "foo.gypi", 415 445 "foo.html", 416 446 "foo.idl", 447 "foo.in", 417 448 "foo.js", 418 449 "foo.mm", 419 450 "foo.php", 451 "foo.pl", 420 452 "foo.pm", 453 "foo.pri", 454 "foo.pro", 455 "foo.rb", 456 "foo.sh", 421 457 "foo.txt", 422 "FooChangeLog.bak", 423 "WebCore/ChangeLog", 424 "WebCore/inspector/front-end/inspector.js", 425 "WebKitTools/Scripts/check-webkit-style", 458 "foo.wm", 459 "foo.xhtml", 460 "foo.y", 461 os.path.join("WebCore", "ChangeLog"), 462 os.path.join("WebCore", "inspector", "front-end", "inspector.js"), 463 os.path.join("WebKitTools", "Scripts", "check-webkit-style"), 426 464 ] 427 465 … … 442 480 paths = [ 443 481 "Makefile", 482 "foo.asdf", # Non-sensical file extension. 444 483 "foo.png", 445 484 "foo.exe", 485 "foo.vcproj", 446 486 ] 447 487 … … 727 767 def test_process__no_checker_dispatched(self): 728 768 """Test the process() method for a path with no dispatched checker.""" 729 self._processor.process(lines=['line1', 'line2'], 730 file_path='foo/do_not_process.txt', 731 line_numbers=[100]) 732 733 # As a sanity check, check that the carriage-return checker was 734 # instantiated. (This code path was already checked in other test 735 # methods in this test case.) 736 carriage_checker = self.carriage_checker 737 self.assertEquals(carriage_checker.lines, ['line1', 'line2']) 738 739 # Check that the style checker was not dispatched. 740 self.assertTrue(self._mock_dispatcher.dispatched_checker is None) 769 path = os.path.join('foo', 'do_not_process.txt') 770 self.assertRaises(AssertionError, self._processor.process, 771 lines=['line1', 'line2'], file_path=path, 772 line_numbers=[100]) 741 773 742 774
Note: See TracChangeset
for help on using the changeset viewer.