fix: broken dynamic import of rplugin modules #534

The removal of `imp` package (#461) in order to supprot Python 3.12
had a bug where rplugins can't be loaded and the ImportError exception
was silenced, making the remote provider throwing lots of errors.
This commit fixes broken dynamic import of python modules from the
registered rplugins.

We add tests for Host._load, with loading rplugins consisting of:
  (1) single-file module  (e.g., `rplugins/simple_plugin.py`)
  (2) package (e.g., `rplugins/mymodule/__init__.py`)

pynvim/plugin/host.py

@@ -1,10 +1,13 @@
 # type: ignore
 """Implements a Nvim host for python plugins."""
+
+import importlib
 import inspect
 import logging
 import os
 import os.path
 import re
+import sys
 from functools import partial
 from traceback import format_exc
 from typing import Any, Sequence
@@ -23,25 +26,18 @@
 host_method_spec = {"poll": {}, "specs": {"nargs": 1}, "shutdown": {}}
 
 
-def handle_import(directory, name):
-    """Import a python file given a known location.
+def _handle_import(path: str, name: str):
+    """Import python module `name` from a known file path or module directory.
 
-    Currently works on both python2 or 3.
+    The path should be the base directory from which the module can be imported.
+    To support python 3.12, the use of `imp` should be avoided.
+    @see https://docs.python.org/3.12/whatsnew/3.12.html#imp
     """
-    try:  # Python3
-        from importlib.util import module_from_spec, spec_from_file_location
-    except ImportError:  # Python2.7
-        import imp
-        from pynvim.compat import find_module
-        file, pathname, descr = find_module(name, [directory])
-        module = imp.load_module(name, file, pathname, descr)
-        return module
-    else:
-        spec = spec_from_file_location(name, location=directory)
-        if spec is not None:
-            return module_from_spec(spec)
-        else:
-            raise ImportError
+    if not name:
+        raise ValueError("Missing module name.")
+
+    sys.path.append(path)
+    return importlib.import_module(name)
 
 
 class Host(object):
@@ -167,22 +163,28 @@ def _missing_handler_error(self, name, kind):
         return msg
 
     def _load(self, plugins: Sequence[str]) -> None:
+        """Load the remote plugins and register handlers defined in the plugins.
+
+        Args:
+            plugins: List of plugin paths to rplugin python modules
+                registered by remote#host#RegisterPlugin('python3', ...)
+                (see the generated rplugin.vim manifest)
+        """
+        # self.nvim.err_write("host init _load\n", async_=True)
         has_script = False
         for path in plugins:
+            path = os.path.normpath(path)  # normalize path
             err = None
             if path in self._loaded:
-                error('{} is already loaded'.format(path))
+                warn('{} is already loaded'.format(path))
                 continue
             try:
                 if path == "script_host.py":
                     module = script_host
                     has_script = True
                 else:
                     directory, name = os.path.split(os.path.splitext(path)[0])
-                    try:
-                        module = handle_import(directory, name)
-                    except ImportError:
-                        return
+                    module = _handle_import(directory, name)
                 handlers = []
                 self._discover_classes(module, handlers, path)
                 self._discover_functions(module, handlers, path, False)

test/fixtures/module_plugin/rplugin/python3/mymodule/__init__.py

@@ -0,0 +1,9 @@
+"""The `mymodule` package for the fixture module plugin."""
+# pylint: disable=all
+
+# Somehow the plugin might be using relative imports.
+from .plugin import MyPlugin as MyPlugin
+
+# ... or absolute import (assuming this is the root package)
+import mymodule.plugin  # noqa: I100
+assert mymodule.plugin.MyPlugin is MyPlugin

test/fixtures/module_plugin/rplugin/python3/mymodule/plugin.py

@@ -0,0 +1,13 @@
+"""Actual implement lies here."""
+import pynvim as neovim
+import pynvim.api
+
+
+@neovim.plugin
+class MyPlugin:
+    def __init__(self, nvim: pynvim.api.Nvim):
+        self.nvim = nvim
+
+    @neovim.command("ModuleHelloWorld")
+    def hello_world(self) -> None:
+        self.nvim.command("echom 'MyPlugin: Hello World!'")

test/fixtures/simple_plugin/rplugin/python3/simple_nvim.py

@@ -0,0 +1,13 @@
+import neovim
+
+import pynvim.api
+
+
+@neovim.plugin
+class SimplePlugin:
+    def __init__(self, nvim: pynvim.api.Nvim):
+        self.nvim = nvim
+
+    @neovim.command("SimpleHelloWorld")
+    def hello_world(self) -> None:
+        self.nvim.command("echom 'SimplePlugin: Hello World!'")

test/test_host.py

@@ -1,8 +1,13 @@
-# -*- coding: utf-8 -*-
 # type: ignore
+# pylint: disable=protected-access
+import os
+from typing import Sequence
+
 from pynvim.plugin.host import Host, host_method_spec
 from pynvim.plugin.script_host import ScriptHost
 
+__PATH__ = os.path.abspath(os.path.dirname(__file__))
+
 
 def test_host_imports(vim):
     h = ScriptHost(vim)
@@ -11,6 +16,24 @@ def test_host_imports(vim):
     assert h.module.__dict__['sys']
 
 
+def test_host_import_rplugin_modules(vim):
+    # Test whether a Host can load and import rplugins (#461).
+    # See also $VIMRUNTIME/autoload/provider/pythonx.vim.
+    h = Host(vim)
+    plugins: Sequence[str] = [  # plugin paths like real rplugins
+        os.path.join(__PATH__, "./fixtures/simple_plugin/rplugin/python3/simple_nvim.py"),
+        os.path.join(__PATH__, "./fixtures/module_plugin/rplugin/python3/mymodule/"),
+        os.path.join(__PATH__, "./fixtures/module_plugin/rplugin/python3/mymodule"),  # duplicate
+    ]
+    h._load(plugins)
+    assert len(h._loaded) == 2
+
+    # pylint: disable-next=unbalanced-tuple-unpacking
+    simple_nvim, mymodule = list(h._loaded.values())
+    assert simple_nvim['module'].__name__ == 'simple_nvim'
+    assert mymodule['module'].__name__ == 'mymodule'
+
+
 def test_host_clientinfo(vim):
     h = Host(vim)
     assert h._request_handlers.keys() == host_method_spec.keys()