[python] Support PathLike filenames and directories

Python 3.6 introduced a file system path protocol (PEP 519[1]).
The standard library APIs accepting file system paths now accept path
objects too. It could be useful to add this here as well
for convenience.

[1] https://www.python.org/dev/peps/pep-0519

Authored by: jstasiak (Jakub Stasiak)

Differential Revision: https://reviews.llvm.org/D54120

llvm-svn: 346586

clang/bindings/python/clang/cindex.py

@@ -67,6 +67,7 @@ import collections
 
 import clang.enumerations
 
+import os
 import sys
 if sys.version_info[0] == 3:
     # Python 3 strings are unicode, translate them to/from utf8 for C-interop.
@@ -123,6 +124,14 @@ elif sys.version_info[0] == 2:
     def b(x):
         return x
 
+# We only support PathLike objects on Python version with os.fspath present
+# to be consistent with the Python standard library. On older Python versions
+# we only support strings and we have dummy fspath to just pass them through.
+try:
+    fspath = os.fspath
+except AttributeError:
+    def fspath(x):
+        return x
 
 # ctypes doesn't implicitly convert c_void_p to the appropriate wrapper
 # object. This is a problem, because it means that from_parameter will see an
@@ -2752,11 +2761,11 @@ class TranslationUnit(ClangObject):
         etc. e.g. ["-Wall", "-I/path/to/include"].
 
         In-memory file content can be provided via unsaved_files. This is an
-        iterable of 2-tuples. The first element is the str filename. The
+        iterable of 2-tuples. The first element is the filename (str or
-        second element defines the content. Content can be provided as str
+        PathLike). The second element defines the content. Content can be
-        source code or as file objects (anything with a read() method). If
+        provided as str source code or as file objects (anything with a read()
-        a file object is being used, content will be read until EOF and the
+        method). If a file object is being used, content will be read until EOF
-        read cursor will not be reset to its original position.
+        and the read cursor will not be reset to its original position.
 
         options is a bitwise or of TranslationUnit.PARSE_XXX flags which will
         control parsing behavior.
@@ -2801,11 +2810,13 @@ class TranslationUnit(ClangObject):
                 if hasattr(contents, "read"):
                     contents = contents.read()
 
-                unsaved_array[i].name = b(name)
+                unsaved_array[i].name = b(fspath(name))
                 unsaved_array[i].contents = b(contents)
                 unsaved_array[i].length = len(contents)
 
-        ptr = conf.lib.clang_parseTranslationUnit(index, filename, args_array,
+        ptr = conf.lib.clang_parseTranslationUnit(index,
+                                    fspath(filename) if filename is not None else None,
+                                    args_array,
                                     len(args), unsaved_array,
                                     len(unsaved_files), options)
 
@@ -2826,11 +2837,13 @@ class TranslationUnit(ClangObject):
 
         index is optional and is the Index instance to use. If not provided,
         a default Index will be created.
+
+        filename can be str or PathLike.
         """
         if index is None:
             index = Index.create()
 
-        ptr = conf.lib.clang_createTranslationUnit(index, filename)
+        ptr = conf.lib.clang_createTranslationUnit(index, fspath(filename))
         if not ptr:
             raise TranslationUnitLoadError(filename)
 
@@ -2983,7 +2996,7 @@ class TranslationUnit(ClangObject):
                     print(value)
                 if not isinstance(value, str):
                     raise TypeError('Unexpected unsaved file contents.')
-                unsaved_files_array[i].name = name
+                unsaved_files_array[i].name = fspath(name)
                 unsaved_files_array[i].contents = value
                 unsaved_files_array[i].length = len(value)
         ptr = conf.lib.clang_reparseTranslationUnit(self, len(unsaved_files),
@@ -3002,10 +3015,10 @@ class TranslationUnit(ClangObject):
         case, the reason(s) why should be available via
         TranslationUnit.diagnostics().
 
-        filename -- The path to save the translation unit to.
+        filename -- The path to save the translation unit to (str or PathLike).
         """
         options = conf.lib.clang_defaultSaveOptions(self)
-        result = int(conf.lib.clang_saveTranslationUnit(self, filename,
+        result = int(conf.lib.clang_saveTranslationUnit(self, fspath(filename),
                                                         options))
         if result != 0:
             raise TranslationUnitSaveError(result,
@@ -3047,10 +3060,10 @@ class TranslationUnit(ClangObject):
                     print(value)
                 if not isinstance(value, str):
                     raise TypeError('Unexpected unsaved file contents.')
-                unsaved_files_array[i].name = b(name)
+                unsaved_files_array[i].name = b(fspath(name))
                 unsaved_files_array[i].contents = b(value)
                 unsaved_files_array[i].length = len(value)
-        ptr = conf.lib.clang_codeCompleteAt(self, path, line, column,
+        ptr = conf.lib.clang_codeCompleteAt(self, fspath(path), line, column,
                 unsaved_files_array, len(unsaved_files), options)
         if ptr:
             return CodeCompletionResults(ptr)
@@ -3078,7 +3091,7 @@ class File(ClangObject):
     @staticmethod
     def from_name(translation_unit, file_name):
         """Retrieve a file handle within the given translation unit."""
-        return File(conf.lib.clang_getFile(translation_unit, file_name))
+        return File(conf.lib.clang_getFile(translation_unit, fspath(file_name)))
 
     @property
     def name(self):
@@ -3229,7 +3242,7 @@ class CompilationDatabase(ClangObject):
         """Builds a CompilationDatabase from the database found in buildDir"""
         errorCode = c_uint()
         try:
-            cdb = conf.lib.clang_CompilationDatabase_fromDirectory(buildDir,
+            cdb = conf.lib.clang_CompilationDatabase_fromDirectory(fspath(buildDir),
                 byref(errorCode))
         except CompilationDatabaseError as e:
             raise CompilationDatabaseError(int(errorCode.value),
@@ -3242,7 +3255,7 @@ class CompilationDatabase(ClangObject):
         build filename. Returns None if filename is not found in the database.
         """
         return conf.lib.clang_CompilationDatabase_getCompileCommands(self,
-                                                                     filename)
+                                                                     fspath(filename))
 
     def getAllCompileCommands(self):
         """
@@ -4090,7 +4103,7 @@ class Config:
             raise Exception("library path must be set before before using " \
                             "any other functionalities in libclang.")
 
-        Config.library_path = path
+        Config.library_path = fspath(path)
 
     @staticmethod
     def set_library_file(filename):
@@ -4099,7 +4112,7 @@ class Config:
             raise Exception("library file must be set before before using " \
                             "any other functionalities in libclang.")
 
-        Config.library_file = filename
+        Config.library_file = fspath(filename)
 
     @staticmethod
     def set_compatibility_check(check_status):

clang/bindings/python/tests/cindex/test_cdb.py

@@ -11,6 +11,8 @@ import os
 import gc
 import unittest
 import sys
+from .util import skip_if_no_fspath
+from .util import str_to_path
 
 
 kInputsDir = os.path.join(os.path.dirname(__file__), 'INPUTS')
@@ -37,6 +39,13 @@ class TestCDB(unittest.TestCase):
         cmds = cdb.getCompileCommands('/home/john.doe/MyProject/project.cpp')
         self.assertNotEqual(len(cmds), 0)
 
+    @skip_if_no_fspath
+    def test_lookup_succeed_pathlike(self):
+        """Same as test_lookup_succeed, but with PathLikes"""
+        cdb = CompilationDatabase.fromDirectory(str_to_path(kInputsDir))
+        cmds = cdb.getCompileCommands(str_to_path('/home/john.doe/MyProject/project.cpp'))
+        self.assertNotEqual(len(cmds), 0)
+
     def test_all_compilecommand(self):
         """Check we get all results from the db"""
         cdb = CompilationDatabase.fromDirectory(kInputsDir)

clang/bindings/python/tests/cindex/test_code_completion.py

@@ -6,6 +6,8 @@ if 'CLANG_LIBRARY_PATH' in os.environ:
 from clang.cindex import TranslationUnit
 
 import unittest
+from .util import skip_if_no_fspath
+from .util import str_to_path
 
 
 class TestCodeCompletion(unittest.TestCase):
@@ -43,6 +45,32 @@ void f() {
         ]
         self.check_completion_results(cr, expected)
 
+    @skip_if_no_fspath
+    def test_code_complete_pathlike(self):
+        files = [(str_to_path('fake.c'), """
+/// Aaa.
+int test1;
+
+/// Bbb.
+void test2(void);
+
+void f() {
+
+}
+""")]
+
+        tu = TranslationUnit.from_source(str_to_path('fake.c'), ['-std=c99'], unsaved_files=files,
+                options=TranslationUnit.PARSE_INCLUDE_BRIEF_COMMENTS_IN_CODE_COMPLETION)
+
+        cr = tu.codeComplete(str_to_path('fake.c'), 9, 1, unsaved_files=files, include_brief_comments=True)
+
+        expected = [
+          "{'int', ResultType} | {'test1', TypedText} || Priority: 50 || Availability: Available || Brief comment: Aaa.",
+          "{'void', ResultType} | {'test2', TypedText} | {'(', LeftParen} | {')', RightParen} || Priority: 50 || Availability: Available || Brief comment: Bbb.",
+          "{'return', TypedText} || Priority: 40 || Availability: Available || Brief comment: None"
+        ]
+        self.check_completion_results(cr, expected)
+
     def test_code_complete_availability(self):
         files = [('fake.cpp', """
 class P {

clang/bindings/python/tests/cindex/test_translation_unit.py

@@ -20,6 +20,8 @@ from clang.cindex import TranslationUnitLoadError
 from clang.cindex import TranslationUnit
 from .util import get_cursor
 from .util import get_tu
+from .util import skip_if_no_fspath
+from .util import str_to_path
 
 
 kInputsDir = os.path.join(os.path.dirname(__file__), 'INPUTS')
@@ -36,6 +38,17 @@ def save_tu(tu):
         yield t.name
 
 
+@contextmanager
+def save_tu_pathlike(tu):
+    """Convenience API to save a TranslationUnit to a file.
+
+    Returns the filename it was saved to.
+    """
+    with tempfile.NamedTemporaryFile() as t:
+        tu.save(str_to_path(t.name))
+        yield t.name
+
+
 class TestTranslationUnit(unittest.TestCase):
     def test_spelling(self):
         path = os.path.join(kInputsDir, 'hello.cpp')
@@ -89,6 +102,22 @@ int SOME_DEFINE;
         spellings = [c.spelling for c in tu.cursor.get_children()]
         self.assertEqual(spellings[-1], 'x')
 
+    @skip_if_no_fspath
+    def test_from_source_accepts_pathlike(self):
+        tu = TranslationUnit.from_source(str_to_path('fake.c'), ['-Iincludes'], unsaved_files = [
+                (str_to_path('fake.c'), """
+#include "fake.h"
+    int x;
+    int SOME_DEFINE;
+    """),
+                    (str_to_path('includes/fake.h'), """
+#define SOME_DEFINE y
+    """)
+                ])
+        spellings = [c.spelling for c in tu.cursor.get_children()]
+        self.assertEqual(spellings[-2], 'x')
+        self.assertEqual(spellings[-1], 'y')
+
     def assert_normpaths_equal(self, path1, path2):
         """ Compares two paths for equality after normalizing them with
             os.path.normpath
@@ -135,6 +164,16 @@ int SOME_DEFINE;
             self.assertTrue(os.path.exists(path))
             self.assertGreater(os.path.getsize(path), 0)
 
+    @skip_if_no_fspath
+    def test_save_pathlike(self):
+        """Ensure TranslationUnit.save() works with PathLike filename."""
+
+        tu = get_tu('int foo();')
+
+        with save_tu_pathlike(tu) as path:
+            self.assertTrue(os.path.exists(path))
+            self.assertGreater(os.path.getsize(path), 0)
+
     def test_save_translation_errors(self):
         """Ensure that saving to an invalid directory raises."""
 
@@ -167,6 +206,22 @@ int SOME_DEFINE;
             # Just in case there is an open file descriptor somewhere.
             del tu2
 
+    @skip_if_no_fspath
+    def test_load_pathlike(self):
+        """Ensure TranslationUnits can be constructed from saved files -
+        PathLike variant."""
+        tu = get_tu('int foo();')
+        self.assertEqual(len(tu.diagnostics), 0)
+        with save_tu(tu) as path:
+            tu2 = TranslationUnit.from_ast_file(filename=str_to_path(path))
+            self.assertEqual(len(tu2.diagnostics), 0)
+
+            foo = get_cursor(tu2, 'foo')
+            self.assertIsNotNone(foo)
+
+            # Just in case there is an open file descriptor somewhere.
+            del tu2
+
     def test_index_parse(self):
         path = os.path.join(kInputsDir, 'hello.cpp')
         index = Index.create()
@@ -185,6 +240,19 @@ int SOME_DEFINE;
         with self.assertRaises(Exception):
             f = tu.get_file('foobar.cpp')
 
+    @skip_if_no_fspath
+    def test_get_file_pathlike(self):
+        """Ensure tu.get_file() works appropriately with PathLike filenames."""
+
+        tu = get_tu('int foo();')
+
+        f = tu.get_file(str_to_path('t.c'))
+        self.assertIsInstance(f, File)
+        self.assertEqual(f.name, 't.c')
+
+        with self.assertRaises(Exception):
+            f = tu.get_file(str_to_path('foobar.cpp'))
+
     def test_get_source_location(self):
         """Ensure tu.get_source_location() works."""
 

clang/bindings/python/tests/cindex/util.py

@@ -1,5 +1,15 @@
 # This file provides common utility functions for the test suite.
 
+import os
+HAS_FSPATH = hasattr(os, 'fspath')
+
+if HAS_FSPATH:
+    from pathlib import Path as str_to_path
+else:
+    str_to_path = None
+
+import unittest
+
 from clang.cindex import Cursor
 from clang.cindex import TranslationUnit
 
@@ -68,8 +78,13 @@ def get_cursors(source, spelling):
     return cursors
 
 
+skip_if_no_fspath = unittest.skipUnless(HAS_FSPATH,
+                                        "Requires file system path protocol / Python 3.6+")
+
 __all__ = [
     'get_cursor',
     'get_cursors',
     'get_tu',
+    'skip_if_no_fspath',
+    'str_to_path',
 ]