Skip to content

Commit 3d2d04a

Browse files
calcmogulcalcmogul
committed
Added task for removing redundant "virtual" specifier instances
1 parent f38b743 commit 3d2d04a

File tree

4 files changed

+172
-95
lines changed

4 files changed

+172
-95
lines changed

‎wpiformat/wpiformat/__init__.py

+2
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
from wpiformat.task import Task
2424
from wpiformat.usingdeclaration import UsingDeclaration
2525
from wpiformat.usingnamespacestd import UsingNamespaceStd
26+
from wpiformat.virtualspecifier import VirtualSpecifier
2627
from wpiformat.whitespace import Whitespace
2728

2829

@@ -351,6 +352,7 @@ def main():
351352
IncludeOrder(),
352353
UsingDeclaration(),
353354
UsingNamespaceStd(),
355+
VirtualSpecifier(),
354356
Whitespace(),
355357
ClangFormat(args.clang_version),
356358
Jni(), # Fixes clang-format formatting

‎wpiformat/wpiformat/cpplint.py

-95
Original file line numberDiff line numberDiff line change
@@ -3446,99 +3446,6 @@ def CheckMakePairUsesDeduction(filename, clean_lines, linenum, error):
34463446
' OR use pair directly OR if appropriate, construct a pair directly')
34473447

34483448

3449-
def CheckRedundantVirtual(filename, clean_lines, linenum, error):
3450-
"""Check if line contains a redundant "virtual" function-specifier.
3451-
3452-
Args:
3453-
filename: The name of the current file.
3454-
clean_lines: A CleansedLines instance containing the file.
3455-
linenum: The number of the line to check.
3456-
error: The function to call with any errors found.
3457-
"""
3458-
# Look for "virtual" on current line.
3459-
line = clean_lines.elided[linenum]
3460-
virtual = Match(r'^(.*)(\bvirtual\b)(.*)$', line)
3461-
if not virtual: return
3462-
3463-
# Ignore "virtual" keywords that are near access-specifiers. These
3464-
# are only used in class base-specifier and do not apply to member
3465-
# functions.
3466-
if (Search(r'\b(public|protected|private)\s+$', virtual.group(1)) or
3467-
Match(r'^\s+(public|protected|private)\b', virtual.group(3))):
3468-
return
3469-
3470-
# Ignore the "virtual" keyword from virtual base classes. Usually
3471-
# there is a column on the same line in these cases (virtual base
3472-
# classes are rare in google3 because multiple inheritance is rare).
3473-
if Match(r'^.*[^:]:[^:].*$', line): return
3474-
3475-
# Look for the next opening parenthesis. This is the start of the
3476-
# parameter list (possibly on the next line shortly after virtual).
3477-
# TODO(unknown): doesn't work if there are virtual functions with
3478-
# decltype() or other things that use parentheses, but csearch suggests
3479-
# that this is rare.
3480-
end_col = -1
3481-
end_line = -1
3482-
start_col = len(virtual.group(2))
3483-
for start_line in range(linenum, min(linenum + 3, clean_lines.NumLines())):
3484-
line = clean_lines.elided[start_line][start_col:]
3485-
parameter_list = Match(r'^([^(]*)\(', line)
3486-
if parameter_list:
3487-
# Match parentheses to find the end of the parameter list
3488-
(_, end_line, end_col) = CloseExpression(
3489-
clean_lines, start_line, start_col + len(parameter_list.group(1)))
3490-
break
3491-
start_col = 0
3492-
3493-
if end_col < 0:
3494-
return # Couldn't find end of parameter list, give up
3495-
3496-
# Look for "override" or "final" after the parameter list
3497-
# (possibly on the next few lines).
3498-
for i in range(end_line, min(end_line + 3, clean_lines.NumLines())):
3499-
line = clean_lines.elided[i][end_col:]
3500-
match = Search(r'\b(override|final)\b', line)
3501-
if match:
3502-
error(filename, linenum, 'readability/inheritance', 4,
3503-
('"virtual" is redundant since function is '
3504-
'already declared as "%s"' % match.group(1)))
3505-
3506-
# Set end_col to check whole lines after we are done with the
3507-
# first line.
3508-
end_col = 0
3509-
if Search(r'[^\w]\s*$', line):
3510-
break
3511-
3512-
3513-
def CheckRedundantOverrideOrFinal(filename, clean_lines, linenum, error):
3514-
"""Check if line contains a redundant "override" or "final" virt-specifier.
3515-
3516-
Args:
3517-
filename: The name of the current file.
3518-
clean_lines: A CleansedLines instance containing the file.
3519-
linenum: The number of the line to check.
3520-
error: The function to call with any errors found.
3521-
"""
3522-
# Look for closing parenthesis nearby. We need one to confirm where
3523-
# the declarator ends and where the virt-specifier starts to avoid
3524-
# false positives.
3525-
line = clean_lines.elided[linenum]
3526-
declarator_end = line.rfind(')')
3527-
if declarator_end >= 0:
3528-
fragment = line[declarator_end:]
3529-
else:
3530-
if linenum > 1 and clean_lines.elided[linenum - 1].rfind(')') >= 0:
3531-
fragment = line
3532-
else:
3533-
return
3534-
3535-
# Check that at most one of "override" or "final" is present, not both
3536-
if Search(r'\boverride\b', fragment) and Search(r'\bfinal\b', fragment):
3537-
error(filename, linenum, 'readability/inheritance', 4,
3538-
('"override" is redundant since function is '
3539-
'already declared as "final"'))
3540-
3541-
35423449
# Returns true if we are at a new block, and it is directly
35433450
# inside of a namespace.
35443451
def IsBlockInNameSpace(nesting_state, is_forward_declaration):
@@ -3590,8 +3497,6 @@ def ProcessLine(filename, is_header, clean_lines, line,
35903497
nesting_state, error)
35913498
CheckInvalidIncrement(filename, clean_lines, line, error)
35923499
CheckMakePairUsesDeduction(filename, clean_lines, line, error)
3593-
CheckRedundantVirtual(filename, clean_lines, line, error)
3594-
CheckRedundantOverrideOrFinal(filename, clean_lines, line, error)
35953500

35963501

35973502
def ProcessFileData(filename, is_header, lines, error):

‎wpiformat/wpiformat/test/test_virtualspecifier.py

+94
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
import os
2+
3+
from .tasktest import *
4+
from wpiformat.virtualspecifier import VirtualSpecifier
5+
6+
7+
def test_virtualspecifier():
8+
test = TaskTest(VirtualSpecifier())
9+
10+
# Redundant virtual specifier on function
11+
test.add_input("./PWM.h",
12+
"class PWM : public SendableBase {" + os.linesep + \
13+
" public:" + os.linesep + \
14+
" explicit PWM(int channel);" + os.linesep + \
15+
" ~PWM() override;" + os.linesep + \
16+
os.linesep + \
17+
" protected:" + os.linesep + \
18+
" virtual void InitSendable(SendableBuilder& builder) override;" + os.linesep + \
19+
"};" + os.linesep)
20+
test.add_output(
21+
"class PWM : public SendableBase {" + os.linesep + \
22+
" public:" + os.linesep + \
23+
" explicit PWM(int channel);" + os.linesep + \
24+
" ~PWM() override;" + os.linesep + \
25+
os.linesep + \
26+
" protected:" + os.linesep + \
27+
" void InitSendable(SendableBuilder& builder) override;" + os.linesep + \
28+
"};" + os.linesep, True, True)
29+
30+
# Redundant virtual specifier on const function
31+
test.add_input("./PIDController.h",
32+
"class PIDController : public PIDInterface {" + os.linesep + \
33+
" public:" + os.linesep + \
34+
" virtual double GetP() const override;" + os.linesep + \
35+
" virtual double GetI() const override;" + os.linesep + \
36+
" virtual double GetD() const override;" + os.linesep + \
37+
"};" + os.linesep)
38+
test.add_output(
39+
"class PIDController : public PIDInterface {" + os.linesep + \
40+
" public:" + os.linesep + \
41+
" double GetP() const override;" + os.linesep + \
42+
" double GetI() const override;" + os.linesep + \
43+
" double GetD() const override;" + os.linesep + \
44+
"};" + os.linesep, True, True)
45+
46+
# Redundant final specifier on const function
47+
test.add_input("./PIDController.h",
48+
"class PIDController : public PIDInterface {" + os.linesep + \
49+
" public:" + os.linesep + \
50+
" double GetP() const override;" + os.linesep + \
51+
" double GetI() const final override;" + os.linesep + \
52+
" double GetD() const override final;" + os.linesep + \
53+
"};" + os.linesep)
54+
test.add_output(
55+
"class PIDController : public PIDInterface {" + os.linesep + \
56+
" public:" + os.linesep + \
57+
" double GetP() const override;" + os.linesep + \
58+
" double GetI() const final;" + os.linesep + \
59+
" double GetD() const final;" + os.linesep + \
60+
"};" + os.linesep, True, True)
61+
62+
# Redundant virtual specifier on destructor
63+
test.add_input("./PWM.h",
64+
"class PWM : public SendableBase {" + os.linesep + \
65+
" public:" + os.linesep + \
66+
" explicit PWM(int channel);" + os.linesep + \
67+
" virtual ~PWM() override;" + os.linesep + \
68+
os.linesep + \
69+
" virtual void SetRaw(uint16_t value);" + os.linesep + \
70+
"};" + os.linesep)
71+
test.add_output(
72+
"class PWM : public SendableBase {" + os.linesep + \
73+
" public:" + os.linesep + \
74+
" explicit PWM(int channel);" + os.linesep + \
75+
" ~PWM() override;" + os.linesep + \
76+
os.linesep + \
77+
" virtual void SetRaw(uint16_t value);" + os.linesep + \
78+
"};" + os.linesep, True, True)
79+
80+
# Idempotence with virtual specifier on destructor
81+
test.add_input("./PWM.h",
82+
"class PWM : public SendableBase {" + os.linesep + \
83+
" public:" + os.linesep + \
84+
" explicit PWM(int channel);" + os.linesep + \
85+
" virtual ~PWM();" + os.linesep + \
86+
"};" + os.linesep)
87+
test.add_output(
88+
"class PWM : public SendableBase {" + os.linesep + \
89+
" public:" + os.linesep + \
90+
" explicit PWM(int channel);" + os.linesep + \
91+
" virtual ~PWM();" + os.linesep + \
92+
"};" + os.linesep, False, True)
93+
94+
test.run(OutputType.FILE)

‎wpiformat/wpiformat/virtualspecifier.py

+76
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
"""This task removes redundant "virtual" specifier instances.
2+
3+
When the "override" specifier is specified in a C++ function signature, the
4+
"virtual" specifier is redundant because "override" implies "virtual".
5+
"""
6+
7+
import regex
8+
9+
from wpiformat.task import Task
10+
11+
12+
class VirtualSpecifier(Task):
13+
14+
def should_process_file(self, config_file, name):
15+
return config_file.is_cpp_file(name)
16+
17+
def run_pipeline(self, config_file, name, lines):
18+
linesep = Task.get_linesep(lines)
19+
20+
file_changed = False
21+
output = ""
22+
23+
# Function signature parts
24+
virtual_spec = r"(?P<virtual>virtual\s+)?"
25+
return_type = r"(?P<return_type>[\w,\*&]+\s+)"
26+
func_args = r"(?P<func_args>\([^\)]*\)(\s*const)?)"
27+
specifiers = r"(?P<specifiers>[^;{]*)?"
28+
29+
# Construct regexes for function signatures
30+
regexes = []
31+
regexes.append(
32+
regex.compile(virtual_spec + r"(?P<func_sig>" + return_type +
33+
r"(?P<func_name>\w+\s*)" + func_args + ")" +
34+
specifiers))
35+
regexes.append(
36+
regex.compile(virtual_spec + r"(?P<func_sig>" +
37+
r"(?P<func_name>[\w~]+\s*)" + func_args + ")" +
38+
specifiers))
39+
40+
for rgx in regexes:
41+
pos = 0
42+
for match in rgx.finditer(lines):
43+
# Append lines before match
44+
output += lines[pos:match.start()]
45+
46+
# Append virtual specifier if it's not redundant
47+
if "final" not in match.group(
48+
"specifiers") and "override" not in match.group(
49+
"specifiers"):
50+
if match.group("virtual"):
51+
output += match.group("virtual")
52+
else:
53+
file_changed = True
54+
output += match.group("func_sig")
55+
56+
# Strip redundant specifiers
57+
specifiers_in = match.group("specifiers")
58+
specifiers_out = specifiers_in
59+
if "final" in specifiers_out:
60+
specifiers_out = regex.sub(r"\s+override", "",
61+
specifiers_out)
62+
if specifiers_in != specifiers_out:
63+
file_changed = True
64+
output += specifiers_out
65+
66+
pos = match.end()
67+
68+
# Append leftover lines in file
69+
if pos < len(lines):
70+
output += lines[pos:]
71+
72+
# Reset line buffer for next regex
73+
lines = output
74+
output = ""
75+
76+
return (lines, file_changed, True)

0 commit comments

Comments
 (0)