[clang-tidy] Fix minor bug in add_new_check.py

[ccotter]

Summary

While rebuilding the list of checks in add_new_check.py,
check is a file is a subdirectory before traversing it.

Test plan: Ran ./add_new_check.py --update-docs and confirmed the list.rst file was unchanged.

[carlosgalvezp]

LGTM, I have a suggestion for further improvement.

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

324–328

It feels this whole code could be made much simpler, readable and safer by just doing:

doc_files = glob.glob(os.path.join(docs_dir, "**", "*.rst"), recursive=True)

[ccotter]

Fix isdir check

[ccotter]

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

324–328

I gave this a shot, but realized the the glob returned the relative path ../docs/clang-tidy/checks/<subdir>/<check>.rst, and the existing logic only adds [<subdir>, <check>.rst] to the list. Python3.10's glob.glob has root_dir which would do the trick for us, but I don't think we can rely on Python3 yet? I think using glob might not improve the readability all that much since I'd have to strip out the docs_dir subpath, but happy to give that a go based on your feedback.

[carlosgalvezp]

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

324–328

Good point, I was thinking of something along these lines:

from pathlib import Path

for doc_file in map(lambda s: Path(s), glob.glob(os.path.join(docs_dir, "*", "*.rst"))):
  doc_files.append([doc_file.parts[-1], doc_file.parts[-2])

The current patch is already a great cleanup so I'm fine with it as well :)

[carlosgalvezp]

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

324–328

Sorry, I meant:

doc_files.append([doc_file.parts[-2], doc_file.parts[-1])

[carlosgalvezp]

Thanks for the fix!

[ccotter]

@carlosgalvezp - could you also help commit this if it looks good?

[carlosgalvezp]

In D140772#4028570, @ccotter wrote:

@carlosgalvezp - could you also help commit this if it looks good?

Sure thing, thanks for the fix!

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

 # Recreates the list of checks in the docs/clang-tidy/checks directory.
 def update_checks_list(clang_tidy_path):
   docs_dir = os.path.join(clang_tidy_path, '../docs/clang-tidy/checks')
   filename = os.path.normpath(os.path.join(docs_dir, 'list.rst'))
   # Read the content of the current list.rst file
   with io.open(filename, 'r', encoding='utf8') as f:
     lines = f.readlines()
   # Get all existing docs
   doc_files = []
-  for subdir in list(filter(lambda s: not s.endswith('.rst') and not s.endswith('.py'),
-                     os.listdir(docs_dir))):
+  for subdir in filter(lambda s: os.path.isdir(os.path.join(docs_dir, s)), os.listdir(docs_dir)):
     for file in filter(lambda s: s.endswith('.rst'), os.listdir(os.path.join(docs_dir, subdir))):
       doc_files.append([subdir, file])
   doc_files.sort()
 
   # We couldn't find the source file from the check name, so try to find the
   # class name that corresponds to the check in the module file.
   def filename_from_module(module_name, check_name):
     module_path = os.path.join(clang_tidy_path, module_name)
     if not os.path.isdir(module_path):
       return ''
     module_file = get_module_filename(module_path, module_name)

clang-tools-extra/docs/ReleaseNotes.rst

 The improvements are...
 
 Improvements to clang-tidy
 --------------------------
 
 - Change to Python 3 in the shebang of `add_new_check.py` and `rename_check.py`,
   as the existing code is not compatible with Python 2.
 
+- Fix a minor bug in `add_new_check.py` to only traverse subdirectories
+  when updating the list of checks in the documentation.
+
 New checks
 ^^^^^^^^^^
 
 - New :doc:`bugprone-suspicious-realloc-usage
   <clang-tidy/checks/bugprone/suspicious-realloc-usage>` check.
 
   Finds usages of ``realloc`` where the return value is assigned to the
   same expression as passed to the first argument.