[clang-tidy] Improve rename_check.py

[ccotter]

Summary

rename_check.py now find and renames the test file. rename_check.py
also will now use 'git mv', so the developer no longer has to manually
add the file after running the script.

[ccotter]

I tested with https://reviews.llvm.org/D141133 where I renamed my new check, but noticed rename_tool.py did not rename or update the check name in the test file. The result of running the updated script against my tool is on github: https://github.com/ccotter/llvm-project/commit/b0f5118d4a2d1c5100170784117b26e59d4df0bc

[carlosgalvezp]

Thanks for the fix! Some minor comments/questions.

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

75

I'm not sure it's the responsibility of this check to do git stuff, there may be use cases where this is not wanted / users might not expect this behavior. Introducing a dependency to git might also make this script harder to unit test in the future.

Thus I think it'd be better to keep the original behavior so the script has one single responsibility. What do you think @njames93 ?

97

Why is this change needed? I'd expect only line 99 to be needed to fix moving the test.

[ccotter]

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

97

Ah, it's not needed, but I thought it was a bit cleaner to have both the globs take the same approach and be recursive, avoiding the for-loop over the subdirectories.

[carlosgalvezp]

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

97

Ah yes I see it now, yes it's good to keep the consistency, thanks!

[ccotter]

• revert git changes

[ccotter]

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

75

I wasn't sure about this - reverted this. Single responsibility is better.

[carlosgalvezp]

LGTM, thanks for the fix! Let's give a few days for other reviewers to get a look.

[njames93]

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

75

Yeah we shouldn't be messing with git indexes.

[ccotter]

Thanks for reviewing and merging my recent changes!

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

 
 from __future__ import unicode_literals
 
 import argparse
 import glob
 import io
 import os
 import re
+import subprocess
 
 def replaceInFileRegex(fileName, sFrom, sTo):
   if sFrom == sTo:
     return
 
   # The documentation files are encoded using UTF-8, however on Windows the
   # default encoding might be different (e.g. CP-1252). To make sure UTF-8 is
   # always used, use `io.open(filename, mode, encoding='utf8')` for reading and
return ''.join(['//===--- ',
                   '-===//'])
 
 
 def fileRename(fileName, sFrom, sTo):
   if sFrom not in fileName or sFrom == sTo:
     return fileName
   newFileName = fileName.replace(sFrom, sTo)
   print("Renaming '%s' -> '%s'..." % (fileName, newFileName))
-  os.rename(fileName, newFileName)
+  subprocess.check_call(["git", "mv", fileName, newFileName])
   return newFileName
 
 
 def deleteMatchingLines(fileName, pattern):
   lines = None
   with io.open(fileName, 'r', encoding='utf8') as f:
     lines = f.readlines()
 
   not_matching_lines = [l for l in lines if not re.search(pattern, l)]
   if len(not_matching_lines) == len(lines):
     return False
 
   print("Removing lines matching '%s' in '%s'..." % (pattern, fileName))
   print('  ' + '  '.join([l for l in lines if re.search(pattern, l)]))
   with io.open(fileName, 'w', encoding='utf8') as f:
     f.writelines(not_matching_lines)
 
   return True
 
 
 def getListOfFiles(clang_tidy_path):
-  files = glob.glob(os.path.join(clang_tidy_path, '*'))
-  for dirname in files:
-    if os.path.isdir(dirname):
-      files += glob.glob(os.path.join(dirname, '*'))
+  files = glob.glob(os.path.join(clang_tidy_path, '**'), recursive=True)
   files += glob.glob(os.path.join(clang_tidy_path, '..', 'test',
-                                  'clang-tidy', '*'))
+                                  'clang-tidy', 'checkers', '**'), recursive=True)
   files += glob.glob(os.path.join(clang_tidy_path, '..', 'docs',
                                   'clang-tidy', 'checks', '*'))
   return [filename for filename in files if os.path.isfile(filename)]
 
 
 # Adapts the module's CMakelist file. Returns 'True' if it could add a new
 # entry and 'False' if the entry already existed.
 def adapt_cmake(module_path, check_name_camel):
if not check_found:
       return 1
 
     modulecpp = next(iter(filter(
         lambda p: p.lower() == old_module.lower() + 'tidymodule.cpp',
         os.listdir(old_module_path))))
     deleteMatchingLines(os.path.join(old_module_path, modulecpp),
                       '\\b' + check_name_camel + '|\\b' + args.old_check_name)
 
+  old_check_filename = '-'.join(args.old_check_name.split('-')[1:])
+  new_check_filename = '-'.join(args.new_check_name.split('-')[1:])
+
   for filename in getListOfFiles(clang_tidy_path):
     originalName = filename
-    filename = fileRename(filename, args.old_check_name,
-                          args.new_check_name)
+    filename = fileRename(filename, old_check_filename, new_check_filename)
     filename = fileRename(filename, check_name_camel, new_check_name_camel)
     replaceInFile(filename, generateCommentLineHeader(originalName),
                   generateCommentLineHeader(filename))
     replaceInFile(filename, generateCommentLineSource(originalName),
                   generateCommentLineSource(filename))
     for header_guard in header_guard_variants:
       replaceInFile(filename, header_guard, header_guard_new)