[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

 #
 # o implement additional SourceLocation, SourceRange, and File methods.
 
 from ctypes import *
 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.
     class c_interop_string(c_char_p):
 
         def __init__(self, p=None):
             if p is None:
                 p = ""
elif sys.version_info[0] == 2:
     def _to_python_string(x, *args):
         return x
 
     c_interop_string.to_python_string = staticmethod(_to_python_string)
 
     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
 # integer and pass the wrong value on platforms where int != void*. Work around
 # this by marshalling object arguments as void**.
 c_object_p = POINTER(c_void_p)
 
 callbacks = {}
def from_source(cls, filename, args=None, unsaved_files=None, options=0,
         This is capable of processing source code both from files on the
         filesystem as well as in-memory contents.
 
         Command-line arguments that would be passed to clang are specified as
         a list via args. These can be used to specify include paths, warnings,
         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.
 
         index is an Index instance to utilize. If not provided, a new Index
         will be created for this TranslationUnit.
 
         To parse source from the filesystem, the filename of the file to parse
def from_source(cls, filename, args=None, unsaved_files=None, options=0,
 
         unsaved_array = None
         if len(unsaved_files) > 0:
             unsaved_array = (_CXUnsavedFile * len(unsaved_files))()
             for i, (name, contents) in enumerate(unsaved_files):
                 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)
 
         if not ptr:
             raise TranslationUnitLoadError("Error parsing translation unit.")
 
         return cls(ptr, index=index)
 
     @classmethod
     def from_ast_file(cls, filename, index=None):
         """Create a TranslationUnit instance from a saved AST file.
 
         A previously-saved AST file (provided with -emit-ast or
         TranslationUnit.save()) is loaded from the filename specified.
 
         If the file cannot be loaded, a TranslationUnitLoadError will be
         raised.
 
         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)
 
         return cls(ptr=ptr, index=index)
 
     def __init__(self, ptr, index):
         """Create a TranslationUnit instance.
 
def reparse(self, unsaved_files=None, options=0):
             for i,(name,value) in enumerate(unsaved_files):
                 if not isinstance(value, str):
                     # FIXME: It would be great to support an efficient version
                     # of this, one day.
                     value = value.read()
                     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),
                 unsaved_files_array, options)
 
     def save(self, filename):
         """Saves the TranslationUnit to a file.
 
         This is equivalent to passing -emit-ast to the clang frontend. The
         saved file can be loaded back into a TranslationUnit. Or, if it
         corresponds to a header, it can be used as a pre-compiled header file.
 
         If an error occurs while saving, a TranslationUnitSaveError is raised.
         If the error was TranslationUnitSaveError.ERROR_INVALID_TU, this means
         the constructed TranslationUnit was not valid at time of save. In this
         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,
                 'Error saving TranslationUnit.')
 
     def codeComplete(self, path, line, column, unsaved_files=None,
                      include_macros=False, include_code_patterns=False,
                      include_brief_comments=False):
def codeComplete(self, path, line, column, unsaved_files=None,
             for i,(name,value) in enumerate(unsaved_files):
                 if not isinstance(value, str):
                     # FIXME: It would be great to support an efficient version
                     # of this, one day.
                     value = value.read()
                     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)
         return None
 
     def get_tokens(self, locations=None, extent=None):
         """Obtain tokens in this translation unit.
 
class File(ClangObject):
     """
     The File class represents a particular source file that is part of a
     translation unit.
     """
 
     @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):
         """Return the complete file and path name of the file."""
         return conf.lib.clang_getFileName(self)
 
     @property
     def time(self):
def from_result(res, fn, args):
                                            "CompilationDatabase loading failed")
         return CompilationDatabase(res)
 
     @staticmethod
     def fromDirectory(buildDir):
         """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),
                                            "CompilationDatabase loading failed")
         return cdb
 
     def getCompileCommands(self, filename):
         """
         Get an iterable object providing all the CompileCommands available to
         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):
         """
         Get an iterable object providing all the CompileCommands available from
         the database.
         """
         return conf.lib.clang_CompilationDatabase_getAllCompileCommands(self)
 
class Config:
 
     @staticmethod
     def set_library_path(path):
         """Set the path in which to search for libclang"""
         if Config.loaded:
             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):
         """Set the exact location of libclang"""
         if Config.loaded:
             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):
         """ Perform compatibility check when loading libclang
 
         The python bindings are only tested and evaluated with the version of
         libclang they are provided with. To ensure correct behavior a (limited)
         compatibility check is performed when loading the bindings. This check

bindings/python/tests/cindex/test_cdb.py

 import os
 from clang.cindex import Config
 if 'CLANG_LIBRARY_PATH' in os.environ:
     Config.set_library_path(os.environ['CLANG_LIBRARY_PATH'])
 
 from clang.cindex import CompilationDatabase
 from clang.cindex import CompilationDatabaseError
 from clang.cindex import CompileCommands
 from clang.cindex import CompileCommand
 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')
 
 
 @unittest.skipIf(sys.platform == 'win32', "TODO: Fix these tests on Windows")
 class TestCDB(unittest.TestCase):
     def test_create_fail(self):
def test_create(self):
         cdb = CompilationDatabase.fromDirectory(kInputsDir)
 
     def test_lookup_succeed(self):
         """Check we get some results if the file exists in the db"""
         cdb = CompilationDatabase.fromDirectory(kInputsDir)
         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)
         cmds = cdb.getAllCompileCommands()
         self.assertEqual(len(cmds), 3)
         expected = [
             { 'wd': '/home/john.doe/MyProject',
               'file': '/home/john.doe/MyProject/project.cpp',

bindings/python/tests/cindex/test_code_completion.py

 import os
 from clang.cindex import Config
 if 'CLANG_LIBRARY_PATH' in os.environ:
     Config.set_library_path(os.environ['CLANG_LIBRARY_PATH'])
 
 from clang.cindex import TranslationUnit
 
 import unittest
+from .util import skip_if_no_fspath
+from .util import str_to_path
 
 
 class TestCodeCompletion(unittest.TestCase):
     def check_completion_results(self, cr, expected):
         self.assertIsNotNone(cr)
         self.assertEqual(len(cr.diagnostics), 0)
 
         completions = [str(c) for c in cr.results]
""")]
 
         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)
 
+    @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 {
 protected:
   int member;
 };
 
 class Q : public P {

bindings/python/tests/cindex/test_translation_unit.py

 from clang.cindex import Index
 from clang.cindex import SourceLocation
 from clang.cindex import SourceRange
 from clang.cindex import TranslationUnitSaveError
 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')
 
 
 @contextmanager
 def save_tu(tu):
     """Convenience API to save a TranslationUnit to a file.
 
     Returns the filename it was saved to.
     """
     with tempfile.NamedTemporaryFile() as t:
         tu.save(t.name)
         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')
         tu = TranslationUnit.from_source(path)
         self.assertEqual(tu.spelling, path)
 
     def test_cursor(self):
         path = os.path.join(kInputsDir, 'hello.cpp')
def test_unsaved_files_2(self):
             from StringIO import StringIO
         except:
             from io import StringIO
         tu = TranslationUnit.from_source('fake.c', unsaved_files = [
                 ('fake.c', StringIO('int x;'))])
         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
         """
         self.assertEqual(os.path.normpath(path1),
                          os.path.normpath(path2))
 
     def test_includes(self):
def test_save(self):
         """Ensure TranslationUnit.save() works."""
 
         tu = get_tu('int foo();')
 
         with save_tu(tu) as path:
             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."""
 
         tu = get_tu('int foo();')
 
         path = '/does/not/exist/llvm-test.ast'
         self.assertFalse(os.path.exists(os.path.dirname(path)))
 
def test_load(self):
             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
 
+    @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()
         tu = index.parse(path)
         self.assertIsInstance(tu, TranslationUnit)
 
     def test_get_file(self):
         """Ensure tu.get_file() works appropriately."""
 
         tu = get_tu('int foo();')
 
         f = tu.get_file('t.c')
         self.assertIsInstance(f, File)
         self.assertEqual(f.name, 't.c')
 
         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."""
 
         tu = get_tu('int foo();')
 
         location = tu.get_location('t.c', 2)
         self.assertIsInstance(location, SourceLocation)
         self.assertEqual(location.offset, 2)

bindings/python/tests/cindex/util.py

 # 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
 
 def get_tu(source, lang='c', all_warnings=False, flags=[]):
     """Obtain a translation unit from source and language.
 
     By default, the translation unit is created from source file "t.<ext>"
     where <ext> is the default file extension for the specified language. By
def get_cursors(source, spelling):
     cursors = []
     for cursor in root_cursor.walk_preorder():
         if cursor.spelling == spelling:
             cursors.append(cursor)
 
     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',
 ]