Changeset 53675 in webkit

Timestamp:

Jan 21, 2010 9:39:24 PM (14 years ago)

Author:

eric@webkit.org

Message:

2010-01-21 Chris Jerdonek <Chris Jerdonek>

Reviewed by Shinichiro Hamaji.

Refactored to move file name and file-reading related code
from cpp_style.py and text_style.py to checker.py.

​https://bugs.webkit.org/show_bug.cgi?id=33775

• Scripts/check-webkit-style:
  • Updates caused by changes to checker.py.

• Scripts/webkitpy/style/checker.py:
  • Added SKIPPED_FILES_WITH_WARNING list.
  • Added SKIPPED_FILES_WITHOUT_WARNING list.
  • Added FileType class.
  • Added ProcessorDispatcher class.
  • In StyleChecker class:
    • Renamed process_patch() to check_patch().
    • Renamed process_file() to check_file().
    • Added _process_file().
    • Related refactoring.
    • Addressed check_patch() FIXME to share code with process_file().

• Scripts/webkitpy/style/checker_unittest.py:
  • Added ProcessorDispatcherSkipTest class.
  • Added ProcessorDispatcherDispatchTest class.
  • Added StyleCheckerCheckFileTest class.

• Scripts/webkitpy/style/cpp_style.py:
  • Renamed process_file_data() to _process_lines.
  • Removed process_file() (moved logic to checker.py).
  • Removed can_handle() (moved logic to checker.py).
  • Added CppProcessor class.
  • Removed is_exempt() (moved logic to checker.py).
  • Added process_file_data() back as a wrapper function.

• Scripts/webkitpy/style/cpp_style_unittest.py:
  • Removed test_can_handle().
  • Removed test_is_exempt().
  • Added CppProcessorTest class.

• Scripts/webkitpy/style/text_style.py:
  • Added TextProcessor class.
  • Removed process_file().
  • Removed can_handle().

• Scripts/webkitpy/style/text_style_unittest.py:
  • Removed test_can_handle().
  • Added TextProcessorTest class.

Location:

trunk/WebKitTools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/check-webkit-style (modified) (2 diffs)
• Scripts/webkitpy/style/checker.py (modified) (7 diffs)
• Scripts/webkitpy/style/checker_unittest.py (modified) (7 diffs)
• Scripts/webkitpy/style/cpp_style.py (modified) (3 diffs)
• Scripts/webkitpy/style/cpp_style_unittest.py (modified) (3 diffs)
• Scripts/webkitpy/style/text_style.py (modified) (2 diffs)
• Scripts/webkitpy/style/text_style_unittest.py (modified) (2 diffs)

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2010-01-21  Chris Jerdonek  <cjerdonek@webkit.org>
+
+        Reviewed by Shinichiro Hamaji.
+
+        Refactored to move file name and file-reading related code
+        from cpp_style.py and text_style.py to checker.py.
+
+        https://bugs.webkit.org/show_bug.cgi?id=33775
+
+        * Scripts/check-webkit-style:
+          - Updates caused by changes to checker.py.
+
+        * Scripts/webkitpy/style/checker.py:
+          - Added SKIPPED_FILES_WITH_WARNING list.
+          - Added SKIPPED_FILES_WITHOUT_WARNING list.
+          - Added FileType class.
+          - Added ProcessorDispatcher class.
+          - In StyleChecker class:
+            - Renamed process_patch() to check_patch().
+            - Renamed process_file() to check_file().
+            - Added _process_file().
+            - Related refactoring.
+            - Addressed check_patch() FIXME to share code with process_file().
+
+        * Scripts/webkitpy/style/checker_unittest.py:
+          - Added ProcessorDispatcherSkipTest class.
+          - Added ProcessorDispatcherDispatchTest class.
+          - Added StyleCheckerCheckFileTest class.
+
+        * Scripts/webkitpy/style/cpp_style.py:
+          - Renamed process_file_data() to _process_lines.
+          - Removed process_file() (moved logic to checker.py).
+          - Removed can_handle() (moved logic to checker.py).
+          - Added CppProcessor class.
+          - Removed is_exempt() (moved logic to checker.py).
+          - Added process_file_data() back as a wrapper function.
+
+        * Scripts/webkitpy/style/cpp_style_unittest.py:
+          - Removed test_can_handle().
+          - Removed test_is_exempt().
+          - Added CppProcessorTest class.
+
+        * Scripts/webkitpy/style/text_style.py:
+          - Added TextProcessor class.
+          - Removed process_file().
+          - Removed can_handle().
+
+        * Scripts/webkitpy/style/text_style_unittest.py:
+          - Removed test_can_handle().
+          - Added TextProcessorTest class.
+
 2010-01-21  Chris Jerdonek  <cjerdonek@webkit.org>
 

trunk/WebKitTools/Scripts/check-webkit-style

@@ -69 +69 @@
     if files:
         for filename in files:
-            style_checker.process_file(filename)
+            style_checker.check_file(filename)
 
     else:
@@ -85 +85 @@
         else:
             patch = scm.create_patch()
-        style_checker.process_patch(patch)
+        style_checker.check_patch(patch)
 
     sys.stderr.write('Total errors found: %d\n' % style_checker.error_count)

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

@@ -30 +30 @@
 """Front end of some style-checker modules."""
 
-# FIXME: Move more code from cpp_style to here.
-
+import codecs
 import getopt
 import os.path
 import sys
 
-import cpp_style
-import text_style
 from diff_parser import DiffParser
+from cpp_style import CppProcessor
+from text_style import TextProcessor
 
 
@@ -159 +158 @@
 
 
+# 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. :)
+    "gtk2drawing.c", # WebCore/platform/gtk/gtk2drawing.c
+    "gtk2drawing.h", # WebCore/platform/gtk/gtk2drawing.h
+    "JavaScriptCore/qt/api/",
+    "WebKit/gtk/tests/",
+    "WebKit/qt/Api/",
+    "WebKit/qt/tests/",
+    ]
+
+
+# Don't include a warning for skipped files that are more common
+# and more obvious.
+SKIPPED_FILES_WITHOUT_WARNING = [
+    "LayoutTests/"
+    ]
+
+
 def webkit_argument_defaults():
     """Return the DefaultArguments instance for use by check-webkit-style."""
@@ -620 +643 @@
 
 
+# Enum-like idiom
+class FileType:
+
+    NONE = 1
+    # Alphabetize remaining types
+    CPP = 2
+    TEXT = 3
+
+
+class ProcessorDispatcher(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',
+        'py',
+        'txt',
+        )
+
+    def _file_extension(self, file_path):
+        """Return the file extension without the leading dot."""
+        return os.path.splitext(file_path)[1].lstrip(".")
+
+    def should_skip_with_warning(self, file_path):
+        """Return whether the given file should be skipped with a warning."""
+        for skipped_file in SKIPPED_FILES_WITH_WARNING:
+            if file_path.find(skipped_file) >= 0:
+                return True
+        return False
+
+    def should_skip_without_warning(self, file_path):
+        """Return whether the given file should be skipped without a warning."""
+        for skipped_file in SKIPPED_FILES_WITHOUT_WARNING:
+            if file_path.find(skipped_file) >= 0:
+                return True
+        return False
+
+    def _file_type(self, file_path):
+        """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 == '-'):
+            # FIXME: Do something about the comment below and the issue it
+            #        raises since cpp_style already relies on the extension.
+            #
+            # Treat stdin as C++. Since the extension is unknown when
+            # reading from stdin, cpp_style tests should not rely on
+            # the extension.
+            return FileType.CPP
+        elif ("ChangeLog" in file_path
+              or "WebKitTools/Scripts/" in file_path
+              or file_extension in self.text_file_extensions):
+            return FileType.TEXT
+        else:
+            return FileType.NONE
+
+    def _create_processor(self, file_type, file_path, handle_style_error, verbosity):
+        """Instantiate and return a style processor based on file type."""
+        if file_type == FileType.NONE:
+            processor = None
+        elif file_type == FileType.CPP:
+            file_extension = self._file_extension(file_path)
+            processor = CppProcessor(file_path, file_extension, handle_style_error, verbosity)
+        elif file_type == FileType.TEXT:
+            processor = TextProcessor(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."
+                             % {"file_type": file_type,
+                                "NONE": FileType.NONE,
+                                "CPP": FileType.CPP,
+                                "TEXT": FileType.TEXT})
+
+        return processor
+
+    def dispatch_processor(self, file_path, handle_style_error, verbosity):
+        """Instantiate and return a style processor based on file path."""
+        file_type = self._file_type(file_path)
+
+        processor = self._create_processor(file_type,
+                                           file_path,
+                                           handle_style_error,
+                                           verbosity)
+        return processor
+
+
 class StyleChecker(object):
 
@@ -633 +754 @@
     """
 
-    def __init__(self, options, write_error=sys.stderr.write):
+    def __init__(self, options, stderr_write=None):
         """Create a StyleChecker instance.
 
@@ -640 +761 @@
           stderr_write: A function that takes a string as a parameter
                         and that is called when a style error occurs.
-
-        """
-        self._write_error = write_error
+                        Defaults to sys.stderr.write. This should be
+                        used only for unit tests.
+
+        """
+        if stderr_write is None:
+            stderr_write = sys.stderr.write
+
+        self._stderr_write = stderr_write
+        self.error_count = 0
         self.options = options
-        self.error_count = 0
-
-    def _handle_error(self, filename, line_number, category, confidence, message):
+
+    def _handle_style_error(self, filename, line_number, category, confidence, message):
         """Handle the occurrence of a style error while checking.
 
@@ -676 +802 @@
             format_string = "%s:%s:  %s  [%s] [%d]\n"
 
-        self._write_error(format_string % (filename, line_number, message,
-                                           category, confidence))
-
-    def process_file(self, filename):
-        """Checks style for the specified file.
-
-        If the specified filename is '-', applies cpp_style to the standard input.
-
-        """
-        if cpp_style.can_handle(filename) or filename == '-':
-            cpp_style.process_file(filename, self._handle_error, self.options.verbosity)
-        elif text_style.can_handle(filename):
-            text_style.process_file(filename, self._handle_error)
-
-    def process_patch(self, patch_string):
-        """Does lint on a single patch.
+        self._stderr_write(format_string % (filename, line_number, message,
+                                            category, confidence))
+
+    def _process_file(self, processor, file_path, handle_style_error):
+        """Process the file using the given processor."""
+        try:
+            # Support the UNIX convention of using "-" for stdin.  Note that
+            # we are not opening the file with universal newline support
+            # (which codecs doesn't support anyway), so the resulting lines do
+            # contain trailing '\r' characters if we are reading a file that
+            # has CRLF endings.
+            # If after the split a trailing '\r' is present, it is removed
+            # below. If it is not expected to be present (i.e. os.linesep !=
+            # '\r\n' as in Windows), a warning is issued below if this file
+            # is processed.
+            if file_path == '-':
+                lines = codecs.StreamReaderWriter(sys.stdin,
+                                                  codecs.getreader('utf8'),
+                                                  codecs.getwriter('utf8'),
+                                                  'replace').read().split('\n')
+            else:
+                lines = codecs.open(file_path, 'r', 'utf8', 'replace').read().split('\n')
+
+            carriage_return_found = False
+            # Remove trailing '\r'.
+            for line_number in range(len(lines)):
+                if lines[line_number].endswith('\r'):
+                    lines[line_number] = lines[line_number].rstrip('\r')
+                    carriage_return_found = True
+
+        except IOError:
+            self._stderr_write("Skipping input '%s': Can't open for reading\n" % file_path)
+            return
+
+        processor.process(lines)
+
+        if carriage_return_found and os.linesep != '\r\n':
+            # FIXME: Make sure this error also shows up when checking
+            #        patches, if appropriate.
+            #
+            # Use 0 for line_number since outputting only one error for
+            # potentially several lines.
+            handle_style_error(file_path, 0, 'whitespace/newline', 1,
+                               'One or more unexpected \\r (^M) found;'
+                               'better to use only a \\n')
+
+    def check_file(self, file_path, handle_style_error=None, process_file=None):
+        """Check style in the given file.
 
         Args:
-          patch_string: A string of a patch.
+          file_path: A string that is the path of the file to process.
+          handle_style_error: The function to call when a style error
+                              occurs. This parameter is meant for internal
+                              use within this class. Defaults to the
+                              style error handling method of this class.
+          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.
+
+        """
+        if handle_style_error is None:
+            handle_style_error = self._handle_style_error
+
+        if process_file is None:
+            process_file = self._process_file
+
+        dispatcher = ProcessorDispatcher()
+
+        if dispatcher.should_skip_without_warning(file_path):
+            return
+        if dispatcher.should_skip_with_warning(file_path):
+            self._stderr_write('Ignoring "%s": this file is exempt from the '
+                               "style guide.\n" % file_path)
+            return
+
+        verbosity = self.options.verbosity
+        processor = dispatcher.dispatch_processor(file_path,
+                                                  handle_style_error,
+                                                  verbosity)
+        process_file(processor, file_path, handle_style_error)
+
+    def check_patch(self, patch_string):
+        """Check style in the given patch.
+
+        Args:
+          patch_string: A string that is a patch string.
 
         """
         patch = DiffParser(patch_string.splitlines())
-        for filename, diff in patch.files.iteritems():
-            file_extension = os.path.splitext(filename)[1]
+        for file_path, diff in patch.files.iteritems():
             line_numbers = set()
 
-            def error_for_patch(filename, line_number, category, confidence, message):
+            def error_for_patch(file_path, line_number, category, confidence, message):
                 """Wrapper function of cpp_style.error for patches.
 
                 This function outputs errors only if the line number
                 corresponds to lines which are modified or added.
+
                 """
                 if not line_numbers:
@@ -715 +908 @@
                             line_numbers.add(line[1])
 
+                # FIXME: Make sure errors having line number zero are
+                #        logged -- like carriage-return errors.
                 if line_number in line_numbers:
-                    self._handle_error(filename, line_number, category, confidence, message)
-
-            # FIXME: Share this code with self.process_file().
-            if cpp_style.can_handle(filename):
-                cpp_style.process_file(filename, error_for_patch, self.options.verbosity)
-            elif text_style.can_handle(filename):
-                text_style.process_file(filename, error_for_patch)
+                    self._handle_style_error(file_path, line_number, category, confidence, message)
+
+            self.check_file(file_path, handle_style_error=error_for_patch)
+

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

@@ -39 +39 @@
 import checker as style
 from checker import CategoryFilter
+from checker import ProcessorDispatcher
 from checker import ProcessorOptions
 from checker import StyleChecker
+from cpp_style import CppProcessor
+from text_style import TextProcessor
 
 class CategoryFilterTest(unittest.TestCase):
@@ -353 +356 @@
 
 
+class ProcessorDispatcherSkipTest(unittest.TestCase):
+
+    """Tests the "should skip" methods of the ProcessorDispatcher class."""
+
+    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"))
+
+        # Check skipped files.
+        paths_to_skip = [
+           "gtk2drawing.c",
+           "gtk2drawing.h",
+           "JavaScriptCore/qt/api/qscriptengine_p.h",
+           "WebCore/platform/gtk/gtk2drawing.c",
+           "WebCore/platform/gtk/gtk2drawing.h",
+           "WebKit/gtk/tests/testatk.c",
+           "WebKit/qt/Api/qwebpage.h",
+           "WebKit/qt/tests/qwebsecurityorigin/tst_qwebsecurityorigin.cpp",
+            ]
+
+        for path in paths_to_skip:
+            self.assertTrue(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",
+            ]
+
+        for path in paths_to_skip:
+            self.assertTrue(dispatcher.should_skip_without_warning(path),
+                            "Checking: " + path)
+
+
+class ProcessorDispatcherDispatchTest(unittest.TestCase):
+
+    """Tests dispatch_processor() method of ProcessorDispatcher 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,
+                                                  verbosity=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__
+        self.assertEquals(got_class, expected_class,
+                          'For path "%(file_path)s" got %(got_class)s when '
+                          "expecting %(expected_class)s."
+                          % {"file_path": file_path,
+                             "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_processor_text(self, file_path):
+        """Assert that the dispatched processor is a TextProcessor."""
+        self.assert_processor(file_path, TextProcessor)
+
+    def test_cpp_paths(self):
+        """Test paths that should be checked as C++."""
+        paths = [
+            "-",
+            "foo.c",
+            "foo.cpp",
+            "foo.h",
+            ]
+
+        for path in paths:
+            self.assert_processor_cpp(path)
+
+        # Check processor 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.verbosity, 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)
+
+    def test_text_paths(self):
+        """Test paths that should be checked as text."""
+        paths = [
+           "ChangeLog",
+           "foo.css",
+           "foo.html",
+           "foo.idl",
+           "foo.js",
+           "foo.mm",
+           "foo.php",
+           "foo.pm",
+           "foo.py",
+           "foo.txt",
+           "FooChangeLog.bak",
+           "WebCore/ChangeLog",
+           "WebCore/inspector/front-end/inspector.js",
+           "WebKitTools/Scripts/check-webkit=style",
+           "WebKitTools/Scripts/modules/text_style.py",
+            ]
+
+        for path in paths:
+            self.assert_processor_text(path)
+
+        # Check processor 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)
+
+    def test_none_paths(self):
+        """Test paths that have no file type.."""
+        paths = [
+           "Makefile",
+           "foo.png",
+           "foo.exe",
+            ]
+
+        for path in paths:
+            self.assert_processor_none(path)
+
+
 class StyleCheckerTest(unittest.TestCase):
 
@@ -358 +520 @@
 
     Attributes:
-      error_message: A string that is the last error message reported
-                     by the test StyleChecker instance.
+      error_messages: A string containing all of the warning messages
+                      written to the mock_stderr_write method of
+                      this class.
 
     """
 
-    error_message = ""
-
-    def mock_write_error(self, error_message):
-        """Store an error message without printing it."""
-        self.error_message = error_message
+    def setUp(self):
+        self.error_messages = ""
+
+    def mock_stderr_write(self, error_message):
+        """A mock sys.stderr.write."""
+        self.error_messages = error_message
         pass
 
+    def mock_handle_style_error(self):
+        pass
+
     def style_checker(self, options):
-        return StyleChecker(options, self.mock_write_error)
+        return StyleChecker(options, self.mock_stderr_write)
 
     def test_init(self):
@@ -382 +549 @@
 
     def write_sample_error(self, style_checker, error_confidence):
-        """Write an error to the given style_checker."""
-        style_checker._handle_error(filename="filename",
-                                    line_number=1,
-                                    category="category",
-                                    confidence=error_confidence,
-                                    message="message")
-
-    def test_handle_error(self):
-        """Test _handler_error() function."""
+        """Write an error to the given style checker."""
+        style_checker._handle_style_error(filename="filename",
+                                          line_number=1,
+                                          category="category",
+                                          confidence=error_confidence,
+                                          message="message")
+
+    def test_handle_style_error(self):
+        """Test _handle_style_error() function."""
         options = ProcessorOptions(output_format="emacs",
                                    verbosity=3)
@@ -397 +564 @@
         # Verify initialized properly.
         self.assertEquals(style_checker.error_count, 0)
-        self.assertEquals(self.error_message, "")
+        self.assertEquals(self.error_messages, "")
 
         # Check that should_print_error is getting called appropriately.
         self.write_sample_error(style_checker, 2)
         self.assertEquals(style_checker.error_count, 0) # Error confidence too low.
-        self.assertEquals(self.error_message, "")
+        self.assertEquals(self.error_messages, "")
 
         self.write_sample_error(style_checker, 3)
         self.assertEquals(style_checker.error_count, 1) # Error confidence just high enough.
-        self.assertEquals(self.error_message, "filename:1:  message  [category] [3]\n")
+        self.assertEquals(self.error_messages, "filename:1:  message  [category] [3]\n")
+
+        # Clear previous errors.
+        self.error_messages = ""
 
         # Check "vs7" output format.
@@ -412 +582 @@
         self.write_sample_error(style_checker, 3)
         self.assertEquals(style_checker.error_count, 2) # Error confidence just high enough.
-        self.assertEquals(self.error_message, "filename(1):  message  [category] [3]\n")
+        self.assertEquals(self.error_messages, "filename(1):  message  [category] [3]\n")
+
+
+class StyleCheckerCheckFileTest(unittest.TestCase):
+
+    """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.
+
+    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.
+
+    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.
+
+    """
+    def setUp(self):
+        self.got_file_path = None
+        self.got_handle_style_error = None
+        self.got_processor = None
+        self.warning_messages = ""
+
+    def mock_stderr_write(self, warning_message):
+        self.warning_messages += warning_message
+
+    def mock_handle_style_error(self):
+        pass
+
+    def mock_process_file(self, processor, file_path, handle_style_error):
+        """A mock _process_file().
+
+        See the documentation for this class for more information
+        on this function.
+
+        """
+        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, "")
+
+        # Create a test StyleChecker instance.
+        #
+        # The verbosity attribute is the only ProcessorOptions
+        # attribute that needs to be checked in this test.
+        # This is because it is the only option is directly
+        # passed to the constructor of a style processor.
+        options = ProcessorOptions(verbosity=3)
+
+        style_checker = StyleChecker(options, self.mock_stderr_write)
+
+        style_checker.check_file(file_path,
+                                 self.mock_handle_style_error,
+                                 self.mock_process_file)
+
+    def test_check_file_on_skip_without_warning(self):
+        """Test check_file() for a skipped-without-warning file."""
+
+        file_path = "LayoutTests/foo.txt"
+
+        dispatcher = ProcessorDispatcher()
+        # Confirm that the input file is truly a skipped-without-warning file.
+        self.assertTrue(dispatcher.should_skip_without_warning(file_path))
+
+        # Check the outcome.
+        self.call_check_file(file_path)
+        self.assert_attributes(None, None, None, "")
+
+    def test_check_file_on_skip_with_warning(self):
+        """Test check_file() for a skipped-with-warning file."""
+
+        file_path = "gtk2drawing.c"
+
+        dispatcher = ProcessorDispatcher()
+        # Check that the input file is truly a skipped-with-warning file.
+        self.assertTrue(dispatcher.should_skip_with_warning(file_path))
+
+        # Check the outcome.
+        self.call_check_file(file_path)
+        self.assert_attributes(None, None, None,
+                               'Ignoring "gtk2drawing.c": this file is exempt from the style guide.\n')
+
+    def test_check_file_on_non_skipped(self):
+
+        # 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 verbosity).
+        file_base = "foo"
+        file_extension = "cpp"
+        file_path = file_base + "." + file_extension
+
+        dispatcher = ProcessorDispatcher()
+        # Check that the input file is truly a C++ file.
+        self.assertEquals(dispatcher._file_type(file_path), style.FileType.CPP)
+
+        # Check the outcome.
+        self.call_check_file(file_path)
+
+        expected_processor = CppProcessor(file_path, file_extension, self.mock_handle_style_error, 3)
+
+        self.assert_attributes(file_path,
+                               self.mock_handle_style_error,
+                               expected_processor,
+                               "")
 
 
@@ -419 +718 @@
 
     unittest.main()
+

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

@@ -5 +5 @@
 # Copyright (C) 2009 Torch Mobile Inc.
 # Copyright (C) 2009 Apple Inc. All rights reserved.
+# Copyright (C) 2010 Chris Jerdonek (cjerdonek@webkit.org)
 #
 # Redistribution and use in source and binary forms, with or without
@@ -2821 +2822 @@
 
 
-def process_file_data(filename, file_extension, lines, error, verbosity):
+def _process_lines(filename, file_extension, lines, error, verbosity):
     """Performs lint checks and reports any errors to the given error function.
 
@@ -2860 +2861 @@
 
 
-def process_file(filename, error, verbosity):
-    """Performs cpp_style on a single file.
-
-    Args:
-      filename: The name of the file to parse.
-      error: The function to call with any errors found.
-      verbosity: An integer that is the verbosity level to use while
-                 checking style.
-    """
-    try:
-        # Support the UNIX convention of using "-" for stdin.  Note that
-        # we are not opening the file with universal newline support
-        # (which codecs doesn't support anyway), so the resulting lines do
-        # contain trailing '\r' characters if we are reading a file that
-        # has CRLF endings.
-        # If after the split a trailing '\r' is present, it is removed
-        # below. If it is not expected to be present (i.e. os.linesep !=
-        # '\r\n' as in Windows), a warning is issued below if this file
-        # is processed.
-
-        if filename == '-':
-            lines = codecs.StreamReaderWriter(sys.stdin,
-                                              codecs.getreader('utf8'),
-                                              codecs.getwriter('utf8'),
-                                              'replace').read().split('\n')
-        else:
-            lines = codecs.open(filename, 'r', 'utf8', 'replace').read().split('\n')
-
-        carriage_return_found = False
-        # Remove trailing '\r'.
-        for line_number in range(len(lines)):
-            if lines[line_number].endswith('\r'):
-                lines[line_number] = lines[line_number].rstrip('\r')
-                carriage_return_found = True
-
-    except IOError:
-        sys.stderr.write(
-            "Skipping input '%s': Can't open for reading\n" % filename)
-        return
-
-    # Note, if no dot is found, this will give the entire filename as the ext.
-    file_extension = filename[filename.rfind('.') + 1:]
-
-    # When reading from stdin, the extension is unknown, so no cpp_style tests
-    # should rely on the extension.
-    if (filename != '-' and not can_handle(filename)):
-        sys.stderr.write('Ignoring %s; not a .cpp, .c or .h file\n' % filename)
-    elif is_exempt(filename):
-        sys.stderr.write('Ignoring %s; This file is exempt from the style guide.\n' % filename)
-    else:
-        process_file_data(filename, file_extension, lines, error, verbosity)
-        if carriage_return_found and os.linesep != '\r\n':
-            # Use 0 for line_number since outputing only one error for potentially
-            # several lines.
-            error(filename, 0, 'whitespace/newline', 1,
-                  'One or more unexpected \\r (^M) found;'
-                  'better to use only a \\n')
-
-
-def can_handle(filename):
-    """Checks if this module supports for the specified file type.
-
-    Args:
-      filename: A filename. It may contain directory names.
-     """
-    return os.path.splitext(filename)[1] in ('.h', '.cpp', '.c')
-
-
-def is_exempt(filename):
-    """Checks if the given file is exempt from the style guide.  For example,
-    some files are purposefully mantained in Mozilla style to ease future
-    merges.
-
-    Args:
-      filename: A filename. It may contain directory names.
-     """
-    if (filename.find('WebKit/qt/Api/') >= 0
-        or filename.find('JavaScriptCore/qt/api/') >= 0
-        or filename.find('WebKit/qt/tests/') >= 0
-        or filename.find('WebKit/gtk/tests/') >= 0):
-        # The Qt API and the Qt and Gtk tests do not follow WebKit style.
+class CppProcessor(object):
+
+    """Processes C++ lines for checking style."""
+
+    def __init__(self, file_path, file_extension, handle_style_error, verbosity):
+        """Create a CppProcessor instance.
+
+        Args:
+          file_extension: A string that is the file extension, without
+                          the leading dot.
+
+        """
+        self.file_extension = file_extension
+        self.file_path = file_path
+        self.handle_style_error = handle_style_error
+        self.verbosity = verbosity
+
+    # Useful for unit testing.
+    def __eq__(self, other):
+        """Return whether this CppProcessor instance is equal to another."""
+        if self.file_extension != other.file_extension:
+            return False
+        if self.file_path != other.file_path:
+            return False
+        if self.handle_style_error != other.handle_style_error:
+            return False
+        if self.verbosity != other.verbosity:
+            return False
+
         return True
 
-    return os.path.basename(filename) in (
-        'gtk2drawing.c',
-        'gtk2drawing.h',
-    )
+    # Useful for unit testing.
+    def __ne__(self, other):
+        # Python does not automatically deduce __ne__() from __eq__().
+        return not self.__eq__(other)
+
+    def process(self, lines):
+        _process_lines(self.file_path, self.file_extension, lines,
+                       self.handle_style_error, self.verbosity)
+
+
+# FIXME: Remove this function (requires refactoring unit tests).
+def process_file_data(filename, file_extension, lines, error, verbosity):
+    processor = CppProcessor(filename, file_extension, error, verbosity)
+    processor.process(lines)
+

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

@@ -5 +5 @@
 # Copyright (C) 2009 Torch Mobile Inc.
 # Copyright (C) 2009 Apple Inc. All rights reserved.
+# Copyright (C) 2010 Chris Jerdonek (cjerdonek@webkit.org)
 #
 # Redistribution and use in source and binary forms, with or without
@@ -42 +43 @@
 import unittest
 import cpp_style
+from cpp_style import CppProcessor
+
 # FIXME: Remove the need to import anything from checker. See the
 #        FIXME notes near the STYLE_CATEGORIES definition for a
@@ -3607 +3610 @@
 
 
-    def test_can_handle(self):
-        """Tests for cpp_style.can_handle()."""
-        self.assert_(not cpp_style.can_handle(''))
-        self.assert_(cpp_style.can_handle('foo.h'))
-        self.assert_(not cpp_style.can_handle('foo.hpp'))
-        self.assert_(cpp_style.can_handle('foo.c'))
-        self.assert_(cpp_style.can_handle('foo.cpp'))
-        self.assert_(not cpp_style.can_handle('foo.cc'))
-        self.assert_(not cpp_style.can_handle('foo.cxx'))
-        self.assert_(not cpp_style.can_handle('foo.C'))
-        self.assert_(not cpp_style.can_handle('foo.mm'))
-        self.assert_(not cpp_style.can_handle('-'))
-
-    def test_is_exempt(self):
-        """Tests for cpp_style.is_exempt()."""
-        self.assert_(not cpp_style.is_exempt(''))
-        self.assert_(not cpp_style.is_exempt('foo.h'))
-        self.assert_(not cpp_style.is_exempt('foo.hpp'))
-        self.assert_(not cpp_style.is_exempt('foo.c'))
-        self.assert_(not cpp_style.is_exempt('foo.cpp'))
-        self.assert_(not cpp_style.is_exempt('-'))
-        self.assert_(cpp_style.is_exempt('gtk2drawing.h'))
-        self.assert_(cpp_style.is_exempt('WebCore/platform/gtk/gtk2drawing.h'))
-        self.assert_(cpp_style.is_exempt('gtk2drawing.c'))
-        self.assert_(cpp_style.is_exempt('WebCore/platform/gtk/gtk2drawing.c'))
-        self.assert_(cpp_style.is_exempt('WebKit/qt/Api/qwebpage.h'))
-        self.assert_(cpp_style.is_exempt('JavaScriptCore/qt/api/qscriptengine_p.h'))
-        self.assert_(cpp_style.is_exempt('WebKit/qt/tests/qwebsecurityorigin/tst_qwebsecurityorigin.cpp'))
-        self.assert_(cpp_style.is_exempt('WebKit/gtk/tests/testatk.c'))
+class CppProcessorTest(unittest.TestCase):
+
+    """Tests CppProcessor class."""
+
+    def mock_handle_style_error(self):
+        pass
+
+    def _processor(self):
+        return CppProcessor("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.verbosity, 3)
+
+    def test_eq(self):
+        """Test __eq__ equality function."""
+        processor1 = self._processor()
+        processor2 = self._processor()
+
+        # == calls __eq__.
+        self.assertTrue(processor1 == processor2)
+
+        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))
+
+    def test_ne(self):
+        """Test __ne__ inequality function."""
+        processor1 = self._processor()
+        processor2 = self._processor()
+
+        # != 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)
+
 
 def tearDown():

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

@@ -1 +1 @@
 # Copyright (C) 2009 Google Inc. All rights reserved.
+# Copyright (C) 2010 Chris Jerdonek (cjerdonek@webkit.org)
 #
 # Redistribution and use in source and binary forms, with or without
@@ -27 +28 @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-"""Does WebKit-lint on text files.
-
-This module shares error count, filter setting, output setting, etc. with cpp_style.
-"""
-
-import codecs
-import os.path
-import sys
-
-import cpp_style
+"""Checks WebKit style for text files."""
 
 
-def process_file_data(filename, lines, error):
-    """Performs lint check for text on the specified lines.
+class TextProcessor(object):
 
-    It reports errors to the given error function.
-    """
-    lines = (['// adjust line numbers to make the first line 1.'] + lines)
+    """Processes text lines for checking style."""
 
-    # FIXME: share with cpp_style.check_style()
-    for line_number, line in enumerate(lines):
-        if '\t' in line:
-            error(filename, line_number, 'whitespace/tab', 5, 'Line contains tab character.')
+    def __init__(self, file_path, handle_style_error):
+        self.file_path = file_path
+        self.handle_style_error = handle_style_error
+
+    def process(self, lines):
+        lines = (["// adjust line numbers to make the first line 1."] + lines)
+
+        # FIXME: share with cpp_style.
+        for line_number, line in enumerate(lines):
+            if "\t" in line:
+                self.handle_style_error(self.file_path, line_number,
+                                        "whitespace/tab", 5,
+                                        "Line contains tab character.")
 
 
-def process_file(filename, error):
-    """Performs lint check for text on a single file."""
-    if (not can_handle(filename)):
-        sys.stderr.write('Ignoring %s; not a supported file\n' % filename)
-        return
+# FIXME: Remove this function (requires refactoring unit tests).
+def process_file_data(filename, lines, error):
+    processor = TextProcessor(filename, error)
+    processor.process(lines)
 
-    # FIXME: share code with cpp_style.process_file().
-    try:
-        # Do not support for filename='-'. cpp_style handles it.
-        lines = codecs.open(filename, 'r', 'utf8', 'replace').read().split('\n')
-    except IOError:
-        sys.stderr.write("Skipping input '%s': Can't open for reading\n" % filename)
-        return
-    process_file_data(filename, lines, error)
-
-
-def can_handle(filename):
-    """Checks if this module supports the specified file type.
-
-    Args:
-      filename: A filename. It may contain directory names.
-    """
-    return ("ChangeLog" in filename
-            or "WebKitTools/Scripts/" in filename
-            or os.path.splitext(filename)[1] in ('.css', '.html', '.idl', '.js', '.mm', '.php', '.pm', '.py', '.txt')) and not "LayoutTests/" in filename

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

@@ -33 +33 @@
 
 import text_style
-
+from text_style import TextProcessor
 
 class TextStyleTestCase(unittest.TestCase):
@@ -77 +77 @@
 
 
-    def test_can_handle(self):
-        """Tests for text_style.can_handle()."""
-        self.assert_(not text_style.can_handle(''))
-        self.assert_(not text_style.can_handle('-'))
-        self.assert_(text_style.can_handle('ChangeLog'))
-        self.assert_(text_style.can_handle('WebCore/ChangeLog'))
-        self.assert_(text_style.can_handle('FooChangeLog.bak'))
-        self.assert_(text_style.can_handle('WebKitTools/Scripts/check-webkit=style'))
-        self.assert_(text_style.can_handle('WebKitTools/Scripts/modules/text_style.py'))
-        self.assert_(not text_style.can_handle('WebKitTools/Scripts'))
+class TextProcessorTest(unittest.TestCase):
 
-        self.assert_(text_style.can_handle('foo.css'))
-        self.assert_(text_style.can_handle('foo.html'))
-        self.assert_(text_style.can_handle('foo.idl'))
-        self.assert_(text_style.can_handle('foo.js'))
-        self.assert_(text_style.can_handle('WebCore/inspector/front-end/inspector.js'))
-        self.assert_(text_style.can_handle('foo.mm'))
-        self.assert_(text_style.can_handle('foo.php'))
-        self.assert_(text_style.can_handle('foo.pm'))
-        self.assert_(text_style.can_handle('foo.py'))
-        self.assert_(text_style.can_handle('foo.txt'))
-        self.assert_(not text_style.can_handle('foo.c'))
-        self.assert_(not text_style.can_handle('foo.c'))
-        self.assert_(not text_style.can_handle('foo.c'))
-        self.assert_(not text_style.can_handle('foo.png'))
-        self.assert_(not text_style.can_handle('foo.c/bar.png'))
-        self.assert_(not text_style.can_handle('WebKit/English.lproj/Localizable.strings'))
-        self.assert_(not text_style.can_handle('Makefile'))
-        self.assert_(not text_style.can_handle('WebCore/Android.mk'))
-        self.assert_(not text_style.can_handle('LayoutTests/inspector/console-tests.js'))
+    """Tests TextProcessor 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)