[SCM] WebKit Debian packaging branch, debian/experimental, updated. debian/1.3.8-1-142-g786665c
levin at chromium.org
levin at chromium.org
Mon Dec 27 16:30:19 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit a18e74bf588ca43c2ba4f4ae61b91c500317b6b1
Author: levin at chromium.org <levin at chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Wed Dec 22 15:31:47 2010 +0000
check-webkit-style shouldn't complaint about underscores in variables in objective C files.
https://bugs.webkit.org/show_bug.cgi?id=51452
Reviewed by Shinichiro Hamaji.
* Scripts/webkitpy/style/checkers/cpp.py:
(_FileState.__init__): Added the information to determine if a file is C or Objective C.
Using the file extension if possible but falling back to the file contents if we have a header file.
(_FileState.is_objective_c): Determine if we have an Objective C by examining the file contents if needed.
(_FileState.is_c_or_objective_c):
(check_using_std): Changed to using _FileState to determine the file type.
(check_max_min_macros): Ditto.
(check_for_null): Ditto.
(check_style): Changed the parameters to various calls since they now need _FileState
to determine the file type.
(check_language): Added the file_state parameter so it could be passed
to check_identifier_name_in_declaration.
(check_identifier_name_in_declaration): Don't warn about underscores in variables if
this is an Objective C file.
(_process_lines): Added information for the _FileState constructor (and moved the
call to a place that had the information).
* Scripts/webkitpy/style/checkers/cpp_unittest.py:
(CppFunctionsTest.test_is_c_or_objective_c): Changed the tests to use FileState and exercise
its functionality.
(WebKitStyleTest.test_names): Add tests for underscores in Objective C files.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@74478 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index 05e4e45..cf30451 100644
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,31 @@
+2010-12-22 David Levin <levin at chromium.org>
+
+ Reviewed by Shinichiro Hamaji.
+
+ check-webkit-style shouldn't complaint about underscores in variables in objective C files.
+ https://bugs.webkit.org/show_bug.cgi?id=51452
+
+ * Scripts/webkitpy/style/checkers/cpp.py:
+ (_FileState.__init__): Added the information to determine if a file is C or Objective C.
+ Using the file extension if possible but falling back to the file contents if we have a header file.
+ (_FileState.is_objective_c): Determine if we have an Objective C by examining the file contents if needed.
+ (_FileState.is_c_or_objective_c):
+ (check_using_std): Changed to using _FileState to determine the file type.
+ (check_max_min_macros): Ditto.
+ (check_for_null): Ditto.
+ (check_style): Changed the parameters to various calls since they now need _FileState
+ to determine the file type.
+ (check_language): Added the file_state parameter so it could be passed
+ to check_identifier_name_in_declaration.
+ (check_identifier_name_in_declaration): Don't warn about underscores in variables if
+ this is an Objective C file.
+ (_process_lines): Added information for the _FileState constructor (and moved the
+ call to a place that had the information).
+ * Scripts/webkitpy/style/checkers/cpp_unittest.py:
+ (CppFunctionsTest.test_is_c_or_objective_c): Changed the tests to use FileState and exercise
+ its functionality.
+ (WebKitStyleTest.test_names): Add tests for underscores in Objective C files.
+
2010-12-21 Andy Estes <aestes at apple.com>
Reviewed by Mark Rowe.
diff --git a/Tools/Scripts/webkitpy/style/checkers/cpp.py b/Tools/Scripts/webkitpy/style/checkers/cpp.py
index 3ffaeb9..94e5bdd 100644
--- a/Tools/Scripts/webkitpy/style/checkers/cpp.py
+++ b/Tools/Scripts/webkitpy/style/checkers/cpp.py
@@ -384,16 +384,6 @@ class _IncludeError(Exception):
pass
-def is_c_or_objective_c(file_extension):
- """Return whether the file extension corresponds to C or Objective-C.
-
- Args:
- file_extension: The file extension without the leading dot.
-
- """
- return file_extension in ['c', 'm']
-
-
class FileInfo:
"""Provides utility functions for filenames.
@@ -934,8 +924,19 @@ class _ClassState(object):
class _FileState(object):
- def __init__(self):
+ def __init__(self, clean_lines, file_extension):
self._did_inside_namespace_indent_warning = False
+ self._clean_lines = clean_lines
+ if file_extension in ['m', 'mm']:
+ self._is_objective_c = True
+ elif file_extension == 'h':
+ # In the case of header files, it is unknown if the file
+ # is objective c or not, so set this value to None and then
+ # if it is requested, use heuristics to guess the value.
+ self._is_objective_c = None
+ else:
+ self._is_objective_c = False
+ self._is_c = file_extension == 'c'
def set_did_inside_namespace_indent_warning(self):
self._did_inside_namespace_indent_warning = True
@@ -943,6 +944,23 @@ class _FileState(object):
def did_inside_namespace_indent_warning(self):
return self._did_inside_namespace_indent_warning
+ def is_objective_c(self):
+ if self._is_objective_c is None:
+ for line in self._clean_lines.elided:
+ # Starting with @ or #import seem like the best indications
+ # that we have an Objective C file.
+ if line.startswith("@") or line.startswith("#import"):
+ self._is_objective_c = True
+ break
+ else:
+ self._is_objective_c = False
+ return self._is_objective_c
+
+ def is_c_or_objective_c(self):
+ """Return whether the file extension corresponds to C or Objective-C."""
+ return self._is_c or self.is_objective_c()
+
+
def check_for_non_standard_constructs(clean_lines, line_number,
class_state, error):
"""Logs an error if we see certain non-ANSI constructs ignored by gcc-2.
@@ -1620,18 +1638,20 @@ def check_namespace_indentation(clean_lines, line_number, file_extension, file_s
if current_indentation_level < 0:
break;
-def check_using_std(file_extension, clean_lines, line_number, error):
+
+def check_using_std(clean_lines, line_number, file_state, error):
"""Looks for 'using std::foo;' statements which should be replaced with 'using namespace std;'.
Args:
- file_extension: The extension of the current file, without the leading dot.
clean_lines: A CleansedLines instance containing the file.
line_number: The number of the line to check.
+ file_state: A _FileState instance which maintains information about
+ the state of things in the file.
error: The function to call with any errors found.
"""
# This check doesn't apply to C or Objective-C implementation files.
- if is_c_or_objective_c(file_extension):
+ if file_state.is_c_or_objective_c():
return
line = clean_lines.elided[line_number] # Get rid of comments and strings.
@@ -1645,18 +1665,19 @@ def check_using_std(file_extension, clean_lines, line_number, error):
"Use 'using namespace std;' instead of 'using std::%s;'." % method_name)
-def check_max_min_macros(file_extension, clean_lines, line_number, error):
+def check_max_min_macros(clean_lines, line_number, file_state, error):
"""Looks use of MAX() and MIN() macros that should be replaced with std::max() and std::min().
Args:
- file_extension: The extension of the current file, without the leading dot.
clean_lines: A CleansedLines instance containing the file.
line_number: The number of the line to check.
+ file_state: A _FileState instance which maintains information about
+ the state of things in the file.
error: The function to call with any errors found.
"""
# This check doesn't apply to C or Objective-C implementation files.
- if is_c_or_objective_c(file_extension):
+ if file_state.is_c_or_objective_c():
return
line = clean_lines.elided[line_number] # Get rid of comments and strings.
@@ -1984,9 +2005,9 @@ def check_for_comparisons_to_zero(clean_lines, line_number, error):
'Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.')
-def check_for_null(file_extension, clean_lines, line_number, error):
+def check_for_null(clean_lines, line_number, file_state, error):
# This check doesn't apply to C or Objective-C implementation files.
- if is_c_or_objective_c(file_extension):
+ if file_state.is_c_or_objective_c():
return
line = clean_lines.elided[line_number]
@@ -2127,15 +2148,15 @@ def check_style(clean_lines, line_number, file_extension, class_state, file_stat
# Some more style checks
check_namespace_indentation(clean_lines, line_number, file_extension, file_state, error)
- check_using_std(file_extension, clean_lines, line_number, error)
- check_max_min_macros(file_extension, clean_lines, line_number, error)
+ check_using_std(clean_lines, line_number, file_state, error)
+ check_max_min_macros(clean_lines, line_number, file_state, error)
check_switch_indentation(clean_lines, line_number, error)
check_braces(clean_lines, line_number, error)
check_exit_statement_simplifications(clean_lines, line_number, error)
check_spacing(file_extension, clean_lines, line_number, error)
check_check(clean_lines, line_number, error)
check_for_comparisons_to_zero(clean_lines, line_number, error)
- check_for_null(file_extension, clean_lines, line_number, error)
+ check_for_null(clean_lines, line_number, file_state, error)
_RE_PATTERN_INCLUDE_NEW_STYLE = re.compile(r'#include +"[^/]+\.h"')
@@ -2337,7 +2358,7 @@ def check_include_line(filename, file_extension, clean_lines, line_number, inclu
def check_language(filename, clean_lines, line_number, file_extension, include_state,
- error):
+ file_state, error):
"""Checks rules from the 'C++ language rules' section of cppguide.html.
Some of these rules are hard to test (function overloading, using
@@ -2349,6 +2370,8 @@ def check_language(filename, clean_lines, line_number, file_extension, include_s
line_number: The number of the line to check.
file_extension: The extension (without the dot) of the filename.
include_state: An _IncludeState instance in which the headers are inserted.
+ file_state: A _FileState instance which maintains information about
+ the state of things in the file.
error: The function to call with any errors found.
"""
# If the line is empty or consists of entirely a comment, no need to
@@ -2538,10 +2561,10 @@ 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)
+ check_identifier_name_in_declaration(filename, line_number, line, file_state, error)
-def check_identifier_name_in_declaration(filename, line_number, line, error):
+def check_identifier_name_in_declaration(filename, line_number, line, file_state, error):
"""Checks if identifier names contain any underscores.
As identifiers in libraries we are using have a bunch of
@@ -2552,6 +2575,8 @@ def check_identifier_name_in_declaration(filename, line_number, line, error):
filename: The name of the current file.
line_number: The number of the line to check.
line: The line of code to check.
+ file_state: A _FileState instance which maintains information about
+ the state of things in the file.
error: The function to call with any errors found.
"""
# We don't check a return statement.
@@ -2629,7 +2654,7 @@ def check_identifier_name_in_declaration(filename, line_number, line, error):
# Remove "m_" and "s_" to allow them.
modified_identifier = sub(r'(^|(?<=::))[ms]_', '', identifier)
- if modified_identifier.find('_') >= 0:
+ if not file_state.is_objective_c() and modified_identifier.find('_') >= 0:
# Various exceptions to the rule: JavaScript op codes functions, const_iterator.
if (not (filename.find('JavaScriptCore') >= 0 and modified_identifier.find('op_') >= 0)
and not modified_identifier.startswith('tst_')
@@ -2980,7 +3005,7 @@ def process_line(filename, file_extension,
check_for_multiline_comments_and_strings(clean_lines, line, error)
check_style(clean_lines, line, file_extension, class_state, file_state, error)
check_language(filename, clean_lines, line, file_extension, include_state,
- error)
+ file_state, error)
check_for_non_standard_constructs(clean_lines, line, class_state, error)
check_posix_threading(clean_lines, line, error)
check_invalid_increment(clean_lines, line, error)
@@ -3002,7 +3027,6 @@ def _process_lines(filename, file_extension, lines, error, min_confidence):
include_state = _IncludeState()
function_state = _FunctionState(min_confidence)
class_state = _ClassState()
- file_state = _FileState()
check_for_copyright(lines, error)
@@ -3011,6 +3035,7 @@ def _process_lines(filename, file_extension, lines, error, min_confidence):
remove_multi_line_comments(lines, error)
clean_lines = CleansedLines(lines)
+ file_state = _FileState(clean_lines, file_extension)
for line in xrange(clean_lines.num_lines()):
process_line(filename, file_extension, clean_lines, line,
include_state, function_state, class_state, file_state, error)
diff --git a/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py b/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py
index d2c2570..70df1ea 100644
--- a/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py
+++ b/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py
@@ -110,9 +110,14 @@ class CppFunctionsTest(unittest.TestCase):
"""Supports testing functions that do not need CppStyleTestBase."""
def test_is_c_or_objective_c(self):
- self.assertTrue(cpp_style.is_c_or_objective_c("c"))
- self.assertTrue(cpp_style.is_c_or_objective_c("m"))
- self.assertFalse(cpp_style.is_c_or_objective_c("cpp"))
+ clean_lines = cpp_style.CleansedLines([''])
+ clean_objc_lines = cpp_style.CleansedLines(['#import "header.h"'])
+ self.assertTrue(cpp_style._FileState(clean_lines, 'c').is_c_or_objective_c())
+ self.assertTrue(cpp_style._FileState(clean_lines, 'm').is_c_or_objective_c())
+ self.assertFalse(cpp_style._FileState(clean_lines, 'cpp').is_c_or_objective_c())
+ self.assertFalse(cpp_style._FileState(clean_lines, 'cc').is_c_or_objective_c())
+ self.assertFalse(cpp_style._FileState(clean_lines, 'h').is_c_or_objective_c())
+ self.assertTrue(cpp_style._FileState(clean_objc_lines, 'h').is_c_or_objective_c())
class CppStyleTestBase(unittest.TestCase):
@@ -3755,6 +3760,30 @@ class WebKitStyleTest(CppStyleTestBase):
self.assert_lint('unsigned long long _length;',
'_length' + name_underscore_error_message)
+ # Allow underscores in Objective C files.
+ self.assert_lint('unsigned long long _length;',
+ '',
+ 'foo.m')
+ self.assert_lint('unsigned long long _length;',
+ '',
+ 'foo.mm')
+ self.assert_lint('#import "header_file.h"\n'
+ 'unsigned long long _length;',
+ '',
+ 'foo.h')
+ self.assert_lint('unsigned long long _length;\n'
+ '@interface WebFullscreenWindow;',
+ '',
+ 'foo.h')
+ self.assert_lint('unsigned long long _length;\n'
+ '@implementation WebFullscreenWindow;',
+ '',
+ 'foo.h')
+ self.assert_lint('unsigned long long _length;\n'
+ '@class WebWindowFadeAnimation;',
+ '',
+ 'foo.h')
+
# Variable name 'l' is easy to confuse with '1'
self.assert_lint('int l;', 'l' + name_tooshort_error_message)
self.assert_lint('size_t l;', 'l' + name_tooshort_error_message)
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list