2009-12-16 Kent Tamura <tkent@chromium.org>
authortkent@chromium.org <tkent@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 17 Dec 2009 05:30:45 +0000 (05:30 +0000)
committertkent@chromium.org <tkent@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 17 Dec 2009 05:30:45 +0000 (05:30 +0000)
        Reviewed by David Levin.

        check-webkit-style supports for TAB check against text files.
        https://bugs.webkit.org/show_bug.cgi?id=32538

        * Scripts/check-webkit-style:
          Move process_patch() to style.py.
        * Scripts/modules/cpp_style.py:
          Add can_handle().
        * Scripts/modules/cpp_style_unittest.py:
          Add tests for can_handle().
        * Scripts/modules/style.py:
          Added. This is a front-end of cpp_style and text_style. It dispatches
          files to an appropriate linter.
        * Scripts/modules/text_style.py:
          Added. This is a linter module for generic text files. It supports
          only for TAB checking at this moment.
        * Scripts/modules/text_style_unittest.py:
          Added. Tests for text_style.py.
        * Scripts/run-webkit-unittests:
          Add text_style_unittest.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@52232 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebKitTools/ChangeLog
WebKitTools/Scripts/check-webkit-style
WebKitTools/Scripts/modules/cpp_style.py
WebKitTools/Scripts/modules/cpp_style_unittest.py
WebKitTools/Scripts/modules/style.py [new file with mode: 0644]
WebKitTools/Scripts/modules/text_style.py [new file with mode: 0644]
WebKitTools/Scripts/modules/text_style_unittest.py [new file with mode: 0644]
WebKitTools/Scripts/run-webkit-unittests

index 7dc2a1a..963d693 100644 (file)
@@ -1,3 +1,27 @@
+2009-12-16  Kent Tamura  <tkent@chromium.org>
+
+        Reviewed by David Levin.
+
+        check-webkit-style supports for TAB check against text files.
+        https://bugs.webkit.org/show_bug.cgi?id=32538
+
+        * Scripts/check-webkit-style:
+          Move process_patch() to style.py.
+        * Scripts/modules/cpp_style.py:
+          Add can_handle().
+        * Scripts/modules/cpp_style_unittest.py:
+          Add tests for can_handle().
+        * Scripts/modules/style.py:
+          Added. This is a front-end of cpp_style and text_style. It dispatches
+          files to an appropriate linter.
+        * Scripts/modules/text_style.py:
+          Added. This is a linter module for generic text files. It supports
+          only for TAB checking at this moment.
+        * Scripts/modules/text_style_unittest.py:
+          Added. Tests for text_style.py.
+        * Scripts/run-webkit-unittests:
+          Add text_style_unittest.
+
 2009-12-16  Eric Seidel  <eric@webkit.org>
 
         Reviewed by David Levin.
index dceca2a..96f2c8d 100755 (executable)
@@ -28,7 +28,7 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-"""Does WebKit-lint on C/C++ files.
+"""Does WebKit-lint on C/C++ or text files.
 
 The goal of this script is to identify places in the code that *may*
 be in non-compliance with WebKit style.  It does not attempt to fix
@@ -47,10 +47,11 @@ import os.path
 import sys
 
 import modules.cpp_style as cpp_style
-from modules.diff_parser import DiffParser
+import modules.style as style
 from modules.scm import detect_scm_system
 
 
+# FIXME: Avoid cpp_style dependency.
 cpp_style._USAGE = """
 Syntax: %(program_name)s [--verbose=#] [--git-commit=<SingleCommit>] [--output=vs7]
         [--filter=-x,+y,...] [file] ...
@@ -115,43 +116,11 @@ Syntax: %(program_name)s [--verbose=#] [--git-commit=<SingleCommit>] [--output=v
     }
 
 
-def process_patch(patch_string):
-    """Does lint on a single patch.
-
-    Args:
-      patch_string: A string of a patch.
-    """
-    patch = DiffParser(patch_string.splitlines())
-    for filename, diff in patch.files.iteritems():
-        file_extension = os.path.splitext(filename)[1]
-
-        if file_extension in ['.cpp', '.c', '.h']:
-            line_numbers = set()
-
-            def error_for_patch(filename, 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:
-                    for line in diff.lines:
-                        # When deleted line is not set, it means that
-                        # the line is newly added.
-                        if not line[0]:
-                            line_numbers.add(line[1])
-
-                if line_number in line_numbers:
-                    cpp_style.error(filename, line_number, category, confidence, message)
-
-            cpp_style.process_file(filename, error=error_for_patch)
-
-
 def main():
-    cpp_style._DEFAULT_FILTER_RULES = cpp_style._WEBKIT_FILTER_RULES
+    style.use_webkit_styles()
 
-    (files, flags) = cpp_style.parse_arguments(sys.argv[1:], ["git-commit="],
-                                               display_help=True)
+    (files, flags) = style.parse_arguments(sys.argv[1:], ["git-commit="],
+                                           display_help=True)
 
     # Change stderr to write with replacement characters so we don't die
     # if we try to print something containing non-ASCII characters.
@@ -167,7 +136,7 @@ def main():
 
     if files:
         for filename in files:
-            cpp_style.process_file(filename)
+            style.process_file(filename)
 
     else:
         cwd = os.path.abspath('.')
@@ -183,10 +152,10 @@ def main():
             patch = scm.create_patch_since_local_commit(commit)
         else:
             patch = scm.create_patch()
-        process_patch(patch)
+        style.process_patch(patch)
 
-    sys.stderr.write('Total errors found: %d\n' % cpp_style.error_count())
-    sys.exit(cpp_style.error_count() > 0)
+    sys.stderr.write('Total errors found: %d\n' % style.error_count())
+    sys.exit(style.error_count() > 0)
 
 
 if __name__ == "__main__":
index dc9d874..3c67c25 100644 (file)
@@ -3158,8 +3158,7 @@ def process_file(filename, error=error):
 
     # When reading from stdin, the extension is unknown, so no cpp_style tests
     # should rely on the extension.
-    if (filename != '-' and file_extension != 'h' and file_extension != 'cpp'
-        and file_extension != 'c'):
+    if (filename != '-' and not can_handle(filename)):
         sys.stderr.write('Ignoring %s; not a .cpp, .c or .h file\n' % filename)
     else:
         process_file_data(filename, file_extension, lines, error)
@@ -3261,3 +3260,12 @@ def parse_arguments(args, additional_flags=[], display_help=False):
     _set_filters(filters)
 
     return (filenames, additional_flag_values)
+
+
+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')
index 1acfc60..80a9868 100644 (file)
@@ -3717,6 +3717,20 @@ class WebKitStyleTest(CppStyleTestBase):
         pass
 
 
+    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 tearDown():
     """A global check to make sure all error-categories have been tested.
 
diff --git a/WebKitTools/Scripts/modules/style.py b/WebKitTools/Scripts/modules/style.py
new file mode 100644 (file)
index 0000000..a64ce0b
--- /dev/null
@@ -0,0 +1,100 @@
+# Copyright (C) 2009 Google Inc. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+#     * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+#     * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+"""Front end of some style-checker modules."""
+
+# FIXME: Move more code from cpp_style to here.
+# check-webkit-style should not refer cpp_style directly.
+
+import os.path
+
+import modules.cpp_style as cpp_style
+import modules.text_style as text_style
+from modules.diff_parser import DiffParser
+
+
+def use_webkit_styles():
+    """Configures this module for WebKit style."""
+    cpp_style._DEFAULT_FILTER_RULES = cpp_style._WEBKIT_FILTER_RULES
+
+
+def parse_arguments(args, additional_flags=[], display_help=False):
+    """Parses the command line arguments.
+
+    See cpp_style.parse_arguments() for details.
+    """
+    return cpp_style.parse_arguments(args, additional_flags, display_help)
+
+
+def process_file(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)
+    elif text_style.can_handle(filename):
+        text_style.process_file(filename)
+
+
+def process_patch(patch_string):
+    """Does lint on a single patch.
+
+    Args:
+      patch_string: A string of a patch.
+    """
+    patch = DiffParser(patch_string.splitlines())
+    for filename, diff in patch.files.iteritems():
+        file_extension = os.path.splitext(filename)[1]
+        line_numbers = set()
+
+        def error_for_patch(filename, 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:
+                for line in diff.lines:
+                    # When deleted line is not set, it means that
+                    # the line is newly added.
+                    if not line[0]:
+                        line_numbers.add(line[1])
+
+            if line_number in line_numbers:
+                cpp_style.error(filename, line_number, category, confidence, message)
+
+        if cpp_style.can_handle(filename):
+            cpp_style.process_file(filename, error=error_for_patch)
+        elif text_style.can_handle(filename):
+            text_style.process_file(filename, error=error_for_patch)
+
+
+def error_count():
+    """Returns the total error count."""
+    return cpp_style.error_count()
diff --git a/WebKitTools/Scripts/modules/text_style.py b/WebKitTools/Scripts/modules/text_style.py
new file mode 100644 (file)
index 0000000..9496558
--- /dev/null
@@ -0,0 +1,78 @@
+# Copyright (C) 2009 Google Inc. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+#     * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+#     * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# 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
+
+
+def process_file_data(filename, lines, error):
+    """Performs lint check for text on the specified lines.
+
+    It reports errors to the given error function.
+    """
+    lines = (['// adjust line numbers to make the first line 1.'] + lines)
+
+    # 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 process_file(filename, error=cpp_style.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: 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
diff --git a/WebKitTools/Scripts/modules/text_style_unittest.py b/WebKitTools/Scripts/modules/text_style_unittest.py
new file mode 100644 (file)
index 0000000..d9115f0
--- /dev/null
@@ -0,0 +1,112 @@
+#!/usr/bin/python
+# Copyright (C) 2009 Google Inc. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+#     * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+#     * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+"""Unit test for text_style.py."""
+
+import unittest
+
+import text_style
+
+
+class TextStyleTestCase(unittest.TestCase):
+    """TestCase for text_style.py"""
+
+    def assertNoError(self, lines):
+        """Asserts that the specified lines has no errors."""
+        self.had_error = False
+
+        def error_for_test(filename, line_number, category, confidence, message):
+            """Records if an error occurs."""
+            self.had_error = True
+
+        text_style.process_file_data('', lines, error_for_test)
+        self.assert_(not self.had_error, '%s should not have any errors.' % lines)
+
+    def assertError(self, lines, expected_line_number):
+        """Asserts that the specified lines has an error."""
+        self.had_error = False
+
+        def error_for_test(filename, line_number, category, confidence, message):
+            """Checks if the expected error occurs."""
+            self.assertEquals(expected_line_number, line_number)
+            self.assertEquals('whitespace/tab', category)
+            self.had_error = True
+
+        text_style.process_file_data('', lines, error_for_test)
+        self.assert_(self.had_error, '%s should have an error [whitespace/tab].' % lines)
+
+
+    def test_no_error(self):
+        """Tests for no error cases."""
+        self.assertNoError([''])
+        self.assertNoError(['abc def', 'ggg'])
+
+
+    def test_error(self):
+        """Tests for error cases."""
+        self.assertError(['2009-12-16\tKent Tamura\t<tkent@chromium.org>'], 1)
+        self.assertError(['2009-12-16 Kent Tamura <tkent@chromium.org>',
+                          '',
+                          '\tReviewed by NOBODY.'], 3)
+
+
+    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'))
+
+        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'))
+
+
+if __name__ == '__main__':
+    unittest.main()
index 403f987..4696831 100755 (executable)
@@ -43,6 +43,7 @@ from modules.cpp_style_unittest import *
 from modules.diff_parser_unittest import *
 from modules.logging_unittest import *
 from modules.multicommandtool_unittest import *
+from modules.text_style_unittest import *
 from modules.webkitport_unittest import *
 from modules.workqueue_unittest import *