Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update rope to 1.11.0 for multi-threading capabilities #498

Merged
merged 8 commits into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 6 additions & 16 deletions pylsp/workspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
]
Expand All @@ -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 = [
Expand Down
160 changes: 82 additions & 78 deletions test/plugins/test_autoimport.py
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -251,79 +254,80 @@ def test_autoimport_code_actions_get_correct_module_name(autoimport_workspace, m
# 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
ccordoba12 marked this conversation as resolved.
Show resolved Hide resolved
# @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")
# )
@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,
{
"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
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.
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")
)
Loading