Skip to content

Commit a37f7ae

Browse files
authored
[clang-tidy] Add type annotations to add_new_check.py, fix minor bug (#106801)
``` > python3 -m mypy --strict clang-tools-extra/clang-tidy/add_new_check.py Success: no issues found in 1 source file ``` Also fix a bug when `--standard` is not provided on the command line: the generated test case has a `None` causing issues: ``` > python3 clang-tools-extra/clang-tidy/add_new_check.py performance XXX Updating clang-tools-extra/clang-tidy/performance/CMakeLists.txt... Creating clang-tools-extra/clang-tidy/performance/XxxCheck.h... Creating clang-tools-extra/clang-tidy/performance/XxxCheck.cpp... Updating clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp... Updating clang-tools-extra/docs/ReleaseNotes.rst... Creating clang-tools-extra/test/clang-tidy/checkers/performance/XXX.cpp... Creating clang-tools-extra/docs/clang-tidy/checks/performance/XXX.rst... Updating clang-tools-extra/docs/clang-tidy/checks/list.rst... Done. Now it's your turn! > head -n 1 clang-tools-extra/test/clang-tidy/checkers/performance/XXX.cpp // RUN: %check_clang_tidy None%s performance-XXX %t ```
1 parent 56b2907 commit a37f7ae

File tree

1 file changed

+53
-38
lines changed

1 file changed

+53
-38
lines changed

clang-tools-extra/clang-tidy/add_new_check.py

Lines changed: 53 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,6 @@
88
#
99
# ===-----------------------------------------------------------------------===#
1010

11-
from __future__ import print_function
12-
from __future__ import unicode_literals
13-
1411
import argparse
1512
import io
1613
import itertools
@@ -19,10 +16,13 @@
1916
import sys
2017
import textwrap
2118

19+
# FIXME Python 3.9: Replace typing.Tuple with builtins.tuple.
20+
from typing import Optional, Tuple
21+
2222

2323
# Adapts the module's CMakelist file. Returns 'True' if it could add a new
2424
# entry and 'False' if the entry already existed.
25-
def adapt_cmake(module_path, check_name_camel):
25+
def adapt_cmake(module_path: str, check_name_camel: str) -> bool:
2626
filename = os.path.join(module_path, "CMakeLists.txt")
2727

2828
# The documentation files are encoded using UTF-8, however on Windows the
@@ -57,14 +57,14 @@ def adapt_cmake(module_path, check_name_camel):
5757

5858
# Adds a header for the new check.
5959
def write_header(
60-
module_path,
61-
module,
62-
namespace,
63-
check_name,
64-
check_name_camel,
65-
description,
66-
lang_restrict,
67-
):
60+
module_path: str,
61+
module: str,
62+
namespace: str,
63+
check_name: str,
64+
check_name_camel: str,
65+
description: str,
66+
lang_restrict: str,
67+
) -> None:
6868
wrapped_desc = "\n".join(
6969
textwrap.wrap(
7070
description, width=80, initial_indent="/// ", subsequent_indent="/// "
@@ -139,7 +139,9 @@ class %(check_name_camel)s : public ClangTidyCheck {
139139

140140

141141
# Adds the implementation of the new check.
142-
def write_implementation(module_path, module, namespace, check_name_camel):
142+
def write_implementation(
143+
module_path: str, module: str, namespace: str, check_name_camel: str
144+
) -> None:
143145
filename = os.path.join(module_path, check_name_camel) + ".cpp"
144146
print("Creating %s..." % filename)
145147
with io.open(filename, "w", encoding="utf8", newline="\n") as f:
@@ -187,7 +189,7 @@ def write_implementation(module_path, module, namespace, check_name_camel):
187189

188190

189191
# Returns the source filename that implements the module.
190-
def get_module_filename(module_path, module):
192+
def get_module_filename(module_path: str, module: str) -> str:
191193
modulecpp = list(
192194
filter(
193195
lambda p: p.lower() == module.lower() + "tidymodule.cpp",
@@ -198,7 +200,9 @@ def get_module_filename(module_path, module):
198200

199201

200202
# Modifies the module to include the new check.
201-
def adapt_module(module_path, module, check_name, check_name_camel):
203+
def adapt_module(
204+
module_path: str, module: str, check_name: str, check_name_camel: str
205+
) -> None:
202206
filename = get_module_filename(module_path, module)
203207
with io.open(filename, "r", encoding="utf8") as f:
204208
lines = f.readlines()
@@ -217,10 +221,10 @@ def adapt_module(module_path, module, check_name, check_name_camel):
217221
+ '");\n'
218222
)
219223

220-
lines = iter(lines)
224+
lines_iter = iter(lines)
221225
try:
222226
while True:
223-
line = next(lines)
227+
line = next(lines_iter)
224228
if not header_added:
225229
match = re.search('#include "(.*)"', line)
226230
if match:
@@ -247,10 +251,11 @@ def adapt_module(module_path, module, check_name, check_name_camel):
247251
# If we didn't find the check name on this line, look on the
248252
# next one.
249253
prev_line = line
250-
line = next(lines)
254+
line = next(lines_iter)
251255
match = re.search(' *"([^"]*)"', line)
252256
if match:
253257
current_check_name = match.group(1)
258+
assert current_check_name
254259
if current_check_name > check_fq_name:
255260
check_added = True
256261
f.write(check_decl)
@@ -262,7 +267,9 @@ def adapt_module(module_path, module, check_name, check_name_camel):
262267

263268

264269
# Adds a release notes entry.
265-
def add_release_notes(module_path, module, check_name, description):
270+
def add_release_notes(
271+
module_path: str, module: str, check_name: str, description: str
272+
) -> None:
266273
wrapped_desc = "\n".join(
267274
textwrap.wrap(
268275
description, width=80, initial_indent=" ", subsequent_indent=" "
@@ -324,9 +331,14 @@ def add_release_notes(module_path, module, check_name, description):
324331

325332

326333
# Adds a test for the check.
327-
def write_test(module_path, module, check_name, test_extension, test_standard):
328-
if test_standard:
329-
test_standard = f"-std={test_standard}-or-later "
334+
def write_test(
335+
module_path: str,
336+
module: str,
337+
check_name: str,
338+
test_extension: str,
339+
test_standard: Optional[str],
340+
) -> None:
341+
test_standard = f"-std={test_standard}-or-later " if test_standard else ""
330342
check_name_dashes = module + "-" + check_name
331343
filename = os.path.normpath(
332344
os.path.join(
@@ -362,7 +374,7 @@ def write_test(module_path, module, check_name, test_extension, test_standard):
362374
)
363375

364376

365-
def get_actual_filename(dirname, filename):
377+
def get_actual_filename(dirname: str, filename: str) -> str:
366378
if not os.path.isdir(dirname):
367379
return ""
368380
name = os.path.join(dirname, filename)
@@ -376,7 +388,7 @@ def get_actual_filename(dirname, filename):
376388

377389

378390
# Recreates the list of checks in the docs/clang-tidy/checks directory.
379-
def update_checks_list(clang_tidy_path):
391+
def update_checks_list(clang_tidy_path: str) -> None:
380392
docs_dir = os.path.join(clang_tidy_path, "../docs/clang-tidy/checks")
381393
filename = os.path.normpath(os.path.join(docs_dir, "list.rst"))
382394
# Read the content of the current list.rst file
@@ -390,12 +402,12 @@ def update_checks_list(clang_tidy_path):
390402
for file in filter(
391403
lambda s: s.endswith(".rst"), os.listdir(os.path.join(docs_dir, subdir))
392404
):
393-
doc_files.append([subdir, file])
405+
doc_files.append((subdir, file))
394406
doc_files.sort()
395407

396408
# We couldn't find the source file from the check name, so try to find the
397409
# class name that corresponds to the check in the module file.
398-
def filename_from_module(module_name, check_name):
410+
def filename_from_module(module_name: str, check_name: str) -> str:
399411
module_path = os.path.join(clang_tidy_path, module_name)
400412
if not os.path.isdir(module_path):
401413
return ""
@@ -433,7 +445,7 @@ def filename_from_module(module_name, check_name):
433445
return ""
434446

435447
# Examine code looking for a c'tor definition to get the base class name.
436-
def get_base_class(code, check_file):
448+
def get_base_class(code: str, check_file: str) -> str:
437449
check_class_name = os.path.splitext(os.path.basename(check_file))[0]
438450
ctor_pattern = check_class_name + r"\([^:]*\)\s*:\s*([A-Z][A-Za-z0-9]*Check)\("
439451
matches = re.search(r"\s+" + check_class_name + "::" + ctor_pattern, code)
@@ -452,7 +464,7 @@ def get_base_class(code, check_file):
452464
return ""
453465

454466
# Some simple heuristics to figure out if a check has an autofix or not.
455-
def has_fixits(code):
467+
def has_fixits(code: str) -> bool:
456468
for needle in [
457469
"FixItHint",
458470
"ReplacementText",
@@ -464,7 +476,7 @@ def has_fixits(code):
464476
return False
465477

466478
# Try to figure out of the check supports fixits.
467-
def has_auto_fix(check_name):
479+
def has_auto_fix(check_name: str) -> str:
468480
dirname, _, check_name = check_name.partition("-")
469481

470482
check_file = get_actual_filename(
@@ -499,7 +511,7 @@ def has_auto_fix(check_name):
499511

500512
return ""
501513

502-
def process_doc(doc_file):
514+
def process_doc(doc_file: Tuple[str, str]) -> Tuple[str, Optional[re.Match[str]]]:
503515
check_name = doc_file[0] + "-" + doc_file[1].replace(".rst", "")
504516

505517
with io.open(os.path.join(docs_dir, *doc_file), "r", encoding="utf8") as doc:
@@ -508,13 +520,13 @@ def process_doc(doc_file):
508520

509521
if match:
510522
# Orphan page, don't list it.
511-
return "", ""
523+
return "", None
512524

513525
match = re.search(r".*:http-equiv=refresh: \d+;URL=(.*).html(.*)", content)
514526
# Is it a redirect?
515527
return check_name, match
516528

517-
def format_link(doc_file):
529+
def format_link(doc_file: Tuple[str, str]) -> str:
518530
check_name, match = process_doc(doc_file)
519531
if not match and check_name and not check_name.startswith("clang-analyzer-"):
520532
return " :doc:`%(check_name)s <%(module)s/%(check)s>`,%(autofix)s\n" % {
@@ -526,7 +538,7 @@ def format_link(doc_file):
526538
else:
527539
return ""
528540

529-
def format_link_alias(doc_file):
541+
def format_link_alias(doc_file: Tuple[str, str]) -> str:
530542
check_name, match = process_doc(doc_file)
531543
if (match or (check_name.startswith("clang-analyzer-"))) and check_name:
532544
module = doc_file[0]
@@ -543,6 +555,7 @@ def format_link_alias(doc_file):
543555
ref_end = "_"
544556
else:
545557
redirect_parts = re.search(r"^\.\./([^/]*)/([^/]*)$", match.group(1))
558+
assert redirect_parts
546559
title = redirect_parts[1] + "-" + redirect_parts[2]
547560
target = redirect_parts[1] + "/" + redirect_parts[2]
548561
autofix = has_auto_fix(title)
@@ -599,7 +612,7 @@ def format_link_alias(doc_file):
599612

600613

601614
# Adds a documentation for the check.
602-
def write_docs(module_path, module, check_name):
615+
def write_docs(module_path: str, module: str, check_name: str) -> None:
603616
check_name_dashes = module + "-" + check_name
604617
filename = os.path.normpath(
605618
os.path.join(
@@ -623,15 +636,15 @@ def write_docs(module_path, module, check_name):
623636
)
624637

625638

626-
def get_camel_name(check_name):
639+
def get_camel_name(check_name: str) -> str:
627640
return "".join(map(lambda elem: elem.capitalize(), check_name.split("-")))
628641

629642

630-
def get_camel_check_name(check_name):
643+
def get_camel_check_name(check_name: str) -> str:
631644
return get_camel_name(check_name) + "Check"
632645

633646

634-
def main():
647+
def main() -> None:
635648
language_to_extension = {
636649
"c": "c",
637650
"c++": "cpp",
@@ -756,6 +769,8 @@ def main():
756769
)
757770
elif language in ["objc", "objc++"]:
758771
language_restrict = "%(lang)s.ObjC"
772+
else:
773+
raise ValueError(f"Unsupported language '{language}' was specified")
759774

760775
write_header(
761776
module_path,
@@ -769,7 +784,7 @@ def main():
769784
write_implementation(module_path, module, namespace, check_name_camel)
770785
adapt_module(module_path, module, check_name, check_name_camel)
771786
add_release_notes(module_path, module, check_name, description)
772-
test_extension = language_to_extension.get(language)
787+
test_extension = language_to_extension[language]
773788
write_test(module_path, module, check_name, test_extension, args.standard)
774789
write_docs(module_path, module, check_name)
775790
update_checks_list(clang_tidy_path)

0 commit comments

Comments
 (0)