[webkit-changes] [197567] trunk/Tools

Fri, 04 Mar 2016 09:58:55 -0800

Title: [197567] trunk/Tools
Revision
197567
Author
[email protected]
Date
2016-03-04 09:58:51 -0800 (Fri, 04 Mar 2016)

Log Message

[check-webkit-style] Added checks for redundant virtual specifiers.
https://bugs.webkit.org/show_bug.cgi?id=155017

Patch by Konstantin Tokarev <[email protected]> on 2016-03-04
Reviewed by Darin Adler.

Added 3 new checks related to virtual, override, and final specifiers:

1. When "override" is present, "virtual" is redundant.
2. When "final" is present, "virtual" is redundant.
3. When "final" is present, "override" is redundant.

* Scripts/webkitpy/style/checkers/cpp.py:
(_FunctionState.begin):
(_FunctionState.is_virtual):
(_check_parameter_name_against_text):
(_error_redundant_specifier):
(check_function_definition):
(CppChecker):
* Scripts/webkitpy/style/checkers/cpp_unittest.py:
(FunctionDetectionTest.perform_function_detection):
(FunctionDetectionTest.test_basic_function_detection):
(FunctionDetectionTest.test_function_declaration_detection):
(FunctionDetectionTest.test_pure_function_detection):
(FunctionDetectionTest.test_override_and_final_function_detection):
(FunctionDetectionTest.test_non_functions):
(FunctionDetectionTest.test_parameter_list):

Modified Paths

Diff

Modified: trunk/Tools/ChangeLog (197566 => 197567)


--- trunk/Tools/ChangeLog	2016-03-04 17:20:46 UTC (rev 197566)
+++ trunk/Tools/ChangeLog	2016-03-04 17:58:51 UTC (rev 197567)
@@ -1,3 +1,32 @@
+2016-03-04  Konstantin Tokarev  <[email protected]>
+
+        [check-webkit-style] Added checks for redundant virtual specifiers.
+        https://bugs.webkit.org/show_bug.cgi?id=155017
+
+        Reviewed by Darin Adler.
+
+        Added 3 new checks related to virtual, override, and final specifiers:
+
+        1. When "override" is present, "virtual" is redundant.
+        2. When "final" is present, "virtual" is redundant.
+        3. When "final" is present, "override" is redundant.
+
+        * Scripts/webkitpy/style/checkers/cpp.py:
+        (_FunctionState.begin):
+        (_FunctionState.is_virtual):
+        (_check_parameter_name_against_text):
+        (_error_redundant_specifier):
+        (check_function_definition):
+        (CppChecker):
+        * Scripts/webkitpy/style/checkers/cpp_unittest.py:
+        (FunctionDetectionTest.perform_function_detection):
+        (FunctionDetectionTest.test_basic_function_detection):
+        (FunctionDetectionTest.test_function_declaration_detection):
+        (FunctionDetectionTest.test_pure_function_detection):
+        (FunctionDetectionTest.test_override_and_final_function_detection):
+        (FunctionDetectionTest.test_non_functions):
+        (FunctionDetectionTest.test_parameter_list):
+
 2016-03-03  Darin Adler  <[email protected]>
 
         Followup to:

Modified: trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py (197566 => 197567)


--- trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py	2016-03-04 17:20:46 UTC (rev 197566)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py	2016-03-04 17:58:51 UTC (rev 197567)
@@ -529,10 +529,13 @@
         self.is_declaration = clean_lines.elided[body_start_position.row][body_start_position.column] == ';'
         self.parameter_start_position = parameter_start_position
         self.parameter_end_position = parameter_end_position
+        self.is_final = False
+        self.is_override = False
         self.is_pure = False
-        if self.is_declaration:
-            characters_after_parameters = SingleLineView(clean_lines.elided, parameter_end_position, body_start_position).single_line
-            self.is_pure = bool(match(r'\s*=\s*0\s*', characters_after_parameters))
+        characters_after_parameters = SingleLineView(clean_lines.elided, parameter_end_position, body_start_position).single_line
+        self.is_final = bool(search(r'\bfinal\b', characters_after_parameters))
+        self.is_override = bool(search(r'\boverride\b', characters_after_parameters))
+        self.is_pure = bool(match(r'\s*=\s*0\s*', characters_after_parameters))
         self._clean_lines = clean_lines
         self._parameter_list = None
 
@@ -544,6 +547,9 @@
          start_modifiers = _rfind_in_lines(r';|\{|\}|((private|public|protected):)|(#.*)', elided, self.parameter_start_position, Position(0, 0))
          return SingleLineView(elided, start_modifiers, self.function_name_start_position).single_line.strip()
 
+    def is_virtual(self):
+        return bool(search(r'\bvirtual\b', self.modifiers_and_return_type()))
+
     def parameter_list(self):
         if not self._parameter_list:
             # Store the final result as a tuple since that is immutable.
@@ -1642,6 +1648,13 @@
         return False
     return True
 
+
+def _error_redundant_specifier(line_number, redundant_specifier, good_specifier, error):
+    error(line_number, 'readability/inheritance', 4,
+          '"%s" is redundant since function is already declared as "%s"'
+          % (redundant_specifier, good_specifier))
+
+
 def check_function_definition(filename, file_extension, clean_lines, line_number, function_state, error):
     """Check that function definitions for style issues.
 
@@ -1674,7 +1687,17 @@
         if not _check_parameter_name_against_text(parameter, parameter.type, error):
             continue  # Since an error was noted for this name, move to the next parameter.
 
+    if function_state.is_virtual():
+        if function_state.is_override:
+            _error_redundant_specifier(line_number, 'virtual', 'override', error)
 
+        if function_state.is_final:
+            _error_redundant_specifier(line_number, 'virtual', 'final', error)
+
+    if function_state.is_override and function_state.is_final:
+        _error_redundant_specifier(line_number, 'override', 'final', error)
+
+
 def check_for_leaky_patterns(clean_lines, line_number, function_state, error):
     """Check for constructs known to be leak prone.
     Args:
@@ -3811,6 +3834,7 @@
         'readability/enum_casing',
         'readability/fn_size',
         'readability/function',
+        'readability/inheritance',
         'readability/multiline_comment',
         'readability/multiline_string',
         'readability/parameter_name',

Modified: trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py (197566 => 197567)


--- trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py	2016-03-04 17:20:46 UTC (rev 197566)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py	2016-03-04 17:58:51 UTC (rev 197567)
@@ -374,7 +374,10 @@
         self.assertEqual(function_state.in_a_function, True)
         self.assertEqual(function_state.current_function, function_information['name'] + '()')
         self.assertEqual(function_state.modifiers_and_return_type(), function_information['modifiers_and_return_type'])
+        self.assertEqual(function_state.is_final, function_information['is_final'])
+        self.assertEqual(function_state.is_override, function_information['is_override'])
         self.assertEqual(function_state.is_pure, function_information['is_pure'])
+        self.assertEqual(function_state.is_virtual(), function_information['is_virtual'])
         self.assertEqual(function_state.is_declaration, function_information['is_declaration'])
         self.assert_positions_equal(function_state.function_name_start_position, function_information['function_name_start_position'])
         self.assert_positions_equal(function_state.parameter_start_position, function_information['parameter_start_position'])
@@ -403,6 +406,9 @@
              'parameter_end_position': (0, 29),
              'body_start_position': (0, 30),
              'end_position': (1, 1),
+             'is_final': False,
+             'is_override': False,
+             'is_virtual': False,
              'is_pure': False,
              'is_declaration': False})
 
@@ -416,6 +422,9 @@
              'parameter_end_position': (0, 23),
              'body_start_position': (0, 23),
              'end_position': (0, 24),
+             'is_final': False,
+             'is_override': False,
+             'is_virtual': False,
              'is_pure': False,
              'is_declaration': True})
 
@@ -428,6 +437,9 @@
              'parameter_end_position': (0, 76),
              'body_start_position': (0, 76),
              'end_position': (0, 77),
+             'is_final': False,
+             'is_override': False,
+             'is_virtual': False,
              'is_pure': False,
              'is_declaration': True})
 
@@ -440,6 +452,9 @@
              'parameter_end_position': (0, 76),
              'body_start_position': (0, 76),
              'end_position': (0, 77),
+             'is_final': False,
+             'is_override': False,
+             'is_virtual': False,
              'is_pure': False,
              'is_declaration': True})
 
@@ -452,6 +467,9 @@
              'parameter_end_position': (0, 77),
              'body_start_position': (0, 77),
              'end_position': (0, 78),
+             'is_final': False,
+             'is_override': False,
+             'is_virtual': False,
              'is_pure': False,
              'is_declaration': True})
 
@@ -464,6 +482,9 @@
              'parameter_end_position': (0, 76),
              'body_start_position': (0, 76),
              'end_position': (0, 77),
+             'is_final': False,
+             'is_override': False,
+             'is_virtual': False,
              'is_pure': False,
              'is_declaration': True})
 
@@ -477,6 +498,9 @@
              'parameter_end_position': (0, 41),
              'body_start_position': (0, 41),
              'end_position': (0, 42),
+             'is_final': False,
+             'is_override': False,
+             'is_virtual': True,
              'is_pure': False,
              'is_declaration': True})
 
@@ -489,6 +513,9 @@
              'parameter_end_position': (0, 37),
              'body_start_position': (0, 41),
              'end_position': (0, 42),
+             'is_final': False,
+             'is_override': False,
+             'is_virtual': True,
              'is_pure': True,
              'is_declaration': True})
 
@@ -504,9 +531,92 @@
              'parameter_end_position': (0, 37),
              'body_start_position': (2, 3),
              'end_position': (2, 4),
+             'is_final': False,
+             'is_override': False,
+             'is_virtual': True,
              'is_pure': True,
              'is_declaration': True})
 
+    def test_override_and_final_function_detection(self):
+        self.perform_function_detection(
+            ['void theTestFunctionName(int override, int final);'],
+            {'name': 'theTestFunctionName',
+             'modifiers_and_return_type': 'void',
+             'function_name_start_position': (0, 5),
+             'parameter_start_position': (0, 24),
+             'parameter_end_position': (0, 49),
+             'body_start_position': (0, 49),
+             'end_position': (0, 50),
+             'is_final': False,
+             'is_override': False,
+             'is_virtual': False,
+             'is_pure': False,
+             'is_declaration': True})
+
+        self.perform_function_detection(
+            ['void theTestFunctionName(int final) override;'],
+            {'name': 'theTestFunctionName',
+             'modifiers_and_return_type': 'void',
+             'function_name_start_position': (0, 5),
+             'parameter_start_position': (0, 24),
+             'parameter_end_position': (0, 35),
+             'body_start_position': (0, 44),
+             'end_position': (0, 45),
+             'is_final': False,
+             'is_override': True,
+             'is_virtual': False,
+             'is_pure': False,
+             'is_declaration': True})
+
+        self.perform_function_detection(
+            ['void theTestFunctionName(int final) const override',
+             '{',
+             '}'],
+            {'name': 'theTestFunctionName',
+             'modifiers_and_return_type': 'void',
+             'function_name_start_position': (0, 5),
+             'parameter_start_position': (0, 24),
+             'parameter_end_position': (0, 35),
+             'body_start_position': (1, 0),
+             'end_position': (2, 1),
+             'is_final': False,
+             'is_override': True,
+             'is_virtual': False,
+             'is_pure': False,
+             'is_declaration': False})
+
+        self.perform_function_detection(
+            ['virtual void theTestFunctionName(int override) final;'],
+            {'name': 'theTestFunctionName',
+             'modifiers_and_return_type': 'virtual void',
+             'function_name_start_position': (0, 13),
+             'parameter_start_position': (0, 32),
+             'parameter_end_position': (0, 46),
+             'body_start_position': (0, 52),
+             'end_position': (0, 53),
+             'is_final': True,
+             'is_override': False,
+             'is_virtual': True,
+             'is_pure': False,
+             'is_declaration': True})
+
+        self.perform_function_detection(
+            ['void theTestFunctionName()',
+             'override',
+             'final;'],
+            {'name': 'theTestFunctionName',
+             'modifiers_and_return_type': 'void',
+             'function_name_start_position': (0, 5),
+             'parameter_start_position': (0, 24),
+             'parameter_end_position': (0, 26),
+             'body_start_position': (2, 5),
+             'end_position': (2, 6),
+             'is_final': True,
+             'is_override': True,
+             'is_virtual': False,
+             'is_pure': False,
+             'is_declaration': True})
+
     def test_ignore_macros(self):
         self.perform_function_detection(['void aFunctionName(int); \\'], None)
 
@@ -525,6 +635,9 @@
              'parameter_end_position': (2, 1),
              'body_start_position': (2, 1),
              'end_position': (2, 2),
+             'is_final': False,
+             'is_override': False,
+             'is_virtual': False,
              'is_pure': False,
              'is_declaration': True})
 
@@ -542,6 +655,9 @@
              'parameter_end_position': (0, 19),
              'body_start_position': (0, 19),
              'end_position': (0, 20),
+             'is_final': False,
+             'is_override': False,
+             'is_virtual': False,
              'is_pure': False,
              'is_declaration': True,
              'parameter_list': ()})
@@ -556,6 +672,9 @@
              'parameter_end_position': (0, 22),
              'body_start_position': (0, 22),
              'end_position': (0, 23),
+             'is_final': False,
+             'is_override': False,
+             'is_virtual': False,
              'is_pure': False,
              'is_declaration': True,
              'parameter_list':
@@ -571,6 +690,9 @@
              'parameter_end_position': (0, 76),
              'body_start_position': (0, 76),
              'end_position': (0, 77),
+             'is_final': False,
+             'is_override': False,
+             'is_virtual': False,
              'is_pure': False,
              'is_declaration': True,
              'parameter_list':
@@ -589,6 +711,9 @@
              'parameter_end_position': (0, 147),
              'body_start_position': (0, 147),
              'end_position': (0, 148),
+             'is_final': False,
+             'is_override': False,
+             'is_virtual': True,
              'is_pure': False,
              'is_declaration': True,
              'parameter_list':
@@ -613,6 +738,9 @@
              'parameter_end_position': (5, 17),
              'body_start_position': (5, 17),
              'end_position': (5, 18),
+             'is_final': False,
+             'is_override': False,
+             'is_virtual': True,
              'is_pure': False,
              'is_declaration': True,
              'parameter_list':

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to