[SCM] WebKit Debian packaging branch, webkit-1.1, updated. upstream/1.1.19-706-ge5415e9
eric at webkit.org
eric at webkit.org
Thu Feb 4 21:24:22 UTC 2010
The following commit has been merged in the webkit-1.1 branch:
commit ae29d1a594555488aa40b303492aa9a05710686f
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Fri Jan 22 05:39:24 2010 +0000
2010-01-21 Chris Jerdonek <cjerdonek at 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.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@53675 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index ed48a2b..d144e9d 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,5 +1,56 @@
2010-01-21 Chris Jerdonek <cjerdonek at 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 at webkit.org>
+
Reviewed by David Kilzer.
Create a unit-tested subroutine to parse patch files created
diff --git a/WebKitTools/Scripts/check-webkit-style b/WebKitTools/Scripts/check-webkit-style
index 1475e6a..a2b584f 100755
--- a/WebKitTools/Scripts/check-webkit-style
+++ b/WebKitTools/Scripts/check-webkit-style
@@ -68,7 +68,7 @@ def main():
if files:
for filename in files:
- style_checker.process_file(filename)
+ style_checker.check_file(filename)
else:
cwd = os.path.abspath('.')
@@ -84,7 +84,7 @@ def main():
patch = scm.create_patch_since_local_commit(commit)
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)
sys.exit(style_checker.error_count > 0)
diff --git a/WebKitTools/Scripts/webkitpy/style/checker.py b/WebKitTools/Scripts/webkitpy/style/checker.py
index c3f4db0..68c5abe 100644
--- a/WebKitTools/Scripts/webkitpy/style/checker.py
+++ b/WebKitTools/Scripts/webkitpy/style/checker.py
@@ -29,15 +29,14 @@
"""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
# These defaults are used by check-webkit-style.
@@ -158,6 +157,30 @@ STYLE_CATEGORIES = [
]
+# 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."""
return ArgumentDefaults(WEBKIT_DEFAULT_OUTPUT_FORMAT,
@@ -619,6 +642,104 @@ class ArgumentParser(object):
return (filenames, options)
+# 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):
"""Supports checking style in files and patches.
@@ -632,20 +753,25 @@ class StyleChecker(object):
"""
- def __init__(self, options, write_error=sys.stderr.write):
+ def __init__(self, options, stderr_write=None):
"""Create a StyleChecker instance.
Args:
options: See options attribute.
stderr_write: A function that takes a string as a parameter
and that is called when a style error occurs.
+ Defaults to sys.stderr.write. This should be
+ used only for unit tests.
"""
- self._write_error = write_error
- self.options = options
+ if stderr_write is None:
+ stderr_write = sys.stderr.write
+
+ self._stderr_write = stderr_write
self.error_count = 0
+ self.options = options
- 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.
Check whether an error should be reported. If so, increment the
@@ -675,37 +801,104 @@ class StyleChecker(object):
else:
format_string = "%s:%s: %s [%s] [%d]\n"
- self._write_error(format_string % (filename, line_number, message,
- category, confidence))
+ self._stderr_write(format_string % (filename, line_number, message,
+ category, confidence))
- def process_file(self, filename):
- """Checks style for the specified file.
+ 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
- If the specified filename is '-', applies cpp_style to the standard input.
+ 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:
+ 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 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)
+ if handle_style_error is None:
+ handle_style_error = self._handle_style_error
- def process_patch(self, patch_string):
- """Does lint on a single patch.
+ 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 of a patch.
+ 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:
for line in diff.lines:
@@ -714,11 +907,10 @@ class StyleChecker(object):
if not line[0]:
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)
+ self._handle_style_error(file_path, line_number, category, confidence, message)
+
+ self.check_file(file_path, handle_style_error=error_for_patch)
- # 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)
diff --git a/WebKitTools/Scripts/webkitpy/style/checker_unittest.py b/WebKitTools/Scripts/webkitpy/style/checker_unittest.py
index 5a6e650..28904a9 100755
--- a/WebKitTools/Scripts/webkitpy/style/checker_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/style/checker_unittest.py
@@ -38,8 +38,11 @@ import unittest
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):
@@ -352,25 +355,189 @@ class ArgumentParserTest(unittest.TestCase):
self.assertEquals(files, ['foo.cpp', 'bar.cpp'])
+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):
"""Test the StyleChecker class.
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 setUp(self):
+ self.error_messages = ""
- def mock_write_error(self, error_message):
- """Store an error message without printing it."""
- self.error_message = error_message
+ 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):
"""Test __init__ constructor."""
@@ -381,40 +548,173 @@ class StyleCheckerTest(unittest.TestCase):
self.assertEquals(style_checker.options, options)
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)
style_checker = self.style_checker(options)
# 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.
style_checker.options.output_format = "vs7"
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,
+ "")
if __name__ == '__main__':
import sys
unittest.main()
+
diff --git a/WebKitTools/Scripts/webkitpy/style/cpp_style.py b/WebKitTools/Scripts/webkitpy/style/cpp_style.py
index 3855b3b..2c39e1a 100644
--- a/WebKitTools/Scripts/webkitpy/style/cpp_style.py
+++ b/WebKitTools/Scripts/webkitpy/style/cpp_style.py
@@ -4,6 +4,7 @@
# Copyright (C) 2009 Google Inc. All rights reserved.
# Copyright (C) 2009 Torch Mobile Inc.
# Copyright (C) 2009 Apple Inc. All rights reserved.
+# Copyright (C) 2010 Chris Jerdonek (cjerdonek at webkit.org)
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are
@@ -2820,7 +2821,7 @@ def process_line(filename, file_extension,
check_invalid_increment(filename, clean_lines, line, error)
-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.
Args:
@@ -2859,90 +2860,49 @@ def process_file_data(filename, file_extension, lines, error, verbosity):
check_for_new_line_at_eof(filename, lines, error)
-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')
+class CppProcessor(object):
- 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
+ """Processes C++ lines for checking style."""
- except IOError:
- sys.stderr.write(
- "Skipping input '%s': Can't open for reading\n" % filename)
- return
+ def __init__(self, file_path, file_extension, handle_style_error, verbosity):
+ """Create a CppProcessor instance.
- # Note, if no dot is found, this will give the entire filename as the ext.
- file_extension = filename[filename.rfind('.') + 1:]
+ Args:
+ file_extension: A string that is the file extension, without
+ the leading dot.
- # 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')
+ """
+ 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
-def can_handle(filename):
- """Checks if this module supports for the specified file type.
+ return True
- Args:
- filename: A filename. It may contain directory names.
- """
- return os.path.splitext(filename)[1] in ('.h', '.cpp', '.c')
+ # 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)
-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.
- return True
+# 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)
- return os.path.basename(filename) in (
- 'gtk2drawing.c',
- 'gtk2drawing.h',
- )
diff --git a/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py b/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py
index 4f4f1af..c03958c 100644
--- a/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py
@@ -4,6 +4,7 @@
# Copyright (C) 2009 Google Inc. All rights reserved.
# Copyright (C) 2009 Torch Mobile Inc.
# Copyright (C) 2009 Apple Inc. All rights reserved.
+# Copyright (C) 2010 Chris Jerdonek (cjerdonek at webkit.org)
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are
@@ -41,6 +42,8 @@ import random
import re
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
# suggestion on how to best do this.
@@ -3606,35 +3609,53 @@ 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 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():
"""A global check to make sure all error-categories have been tested.
diff --git a/WebKitTools/Scripts/webkitpy/style/text_style.py b/WebKitTools/Scripts/webkitpy/style/text_style.py
index 385bbaa..4f869af 100644
--- a/WebKitTools/Scripts/webkitpy/style/text_style.py
+++ b/WebKitTools/Scripts/webkitpy/style/text_style.py
@@ -1,4 +1,5 @@
# Copyright (C) 2009 Google Inc. All rights reserved.
+# Copyright (C) 2010 Chris Jerdonek (cjerdonek at webkit.org)
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are
@@ -26,53 +27,30 @@
# (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.
+"""Checks WebKit style for text files."""
-This module shares error count, filter setting, output setting, etc. with cpp_style.
-"""
-import codecs
-import os.path
-import sys
+class TextProcessor(object):
-import cpp_style
+ """Processes text lines for checking style."""
+ def __init__(self, file_path, handle_style_error):
+ self.file_path = file_path
+ self.handle_style_error = handle_style_error
-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(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: 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.
+# FIXME: Remove this function (requires refactoring unit tests).
+def process_file_data(filename, lines, error):
+ processor = TextProcessor(filename, error)
+ processor.process(lines)
- 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/webkitpy/style/text_style_unittest.py b/WebKitTools/Scripts/webkitpy/style/text_style_unittest.py
index d9115f0..39b4254 100644
--- a/WebKitTools/Scripts/webkitpy/style/text_style_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/style/text_style_unittest.py
@@ -32,7 +32,7 @@
import unittest
import text_style
-
+from text_style import TextProcessor
class TextStyleTestCase(unittest.TestCase):
"""TestCase for text_style.py"""
@@ -76,36 +76,18 @@ class TextStyleTestCase(unittest.TestCase):
'\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'))
+class TextProcessorTest(unittest.TestCase):
+
+ """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)
if __name__ == '__main__':
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list