[python] Support PathLike filenames and directories

[jstasiak]

Summary

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

[mgorny]

Also please remember to submit patches with -U9999, so that Phab has full context.

bindings/python/tests/cindex/test_cdb.py

42

Please use conditional skipping instead. Maybe write a @skip_if_no_fspath decorator in util.py and use it for all tests. Skipping will have the advantage that instead of silently pretending we have less tests, we'd explicitly tell user why some of the tests aren't run.

bindings/python/tests/cindex/util.py

6

Wouldn't it be better to use if HAS_FSPATH here? In normal circumstances, the pathlib import will be unnecessarily attempted (and it will succeed if pathlib package is installed), even though it will never be used inside tests.

[jstasiak]

Tests are skipped using unittest skipping mechanism now and pathlib imported only when necessary.

[jstasiak]

Thanks for the feedback, you're right those were things worth improving, I updated the code.

[mgorny]

LGTM, thanks!

bindings/python/clang/cindex.py

133

Optionally: this is now a little bit nitpick but could you use a different variable name (e.g. commonly used elsewhere x)? This could be a little bit confusing with Python's string module.

[jstasiak]

That's fair, changed string to just x, should be obvious from the context what x is.

Thank you for the review. As I don't have commit access I'd like to ask you to commit this on my behalf.

[mgorny]

Merged. I will get back to you if something explodes ;-).

bindings/python/clang/cindex.py

 
 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.

     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(string):
+        return string
 
 # 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

         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
-        second element defines the content. Content can be provided as str
-        source code or as file objects (anything with a read() method). If
-        a file object is being used, content will be read until EOF and the
-        read cursor will not be reset to its original position.
+        iterable of 2-tuples. The first element is the filename (str or
+        PathLike). The second element defines the content. Content can be
+        provided as str source code or as file objects (anything with a read()
+        method). If a file object is being used, content will be read until EOF
+        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.

                 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)
 

 
         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)
 

                     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),

         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,

                     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)

     @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):

         """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),

         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):
         """

             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):

             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):

bindings/python/tests/cindex/test_cdb.py

 import gc
 import unittest
 import sys
+from .util import HAS_FSPATH
+from .util import str_to_path
 
 
 kInputsDir = os.path.join(os.path.dirname(__file__), 'INPUTS')

         cmds = cdb.getCompileCommands('/home/john.doe/MyProject/project.cpp')
         self.assertNotEqual(len(cmds), 0)
 
+    if HAS_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)

bindings/python/tests/cindex/test_code_completion.py

 from clang.cindex import TranslationUnit
 
 import unittest
+from .util import HAS_FSPATH
+from .util import str_to_path
 
 
 class TestCodeCompletion(unittest.TestCase):

         ]
         self.check_completion_results(cr, expected)
 
+    if HAS_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 {

bindings/python/tests/cindex/test_translation_unit.py

 from clang.cindex import TranslationUnit
 from .util import get_cursor
 from .util import get_tu
+from .util import HAS_FSPATH
+from .util import str_to_path
 
 
 kInputsDir = os.path.join(os.path.dirname(__file__), 'INPUTS')

         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')

         spellings = [c.spelling for c in tu.cursor.get_children()]
         self.assertEqual(spellings[-1], 'x')
 
+    if HAS_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

             self.assertTrue(os.path.exists(path))
             self.assertGreater(os.path.getsize(path), 0)
 
+    if HAS_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."""
 

             # Just in case there is an open file descriptor somewhere.
             del tu2
 
+            # We can also pass the filename as Path-like object
+            if HAS_FSPATH:
+                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()

         with self.assertRaises(Exception):
             f = tu.get_file('foobar.cpp')
 
+    if HAS_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."""
 

bindings/python/tests/cindex/util.py

 # This file provides common utility functions for the test suite.
 
+import os
+HAS_FSPATH = hasattr(os, 'fspath')
+
+try:
+    import pathlib
+except ImportError:
+    str_to_path = None
+else:
+    str_to_path = pathlib.Path
+
 from clang.cindex import Cursor
 from clang.cindex import TranslationUnit
 

     'get_cursor',
     'get_cursors',
     'get_tu',
+    'HAS_FSPATH',
+    'str_to_path',
 ]