[clang-tidy] Make add_new_check.py Python 3 compatible

[omtcyfz]

Summary

add_new_check.py didn't support Python 3.X for some reason, even though it used print in Python 3.X-compatible way. With just a few tweaks add_new_check.py supports both Python 2.7.X and Python 3.X.

Tested: Python 2.7.6, Python 2.7.12, Python 3.5.2.

Currently stalled, not updated for a while.

[bkramer]

clang-tidy/add_new_check.py

16

Can we rely on six being available? It's not part of the default python installation afaik.

[omtcyfz]

clang-tidy/add_new_check.py

16

Yes, I am pretty sure about that. Absolute majority of Python projects relies on having six available and Python installation without six seems really weird to me.

This is THE standard library for Python 2.7.X/3.X compatibility issues.

[bkramer]

Okay. If six is installed by default on Ubuntu and macOS we should be fine. The script isn't in the critical path for anyone.

[omtcyfz]

UPD: This patch is eventually causing some problems in Python 3. Working on that.

[alexfh]

clang-tidy/add_new_check.py

58

Too many parentheses.

97–100

Is it better to encode the result of the formatting operator instead?

(""" ..... """ % {'header_guard': header_guard, ... }).encode()

[omtcyfz]

No longer relevant.

clang-tidy/add_new_check.py

 #!/usr/bin/env python
 #
 #===- add_new_check.py - clang-tidy check generator ----------*- python -*--===#
 #
 #                     The LLVM Compiler Infrastructure
 #
 # This file is distributed under the University of Illinois Open Source
 # License. See LICENSE.TXT for details.
 #
 #===------------------------------------------------------------------------===#
 
 import os
 import re
 import sys
 
+from six.moves import filter
+
 
 # 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):
   filename = os.path.join(module_path, 'CMakeLists.txt')
-  with open(filename, 'r') as f:
+  with open(filename, 'rb') as f:
     lines = f.readlines()
 
   cpp_file = check_name_camel + '.cpp'
 
   # Figure out whether this check already exists.
   for line in lines:
     if line.strip() == cpp_file:
       return False

 # Adds a header for the new check.
 def write_header(module_path, module, check_name, check_name_camel):
   check_name_dashes = module + '-' + check_name
   filename = os.path.join(module_path, check_name_camel) + '.h'
   print('Creating %s...' % filename)
   with open(filename, 'wb') as f:
     header_guard = ('LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_' + module.upper() + '_'
                     + check_name.upper().replace('-', '_') + '_H')
-    f.write('//===--- ')
-    f.write(os.path.basename(filename))
-    f.write(' - clang-tidy')
-    f.write('-' * max(0, 43 - len(os.path.basename(filename))))
-    f.write('*- C++ -*-===//')
-    f.write("""
+    f.write(b'//===--- ')
+    f.write((os.path.basename(filename)).encode())
+    f.write(b' - clang-tidy')
+    f.write(b'-' * max(0, 43 - len(os.path.basename(filename))))
+    f.write(b'*- C++ -*-===//')
+    f.write(b"""
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
public:
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 };
 
 } // namespace %(module)s
 } // namespace tidy
 } // namespace clang
 
 #endif // %(header_guard)s
-""" % {'header_guard': header_guard,
-       'check_name': check_name_camel,
-       'check_name_dashes': check_name_dashes,
-       'module': module})
+""" % {b'header_guard': header_guard.encode(),
+       b'check_name': check_name_camel.encode(),
+       b'check_name_dashes': check_name_dashes.encode(),
+       b'module': module.encode()})
 
 
 # Adds the implementation of the new check.
 def write_implementation(module_path, module, check_name_camel):
   filename = os.path.join(module_path, check_name_camel) + '.cpp'
   print('Creating %s...' % filename)
   with open(filename, 'wb') as f:
-    f.write('//===--- ')
-    f.write(os.path.basename(filename))
-    f.write(' - clang-tidy')
-    f.write('-' * max(0, 52 - len(os.path.basename(filename))))
-    f.write('-===//')
-    f.write("""
+    f.write(b'//===--- ')
+    f.write((os.path.basename(filename)).encode())
+    f.write(b' - clang-tidy')
+    f.write(b'-' * max(0, 52 - len(os.path.basename(filename))))
+    f.write(b'-===//')
+    f.write(b"""
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
void %(check_name)s::check(const MatchFinder::MatchResult &Result) {
   diag(MatchedDecl->getLocation(), "function %%0 is insufficiently awesome")
       << MatchedDecl
       << FixItHint::CreateInsertion(MatchedDecl->getLocation(), "awesome_");
 }
 
 } // namespace %(module)s
 } // namespace tidy
 } // namespace clang
-""" % {'check_name': check_name_camel,
-       'module': module})
+""" % {b'check_name': check_name_camel.encode(),
+       b'module': module.encode()})
 
 
 # Modifies the module to include the new check.
 def adapt_module(module_path, module, check_name, check_name_camel):
   modulecpp = filter(lambda p: p.lower() == module.lower() + 'tidymodule.cpp',
-                     os.listdir(module_path))[0]
-  filename = os.path.join(module_path, modulecpp)
-  with open(filename, 'r') as f:
+                     os.listdir(module_path))
+  filename = os.path.join(module_path, next(modulecpp))
+  with open(filename, 'rb') as f:
     lines = f.readlines()
 
   print('Updating %s...' % filename)
   with open(filename, 'wb') as f:
     header_added = False
     header_found = False
     check_added = False
     check_decl = ('    CheckFactories.registerCheck<' + check_name_camel +

 
 # Adds a test for the check.
 def write_test(module_path, module, check_name):
   check_name_dashes = module + '-' + check_name
   filename = os.path.normpath(os.path.join(module_path, '../../test/clang-tidy',
                                            check_name_dashes + '.cpp'))
   print('Creating %s...' % filename)
   with open(filename, 'wb') as f:
-    f.write("""// RUN: %%check_clang_tidy %%s %(check_name_dashes)s %%t
+    f.write(b"""// RUN: %%check_clang_tidy %%s %(check_name_dashes)s %%t
 
 // FIXME: Add something that triggers the check here.
 void f();
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'f' is insufficiently awesome [%(check_name_dashes)s]
 
 // FIXME: Verify the applied fix.
 //   * Make the CHECK patterns specific enough and try to make verified lines
 //     unique to avoid incorrect matches.
 //   * Use {{}} for regular expressions.
 // CHECK-FIXES: {{^}}void awesome_f();{{$}}
 
 // FIXME: Add something that doesn't trigger the check here.
 void awesome_f2();
-""" % {'check_name_dashes': check_name_dashes})
+""" % {b'check_name_dashes': check_name_dashes.encode()})
 
 
 # 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'))
-  with open(filename, 'r') as f:
+  with open(filename, 'rb') as f:
     lines = f.readlines()
   doc_files = filter(lambda s: s.endswith('.rst') and s != 'list.rst',
                      os.listdir(docs_dir))
-  doc_files.sort()
+  doc_files = sorted(doc_files)
 
   def format_link(doc_file):
     check_name = doc_file.replace('.rst', '')
-    with open(os.path.join(docs_dir, doc_file), 'r') as doc:
+    with open(os.path.join(docs_dir, doc_file), 'rb') as doc:
       match = re.search('.*:http-equiv=refresh: \d+;URL=(.*).html.*',
                         doc.read())
       if match:
         return '   %(check)s (redirects to %(target)s) <%(check)s>\n' % {
             'check': check_name,
             'target': match.group(1)
         }
       return '   %s\n' % check_name
 
   checks = map(format_link, doc_files)
 
   print('Updating %s...' % filename)
   with open(filename, 'wb') as f:
     for line in lines:
       f.write(line)
-      if line.startswith('.. toctree::'):
+      if line.startswith(b'.. toctree::'):
         f.writelines(checks)
         break
 
 
 # Adds a documentation for the check.
 def write_docs(module_path, module, check_name):
   check_name_dashes = module + '-' + check_name
   filename = os.path.normpath(os.path.join(
       module_path, '../../docs/clang-tidy/checks/', check_name_dashes + '.rst'))
   print('Creating %s...' % filename)
   with open(filename, 'wb') as f:
-    f.write(""".. title:: clang-tidy - %(check_name_dashes)s
+    f.write(b""".. title:: clang-tidy - %(check_name_dashes)s
 
 %(check_name_dashes)s
 %(underline)s
 
 FIXME: Describe what patterns does the check detect and why. Give examples.
-""" % {'check_name_dashes': check_name_dashes,
-       'underline': '=' * len(check_name_dashes)})
+""" % {b'check_name_dashes': check_name_dashes.encode(),
+       b'underline': b'=' * len(check_name_dashes)})
 
 
 def main():
   if len(sys.argv) == 2 and sys.argv[1] == '--update-docs':
     update_checks_list(os.path.dirname(sys.argv[0]))
     return
 
   if len(sys.argv) != 3:
-    print """\
+    print("""\
 Usage: add_new_check.py <module> <check>, e.g.
   add_new_check.py misc awesome-functions
 
 Alternatively, run 'add_new_check.py --update-docs' to just update the list of
-documentation files."""
+documentation files.""")
 
     return
 
   module = sys.argv[1]
   check_name = sys.argv[2]
 
   if check_name.startswith(module):
-    print 'Check name "%s" must not start with the module "%s". Exiting.' % (
-        check_name, module)
+    print('Check name "%s" must not start with the module "%s". Exiting.' % (
+            check_name, module))
     return
   check_name_camel = ''.join(map(lambda elem: elem.capitalize(),
                                  check_name.split('-'))) + 'Check'
   clang_tidy_path = os.path.dirname(sys.argv[0])
   module_path = os.path.join(clang_tidy_path, module)
 
   if not adapt_cmake(module_path, check_name_camel):
     return