Modified: trunk/Tools/ChangeLog (89507 => 89508)
--- trunk/Tools/ChangeLog 2011-06-23 01:05:09 UTC (rev 89507)
+++ trunk/Tools/ChangeLog 2011-06-23 01:05:17 UTC (rev 89508)
@@ -1,3 +1,17 @@
+2011-06-22 David Levin <[email protected]>
+
+ Reviewed by Adam Barth.
+
+ check-webkit-style should detect returning (Own|Ref)Ptr instead of the Pass*Ptr version.
+ https://bugs.webkit.org/show_bug.cgi?id=63204
+
+ * Scripts/webkitpy/style/checkers/cpp.py: Added a check for the return value and combined
+ with similar code for the parameter checking.
+ * Scripts/webkitpy/style/checkers/cpp_unittest.py: Removed pass_ptr checks from
+ those done for single lines since they don't make sense in that case (variable decls look like function decls).
+ Removed some redundant comments (one of which was slightly wrong).
+ Added checks for the new functionality and minor other test changes.
+
2011-06-22 Nate Chapin <[email protected]>
Reviewed by Adam Barth.
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py (89507 => 89508)
--- trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py 2011-06-23 01:05:09 UTC (rev 89507)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py 2011-06-23 01:05:17 UTC (rev 89508)
@@ -1566,6 +1566,24 @@
return True
+def check_function_definition_and_pass_ptr(type_text, row, location_description, error):
+ """Check that function definitions for use Pass*Ptr instead of *Ptr.
+
+ Args:
+ type_text: A string containing the type. (For return values, it may contain more than the type.)
+ row: The row number of the type.
+ location_description: Used to indicate where the type is. This is either 'parameter' or 'return'.
+ error: The function to call with any errors found.
+ """
+ match_ref_or_own_ptr = '(?=\W|^)(Ref|Own)Ptr(?=\W)'
+ bad_type_usage = search(match_ref_or_own_ptr, type_text)
+ if not bad_type_usage:
+ return
+ type_name = bad_type_usage.group(0)
+ error(row, 'readability/pass_ptr', 5,
+ 'The %s type should use Pass%s instead of %s.' % (location_description, type_name, type_name))
+
+
def check_function_definition(filename, file_extension, clean_lines, line_number, function_state, error):
"""Check that function definitions for style issues.
@@ -1597,13 +1615,11 @@
error(function_state.function_name_start_position.row, 'readability/webkit_api', 5,
'WEBKIT_API should not be used with a pure virtual function.')
+ check_function_definition_and_pass_ptr(modifiers_and_return_type, function_state.function_name_start_position.row, 'return', error)
+
parameter_list = function_state.parameter_list()
for parameter in parameter_list:
- bad_parameter_type = search('(?=\W|^)(Ref|Own)Ptr(?=\W)', parameter.type)
- if bad_parameter_type:
- type_name = bad_parameter_type.group(0)
- error(parameter.row, 'readability/pass_ptr', 5,
- 'The parameter type should use Pass%s instead of %s.' % (type_name, type_name))
+ check_function_definition_and_pass_ptr(parameter.type, parameter.row, 'parameter', error)
# Do checks specific to function declarations and parameter names.
if not function_state.is_declaration or not parameter.name:
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py (89507 => 89508)
--- trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py 2011-06-23 01:05:09 UTC (rev 89507)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py 2011-06-23 01:05:17 UTC (rev 89508)
@@ -259,6 +259,7 @@
'-legal/copyright',
'-readability/fn_size',
'-readability/parameter_name',
+ '-readability/pass_ptr',
'-whitespace/ending_newline')
return self.perform_lint(code, filename, basic_error_rules)
@@ -3205,7 +3206,6 @@
self.perform_pass_ptr_check(code))
def test_pass_ref_ptr_in_function(self):
- # Local variables should never be PassRefPtr.
self.assert_pass_ptr_check(
'int myFunction()\n'
'{\n'
@@ -3215,7 +3215,6 @@
'http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5]')
def test_pass_own_ptr_in_function(self):
- # Local variables should never be PassRefPtr.
self.assert_pass_ptr_check(
'int myFunction()\n'
'{\n'
@@ -3225,7 +3224,6 @@
'http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5]')
def test_pass_other_type_ptr_in_function(self):
- # Local variables should never be PassRefPtr.
self.assert_pass_ptr_check(
'int myFunction()\n'
'{\n'
@@ -3249,15 +3247,26 @@
self.assert_pass_ptr_check(
'PassRefPtr<Type1> myFunction();\n',
'')
+ self.assert_pass_ptr_check(
+ 'OwnRefPtr<Type1> myFunction();\n',
+ '')
+ self.assert_pass_ptr_check(
+ 'RefPtr<Type1> myFunction(int)\n'
+ '{\n'
+ '}',
+ 'The return type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5]')
+ self.assert_pass_ptr_check(
+ 'OwnPtr<Type1> myFunction(int)\n'
+ '{\n'
+ '}',
+ 'The return type should use PassOwnPtr instead of OwnPtr. [readability/pass_ptr] [5]')
- def test_pass_ref_ptr_parameter_value(self):
+ def test_ref_ptr_parameter_value(self):
self.assert_pass_ptr_check(
'int myFunction(PassRefPtr<Type1>)\n'
'{\n'
'}',
'')
-
- def test_ref_ptr_parameter_value(self):
self.assert_pass_ptr_check(
'int myFunction(RefPtr<Type1>)\n'
'{\n'
@@ -3266,6 +3275,11 @@
def test_own_ptr_parameter_value(self):
self.assert_pass_ptr_check(
+ 'int myFunction(PassOwnPtr<Type1>)\n'
+ '{\n'
+ '}',
+ '')
+ self.assert_pass_ptr_check(
'int myFunction(OwnPtr<Type1>)\n'
'{\n'
'}',
