Add code style check error for using static_cast instead of toFoo function.

Tools/Scripts/webkitpy/style/checkers/cpp.py

 # -*- coding: utf-8 -*-
 #
 # Copyright (C) 2009, 2010, 2012 Google Inc. All rights reserved.
 # Copyright (C) 2009 Torch Mobile Inc.
 # Copyright (C) 2009 Apple Inc. All rights reserved.
 # Copyright (C) 2010 Chris Jerdonek (cjerdonek@webkit.org)
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
 # met:
@@ skipping 3266 matching lines @@
         error(line_number, 'runtime/bitfields', 5,
               'Please declare integral type bitfields with either signed or unsigned.')
 
     check_identifier_name_in_declaration(filename, line_number, line, file_state, error)
 
     # Check for unsigned int (should be just 'unsigned')
     if search(r'\bunsigned int\b', line):
         error(line_number, 'runtime/unsigned', 1,
               'Omit int when using unsigned')
 
-    # Check that we're not using static_cast<Text*>.
-    if search(r'\bstatic_cast<Text\*>', line):
-        error(line_number, 'readability/check', 4,
-              'Consider using toText helper function in WebCore/dom/Text.h '
-              'instead of static_cast<Text*>')
+    # Check whether using static_cast<Classname*>. If yes, suggest to use toClassname().
+    check_for_object_static_cast(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
     underscores, we only warn about the declarations of identifiers
     and don't check use of identifiers.
 
     Args:
       filename: The name of the current file.
@@ skipping 112 matching lines @@
             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_for_toFoo_definition(filename, pattern, error):
+    """ 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:
+      filename: The name of the file.
+      pattern: The conversion function pattern to grep for.
+      error: The function to call with any errors found.
+    """
+
+    def get_abs_filepath(filename, subdirectory=None):
+        if subdirectory:
+            start_dir = subdirectory
+        else:
+            start_dir = os.path.dirname(os.path.abspath(filename))
+
+        for root, dirs, names in os.walk(start_dir):
+            if filename in names:
+                return os.path.join(root, filename)
+        return filename
+
+    def grep(filename, pattern):
+        # List of all geninuine alternatives to static_cast.

[Dirk Pranke, 2013/09/17 01:54:55]

Nit: typo, "genuine".

[r.kasibhatla, 2013/09/18 09:12:41]

Done.

+        matches = []
+        for n, line in enumerate(open(filename)):
+            if pattern in line:
+                # We should exclude the match for dummy conversion function.
+                # The dummy conversion function is to catch invalid conversions
+                # and shouldn't be part of listing possible alternatives.
+                #r = re.compile(r'%s(\s+)%s(\(\w+)%s(\*+\))' % ("void", pattern, object_class))

[Dirk Pranke, 2013/09/17 01:54:55]

Delete this commented-out line?

[r.kasibhatla, 2013/09/18 09:12:41]

Done.

+                result = re.search(r'%s(\s+)%s' % ("void", pattern), line)
+                if not result:
+                    matches.append([line, n])
+        return matches
+
+    matches = grep(get_abs_filepath(filename), pattern)
+    if matches:
+        return True
+
+    return False
+
+
+def check_for_object_static_cast(line_number, line, error):
+    """Checks for a C-style cast by looking for the pattern.
+    Args:
+      line_number: The number of the line to check.
+      line: The line of code to check.
+      raw_line: The raw line of code to check, with comments.
+      error: The function to call with any errors found.
+    """
+    matched = search(r'\bstatic_cast<(\w+\s?\*+\s?)>', line)

[inferno, 2013/09/16 16:56:46]

I think you should cover other variants like static_pointer_cast as well. 
You are not checking if the static_cast is part of a toFoo* function. In that
case, it will show a wrong error.

[r.kasibhatla, 2013/09/18 09:12:41]

I was looking at including static_pointer_cast in the style check, but I am not
sure how do we enforce it. static_pointer_cast is defined only as part of
templates RefPtr, PassRefPtr, OwnPtr & PassOwnPtr and it returns a RefPtr,
PassRefPtr, OwnPtr or PassOwnPtr respectively. Unlike static_cast which returns
separate value for each toFoo instance, static_pointer_cast always returns one
of the 4 types which the template parameter changing. So, I am not sure if we
raise the flag on static_pointer_cast, how would the user define its
corresponding toFoo function. Any suggestions?
Done.

+    if not matched:
+        return
+
+    # We have atleast one hit for static_cast<Classname> pattern.
+    class_object = matched.group(1)
+    class_name = re.sub('[\*]', '', class_object)
+    header = ''.join((class_name, '.h'))
+    is_func_defined = check_for_toFoo_definition(header, ''.join(('to', class_name)), error)
+
+    if is_func_defined:

[inferno, 2013/09/16 16:56:46]

We are showing the error only if toFoo* function is defined. Atleast 50% of the
times, people don't use toFoo function because it is not defined. So, we want to
encourage them to define it if it does not exist already.

[r.kasibhatla, 2013/09/18 09:12:41]

Done.

+        error(line_number, 'readability/check', 4,
+              'Consider using to%s helper function in %s instead of static_cast<%s>' %

[inferno, 2013/09/16 16:56:46]

change "Consider using" to a more stronger message. We don't want to allow this
pattern at all.

[r.kasibhatla, 2013/09/18 09:12:41]

Done.

+              (class_name, header, class_object))
+
+
 def check_c_style_cast(line_number, line, raw_line, cast_type, pattern,
                        error):
     """Checks for a C-style cast by looking for the pattern.
 
     This also handles sizeof(type) warnings, due to similarity of content.
 
     Args:
       line_number: The number of the line to check.
       line: The line of code to check.
       raw_line: The raw line of code to check, with comments.
@@ skipping 479 matching lines @@
                        self.handle_style_error, self.min_confidence)
 
 
 # FIXME: Remove this function (requires refactoring unit tests).
 def process_file_data(filename, file_extension, lines, error, min_confidence, unit_test_config):
     global _unit_test_config
     _unit_test_config = unit_test_config
     checker = CppChecker(filename, file_extension, error, min_confidence)
     checker.check(lines)
     _unit_test_config = {}