Fix python 2-vs-3 issues in add_new_check.py and rename_check.py

[mattbeardsley]

Summary

As of this commit:

https://github.com/llvm/llvm-project/commit/307b1fdd

If either of those scripts are invoked with python 2, neither works due to:

"TypeError: write() argument 1 must be unicode, not str"

And if rename_check.py is invoked with python 3:

"ValueError: binary mode doesn't take an encoding argument"

(referring to with io.open(filename, 'wb', encoding='utf8') as f:), and

Another issue in rename_check.py in python 2:

"TypeError: list object is not an iterator"

(referring to next(filter( ... os.listdir(old_module_path))))

(so, rename_check doesn't work with either 2 or 3, and add_new_check
doesn't work with 2, but does work with 3)

I ran these steps to test both python versions:
(manually - appears to be the "status quo" for these files)

python3 clang-tools-extra/clang-tidy/add_new_check.py readability ggggg
python3 clang-tools-extra/clang-tidy/rename_check.py readability-ggggg readability-hhhhh

git checkout HEAD -- clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/list.rst
rm -f clang-tools-extra/clang-tidy/readability/GggggCheck.cpp clang-tools-extra/clang-tidy/readability/GggggCheck.h clang-tools-extra/docs/clang-tidy/checks/readability-ggggg.rst clang-tools-extra/test/clang-tidy/checkers/readability-ggggg.cpp clang-tools-extra/clang-tidy/readability/HhhhhCheck.cpp clang-tools-extra/clang-tidy/readability/HhhhhCheck.h clang-tools-extra/docs/clang-tidy/checks/readability-hhhhh.rst

python2 clang-tools-extra/clang-tidy/add_new_check.py readability ggggg
python2 clang-tools-extra/clang-tidy/rename_check.py readability-ggggg readability-hhhhh

git checkout HEAD -- clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/list.rst
rm -f clang-tools-extra/clang-tidy/readability/GggggCheck.cpp clang-tools-extra/clang-tidy/readability/GggggCheck.h clang-tools-extra/docs/clang-tidy/checks/readability-ggggg.rst clang-tools-extra/test/clang-tidy/checkers/readability-ggggg.cpp clang-tools-extra/clang-tidy/readability/HhhhhCheck.cpp clang-tools-extra/clang-tidy/readability/HhhhhCheck.h clang-tools-extra/docs/clang-tidy/checks/readability-hhhhh.rst

[kbobyrev]

Hey, thanks for noticing!

Ugh, I think that's not good, I was trying to make sure the io.open piece really works with Python 2 and Python 3 as this was the purpose fo the patch :( I need t check that again, I think it's really unfortunate to lose Python 2 support if that's the only reason here. Personally, I think moving to Python 3 completely is a great idea but I would anticipate many buildbots and some older distro versions not having it/being reluctant. As of right now, we still have the >= 2.7 requirement so I think the best option is to keep both somehow :(

Thanks for bringing it up!

[mattbeardsley]

Reverted python-3-only enforcement, fixed both scripts to work with both python 2 and python 3
(generally based on the guidance here: https://python-future.org/compatible_idioms.html#strings-and-bytes)

[mattbeardsley]

No worries! Figured I'd start the conversation with the easy fix, but was fully prepared to hear that we can't let go of python 2 just yet, as much as we may want to :).
Let me know if this fix looks suitable. Thanks for your time!

[kbobyrev]

Nice, thank you!

[mattbeardsley]

Thanks! I don't have commit access, I was just reading here what to do:

Prior to obtaining commit access, it is common practice to request that someone with commit access commits on your behalf. When doing so, please provide the name and email address you would like to use in the Author property of the commit.

When you have a chance, would you be able to help with that? Here's my info:
Matt Beardsley
[e-mail]

[kbobyrev]

Done, thank you again for noticing!

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

 #!/usr/bin/env python
 #
 #===- add_new_check.py - clang-tidy check generator ---------*- python -*--===#
 #
 # Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 # See https://llvm.org/LICENSE.txt for license information.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 #
 #===-----------------------------------------------------------------------===#
 
 from __future__ import print_function
+from __future__ import unicode_literals
 
 import argparse
 import io
 import os
 import re
 import sys
 
 # Adapts the module's CMakelist file. Returns 'True' if it could add a new

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

 #!/usr/bin/env python
 #
 #===- rename_check.py - clang-tidy check renamer ------------*- python -*--===#
 #
 # Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 # See https://llvm.org/LICENSE.txt for license information.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 #
 #===-----------------------------------------------------------------------===#
 
+from __future__ import unicode_literals
+
 import argparse
 import glob
 import io
 import os
 import re
 
 def replaceInFileRegex(fileName, sFrom, sTo):
   if sFrom == sTo:
def adapt_cmake(module_path, check_name_camel):
   cpp_file = check_name_camel + '.cpp'
 
   # Figure out whether this check already exists.
   for line in lines:
     if line.strip() == cpp_file:
       return False
 
   print('Updating %s...' % filename)
-  with io.open(filename, 'wb', encoding='utf8') as f:
+  with io.open(filename, 'w', encoding='utf8') as f:
     cpp_found = False
     file_added = False
     for line in lines:
       cpp_line = line.strip().endswith('.cpp')
       if (not file_added) and (cpp_line or cpp_found):
         cpp_found = True
         if (line.strip() > cpp_file) or (not cpp_line):
-          f.write(('  ' + cpp_file + '\n').encode())
+          f.write('  ' + cpp_file + '\n')
           file_added = True
-      f.write(line.encode())
+      f.write(line)
 
   return True
 
 # Modifies the module to include the new check.
 def adapt_module(module_path, module, check_name, check_name_camel):
-  modulecpp = next(filter(lambda p: p.lower() == module.lower() + 'tidymodule.cpp', os.listdir(module_path)))
+  modulecpp = next(iter(filter(
+      lambda p: p.lower() == module.lower() + 'tidymodule.cpp',
+      os.listdir(module_path))))
   filename = os.path.join(module_path, modulecpp)
   with io.open(filename, 'r', encoding='utf8') as f:
     lines = f.readlines()
 
   print('Updating %s...' % filename)
-  with io.open(filename, 'wb', encoding='utf8') as f:
+  with io.open(filename, 'w', encoding='utf8') as f:
     header_added = False
     header_found = False
     check_added = False
     check_decl = ('    CheckFactories.registerCheck<' + check_name_camel +
                   '>(\n        "' + check_name + '");\n')
 
     for line in lines:
       if not header_added:
         match = re.search('#include "(.*)"', line)
         if match:
           header_found = True
           if match.group(1) > check_name_camel:
             header_added = True
-            f.write(('#include "' + check_name_camel + '.h"\n').encode())
+            f.write('#include "' + check_name_camel + '.h"\n')
         elif header_found:
           header_added = True
-          f.write(('#include "' + check_name_camel + '.h"\n').encode())
+          f.write('#include "' + check_name_camel + '.h"\n')
 
       if not check_added:
         if line.strip() == '}':
           check_added = True
-          f.write(check_decl.encode())
+          f.write(check_decl)
         else:
           match = re.search('registerCheck<(.*)>', line)
           if match and match.group(1) > check_name_camel:
             check_added = True
-            f.write(check_decl.encode())
-      f.write(line.encode())
+            f.write(check_decl)
+      f.write(line)
 
 
 # Adds a release notes entry.
 def add_release_notes(clang_tidy_path, old_check_name, new_check_name):
   filename = os.path.normpath(os.path.join(clang_tidy_path,
                                            '../docs/ReleaseNotes.rst'))
   with io.open(filename, 'r', encoding='utf8') as f:
     lines = f.readlines()
 
   lineMatcher = re.compile('Renamed checks')
   nextSectionMatcher = re.compile('Improvements to include-fixer')
   checkMatcher = re.compile('- The \'(.*)')
 
   print('Updating %s...' % filename)
-  with io.open(filename, 'wb', encoding='utf8') as f:
+  with io.open(filename, 'w', encoding='utf8') as f:
     note_added = False
     header_found = False
     add_note_here = False
 
     for line in lines:
       if not note_added:
         match = lineMatcher.match(line)
         match_next = nextSectionMatcher.match(line)
         match_check = checkMatcher.match(line)
         if match_check:
           last_check = match_check.group(1)
           if last_check > old_check_name:
             add_note_here = True
 
         if match_next:
           add_note_here = True
 
         if match:
           header_found = True
-          f.write(line.encode())
+          f.write(line)
           continue
 
         if line.startswith('^^^^'):
-          f.write(line.encode())
+          f.write(line)
           continue
 
         if header_found and add_note_here:
           if not line.startswith('^^^^'):
-            f.write(("""- The '%s' check was renamed to :doc:`%s
+            f.write("""- The '%s' check was renamed to :doc:`%s
   <clang-tidy/checks/%s>`
 
-""" % (old_check_name, new_check_name, new_check_name)).encode())
+""" % (old_check_name, new_check_name, new_check_name))
             note_added = True
 
-      f.write(line.encode())
+      f.write(line)
 
 def main():
   parser = argparse.ArgumentParser(description='Rename clang-tidy check.')
   parser.add_argument('old_check_name', type=str,
                       help='Old check name.')
   parser.add_argument('new_check_name', type=str,
                       help='New check name.')
   parser.add_argument('--check_class_name', type=str,
if (args.old_check_name != args.new_check_name):
     # Remove the check from the old module.
     cmake_lists = os.path.join(old_module_path, 'CMakeLists.txt')
     check_found = deleteMatchingLines(cmake_lists, '\\b' + check_name_camel)
     if not check_found:
       print("Check name '%s' not found in %s. Exiting." %
             (check_name_camel, cmake_lists))
       return 1
 
-    modulecpp = next(filter(
+    modulecpp = next(iter(filter(
         lambda p: p.lower() == old_module.lower() + 'tidymodule.cpp',
-        os.listdir(old_module_path)))
+        os.listdir(old_module_path))))
     deleteMatchingLines(os.path.join(old_module_path, modulecpp),
                       '\\b' + check_name_camel + '|\\b' + args.old_check_name)
 
   for filename in getListOfFiles(clang_tidy_path):
     originalName = filename
     filename = fileRename(filename, args.old_check_name,
                           args.new_check_name)
     filename = fileRename(filename, check_name_camel, new_check_name_camel)