From d724e6cb9c2ff6bb292184195916d0c9d75d8045 Mon Sep 17 00:00:00 2001 From: Wasim Lorgat Date: Thu, 7 Dec 2023 11:42:50 +0000 Subject: [PATCH] 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@83597922b77aa08dab9f54df96bf618c667c72a2: test that namespace completions are preferred -------------------- Commit message for posit-dev/positron-python@1d3855a05addcb9952355085d2e044c5dedb70cf: prefer completions using the user's namespace over static analysis Relates to https://github.com/posit-dev/positron/issues/601. Authored-by: Wasim Lorgat Signed-off-by: Wasim Lorgat --- .../pythonFiles/positron/jedi.py | 63 +++++++++++ .../pythonFiles/positron/positron_jedilsp.py | 3 +- .../tests/positron/test_positron_jedilsp.py | 104 ++++++++++++++---- 3 files changed, 149 insertions(+), 21 deletions(-) create mode 100644 extensions/positron-python/pythonFiles/positron/jedi.py diff --git a/extensions/positron-python/pythonFiles/positron/jedi.py b/extensions/positron-python/pythonFiles/positron/jedi.py new file mode 100644 index 00000000000..4442f07c208 --- /dev/null +++ b/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 --- diff --git a/extensions/positron-python/pythonFiles/positron/positron_jedilsp.py b/extensions/positron-python/pythonFiles/positron/positron_jedilsp.py index 02f23aa2e55..d2db1f6ab72 100644 --- a/extensions/positron-python/pythonFiles/positron/positron_jedilsp.py +++ b/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) diff --git a/extensions/positron-python/pythonFiles/tests/positron/test_positron_jedilsp.py b/extensions/positron-python/pythonFiles/tests/positron/test_positron_jedilsp.py index f6f8835ad4d..59914372163 100644 --- a/extensions/positron-python/pythonFiles/tests/positron/test_positron_jedilsp.py +++ b/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"