[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373
ukai at chromium.org
ukai at chromium.org
Thu Apr 8 00:42:20 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit ba09fa444b399bd38e294431094bd2db0887d54a
Author: ukai at chromium.org <ukai at chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Fri Dec 18 17:47:21 2009 +0000
2009-12-18 Fumitoshi Ukai <ukai at chromium.org>
Reviewed by David Levin.
Check one space before end of line comments.
https://bugs.webkit.org/show_bug.cgi?id=32597
Fix to check one space before end of line comments in whitespace and build/header_guard.
Also fix build/header_guard to use WebKit header guard defines.
* Scripts/modules/cpp_style.py:
* Scripts/modules/cpp_style_unittest.py:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@52318 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 138f0ad..92f66ee 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,16 @@
+2009-12-18 Fumitoshi Ukai <ukai at chromium.org>
+
+ Reviewed by David Levin.
+
+ Check one space before end of line comments.
+ https://bugs.webkit.org/show_bug.cgi?id=32597
+
+ Fix to check one space before end of line comments in whitespace and build/header_guard.
+ Also fix build/header_guard to use WebKit header guard defines.
+
+ * Scripts/modules/cpp_style.py:
+ * Scripts/modules/cpp_style_unittest.py:
+
2009-12-17 Sam Weinig <sam at webkit.org>
Reviewed by Mark Rowe.
diff --git a/WebKitTools/Scripts/modules/cpp_style.py b/WebKitTools/Scripts/modules/cpp_style.py
index 3c67c25..4fba777 100644
--- a/WebKitTools/Scripts/modules/cpp_style.py
+++ b/WebKitTools/Scripts/modules/cpp_style.py
@@ -69,7 +69,6 @@ _DEFAULT_OUTPUT_FORMAT = 'emacs'
# the filter rules as specified by a --filter flag.
_WEBKIT_FILTER_RULES = [
'-build/endif_comment',
- '-build/header_guard',
'-build/include_what_you_use', # <string> for std::string
'-build/storage_class', # const static
'-legal/copyright',
@@ -87,7 +86,6 @@ _WEBKIT_FILTER_RULES = [
'-runtime/threadsafe_fn',
'-runtime/rtti',
'-whitespace/blank_line',
- '-whitespace/comments',
'-whitespace/end_of_line',
'-whitespace/labels',
]
@@ -874,8 +872,7 @@ def get_header_guard_cpp_variable(filename):
"""
- fileinfo = FileInfo(filename)
- return sub(r'[-./\s]', '_', fileinfo.repository_name()).upper() + '_'
+ return sub(r'[-.\s]', '_', os.path.basename(filename))
def check_for_header_guard(filename, lines, error):
@@ -918,23 +915,14 @@ def check_for_header_guard(filename, lines, error):
cppvar)
return
- # The guard should be PATH_FILE_H_, but we also allow PATH_FILE_H__
- # for backward compatibility.
+ # The guard should be File_h.
if ifndef != cppvar:
- error_level = 0
- if ifndef != cppvar + '_':
- error_level = 5
-
- error(filename, ifndef_line_number, 'build/header_guard', error_level,
+ error(filename, ifndef_line_number, 'build/header_guard', 5,
'#ifndef header guard has wrong style, please use: %s' % cppvar)
- if endif != ('#endif // %s' % cppvar):
- error_level = 0
- if endif != ('#endif // %s' % (cppvar + '_')):
- error_level = 5
-
- error(filename, endif_line_number, 'build/header_guard', error_level,
- '#endif line should be "#endif // %s"' % cppvar)
+ if endif != ('#endif // %s' % cppvar):
+ error(filename, endif_line_number, 'build/header_guard', 5,
+ '#endif line should be "#endif // %s"' % cppvar)
def check_for_unicode_replacement_characters(filename, lines, error):
@@ -1524,14 +1512,14 @@ def check_spacing(filename, clean_lines, line_number, error):
# Check if the // may be in quotes. If so, ignore it
# Comparisons made explicit for clarity -- pylint: disable-msg=C6403
if (line.count('"', 0, comment_position) - line.count('\\"', 0, comment_position)) % 2 == 0: # not in quotes
- # Allow one space for new scopes, two spaces otherwise:
- if (not match(r'^\s*{ //', line)
- and ((comment_position >= 1
- and line[comment_position-1] not in string.whitespace)
+ # Allow one space before end of line comment.
+ if (not match(r'^\s*$', line[:comment_position])
+ and (comment_position >= 1
+ and ((line[comment_position - 1] not in string.whitespace)
or (comment_position >= 2
- and line[comment_position-2] not in string.whitespace))):
- error(filename, line_number, 'whitespace/comments', 2,
- 'At least two spaces is best between code and comments')
+ and line[comment_position - 2] in string.whitespace)))):
+ error(filename, line_number, 'whitespace/comments', 5,
+ 'One space before end of line comments')
# There should always be a space between the // and the comment
commentend = comment_position + 2
if commentend < len(line) and not line[commentend] == ' ':
diff --git a/WebKitTools/Scripts/modules/cpp_style_unittest.py b/WebKitTools/Scripts/modules/cpp_style_unittest.py
index 80a9868..53e1205 100644
--- a/WebKitTools/Scripts/modules/cpp_style_unittest.py
+++ b/WebKitTools/Scripts/modules/cpp_style_unittest.py
@@ -399,13 +399,13 @@ class CppStyleTest(CppStyleTestBase):
' [readability/casting] [4]')
# Checks for false positives...
self.assert_lint(
- 'int a = int(); // Constructor, o.k.',
+ 'int a = int(); // Constructor, o.k.',
'')
self.assert_lint(
- 'X::X() : a(int()) {} // default Constructor, o.k.',
+ 'X::X() : a(int()) {} // default Constructor, o.k.',
'')
self.assert_lint(
- 'operator bool(); // Conversion operator, o.k.',
+ 'operator bool(); // Conversion operator, o.k.',
'')
# The second parameter to a gMock method definition is a function signature
@@ -720,7 +720,7 @@ class CppStyleTest(CppStyleTestBase):
# missing explicit, with distracting comment, is still bad
self.assert_multi_line_lint(
'''class Foo {
- Foo(int f); // simpler than Foo(blargh, blarg)
+ Foo(int f); // simpler than Foo(blargh, blarg)
};''',
'Single-argument constructors should be marked explicit.'
' [runtime/explicit] [5]')
@@ -1088,7 +1088,7 @@ class CppStyleTest(CppStyleTestBase):
' [readability/check] [2]')
self.assert_lint(
- ' EXPECT_TRUE(42 < x) // Random comment.',
+ ' EXPECT_TRUE(42 < x) // Random comment.',
'Consider using EXPECT_LT instead of EXPECT_TRUE(a < b)'
' [readability/check] [2]')
self.assert_lint(
@@ -1248,30 +1248,31 @@ class CppStyleTest(CppStyleTestBase):
' [whitespace/operators] [3]')
self.assert_lint('a<Foo*> t <<= *b/c;', 'Missing spaces around /'
' [whitespace/operators] [3]')
- self.assert_lint('a<Foo*> t <<= b/c; //Test', ['At least two spaces'
- ' is best between code and comments [whitespace/'
- 'comments] [2]', 'Should have a space between // '
- 'and comment [whitespace/comments] [4]', 'Missing'
+ self.assert_lint('a<Foo*> t <<= b/c; //Test', [
+ 'Should have a space between // and comment '
+ '[whitespace/comments] [4]', 'Missing'
' spaces around / [whitespace/operators] [3]'])
- self.assert_lint('a<Foo*> t <<= b||c; //Test', ['Should have a space'
- ' between // and comment [whitespace/comments] [4]',
+ self.assert_lint('a<Foo*> t <<= b||c; //Test', ['One space before end'
+ ' of line comments [whitespace/comments] [5]',
+ 'Should have a space between // and comment '
+ '[whitespace/comments] [4]',
'Missing spaces around || [whitespace/operators] [3]'])
- self.assert_lint('a<Foo*> t <<= b&&c; // Test', 'Missing spaces around'
+ self.assert_lint('a<Foo*> t <<= b&&c; // Test', 'Missing spaces around'
' && [whitespace/operators] [3]')
- self.assert_lint('a<Foo*> t <<= b&&&c; // Test', 'Missing spaces around'
+ self.assert_lint('a<Foo*> t <<= b&&&c; // Test', 'Missing spaces around'
' && [whitespace/operators] [3]')
- self.assert_lint('a<Foo*> t <<= b&&*c; // Test', 'Missing spaces around'
+ self.assert_lint('a<Foo*> t <<= b&&*c; // Test', 'Missing spaces around'
' && [whitespace/operators] [3]')
- self.assert_lint('a<Foo*> t <<= b && *c; // Test', '')
- self.assert_lint('a<Foo*> t <<= b && &c; // Test', '')
+ self.assert_lint('a<Foo*> t <<= b && *c; // Test', '')
+ self.assert_lint('a<Foo*> t <<= b && &c; // Test', '')
self.assert_lint('a<Foo*> t <<= b || &c; /*Test', 'Complex multi-line '
'/*...*/-style comment found. Lint may give bogus '
'warnings. Consider replacing these with //-style'
' comments, with #if 0...#endif, or with more clearly'
' structured multi-line comments. [readability/multiline_comment] [5]')
self.assert_lint('a<Foo&> t <<= &b | &c;', '')
- self.assert_lint('a<Foo*> t <<= &b & &c; // Test', '')
- self.assert_lint('a<Foo*> t <<= *b / &c; // Test', '')
+ self.assert_lint('a<Foo*> t <<= &b & &c; // Test', '')
+ self.assert_lint('a<Foo*> t <<= *b / &c; // Test', '')
self.assert_lint('if (a=b == 1)', 'Missing spaces around = [whitespace/operators] [4]')
self.assert_lint('a = 1<<20', 'Missing spaces around << [whitespace/operators] [3]')
self.assert_lint('if (a = b == 1)', '')
@@ -1305,7 +1306,7 @@ class CppStyleTest(CppStyleTestBase):
'For a static/global string constant, use a C style '
'string instead: "char foo[]".'
' [runtime/string] [4]')
- self.assert_lint('string kFoo = "hello"; // English',
+ self.assert_lint('string kFoo = "hello"; // English',
'For a static/global string constant, use a C style '
'string instead: "char kFoo[]".'
' [runtime/string] [4]')
@@ -1364,25 +1365,27 @@ class CppStyleTest(CppStyleTestBase):
def test_two_spaces_between_code_and_comments(self):
self.assert_lint('} // namespace foo',
- 'At least two spaces is best between code and comments'
- ' [whitespace/comments] [2]')
+ '')
self.assert_lint('}// namespace foo',
- 'At least two spaces is best between code and comments'
- ' [whitespace/comments] [2]')
+ 'One space before end of line comments'
+ ' [whitespace/comments] [5]')
self.assert_lint('printf("foo"); // Outside quotes.',
- 'At least two spaces is best between code and comments'
- ' [whitespace/comments] [2]')
- self.assert_lint('int i = 0; // Having two spaces is fine.', '')
- self.assert_lint('int i = 0; // Having three spaces is OK.', '')
+ '')
+ self.assert_lint('int i = 0; // Having one space is fine.','')
+ self.assert_lint('int i = 0; // Having two spaces is bad.',
+ 'One space before end of line comments'
+ ' [whitespace/comments] [5]')
+ self.assert_lint('int i = 0; // Having three spaces is bad.',
+ 'One space before end of line comments'
+ ' [whitespace/comments] [5]')
self.assert_lint('// Top level comment', '')
self.assert_lint(' // Line starts with four spaces.', '')
self.assert_lint('foo();\n'
'{ // A scope is opening.', '')
self.assert_lint(' foo();\n'
' { // An indented scope is opening.', '')
- self.assert_lint('if (foo) { // not a pure scope; comment is too close!',
- 'At least two spaces is best between code and comments'
- ' [whitespace/comments] [2]')
+ self.assert_lint('if (foo) { // not a pure scope',
+ '')
self.assert_lint('printf("// In quotes.")', '')
self.assert_lint('printf("\\"%s // In quotes.")', '')
self.assert_lint('printf("%s", "// In quotes.")', '')
@@ -1561,7 +1564,7 @@ class CppStyleTest(CppStyleTestBase):
def test_tab(self):
self.assert_lint('\tint a;',
'Tab found; better to use spaces [whitespace/tab] [1]')
- self.assert_lint('int a = 5;\t\t// set a to 5',
+ self.assert_lint('int a = 5;\t// set a to 5',
'Tab found; better to use spaces [whitespace/tab] [1]')
def test_parse_arguments(self):
@@ -1730,7 +1733,7 @@ class CppStyleTest(CppStyleTestBase):
' [build/forward_decl] [5]')
def test_build_header_guard(self):
- file_path = 'mydir/foo.h'
+ file_path = 'mydir/Foo.h'
# We can't rely on our internal stuff to get a sane path on the open source
# side of things, so just parse out the suggested header guard. This
@@ -1740,7 +1743,7 @@ class CppStyleTest(CppStyleTestBase):
cpp_style.process_file_data(file_path, 'h', [], error_collector)
expected_guard = ''
matcher = re.compile(
- 'No \#ifndef header guard found\, suggested CPP variable is\: ([A-Z_0-9]+) ')
+ 'No \#ifndef header guard found\, suggested CPP variable is\: ([A-Za-z_0-9]+) ')
for error in error_collector.result_list():
matches = matcher.match(error)
if matches:
@@ -1794,7 +1797,7 @@ class CppStyleTest(CppStyleTestBase):
self.assertEquals(
1,
error_collector.result_list().count(
- '#endif line should be "#endif // %s"'
+ '#endif line should be "#endif // %s"'
' [build/header_guard] [5]' % expected_guard),
error_collector.result_list())
@@ -1808,7 +1811,7 @@ class CppStyleTest(CppStyleTestBase):
self.assertEquals(
1,
error_collector.result_list().count(
- '#endif line should be "#endif // %s"'
+ '#endif line should be "#endif // %s"'
' [build/header_guard] [5]' % expected_guard),
error_collector.result_list())
@@ -1822,7 +1825,7 @@ class CppStyleTest(CppStyleTestBase):
self.assertEquals(
1,
error_collector.result_list().count(
- '#endif line should be "#endif // %s"'
+ '#endif line should be "#endif // %s"'
' [build/header_guard] [5]' % expected_guard),
error_collector.result_list())
@@ -1831,42 +1834,12 @@ class CppStyleTest(CppStyleTestBase):
cpp_style.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)
-
- # No header guard errors for old-style guard
- 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],
+ '#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)
- old_verbose_level = cpp_style._cpp_style_state.verbose_level
- try:
- cpp_style._cpp_style_state.verbose_level = 0
- # Warn on old-style guard if verbosity is 0.
- 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.assertEquals(
- 1,
- error_collector.result_list().count(
- '#ifndef header guard has wrong style, please use: %s'
- ' [build/header_guard] [0]' % expected_guard),
- error_collector.result_list())
- finally:
- cpp_style._cpp_style_state.verbose_level = old_verbose_level
-
# Completely incorrect header guard
error_collector = ErrorCollector(self.assert_)
cpp_style.process_file_data(file_path, 'h',
@@ -1883,7 +1856,7 @@ class CppStyleTest(CppStyleTestBase):
self.assertEquals(
1,
error_collector.result_list().count(
- '#endif line should be "#endif // %s"'
+ '#endif line should be "#endif // %s"'
' [build/header_guard] [5]' % expected_guard),
error_collector.result_list())
@@ -2912,7 +2885,7 @@ class WebKitStyleTest(CppStyleTestBase):
'foo.cpp')
self.assert_multi_line_lint(
'namespace WebCore {\n\n'
- 'const char* foo(void* a = ";", // ;\n'
+ 'const char* foo(void* a = ";", // ;\n'
' void* b);\n'
' void* p;\n'
'}\n',
@@ -2921,7 +2894,7 @@ class WebKitStyleTest(CppStyleTestBase):
self.assert_multi_line_lint(
'namespace WebCore {\n\n'
'const char* foo[] = {\n'
- ' "void* b);", // ;\n'
+ ' "void* b);", // ;\n'
' "asfdf",\n'
' }\n'
' void* p;\n'
@@ -2931,7 +2904,7 @@ class WebKitStyleTest(CppStyleTestBase):
self.assert_multi_line_lint(
'namespace WebCore {\n\n'
'const char* foo[] = {\n'
- ' "void* b);", // }\n'
+ ' "void* b);", // }\n'
' "asfdf",\n'
' }\n'
'}\n',
@@ -2941,7 +2914,7 @@ class WebKitStyleTest(CppStyleTestBase):
' namespace WebCore {\n\n'
' void Document::Foo()\n'
' {\n'
- 'start: // infinite loops are fun!\n'
+ 'start: // infinite loops are fun!\n'
' goto start;\n'
' }',
'namespace should never be indented. [whitespace/indent] [4]',
@@ -3495,7 +3468,7 @@ class WebKitStyleTest(CppStyleTestBase):
' [readability/null] [4]',
'foo.cpp')
self.assert_lint(
- '"A string with NULL" // and a comment with NULL is tricky to flag correctly in cpp_style.',
+ '"A string with NULL" // and a comment with NULL is tricky to flag correctly in cpp_style.',
'Use 0 instead of NULL.'
' [readability/null] [4]',
'foo.cpp')
@@ -3712,6 +3685,15 @@ class WebKitStyleTest(CppStyleTestBase):
self.assert_lint('typedef VectorType::const_iterator const_iterator;', '')
+ def test_comments(self):
+ # A comment at the beginning of a line is ok.
+ self.assert_lint('// comment', '')
+ self.assert_lint(' // comment', '')
+
+ self.assert_lint('} // namespace WebCore',
+ 'One space before end of line comments'
+ ' [whitespace/comments] [5]')
+
def test_other(self):
# FIXME: Implement this.
pass
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list