Merged PR posit-dev/positron-python#265: Prefer namespace completions

Merge pull request #265 from posit-dev/prefer-namespace-completions

Prefer namespace completions
--------------------
Commit message for posit-dev/positron-python@8359792:

test that namespace completions are preferred

--------------------
Commit message for posit-dev/positron-python@1d3855a:

prefer completions using the user's namespace over static analysis

Relates to #601.


Authored-by: Wasim Lorgat <[email protected]>
Signed-off-by: Wasim Lorgat <[email protected]>

Author: seeM

extensions/positron-python/pythonFiles/positron/jedi.py

@@ -0,0 +1,63 @@
+#
+# Copyright (C) 2023 Posit Software, PBC. All rights reserved.
+#
+
+from jedi import cache
+from jedi.api import Interpreter
+from jedi.api.interpreter import MixedModuleContext
+from jedi.file_io import KnownContentFileIO
+from jedi.inference.value import ModuleValue
+
+
+class PositronMixedModuleContext(MixedModuleContext):
+    """
+    Like `jedi.api.interpreter.MixedModuleContext` but prefers values from the user's namespace over
+    static analysis. See the `PositronInterpreter` docs for more details.
+    """
+
+    def get_filters(self, until_position=None, origin_scope=None):
+        filters = super().get_filters(until_position, origin_scope)
+
+        # Store the first filter – which corresponds to static analysis of the source code.
+        merged_filter = next(filters)
+
+        # Yield the remaining filters – which correspond to the user's namespaces.
+        yield from filters
+
+        # Finally, yield the first filter.
+        yield merged_filter
+
+
+class PositronInterpreter(Interpreter):
+    """
+    Like `jedi.Interpreter` but prefers values from the user's namespace over static analysis.
+
+    For example, given the namespace: `{"x": {"a": 0}}`, and the code:
+
+    ```
+    x = {"b": 0}
+    x['
+    ```
+
+    Completing the line `x['` should return `a` and not `b`.
+    """
+
+    @cache.memoize_method
+    def _get_module_context(self):
+        if self.path is None:
+            file_io = None
+        else:
+            file_io = KnownContentFileIO(self.path, self._code)
+        tree_module_value = ModuleValue(
+            self._inference_state,
+            self._module_node,
+            file_io=file_io,
+            string_names=("__main__",),
+            code_lines=self._code_lines,
+        )
+        # --- Start Positron ---
+        return PositronMixedModuleContext(
+            tree_module_value,
+            self.namespaces,
+        )
+        # --- End Positron ---

extensions/positron-python/pythonFiles/positron/positron_jedilsp.py

@@ -88,6 +88,7 @@
 from pygls.workspace import Document
 
 from .help import ShowTopicRequest
+from .jedi import PositronInterpreter
 
 if TYPE_CHECKING:
     from .positron_ipkernel import PositronIPyKernel
@@ -675,4 +676,4 @@ def interpreter(
     if kernel is not None:
         namespaces.append(kernel.get_user_ns())
 
-    return Interpreter(document.source, namespaces, path=document.path, project=project)
+    return PositronInterpreter(document.source, namespaces, path=document.path, project=project)

extensions/positron-python/pythonFiles/tests/positron/test_positron_jedilsp.py

@@ -1,30 +1,53 @@
-from typing import Any, Dict, Optional, Tuple
+from typing import Any, Dict, Callable, List, Optional, Tuple
 from unittest.mock import Mock
-from urllib.parse import unquote, urlparse
 
 import pytest
+from IPython.terminal.interactiveshell import TerminalInteractiveShell
 from jedi import Project
-from lsprotocol.types import Position, TextDocumentIdentifier
+from positron.positron_ipkernel import PositronIPyKernel
+from pygls.workspace.text_document import TextDocument
+from lsprotocol.types import (
+    CompletionParams,
+    MarkupKind,
+    Position,
+    TextDocumentIdentifier,
+)
 
 from positron.help import ShowTopicRequest
-from positron.positron_jedilsp import HelpTopicParams, positron_help_topic_request
+from positron.positron_jedilsp import (
+    HelpTopicParams,
+    positron_completion,
+    positron_help_topic_request,
+)
+
 
+@pytest.fixture
+def mock_server(kernel: PositronIPyKernel) -> Callable[[str, str], Mock]:
+    """
+    Minimum interface for a pylgs server to support LSP unit tests.
+    """
 
-def mock_server(uri: str, source: str, namespace: Dict[str, Any]) -> Mock:
-    document = Mock()
-    document.path = unquote(urlparse(uri).path)
-    document.source = source
+    # Return a function that returns a mock server rather than an instantiated mock server,
+    # since uri and source change between tests.
+    def inner(uri: str, source: str) -> Mock:
+        server = Mock()
+        server.client_capabilities.text_document.completion.completion_item.documentation_format = (
+            list(MarkupKind)
+        )
+        server.initialization_options.completion.disable_snippets = False
+        server.initialization_options.completion.resolve_eagerly = False
+        server.initialization_options.completion.ignore_patterns = []
+        server.kernel = kernel
+        server.project = Project("")
+        server.workspace.get_document.return_value = TextDocument(uri, source)
 
-    server = Mock()
-    server.kernel.get_user_ns.return_value = namespace
-    server.project = Project("")
-    server.workspace.get_document.return_value = document
+        return server
 
-    return server
+    return inner
 
 
 @pytest.mark.parametrize(
-    ("source", "position", "namespace", "topic"),
+    ("source", "position", "namespace", "expected_topic"),
     [
         # An unknown variable should not be resolved.
         ("x", (0, 0), {}, None),
@@ -33,12 +56,53 @@ def mock_server(uri: str, source: str, namespace: Dict[str, Any]) -> Mock:
     ],
 )
 def test_positron_help_topic_request(
-    source: str, position: Tuple[int, int], namespace: Dict[str, Any], topic: Optional[str]
+    mock_server: Mock,
+    shell: TerminalInteractiveShell,
+    source: str,
+    position: Tuple[int, int],
+    namespace: Dict[str, Any],
+    expected_topic: Optional[str],
 ) -> None:
+    shell.user_ns.update(namespace)
+
     params = HelpTopicParams(TextDocumentIdentifier("file:///foo.py"), Position(*position))
-    server = mock_server(params.text_document.uri, source, namespace)
-    actual = positron_help_topic_request(server, params)
-    if topic is None:
-        assert actual is None
+    server = mock_server(params.text_document.uri, source)
+
+    topic = positron_help_topic_request(server, params)
+
+    if expected_topic is None:
+        assert topic is None
     else:
-        assert actual == ShowTopicRequest(topic)
+        assert topic == ShowTopicRequest(expected_topic)
+
+
+@pytest.mark.parametrize(
+    ("source", "position", "namespace", "expected_completions"),
+    [
+        # When completions match a variable defined in the source _and_ a variable in the user's namespace,
+        # prefer the namespace variable.
+        ('x = {"a": 0}\nx["', (1, 3), {"x": {"b": 0}}, ['"b"']),
+    ],
+)
+def test_positron_completion(
+    mock_server: Mock,
+    shell: TerminalInteractiveShell,
+    source: str,
+    position: Tuple[int, int],
+    namespace: Dict[str, Any],
+    expected_completions: List[str],
+) -> None:
+    shell.user_ns.update(namespace)
+
+    params = CompletionParams(TextDocumentIdentifier("file:///foo.py"), Position(*position))
+    server = mock_server(params.text_document.uri, source)
+
+    completion_list = positron_completion(server, params)
+
+    assert completion_list is not None, "No completions returned"
+
+    # TODO: This is actually a bug, we shouldn't be returning magic completions when completing dict keys.
+    completions = [item for item in completion_list.items if not item.label.startswith("%")]
+
+    completion_labels = [item.label for item in completions]
+    assert completion_labels == expected_completions, "Unexpected completion labels"