[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373
hamaji at chromium.org
hamaji at chromium.org
Thu Apr 8 00:17:33 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit 65c1ae21f92828178cc1009cccc2a5e2bee8a726
Author: hamaji at chromium.org <hamaji at chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Fri Dec 4 07:42:19 2009 +0000
2009-12-03 Shinichiro Hamaji <hamaji at chromium.org>
Reviewed by David Levin.
check-webkit-style should check for camelCase variable names
https://bugs.webkit.org/show_bug.cgi?id=32051
* Scripts/modules/cpp_style.py:
* Scripts/modules/cpp_style_unittest.py:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@51682 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 2d36a93..85dc830 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,13 @@
+2009-12-03 Shinichiro Hamaji <hamaji at chromium.org>
+
+ Reviewed by David Levin.
+
+ check-webkit-style should check for camelCase variable names
+ https://bugs.webkit.org/show_bug.cgi?id=32051
+
+ * Scripts/modules/cpp_style.py:
+ * Scripts/modules/cpp_style_unittest.py:
+
2009-12-03 Chris Fleizach <cfleizach at apple.com>
Reviewed by Eric Seidel.
diff --git a/WebKitTools/Scripts/modules/cpp_style.py b/WebKitTools/Scripts/modules/cpp_style.py
index b17e4fb..fc143a9 100644
--- a/WebKitTools/Scripts/modules/cpp_style.py
+++ b/WebKitTools/Scripts/modules/cpp_style.py
@@ -130,6 +130,7 @@ _ERROR_CATEGORIES = '''\
readability/function
readability/multiline_comment
readability/multiline_string
+ readability/naming
readability/null
readability/streams
readability/todo
@@ -243,14 +244,14 @@ _PRIMARY_HEADER = 1
_OTHER_HEADER = 2
+# The regexp compilation caching is inlined in all regexp functions for
+# performance reasons; factoring it out into a separate function turns out
+# to be noticeably expensive.
_regexp_compile_cache = {}
def match(pattern, s):
"""Matches the string with the pattern, caching the compiled regexp."""
- # The regexp compilation caching is inlined in both match and search for
- # performance reasons; factoring it out into a separate function turns out
- # to be noticeably expensive.
if not pattern in _regexp_compile_cache:
_regexp_compile_cache[pattern] = sre_compile.compile(pattern)
return _regexp_compile_cache[pattern].match(s)
@@ -263,6 +264,20 @@ def search(pattern, s):
return _regexp_compile_cache[pattern].search(s)
+def sub(pattern, replacement, s):
+ """Substitutes occurrences of a pattern, caching the compiled regexp."""
+ if not pattern in _regexp_compile_cache:
+ _regexp_compile_cache[pattern] = sre_compile.compile(pattern)
+ return _regexp_compile_cache[pattern].sub(replacement, s)
+
+
+def subn(pattern, replacement, s):
+ """Substitutes occurrences of a pattern, caching the compiled regexp."""
+ if not pattern in _regexp_compile_cache:
+ _regexp_compile_cache[pattern] = sre_compile.compile(pattern)
+ return _regexp_compile_cache[pattern].subn(replacement, s)
+
+
class _IncludeState(dict):
"""Tracks line numbers for includes, and the order in which includes appear.
@@ -868,7 +883,7 @@ def get_header_guard_cpp_variable(filename):
"""
fileinfo = FileInfo(filename)
- return re.sub(r'[-./\s]', '_', fileinfo.repository_name()).upper() + '_'
+ return sub(r'[-./\s]', '_', fileinfo.repository_name()).upper() + '_'
def check_for_header_guard(filename, lines, error):
@@ -1542,7 +1557,7 @@ def check_spacing(filename, clean_lines, line_number, error):
line = clean_lines.elided[line_number] # get rid of comments and strings
# Don't try to do spacing checks for operator methods
- line = re.sub(r'operator(==|!=|<|<<|<=|>=|>>|>)\(', 'operator\(', line)
+ line = sub(r'operator(==|!=|<|<<|<=|>=|>>|>)\(', 'operator\(', line)
# Don't try to do spacing checks for #include or #import statements at
# minimum because it messes up checks for spacing around /
if match(r'\s*#\s*(?:include|import)', line):
@@ -2633,6 +2648,106 @@ def check_language(filename, clean_lines, line_number, file_extension, include_s
'http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Namespaces'
' for more information.')
+ check_identifier_name_in_declaration(filename, line_number, line, error)
+
+
+def check_identifier_name_in_declaration(filename, line_number, line, error):
+ """Checks if identifier names contain any underscores.
+
+ As identifiers in libraries we are using have a bunch of
+ underscores, we only warn about the declarations of identifiers
+ and don't check use of identifiers.
+
+ Args:
+ filename: The name of the current file.
+ line_number: The number of the line to check.
+ line: The line of code to check.
+ error: The function to call with any errors found.
+ """
+ # We don't check a return statement.
+ if match(r'\s*return\b', line):
+ return
+
+ # Basically, a declaration is a type name followed by whitespaces
+ # followed by an identifier. The type name can be complicated
+ # due to type adjectives and templates. We remove them first to
+ # simplify the process to find declarations of identifiers.
+
+ # Convert "long long", "long double", and "long long int" to
+ # simple types, but don't remove simple "long".
+ line = sub(r'long (long )?(?=long|double|int)', '', line)
+ line = sub(r'\b(unsigned|signed|inline|using|static|const|volatile|auto|register|extern|typedef|restrict|struct|class|virtual)(?=\W)', '', line)
+
+ # Remove all template parameters by removing matching < and >.
+ # Loop until no templates are removed to remove nested templates.
+ while True:
+ line, number_of_replacements = subn(r'<([\w\s:]|::)+\s*[*&]*\s*>', '', line)
+ if not number_of_replacements:
+ break
+
+ # Declarations of local variables can be in condition expressions
+ # of control flow statements (e.g., "if (RenderObject* p = o->parent())").
+ # We remove the keywords and the first parenthesis.
+ #
+ # Declarations in "while", "if", and "switch" are different from
+ # other declarations in two aspects:
+ #
+ # - There can be only one declaration between the parentheses.
+ # (i.e., you cannot write "if (int i = 0, j = 1) {}")
+ # - The variable must be initialized.
+ # (i.e., you cannot write "if (int i) {}")
+ #
+ # and we will need different treatments for them.
+ line = sub(r'^\s*for\s*\(', '', line)
+ line, control_statement = subn(r'^\s*(while|else if|if|switch)\s*\(', '', line)
+
+ # Detect variable and functions.
+ type_regexp = r'\w([\w]|\s*[*&]\s*|::)+'
+ identifier_regexp = r'(?P<identifier>[\w:]+)'
+ character_after_identifier_regexp = r'(?P<character_after_identifier>[[;()=,])'
+ declaration_without_type_regexp = r'\s*' + identifier_regexp + r'\s*' + character_after_identifier_regexp
+ declaration_with_type_regexp = r'\s*' + type_regexp + r'\s' + declaration_without_type_regexp
+ is_function_arguments = False
+ number_of_identifiers = 0
+ while True:
+ # If we are seeing the first identifier or arguments of a
+ # function, there should be a type name before an identifier.
+ if not number_of_identifiers or is_function_arguments:
+ declaration_regexp = declaration_with_type_regexp
+ else:
+ declaration_regexp = declaration_without_type_regexp
+
+ matched = match(declaration_regexp, line)
+ if not matched:
+ return
+ identifier = matched.group('identifier')
+ character_after_identifier = matched.group('character_after_identifier')
+
+ # If we removed a non-for-control statement, the character after
+ # the identifier should be '='. With this rule, we can avoid
+ # warning for cases like "if (val & INT_MAX) {".
+ if control_statement and character_after_identifier != '=':
+ return
+
+ is_function_arguments = is_function_arguments or character_after_identifier == '('
+
+ # Remove "m_" and "s_" to allow them.
+ modified_identifier = sub(r'(^|(?<=::))[ms]_', '', identifier)
+ if modified_identifier.find('_') >= 0:
+ error(filename, line_number, 'readability/naming', 4, identifier + " is incorrectly named. Don't use underscores in your identifier names.")
+
+ # There can be only one declaration in non-for-control statements.
+ if control_statement:
+ return
+ # We should continue checking if this is a function
+ # declaration because we need to check its arguments.
+ # Also, we need to check multiple declarations.
+ if character_after_identifier != '(' and character_after_identifier != ',':
+ return
+
+ number_of_identifiers += 1
+ line = line[matched.end():]
+
def check_c_style_cast(filename, line_number, line, raw_line, cast_type, pattern,
error):
diff --git a/WebKitTools/Scripts/modules/cpp_style_unittest.py b/WebKitTools/Scripts/modules/cpp_style_unittest.py
index aebcf82..3f0f230 100644
--- a/WebKitTools/Scripts/modules/cpp_style_unittest.py
+++ b/WebKitTools/Scripts/modules/cpp_style_unittest.py
@@ -936,15 +936,15 @@ class CppStyleTest(CppStyleTestBase):
self.assert_lint('int doublesize[some_var * 2];', errmsg)
self.assert_lint('int a[afunction()];', errmsg)
self.assert_lint('int a[function(kMaxFooBars)];', errmsg)
- self.assert_lint('bool a_list[items_->size()];', errmsg)
+ self.assert_lint('bool aList[items_->size()];', errmsg)
self.assert_lint('namespace::Type buffer[len+1];', errmsg)
self.assert_lint('int a[64];', '')
self.assert_lint('int a[0xFF];', '')
self.assert_lint('int first[256], second[256];', '')
- self.assert_lint('int array_name[kCompileTimeConstant];', '')
+ self.assert_lint('int arrayName[kCompileTimeConstant];', '')
self.assert_lint('char buf[somenamespace::kBufSize];', '')
- self.assert_lint('int array_name[ALL_CAPS];', '')
+ self.assert_lint('int arrayName[ALL_CAPS];', '')
self.assert_lint('AClass array1[foo::bar::ALL_CAPS];', '')
self.assert_lint('int a[kMaxStrLen + 1];', '')
self.assert_lint('int a[sizeof(foo)];', '')
@@ -1524,14 +1524,14 @@ class CppStyleTest(CppStyleTestBase):
def test_indent(self):
self.assert_lint('static int noindent;', '')
- self.assert_lint(' int four_space_indent;', '')
- self.assert_lint(' int one_space_indent;',
+ self.assert_lint(' int fourSpaceIndent;', '')
+ self.assert_lint(' int oneSpaceIndent;',
'Weird number of spaces at line-start. '
'Are you using a 4-space indent? [whitespace/indent] [3]')
- self.assert_lint(' int three_space_indent;',
+ self.assert_lint(' int threeSpaceIndent;',
'Weird number of spaces at line-start. '
'Are you using a 4-space indent? [whitespace/indent] [3]')
- self.assert_lint(' char* one_space_indent = "public:";',
+ self.assert_lint(' char* oneSpaceIndent = "public:";',
'Weird number of spaces at line-start. '
'Are you using a 4-space indent? [whitespace/indent] [3]')
self.assert_lint(' public:', '')
@@ -1964,7 +1964,7 @@ class CppStyleTest(CppStyleTestBase):
self.assert_lint('double const static foo = 2.0;',
build_storage_class_error_message)
- self.assert_lint('uint64 typedef unsigned_long_long;',
+ self.assert_lint('uint64 typedef unsignedLongLong;',
build_storage_class_error_message)
self.assert_lint('int register foo = 0;',
@@ -2048,6 +2048,7 @@ class CppStyleTest(CppStyleTestBase):
'Changing pointer instead of value (or unused value of '
'operator*). [runtime/invalid_increment] [5]')
+
class CleansedLinesTest(unittest.TestCase):
def test_init(self):
lines = ['Line 1',
@@ -3601,8 +3602,95 @@ class WebKitStyleTest(CppStyleTestBase):
'foo.h')
def test_names(self):
- # FIXME: Implement this.
- pass
+ name_error_message = " is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]"
+
+ # Basic cases from WebKit style guide.
+ self.assert_lint('struct Data;', '')
+ self.assert_lint('size_t bufferSize;', '')
+ self.assert_lint('class HTMLDocument;', '')
+ self.assert_lint('String mimeType();', '')
+ self.assert_lint('size_t buffer_size;',
+ 'buffer_size' + name_error_message)
+ self.assert_lint('short m_length;', '')
+ self.assert_lint('short _length;',
+ '_length' + name_error_message)
+ self.assert_lint('short length_;',
+ 'length_' + name_error_message)
+
+ # Pointers, references, functions, templates, and adjectives.
+ self.assert_lint('char* under_score;',
+ 'under_score' + name_error_message)
+ self.assert_lint('const int UNDER_SCORE;',
+ 'UNDER_SCORE' + name_error_message)
+ self.assert_lint('static inline const char const& const under_score;',
+ 'under_score' + name_error_message)
+ self.assert_lint('WebCore::RenderObject* under_score;',
+ 'under_score' + name_error_message)
+ self.assert_lint('int func_name();',
+ 'func_name' + name_error_message)
+ self.assert_lint('RefPtr<RenderObject*> under_score;',
+ 'under_score' + name_error_message)
+ self.assert_lint('WTF::Vector<WTF::RefPtr<const RenderObject* const> > under_score;',
+ 'under_score' + name_error_message)
+ self.assert_lint('int under_score[];',
+ 'under_score' + name_error_message)
+ self.assert_lint('struct dirent* under_score;',
+ 'under_score' + name_error_message)
+ self.assert_lint('long under_score;',
+ 'under_score' + name_error_message)
+ self.assert_lint('long long under_score;',
+ 'under_score' + name_error_message)
+ self.assert_lint('long double under_score;',
+ 'under_score' + name_error_message)
+ self.assert_lint('long long int under_score;',
+ 'under_score' + name_error_message)
+
+ # Declarations in control statement.
+ self.assert_lint('if (int under_score = 42) {',
+ 'under_score' + name_error_message)
+ self.assert_lint('else if (int under_score = 42) {',
+ 'under_score' + name_error_message)
+ self.assert_lint('for (int under_score = 42; cond; i++) {',
+ 'under_score' + name_error_message)
+ self.assert_lint('while (foo & under_score = bar) {',
+ 'under_score' + name_error_message)
+ self.assert_lint('for (foo * under_score = p; cond; i++) {',
+ 'under_score' + name_error_message)
+ self.assert_lint('for (foo * under_score; cond; i++) {',
+ 'under_score' + name_error_message)
+ self.assert_lint('while (foo & value_in_thirdparty_library) {', '')
+ self.assert_lint('while (foo * value_in_thirdparty_library) {', '')
+
+ # More member variables and functions.
+ self.assert_lint('int SomeClass::s_validName', '')
+ self.assert_lint('int m_under_score;',
+ 'm_under_score' + name_error_message)
+ self.assert_lint('int SomeClass::s_under_score = 0;',
+ 'SomeClass::s_under_score' + name_error_message)
+ self.assert_lint('int SomeClass::under_score = 0;',
+ 'SomeClass::under_score' + name_error_message)
+
+ # Other statements.
+ self.assert_lint('return INT_MAX;', '')
+ self.assert_lint('return_t under_score;',
+ 'under_score' + name_error_message)
+ self.assert_lint('goto under_score;',
+ 'under_score' + name_error_message)
+
+ # Multiple variables in one line.
+ self.assert_lint('void myFunction(int variable1, int another_variable);',
+ 'another_variable' + name_error_message)
+ self.assert_lint('int variable1, another_variable;',
+ 'another_variable' + name_error_message)
+ self.assert_lint('int first_variable, secondVariable;',
+ 'first_variable' + name_error_message)
+ self.assert_lint('void my_function(int variable_1, int variable_2);',
+ ['my_function' + name_error_message,
+ 'variable_1' + name_error_message,
+ 'variable_2' + name_error_message])
+ self.assert_lint('for (int variable_1, variable_2;;) {',
+ ['variable_1' + name_error_message,
+ 'variable_2' + name_error_message])
def test_other(self):
# FIXME: Implement this.
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list