[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373
eric at webkit.org
eric at webkit.org
Thu Apr 8 01:05:26 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit 6f94cad9ef020f01d502cff143fa62e7c6762a79
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Thu Jan 14 10:19:13 2010 +0000
2010-01-14 Chris Jerdonek <chris.jerdonek at gmail.com>
Reviewed by Shinichiro Hamaji.
Moved error() from cpp_style.py to checker.py.
https://bugs.webkit.org/show_bug.cgi?id=33620
* Scripts/check-webkit-style:
- Addressed FIXME to not set global state.
* Scripts/webkitpy/style/checker.py:
- Added argument validation to ProcessorOptions constructor.
- Added should_report_error() to ProcessorOptions class.
- Removed set_options().
- Added StyleChecker class.
* Scripts/webkitpy/style/checker_unittest.py:
- Added unit test class for ProcessorOptions class.
- Added unit test to check that parse() strips white space.
* Scripts/webkitpy/style/cpp_style.py:
- Removed "filter" and "output_format" methods.
- Removed should_print_error() and error() functions.
- Removed default parameter value from process_file().
* Scripts/webkitpy/style/cpp_style_unittest.py:
- Removed call to cpp_style._should_print_error().
- Removed test_filter() and test_filter_appending().
* Scripts/webkitpy/style/text_style.py:
- Removed default parameter value from process_file().
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@53251 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 0d942dd..4c91861 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,36 @@
+2010-01-14 Chris Jerdonek <chris.jerdonek at gmail.com>
+
+ Reviewed by Shinichiro Hamaji.
+
+ Moved error() from cpp_style.py to checker.py.
+
+ https://bugs.webkit.org/show_bug.cgi?id=33620
+
+ * Scripts/check-webkit-style:
+ - Addressed FIXME to not set global state.
+
+ * Scripts/webkitpy/style/checker.py:
+ - Added argument validation to ProcessorOptions constructor.
+ - Added should_report_error() to ProcessorOptions class.
+ - Removed set_options().
+ - Added StyleChecker class.
+
+ * Scripts/webkitpy/style/checker_unittest.py:
+ - Added unit test class for ProcessorOptions class.
+ - Added unit test to check that parse() strips white space.
+
+ * Scripts/webkitpy/style/cpp_style.py:
+ - Removed "filter" and "output_format" methods.
+ - Removed should_print_error() and error() functions.
+ - Removed default parameter value from process_file().
+
+ * Scripts/webkitpy/style/cpp_style_unittest.py:
+ - Removed call to cpp_style._should_print_error().
+ - Removed test_filter() and test_filter_appending().
+
+ * Scripts/webkitpy/style/text_style.py:
+ - Removed default parameter value from process_file().
+
2010-01-14 Diego Gonzalez <diego.gonzalez at openbossa.org>
Reviewed by Kenneth Rohde Christiansen.
diff --git a/WebKitTools/Scripts/check-webkit-style b/WebKitTools/Scripts/check-webkit-style
index 44e306e..a5ab450 100755
--- a/WebKitTools/Scripts/check-webkit-style
+++ b/WebKitTools/Scripts/check-webkit-style
@@ -66,13 +66,11 @@ def main():
parser = checker.ArgumentParser(defaults)
(files, options) = parser.parse(sys.argv[1:])
- # FIXME: Eliminate the need to call this function.
- # Options should be passed into process_file instead.
- checker.set_options(options)
+ styleChecker = checker.StyleChecker(options)
if files:
for filename in files:
- checker.process_file(filename)
+ styleChecker.process_file(filename)
else:
cwd = os.path.abspath('.')
@@ -88,7 +86,7 @@ def main():
patch = scm.create_patch_since_local_commit(commit)
else:
patch = scm.create_patch()
- checker.process_patch(patch)
+ styleChecker.process_patch(patch)
sys.stderr.write('Total errors found: %d\n' % checker.error_count())
sys.exit(checker.error_count() > 0)
diff --git a/WebKitTools/Scripts/webkitpy/style/checker.py b/WebKitTools/Scripts/webkitpy/style/checker.py
index aa84308..a62d9a1 100644
--- a/WebKitTools/Scripts/webkitpy/style/checker.py
+++ b/WebKitTools/Scripts/webkitpy/style/checker.py
@@ -305,8 +305,8 @@ class ProcessorOptions(object):
output formats are "emacs" which emacs can parse
and "vs7" which Microsoft Visual Studio 7 can parse.
- verbosity: An integer 1-5 that restricts output to errors with a
- confidence score at or above this value.
+ verbosity: An integer between 1-5 inclusive that restricts output
+ to errors with a confidence score at or above this value.
The default is 1, which displays all errors.
filter: A CategoryFilter instance. The default is the empty filter,
@@ -318,35 +318,54 @@ class ProcessorOptions(object):
extra_flag_values: A string-string dictionary of all flag key-value
pairs that are not otherwise represented by this
class. The default is the empty dictionary.
+
"""
- def __init__(self, output_format, verbosity=1, filter=None,
+ def __init__(self, output_format="emacs", verbosity=1, filter=None,
git_commit=None, extra_flag_values=None):
if filter is None:
filter = CategoryFilter([])
if extra_flag_values is None:
extra_flag_values = {}
+ if output_format not in ("emacs", "vs7"):
+ raise ValueError('Invalid "output_format" parameter: '
+ 'value must be "emacs" or "vs7". '
+ 'Value given: "%s".' % output_format)
+
+ if (verbosity < 1) or (verbosity > 5):
+ raise ValueError('Invalid "verbosity" parameter: '
+ "value must be an integer between 1-5 inclusive. "
+ 'Value given: "%s".' % verbosity)
+
self.output_format = output_format
self.verbosity = verbosity
self.filter = filter
self.git_commit = git_commit
self.extra_flag_values = extra_flag_values
+ def should_report_error(self, category, confidence_in_error):
+ """Return whether an error should be reported.
-# FIXME: Eliminate the need for this function.
-# Options should be passed into process_file instead.
-def set_options(options):
- """Initialize global _CppStyleState instance.
+ An error should be reported if the confidence in the error
+ is at least the current verbosity level, and if the current
+ filter says that the category should be checked.
- This needs to be called before calling process_file.
+ Args:
+ category: A string that is a style category.
+ confidence_in_error: An integer between 1 and 5, inclusive, that
+ represents the application's confidence in
+ the error. A higher number signifies greater
+ confidence.
- Args:
- options: A ProcessorOptions instance.
- """
- cpp_style._set_output_format(options.output_format)
- cpp_style._set_verbose_level(options.verbosity)
- cpp_style._set_filter(options.filter)
+ """
+ if confidence_in_error < self.verbosity:
+ return False
+
+ if self.filter is None:
+ return True # All categories should be checked by default.
+
+ return self.filter.should_check(category)
# This class should not have knowledge of the flag key names.
@@ -436,6 +455,7 @@ class ArgumentParser(object):
Args:
error_message: A string that is an error message to print.
+
"""
usage = self.create_usage(self.defaults)
self.doc_print(usage)
@@ -551,7 +571,7 @@ class ArgumentParser(object):
output_format)
verbosity = int(verbosity)
- if ((verbosity < 1) or (verbosity > 5)):
+ if (verbosity < 1) or (verbosity > 5):
raise ValueError('Invalid --verbose value %s: value must '
'be between 1-5.' % verbosity)
@@ -563,48 +583,99 @@ class ArgumentParser(object):
return (filenames, options)
-def process_file(filename):
- """Checks style for the specified file.
+class StyleChecker(object):
- 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)
+ """Supports checking style in files and patches."""
+ def __init__(self, options):
+ """Create a StyleChecker instance.
-def process_patch(patch_string):
- """Does lint on a single patch.
+ Args:
+ options: A ProcessorOptions instance.
- 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)
+ """
+ self.options = options
+
+ # FIXME: Eliminate the need to set global state here.
+ cpp_style._set_verbose_level(options.verbosity)
+
+ def _handle_error(self, filename, line_number, category, confidence, message):
+ """Logs the fact we've found a lint error.
+
+ We log the error location and our confidence in the error, i.e.
+ how certain we are the error is a legitimate style regression
+ versus a misidentification or justified use.
+
+ Args:
+ filename: The name of the file containing the error.
+ line_number: The number of the line containing the error.
+ category: A string used to describe the "category" this bug
+ falls under: "whitespace", say, or "runtime".
+ Categories may have a hierarchy separated by slashes:
+ "whitespace/indent".
+ confidence: A number from 1-5 representing a confidence score
+ for the error, with 5 meaning that we are certain
+ of the problem, and 1 meaning that it could be a
+ legitimate construct.
+ message: The error message.
+
+ """
+ if not self.options.should_report_error(category, confidence):
+ return
+
+ # FIXME: Eliminate the need to reference cpp_style here.
+ cpp_style._cpp_style_state.increment_error_count()
+
+ if self.options.output_format == 'vs7':
+ sys.stderr.write('%s(%s): %s [%s] [%d]\n' % (
+ filename, line_number, message, category, confidence))
+ else:
+ sys.stderr.write('%s:%s: %s [%s] [%d]\n' % (
+ 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)
elif text_style.can_handle(filename):
- text_style.process_file(filename, error=error_for_patch)
+ text_style.process_file(filename, self._handle_error)
+
+ def process_patch(self, 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:
+ self._handle_error(filename, line_number, category, confidence, message)
+
+ if cpp_style.can_handle(filename):
+ cpp_style.process_file(filename, error_for_patch)
+ elif text_style.can_handle(filename):
+ text_style.process_file(filename, error_for_patch)
def error_count():
diff --git a/WebKitTools/Scripts/webkitpy/style/checker_unittest.py b/WebKitTools/Scripts/webkitpy/style/checker_unittest.py
index a32e642..11e0f52 100755
--- a/WebKitTools/Scripts/webkitpy/style/checker_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/style/checker_unittest.py
@@ -38,7 +38,7 @@ import unittest
import checker as style
from checker import CategoryFilter
-
+from checker import ProcessorOptions
class CategoryFilterTest(unittest.TestCase):
@@ -85,6 +85,54 @@ class CategoryFilterTest(unittest.TestCase):
self.assertTrue(filter.should_check("a"))
+class ProcessorOptionsTest(unittest.TestCase):
+
+ """Tests ProcessorOptions class."""
+
+ def test_init(self):
+ """Test __init__ constructor."""
+ # Check default parameters.
+ options = ProcessorOptions()
+ self.assertEquals(options.extra_flag_values, {})
+ self.assertEquals(options.filter, CategoryFilter([]))
+ self.assertEquals(options.git_commit, None)
+ self.assertEquals(options.output_format, "emacs")
+ self.assertEquals(options.verbosity, 1)
+
+ self.assertRaises(ValueError, ProcessorOptions, output_format="bad")
+ ProcessorOptions(output_format="emacs") # No ValueError: works
+ ProcessorOptions(output_format="vs7") # works
+ self.assertRaises(ValueError, ProcessorOptions, verbosity=0)
+ self.assertRaises(ValueError, ProcessorOptions, verbosity=6)
+ ProcessorOptions(verbosity=1) # works
+ ProcessorOptions(verbosity=5) # works
+
+ # Check attributes.
+ options = ProcessorOptions(extra_flag_values={"extra_value" : 2},
+ filter=CategoryFilter(["+"]),
+ git_commit="commit",
+ output_format="vs7",
+ verbosity=3)
+ self.assertEquals(options.extra_flag_values, {"extra_value" : 2})
+ self.assertEquals(options.filter, CategoryFilter(["+"]))
+ self.assertEquals(options.git_commit, "commit")
+ self.assertEquals(options.output_format, "vs7")
+ self.assertEquals(options.verbosity, 3)
+
+ def test_should_report_error(self):
+ """Test should_report_error()."""
+ filter = CategoryFilter(["-xyz"])
+ options = ProcessorOptions(filter=filter, verbosity=3)
+
+ # Test verbosity
+ self.assertTrue(options.should_report_error("abc", 3))
+ self.assertFalse(options.should_report_error("abc", 2))
+
+ # Test filter
+ self.assertTrue(options.should_report_error("xy", 3))
+ self.assertFalse(options.should_report_error("xyz", 3))
+
+
class DefaultArgumentsTest(unittest.TestCase):
"""Tests validity of default arguments used by check-webkit-style."""
@@ -239,6 +287,10 @@ class ArgumentParserTest(unittest.TestCase):
(files, options) = parse(['--filter=+foo,-bar'])
self.assertEquals(options.filter,
CategoryFilter(["-", "+whitespace", "+foo", "-bar"]))
+ # Spurious white space in filter rules.
+ (files, options) = parse(['--filter=+foo ,-bar'])
+ self.assertEquals(options.filter,
+ CategoryFilter(["-", "+whitespace", "+foo", "-bar"]))
# Pass extra flag values.
(files, options) = parse(['--extra'], ['extra'])
diff --git a/WebKitTools/Scripts/webkitpy/style/cpp_style.py b/WebKitTools/Scripts/webkitpy/style/cpp_style.py
index 6999a7b..469d2e0 100644
--- a/WebKitTools/Scripts/webkitpy/style/cpp_style.py
+++ b/WebKitTools/Scripts/webkitpy/style/cpp_style.py
@@ -248,17 +248,6 @@ class _CppStyleState(object):
def __init__(self):
self.verbose_level = 1 # global setting.
self.error_count = 0 # global count of reported errors
- # filter to apply when emitting error messages
- self.filter = None
-
- # output format:
- # "emacs" - format that emacs can parse (default)
- # "vs7" - format that Microsoft Visual Studio 7 can parse
- self.output_format = 'emacs'
-
- def set_output_format(self, output_format):
- """Sets the output format for errors."""
- self.output_format = output_format
def set_verbose_level(self, level):
"""Sets the module's verbosity, and returns the previous setting."""
@@ -266,15 +255,6 @@ class _CppStyleState(object):
self.verbose_level = level
return last_verbose_level
- def set_filter(self, filter):
- """Sets the error-message filter.
-
- Args:
- filter: A CategoryFilter instance.
-
- """
- self.filter = filter
-
def reset_error_count(self):
"""Sets the module's error statistic back to zero."""
self.error_count = 0
@@ -287,16 +267,6 @@ class _CppStyleState(object):
_cpp_style_state = _CppStyleState()
-def _output_format():
- """Gets the module's output format."""
- return _cpp_style_state.output_format
-
-
-def _set_output_format(output_format):
- """Sets the module's output format."""
- _cpp_style_state.set_output_format(output_format)
-
-
def _verbose_level():
"""Returns the module's verbosity setting."""
return _cpp_style_state.verbose_level
@@ -307,24 +277,6 @@ def _set_verbose_level(level):
return _cpp_style_state.set_verbose_level(level)
-def _filter():
- """Returns the module's CategoryFilter instance."""
- return _cpp_style_state.filter
-
-
-def _set_filter(filter):
- """Sets the module's error-message filter.
-
- The filter is applied when deciding whether to emit a given
- error message.
-
- Args:
- filter: A CategoryFilter instance.
-
- """
- _cpp_style_state.set_filter(filter)
-
-
def error_count():
"""Returns the global count of reported errors."""
return _cpp_style_state.error_count
@@ -477,51 +429,6 @@ class FileInfo:
return self.extension()[1:] in ('c', 'cc', 'cpp', 'cxx')
-def _should_print_error(category, confidence):
- """Returns true iff confidence >= verbose, and category passes filter."""
- # There are two ways we might decide not to print an error message:
- # the verbosity level isn't high enough, or the filters filter it out.
- if confidence < _cpp_style_state.verbose_level:
- return False
-
- filter = _filter()
-
- if filter is None:
- return True # All categories should be checked by default.
-
- return filter.should_check(category)
-
-
-def error(filename, line_number, category, confidence, message):
- """Logs the fact we've found a lint error.
-
- We log where the error was found, and also our confidence in the error,
- that is, how certain we are this is a legitimate style regression, and
- not a misidentification or a use that's sometimes justified.
-
- Args:
- filename: The name of the file containing the error.
- line_number: The number of the line containing the error.
- category: A string used to describe the "category" this bug
- falls under: "whitespace", say, or "runtime". Categories
- may have a hierarchy separated by slashes: "whitespace/indent".
- confidence: A number from 1-5 representing a confidence score for
- the error, with 5 meaning that we are certain of the problem,
- and 1 meaning that it could be a legitimate construct.
- message: The error message.
- """
- # There are two ways we might decide not to print an error message:
- # the verbosity level isn't high enough, or the filters filter it out.
- if _should_print_error(category, confidence):
- _cpp_style_state.increment_error_count()
- if _cpp_style_state.output_format == 'vs7':
- sys.stderr.write('%s(%s): %s [%s] [%d]\n' % (
- filename, line_number, message, category, confidence))
- else:
- sys.stderr.write('%s:%s: %s [%s] [%d]\n' % (
- filename, line_number, message, category, confidence))
-
-
# Matches standard C++ escape esequences per 2.13.2.3 of the C++ standard.
_RE_PATTERN_CLEANSE_LINE_ESCAPES = re.compile(
r'\\([abfnrtv?"\\\']|\d+|x[0-9a-fA-F]+)')
@@ -2969,7 +2876,7 @@ def process_file_data(filename, file_extension, lines, error):
check_for_new_line_at_eof(filename, lines, error)
-def process_file(filename, error=error):
+def process_file(filename, error):
"""Performs cpp_style on a single file.
Args:
diff --git a/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py b/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py
index 6bb5ae9..cd702d2 100644
--- a/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py
@@ -45,7 +45,6 @@ import cpp_style
# FIXME notes near the STYLE_CATEGORIES definition for a
# suggestion on how to best do this.
from checker import STYLE_CATEGORIES
-from checker import CategoryFilter
# This class works as an error collector and replaces cpp_style.Error
# function for the unit tests. We also verify each category we see
@@ -66,8 +65,7 @@ class ErrorCollector:
'Message "%s" has category "%s",'
' which is not in STYLE_CATEGORIES' % (message, category))
self._seen_style_categories[category] = 1
- if cpp_style._should_print_error(category, confidence):
- self._errors.append('%s [%s] [%d]' % (message, category, confidence))
+ self._errors.append('%s [%s] [%d]' % (message, category, confidence))
def results(self):
if len(self._errors) < 2:
@@ -1571,34 +1569,6 @@ class CppStyleTest(CppStyleTestBase):
self.assert_lint('int a = 5;\t// set a to 5',
'Tab found; better to use spaces [whitespace/tab] [1]')
- def test_filter(self):
- old_filter = cpp_style._cpp_style_state.filter
- try:
- cpp_style._cpp_style_state.set_filter(CategoryFilter(['-', '+whitespace', '-whitespace/indent']))
- self.assert_lint(
- '// Hello there ',
- 'Line ends in whitespace. Consider deleting these extra spaces.'
- ' [whitespace/end_of_line] [4]')
- self.assert_lint('int a = (int)1.0;', '')
- self.assert_lint(' weird opening space', '')
- finally:
- cpp_style._cpp_style_state.filter = old_filter
-
- def test_filter_appending(self):
- old_filter = cpp_style._cpp_style_state.filter
- try:
- # Reset filters
- cpp_style._cpp_style_state.set_filter(CategoryFilter(['-whitespace']))
- self.assert_lint('// Hello there ', '')
- cpp_style._cpp_style_state.set_filter(CategoryFilter(['-whitespace', '+whitespace/end_of_line']))
- self.assert_lint(
- '// Hello there ',
- 'Line ends in whitespace. Consider deleting these extra spaces.'
- ' [whitespace/end_of_line] [4]')
- self.assert_lint(' weird opening space', '')
- finally:
- cpp_style._cpp_style_state.filter = old_filter
-
def test_unnamed_namespaces_in_headers(self):
self.assert_language_rules_check(
'foo.h', 'namespace {',
diff --git a/WebKitTools/Scripts/webkitpy/style/text_style.py b/WebKitTools/Scripts/webkitpy/style/text_style.py
index 9496558..385bbaa 100644
--- a/WebKitTools/Scripts/webkitpy/style/text_style.py
+++ b/WebKitTools/Scripts/webkitpy/style/text_style.py
@@ -51,7 +51,7 @@ def process_file_data(filename, lines, error):
error(filename, line_number, 'whitespace/tab', 5, 'Line contains tab character.')
-def process_file(filename, error=cpp_style.error):
+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)
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list