Skip to content

Commit 45b2f14

Browse files
calcmogulcalcmogul
committed
Added task for removing redundant "virtual" specifier instances
1 parent 0831768 commit 45b2f14

File tree

4 files changed

+268
-95
lines changed

4 files changed

+268
-95
lines changed

‎wpiformat/wpiformat/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
from wpiformat.task import Task
2626
from wpiformat.usingdeclaration import UsingDeclaration
2727
from wpiformat.usingnamespacestd import UsingNamespaceStd
28+
from wpiformat.virtualspecifier import VirtualSpecifier
2829
from wpiformat.whitespace import Whitespace
2930

3031

@@ -479,6 +480,7 @@ def main():
479480
IncludeOrder(),
480481
UsingDeclaration(),
481482
UsingNamespaceStd(),
483+
VirtualSpecifier(),
482484
Whitespace(),
483485
ClangFormat(args.clang_version),
484486
Jni(), # Fixes clang-format formatting

‎wpiformat/wpiformat/cpplint.py

Lines changed: 0 additions & 95 deletions
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

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
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(
12+
"./PWM.h",
13+
"class PWM : public SendableBase {"
14+
+ os.linesep
15+
+ " public:"
16+
+ os.linesep
17+
+ " explicit PWM(int channel);"
18+
+ os.linesep
19+
+ " ~PWM() override;"
20+
+ os.linesep
21+
+ os.linesep
22+
+ " protected:"
23+
+ os.linesep
24+
+ " virtual void InitSendable(SendableBuilder& builder) override;"
25+
+ os.linesep
26+
+ "};"
27+
+ os.linesep,
28+
)
29+
test.add_output(
30+
"class PWM : public SendableBase {"
31+
+ os.linesep
32+
+ " public:"
33+
+ os.linesep
34+
+ " explicit PWM(int channel);"
35+
+ os.linesep
36+
+ " ~PWM() override;"
37+
+ os.linesep
38+
+ os.linesep
39+
+ " protected:"
40+
+ os.linesep
41+
+ " void InitSendable(SendableBuilder& builder) override;"
42+
+ os.linesep
43+
+ "};"
44+
+ os.linesep,
45+
True,
46+
True,
47+
)
48+
49+
# Redundant virtual specifier on const function
50+
test.add_input(
51+
"./PIDController.h",
52+
"class PIDController : public PIDInterface {"
53+
+ os.linesep
54+
+ " public:"
55+
+ os.linesep
56+
+ " virtual double GetP() const override;"
57+
+ os.linesep
58+
+ " virtual double GetI() const override;"
59+
+ os.linesep
60+
+ " virtual double GetD() const override;"
61+
+ os.linesep
62+
+ "};"
63+
+ os.linesep,
64+
)
65+
test.add_output(
66+
"class PIDController : public PIDInterface {"
67+
+ os.linesep
68+
+ " public:"
69+
+ os.linesep
70+
+ " double GetP() const override;"
71+
+ os.linesep
72+
+ " double GetI() const override;"
73+
+ os.linesep
74+
+ " double GetD() const override;"
75+
+ os.linesep
76+
+ "};"
77+
+ os.linesep,
78+
True,
79+
True,
80+
)
81+
82+
# Redundant final specifier on const function
83+
test.add_input(
84+
"./PIDController.h",
85+
"class PIDController : public PIDInterface {"
86+
+ os.linesep
87+
+ " public:"
88+
+ os.linesep
89+
+ " double GetP() const override;"
90+
+ os.linesep
91+
+ " double GetI() const final override;"
92+
+ os.linesep
93+
+ " double GetD() const override final;"
94+
+ os.linesep
95+
+ "};"
96+
+ os.linesep,
97+
)
98+
test.add_output(
99+
"class PIDController : public PIDInterface {"
100+
+ os.linesep
101+
+ " public:"
102+
+ os.linesep
103+
+ " double GetP() const override;"
104+
+ os.linesep
105+
+ " double GetI() const final;"
106+
+ os.linesep
107+
+ " double GetD() const final;"
108+
+ os.linesep
109+
+ "};"
110+
+ os.linesep,
111+
True,
112+
True,
113+
)
114+
115+
# Redundant virtual specifier on destructor
116+
test.add_input(
117+
"./PWM.h",
118+
"class PWM : public SendableBase {"
119+
+ os.linesep
120+
+ " public:"
121+
+ os.linesep
122+
+ " explicit PWM(int channel);"
123+
+ os.linesep
124+
+ " virtual ~PWM() override;"
125+
+ os.linesep
126+
+ os.linesep
127+
+ " virtual void SetRaw(uint16_t value);"
128+
+ os.linesep
129+
+ "};"
130+
+ os.linesep,
131+
)
132+
test.add_output(
133+
"class PWM : public SendableBase {"
134+
+ os.linesep
135+
+ " public:"
136+
+ os.linesep
137+
+ " explicit PWM(int channel);"
138+
+ os.linesep
139+
+ " ~PWM() override;"
140+
+ os.linesep
141+
+ os.linesep
142+
+ " virtual void SetRaw(uint16_t value);"
143+
+ os.linesep
144+
+ "};"
145+
+ os.linesep,
146+
True,
147+
True,
148+
)
149+
150+
# Idempotence with virtual specifier on destructor
151+
test.add_input(
152+
"./PWM.h",
153+
"class PWM : public SendableBase {"
154+
+ os.linesep
155+
+ " public:"
156+
+ os.linesep
157+
+ " explicit PWM(int channel);"
158+
+ os.linesep
159+
+ " virtual ~PWM();"
160+
+ os.linesep
161+
+ "};"
162+
+ os.linesep,
163+
)
164+
test.add_output(
165+
"class PWM : public SendableBase {"
166+
+ os.linesep
167+
+ " public:"
168+
+ os.linesep
169+
+ " explicit PWM(int channel);"
170+
+ os.linesep
171+
+ " virtual ~PWM();"
172+
+ os.linesep
173+
+ "};"
174+
+ os.linesep,
175+
False,
176+
True,
177+
)
178+
179+
test.run(OutputType.FILE)

‎wpiformat/wpiformat/virtualspecifier.py

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
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+
def should_process_file(self, config_file, name):
14+
return config_file.is_cpp_file(name)
15+
16+
def run_pipeline(self, config_file, name, lines):
17+
linesep = Task.get_linesep(lines)
18+
19+
file_changed = False
20+
output = ""
21+
22+
# Function signature parts
23+
virtual_spec = r"(?P<virtual>virtual\s+)?"
24+
return_type = r"(?P<return_type>[\w,\*&]+\s+)"
25+
func_args = r"(?P<func_args>\([^\)]*\)(\s*const)?)"
26+
specifiers = r"(?P<specifiers>[^;{]*)?"
27+
28+
# Construct regexes for function signatures
29+
regexes = []
30+
regexes.append(
31+
regex.compile(
32+
virtual_spec
33+
+ r"(?P<func_sig>"
34+
+ return_type
35+
+ r"(?P<func_name>\w+\s*)"
36+
+ func_args
37+
+ ")"
38+
+ specifiers
39+
)
40+
)
41+
regexes.append(
42+
regex.compile(
43+
virtual_spec
44+
+ r"(?P<func_sig>"
45+
+ r"(?P<func_name>[\w~]+\s*)"
46+
+ func_args
47+
+ ")"
48+
+ specifiers
49+
)
50+
)
51+
52+
for rgx in regexes:
53+
pos = 0
54+
for match in rgx.finditer(lines):
55+
# Append lines before match
56+
output += lines[pos : match.start()]
57+
58+
# Append virtual specifier if it's not redundant
59+
if "final" not in match.group(
60+
"specifiers"
61+
) and "override" not in match.group("specifiers"):
62+
if match.group("virtual"):
63+
output += match.group("virtual")
64+
else:
65+
file_changed = True
66+
output += match.group("func_sig")
67+
68+
# Strip redundant specifiers
69+
specifiers_in = match.group("specifiers")
70+
specifiers_out = specifiers_in
71+
if "final" in specifiers_out:
72+
specifiers_out = regex.sub(r"\s+override", "", specifiers_out)
73+
if specifiers_in != specifiers_out:
74+
file_changed = True
75+
output += specifiers_out
76+
77+
pos = match.end()
78+
79+
# Append leftover lines in file
80+
if pos < len(lines):
81+
output += lines[pos:]
82+
83+
# Reset line buffer for next regex
84+
lines = output
85+
output = ""
86+
87+
return (lines, file_changed, True)

0 commit comments

Comments
 (0)