[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:11:37 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit 2a1aaae8114988d856006378c008d33b6faaec3a
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Mon Jan 18 02:22:42 2010 +0000
2010-01-17 Chris Jerdonek <cjerdonek at webkit.org>
Reviewed by Shinichiro Hamaji.
Finished eliminating _cpp_style_state global state variable from
check-webkit-style code and eliminating _CppStyleState class.
https://bugs.webkit.org/show_bug.cgi?id=33764
* Scripts/webkitpy/style/checker.py:
- Minor updates caused by changes to cpp_style.py.
* Scripts/webkitpy/style/cpp_style.py:
- Removed _CppStyleState class.
- Removed verbose_level functions.
- Added verbosity as a parameter to _FunctionState constructor.
- Added verbosity as a parameter to process_file().
- Added verbosity as a parameter to process_file_data().
* Scripts/webkitpy/style/cpp_style_unittest.py:
- Added helper functions to set verbosity while running tests.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@53382 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 230d79b..6c3ff69 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,25 @@
+2010-01-17 Chris Jerdonek <cjerdonek at webkit.org>
+
+ Reviewed by Shinichiro Hamaji.
+
+ Finished eliminating _cpp_style_state global state variable from
+ check-webkit-style code and eliminating _CppStyleState class.
+
+ https://bugs.webkit.org/show_bug.cgi?id=33764
+
+ * Scripts/webkitpy/style/checker.py:
+ - Minor updates caused by changes to cpp_style.py.
+
+ * Scripts/webkitpy/style/cpp_style.py:
+ - Removed _CppStyleState class.
+ - Removed verbose_level functions.
+ - Added verbosity as a parameter to _FunctionState constructor.
+ - Added verbosity as a parameter to process_file().
+ - Added verbosity as a parameter to process_file_data().
+
+ * Scripts/webkitpy/style/cpp_style_unittest.py:
+ - Added helper functions to set verbosity while running tests.
+
2010-01-17 Adam Barth <abarth at webkit.org>
Reviewed by Shinichiro Hamaji.
diff --git a/WebKitTools/Scripts/webkitpy/style/checker.py b/WebKitTools/Scripts/webkitpy/style/checker.py
index c46a545..c3f4db0 100644
--- a/WebKitTools/Scripts/webkitpy/style/checker.py
+++ b/WebKitTools/Scripts/webkitpy/style/checker.py
@@ -645,9 +645,6 @@ class StyleChecker(object):
self.options = options
self.error_count = 0
- # 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):
"""Handle the occurrence of a style error while checking.
@@ -688,7 +685,7 @@ class StyleChecker(object):
"""
if cpp_style.can_handle(filename) or filename == '-':
- cpp_style.process_file(filename, self._handle_error)
+ 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)
@@ -720,7 +717,8 @@ class StyleChecker(object):
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)
+ 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/cpp_style.py b/WebKitTools/Scripts/webkitpy/style/cpp_style.py
index 77c078d..c4d3445 100644
--- a/WebKitTools/Scripts/webkitpy/style/cpp_style.py
+++ b/WebKitTools/Scripts/webkitpy/style/cpp_style.py
@@ -242,39 +242,19 @@ class _IncludeState(dict):
return error_message
-class _CppStyleState(object):
- """Maintains module-wide state.."""
-
- def __init__(self):
- self.verbose_level = 1 # global setting.
-
- def set_verbose_level(self, level):
- """Sets the module's verbosity, and returns the previous setting."""
- last_verbose_level = self.verbose_level
- self.verbose_level = level
- return last_verbose_level
-
-
-_cpp_style_state = _CppStyleState()
-
-
-def _verbose_level():
- """Returns the module's verbosity setting."""
- return _cpp_style_state.verbose_level
-
-
-def _set_verbose_level(level):
- """Sets the module's verbosity, and returns the previous setting."""
- return _cpp_style_state.set_verbose_level(level)
+class _FunctionState(object):
+ """Tracks current function name and the number of lines in its body.
+ Attributes:
+ verbosity: The verbosity level to use while checking style.
-class _FunctionState(object):
- """Tracks current function name and the number of lines in its body."""
+ """
_NORMAL_TRIGGER = 250 # for --v=0, 500 for --v=1, etc.
_TEST_TRIGGER = 400 # about 50% more than _NORMAL_TRIGGER.
- def __init__(self):
+ def __init__(self, verbosity):
+ self.verbosity = verbosity
self.in_a_function = False
self.lines_in_function = 0
self.current_function = ''
@@ -306,7 +286,7 @@ class _FunctionState(object):
base_trigger = self._TEST_TRIGGER
else:
base_trigger = self._NORMAL_TRIGGER
- trigger = base_trigger * 2 ** _verbose_level()
+ trigger = base_trigger * 2 ** self.verbosity
if self.lines_in_function > trigger:
error_level = int(math.log(self.lines_in_function / base_trigger, 2))
@@ -2831,7 +2811,7 @@ def process_line(filename, file_extension,
check_invalid_increment(filename, clean_lines, line, error)
-def process_file_data(filename, file_extension, lines, error):
+def process_file_data(filename, file_extension, lines, error, verbosity):
"""Performs lint checks and reports any errors to the given error function.
Args:
@@ -2845,7 +2825,7 @@ def process_file_data(filename, file_extension, lines, error):
['// marker so line numbers end in a known way'])
include_state = _IncludeState()
- function_state = _FunctionState()
+ function_state = _FunctionState(verbosity)
class_state = _ClassState()
file_state = _FileState()
@@ -2870,12 +2850,14 @@ def process_file_data(filename, file_extension, lines, error):
check_for_new_line_at_eof(filename, lines, error)
-def process_file(filename, 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
@@ -2916,7 +2898,7 @@ def process_file(filename, error):
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)
+ 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.
diff --git a/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py b/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py
index a20048e..2dfe451 100644
--- a/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py
@@ -109,7 +109,23 @@ class MockIo:
class CppStyleTestBase(unittest.TestCase):
- """Provides some useful helper functions for cpp_style tests."""
+ """Provides some useful helper functions for cpp_style tests.
+
+ Attributes:
+ verbosity: An integer that is the current verbosity level for
+ the tests.
+
+ """
+
+ # FIXME: Refactor the unit tests so the verbosity level is passed
+ # explicitly, just like it is in the real code.
+ verbosity = 1;
+
+ # Helper function to avoid needing to explicitly pass verbosity
+ # in all the unit test calls to cpp_style.process_file_data().
+ def process_file_data(self, filename, file_extension, lines, error):
+ """Call cpp_style.process_file_data() with the current verbosity."""
+ return cpp_style.process_file_data(filename, file_extension, lines, error, self.verbosity)
# Perform lint on single line of input and return the error message.
def perform_single_line_lint(self, code, file_name):
@@ -118,7 +134,7 @@ class CppStyleTestBase(unittest.TestCase):
cpp_style.remove_multi_line_comments(file_name, lines, error_collector)
clean_lines = cpp_style.CleansedLines(lines)
include_state = cpp_style._IncludeState()
- function_state = cpp_style._FunctionState()
+ function_state = cpp_style._FunctionState(self.verbosity)
ext = file_name[file_name.rfind('.') + 1:]
class_state = cpp_style._ClassState()
file_state = cpp_style._FileState()
@@ -178,7 +194,7 @@ class CppStyleTestBase(unittest.TestCase):
"""
file_name = 'foo.cpp'
error_collector = ErrorCollector(self.assert_)
- function_state = cpp_style._FunctionState()
+ function_state = cpp_style._FunctionState(self.verbosity)
lines = code.split('\n')
cpp_style.remove_multi_line_comments(file_name, lines, error_collector)
lines = cpp_style.CleansedLines(lines)
@@ -236,7 +252,7 @@ class CppStyleTestBase(unittest.TestCase):
def assert_blank_lines_check(self, lines, start_errors, end_errors):
error_collector = ErrorCollector(self.assert_)
- cpp_style.process_file_data('foo.cpp', 'cpp', lines, error_collector)
+ self.process_file_data('foo.cpp', 'cpp', lines, error_collector)
self.assertEquals(
start_errors,
error_collector.results().count(
@@ -694,10 +710,10 @@ class CppStyleTest(CppStyleTestBase):
file_path = 'mydir/foo.cpp'
error_collector = ErrorCollector(self.assert_)
- cpp_style.process_file_data(file_path, 'cpp',
- ['const char* str = "This is a\\',
- ' multiline string.";'],
- error_collector)
+ self.process_file_data(file_path, 'cpp',
+ ['const char* str = "This is a\\',
+ ' multiline string.";'],
+ error_collector)
self.assertEquals(
2, # One per line.
error_collector.result_list().count(multiline_string_error_message))
@@ -1408,8 +1424,8 @@ class CppStyleTest(CppStyleTestBase):
def test_newline_at_eof(self):
def do_test(self, data, is_missing_eof):
error_collector = ErrorCollector(self.assert_)
- cpp_style.process_file_data('foo.cpp', 'cpp', data.split('\n'),
- error_collector)
+ self.process_file_data('foo.cpp', 'cpp', data.split('\n'),
+ error_collector)
# The warning appears only once.
self.assertEquals(
int(is_missing_eof),
@@ -1423,10 +1439,9 @@ class CppStyleTest(CppStyleTestBase):
def test_invalid_utf8(self):
def do_test(self, raw_bytes, has_invalid_utf8):
error_collector = ErrorCollector(self.assert_)
- cpp_style.process_file_data(
- 'foo.cpp', 'cpp',
- unicode(raw_bytes, 'utf8', 'replace').split('\n'),
- error_collector)
+ self.process_file_data('foo.cpp', 'cpp',
+ unicode(raw_bytes, 'utf8', 'replace').split('\n'),
+ error_collector)
# The warning appears only once.
self.assertEquals(
int(has_invalid_utf8),
@@ -1459,40 +1474,40 @@ class CppStyleTest(CppStyleTestBase):
def test_allow_blank_line_before_closing_namespace(self):
error_collector = ErrorCollector(self.assert_)
- cpp_style.process_file_data('foo.cpp', 'cpp',
- ['namespace {', '', '} // namespace'],
- error_collector)
+ self.process_file_data('foo.cpp', 'cpp',
+ ['namespace {', '', '} // namespace'],
+ error_collector)
self.assertEquals(0, error_collector.results().count(
'Blank line at the end of a code block. Is this needed?'
' [whitespace/blank_line] [3]'))
def test_allow_blank_line_before_if_else_chain(self):
error_collector = ErrorCollector(self.assert_)
- cpp_style.process_file_data('foo.cpp', 'cpp',
- ['if (hoge) {',
- '', # No warning
- '} else if (piyo) {',
- '', # No warning
- '} else if (piyopiyo) {',
- ' hoge = true;', # No warning
- '} else {',
- '', # Warning on this line
- '}'],
- error_collector)
+ self.process_file_data('foo.cpp', 'cpp',
+ ['if (hoge) {',
+ '', # No warning
+ '} else if (piyo) {',
+ '', # No warning
+ '} else if (piyopiyo) {',
+ ' hoge = true;', # No warning
+ '} else {',
+ '', # Warning on this line
+ '}'],
+ error_collector)
self.assertEquals(1, error_collector.results().count(
'Blank line at the end of a code block. Is this needed?'
' [whitespace/blank_line] [3]'))
def test_else_on_same_line_as_closing_braces(self):
error_collector = ErrorCollector(self.assert_)
- cpp_style.process_file_data('foo.cpp', 'cpp',
- ['if (hoge) {',
- '',
- '}',
- ' else {' # Warning on this line
- '',
- '}'],
- error_collector)
+ self.process_file_data('foo.cpp', 'cpp',
+ ['if (hoge) {',
+ '',
+ '}',
+ ' else {' # Warning on this line
+ '',
+ '}'],
+ error_collector)
self.assertEquals(1, error_collector.results().count(
'An else should appear on the same line as the preceding }'
' [whitespace/newline] [4]'))
@@ -1636,7 +1651,7 @@ class CppStyleTest(CppStyleTestBase):
# doesn't allow us to test the suggested header guard, but it does let us
# test all the other header tests.
error_collector = ErrorCollector(self.assert_)
- cpp_style.process_file_data(file_path, 'h', [], error_collector)
+ self.process_file_data(file_path, 'h', [], error_collector)
expected_guard = ''
matcher = re.compile(
'No \#ifndef header guard found\, suggested CPP variable is\: ([A-Za-z_0-9]+) ')
@@ -1651,8 +1666,8 @@ class CppStyleTest(CppStyleTestBase):
# Wrong guard
error_collector = ErrorCollector(self.assert_)
- cpp_style.process_file_data(file_path, 'h',
- ['#ifndef FOO_H', '#define FOO_H'], error_collector)
+ self.process_file_data(file_path, 'h',
+ ['#ifndef FOO_H', '#define FOO_H'], error_collector)
self.assertEquals(
1,
error_collector.result_list().count(
@@ -1662,8 +1677,8 @@ class CppStyleTest(CppStyleTestBase):
# No define
error_collector = ErrorCollector(self.assert_)
- cpp_style.process_file_data(file_path, 'h',
- ['#ifndef %s' % expected_guard], error_collector)
+ self.process_file_data(file_path, 'h',
+ ['#ifndef %s' % expected_guard], error_collector)
self.assertEquals(
1,
error_collector.result_list().count(
@@ -1673,10 +1688,10 @@ class CppStyleTest(CppStyleTestBase):
# Mismatched define
error_collector = ErrorCollector(self.assert_)
- cpp_style.process_file_data(file_path, 'h',
- ['#ifndef %s' % expected_guard,
- '#define FOO_H'],
- error_collector)
+ self.process_file_data(file_path, 'h',
+ ['#ifndef %s' % expected_guard,
+ '#define FOO_H'],
+ error_collector)
self.assertEquals(
1,
error_collector.result_list().count(
@@ -1686,22 +1701,22 @@ class CppStyleTest(CppStyleTestBase):
# No header guard errors
error_collector = ErrorCollector(self.assert_)
- cpp_style.process_file_data(file_path, 'h',
- ['#ifndef %s' % expected_guard,
- '#define %s' % expected_guard,
- '#endif // %s' % expected_guard],
- error_collector)
+ self.process_file_data(file_path, 'h',
+ ['#ifndef %s' % expected_guard,
+ '#define %s' % expected_guard,
+ '#endif // %s' % expected_guard],
+ error_collector)
for line in error_collector.result_list():
if line.find('build/header_guard') != -1:
self.fail('Unexpected error: %s' % line)
# Completely incorrect header guard
error_collector = ErrorCollector(self.assert_)
- cpp_style.process_file_data(file_path, 'h',
- ['#ifndef FOO',
- '#define FOO',
- '#endif // FOO'],
- error_collector)
+ self.process_file_data(file_path, 'h',
+ ['#ifndef FOO',
+ '#define FOO',
+ '#endif // FOO'],
+ error_collector)
self.assertEquals(
1,
error_collector.result_list().count(
@@ -1843,13 +1858,13 @@ class CppStyleTest(CppStyleTestBase):
# There should be a copyright message in the first 10 lines
error_collector = ErrorCollector(self.assert_)
- cpp_style.process_file_data(file_path, 'cpp', [], error_collector)
+ self.process_file_data(file_path, 'cpp', [], error_collector)
self.assertEquals(
1,
error_collector.result_list().count(legal_copyright_message))
error_collector = ErrorCollector(self.assert_)
- cpp_style.process_file_data(
+ self.process_file_data(
file_path, 'cpp',
['' for unused_i in range(10)] + [copyright_line],
error_collector)
@@ -1859,13 +1874,13 @@ class CppStyleTest(CppStyleTestBase):
# Test that warning isn't issued if Copyright line appears early enough.
error_collector = ErrorCollector(self.assert_)
- cpp_style.process_file_data(file_path, 'cpp', [copyright_line], error_collector)
+ self.process_file_data(file_path, 'cpp', [copyright_line], error_collector)
for message in error_collector.result_list():
if message.find('legal/copyright') != -1:
self.fail('Unexpected error: %s' % message)
error_collector = ErrorCollector(self.assert_)
- cpp_style.process_file_data(
+ self.process_file_data(
file_path, 'cpp',
['' for unused_i in range(9)] + [copyright_line],
error_collector)
@@ -2200,6 +2215,13 @@ class CheckForFunctionLengthsTest(CppStyleTestBase):
cpp_style._FunctionState._NORMAL_TRIGGER = self.old_normal_trigger
cpp_style._FunctionState._TEST_TRIGGER = self.old_test_trigger
+ # FIXME: Eliminate the need for this function.
+ def set_verbosity(self, verbosity):
+ """Set new test verbosity and return old test verbosity."""
+ old_verbosity = self.verbosity
+ self.verbosity = verbosity
+ return old_verbosity
+
def assert_function_lengths_check(self, code, expected_message):
"""Check warnings for long function bodies are as expected.
@@ -2239,7 +2261,7 @@ class CheckForFunctionLengthsTest(CppStyleTestBase):
lines: Number of lines to generate.
error_level: --v setting for cpp_style.
"""
- trigger_level = self.trigger_lines(cpp_style._verbose_level())
+ trigger_level = self.trigger_lines(self.verbosity)
self.assert_function_lengths_check(
'void test(int x)' + self.function_body(lines),
('Small and focused functions are preferred: '
@@ -2322,29 +2344,29 @@ class CheckForFunctionLengthsTest(CppStyleTestBase):
'')
def test_function_length_check_definition_below_severity0(self):
- old_verbosity = cpp_style._set_verbose_level(0)
+ old_verbosity = self.set_verbosity(0)
self.assert_function_length_check_definition_ok(self.trigger_lines(0) - 1)
- cpp_style._set_verbose_level(old_verbosity)
+ self.set_verbosity(old_verbosity)
def test_function_length_check_definition_at_severity0(self):
- old_verbosity = cpp_style._set_verbose_level(0)
+ old_verbosity = self.set_verbosity(0)
self.assert_function_length_check_definition_ok(self.trigger_lines(0))
- cpp_style._set_verbose_level(old_verbosity)
+ self.set_verbosity(old_verbosity)
def test_function_length_check_definition_above_severity0(self):
- old_verbosity = cpp_style._set_verbose_level(0)
+ old_verbosity = self.set_verbosity(0)
self.assert_function_length_check_above_error_level(0)
- cpp_style._set_verbose_level(old_verbosity)
+ self.set_verbosity(old_verbosity)
def test_function_length_check_definition_below_severity1v0(self):
- old_verbosity = cpp_style._set_verbose_level(0)
+ old_verbosity = self.set_verbosity(0)
self.assert_function_length_check_below_error_level(1)
- cpp_style._set_verbose_level(old_verbosity)
+ self.set_verbosity(old_verbosity)
def test_function_length_check_definition_at_severity1v0(self):
- old_verbosity = cpp_style._set_verbose_level(0)
+ old_verbosity = self.set_verbosity(0)
self.assert_function_length_check_at_error_level(1)
- cpp_style._set_verbose_level(old_verbosity)
+ self.set_verbosity(old_verbosity)
def test_function_length_check_definition_below_severity1(self):
self.assert_function_length_check_definition_ok(self.trigger_lines(1) - 1)
@@ -2358,7 +2380,7 @@ class CheckForFunctionLengthsTest(CppStyleTestBase):
def test_function_length_check_definition_severity1_plus_blanks(self):
error_level = 1
error_lines = self.trigger_lines(error_level) + 1
- trigger_level = self.trigger_lines(cpp_style._verbose_level())
+ trigger_level = self.trigger_lines(self.verbosity)
self.assert_function_lengths_check(
'void test_blanks(int x)' + self.function_body(error_lines),
('Small and focused functions are preferred: '
@@ -2370,7 +2392,7 @@ class CheckForFunctionLengthsTest(CppStyleTestBase):
def test_function_length_check_complex_definition_severity1(self):
error_level = 1
error_lines = self.trigger_lines(error_level) + 1
- trigger_level = self.trigger_lines(cpp_style._verbose_level())
+ trigger_level = self.trigger_lines(self.verbosity)
self.assert_function_lengths_check(
('my_namespace::my_other_namespace::MyVeryLongTypeName*\n'
'my_namespace::my_other_namespace::MyFunction(int arg1, char* arg2)'
@@ -2385,7 +2407,7 @@ class CheckForFunctionLengthsTest(CppStyleTestBase):
def test_function_length_check_definition_severity1_for_test(self):
error_level = 1
error_lines = self.trigger_test_lines(error_level) + 1
- trigger_level = self.trigger_test_lines(cpp_style._verbose_level())
+ trigger_level = self.trigger_test_lines(self.verbosity)
self.assert_function_lengths_check(
'TEST_F(Test, Mutator)' + self.function_body(error_lines),
('Small and focused functions are preferred: '
@@ -2397,7 +2419,7 @@ class CheckForFunctionLengthsTest(CppStyleTestBase):
def test_function_length_check_definition_severity1_for_split_line_test(self):
error_level = 1
error_lines = self.trigger_test_lines(error_level) + 1
- trigger_level = self.trigger_test_lines(cpp_style._verbose_level())
+ trigger_level = self.trigger_test_lines(self.verbosity)
self.assert_function_lengths_check(
('TEST_F(GoogleUpdateRecoveryRegistryProtectedTest,\n'
' FixGoogleUpdate_AllValues_MachineApp)' # note: 4 spaces
@@ -2412,7 +2434,7 @@ class CheckForFunctionLengthsTest(CppStyleTestBase):
def test_function_length_check_definition_severity1_for_bad_test_doesnt_break(self):
error_level = 1
error_lines = self.trigger_test_lines(error_level) + 1
- trigger_level = self.trigger_test_lines(cpp_style._verbose_level())
+ trigger_level = self.trigger_test_lines(self.verbosity)
self.assert_function_lengths_check(
('TEST_F('
+ self.function_body(error_lines)),
@@ -2425,7 +2447,7 @@ class CheckForFunctionLengthsTest(CppStyleTestBase):
def test_function_length_check_definition_severity1_with_embedded_no_lints(self):
error_level = 1
error_lines = self.trigger_lines(error_level) + 1
- trigger_level = self.trigger_lines(cpp_style._verbose_level())
+ trigger_level = self.trigger_lines(self.verbosity)
self.assert_function_lengths_check(
'void test(int x)' + self.function_body_with_no_lints(error_lines),
('Small and focused functions are preferred: '
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list