<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[197567] trunk/Tools</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/197567">197567</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2016-03-04 09:58:51 -0800 (Fri, 04 Mar 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>[check-webkit-style] Added checks for redundant virtual specifiers.
https://bugs.webkit.org/show_bug.cgi?id=155017
Patch by Konstantin Tokarev <annulen@yandex.ru> 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):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkToolsChangeLog">trunk/Tools/ChangeLog</a></li>
<li><a href="#trunkToolsScriptswebkitpystylecheckerscpppy">trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py</a></li>
<li><a href="#trunkToolsScriptswebkitpystylecheckerscpp_unittestpy">trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkToolsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Tools/ChangeLog (197566 => 197567)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/ChangeLog 2016-03-04 17:20:46 UTC (rev 197566)
+++ trunk/Tools/ChangeLog 2016-03-04 17:58:51 UTC (rev 197567)
</span><span class="lines">@@ -1,3 +1,32 @@
</span><ins>+2016-03-04 Konstantin Tokarev <annulen@yandex.ru>
+
+ [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):
+
</ins><span class="cx"> 2016-03-03 Darin Adler <darin@apple.com>
</span><span class="cx">
</span><span class="cx"> Followup to:
</span></span></pre></div>
<a id="trunkToolsScriptswebkitpystylecheckerscpppy"></a>
<div class="modfile"><h4>Modified: trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py (197566 => 197567)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -529,10 +529,13 @@
</span><span class="cx"> self.is_declaration = clean_lines.elided[body_start_position.row][body_start_position.column] == ';'
</span><span class="cx"> self.parameter_start_position = parameter_start_position
</span><span class="cx"> self.parameter_end_position = parameter_end_position
</span><ins>+ self.is_final = False
+ self.is_override = False
</ins><span class="cx"> self.is_pure = False
</span><del>- 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))
</del><ins>+ 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))
</ins><span class="cx"> self._clean_lines = clean_lines
</span><span class="cx"> self._parameter_list = None
</span><span class="cx">
</span><span class="lines">@@ -544,6 +547,9 @@
</span><span class="cx"> start_modifiers = _rfind_in_lines(r';|\{|\}|((private|public|protected):)|(#.*)', elided, self.parameter_start_position, Position(0, 0))
</span><span class="cx"> return SingleLineView(elided, start_modifiers, self.function_name_start_position).single_line.strip()
</span><span class="cx">
</span><ins>+ def is_virtual(self):
+ return bool(search(r'\bvirtual\b', self.modifiers_and_return_type()))
+
</ins><span class="cx"> def parameter_list(self):
</span><span class="cx"> if not self._parameter_list:
</span><span class="cx"> # Store the final result as a tuple since that is immutable.
</span><span class="lines">@@ -1642,6 +1648,13 @@
</span><span class="cx"> return False
</span><span class="cx"> return True
</span><span class="cx">
</span><ins>+
+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))
+
+
</ins><span class="cx"> def check_function_definition(filename, file_extension, clean_lines, line_number, function_state, error):
</span><span class="cx"> """Check that function definitions for style issues.
</span><span class="cx">
</span><span class="lines">@@ -1674,7 +1687,17 @@
</span><span class="cx"> if not _check_parameter_name_against_text(parameter, parameter.type, error):
</span><span class="cx"> continue # Since an error was noted for this name, move to the next parameter.
</span><span class="cx">
</span><ins>+ if function_state.is_virtual():
+ if function_state.is_override:
+ _error_redundant_specifier(line_number, 'virtual', 'override', error)
</ins><span class="cx">
</span><ins>+ 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)
+
+
</ins><span class="cx"> def check_for_leaky_patterns(clean_lines, line_number, function_state, error):
</span><span class="cx"> """Check for constructs known to be leak prone.
</span><span class="cx"> Args:
</span><span class="lines">@@ -3811,6 +3834,7 @@
</span><span class="cx"> 'readability/enum_casing',
</span><span class="cx"> 'readability/fn_size',
</span><span class="cx"> 'readability/function',
</span><ins>+ 'readability/inheritance',
</ins><span class="cx"> 'readability/multiline_comment',
</span><span class="cx"> 'readability/multiline_string',
</span><span class="cx"> 'readability/parameter_name',
</span></span></pre></div>
<a id="trunkToolsScriptswebkitpystylecheckerscpp_unittestpy"></a>
<div class="modfile"><h4>Modified: trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py (197566 => 197567)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -374,7 +374,10 @@
</span><span class="cx"> self.assertEqual(function_state.in_a_function, True)
</span><span class="cx"> self.assertEqual(function_state.current_function, function_information['name'] + '()')
</span><span class="cx"> self.assertEqual(function_state.modifiers_and_return_type(), function_information['modifiers_and_return_type'])
</span><ins>+ self.assertEqual(function_state.is_final, function_information['is_final'])
+ self.assertEqual(function_state.is_override, function_information['is_override'])
</ins><span class="cx"> self.assertEqual(function_state.is_pure, function_information['is_pure'])
</span><ins>+ self.assertEqual(function_state.is_virtual(), function_information['is_virtual'])
</ins><span class="cx"> self.assertEqual(function_state.is_declaration, function_information['is_declaration'])
</span><span class="cx"> self.assert_positions_equal(function_state.function_name_start_position, function_information['function_name_start_position'])
</span><span class="cx"> self.assert_positions_equal(function_state.parameter_start_position, function_information['parameter_start_position'])
</span><span class="lines">@@ -403,6 +406,9 @@
</span><span class="cx"> 'parameter_end_position': (0, 29),
</span><span class="cx"> 'body_start_position': (0, 30),
</span><span class="cx"> 'end_position': (1, 1),
</span><ins>+ 'is_final': False,
+ 'is_override': False,
+ 'is_virtual': False,
</ins><span class="cx"> 'is_pure': False,
</span><span class="cx"> 'is_declaration': False})
</span><span class="cx">
</span><span class="lines">@@ -416,6 +422,9 @@
</span><span class="cx"> 'parameter_end_position': (0, 23),
</span><span class="cx"> 'body_start_position': (0, 23),
</span><span class="cx"> 'end_position': (0, 24),
</span><ins>+ 'is_final': False,
+ 'is_override': False,
+ 'is_virtual': False,
</ins><span class="cx"> 'is_pure': False,
</span><span class="cx"> 'is_declaration': True})
</span><span class="cx">
</span><span class="lines">@@ -428,6 +437,9 @@
</span><span class="cx"> 'parameter_end_position': (0, 76),
</span><span class="cx"> 'body_start_position': (0, 76),
</span><span class="cx"> 'end_position': (0, 77),
</span><ins>+ 'is_final': False,
+ 'is_override': False,
+ 'is_virtual': False,
</ins><span class="cx"> 'is_pure': False,
</span><span class="cx"> 'is_declaration': True})
</span><span class="cx">
</span><span class="lines">@@ -440,6 +452,9 @@
</span><span class="cx"> 'parameter_end_position': (0, 76),
</span><span class="cx"> 'body_start_position': (0, 76),
</span><span class="cx"> 'end_position': (0, 77),
</span><ins>+ 'is_final': False,
+ 'is_override': False,
+ 'is_virtual': False,
</ins><span class="cx"> 'is_pure': False,
</span><span class="cx"> 'is_declaration': True})
</span><span class="cx">
</span><span class="lines">@@ -452,6 +467,9 @@
</span><span class="cx"> 'parameter_end_position': (0, 77),
</span><span class="cx"> 'body_start_position': (0, 77),
</span><span class="cx"> 'end_position': (0, 78),
</span><ins>+ 'is_final': False,
+ 'is_override': False,
+ 'is_virtual': False,
</ins><span class="cx"> 'is_pure': False,
</span><span class="cx"> 'is_declaration': True})
</span><span class="cx">
</span><span class="lines">@@ -464,6 +482,9 @@
</span><span class="cx"> 'parameter_end_position': (0, 76),
</span><span class="cx"> 'body_start_position': (0, 76),
</span><span class="cx"> 'end_position': (0, 77),
</span><ins>+ 'is_final': False,
+ 'is_override': False,
+ 'is_virtual': False,
</ins><span class="cx"> 'is_pure': False,
</span><span class="cx"> 'is_declaration': True})
</span><span class="cx">
</span><span class="lines">@@ -477,6 +498,9 @@
</span><span class="cx"> 'parameter_end_position': (0, 41),
</span><span class="cx"> 'body_start_position': (0, 41),
</span><span class="cx"> 'end_position': (0, 42),
</span><ins>+ 'is_final': False,
+ 'is_override': False,
+ 'is_virtual': True,
</ins><span class="cx"> 'is_pure': False,
</span><span class="cx"> 'is_declaration': True})
</span><span class="cx">
</span><span class="lines">@@ -489,6 +513,9 @@
</span><span class="cx"> 'parameter_end_position': (0, 37),
</span><span class="cx"> 'body_start_position': (0, 41),
</span><span class="cx"> 'end_position': (0, 42),
</span><ins>+ 'is_final': False,
+ 'is_override': False,
+ 'is_virtual': True,
</ins><span class="cx"> 'is_pure': True,
</span><span class="cx"> 'is_declaration': True})
</span><span class="cx">
</span><span class="lines">@@ -504,9 +531,92 @@
</span><span class="cx"> 'parameter_end_position': (0, 37),
</span><span class="cx"> 'body_start_position': (2, 3),
</span><span class="cx"> 'end_position': (2, 4),
</span><ins>+ 'is_final': False,
+ 'is_override': False,
+ 'is_virtual': True,
</ins><span class="cx"> 'is_pure': True,
</span><span class="cx"> 'is_declaration': True})
</span><span class="cx">
</span><ins>+ 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})
+
</ins><span class="cx"> def test_ignore_macros(self):
</span><span class="cx"> self.perform_function_detection(['void aFunctionName(int); \\'], None)
</span><span class="cx">
</span><span class="lines">@@ -525,6 +635,9 @@
</span><span class="cx"> 'parameter_end_position': (2, 1),
</span><span class="cx"> 'body_start_position': (2, 1),
</span><span class="cx"> 'end_position': (2, 2),
</span><ins>+ 'is_final': False,
+ 'is_override': False,
+ 'is_virtual': False,
</ins><span class="cx"> 'is_pure': False,
</span><span class="cx"> 'is_declaration': True})
</span><span class="cx">
</span><span class="lines">@@ -542,6 +655,9 @@
</span><span class="cx"> 'parameter_end_position': (0, 19),
</span><span class="cx"> 'body_start_position': (0, 19),
</span><span class="cx"> 'end_position': (0, 20),
</span><ins>+ 'is_final': False,
+ 'is_override': False,
+ 'is_virtual': False,
</ins><span class="cx"> 'is_pure': False,
</span><span class="cx"> 'is_declaration': True,
</span><span class="cx"> 'parameter_list': ()})
</span><span class="lines">@@ -556,6 +672,9 @@
</span><span class="cx"> 'parameter_end_position': (0, 22),
</span><span class="cx"> 'body_start_position': (0, 22),
</span><span class="cx"> 'end_position': (0, 23),
</span><ins>+ 'is_final': False,
+ 'is_override': False,
+ 'is_virtual': False,
</ins><span class="cx"> 'is_pure': False,
</span><span class="cx"> 'is_declaration': True,
</span><span class="cx"> 'parameter_list':
</span><span class="lines">@@ -571,6 +690,9 @@
</span><span class="cx"> 'parameter_end_position': (0, 76),
</span><span class="cx"> 'body_start_position': (0, 76),
</span><span class="cx"> 'end_position': (0, 77),
</span><ins>+ 'is_final': False,
+ 'is_override': False,
+ 'is_virtual': False,
</ins><span class="cx"> 'is_pure': False,
</span><span class="cx"> 'is_declaration': True,
</span><span class="cx"> 'parameter_list':
</span><span class="lines">@@ -589,6 +711,9 @@
</span><span class="cx"> 'parameter_end_position': (0, 147),
</span><span class="cx"> 'body_start_position': (0, 147),
</span><span class="cx"> 'end_position': (0, 148),
</span><ins>+ 'is_final': False,
+ 'is_override': False,
+ 'is_virtual': True,
</ins><span class="cx"> 'is_pure': False,
</span><span class="cx"> 'is_declaration': True,
</span><span class="cx"> 'parameter_list':
</span><span class="lines">@@ -613,6 +738,9 @@
</span><span class="cx"> 'parameter_end_position': (5, 17),
</span><span class="cx"> 'body_start_position': (5, 17),
</span><span class="cx"> 'end_position': (5, 18),
</span><ins>+ 'is_final': False,
+ 'is_override': False,
+ 'is_virtual': True,
</ins><span class="cx"> 'is_pure': False,
</span><span class="cx"> 'is_declaration': True,
</span><span class="cx"> 'parameter_list':
</span></span></pre>
</div>
</div>
</body>
</html>