diff options
Diffstat (limited to 'WebKitTools/Scripts/webkitpy/style')
18 files changed, 647 insertions, 655 deletions
diff --git a/WebKitTools/Scripts/webkitpy/style/checker.py b/WebKitTools/Scripts/webkitpy/style/checker.py index 84ae3da..59a3d39 100644 --- a/WebKitTools/Scripts/webkitpy/style/checker.py +++ b/WebKitTools/Scripts/webkitpy/style/checker.py @@ -30,20 +30,19 @@ """Front end of some style-checker modules.""" -import codecs import logging import os.path import sys +from checkers.common import categories as CommonCategories +from checkers.common import CarriageReturnChecker +from checkers.cpp import CppChecker +from checkers.python import PythonChecker +from checkers.text import TextChecker from error_handlers import DefaultStyleErrorHandler from filter import FilterConfiguration from optparser import ArgumentParser from optparser import DefaultCommandOptionValues -from processors.common import categories as CommonCategories -from processors.common import CarriageReturnProcessor -from processors.cpp import CppProcessor -from processors.python import PythonProcessor -from processors.text import TextProcessor from webkitpy.style_references import parse_patch from webkitpy.style_references import configure_logging as _configure_logging @@ -158,11 +157,51 @@ _PATH_RULES_SPECIFIER = [ ] +_CPP_FILE_EXTENSIONS = [ + 'c', + 'cpp', + 'h', + ] + +_PYTHON_FILE_EXTENSION = 'py' + +# FIXME: Include 'vcproj' files as text files after creating a mechanism +# for exempting them from the carriage-return checker (since they +# are Windows-only files). +_TEXT_FILE_EXTENSIONS = [ + 'ac', + 'cc', + 'cgi', + 'css', + 'exp', + 'flex', + 'gyp', + 'gypi', + 'html', + 'idl', + 'in', + 'js', + 'mm', + 'php', + 'pl', + 'pm', + 'pri', + 'pro', + 'rb', + 'sh', + 'txt', +# 'vcproj', # See FIXME above. + 'wm', + 'xhtml', + 'y', + ] + + +# Files to skip that are less obvious. +# # Some files should be skipped when checking style. For example, # WebKit maintains some files in Mozilla style on purpose to ease # future merges. -# -# Include a warning for skipped files that are less obvious. _SKIPPED_FILES_WITH_WARNING = [ # The Qt API and tests do not follow WebKit style. # They follow Qt style. :) @@ -175,11 +214,12 @@ _SKIPPED_FILES_WITH_WARNING = [ ] -# Don't include a warning for skipped files that are more common -# and more obvious. +# Files to skip that are more common or obvious. +# +# This list should be in addition to files with FileType.NONE. Files +# with FileType.NONE are automatically skipped without warning. _SKIPPED_FILES_WITHOUT_WARNING = [ "LayoutTests/", - ".pyc", ] @@ -192,8 +232,8 @@ _MAX_REPORTS_PER_CATEGORY = { def _all_categories(): """Return the set of all categories used by check-webkit-style.""" - # Take the union across all processors. - categories = CommonCategories.union(CppProcessor.categories) + # Take the union across all checkers. + categories = CommonCategories.union(CppChecker.categories) # FIXME: Consider adding all of the pep8 categories. Since they # are not too meaningful for documentation purposes, for @@ -221,7 +261,7 @@ def check_webkit_style_parser(): def check_webkit_style_configuration(options): - """Return a StyleCheckerConfiguration instance for check-webkit-style. + """Return a StyleProcessorConfiguration instance for check-webkit-style. Args: options: A CommandOptionValues instance. @@ -232,7 +272,7 @@ def check_webkit_style_configuration(options): path_specific=_PATH_RULES_SPECIFIER, user_rules=options.filter_rules) - return StyleCheckerConfiguration(filter_configuration=filter_configuration, + return StyleProcessorConfiguration(filter_configuration=filter_configuration, max_reports_per_category=_MAX_REPORTS_PER_CATEGORY, min_confidence=options.min_confidence, output_format=options.output_format, @@ -330,34 +370,17 @@ def configure_logging(stream, logger=None, is_verbose=False): # Enum-like idiom class FileType: - NONE = 1 + NONE = 0 # FileType.NONE evaluates to False. # Alphabetize remaining types - CPP = 2 - PYTHON = 3 - TEXT = 4 + CPP = 1 + PYTHON = 2 + TEXT = 3 -class ProcessorDispatcher(object): +class CheckerDispatcher(object): """Supports determining whether and how to check style, based on path.""" - cpp_file_extensions = ( - 'c', - 'cpp', - 'h', - ) - - text_file_extensions = ( - 'css', - 'html', - 'idl', - 'js', - 'mm', - 'php', - 'pm', - 'txt', - ) - def _file_extension(self, file_path): """Return the file extension without the leading dot.""" return os.path.splitext(file_path)[1].lstrip(".") @@ -371,6 +394,16 @@ class ProcessorDispatcher(object): def should_skip_without_warning(self, file_path): """Return whether the given file should be skipped without a warning.""" + if not self._file_type(file_path): # FileType.NONE. + return True + # Since "LayoutTests" is in _SKIPPED_FILES_WITHOUT_WARNING, make + # an exception to prevent files like "LayoutTests/ChangeLog" and + # "LayoutTests/ChangeLog-2009-06-16" from being skipped. + # + # FIXME: Figure out a good way to avoid having to add special logic + # for this special case. + if os.path.basename(file_path).startswith('ChangeLog'): + return False for skipped_file in _SKIPPED_FILES_WITHOUT_WARNING: if file_path.find(skipped_file) >= 0: return True @@ -380,7 +413,7 @@ class ProcessorDispatcher(object): """Return the file type corresponding to the given file.""" file_extension = self._file_extension(file_path) - if (file_extension in self.cpp_file_extensions) or (file_path == '-'): + if (file_extension in _CPP_FILE_EXTENSIONS) or (file_path == '-'): # FIXME: Do something about the comment below and the issue it # raises since cpp_style already relies on the extension. # @@ -388,28 +421,28 @@ class ProcessorDispatcher(object): # reading from stdin, cpp_style tests should not rely on # the extension. return FileType.CPP - elif file_extension == "py": + elif file_extension == _PYTHON_FILE_EXTENSION: return FileType.PYTHON - elif ("ChangeLog" in file_path or + elif (os.path.basename(file_path).startswith('ChangeLog') or (not file_extension and "WebKitTools/Scripts/" in file_path) or - file_extension in self.text_file_extensions): + file_extension in _TEXT_FILE_EXTENSIONS): return FileType.TEXT else: return FileType.NONE - def _create_processor(self, file_type, file_path, handle_style_error, - min_confidence): - """Instantiate and return a style processor based on file type.""" + def _create_checker(self, file_type, file_path, handle_style_error, + min_confidence): + """Instantiate and return a style checker based on file type.""" if file_type == FileType.NONE: - processor = None + checker = None elif file_type == FileType.CPP: file_extension = self._file_extension(file_path) - processor = CppProcessor(file_path, file_extension, - handle_style_error, min_confidence) + checker = CppChecker(file_path, file_extension, + handle_style_error, min_confidence) elif file_type == FileType.PYTHON: - processor = PythonProcessor(file_path, handle_style_error) + checker = PythonChecker(file_path, handle_style_error) elif file_type == FileType.TEXT: - processor = TextProcessor(file_path, handle_style_error) + checker = TextChecker(file_path, handle_style_error) else: raise ValueError('Invalid file type "%(file_type)s": the only valid file types ' "are %(NONE)s, %(CPP)s, and %(TEXT)s." @@ -418,24 +451,24 @@ class ProcessorDispatcher(object): "CPP": FileType.CPP, "TEXT": FileType.TEXT}) - return processor + return checker - def dispatch_processor(self, file_path, handle_style_error, min_confidence): - """Instantiate and return a style processor based on file path.""" + def dispatch(self, file_path, handle_style_error, min_confidence): + """Instantiate and return a style checker based on file path.""" file_type = self._file_type(file_path) - processor = self._create_processor(file_type, - file_path, - handle_style_error, - min_confidence) - return processor + checker = self._create_checker(file_type, + file_path, + handle_style_error, + min_confidence) + return checker # FIXME: Remove the stderr_write attribute from this class and replace # its use with calls to a logging module logger. -class StyleCheckerConfiguration(object): +class StyleProcessorConfiguration(object): - """Stores configuration values for the StyleChecker class. + """Stores configuration values for the StyleProcessor class. Attributes: min_confidence: An integer between 1 and 5 inclusive that is the @@ -455,7 +488,7 @@ class StyleCheckerConfiguration(object): min_confidence, output_format, stderr_write): - """Create a StyleCheckerConfiguration instance. + """Create a StyleProcessorConfiguration instance. Args: filter_configuration: A FilterConfiguration instance. The default @@ -528,7 +561,13 @@ class ProcessorBase(object): """The base class for processors of lists of lines.""" def should_process(self, file_path): - """Return whether the file at file_path should be processed.""" + """Return whether the file at file_path should be processed. + + The TextFileReader class calls this method prior to reading in + the lines of a file. Use this method, for example, to prevent + the style checker from reading binary files into memory. + + """ raise NotImplementedError('Subclasses should implement.') def process(self, lines, file_path, **kwargs): @@ -540,7 +579,7 @@ class ProcessorBase(object): **kwargs: This argument signifies that the process() method of subclasses of ProcessorBase may support additional keyword arguments. - For example, a style processor's process() method + For example, a style checker's check() method may support a "reportable_lines" parameter that represents the line numbers of the lines for which style errors should be reported. @@ -549,209 +588,129 @@ class ProcessorBase(object): raise NotImplementedError('Subclasses should implement.') -# FIXME: Modify this class to start using the TextFileReader class in -# webkitpy/style/filereader.py. This probably means creating -# a StyleProcessor class that inherits from ProcessorBase. -class StyleChecker(object): +class StyleProcessor(ProcessorBase): - """Supports checking style in files and patches. + """A ProcessorBase for checking style. - Attributes: - error_count: An integer that is the total number of reported - errors for the lifetime of this StyleChecker - instance. - file_count: An integer that is the total number of processed - files. Note that the number of skipped files is - included in this value. + Attributes: + error_count: An integer that is the total number of reported + errors for the lifetime of this instance. """ - def __init__(self, configuration): - """Create a StyleChecker instance. + def __init__(self, configuration, mock_dispatcher=None, + mock_increment_error_count=None, + mock_carriage_checker_class=None): + """Create an instance. Args: - configuration: A StyleCheckerConfiguration instance that controls - the behavior of style checking. + configuration: A StyleProcessorConfiguration instance. + mock_dispatcher: A mock CheckerDispatcher instance. This + parameter is for unit testing. Defaults to a + CheckerDispatcher instance. + mock_increment_error_count: A mock error-count incrementer. + mock_carriage_checker_class: A mock class for checking and + transforming carriage returns. + This parameter is for unit testing. + Defaults to CarriageReturnChecker. """ - self._configuration = configuration - self.error_count = 0 - self.file_count = 0 - - def _increment_error_count(self): - """Increment the total count of reported errors.""" - self.error_count += 1 - - def _read_lines(self, file_path): - """Read the file at a path, and return its lines. - - Raises: - IOError: if the file does not exist or cannot be read. - - """ - # Support the UNIX convention of using "-" for stdin. - if file_path == '-': - file = codecs.StreamReaderWriter(sys.stdin, - codecs.getreader('utf8'), - codecs.getwriter('utf8'), - 'replace') + if mock_dispatcher is None: + dispatcher = CheckerDispatcher() else: - # We do not open the file with universal newline support - # (codecs does not support it anyway), so the resulting - # lines contain trailing "\r" characters if we are reading - # a file with CRLF endings. - file = codecs.open(file_path, 'r', 'utf8', 'replace') - - contents = file.read() - - lines = contents.split("\n") - return lines - - def _process_file(self, processor, file_path, handle_style_error): - """Process the file using the given style processor.""" - try: - lines = self._read_lines(file_path) - except IOError: - message = 'Could not read file. Skipping: "%s"' % file_path - _log.warn(message) - return - - # Check for and remove trailing carriage returns ("\r"). - # - # FIXME: We should probably use the SVN "eol-style" property - # or a white list to decide whether or not to do - # the carriage-return check. Originally, we did the - # check only if (os.linesep != '\r\n'). - carriage_return_processor = CarriageReturnProcessor(handle_style_error) - lines = carriage_return_processor.process(lines) - - processor.process(lines) + dispatcher = mock_dispatcher - def check_paths(self, paths, mock_check_file=None, mock_os=None): - """Check style in the given files or directories. + if mock_increment_error_count is None: + # The following blank line is present to avoid flagging by pep8.py. - Args: - paths: A list of file paths and directory paths. - mock_check_file: A mock of self.check_file for unit testing. - mock_os: A mock os for unit testing. - - """ - check_file = self.check_file if mock_check_file is None else \ - mock_check_file - os_module = os if mock_os is None else mock_os - - for path in paths: - if os_module.path.isdir(path): - self._check_directory(directory=path, - check_file=check_file, - mock_os_walk=os_module.walk) - else: - check_file(path) + def increment_error_count(): + """Increment the total count of reported errors.""" + self.error_count += 1 + else: + increment_error_count = mock_increment_error_count - def _check_directory(self, directory, check_file, mock_os_walk=None): - """Check style in all files in a directory, recursively. + if mock_carriage_checker_class is None: + # This needs to be a class rather than an instance since the + # process() method instantiates one using parameters. + carriage_checker_class = CarriageReturnChecker + else: + carriage_checker_class = mock_carriage_checker_class - Args: - directory: A path to a directory. - check_file: The function to use in place of self.check_file(). - mock_os_walk: A mock os.walk for unit testing. + self.error_count = 0 - """ - os_walk = os.walk if mock_os_walk is None else mock_os_walk + self._carriage_checker_class = carriage_checker_class + self._configuration = configuration + self._dispatcher = dispatcher + self._increment_error_count = increment_error_count - for dir_path, dir_names, file_names in os_walk(directory): - for file_name in file_names: - file_path = os.path.join(dir_path, file_name) - check_file(file_path) + def should_process(self, file_path): + """Return whether the file should be checked for style.""" + if self._dispatcher.should_skip_without_warning(file_path): + return False + if self._dispatcher.should_skip_with_warning(file_path): + _log.warn('File exempt from style guide. Skipping: "%s"' + % file_path) + return False + return True - def check_file(self, file_path, line_numbers=None, - mock_handle_style_error=None, - mock_os_path_exists=None, - mock_process_file=None): - """Check style in the given file. + def process(self, lines, file_path, line_numbers=None): + """Check the given lines for style. - Args: + Arguments: + lines: A list of all lines in the file to check. file_path: The path of the file to process. If possible, the path should be relative to the source root. Otherwise, path-specific logic may not behave as expected. - line_numbers: An array of line numbers of the lines for which + line_numbers: A list of line numbers of the lines for which style errors should be reported, or None if errors - for all lines should be reported. Normally, this - array contains the line numbers corresponding to the - modified lines of a patch. - mock_handle_style_error: A unit-testing replacement for the function - to call when a style error occurs. Defaults - to a DefaultStyleErrorHandler instance. - mock_os_path_exists: A unit-test replacement for os.path.exists. - This parameter should only be used for unit - tests. - mock_process_file: The function to call to process the file. This - parameter should be used only for unit tests. - Defaults to the file processing method of this - class. - - Raises: - SystemExit: if the file does not exist. + for all lines should be reported. When not None, this + list normally contains the line numbers corresponding + to the modified lines of a patch. """ - if mock_handle_style_error is None: - increment = self._increment_error_count - handle_style_error = DefaultStyleErrorHandler( - configuration=self._configuration, - file_path=file_path, - increment_error_count=increment, - line_numbers=line_numbers) - else: - handle_style_error = mock_handle_style_error - - os_path_exists = (os.path.exists if mock_os_path_exists is None else - mock_os_path_exists) - process_file = (self._process_file if mock_process_file is None else - mock_process_file) - - if not os_path_exists(file_path) and file_path != "-": - _log.error("File does not exist: %s" % file_path) - sys.exit(1) + _log.debug("Checking style: " + file_path) - _log.debug("Checking: " + file_path) + style_error_handler = DefaultStyleErrorHandler( + configuration=self._configuration, + file_path=file_path, + increment_error_count=self._increment_error_count, + line_numbers=line_numbers) - self.file_count += 1 + carriage_checker = self._carriage_checker_class(style_error_handler) - dispatcher = ProcessorDispatcher() - - if dispatcher.should_skip_without_warning(file_path): - return - if dispatcher.should_skip_with_warning(file_path): - _log.warn('File exempt from style guide. Skipping: "%s"' - % file_path) - return + # FIXME: We should probably use the SVN "eol-style" property + # or a white list to decide whether or not to do + # the carriage-return check. Originally, we did the + # check only if (os.linesep != '\r\n'). + # + # Check for and remove trailing carriage returns ("\r"). + lines = carriage_checker.check(lines) min_confidence = self._configuration.min_confidence - processor = dispatcher.dispatch_processor(file_path, - handle_style_error, - min_confidence) - if processor is None: - _log.debug('File not a recognized type to check. Skipping: "%s"' - % file_path) - return + checker = self._dispatcher.dispatch(file_path, + style_error_handler, + min_confidence) + + if checker is None: + raise AssertionError("File should not be checked: '%s'" % file_path) - _log.debug("Using class: " + processor.__class__.__name__) + _log.debug("Using class: " + checker.__class__.__name__) - process_file(processor, file_path, handle_style_error) + checker.check(lines) -class PatchChecker(object): +class PatchReader(object): """Supports checking style in patches.""" - def __init__(self, style_checker): - """Create a PatchChecker instance. + def __init__(self, text_file_reader): + """Create a PatchReader instance. Args: - style_checker: A StyleChecker instance. + text_file_reader: A TextFileReader instance. """ - self._file_checker = style_checker + self._text_file_reader = text_file_reader def check(self, patch_string): """Check style in the given patch.""" @@ -775,5 +734,5 @@ class PatchChecker(object): # This optimization also prevents the program from exiting # due to a deleted file. if line_numbers: - self._file_checker.check_file(file_path=path, - line_numbers=line_numbers) + self._text_file_reader.process_file(file_path=path, + line_numbers=line_numbers) diff --git a/WebKitTools/Scripts/webkitpy/style/checker_unittest.py b/WebKitTools/Scripts/webkitpy/style/checker_unittest.py index 401a7b3..6e1eaa2 100755 --- a/WebKitTools/Scripts/webkitpy/style/checker_unittest.py +++ b/WebKitTools/Scripts/webkitpy/style/checker_unittest.py @@ -49,18 +49,21 @@ from checker import _all_categories from checker import check_webkit_style_configuration from checker import check_webkit_style_parser from checker import configure_logging -from checker import ProcessorDispatcher -from checker import PatchChecker -from checker import StyleChecker -from checker import StyleCheckerConfiguration +from checker import CheckerDispatcher +from checker import PatchReader +from checker import ProcessorBase +from checker import StyleProcessor +from checker import StyleProcessorConfiguration +from checkers.cpp import CppChecker +from checkers.python import PythonChecker +from checkers.text import TextChecker +from error_handlers import DefaultStyleErrorHandler from filter import validate_filter_rules from filter import FilterConfiguration from optparser import ArgumentParser from optparser import CommandOptionValues -from processors.cpp import CppProcessor -from processors.python import PythonProcessor -from processors.text import TextProcessor from webkitpy.common.system.logtesting import LoggingTestCase +from webkitpy.style.filereader import TextFileReader class ConfigureLoggingTestBase(unittest.TestCase): @@ -265,16 +268,17 @@ class CheckWebKitStyleFunctionTest(unittest.TestCase): parser = check_webkit_style_parser() -class ProcessorDispatcherSkipTest(unittest.TestCase): +class CheckerDispatcherSkipTest(unittest.TestCase): - """Tests the "should skip" methods of the ProcessorDispatcher class.""" + """Tests the "should skip" methods of the CheckerDispatcher class.""" + + def setUp(self): + self._dispatcher = CheckerDispatcher() def test_should_skip_with_warning(self): """Test should_skip_with_warning().""" - dispatcher = ProcessorDispatcher() - # Check a non-skipped file. - self.assertFalse(dispatcher.should_skip_with_warning("foo.txt")) + self.assertFalse(self._dispatcher.should_skip_with_warning("foo.txt")) # Check skipped files. paths_to_skip = [ @@ -289,51 +293,72 @@ class ProcessorDispatcherSkipTest(unittest.TestCase): ] for path in paths_to_skip: - self.assertTrue(dispatcher.should_skip_with_warning(path), + self.assertTrue(self._dispatcher.should_skip_with_warning(path), "Checking: " + path) - def test_should_skip_without_warning(self): - """Test should_skip_without_warning().""" - dispatcher = ProcessorDispatcher() - - # Check a non-skipped file. - self.assertFalse(dispatcher.should_skip_without_warning("foo.txt")) - - # Check skipped files. - paths_to_skip = [ - # LayoutTests folder - "LayoutTests/foo.txt", - ] + def _assert_should_skip_without_warning(self, path, is_checker_none, + expected): + # Check the file type before asserting the return value. + checker = self._dispatcher.dispatch(file_path=path, + handle_style_error=None, + min_confidence=3) + message = 'while checking: %s' % path + self.assertEquals(checker is None, is_checker_none, message) + self.assertEquals(self._dispatcher.should_skip_without_warning(path), + expected, message) + + def test_should_skip_without_warning__true(self): + """Test should_skip_without_warning() for True return values.""" + # Check a file with NONE file type. + path = 'foo.asdf' # Non-sensical file extension. + self._assert_should_skip_without_warning(path, + is_checker_none=True, + expected=True) + + # Check files with non-NONE file type. These examples must be + # drawn from the _SKIPPED_FILES_WITHOUT_WARNING configuration + # variable. + path = os.path.join('LayoutTests', 'foo.txt') + self._assert_should_skip_without_warning(path, + is_checker_none=False, + expected=True) + + def test_should_skip_without_warning__false(self): + """Test should_skip_without_warning() for False return values.""" + paths = ['foo.txt', + os.path.join('LayoutTests', 'ChangeLog'), + ] - for path in paths_to_skip: - self.assertTrue(dispatcher.should_skip_without_warning(path), - "Checking: " + path) + for path in paths: + self._assert_should_skip_without_warning(path, + is_checker_none=False, + expected=False) -class ProcessorDispatcherDispatchTest(unittest.TestCase): +class CheckerDispatcherDispatchTest(unittest.TestCase): - """Tests dispatch_processor() method of ProcessorDispatcher class.""" + """Tests dispatch() method of CheckerDispatcher class.""" def mock_handle_style_error(self): pass - def dispatch_processor(self, file_path): - """Call dispatch_processor() with the given file path.""" - dispatcher = ProcessorDispatcher() - processor = dispatcher.dispatch_processor(file_path, - self.mock_handle_style_error, - min_confidence=3) - return processor - - def assert_processor_none(self, file_path): - """Assert that the dispatched processor is None.""" - processor = self.dispatch_processor(file_path) - self.assertTrue(processor is None, 'Checking: "%s"' % file_path) - - def assert_processor(self, file_path, expected_class): - """Assert the type of the dispatched processor.""" - processor = self.dispatch_processor(file_path) - got_class = processor.__class__ + def dispatch(self, file_path): + """Call dispatch() with the given file path.""" + dispatcher = CheckerDispatcher() + checker = dispatcher.dispatch(file_path, + self.mock_handle_style_error, + min_confidence=3) + return checker + + def assert_checker_none(self, file_path): + """Assert that the dispatched checker is None.""" + checker = self.dispatch(file_path) + self.assertTrue(checker is None, 'Checking: "%s"' % file_path) + + def assert_checker(self, file_path, expected_class): + """Assert the type of the dispatched checker.""" + checker = self.dispatch(file_path) + got_class = checker.__class__ self.assertEquals(got_class, expected_class, 'For path "%(file_path)s" got %(got_class)s when ' "expecting %(expected_class)s." @@ -341,17 +366,17 @@ class ProcessorDispatcherDispatchTest(unittest.TestCase): "got_class": got_class, "expected_class": expected_class}) - def assert_processor_cpp(self, file_path): - """Assert that the dispatched processor is a CppProcessor.""" - self.assert_processor(file_path, CppProcessor) + def assert_checker_cpp(self, file_path): + """Assert that the dispatched checker is a CppChecker.""" + self.assert_checker(file_path, CppChecker) - def assert_processor_python(self, file_path): - """Assert that the dispatched processor is a PythonProcessor.""" - self.assert_processor(file_path, PythonProcessor) + def assert_checker_python(self, file_path): + """Assert that the dispatched checker is a PythonChecker.""" + self.assert_checker(file_path, PythonChecker) - def assert_processor_text(self, file_path): - """Assert that the dispatched processor is a TextProcessor.""" - self.assert_processor(file_path, TextProcessor) + def assert_checker_text(self, file_path): + """Assert that the dispatched checker is a TextChecker.""" + self.assert_checker(file_path, TextChecker) def test_cpp_paths(self): """Test paths that should be checked as C++.""" @@ -363,26 +388,26 @@ class ProcessorDispatcherDispatchTest(unittest.TestCase): ] for path in paths: - self.assert_processor_cpp(path) + self.assert_checker_cpp(path) - # Check processor attributes on a typical input. + # Check checker attributes on a typical input. file_base = "foo" file_extension = "c" file_path = file_base + "." + file_extension - self.assert_processor_cpp(file_path) - processor = self.dispatch_processor(file_path) - self.assertEquals(processor.file_extension, file_extension) - self.assertEquals(processor.file_path, file_path) - self.assertEquals(processor.handle_style_error, self.mock_handle_style_error) - self.assertEquals(processor.min_confidence, 3) + self.assert_checker_cpp(file_path) + checker = self.dispatch(file_path) + self.assertEquals(checker.file_extension, file_extension) + self.assertEquals(checker.file_path, file_path) + self.assertEquals(checker.handle_style_error, self.mock_handle_style_error) + self.assertEquals(checker.min_confidence, 3) # Check "-" for good measure. file_base = "-" file_extension = "" file_path = file_base - self.assert_processor_cpp(file_path) - processor = self.dispatch_processor(file_path) - self.assertEquals(processor.file_extension, file_extension) - self.assertEquals(processor.file_path, file_path) + self.assert_checker_cpp(file_path) + checker = self.dispatch(file_path) + self.assertEquals(checker.file_extension, file_extension) + self.assertEquals(checker.file_path, file_path) def test_python_paths(self): """Test paths that should be checked as Python.""" @@ -392,63 +417,81 @@ class ProcessorDispatcherDispatchTest(unittest.TestCase): ] for path in paths: - self.assert_processor_python(path) + self.assert_checker_python(path) - # Check processor attributes on a typical input. + # Check checker attributes on a typical input. file_base = "foo" file_extension = "css" file_path = file_base + "." + file_extension - self.assert_processor_text(file_path) - processor = self.dispatch_processor(file_path) - self.assertEquals(processor.file_path, file_path) - self.assertEquals(processor.handle_style_error, + self.assert_checker_text(file_path) + checker = self.dispatch(file_path) + self.assertEquals(checker.file_path, file_path) + self.assertEquals(checker.handle_style_error, self.mock_handle_style_error) def test_text_paths(self): """Test paths that should be checked as text.""" paths = [ "ChangeLog", + "ChangeLog-2009-06-16", + "foo.ac", + "foo.cc", + "foo.cgi", "foo.css", + "foo.exp", + "foo.flex", + "foo.gyp", + "foo.gypi", "foo.html", "foo.idl", + "foo.in", "foo.js", "foo.mm", "foo.php", + "foo.pl", "foo.pm", + "foo.pri", + "foo.pro", + "foo.rb", + "foo.sh", "foo.txt", - "FooChangeLog.bak", - "WebCore/ChangeLog", - "WebCore/inspector/front-end/inspector.js", - "WebKitTools/Scripts/check-webkit-style", + "foo.wm", + "foo.xhtml", + "foo.y", + os.path.join("WebCore", "ChangeLog"), + os.path.join("WebCore", "inspector", "front-end", "inspector.js"), + os.path.join("WebKitTools", "Scripts", "check-webkit-style"), ] for path in paths: - self.assert_processor_text(path) + self.assert_checker_text(path) - # Check processor attributes on a typical input. + # Check checker attributes on a typical input. file_base = "foo" file_extension = "css" file_path = file_base + "." + file_extension - self.assert_processor_text(file_path) - processor = self.dispatch_processor(file_path) - self.assertEquals(processor.file_path, file_path) - self.assertEquals(processor.handle_style_error, self.mock_handle_style_error) + self.assert_checker_text(file_path) + checker = self.dispatch(file_path) + self.assertEquals(checker.file_path, file_path) + self.assertEquals(checker.handle_style_error, self.mock_handle_style_error) def test_none_paths(self): """Test paths that have no file type..""" paths = [ "Makefile", + "foo.asdf", # Non-sensical file extension. "foo.png", "foo.exe", + "foo.vcproj", ] for path in paths: - self.assert_processor_none(path) + self.assert_checker_none(path) -class StyleCheckerConfigurationTest(unittest.TestCase): +class StyleProcessorConfigurationTest(unittest.TestCase): - """Tests the StyleCheckerConfiguration class.""" + """Tests the StyleProcessorConfiguration class.""" def setUp(self): self._error_messages = [] @@ -458,11 +501,11 @@ class StyleCheckerConfigurationTest(unittest.TestCase): self._error_messages.append(message) def _style_checker_configuration(self, output_format="vs7"): - """Return a StyleCheckerConfiguration instance for testing.""" + """Return a StyleProcessorConfiguration instance for testing.""" base_rules = ["-whitespace", "+whitespace/tab"] filter_configuration = FilterConfiguration(base_rules=base_rules) - return StyleCheckerConfiguration( + return StyleProcessorConfiguration( filter_configuration=filter_configuration, max_reports_per_category={"whitespace/newline": 1}, min_confidence=3, @@ -512,277 +555,246 @@ class StyleCheckerConfigurationTest(unittest.TestCase): ["foo.h(100): message [whitespace/tab] [5]\n"]) -class StyleCheckerTest(unittest.TestCase): +class StyleProcessor_EndToEndTest(LoggingTestCase): - """Test the StyleChecker class.""" + """Test the StyleProcessor class with an emphasis on end-to-end tests.""" - def _mock_stderr_write(self, message): - pass + def setUp(self): + LoggingTestCase.setUp(self) + self._messages = [] - def _style_checker(self, configuration): - return StyleChecker(configuration) + def _mock_stderr_write(self, message): + """Save a message so it can later be asserted.""" + self._messages.append(message) def test_init(self): """Test __init__ constructor.""" - configuration = StyleCheckerConfiguration( + configuration = StyleProcessorConfiguration( filter_configuration=FilterConfiguration(), max_reports_per_category={}, min_confidence=3, output_format="vs7", stderr_write=self._mock_stderr_write) + processor = StyleProcessor(configuration) - style_checker = self._style_checker(configuration) - - self.assertEquals(style_checker._configuration, configuration) - self.assertEquals(style_checker.error_count, 0) - self.assertEquals(style_checker.file_count, 0) - - -class StyleCheckerCheckFileBase(LoggingTestCase): - - def setUp(self): - LoggingTestCase.setUp(self) - self.warning_messages = "" - - def mock_stderr_write(self, warning_message): - self.warning_messages += warning_message - - def _style_checker_configuration(self): - return StyleCheckerConfiguration( - filter_configuration=FilterConfiguration(), - max_reports_per_category={"whitespace/newline": 1}, - min_confidence=3, - output_format="vs7", - stderr_write=self.mock_stderr_write) + self.assertEquals(processor.error_count, 0) + self.assertEquals(self._messages, []) + def test_process(self): + configuration = StyleProcessorConfiguration( + filter_configuration=FilterConfiguration(), + max_reports_per_category={}, + min_confidence=3, + output_format="vs7", + stderr_write=self._mock_stderr_write) + processor = StyleProcessor(configuration) -class StyleCheckerCheckFileTest(StyleCheckerCheckFileBase): + processor.process(lines=['line1', 'Line with tab:\t'], + file_path='foo.txt') + self.assertEquals(processor.error_count, 1) + expected_messages = ['foo.txt(2): Line contains tab character. ' + '[whitespace/tab] [5]\n'] + self.assertEquals(self._messages, expected_messages) - """Test the check_file() method of the StyleChecker class. - The check_file() method calls its process_file parameter when - given a file that should not be skipped. +class StyleProcessor_CodeCoverageTest(LoggingTestCase): - The "got_*" attributes of this class are the parameters passed - to process_file by calls to check_file() made by this test - class. These attributes allow us to check the parameter values - passed internally to the process_file function. + """Test the StyleProcessor class with an emphasis on code coverage. - Attributes: - got_file_path: The file_path parameter passed by check_file() - to its process_file parameter. - got_handle_style_error: The handle_style_error parameter passed - by check_file() to its process_file - parameter. - got_processor: The processor parameter passed by check_file() to - its process_file parameter. - warning_messages: A string containing all of the warning messages - written to the mock_stderr_write method of - this class. + This class makes heavy use of mock objects. """ - def setUp(self): - StyleCheckerCheckFileBase.setUp(self) - self.got_file_path = None - self.got_handle_style_error = None - self.got_processor = None - def mock_handle_style_error(self): - pass - - def mock_os_path_exists(self, path): - # We deliberately make it so that this method returns False unless - # the caller has made an effort to put "does_exist" in the path. - return path.find("does_exist") > -1 - - def mock_process_file(self, processor, file_path, handle_style_error): - """A mock _process_file(). + class MockDispatchedChecker(object): - See the documentation for this class for more information - on this function. + """A mock checker dispatched by the MockDispatcher.""" - """ - self.got_file_path = file_path - self.got_handle_style_error = handle_style_error - self.got_processor = processor - - def assert_attributes(self, - expected_file_path, - expected_handle_style_error, - expected_processor, - expected_warning_messages): - """Assert that the attributes of this class equal the given values.""" - self.assertEquals(self.got_file_path, expected_file_path) - self.assertEquals(self.got_handle_style_error, expected_handle_style_error) - self.assertEquals(self.got_processor, expected_processor) - self.assertEquals(self.warning_messages, expected_warning_messages) - - def call_check_file(self, file_path): - """Call the check_file() method of a test StyleChecker instance.""" - # Confirm that the attributes are reset. - self.assert_attributes(None, None, None, "") - - configuration = self._style_checker_configuration() + def __init__(self, file_path, min_confidence, style_error_handler): + self.file_path = file_path + self.min_confidence = min_confidence + self.style_error_handler = style_error_handler - style_checker = StyleChecker(configuration) + def check(self, lines): + self.lines = lines - style_checker.check_file(file_path=file_path, - mock_handle_style_error=self.mock_handle_style_error, - mock_os_path_exists=self.mock_os_path_exists, - mock_process_file=self.mock_process_file) + class MockDispatcher(object): - self.assertEquals(style_checker.file_count, 1) + """A mock CheckerDispatcher class.""" - def test_check_file_does_not_exist(self): - file_path = "file_does_not_exist.txt" - - # Confirm that the file does not exist. - self.assertFalse(self.mock_os_path_exists(file_path)) + def __init__(self): + self.dispatched_checker = None - # Check the outcome. - self.assertRaises(SystemExit, self.call_check_file, file_path) - self.assertLog(["ERROR: File does not exist: " - "file_does_not_exist.txt\n"]) + def should_skip_with_warning(self, file_path): + return file_path.endswith('skip_with_warning.txt') - def test_check_file_stdin(self): - file_path = "-" + def should_skip_without_warning(self, file_path): + return file_path.endswith('skip_without_warning.txt') - # Confirm that the file does not exist. - self.assertFalse(self.mock_os_path_exists(file_path)) + def dispatch(self, file_path, style_error_handler, min_confidence): + if file_path.endswith('do_not_process.txt'): + return None - # Check the outcome. - self.call_check_file(file_path) - expected_processor = CppProcessor(file_path, - "", - self.mock_handle_style_error, 3) - self.assert_attributes(file_path, - self.mock_handle_style_error, - expected_processor, - "") + checker = StyleProcessor_CodeCoverageTest.MockDispatchedChecker( + file_path, + min_confidence, + style_error_handler) - def test_check_file_on_skip_without_warning(self): - """Test check_file() for a skipped-without-warning file.""" + # Save the dispatched checker so the current test case has a + # way to access and check it. + self.dispatched_checker = checker - file_path = "LayoutTests/does_exist/foo.txt" + return checker - dispatcher = ProcessorDispatcher() - # Confirm that the input file is truly a skipped-without-warning file. - self.assertTrue(dispatcher.should_skip_without_warning(file_path)) + def setUp(self): + LoggingTestCase.setUp(self) + # We can pass an error-message swallower here because error message + # output is tested instead in the end-to-end test case above. + configuration = StyleProcessorConfiguration( + filter_configuration=FilterConfiguration(), + max_reports_per_category={"whitespace/newline": 1}, + min_confidence=3, + output_format="vs7", + stderr_write=self._swallow_stderr_message) + + mock_carriage_checker_class = self._create_carriage_checker_class() + mock_dispatcher = self.MockDispatcher() + # We do not need to use a real incrementer here because error-count + # incrementing is tested instead in the end-to-end test case above. + mock_increment_error_count = self._do_nothing + + processor = StyleProcessor(configuration=configuration, + mock_carriage_checker_class=mock_carriage_checker_class, + mock_dispatcher=mock_dispatcher, + mock_increment_error_count=mock_increment_error_count) + + self._configuration = configuration + self._mock_dispatcher = mock_dispatcher + self._processor = processor + + def _do_nothing(self): + # We provide this function so the caller can pass it to the + # StyleProcessor constructor. This lets us assert the equality of + # the DefaultStyleErrorHandler instance generated by the process() + # method with an expected instance. + pass - # Check the outcome. - self.call_check_file(file_path) - self.assert_attributes(None, None, None, "") + def _swallow_stderr_message(self, message): + """Swallow a message passed to stderr.write().""" + # This is a mock stderr.write() for passing to the constructor + # of the StyleProcessorConfiguration class. + pass - def test_check_file_on_skip_with_warning(self): - """Test check_file() for a skipped-with-warning file.""" + def _create_carriage_checker_class(self): - file_path = "does_exist/gtk2drawing.c" + # Create a reference to self with a new name so its name does not + # conflict with the self introduced below. + test_case = self - dispatcher = ProcessorDispatcher() - # Check that the input file is truly a skipped-with-warning file. - self.assertTrue(dispatcher.should_skip_with_warning(file_path)) + class MockCarriageChecker(object): - # Check the outcome. - self.call_check_file(file_path) - self.assert_attributes(None, None, None, "") - self.assertLog(["WARNING: File exempt from style guide. " - 'Skipping: "does_exist/gtk2drawing.c"\n']) + """A mock carriage-return checker.""" - def test_check_file_on_non_skipped(self): + def __init__(self, style_error_handler): + self.style_error_handler = style_error_handler - # We use a C++ file since by using a CppProcessor, we can check - # that all of the possible information is getting passed to - # process_file (in particular, the min_confidence parameter). - file_base = "foo_does_exist" - file_extension = "cpp" - file_path = file_base + "." + file_extension + # This gives the current test case access to the + # instantiated carriage checker. + test_case.carriage_checker = self - dispatcher = ProcessorDispatcher() - # Check that the input file is truly a C++ file. - self.assertEquals(dispatcher._file_type(file_path), style.FileType.CPP) + def check(self, lines): + # Save the lines so the current test case has a way to access + # and check them. + self.lines = lines - # Check the outcome. - self.call_check_file(file_path) + return lines - expected_processor = CppProcessor(file_path, file_extension, self.mock_handle_style_error, 3) + return MockCarriageChecker - self.assert_attributes(file_path, - self.mock_handle_style_error, - expected_processor, - "") + def test_should_process__skip_without_warning(self): + """Test should_process() for a skip-without-warning file.""" + file_path = "foo/skip_without_warning.txt" + self.assertFalse(self._processor.should_process(file_path)) -class StyleCheckerCheckPathsTest(unittest.TestCase): + def test_should_process__skip_with_warning(self): + """Test should_process() for a skip-with-warning file.""" + file_path = "foo/skip_with_warning.txt" - """Test the check_paths() method of the StyleChecker class.""" + self.assertFalse(self._processor.should_process(file_path)) - class MockOs(object): + self.assertLog(['WARNING: File exempt from style guide. ' + 'Skipping: "foo/skip_with_warning.txt"\n']) - class MockPath(object): + def test_should_process__true_result(self): + """Test should_process() for a file that should be processed.""" + file_path = "foo/skip_process.txt" - """A mock os.path.""" + self.assertTrue(self._processor.should_process(file_path)) - def isdir(self, path): - return path == "directory" + def test_process__checker_dispatched(self): + """Test the process() method for a path with a dispatched checker.""" + file_path = 'foo.txt' + lines = ['line1', 'line2'] + line_numbers = [100] - def __init__(self): - self.path = self.MockPath() + expected_error_handler = DefaultStyleErrorHandler( + configuration=self._configuration, + file_path=file_path, + increment_error_count=self._do_nothing, + line_numbers=line_numbers) - def walk(self, directory): - """A mock of os.walk.""" - if directory == "directory": - dirs = [("dir_path1", [], ["file1", "file2"]), - ("dir_path2", [], ["file3"])] - return dirs - return None + self._processor.process(lines=lines, + file_path=file_path, + line_numbers=line_numbers) - def setUp(self): - self._checked_files = [] + # Check that the carriage-return checker was instantiated correctly + # and was passed lines correctly. + carriage_checker = self.carriage_checker + self.assertEquals(carriage_checker.style_error_handler, + expected_error_handler) + self.assertEquals(carriage_checker.lines, ['line1', 'line2']) - def _mock_check_file(self, file): - self._checked_files.append(file) + # Check that the style checker was dispatched correctly and was + # passed lines correctly. + checker = self._mock_dispatcher.dispatched_checker + self.assertEquals(checker.file_path, 'foo.txt') + self.assertEquals(checker.min_confidence, 3) + self.assertEquals(checker.style_error_handler, expected_error_handler) - def test_check_paths(self): - """Test StyleChecker.check_paths().""" - checker = StyleChecker(configuration=None) - mock_check_file = self._mock_check_file - mock_os = self.MockOs() + self.assertEquals(checker.lines, ['line1', 'line2']) - # Confirm that checked files is empty at the outset. - self.assertEquals(self._checked_files, []) - checker.check_paths(["path1", "directory"], - mock_check_file=mock_check_file, - mock_os=mock_os) - self.assertEquals(self._checked_files, - ["path1", - os.path.join("dir_path1", "file1"), - os.path.join("dir_path1", "file2"), - os.path.join("dir_path2", "file3")]) + def test_process__no_checker_dispatched(self): + """Test the process() method for a path with no dispatched checker.""" + path = os.path.join('foo', 'do_not_process.txt') + self.assertRaises(AssertionError, self._processor.process, + lines=['line1', 'line2'], file_path=path, + line_numbers=[100]) -class PatchCheckerTest(unittest.TestCase): +class PatchReaderTest(unittest.TestCase): - """Test the PatchChecker class.""" + """Test the PatchReader class.""" - class MockStyleChecker(object): + class MockTextFileReader(object): def __init__(self): - self.checked_files = [] + self.passed_to_process_file = [] """A list of (file_path, line_numbers) pairs.""" - def check_file(self, file_path, line_numbers): - self.checked_files.append((file_path, line_numbers)) + def process_file(self, file_path, line_numbers): + self.passed_to_process_file.append((file_path, line_numbers)) def setUp(self): - style_checker = self.MockStyleChecker() - self._style_checker = style_checker - self._patch_checker = PatchChecker(style_checker) + file_reader = self.MockTextFileReader() + self._file_reader = file_reader + self._patch_checker = PatchReader(file_reader) def _call_check_patch(self, patch_string): self._patch_checker.check(patch_string) - def _assert_checked(self, checked_files): - self.assertEquals(self._style_checker.checked_files, checked_files) + def _assert_checked(self, passed_to_process_file): + self.assertEquals(self._file_reader.passed_to_process_file, + passed_to_process_file) def test_check_patch(self): # The modified line_numbers array for this patch is: [2]. diff --git a/WebKitTools/Scripts/webkitpy/style/processors/__init__.py b/WebKitTools/Scripts/webkitpy/style/checkers/__init__.py index ef65bee..ef65bee 100644 --- a/WebKitTools/Scripts/webkitpy/style/processors/__init__.py +++ b/WebKitTools/Scripts/webkitpy/style/checkers/__init__.py diff --git a/WebKitTools/Scripts/webkitpy/style/processors/common.py b/WebKitTools/Scripts/webkitpy/style/checkers/common.py index 30b8fed..a2d933f 100644 --- a/WebKitTools/Scripts/webkitpy/style/processors/common.py +++ b/WebKitTools/Scripts/webkitpy/style/checkers/common.py @@ -23,7 +23,7 @@ """Supports style checking not specific to any one file type.""" -# FIXME: Test this list in the same way that the list of CppProcessor +# FIXME: Test this list in the same way that the list of CppChecker # categories is tested, for example by checking that all of its # elements appear in the unit tests. This should probably be done # after moving the relevant cpp_unittest.ErrorCollector code @@ -33,14 +33,14 @@ categories = set([ ]) -class CarriageReturnProcessor(object): +class CarriageReturnChecker(object): """Supports checking for and handling carriage returns.""" def __init__(self, handle_style_error): self._handle_style_error = handle_style_error - def process(self, lines): + def check(self, lines): """Check for and strip trailing carriage returns from lines.""" for line_number in range(len(lines)): if not lines[line_number].endswith("\r"): diff --git a/WebKitTools/Scripts/webkitpy/style/processors/common_unittest.py b/WebKitTools/Scripts/webkitpy/style/checkers/common_unittest.py index 3dde7b9..b67b7b0 100644 --- a/WebKitTools/Scripts/webkitpy/style/processors/common_unittest.py +++ b/WebKitTools/Scripts/webkitpy/style/checkers/common_unittest.py @@ -24,16 +24,16 @@ import unittest -from common import CarriageReturnProcessor +from common import CarriageReturnChecker -# FIXME: The unit tests for the cpp, text, and common processors should +# FIXME: The unit tests for the cpp, text, and common checkers should # share supporting test code. This can include, for example, the # mock style error handling code and the code to check that all -# of a processor's categories are covered by the unit tests. +# of a checker's categories are covered by the unit tests. # Such shared code can be located in a shared test file, perhaps # even this file. -class CarriageReturnProcessorTest(unittest.TestCase): +class CarriageReturnCheckerTest(unittest.TestCase): """Tests check_no_carriage_return().""" @@ -55,8 +55,8 @@ class CarriageReturnProcessorTest(unittest.TestCase): """Process the given line and assert that the result is correct.""" handle_style_error = self._mock_style_error_handler - processor = CarriageReturnProcessor(handle_style_error) - output_lines = processor.process(input_lines) + checker = CarriageReturnChecker(handle_style_error) + output_lines = checker.check(input_lines) # Check both the return value and error messages. self.assertEquals(output_lines, expected_lines) @@ -82,7 +82,7 @@ class CarriageReturnProcessorTest(unittest.TestCase): []) def test_carriage_in_middle(self): - # The CarriageReturnProcessor checks only the final character + # The CarriageReturnChecker checks only the final character # of each line. self.assert_carriage_return(["carriage\r in a string"], ["carriage\r in a string"], diff --git a/WebKitTools/Scripts/webkitpy/style/processors/cpp.py b/WebKitTools/Scripts/webkitpy/style/checkers/cpp.py index 23be9f9..3e787d6 100644 --- a/WebKitTools/Scripts/webkitpy/style/processors/cpp.py +++ b/WebKitTools/Scripts/webkitpy/style/checkers/cpp.py @@ -2499,6 +2499,10 @@ def check_identifier_name_in_declaration(filename, line_number, line, error): and not modified_identifier == "const_iterator"): error(line_number, 'readability/naming', 4, identifier + " is incorrectly named. Don't use underscores in your identifier names.") + # Check for variables named 'l', these are too easy to confuse with '1' in some fonts + if modified_identifier == 'l': + error(line_number, 'readability/naming', 4, identifier + " is incorrectly named. Don't use the single letter 'l' as an identifier name.") + # There can be only one declaration in non-for-control statements. if control_statement: return @@ -2511,7 +2515,6 @@ def check_identifier_name_in_declaration(filename, line_number, line, error): number_of_identifiers += 1 line = line[matched.end():] - def check_c_style_cast(line_number, line, raw_line, cast_type, pattern, error): """Checks for a C-style cast by looking for the pattern. @@ -2877,7 +2880,7 @@ def _process_lines(filename, file_extension, lines, error, min_confidence): check_for_new_line_at_eof(lines, error) -class CppProcessor(object): +class CppChecker(object): """Processes C++ lines for checking style.""" @@ -2952,7 +2955,7 @@ class CppProcessor(object): def __init__(self, file_path, file_extension, handle_style_error, min_confidence): - """Create a CppProcessor instance. + """Create a CppChecker instance. Args: file_extension: A string that is the file extension, without @@ -2966,7 +2969,7 @@ class CppProcessor(object): # Useful for unit testing. def __eq__(self, other): - """Return whether this CppProcessor instance is equal to another.""" + """Return whether this CppChecker instance is equal to another.""" if self.file_extension != other.file_extension: return False if self.file_path != other.file_path: @@ -2983,12 +2986,12 @@ class CppProcessor(object): # Python does not automatically deduce __ne__() from __eq__(). return not self.__eq__(other) - def process(self, lines): + def check(self, lines): _process_lines(self.file_path, self.file_extension, lines, self.handle_style_error, self.min_confidence) # FIXME: Remove this function (requires refactoring unit tests). def process_file_data(filename, file_extension, lines, error, min_confidence): - processor = CppProcessor(filename, file_extension, error, min_confidence) - processor.process(lines) + checker = CppChecker(filename, file_extension, error, min_confidence) + checker.check(lines) diff --git a/WebKitTools/Scripts/webkitpy/style/processors/cpp_unittest.py b/WebKitTools/Scripts/webkitpy/style/checkers/cpp_unittest.py index 0a3fe08..5a5aabd 100644 --- a/WebKitTools/Scripts/webkitpy/style/processors/cpp_unittest.py +++ b/WebKitTools/Scripts/webkitpy/style/checkers/cpp_unittest.py @@ -42,13 +42,13 @@ import random import re import unittest import cpp as cpp_style -from cpp import CppProcessor +from cpp import CppChecker # This class works as an error collector and replaces cpp_style.Error # function for the unit tests. We also verify each category we see # is in STYLE_CATEGORIES, to help keep that list up to date. class ErrorCollector: - _all_style_categories = CppProcessor.categories + _all_style_categories = CppChecker.categories # This is a list including all categories seen in any unit test. _seen_style_categories = {} @@ -3538,7 +3538,8 @@ class WebKitStyleTest(CppStyleTestBase): 'foo.h') def test_names(self): - name_error_message = " is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]" + name_underscore_error_message = " is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]" + name_tooshort_error_message = " is incorrectly named. Don't use the single letter 'l' as an identifier name. [readability/naming] [4]" # Basic cases from WebKit style guide. self.assert_lint('struct Data;', '') @@ -3546,60 +3547,65 @@ class WebKitStyleTest(CppStyleTestBase): self.assert_lint('class HTMLDocument;', '') self.assert_lint('String mimeType();', '') self.assert_lint('size_t buffer_size;', - 'buffer_size' + name_error_message) + 'buffer_size' + name_underscore_error_message) self.assert_lint('short m_length;', '') self.assert_lint('short _length;', - '_length' + name_error_message) + '_length' + name_underscore_error_message) self.assert_lint('short length_;', - 'length_' + name_error_message) + 'length_' + name_underscore_error_message) self.assert_lint('unsigned _length;', - '_length' + name_error_message) + '_length' + name_underscore_error_message) self.assert_lint('unsigned int _length;', - '_length' + name_error_message) + '_length' + name_underscore_error_message) self.assert_lint('unsigned long long _length;', - '_length' + name_error_message) + '_length' + name_underscore_error_message) + + # Variable name 'l' is easy to confuse with '1' + self.assert_lint('int l;', 'l' + name_tooshort_error_message) + self.assert_lint('size_t l;', 'l' + name_tooshort_error_message) + self.assert_lint('long long l;', 'l' + name_tooshort_error_message) # Pointers, references, functions, templates, and adjectives. self.assert_lint('char* under_score;', - 'under_score' + name_error_message) + 'under_score' + name_underscore_error_message) self.assert_lint('const int UNDER_SCORE;', - 'UNDER_SCORE' + name_error_message) + 'UNDER_SCORE' + name_underscore_error_message) self.assert_lint('static inline const char const& const under_score;', - 'under_score' + name_error_message) + 'under_score' + name_underscore_error_message) self.assert_lint('WebCore::RenderObject* under_score;', - 'under_score' + name_error_message) + 'under_score' + name_underscore_error_message) self.assert_lint('int func_name();', - 'func_name' + name_error_message) + 'func_name' + name_underscore_error_message) self.assert_lint('RefPtr<RenderObject*> under_score;', - 'under_score' + name_error_message) + 'under_score' + name_underscore_error_message) self.assert_lint('WTF::Vector<WTF::RefPtr<const RenderObject* const> > under_score;', - 'under_score' + name_error_message) + 'under_score' + name_underscore_error_message) self.assert_lint('int under_score[];', - 'under_score' + name_error_message) + 'under_score' + name_underscore_error_message) self.assert_lint('struct dirent* under_score;', - 'under_score' + name_error_message) + 'under_score' + name_underscore_error_message) self.assert_lint('long under_score;', - 'under_score' + name_error_message) + 'under_score' + name_underscore_error_message) self.assert_lint('long long under_score;', - 'under_score' + name_error_message) + 'under_score' + name_underscore_error_message) self.assert_lint('long double under_score;', - 'under_score' + name_error_message) + 'under_score' + name_underscore_error_message) self.assert_lint('long long int under_score;', - 'under_score' + name_error_message) + 'under_score' + name_underscore_error_message) # Declarations in control statement. self.assert_lint('if (int under_score = 42) {', - 'under_score' + name_error_message) + 'under_score' + name_underscore_error_message) self.assert_lint('else if (int under_score = 42) {', - 'under_score' + name_error_message) + 'under_score' + name_underscore_error_message) self.assert_lint('for (int under_score = 42; cond; i++) {', - 'under_score' + name_error_message) + 'under_score' + name_underscore_error_message) self.assert_lint('while (foo & under_score = bar) {', - 'under_score' + name_error_message) + 'under_score' + name_underscore_error_message) self.assert_lint('for (foo * under_score = p; cond; i++) {', - 'under_score' + name_error_message) + 'under_score' + name_underscore_error_message) self.assert_lint('for (foo * under_score; cond; i++) {', - 'under_score' + name_error_message) + 'under_score' + name_underscore_error_message) self.assert_lint('while (foo & value_in_thirdparty_library) {', '') self.assert_lint('while (foo * value_in_thirdparty_library) {', '') self.assert_lint('if (mli && S_OK == mli->foo()) {', '') @@ -3607,38 +3613,38 @@ class WebKitStyleTest(CppStyleTestBase): # More member variables and functions. self.assert_lint('int SomeClass::s_validName', '') self.assert_lint('int m_under_score;', - 'm_under_score' + name_error_message) + 'm_under_score' + name_underscore_error_message) self.assert_lint('int SomeClass::s_under_score = 0;', - 'SomeClass::s_under_score' + name_error_message) + 'SomeClass::s_under_score' + name_underscore_error_message) self.assert_lint('int SomeClass::under_score = 0;', - 'SomeClass::under_score' + name_error_message) + 'SomeClass::under_score' + name_underscore_error_message) # Other statements. self.assert_lint('return INT_MAX;', '') self.assert_lint('return_t under_score;', - 'under_score' + name_error_message) + 'under_score' + name_underscore_error_message) self.assert_lint('goto under_score;', - 'under_score' + name_error_message) + 'under_score' + name_underscore_error_message) self.assert_lint('delete static_cast<Foo*>(p);', '') # Multiple variables in one line. self.assert_lint('void myFunction(int variable1, int another_variable);', - 'another_variable' + name_error_message) + 'another_variable' + name_underscore_error_message) self.assert_lint('int variable1, another_variable;', - 'another_variable' + name_error_message) + 'another_variable' + name_underscore_error_message) self.assert_lint('int first_variable, secondVariable;', - 'first_variable' + name_error_message) + 'first_variable' + name_underscore_error_message) self.assert_lint('void my_function(int variable_1, int variable_2);', - ['my_function' + name_error_message, - 'variable_1' + name_error_message, - 'variable_2' + name_error_message]) + ['my_function' + name_underscore_error_message, + 'variable_1' + name_underscore_error_message, + 'variable_2' + name_underscore_error_message]) self.assert_lint('for (int variable_1, variable_2;;) {', - ['variable_1' + name_error_message, - 'variable_2' + name_error_message]) + ['variable_1' + name_underscore_error_message, + 'variable_2' + name_underscore_error_message]) # There is an exception for op code functions but only in the JavaScriptCore directory. self.assert_lint('void this_op_code(int var1, int var2)', '', 'JavaScriptCore/foo.cpp') - self.assert_lint('void this_op_code(int var1, int var2)', 'this_op_code' + name_error_message) + self.assert_lint('void this_op_code(int var1, int var2)', 'this_op_code' + name_underscore_error_message) # GObject requires certain magical names in class declarations. self.assert_lint('void webkit_dom_object_init();', '') @@ -3656,7 +3662,7 @@ class WebKitStyleTest(CppStyleTestBase): # Bitfields. self.assert_lint('unsigned _fillRule : 1;', - '_fillRule' + name_error_message) + '_fillRule' + name_underscore_error_message) def test_comments(self): @@ -3673,52 +3679,52 @@ class WebKitStyleTest(CppStyleTestBase): pass -class CppProcessorTest(unittest.TestCase): +class CppCheckerTest(unittest.TestCase): - """Tests CppProcessor class.""" + """Tests CppChecker class.""" def mock_handle_style_error(self): pass - def _processor(self): - return CppProcessor("foo", "h", self.mock_handle_style_error, 3) + def _checker(self): + return CppChecker("foo", "h", self.mock_handle_style_error, 3) def test_init(self): """Test __init__ constructor.""" - processor = self._processor() - self.assertEquals(processor.file_extension, "h") - self.assertEquals(processor.file_path, "foo") - self.assertEquals(processor.handle_style_error, self.mock_handle_style_error) - self.assertEquals(processor.min_confidence, 3) + checker = self._checker() + self.assertEquals(checker.file_extension, "h") + self.assertEquals(checker.file_path, "foo") + self.assertEquals(checker.handle_style_error, self.mock_handle_style_error) + self.assertEquals(checker.min_confidence, 3) def test_eq(self): """Test __eq__ equality function.""" - processor1 = self._processor() - processor2 = self._processor() + checker1 = self._checker() + checker2 = self._checker() # == calls __eq__. - self.assertTrue(processor1 == processor2) + self.assertTrue(checker1 == checker2) def mock_handle_style_error2(self): pass # Verify that a difference in any argument cause equality to fail. - processor = CppProcessor("foo", "h", self.mock_handle_style_error, 3) - self.assertFalse(processor == CppProcessor("bar", "h", self.mock_handle_style_error, 3)) - self.assertFalse(processor == CppProcessor("foo", "c", self.mock_handle_style_error, 3)) - self.assertFalse(processor == CppProcessor("foo", "h", mock_handle_style_error2, 3)) - self.assertFalse(processor == CppProcessor("foo", "h", self.mock_handle_style_error, 4)) + checker = CppChecker("foo", "h", self.mock_handle_style_error, 3) + self.assertFalse(checker == CppChecker("bar", "h", self.mock_handle_style_error, 3)) + self.assertFalse(checker == CppChecker("foo", "c", self.mock_handle_style_error, 3)) + self.assertFalse(checker == CppChecker("foo", "h", mock_handle_style_error2, 3)) + self.assertFalse(checker == CppChecker("foo", "h", self.mock_handle_style_error, 4)) def test_ne(self): """Test __ne__ inequality function.""" - processor1 = self._processor() - processor2 = self._processor() + checker1 = self._checker() + checker2 = self._checker() # != calls __ne__. # By default, __ne__ always returns true on different objects. # Thus, just check the distinguishing case to verify that the # code defines __ne__. - self.assertFalse(processor1 != processor2) + self.assertFalse(checker1 != checker2) def tearDown(): diff --git a/WebKitTools/Scripts/webkitpy/style/processors/python.py b/WebKitTools/Scripts/webkitpy/style/checkers/python.py index 8ab936d..70d4450 100644 --- a/WebKitTools/Scripts/webkitpy/style/processors/python.py +++ b/WebKitTools/Scripts/webkitpy/style/checkers/python.py @@ -25,7 +25,7 @@ from ...style_references import pep8 -class PythonProcessor(object): +class PythonChecker(object): """Processes text lines for checking style.""" @@ -33,7 +33,7 @@ class PythonProcessor(object): self._file_path = file_path self._handle_style_error = handle_style_error - def process(self, lines): + def check(self, lines): # Initialize pep8.options, which is necessary for # Checker.check_all() to execute. pep8.process_options(arglist=[self._file_path]) diff --git a/WebKitTools/Scripts/webkitpy/style/processors/python_unittest.py b/WebKitTools/Scripts/webkitpy/style/checkers/python_unittest.py index 3ce3311..e003eb8 100644 --- a/WebKitTools/Scripts/webkitpy/style/processors/python_unittest.py +++ b/WebKitTools/Scripts/webkitpy/style/checkers/python_unittest.py @@ -25,25 +25,25 @@ import os import unittest -from python import PythonProcessor +from python import PythonChecker -class PythonProcessorTest(unittest.TestCase): +class PythonCheckerTest(unittest.TestCase): - """Tests the PythonProcessor class.""" + """Tests the PythonChecker class.""" def test_init(self): """Test __init__() method.""" def _mock_handle_style_error(self): pass - processor = PythonProcessor("foo.txt", _mock_handle_style_error) - self.assertEquals(processor._file_path, "foo.txt") - self.assertEquals(processor._handle_style_error, + checker = PythonChecker("foo.txt", _mock_handle_style_error) + self.assertEquals(checker._file_path, "foo.txt") + self.assertEquals(checker._handle_style_error, _mock_handle_style_error) - def test_process(self): - """Test process() method.""" + def test_check(self): + """Test check() method.""" errors = [] def _mock_handle_style_error(line_number, category, confidence, @@ -54,8 +54,8 @@ class PythonProcessorTest(unittest.TestCase): current_dir = os.path.dirname(__file__) file_path = os.path.join(current_dir, "python_unittest_input.py") - processor = PythonProcessor(file_path, _mock_handle_style_error) - processor.process(lines=[]) + checker = PythonChecker(file_path, _mock_handle_style_error) + checker.check(lines=[]) self.assertEquals(len(errors), 1) self.assertEquals(errors[0], diff --git a/WebKitTools/Scripts/webkitpy/style/processors/python_unittest_input.py b/WebKitTools/Scripts/webkitpy/style/checkers/python_unittest_input.py index 9f1d118..9f1d118 100644 --- a/WebKitTools/Scripts/webkitpy/style/processors/python_unittest_input.py +++ b/WebKitTools/Scripts/webkitpy/style/checkers/python_unittest_input.py diff --git a/WebKitTools/Scripts/webkitpy/style/processors/text.py b/WebKitTools/Scripts/webkitpy/style/checkers/text.py index 307e5b8..0d03938 100644 --- a/WebKitTools/Scripts/webkitpy/style/processors/text.py +++ b/WebKitTools/Scripts/webkitpy/style/checkers/text.py @@ -30,7 +30,7 @@ """Checks WebKit style for text files.""" -class TextProcessor(object): +class TextChecker(object): """Processes text lines for checking style.""" @@ -38,7 +38,7 @@ class TextProcessor(object): self.file_path = file_path self.handle_style_error = handle_style_error - def process(self, lines): + def check(self, lines): lines = (["// adjust line numbers to make the first line 1."] + lines) # FIXME: share with cpp_style. @@ -51,6 +51,6 @@ class TextProcessor(object): # FIXME: Remove this function (requires refactoring unit tests). def process_file_data(filename, lines, error): - processor = TextProcessor(filename, error) - processor.process(lines) + checker = TextChecker(filename, error) + checker.check(lines) diff --git a/WebKitTools/Scripts/webkitpy/style/processors/text_unittest.py b/WebKitTools/Scripts/webkitpy/style/checkers/text_unittest.py index 62f825b..ced49a9 100644 --- a/WebKitTools/Scripts/webkitpy/style/processors/text_unittest.py +++ b/WebKitTools/Scripts/webkitpy/style/checkers/text_unittest.py @@ -32,7 +32,7 @@ import unittest import text as text_style -from text import TextProcessor +from text import TextChecker class TextStyleTestCase(unittest.TestCase): """TestCase for text_style.py""" @@ -76,18 +76,18 @@ class TextStyleTestCase(unittest.TestCase): '\tReviewed by NOBODY.'], 3) -class TextProcessorTest(unittest.TestCase): +class TextCheckerTest(unittest.TestCase): - """Tests TextProcessor class.""" + """Tests TextChecker class.""" def mock_handle_style_error(self): pass def test_init(self): """Test __init__ constructor.""" - processor = TextProcessor("foo.txt", self.mock_handle_style_error) - self.assertEquals(processor.file_path, "foo.txt") - self.assertEquals(processor.handle_style_error, self.mock_handle_style_error) + checker = TextChecker("foo.txt", self.mock_handle_style_error) + self.assertEquals(checker.file_path, "foo.txt") + self.assertEquals(checker.handle_style_error, self.mock_handle_style_error) if __name__ == '__main__': diff --git a/WebKitTools/Scripts/webkitpy/style/error_handlers.py b/WebKitTools/Scripts/webkitpy/style/error_handlers.py index 5666bfb..0bede24 100644 --- a/WebKitTools/Scripts/webkitpy/style/error_handlers.py +++ b/WebKitTools/Scripts/webkitpy/style/error_handlers.py @@ -63,7 +63,7 @@ class DefaultStyleErrorHandler(object): Args: file_path: The path to the file containing the error. This is used for reporting to the user. - configuration: A StyleCheckerConfiguration instance. + configuration: A StyleProcessorConfiguration instance. increment_error_count: A function that takes no arguments and increments the total count of reportable errors. diff --git a/WebKitTools/Scripts/webkitpy/style/error_handlers_unittest.py b/WebKitTools/Scripts/webkitpy/style/error_handlers_unittest.py index 05e725a..23619cc 100644 --- a/WebKitTools/Scripts/webkitpy/style/error_handlers_unittest.py +++ b/WebKitTools/Scripts/webkitpy/style/error_handlers_unittest.py @@ -25,7 +25,7 @@ import unittest -from checker import StyleCheckerConfiguration +from checker import StyleProcessorConfiguration from error_handlers import DefaultStyleErrorHandler from filter import FilterConfiguration @@ -51,11 +51,11 @@ class DefaultStyleErrorHandlerTest(unittest.TestCase): self._error_messages.append(message) def _style_checker_configuration(self): - """Return a StyleCheckerConfiguration instance for testing.""" + """Return a StyleProcessorConfiguration instance for testing.""" base_rules = ["-whitespace", "+whitespace/tab"] filter_configuration = FilterConfiguration(base_rules=base_rules) - return StyleCheckerConfiguration( + return StyleProcessorConfiguration( filter_configuration=filter_configuration, max_reports_per_category={"whitespace/tab": 2}, min_confidence=3, diff --git a/WebKitTools/Scripts/webkitpy/style/filereader.py b/WebKitTools/Scripts/webkitpy/style/filereader.py index 081e6dc..48455b3 100644 --- a/WebKitTools/Scripts/webkitpy/style/filereader.py +++ b/WebKitTools/Scripts/webkitpy/style/filereader.py @@ -103,6 +103,10 @@ class TextFileReader(object): """ self.file_count += 1 + if not os.path.exists(file_path) and file_path != "-": + _log.error("File does not exist: '%s'" % file_path) + sys.exit(1) + if not self._processor.should_process(file_path): _log.debug("Skipping file: '%s'" % file_path) return @@ -111,10 +115,6 @@ class TextFileReader(object): try: lines = self._read_lines(file_path) except IOError, err: - if not os.path.exists(file_path): - _log.error("File does not exist: '%s'" % file_path) - sys.exit(1) - message = ("Could not read file. Skipping: '%s'\n %s" % (file_path, err)) _log.warn(message) diff --git a/WebKitTools/Scripts/webkitpy/style/filereader_unittest.py b/WebKitTools/Scripts/webkitpy/style/filereader_unittest.py index 8d1a159..558ec5a 100644 --- a/WebKitTools/Scripts/webkitpy/style/filereader_unittest.py +++ b/WebKitTools/Scripts/webkitpy/style/filereader_unittest.py @@ -22,6 +22,9 @@ """Contains unit tests for filereader.py.""" +from __future__ import with_statement + +import codecs import os import shutil import tempfile @@ -67,14 +70,12 @@ class TextFileReaderTest(LoggingTestCase): LoggingTestCase.tearDown(self) shutil.rmtree(self._temp_dir) - def _create_file(self, rel_path, text): + def _create_file(self, rel_path, text, encoding="utf-8"): """Create a file with given text and return the path to the file.""" + # FIXME: There are better/more secure APIs for creatin tmp file paths. file_path = os.path.join(self._temp_dir, rel_path) - - file = open(file_path, 'w') - file.write(text) - file.close() - + with codecs.open(file_path, "w", encoding) as file: + file.write(text) return file_path def _passed_to_processor(self): @@ -86,10 +87,6 @@ class TextFileReaderTest(LoggingTestCase): self.assertEquals(passed_to_processor, self._passed_to_processor()) self.assertEquals(file_count, self._file_reader.file_count) - def test_process_file__should_not_process(self): - self._file_reader.process_file('should_not_process.txt') - self._assert_file_reader([], 1) - def test_process_file__does_not_exist(self): try: self._file_reader.process_file('does_not_exist.txt') @@ -121,6 +118,12 @@ class TextFileReaderTest(LoggingTestCase): self._assert_file_reader([], 1) + def test_process_file__should_not_process(self): + file_path = self._create_file('should_not_process.txt', 'contents') + + self._file_reader.process_file(file_path) + self._assert_file_reader([], 1) + def test_process_file__multiple_lines(self): file_path = self._create_file('foo.txt', 'line one\r\nline two\n') @@ -128,6 +131,13 @@ class TextFileReaderTest(LoggingTestCase): processed = [(['line one\r', 'line two', ''], file_path, None)] self._assert_file_reader(processed, 1) + def test_process_file__file_stdin(self): + file_path = self._create_file('-', 'file contents') + + self._file_reader.process_file(file_path=file_path, test_kwarg='foo') + processed = [(['file contents'], file_path, 'foo')] + self._assert_file_reader(processed, 1) + def test_process_file__with_kwarg(self): file_path = self._create_file('foo.txt', 'file contents') diff --git a/WebKitTools/Scripts/webkitpy/style/optparser.py b/WebKitTools/Scripts/webkitpy/style/optparser.py index 576c16a..bb4788a 100644 --- a/WebKitTools/Scripts/webkitpy/style/optparser.py +++ b/WebKitTools/Scripts/webkitpy/style/optparser.py @@ -147,7 +147,8 @@ class CommandOptionValues(object): git_commit=None, is_verbose=False, min_confidence=1, - output_format="emacs"): + output_format="emacs", + squash=False): if filter_rules is None: filter_rules = [] @@ -166,6 +167,7 @@ class CommandOptionValues(object): self.is_verbose = is_verbose self.min_confidence = min_confidence self.output_format = output_format + self.squash = squash # Useful for unit testing. def __eq__(self, other): @@ -180,6 +182,8 @@ class CommandOptionValues(object): return False if self.output_format != other.output_format: return False + if self.squash != other.squash: + return False return True @@ -214,6 +218,8 @@ class ArgumentPrinter(object): flags['filter'] = ",".join(filter_rules) if options.git_commit: flags['git-commit'] = options.git_commit + if options.squash: + flags['squash'] = options.squash flag_string = '' # Alphabetizing lets us unit test this method. @@ -303,9 +309,10 @@ class ArgumentParser(object): parser.add_option("-f", "--filter-rules", metavar="RULES", dest="filter_value", help=filter_help) - git_help = "check all changes after the given git commit." - parser.add_option("-g", "--git-commit", "--git-diff", "--git-since", - metavar="COMMIT", dest="git_since", help=git_help,) + git_commit_help = ("check all changes in the given git commit. " + "Use 'commit_id..' to check all changes after commmit_id") + parser.add_option("-g", "--git-diff", "--git-commit", + metavar="COMMIT", dest="git_commit", help=git_commit_help,) min_confidence_help = ("set the minimum confidence of style errors " "to report. Can be an integer 1-5, with 1 " @@ -323,6 +330,14 @@ class ArgumentParser(object): dest="output_format", default=default_output_format, help=output_format_help) + squash_help = ("All diffs from the remote branch are checked." + "If excluded, prompts whether to squash when there are multiple commits.") + parser.add_option("-s", "--squash", action="store_true", dest="squash", help=squash_help) + + squash_help = ("Only working copy diffs are checked." + "If excluded, prompts whether to squash when there are multiple commits.") + parser.add_option("--no-squash", action="store_false", dest="squash", help=squash_help) + verbose_help = "enable verbose logging." parser.add_option("-v", "--verbose", dest="is_verbose", default=False, action="store_true", help=verbose_help) @@ -407,7 +422,7 @@ class ArgumentParser(object): (options, paths) = self._parser.parse_args(args=args) filter_value = options.filter_value - git_commit = options.git_since + git_commit = options.git_commit is_verbose = options.is_verbose min_confidence = options.min_confidence output_format = options.output_format @@ -423,14 +438,6 @@ class ArgumentParser(object): self._parse_error('You cannot provide both paths and a git ' 'commit at the same time.') - # FIXME: Add unit tests. - if git_commit and '..' in git_commit: - # FIXME: If the range is a "...", the code should find the common - # ancestor and start there. See git diff --help for how - # "..." usually works. - self._parse_error('invalid --git-commit option: option does ' - 'not support ranges "..": %s' % git_commit) - min_confidence = int(min_confidence) if (min_confidence < 1) or (min_confidence > 5): self._parse_error('option --min-confidence: invalid integer: ' @@ -451,7 +458,8 @@ class ArgumentParser(object): git_commit=git_commit, is_verbose=is_verbose, min_confidence=min_confidence, - output_format=output_format) + output_format=output_format, + squash=options.squash) return (paths, options) diff --git a/WebKitTools/Scripts/webkitpy/style/optparser_unittest.py b/WebKitTools/Scripts/webkitpy/style/optparser_unittest.py index 1c525c6..b7e3eda 100644 --- a/WebKitTools/Scripts/webkitpy/style/optparser_unittest.py +++ b/WebKitTools/Scripts/webkitpy/style/optparser_unittest.py @@ -114,10 +114,6 @@ class ArgumentParserTest(LoggingTestCase): self.assertRaises(SystemExit, parse, ['--bad']) self.assertLog(['ERROR: no such option: --bad\n']) - self.assertRaises(SystemExit, parse, ['--git-diff=aa..bb']) - self.assertLog(['ERROR: invalid --git-commit option: ' - 'option does not support ranges "..": aa..bb\n']) - self.assertRaises(SystemExit, parse, ['--min-confidence=bad']) self.assertLog(['ERROR: option --min-confidence: ' "invalid integer value: 'bad'\n"]) @@ -173,8 +169,6 @@ class ArgumentParserTest(LoggingTestCase): self.assertEquals(options.git_commit, 'commit') (files, options) = parse(['--git-diff=commit']) self.assertEquals(options.git_commit, 'commit') - (files, options) = parse(['--git-since=commit']) - self.assertEquals(options.git_commit, 'commit') (files, options) = parse(['--verbose']) self.assertEquals(options.is_verbose, True) |