diff --git a/pylsp/plugins/rope_autoimport.py b/pylsp/plugins/rope_autoimport.py index ca3db1cf..ebcc7070 100644 --- a/pylsp/plugins/rope_autoimport.py +++ b/pylsp/plugins/rope_autoimport.py @@ -295,7 +295,7 @@ def pylsp_code_actions( word = get_name_or_module(document, diagnostic) log.debug(f"autoimport: searching for word: {word}") rope_config = config.settings(document_path=document.path).get("rope", {}) - autoimport = workspace._rope_autoimport(rope_config, feature="code_actions") + autoimport = workspace._rope_autoimport(rope_config) suggestions = list(autoimport.search_full(word)) log.debug("autoimport: suggestions: %s", suggestions) results = list( diff --git a/pylsp/workspace.py b/pylsp/workspace.py index 5c6880c9..5e8e548f 100644 --- a/pylsp/workspace.py +++ b/pylsp/workspace.py @@ -8,7 +8,7 @@ import re import uuid import functools -from typing import Literal, Optional, Generator, Callable, List +from typing import Optional, Generator, Callable, List from threading import RLock import jedi @@ -58,30 +58,20 @@ def __init__(self, root_uri, endpoint, config=None): # Whilst incubating, keep rope private self.__rope = None self.__rope_config = None - - # We have a sperate AutoImport object for each feature to avoid sqlite errors - # from accessing the same database from multiple threads. - # An upstream fix discussion is here: https://github.com/python-rope/rope/issues/713 - self.__rope_autoimport = ( - {} - ) # Type: Dict[Literal["completions", "code_actions"], rope.contrib.autoimport.sqlite.AutoImport] + self.__rope_autoimport = None def _rope_autoimport( self, rope_config: Optional, memory: bool = False, - feature: Literal["completions", "code_actions"] = "completions", ): # pylint: disable=import-outside-toplevel from rope.contrib.autoimport.sqlite import AutoImport - if feature not in ["completions", "code_actions"]: - raise ValueError(f"Unknown feature {feature}") - - if self.__rope_autoimport.get(feature, None) is None: + if self.__rope_autoimport is None: project = self._rope_project_builder(rope_config) - self.__rope_autoimport[feature] = AutoImport(project, memory=memory) - return self.__rope_autoimport[feature] + self.__rope_autoimport = AutoImport(project, memory=memory) + return self.__rope_autoimport def _rope_project_builder(self, rope_config): # pylint: disable=import-outside-toplevel @@ -388,8 +378,8 @@ def _create_cell_document( ) def close(self): - for _, autoimport in self.__rope_autoimport.items(): - autoimport.close() + if self.__rope_autoimport: + self.__rope_autoimport.close() class Document: diff --git a/pyproject.toml b/pyproject.toml index 17569525..be764e4b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -34,7 +34,7 @@ all = [ "pydocstyle>=6.3.0,<6.4.0", "pyflakes>=3.1.0,<3.2.0", "pylint>=2.5.0,<3.1", - "rope>1.2.0", + "rope>=1.11.0", "yapf>=0.33.0", "whatthepatch>=1.0.2,<2.0.0" ] @@ -45,7 +45,7 @@ pycodestyle = ["pycodestyle>=2.11.0,<2.12.0"] pydocstyle = ["pydocstyle>=6.3.0,<6.4.0"] pyflakes = ["pyflakes>=3.1.0,<3.2.0"] pylint = ["pylint>=2.5.0,<3.1"] -rope = ["rope>1.2.0"] +rope = ["rope>=1.11.0"] yapf = ["yapf>=0.33.0", "whatthepatch>=1.0.2,<2.0.0"] websockets = ["websockets>=10.3"] test = [ diff --git a/test/plugins/test_autoimport.py b/test/plugins/test_autoimport.py index ec5c0a33..9f02965b 100644 --- a/test/plugins/test_autoimport.py +++ b/test/plugins/test_autoimport.py @@ -1,13 +1,16 @@ # Copyright 2022- Python Language Server Contributors. from typing import Any, Dict, List -from unittest.mock import Mock +from unittest.mock import Mock, patch + +from test.test_notebook_document import wait_for_condition +from test.test_utils import send_initialize_request, send_notebook_did_open import jedi import parso import pytest -from pylsp import lsp, uris +from pylsp import IS_WIN, lsp, uris from pylsp.config.config import Config from pylsp.plugins.rope_autoimport import ( _get_score, @@ -25,13 +28,18 @@ DOC_URI = uris.from_fs_path(__file__) -def contains_autoimport(suggestion: Dict[str, Any], module: str) -> bool: - """Checks if `suggestion` contains an autoimport for `module`.""" +def contains_autoimport_completion(suggestion: Dict[str, Any], module: str) -> bool: + """Checks if `suggestion` contains an autoimport completion for `module`.""" return suggestion.get("label", "") == module and "import" in suggestion.get( "detail", "" ) +def contains_autoimport_quickfix(suggestion: Dict[str, Any], module: str) -> bool: + """Checks if `suggestion` contains an autoimport quick fix for `module`.""" + return suggestion.get("title", "") == f"import {module}" + + @pytest.fixture(scope="session") def autoimport_workspace(tmp_path_factory) -> Workspace: "Special autoimport workspace. Persists across sessions to make in-memory sqlite3 database fast." @@ -248,82 +256,89 @@ def test_autoimport_code_actions_get_correct_module_name(autoimport_workspace, m assert module_name == "os" -# rope autoimport launches a sqlite database which checks from which thread it is called. -# This makes the test below fail because we access the db from a different thread. -# See https://stackoverflow.com/questions/48218065/objects-created-in-a-thread-can-only-be-used-in-that-same-thread -# @pytest.mark.skipif(IS_WIN, reason="Flaky on Windows") -# def test_autoimport_completions_for_notebook_document( -# client_server_pair, -# ): -# client, server = client_server_pair -# send_initialize_request(client) - -# with patch.object(server._endpoint, "notify") as mock_notify: -# # Expectations: -# # 1. We receive an autoimport suggestion for "os" in the first cell because -# # os is imported after that. -# # 2. We don't receive an autoimport suggestion for "os" in the second cell because it's -# # already imported in the second cell. -# # 3. We don't receive an autoimport suggestion for "os" in the third cell because it's -# # already imported in the second cell. -# # 4. We receive an autoimport suggestion for "sys" because it's not already imported -# send_notebook_did_open(client, ["os", "import os\nos", "os", "sys"]) -# wait_for_condition(lambda: mock_notify.call_count >= 3) - -# server.m_workspace__did_change_configuration( -# settings={ -# "pylsp": { -# "plugins": { -# "rope_autoimport": { -# "memory": True, -# "completions": {"enabled": True}, -# }, -# } -# } -# } -# ) -# rope_autoimport_settings = server.workspace._config.plugin_settings( -# "rope_autoimport" -# ) -# assert rope_autoimport_settings.get("completions", {}).get("enabled", False) is True -# assert rope_autoimport_settings.get("memory", False) is True - -# # 1. -# suggestions = server.completions("cell_1_uri", {"line": 0, "character": 2}).get( -# "items" -# ) -# assert any( -# suggestion -# for suggestion in suggestions -# if contains_autoimport(suggestion, "os") -# ) - -# # 2. -# suggestions = server.completions("cell_2_uri", {"line": 1, "character": 2}).get( -# "items" -# ) -# assert not any( -# suggestion -# for suggestion in suggestions -# if contains_autoimport(suggestion, "os") -# ) - -# # 3. -# suggestions = server.completions("cell_3_uri", {"line": 0, "character": 2}).get( -# "items" -# ) -# assert not any( -# suggestion -# for suggestion in suggestions -# if contains_autoimport(suggestion, "os") -# ) - -# # 4. -# suggestions = server.completions("cell_4_uri", {"line": 0, "character": 3}).get( -# "items" -# ) -# assert any( -# suggestion -# for suggestion in suggestions -# if contains_autoimport(suggestion, "sys") -# ) +def make_context(module_name, line, character_start, character_end): + return { + "diagnostics": [ + { + "message": f"undefined name '{module_name}'", + "range": { + "start": {"line": line, "character": character_start}, + "end": {"line": line, "character": character_end}, + }, + } + ] + } + + +def position(line, character): + return {"line": line, "character": character} + + +@pytest.mark.skipif(IS_WIN, reason="Flaky on Windows") +def test_autoimport_code_actions_and_completions_for_notebook_document( + client_server_pair, +): + client, server = client_server_pair + send_initialize_request( + client, + { + "pylsp": { + "plugins": { + "rope_autoimport": { + "memory": True, + "enabled": True, + "completions": {"enabled": True}, + }, + } + } + }, + ) + + with patch.object(server._endpoint, "notify") as mock_notify: + # Expectations: + # 1. We receive an autoimport suggestion for "os" in the first cell because + # os is imported after that. + # 2. We don't receive an autoimport suggestion for "os" in the second cell because it's + # already imported in the second cell. + # 3. We don't receive an autoimport suggestion for "os" in the third cell because it's + # already imported in the second cell. + # 4. We receive an autoimport suggestion for "sys" because it's not already imported. + # 5. If diagnostics doesn't contain "undefined name ...", we send empty quick fix suggestions. + send_notebook_did_open(client, ["os", "import os\nos", "os", "sys"]) + wait_for_condition(lambda: mock_notify.call_count >= 3) + + rope_autoimport_settings = server.workspace._config.plugin_settings( + "rope_autoimport" + ) + assert rope_autoimport_settings.get("completions", {}).get("enabled", False) is True + assert rope_autoimport_settings.get("memory", False) is True + + # 1. + quick_fixes = server.code_actions("cell_1_uri", {}, make_context("os", 0, 0, 2)) + assert any(s for s in quick_fixes if contains_autoimport_quickfix(s, "os")) + + completions = server.completions("cell_1_uri", position(0, 2)).get("items") + assert any(s for s in completions if contains_autoimport_completion(s, "os")) + + # 2. + # We don't test code actions here as in this case, there would be no code actions sent bc + # there wouldn't be a diagnostics message. + completions = server.completions("cell_2_uri", position(1, 2)).get("items") + assert not any(s for s in completions if contains_autoimport_completion(s, "os")) + + # 3. + # Same as in 2. + completions = server.completions("cell_3_uri", position(0, 2)).get("items") + assert not any(s for s in completions if contains_autoimport_completion(s, "os")) + + # 4. + quick_fixes = server.code_actions("cell_4_uri", {}, make_context("sys", 0, 0, 3)) + assert any(s for s in quick_fixes if contains_autoimport_quickfix(s, "sys")) + + completions = server.completions("cell_4_uri", position(0, 3)).get("items") + assert any(s for s in completions if contains_autoimport_completion(s, "sys")) + + # 5. + context = {"diagnostics": [{"message": "A random message"}]} + quick_fixes = server.code_actions("cell_4_uri", {}, context) + assert len(quick_fixes) == 0