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

Side by Side 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 unified diff | Download patch
« no previous file with comments | « no previous file | Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 # -*- coding: utf-8 -*- 1 # -*- coding: utf-8 -*-
2 # 2 #
3 # Copyright (C) 2009, 2010, 2012 Google Inc. All rights reserved. 3 # Copyright (C) 2009, 2010, 2012 Google Inc. All rights reserved.
4 # Copyright (C) 2009 Torch Mobile Inc. 4 # Copyright (C) 2009 Torch Mobile Inc.
5 # Copyright (C) 2009 Apple Inc. All rights reserved. 5 # Copyright (C) 2009 Apple Inc. All rights reserved.
6 # Copyright (C) 2010 Chris Jerdonek (cjerdonek@webkit.org) 6 # Copyright (C) 2010 Chris Jerdonek (cjerdonek@webkit.org)
7 # 7 #
8 # Redistribution and use in source and binary forms, with or without 8 # Redistribution and use in source and binary forms, with or without
9 # modification, are permitted provided that the following conditions are 9 # modification, are permitted provided that the following conditions are
10 # met: 10 # met:
(...skipping 3393 matching lines...) Expand 10 before | Expand all | Expand 10 after
3404 # We should continue checking if this is a function 3404 # We should continue checking if this is a function
3405 # declaration because we need to check its arguments. 3405 # declaration because we need to check its arguments.
3406 # Also, we need to check multiple declarations. 3406 # Also, we need to check multiple declarations.
3407 if character_after_identifier != '(' and character_after_identifier != ' ,': 3407 if character_after_identifier != '(' and character_after_identifier != ' ,':
3408 return 3408 return
3409 3409
3410 number_of_identifiers += 1 3410 number_of_identifiers += 1
3411 line = line[matched.end():] 3411 line = line[matched.end():]
3412 3412
3413 3413
3414 def check_for_toFoo_definition(filename, pattern, error): 3414 _RE_GENERAL_MACRO_PATTERN_STRING = None
3415 """ Reports for using static_cast instead of toFoo convenience function. 3415 _RE_EXPLICIT_FUNCTION_PATTERN_STRING = None
3416 3416
3417 This function will output warnings to make sure you are actually using 3417
3418 the added toFoo conversion functions rather than directly hard coding 3418 def check_for_toFoo_definition(filename, search_function_name, error):
3419 the static_cast<Classname*> call. For example, you should toHTMLELement(Node *) 3419 """Checks for a Cpp-style static cast on objects by searching for the patter n.
3420 to convert Node* to HTMLElement*, instead of static_cast<HTMLElement*>(Node* )
3421 3420
3422 Args: 3421 Args:
3423 filename: The name of the header file in which to check for toFoo definiti on. 3422 filename: The name of the header file in which to check for toFoo definiti on.
3424 pattern: The conversion function pattern to grep for. 3423 search_function_name: The toFoo convenience function string.
3425 error: The function to call with any errors found. 3424 error: The function to call with any errors found.
3426 """ 3425 """
3427 def get_abs_filepath(filename): 3426 def get_abs_filepath(filename):
3428 fileSystem = FileSystem() 3427 fileSystem = FileSystem()
3429 base_dir = fileSystem.path_to_module(FileSystem.__module__).split('WebKi t', 1)[0] 3428 base_dir = fileSystem.path_to_module(FileSystem.__module__).split('WebKi t', 1)[0]
3430 base_dir = ''.join((base_dir, 'WebKit/Source')) 3429 base_dir = ''.join((base_dir, 'WebKit/Source'))
3431 for root, dirs, names in os.walk(base_dir): 3430 for root, dirs, names in os.walk(base_dir):
3432 if filename in names: 3431 if filename in names:
3433 return os.path.join(root, filename) 3432 return os.path.join(root, filename)
3434 return None 3433 return None
3435 3434
3436 def grep(lines, pattern, error): 3435 def grep(lines, search_function_name, error):
3437 matches = [] 3436 matches = []
3438 function_state = None 3437 function_state = None
3439 for line_number in xrange(lines.num_lines()): 3438 for line_number in xrange(lines.num_lines()):
3440 line = (lines.elided[line_number]).rstrip() 3439 line = (lines.elided[line_number]).rstrip()
3441 try: 3440 try:
3442 if pattern in line: 3441 if re.search(_RE_GENERAL_MACRO_PATTERN_STRING, line):
3442 # Search for DEFINE_TYPE_CASTS macro pattern in the source h eader.
3443 matches.append([line, line_number + 1, line_number + 1])
3444 elif re.search(_RE_EXPLICIT_FUNCTION_PATTERN_STRING, line):
3445 # DEFINE_TYPE_CASTS macro pattern not found in source header .
3446 # Instead, search for toFoo() function pattern in source hea der.
3443 if not function_state: 3447 if not function_state:
3444 function_state = _FunctionState(1) 3448 function_state = _FunctionState(1)
3445 detect_functions(lines, line_number, function_state, error) 3449 detect_functions(lines, line_number, function_state, error)
3446 # Exclude the match of dummy conversion function. Dummy func tion is just to 3450 # Exclude the match of dummy conversion function. Dummy func tion is just to
3447 # catch invalid conversions and shouldn't be part of possibl e alternatives. 3451 # catch invalid conversions and shouldn't be part of possibl e alternatives.
3448 result = re.search(r'%s(\s+)%s' % ("void", pattern), line) 3452 result = re.search(r'%s(\s+)%s' % ("void", search_function_n ame), line)
3449 if not result: 3453 if not result:
3450 matches.append([line, function_state.body_start_position .row, function_state.end_position.row + 1]) 3454 matches.append([line, function_state.body_start_position .row, function_state.end_position.row + 1])
3451 function_state = None 3455 function_state = None
3452 except UnicodeDecodeError: 3456 except UnicodeDecodeError:
3453 # There would be no non-ascii characters in the codebase ever. T he only exception 3457 # There would be no non-ascii characters in the codebase ever. T he only exception
3454 # would be comments/copyright text which might have non-ascii ch aracters. Hence, 3458 # would be comments/copyright text which might have non-ascii ch aracters. Hence,
3455 # it is prefectly safe to catch the UnicodeDecodeError and just pass the line. 3459 # it is prefectly safe to catch the UnicodeDecodeError and just pass the line.
3456 pass 3460 pass
3457 3461
3458 return matches 3462 return matches
(...skipping 25 matching lines...) Expand all
3484 file_path = get_abs_filepath(filename) 3488 file_path = get_abs_filepath(filename)
3485 if not file_path: 3489 if not file_path:
3486 return None 3490 return None
3487 try: 3491 try:
3488 f = open(file_path) 3492 f = open(file_path)
3489 clean_lines = CleansedLines(f.readlines()) 3493 clean_lines = CleansedLines(f.readlines())
3490 finally: 3494 finally:
3491 f.close() 3495 f.close()
3492 3496
3493 # Make a list of all genuine alternatives to static_cast. 3497 # Make a list of all genuine alternatives to static_cast.
3494 matches = grep(clean_lines, pattern, error) 3498 matches = grep(clean_lines, search_function_name, error)
3495 return matches 3499 return matches
3496 3500
3497 3501
3498 def check_for_object_static_cast(processing_file, line_number, line, error): 3502 def check_for_object_static_cast(processing_file, line_number, line, error):
3499 """Checks for a Cpp-style static cast on objects by looking for the pattern. 3503 """ Reports for using static_cast instead of toFoo convenience function.
3504
3505 This function will output warnings to make sure you are actually using
3506 the added toFoo conversion functions rather than directly hard coding
3507 the static_cast<Classname*> call. For example, you should toHTMLELement(Node *)
3508 to convert Node* to HTMLElement*, instead of static_cast<HTMLElement*>(Node* )
3500 3509
3501 Args: 3510 Args:
3502 processing_file: The name of the processing file. 3511 processing_file: The name of the processing file.
3503 line_number: The number of the line to check. 3512 line_number: The number of the line to check.
3504 line: The line of code to check. 3513 line: The line of code to check.
3505 error: The function to call with any errors found. 3514 error: The function to call with any errors found.
3506 """ 3515 """
3516
3507 matched = search(r'\bstatic_cast<(\s*\w*:?:?\w+\s*\*+\s*)>', line) 3517 matched = search(r'\bstatic_cast<(\s*\w*:?:?\w+\s*\*+\s*)>', line)
3508 if not matched: 3518 if not matched:
3509 return 3519 return
3510 3520
3511 class_name = re.sub('[\*]', '', matched.group(1)) 3521 class_name = re.sub('[\*]', '', matched.group(1))
3512 class_name = class_name.strip() 3522 class_name = class_name.strip()
3513 # Ignore (for now) when the casting is to void*, 3523 # Ignore (for now) when the casting is to void*,
3514 if class_name == 'void': 3524 if class_name == 'void':
3515 return 3525 return
3516 3526
3517 namespace_pos = class_name.find(':') 3527 namespace_pos = class_name.find(':')
3518 if not namespace_pos == -1: 3528 if not namespace_pos == -1:
3519 class_name = class_name[namespace_pos + 2:] 3529 class_name = class_name[namespace_pos + 2:]
3520 3530
3521 header_file = ''.join((class_name, '.h')) 3531 header_file = ''.join((class_name, '.h'))
3522 matches = check_for_toFoo_definition(header_file, ''.join(('to', class_name) ), error) 3532 toFoo_name = ''.join(('to', class_name))
3523 # Ignore (for now) if not able to find the header where toFoo might be defin ed. 3533 _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
3524 # TODO: Handle cases where Classname might be defined in some other header o r cpp file. 3534 _RE_EXPLICIT_FUNCTION_PATTERN_STRING = re.compile(r'[ \w]*%s\*\s+%s\((.+)\)$ ' % (class_name, toFoo_name))
3535 matches = check_for_toFoo_definition(header_file, toFoo_name, error)
3536
3537 # FIXME: Handle cases where Classname might be defined in some other header or cpp file.
3538 # It will ensure check_for_toFoo_definition nevers 'None' and we can remove this unneccessary check.
3525 if matches is None: 3539 if matches is None:
3526 return 3540 return
3527 3541
3528 report_error = True 3542 report_error = True
3529 # Ensure found static_cast instance is not from within toFoo definition itse lf. 3543 # Ensure found static_cast instance is not from within toFoo definition itse lf.
3530 if (os.path.basename(processing_file) == header_file): 3544 if (os.path.basename(processing_file) == header_file):
3531 for item in matches: 3545 for item in matches:
3532 if line_number in range(item[1], item[2]): 3546 if line_number in range(item[1], item[2]):
3533 report_error = False 3547 report_error = False
3534 break 3548 break
3535 3549
3536 if report_error: 3550 if report_error:
3537 if len(matches): 3551 if len(matches):
3538 # toFoo is defined - enforce using it. 3552 # toFoo is defined - enforce using it.
3539 # TODO: Suggest an appropriate toFoo from the alternatives present i n matches. 3553 # FIXME: Suggest an appropriate toFoo from the alternatives present in matches.
3540 error(line_number, 'runtime/casting', 4, 3554 error(line_number, 'security/casting', 4,
3541 'static_cast of class objects is not allowed. Use to%s defined in %s.' % 3555 'static_cast of class objects is not allowed. Use to%s defined in %s.' %
3542 (class_name, header_file)) 3556 (class_name, header_file))
3543 else: 3557 else:
3544 # No toFoo defined - enforce definition & usage. 3558 # No toFoo defined - enforce definition & usage.
3545 # TODO: Automate the generation of toFoo() to avoid any slippages ev er.
3546 error(line_number, 'runtime/casting', 4, 3559 error(line_number, 'runtime/casting', 4,
3547 'static_cast of class objects is not allowed. Add to%s in %s a nd use it instead.' % 3560 'static_cast of class objects is not allowed. Define cast macr o DEFINE_TYPE_CASTS(%s) in %s and use it.' %
3548 (class_name, header_file)) 3561 (class_name, header_file))
3549 3562
3550 3563
3551 def check_c_style_cast(line_number, line, raw_line, cast_type, pattern, 3564 def check_c_style_cast(line_number, line, raw_line, cast_type, pattern,
3552 error): 3565 error):
3553 """Checks for a C-style cast by looking for the pattern. 3566 """Checks for a C-style cast by looking for the pattern.
3554 3567
3555 This also handles sizeof(type) warnings, due to similarity of content. 3568 This also handles sizeof(type) warnings, due to similarity of content.
3556 3569
3557 Args: 3570 Args:
(...skipping 419 matching lines...) Expand 10 before | Expand all | Expand 10 after
3977 'runtime/memset', 3990 'runtime/memset',
3978 'runtime/printf', 3991 'runtime/printf',
3979 'runtime/printf_format', 3992 'runtime/printf_format',
3980 'runtime/references', 3993 'runtime/references',
3981 'runtime/rtti', 3994 'runtime/rtti',
3982 'runtime/sizeof', 3995 'runtime/sizeof',
3983 'runtime/string', 3996 'runtime/string',
3984 'runtime/threadsafe_fn', 3997 'runtime/threadsafe_fn',
3985 'runtime/unsigned', 3998 'runtime/unsigned',
3986 'runtime/virtual', 3999 'runtime/virtual',
4000 'security/casting',
3987 'whitespace/blank_line', 4001 'whitespace/blank_line',
3988 'whitespace/braces', 4002 'whitespace/braces',
3989 'whitespace/comma', 4003 'whitespace/comma',
3990 'whitespace/comments', 4004 'whitespace/comments',
3991 'whitespace/declaration', 4005 'whitespace/declaration',
3992 'whitespace/end_of_line', 4006 'whitespace/end_of_line',
3993 'whitespace/ending_newline', 4007 'whitespace/ending_newline',
3994 'whitespace/indent', 4008 'whitespace/indent',
3995 'whitespace/line_length', 4009 'whitespace/line_length',
3996 'whitespace/newline', 4010 'whitespace/newline',
(...skipping 42 matching lines...) Expand 10 before | Expand all | Expand 10 after
4039 4053
4040 def check(self, lines): 4054 def check(self, lines):
4041 _process_lines(self.file_path, self.file_extension, lines, 4055 _process_lines(self.file_path, self.file_extension, lines,
4042 self.handle_style_error, self.min_confidence) 4056 self.handle_style_error, self.min_confidence)
4043 4057
4044 4058
4045 # FIXME: Remove this function (requires refactoring unit tests). 4059 # FIXME: Remove this function (requires refactoring unit tests).
4046 def process_file_data(filename, file_extension, lines, error, min_confidence, fs =None): 4060 def process_file_data(filename, file_extension, lines, error, min_confidence, fs =None):
4047 checker = CppChecker(filename, file_extension, error, min_confidence, fs) 4061 checker = CppChecker(filename, file_extension, error, min_confidence, fs)
4048 checker.check(lines) 4062 checker.check(lines)
OLDNEW
« 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