[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc
levin at chromium.org
levin at chromium.org
Wed Dec 22 15:50:44 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit 0d8b6900729d782f5af90e3224adb7565ce8ca74
Author: levin at chromium.org <levin at chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Sun Nov 14 19:59:42 2010 +0000
2010-11-14 David Levin <levin at chromium.org>
Reviewed by Shinichiro Hamaji.
check-webkit-style function detection and the line count style checks should be separate.
https://bugs.webkit.org/show_bug.cgi?id=49512
* Scripts/webkitpy/style/checkers/cpp.py: Do the separation.
* Scripts/webkitpy/style/checkers/cpp_unittest.py: Adjust the test to
call the detection function and fix line counts in two places now that
the code really only counts the lines in the body of the function.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@71986 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index e619435..2035a29 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,15 @@
+2010-11-14 David Levin <levin at chromium.org>
+
+ Reviewed by Shinichiro Hamaji.
+
+ check-webkit-style function detection and the line count style checks should be separate.
+ https://bugs.webkit.org/show_bug.cgi?id=49512
+
+ * Scripts/webkitpy/style/checkers/cpp.py: Do the separation.
+ * Scripts/webkitpy/style/checkers/cpp_unittest.py: Adjust the test to
+ call the detection function and fix line counts in two places now that
+ the code really only counts the lines in the body of the function.
+
2010-11-14 Andreas Kling <kling at webkit.org>
Reviewed by Antonio Gomes.
diff --git a/WebKitTools/Scripts/webkitpy/style/checkers/cpp.py b/WebKitTools/Scripts/webkitpy/style/checkers/cpp.py
index 04573bd..d504e4a 100644
--- a/WebKitTools/Scripts/webkitpy/style/checkers/cpp.py
+++ b/WebKitTools/Scripts/webkitpy/style/checkers/cpp.py
@@ -316,9 +316,12 @@ class _FunctionState(object):
self.current_function = ''
self.in_a_function = False
self.lines_in_function = 0
- self.ending_line_number = 0
+ # Make sure these will not be mistaken for real lines (even when a
+ # small amount is added to them).
+ self.body_start_line_number = -1000
+ self.ending_line_number = -1000
- def begin(self, function_name, ending_line_number):
+ def begin(self, function_name, body_start_line_number, ending_line_number):
"""Start analyzing function body.
Args:
@@ -328,11 +331,12 @@ class _FunctionState(object):
self.in_a_function = True
self.lines_in_function = 0
self.current_function = function_name
+ self.body_start_line_number = body_start_line_number
self.ending_line_number = ending_line_number
- def count(self):
+ def count(self, line_number):
"""Count line in current function body."""
- if self.in_a_function:
+ if self.in_a_function and line_number >= self.body_start_line_number:
self.lines_in_function += 1
def check(self, error, line_number):
@@ -1146,16 +1150,85 @@ def is_blank_line(line):
return not line or line.isspace()
-def check_for_function_lengths(clean_lines, line_number, function_state, error):
- """Reports for long function bodies.
-
- For an overview why this is done, see:
- http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Write_Short_Functions
+def detect_functions(clean_lines, line_number, function_state, error):
+ """Finds where functions start and end.
Uses a simplistic algorithm assuming other style guidelines
(especially spacing) are followed.
Trivial bodies are unchecked, so constructors with huge initializer lists
may be missed.
+
+ Args:
+ clean_lines: A CleansedLines instance containing the file.
+ line_number: The number of the line to check.
+ function_state: Current function name and lines in body so far.
+ error: The function to call with any errors found.
+ """
+ # Are we now past the end of a function?
+ if function_state.ending_line_number + 1 == line_number:
+ function_state.end()
+
+ # If we're in a function, don't try to detect a new one.
+ if function_state.in_a_function:
+ return
+
+ lines = clean_lines.lines
+ line = lines[line_number]
+ raw = clean_lines.raw_lines
+ raw_line = raw[line_number]
+
+ regexp = r'\s*(\w(\w|::|\*|\&|\s|<|>|,|~)*)\(' # decls * & space::name( ...
+ match_result = match(regexp, line)
+ if not match_result:
+ return
+
+ # If the name is all caps and underscores, figure it's a macro and
+ # ignore it, unless it's TEST or TEST_F.
+ function_name = match_result.group(1).split()[-1]
+ if function_name != 'TEST' and function_name != 'TEST_F' and match(r'[A-Z_]+$', function_name):
+ return
+
+ joined_line = ''
+ for start_line_number in xrange(line_number, clean_lines.num_lines()):
+ start_line = lines[start_line_number]
+ joined_line += ' ' + start_line.lstrip()
+ if search(r'(;|})', start_line): # Declarations and trivial functions
+ return # ... ignore
+
+ if search(r'{', start_line):
+ # Replace template constructs with _ so that no spaces remain in the function name,
+ # while keeping the column numbers of other characters the same as "line".
+ line_with_no_templates = iteratively_replace_matches_with_char(r'<[^<>]*>', '_', line)
+ match_function = search(r'((\w|:|<|>|,|~)*)\(', line_with_no_templates)
+ if not match_function:
+ return # The '(' must have been inside of a template.
+
+ # Use the column numbers from the modified line to find the
+ # function name in the original line.
+ function = line[match_function.start(1):match_function.end(1)]
+
+ if match(r'TEST', function): # Handle TEST... macros
+ parameter_regexp = search(r'(\(.*\))', joined_line)
+ if parameter_regexp: # Ignore bad syntax
+ function += parameter_regexp.group(1)
+ else:
+ function += '()'
+ open_brace_index = start_line.find('{')
+ ending_line_number = close_expression(clean_lines, start_line_number, open_brace_index)[1]
+ function_state.begin(function, start_line_number + 1, ending_line_number)
+ return
+
+ # No body for the function (or evidence of a non-function) was found.
+ error(line_number, 'readability/fn_size', 5,
+ 'Lint failed to find start of function body.')
+
+
+def check_for_function_lengths(clean_lines, line_number, function_state, error):
+ """Reports for issues related to functions.
+
+ For an overview why this is done, see:
+ http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Write_Short_Functions
+
Blank/comment lines are not counted so as to avoid encouraging the removal
of vertical space and commments just to get through a lint check.
NOLINT *on the last line of a function* disables this check.
@@ -1170,58 +1243,12 @@ def check_for_function_lengths(clean_lines, line_number, function_state, error):
line = lines[line_number]
raw = clean_lines.raw_lines
raw_line = raw[line_number]
- joined_line = ''
- starting_func = False
- regexp = r'\s*(\w(\w|::|\*|\&|\s|<|>|,|~)*)\(' # decls * & space::name( ...
- match_result = None
- if not function_state.in_a_function:
- match_result = match(regexp, line)
- if match_result:
- # If the name is all caps and underscores, figure it's a macro and
- # ignore it, unless it's TEST or TEST_F.
- function_name = match_result.group(1).split()[-1]
- if function_name == 'TEST' or function_name == 'TEST_F' or (not match(r'[A-Z_]+$', function_name)):
- starting_func = True
-
- if starting_func:
- for start_line_number in xrange(line_number, clean_lines.num_lines()):
- start_line = lines[start_line_number]
- joined_line += ' ' + start_line.lstrip()
- if search(r'(;|})', start_line): # Declarations and trivial functions
- break # ... ignore
- if search(r'{', start_line):
- # Replace template constructs with _ so that no spaces remain in the function name,
- # while keeping the column numbers of other characters the same as "line".
- line_with_no_templates = iteratively_replace_matches_with_char(r'<[^<>]*>', '_', line)
- match_function = search(r'((\w|:|<|>|,|~)*)\(', line_with_no_templates)
- if not match_function:
- return # The '(' must have been inside of a template.
-
- # Use the column numbers from the modified line to find the
- # function name in the original line.
- function = line[match_function.start(1):match_function.end(1)]
-
- if match(r'TEST', function): # Handle TEST... macros
- parameter_regexp = search(r'(\(.*\))', joined_line)
- if parameter_regexp: # Ignore bad syntax
- function += parameter_regexp.group(1)
- else:
- function += '()'
- open_brace_index = start_line.find('{')
- ending_line_number = close_expression(clean_lines, start_line_number, open_brace_index)[1]
- function_state.begin(function, ending_line_number)
- break
- else:
- # No body for the function (or evidence of a non-function) was found.
- error(line_number, 'readability/fn_size', 5,
- 'Lint failed to find start of function body.')
- elif function_state.in_a_function and function_state.ending_line_number == line_number: # function end
+ if function_state.ending_line_number == line_number: # last line
if not search(r'\bNOLINT\b', raw_line):
function_state.check(error, line_number)
- function_state.end()
elif not match(r'^\s*$', line):
- function_state.count() # Count non-blank/non-comment lines.
+ function_state.count(line_number) # Count non-blank/non-comment lines.
def check_spacing(file_extension, clean_lines, line_number, error):
@@ -2902,6 +2929,7 @@ def process_line(filename, file_extension,
"""
raw_lines = clean_lines.raw_lines
+ detect_functions(clean_lines, line, function_state, error)
check_for_function_lengths(clean_lines, line, function_state, error)
if search(r'\bNOLINT\b', raw_lines[line]): # ignore nolint lines
return
diff --git a/WebKitTools/Scripts/webkitpy/style/checkers/cpp_unittest.py b/WebKitTools/Scripts/webkitpy/style/checkers/cpp_unittest.py
index 9ba04a0..9031ed8 100644
--- a/WebKitTools/Scripts/webkitpy/style/checkers/cpp_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/style/checkers/cpp_unittest.py
@@ -205,6 +205,8 @@ class CppStyleTestBase(unittest.TestCase):
cpp_style.remove_multi_line_comments(lines, error_collector)
lines = cpp_style.CleansedLines(lines)
for i in xrange(lines.num_lines()):
+ cpp_style.detect_functions(lines, i,
+ function_state, error_collector)
cpp_style.check_for_function_lengths(lines, i,
function_state, error_collector)
return error_collector.results()
@@ -2444,7 +2446,7 @@ class CheckForFunctionLengthsTest(CppStyleTestBase):
'test_indent() has %d non-comment lines '
'(error triggered by exceeding %d lines).'
' [readability/fn_size] [%d]')
- % (error_lines + 1, trigger_level, error_level))
+ % (error_lines, trigger_level, error_level))
def test_function_length_check_definition_severity1_plus_blanks(self):
error_level = 1
@@ -2498,7 +2500,7 @@ class CheckForFunctionLengthsTest(CppStyleTestBase):
'FixGoogleUpdate_AllValues_MachineApp) has %d non-comment lines '
'(error triggered by exceeding %d lines).'
' [readability/fn_size] [%d]')
- % (error_lines+1, trigger_level, error_level))
+ % (error_lines, trigger_level, error_level))
def test_function_length_check_definition_severity1_for_bad_test_doesnt_break(self):
error_level = 1
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list