Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2310)

Unified Diff: Tools/Scripts/webkitpy/style/checkers/cpp.py

Issue 145033013: Update the static_cast style check to check for DEFINE_TYPE_CASTS patterns as well. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: Corrected the message! Created 6 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Tools/Scripts/webkitpy/style/checkers/cpp.py
diff --git a/Tools/Scripts/webkitpy/style/checkers/cpp.py b/Tools/Scripts/webkitpy/style/checkers/cpp.py
index 52519527f81faf8f6855ed1f6cadf92134611399..07429358658fe41456ef7262b8203138cb27100f 100644
--- a/Tools/Scripts/webkitpy/style/checkers/cpp.py
+++ b/Tools/Scripts/webkitpy/style/checkers/cpp.py
@@ -3411,17 +3411,16 @@ def check_identifier_name_in_declaration(filename, line_number, line, file_state
line = line[matched.end():]
-def check_for_toFoo_definition(filename, pattern, error):
- """ Reports for using static_cast instead of toFoo convenience function.
+_RE_GENERAL_MACRO_PATTERN_STRING = None
+_RE_EXPLICIT_FUNCTION_PATTERN_STRING = None
- This function will output warnings to make sure you are actually using
- the added toFoo conversion functions rather than directly hard coding
- the static_cast<Classname*> call. For example, you should toHTMLELement(Node*)
- to convert Node* to HTMLElement*, instead of static_cast<HTMLElement*>(Node*)
+
+def check_for_toFoo_definition(filename, search_function_name, error):
+ """Checks for a Cpp-style static cast on objects by searching for the pattern.
Args:
filename: The name of the header file in which to check for toFoo definition.
- pattern: The conversion function pattern to grep for.
+ search_function_name: The toFoo convenience function string.
error: The function to call with any errors found.
"""
def get_abs_filepath(filename):
@@ -3433,19 +3432,24 @@ def check_for_toFoo_definition(filename, pattern, error):
return os.path.join(root, filename)
return None
- def grep(lines, pattern, error):
+ def grep(lines, search_function_name, error):
matches = []
function_state = None
for line_number in xrange(lines.num_lines()):
line = (lines.elided[line_number]).rstrip()
try:
- if pattern in line:
+ if re.search(_RE_GENERAL_MACRO_PATTERN_STRING, line):
+ # Search for DEFINE_TYPE_CASTS macro pattern in the source header.
+ matches.append([line, line_number + 1, line_number + 1])
+ elif re.search(_RE_EXPLICIT_FUNCTION_PATTERN_STRING, line):
+ # DEFINE_TYPE_CASTS macro pattern not found in source header.
+ # Instead, search for toFoo() function pattern in source header.
if not function_state:
function_state = _FunctionState(1)
detect_functions(lines, line_number, function_state, error)
# Exclude the match of dummy conversion function. Dummy function is just to
# catch invalid conversions and shouldn't be part of possible alternatives.
- result = re.search(r'%s(\s+)%s' % ("void", pattern), line)
+ result = re.search(r'%s(\s+)%s' % ("void", search_function_name), line)
if not result:
matches.append([line, function_state.body_start_position.row, function_state.end_position.row + 1])
function_state = None
@@ -3491,12 +3495,17 @@ def check_for_toFoo_definition(filename, pattern, error):
f.close()
# Make a list of all genuine alternatives to static_cast.
- matches = grep(clean_lines, pattern, error)
+ matches = grep(clean_lines, search_function_name, error)
return matches
def check_for_object_static_cast(processing_file, line_number, line, error):
- """Checks for a Cpp-style static cast on objects by looking for the pattern.
+ """ Reports for using static_cast instead of toFoo convenience function.
+
+ This function will output warnings to make sure you are actually using
+ the added toFoo conversion functions rather than directly hard coding
+ the static_cast<Classname*> call. For example, you should toHTMLELement(Node*)
+ to convert Node* to HTMLElement*, instead of static_cast<HTMLElement*>(Node*)
Args:
processing_file: The name of the processing file.
@@ -3504,6 +3513,7 @@ def check_for_object_static_cast(processing_file, line_number, line, error):
line: The line of code to check.
error: The function to call with any errors found.
"""
+
matched = search(r'\bstatic_cast<(\s*\w*:?:?\w+\s*\*+\s*)>', line)
if not matched:
return
@@ -3519,9 +3529,13 @@ def check_for_object_static_cast(processing_file, line_number, line, error):
class_name = class_name[namespace_pos + 2:]
header_file = ''.join((class_name, '.h'))
- matches = check_for_toFoo_definition(header_file, ''.join(('to', class_name)), error)
- # Ignore (for now) if not able to find the header where toFoo might be defined.
- # TODO: Handle cases where Classname might be defined in some other header or cpp file.
+ toFoo_name = ''.join(('to', class_name))
+ _RE_GENERAL_MACRO_PATTERN_STRING = re.compile(r'\s*DEFINE_(\w*)TYPE_CASTS\(%s,(.+)(\);$)' % class_name)
sof 2014/02/05 13:26:47 Doesn't this need a 'global' to be able to work as
+ _RE_EXPLICIT_FUNCTION_PATTERN_STRING = re.compile(r'[ \w]*%s\*\s+%s\((.+)\)$' % (class_name, toFoo_name))
+ matches = check_for_toFoo_definition(header_file, toFoo_name, error)
+
+ # FIXME: Handle cases where Classname might be defined in some other header or cpp file.
+ # It will ensure check_for_toFoo_definition nevers 'None' and we can remove this unneccessary check.
if matches is None:
return
@@ -3536,15 +3550,14 @@ def check_for_object_static_cast(processing_file, line_number, line, error):
if report_error:
if len(matches):
# toFoo is defined - enforce using it.
- # TODO: Suggest an appropriate toFoo from the alternatives present in matches.
- error(line_number, 'runtime/casting', 4,
+ # FIXME: Suggest an appropriate toFoo from the alternatives present in matches.
+ error(line_number, 'security/casting', 4,
'static_cast of class objects is not allowed. Use to%s defined in %s.' %
(class_name, header_file))
else:
# No toFoo defined - enforce definition & usage.
- # TODO: Automate the generation of toFoo() to avoid any slippages ever.
error(line_number, 'runtime/casting', 4,
- 'static_cast of class objects is not allowed. Add to%s in %s and use it instead.' %
+ 'static_cast of class objects is not allowed. Define cast macro DEFINE_TYPE_CASTS(%s) in %s and use it.' %
(class_name, header_file))
@@ -3984,6 +3997,7 @@ class CppChecker(object):
'runtime/threadsafe_fn',
'runtime/unsigned',
'runtime/virtual',
+ 'security/casting',
'whitespace/blank_line',
'whitespace/braces',
'whitespace/comma',
« no previous file with comments | « no previous file | Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698