From 70076fa9308ee1058cb461d549630c296177b8a9 Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Mon, 5 Jun 2023 12:21:26 +0200 Subject: [PATCH 01/27] README.md --- README.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/README.md b/README.md index 27cb4686..131fdf45 100644 --- a/README.md +++ b/README.md @@ -161,6 +161,24 @@ pip install 'python-lsp-server[websockets]' ## Development +Dev install + +``` +# create conda env +cc python-lsp-server +ca python-lsp-server + +pip install ".[all]" +pip install ".[websockets]" +``` + +Run server with ws + +``` +pylsp --ws -v # Info level logging +pylsp --ws -v -v # Debug level logging +``` + To run the test suite: ```sh From ba0f9add8cd1af1b118094ff1bf058981dcee30d Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Wed, 7 Jun 2023 11:01:51 +0200 Subject: [PATCH 02/27] Add notebook support Support notebookDocument/didOpen and textDocument/publishDiagnostics on a notebook document --- pylsp/lsp.py | 5 ++ pylsp/lsp_types.py | 61 +++++++++++++++++++++++ pylsp/python_lsp.py | 94 +++++++++++++++++++++++++++++++++--- pylsp/workspace.py | 56 +++++++++++++++++++++ test/test_language_server.py | 68 ++++++++++++++++++++++++++ test/test_workspace.py | 10 ++++ 6 files changed, 288 insertions(+), 6 deletions(-) create mode 100644 pylsp/lsp_types.py diff --git a/pylsp/lsp.py b/pylsp/lsp.py index f97a0a2e..49d8389c 100644 --- a/pylsp/lsp.py +++ b/pylsp/lsp.py @@ -90,3 +90,8 @@ class TextDocumentSyncKind: NONE = 0 FULL = 1 INCREMENTAL = 2 + + +class NotebookCellKind: + Markup = 1 + Code = 2 \ No newline at end of file diff --git a/pylsp/lsp_types.py b/pylsp/lsp_types.py new file mode 100644 index 00000000..7637bd42 --- /dev/null +++ b/pylsp/lsp_types.py @@ -0,0 +1,61 @@ +from typing import TypedDict, Optional, Union, List, NewType +from .lsp import DiagnosticSeverity, DiagnosticTag + +""" +Types derived from the LSP protocol +See: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/ +""" + +URI = NewType('URI', str) + +NotebookCell = TypedDict('NotebookCell', { + 'kind': str, + 'document': URI, + 'metadata': Optional[dict], + 'executionSummary': Optional[dict], +}) + +NotebookDocument = TypedDict('NotebookDocument', { + 'uri': str, + 'notebookType': str, + 'version': int, + 'metadata': Optional[dict], + 'cells': List[NotebookCell], +}) + +CodeDescription = TypedDict('CodeDescription', { + 'href': URI, +}) + +Position = TypedDict('Position', { + 'line': int, + 'character': int, +}) + +Range = TypedDict('Range', { + 'start': Position, + 'end': Position, +}) + +Location = TypedDict('Location', { + 'uri': URI, + 'range': Range, +}) + +DiagnosticRelatedInformation = TypedDict('DiagnosticRelatedInformation', { + 'location': dict, + 'message': str, +}) + +Diagnostic = TypedDict('Diagnostic', { + 'range': dict, + 'severity': Optional[DiagnosticSeverity], + 'code': Optional[Union[int, str]], + 'codeDescription': Optional[CodeDescription], + 'source': Optional[str], + 'message': str, + 'tags': Optional[List[DiagnosticTag]], + 'relatedInformation': Optional[List[DiagnosticRelatedInformation]], + 'data': Optional[dict], +}) + diff --git a/pylsp/python_lsp.py b/pylsp/python_lsp.py index 43f886cc..89e2ed1f 100644 --- a/pylsp/python_lsp.py +++ b/pylsp/python_lsp.py @@ -7,15 +7,20 @@ import socketserver import threading import ujson as json +import uuid + from pylsp_jsonrpc.dispatchers import MethodDispatcher from pylsp_jsonrpc.endpoint import Endpoint from pylsp_jsonrpc.streams import JsonRpcStreamReader, JsonRpcStreamWriter +from typing import List, Dict, Any + from . import lsp, _utils, uris from .config import config -from .workspace import Workspace +from .workspace import Workspace, Document, Cell, Notebook from ._version import __version__ +from .lsp_types import Diagnostic log = logging.getLogger(__name__) @@ -266,6 +271,10 @@ def capabilities(self): }, 'openClose': True, }, + # TODO: add notebookDocumentSync when we support it + # 'notebookDocumentSync' : { + # 'notebook': '*', + # }, 'workspace': { 'workspaceFolders': { 'supported': True, @@ -375,11 +384,74 @@ def hover(self, doc_uri, position): def lint(self, doc_uri, is_saved): # Since we're debounced, the document may no longer be open workspace = self._match_uri_to_workspace(doc_uri) - if doc_uri in workspace.documents: - workspace.publish_diagnostics( - doc_uri, - flatten(self._hook('pylsp_lint', doc_uri, is_saved=is_saved)) - ) + document_object = workspace.documents.get(doc_uri, None) + if isinstance(document_object, Document): + self._lint_text_document(doc_uri, workspace, is_saved=is_saved) + elif isinstance(document_object, Notebook): + self._lint_notebook_document(document_object, workspace) + + def _lint_text_document(self, doc_uri, workspace, is_saved): + workspace.publish_diagnostics( + doc_uri, + flatten(self._hook('pylsp_lint', doc_uri, is_saved=is_saved)) + ) + + def _lint_notebook_document(self, notebook_document, workspace): + """ + Lint a notebook document. + + This is a bit more complicated than linting a text document, because we need to + send the entire notebook document to the pylsp_lint hook, but we need to send + the diagnostics back to the client on a per-cell basis. + """ + + # First, we create a temp TextDocument that represents the whole notebook + # contents. We'll use this to send to the pylsp_lint hook. + random_uri = str(uuid.uuid4()) + + # cell_list helps us map the diagnostics back to the correct cell later. + cell_list: List[Dict[str, Any]] = [] + + offset = 0 + total_source = "" + for cell in notebook_document.cells: + cell_uri = cell['document'] + cell_document = workspace.get_cell_document(cell_uri) + + lines = cell_document.lines + num_lines = len(lines) + start = offset + 1 + end = offset + num_lines + offset += num_lines + + data = { + 'uri': cell_uri, + 'line_start': start, + 'line_end': end, + 'source': cell_document.source + } + + cell_list.append(data) + total_source = total_source + "\n" + cell_document.source + + workspace.put_document(random_uri, total_source) + document_diagnostics: List[Diagnostic] = flatten(self._hook('pylsp_lint', random_uri, is_saved=True)) + + # Now we need to map the diagnostics back to the correct cell and + # publish them. + for cell in cell_list: + cell_diagnostics: List[Diagnostic] = [] + for diagnostic in document_diagnostics: + if diagnostic['range']['start']['line'] > cell['line_end']: + break + diagnostic['range']['start']['line'] = diagnostic['range']['start']['line'] - cell['line_start'] + 1 + diagnostic['range']['end']['line'] = diagnostic['range']['end']['line'] - cell['line_start'] + 1 + cell_diagnostics.append(diagnostic) + document_diagnostics.pop(0) + + workspace.publish_diagnostics(cell['uri'], cell_diagnostics) + + workspace.remove_document(random_uri) def references(self, doc_uri, position, exclude_declaration): return flatten(self._hook( @@ -404,6 +476,16 @@ def m_text_document__did_close(self, textDocument=None, **_kwargs): workspace.publish_diagnostics(textDocument['uri'], []) workspace.rm_document(textDocument['uri']) + # TODO define params + def m_notebook_document__did_open(self, notebookDocument=None, cellTextDocuments=[], **_kwargs): + workspace = self._match_uri_to_workspace(notebookDocument['uri']) + workspace.put_notebook_document(notebookDocument['uri'], notebookDocument['notebookType'], cells=notebookDocument['cells'], version=notebookDocument.get('version'), metadata=notebookDocument.get('metadata')) + log.debug(f">>> cellTextDocuments: {cellTextDocuments}") + for cell in cellTextDocuments: + workspace.put_cell_document(cell['uri'], cell['languageId'], cell['text'], version=cell.get('version')) + # self._hook('pylsp_document_did_open', textDocument['uri']) # This hook seems only relevant for rope + self.lint(notebookDocument['uri'], is_saved=True) + def m_text_document__did_open(self, textDocument=None, **_kwargs): workspace = self._match_uri_to_workspace(textDocument['uri']) workspace.put_document(textDocument['uri'], textDocument['text'], version=textDocument.get('version')) diff --git a/pylsp/workspace.py b/pylsp/workspace.py index ea7f55e8..43a729fe 100644 --- a/pylsp/workspace.py +++ b/pylsp/workspace.py @@ -105,12 +105,21 @@ def get_document(self, doc_uri): """ return self._docs.get(doc_uri) or self._create_document(doc_uri) + def get_cell_document(self, doc_uri): + return self._docs.get(doc_uri) + def get_maybe_document(self, doc_uri): return self._docs.get(doc_uri) def put_document(self, doc_uri, source, version=None): self._docs[doc_uri] = self._create_document(doc_uri, source=source, version=version) + def put_notebook_document(self, doc_uri, notebook_type, cells, version=None, metadata=None): + self._docs[doc_uri] = self._create_notebook_document(doc_uri, notebook_type, cells, version, metadata) + + def put_cell_document(self, doc_uri, language_id, source, version=None): + self._docs[doc_uri] = self._create_cell_document(doc_uri, language_id, source, version) + def rm_document(self, doc_uri): self._docs.pop(doc_uri) @@ -257,6 +266,29 @@ def _create_document(self, doc_uri, source=None, version=None): extra_sys_path=self.source_roots(path), rope_project_builder=self._rope_project_builder, ) + + def _create_notebook_document(self, doc_uri, notebook_type, cells, version=None, metadata=None): + return Notebook( + doc_uri, + notebook_type, + self, + cells=cells, + version=version, + metadata=metadata + ) + + def _create_cell_document(self, doc_uri, language_id, source=None, version=None): + # TODO: remove what is unnecessary here. + path = uris.to_fs_path(doc_uri) + return Cell( + doc_uri, + language_id=language_id, + workspace=self, + source=source, + version=version, + extra_sys_path=self.source_roots(path), + rope_project_builder=self._rope_project_builder, + ) def close(self): if self.__rope_autoimport is not None: @@ -441,3 +473,27 @@ def sys_path(self, environment_path=None, env_vars=None): environment = self.get_enviroment(environment_path=environment_path, env_vars=env_vars) path.extend(environment.get_sys_path()) return path + + +class Notebook: + """Represents a notebook.""" + def __init__(self, uri, notebook_type, workspace, cells=None, version=None, metadata=None): + self.uri = uri + self.notebook_type = notebook_type + self.workspace = workspace + self.version = version + self.cells = cells or [] + self.metadata = metadata or {} + + def __str__(self): + return "Notebook with URI '%s'" % str(self.uri) + +# We inherit from Document for now to get the same API. However, cell document differ from the text documents in that +# as they have a language id. +class Cell(Document): + """Represents a cell in a notebook.""" + + def __init__(self, uri, language_id, workspace, source=None, version=None, local=True, extra_sys_path=None, + rope_project_builder=None): + super().__init__(uri, workspace, source, version, local, extra_sys_path, rope_project_builder) + self.language_id = language_id \ No newline at end of file diff --git a/test/test_language_server.py b/test/test_language_server.py index 92d1ea84..90f1c546 100644 --- a/test/test_language_server.py +++ b/test/test_language_server.py @@ -12,6 +12,7 @@ import pytest from pylsp.python_lsp import start_io_lang_server, PythonLSPServer +from pylsp.lsp import NotebookCellKind CALL_TIMEOUT = 10 RUNNING_IN_CI = bool(os.environ.get('CI')) @@ -118,3 +119,70 @@ def test_not_exit_without_check_parent_process_flag(client_server): # pylint: d def test_missing_message(client_server): # pylint: disable=redefined-outer-name with pytest.raises(JsonRpcMethodNotFound): client_server._endpoint.request('unknown_method').result(timeout=CALL_TIMEOUT) + + +# TODO: make this assert on content of diagnostics message +# Run this test if you want to see the diagnostics messages of an LSP server +def test_text_document__did_open(client_server): + client_server._endpoint.request('initialize', { + 'processId': 1234, + 'rootPath': os.path.dirname(__file__), + 'initializationOptions': {} + }).result(timeout=CALL_TIMEOUT) + client_server._endpoint.notify('textDocument/didOpen', { + 'textDocument': { + 'uri': os.path.join(os.path.dirname(__file__)), + 'version': 1, + 'text': 'import sys\nx=2\ny=x+2' + } + }) + +# TODO: flesh the unit test out so that it tests the following: +# The workspace is updated with the new notebook document and two cell documents +def test_notebook_document__did_open(client_server): + client_server._endpoint.request('initialize', { + 'processId': 1234, + 'rootPath': os.path.dirname(__file__), + 'initializationOptions': {} + }).result(timeout=CALL_TIMEOUT) + client_server._endpoint.notify('notebookDocument/didOpen', { + 'notebookDocument': { + 'uri': 'notebook_uri', + 'notebookType': 'jupyter-notebook', + 'cells': [ + { + 'kind': NotebookCellKind.Code, + 'document': "cell_1_uri", + }, + # TODO: add markdown cell support later + # { + # 'kind': NotebookCellKind.Markup, + # 'document': "cell_2_uri", + # }, + { + 'kind': NotebookCellKind.Code, + 'document': "cell_3_uri", + } + ] + }, + 'cellTextDocuments': [ + { + 'uri': 'cell_1_uri', + 'languageId': 'python', + 'text': 'import sys', + }, + # { + # 'uri': 'cell_2_uri', + # 'languageId': 'markdown', + # 'text': '# Title\n\n Some text', + # }, + { + 'uri': 'cell_3_uri', + 'languageId': 'python', + 'text': 'x = 2\ny = x + 2\nprint(z)', + } + ] + }) +# time.sleep(0.1) +# # Assert that the documents are created in the workspace +# assert len(client_server.workspaces) == 1 diff --git a/test/test_workspace.py b/test/test_workspace.py index 94a9cba1..eeebe065 100644 --- a/test/test_workspace.py +++ b/test/test_workspace.py @@ -23,6 +23,16 @@ def test_put_document(pylsp): assert DOC_URI in pylsp.workspace._docs +def test_put_notebook_document(pylsp): + pylsp.workspace.put_notebook_document(DOC_URI, 'jupyter-notebook', []) + assert DOC_URI in pylsp.workspace._docs + + +def test_put_cell_document(pylsp): + pylsp.workspace.put_cell_document(DOC_URI, 'python', 'content') + assert DOC_URI in pylsp.workspace._docs + + def test_get_document(pylsp): pylsp.workspace.put_document(DOC_URI, 'TEXT') assert pylsp.workspace.get_document(DOC_URI).source == 'TEXT' From d5bf4f99eca6f43b725ae927b2861cbeec097283 Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Wed, 7 Jun 2023 11:19:01 +0200 Subject: [PATCH 03/27] Add example messages for reference --- .../example_1.json | 36 ++++++++++ .../example_2.json | 68 +++++++++++++++++++ 2 files changed, 104 insertions(+) create mode 100644 test/data/publish_diagnostics_message_examples/example_1.json create mode 100644 test/data/publish_diagnostics_message_examples/example_2.json diff --git a/test/data/publish_diagnostics_message_examples/example_1.json b/test/data/publish_diagnostics_message_examples/example_1.json new file mode 100644 index 00000000..4fa425b9 --- /dev/null +++ b/test/data/publish_diagnostics_message_examples/example_1.json @@ -0,0 +1,36 @@ +{ + "diagnostics": [ + { + "message": "invalid syntax", + "range": { + "end": { + "character": 15, + "line": 1 + }, + "start": { + "character": 7, + "line": 1 + } + }, + "severity": 1, + "source": "pyflakes" + }, + { + "code": "W292", + "message": "W292 no newline at end of file", + "range": { + "end": { + "character": 7, + "line": 1 + }, + "start": { + "character": 7, + "line": 1 + } + }, + "severity": 2, + "source": "pycodestyle" + } + ], + "uri": "/Users/.../code/python-lsp-server/test" +} \ No newline at end of file diff --git a/test/data/publish_diagnostics_message_examples/example_2.json b/test/data/publish_diagnostics_message_examples/example_2.json new file mode 100644 index 00000000..006f95a6 --- /dev/null +++ b/test/data/publish_diagnostics_message_examples/example_2.json @@ -0,0 +1,68 @@ +{ + "diagnostics": [ + { + "message": "'sys' imported but unused", + "range": { + "end": { + "character": 11, + "line": 0 + }, + "start": { + "character": 0, + "line": 0 + } + }, + "severity": 2, + "source": "pyflakes" + }, + { + "code": "E225", + "message": "E225 missing whitespace around operator", + "range": { + "end": { + "character": 4, + "line": 1 + }, + "start": { + "character": 1, + "line": 1 + } + }, + "severity": 2, + "source": "pycodestyle" + }, + { + "code": "W292", + "message": "W292 no newline at end of file", + "range": { + "end": { + "character": 5, + "line": 2 + }, + "start": { + "character": 5, + "line": 2 + } + }, + "severity": 2, + "source": "pycodestyle" + }, + { + "code": "E225", + "message": "E225 missing whitespace around operator", + "range": { + "end": { + "character": 5, + "line": 2 + }, + "start": { + "character": 1, + "line": 2 + } + }, + "severity": 2, + "source": "pycodestyle" + } + ], + uri: "/Users/.../code/python-lsp-server/test" +} \ No newline at end of file From 78233c7e0fbd908673e79a697f70df77e61eda14 Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Wed, 7 Jun 2023 11:37:45 +0200 Subject: [PATCH 04/27] lsp_types: add DocumentUri and use correctly --- pylsp/lsp_types.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pylsp/lsp_types.py b/pylsp/lsp_types.py index 7637bd42..9fc2de9f 100644 --- a/pylsp/lsp_types.py +++ b/pylsp/lsp_types.py @@ -7,10 +7,11 @@ """ URI = NewType('URI', str) +DocumentUri = NewType('DocumentUri', str) NotebookCell = TypedDict('NotebookCell', { 'kind': str, - 'document': URI, + 'document': DocumentUri, 'metadata': Optional[dict], 'executionSummary': Optional[dict], }) From 09d68793483a15ce67e880149d1dec4da0f2cac3 Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Sun, 11 Jun 2023 19:09:54 +0200 Subject: [PATCH 05/27] add diagnostics support --- pylsp/lsp_types.py | 62 ------------------------ pylsp/python_lsp.py | 93 +++++++++++++++++++++++++++++++----- pylsp/workspace.py | 32 ++++++++++--- test/test_language_server.py | 68 +++++++++++++++++++++++--- 4 files changed, 167 insertions(+), 88 deletions(-) delete mode 100644 pylsp/lsp_types.py diff --git a/pylsp/lsp_types.py b/pylsp/lsp_types.py deleted file mode 100644 index 9fc2de9f..00000000 --- a/pylsp/lsp_types.py +++ /dev/null @@ -1,62 +0,0 @@ -from typing import TypedDict, Optional, Union, List, NewType -from .lsp import DiagnosticSeverity, DiagnosticTag - -""" -Types derived from the LSP protocol -See: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/ -""" - -URI = NewType('URI', str) -DocumentUri = NewType('DocumentUri', str) - -NotebookCell = TypedDict('NotebookCell', { - 'kind': str, - 'document': DocumentUri, - 'metadata': Optional[dict], - 'executionSummary': Optional[dict], -}) - -NotebookDocument = TypedDict('NotebookDocument', { - 'uri': str, - 'notebookType': str, - 'version': int, - 'metadata': Optional[dict], - 'cells': List[NotebookCell], -}) - -CodeDescription = TypedDict('CodeDescription', { - 'href': URI, -}) - -Position = TypedDict('Position', { - 'line': int, - 'character': int, -}) - -Range = TypedDict('Range', { - 'start': Position, - 'end': Position, -}) - -Location = TypedDict('Location', { - 'uri': URI, - 'range': Range, -}) - -DiagnosticRelatedInformation = TypedDict('DiagnosticRelatedInformation', { - 'location': dict, - 'message': str, -}) - -Diagnostic = TypedDict('Diagnostic', { - 'range': dict, - 'severity': Optional[DiagnosticSeverity], - 'code': Optional[Union[int, str]], - 'codeDescription': Optional[CodeDescription], - 'source': Optional[str], - 'message': str, - 'tags': Optional[List[DiagnosticTag]], - 'relatedInformation': Optional[List[DiagnosticRelatedInformation]], - 'data': Optional[dict], -}) - diff --git a/pylsp/python_lsp.py b/pylsp/python_lsp.py index 89e2ed1f..556fb22a 100644 --- a/pylsp/python_lsp.py +++ b/pylsp/python_lsp.py @@ -14,13 +14,13 @@ from pylsp_jsonrpc.endpoint import Endpoint from pylsp_jsonrpc.streams import JsonRpcStreamReader, JsonRpcStreamWriter + from typing import List, Dict, Any from . import lsp, _utils, uris from .config import config from .workspace import Workspace, Document, Cell, Notebook from ._version import __version__ -from .lsp_types import Diagnostic log = logging.getLogger(__name__) @@ -385,7 +385,9 @@ def lint(self, doc_uri, is_saved): # Since we're debounced, the document may no longer be open workspace = self._match_uri_to_workspace(doc_uri) document_object = workspace.documents.get(doc_uri, None) - if isinstance(document_object, Document): + if isinstance(document_object, Cell): + self._lint_notebook_document(document_object.parent, workspace) + elif isinstance(document_object, Document): self._lint_text_document(doc_uri, workspace, is_saved=is_saved) elif isinstance(document_object, Notebook): self._lint_notebook_document(document_object, workspace) @@ -435,12 +437,12 @@ def _lint_notebook_document(self, notebook_document, workspace): total_source = total_source + "\n" + cell_document.source workspace.put_document(random_uri, total_source) - document_diagnostics: List[Diagnostic] = flatten(self._hook('pylsp_lint', random_uri, is_saved=True)) + document_diagnostics = flatten(self._hook('pylsp_lint', random_uri, is_saved=True)) # Now we need to map the diagnostics back to the correct cell and # publish them. for cell in cell_list: - cell_diagnostics: List[Diagnostic] = [] + cell_diagnostics = [] for diagnostic in document_diagnostics: if diagnostic['range']['start']['line'] > cell['line_end']: break @@ -471,21 +473,86 @@ def folding(self, doc_uri): def m_completion_item__resolve(self, **completionItem): return self.completion_item_resolve(completionItem) - def m_text_document__did_close(self, textDocument=None, **_kwargs): - workspace = self._match_uri_to_workspace(textDocument['uri']) - workspace.publish_diagnostics(textDocument['uri'], []) - workspace.rm_document(textDocument['uri']) - - # TODO define params + # TODO: add m_notebook_document__did_close def m_notebook_document__did_open(self, notebookDocument=None, cellTextDocuments=[], **_kwargs): workspace = self._match_uri_to_workspace(notebookDocument['uri']) - workspace.put_notebook_document(notebookDocument['uri'], notebookDocument['notebookType'], cells=notebookDocument['cells'], version=notebookDocument.get('version'), metadata=notebookDocument.get('metadata')) - log.debug(f">>> cellTextDocuments: {cellTextDocuments}") + workspace.put_notebook_document(notebookDocument['uri'], notebookDocument['notebookType'], + cells=notebookDocument['cells'], version=notebookDocument.get('version'), + metadata=notebookDocument.get('metadata')) for cell in cellTextDocuments: - workspace.put_cell_document(cell['uri'], cell['languageId'], cell['text'], version=cell.get('version')) + workspace.put_cell_document(cell['uri'], cell['languageId'], notebookDocument['uri'], cell['text'], + version=cell.get('version')) # self._hook('pylsp_document_did_open', textDocument['uri']) # This hook seems only relevant for rope self.lint(notebookDocument['uri'], is_saved=True) + def m_notebook_document__did_change(self, notebookDocument=None, contentChanges=None, **_kwargs): + """ + Changes to the notebook document. + + This could be one of the following: + 1. Notebook metadata changed + 2. Cell(s) added + 3. Cell(s) deleted + 4. Cell(s) data changed + 4.1 Cell metadata changed + 4.2 Cell source changed + """ + workspace = self._match_uri_to_workspace(notebookDocument['uri']) + + notebook_metadata = contentChanges.get('metadata') + if notebook_metadata: + # Case 1 + workspace.update_notebook_metadata(notebookDocument['uri'], notebook_metadata) + + cells = contentChanges.get('cells') + if cells: + # Change to cells + structure = cells.get('structure') + if structure: + # Case 2 or 3 + notebook_cell_array_change = structure['array'] + start = notebook_cell_array_change['start'] + cell_delete_count = notebook_cell_array_change['deleteCount'] + if cell_delete_count == 0: + # Case 2 + # Cell documents + opened_cells = structure['didOpen'] + for cell_document in opened_cells: + workspace.put_cell_document(cell_document['uri'], cell_document['languageId'], notebookDocument['uri'], + cell_document['text'], cell_document.get('version')) + # Cell metadata which is added to Notebook + opened_cells = notebook_cell_array_change['cells'] + workspace.add_notebook_cells(notebookDocument['uri'], opened_cells, start) + else: + # Case 3 + # Cell documents + closed_cells = structure['didClose'] + for cell_document in closed_cells: + workspace.rm_cell_document(cell_document['uri']) + # Cell metadata which is removed from Notebook + workspace.remove_notebook_cells(notebookDocument['uri'], start, cell_delete_count) + + data = cells.get('data') + if data: + # Case 4.1 + for cell in data: + # update NotebookDocument.cells properties + pass + + text_content = cells.get('textContent') + if text_content: + # Case 4.2 + for cell in text_content: + cell_uri = cell['document']['uri'] + # Even though the protocol says that changes is an array, we assume that it's always a single + # element array that contains the last change to the cell source. + workspace.update_document(cell_uri, cell['changes'][0]) + + def m_text_document__did_close(self, textDocument=None, **_kwargs): + workspace = self._match_uri_to_workspace(textDocument['uri']) + workspace.publish_diagnostics(textDocument['uri'], []) + workspace.rm_document(textDocument['uri']) + def m_text_document__did_open(self, textDocument=None, **_kwargs): workspace = self._match_uri_to_workspace(textDocument['uri']) workspace.put_document(textDocument['uri'], textDocument['text'], version=textDocument.get('version')) diff --git a/pylsp/workspace.py b/pylsp/workspace.py index 43a729fe..1e441639 100644 --- a/pylsp/workspace.py +++ b/pylsp/workspace.py @@ -8,7 +8,7 @@ import re import uuid import functools -from typing import Optional, Generator, Callable +from typing import Optional, Generator, Callable, List from threading import RLock import jedi @@ -117,12 +117,22 @@ def put_document(self, doc_uri, source, version=None): def put_notebook_document(self, doc_uri, notebook_type, cells, version=None, metadata=None): self._docs[doc_uri] = self._create_notebook_document(doc_uri, notebook_type, cells, version, metadata) - def put_cell_document(self, doc_uri, language_id, source, version=None): - self._docs[doc_uri] = self._create_cell_document(doc_uri, language_id, source, version) + def add_notebook_cells(self, doc_uri, cells, start): + self._docs[doc_uri].add_cells(cells, start) + + def remove_notebook_cells(self, doc_uri, start, delete_count): + self._docs[doc_uri].remove_cells(start, delete_count) + + def update_notebook_metadata(self, doc_uri, metadata): + self._docs[doc_uri].metadata = metadata + + def put_cell_document(self, doc_uri, language_id, parent, source, version=None): + self._docs[doc_uri] = self._create_cell_document(doc_uri, language_id, parent, source, version) def rm_document(self, doc_uri): self._docs.pop(doc_uri) + def update_document(self, doc_uri, change, version=None): self._docs[doc_uri].apply_change(change) self._docs[doc_uri].version = version @@ -277,13 +287,14 @@ def _create_notebook_document(self, doc_uri, notebook_type, cells, version=None, metadata=metadata ) - def _create_cell_document(self, doc_uri, language_id, source=None, version=None): + def _create_cell_document(self, doc_uri, language_id, parent, source=None, version=None): # TODO: remove what is unnecessary here. path = uris.to_fs_path(doc_uri) return Cell( doc_uri, language_id=language_id, workspace=self, + parent=parent, source=source, version=version, extra_sys_path=self.source_roots(path), @@ -487,13 +498,20 @@ def __init__(self, uri, notebook_type, workspace, cells=None, version=None, meta def __str__(self): return "Notebook with URI '%s'" % str(self.uri) + + def add_cells(self, new_cells: List, start: int) -> None: + self.cells[start:start] = new_cells + + def remove_cells(self, start: int, delete_count: int) -> None: + del self.cells[start:start+delete_count] # We inherit from Document for now to get the same API. However, cell document differ from the text documents in that -# as they have a language id. +# they have a language id. class Cell(Document): """Represents a cell in a notebook.""" - def __init__(self, uri, language_id, workspace, source=None, version=None, local=True, extra_sys_path=None, + def __init__(self, uri, language_id, workspace, parent, source=None, version=None, local=True, extra_sys_path=None, rope_project_builder=None): super().__init__(uri, workspace, source, version, local, extra_sys_path, rope_project_builder) - self.language_id = language_id \ No newline at end of file + self.language_id = language_id + self.parent = parent \ No newline at end of file diff --git a/test/test_language_server.py b/test/test_language_server.py index 90f1c546..9b5e0b25 100644 --- a/test/test_language_server.py +++ b/test/test_language_server.py @@ -121,7 +121,6 @@ def test_missing_message(client_server): # pylint: disable=redefined-outer-name client_server._endpoint.request('unknown_method').result(timeout=CALL_TIMEOUT) -# TODO: make this assert on content of diagnostics message # Run this test if you want to see the diagnostics messages of an LSP server def test_text_document__did_open(client_server): client_server._endpoint.request('initialize', { @@ -136,9 +135,10 @@ def test_text_document__did_open(client_server): 'text': 'import sys\nx=2\ny=x+2' } }) + # TODO: assert on the content of diagnostics message -# TODO: flesh the unit test out so that it tests the following: -# The workspace is updated with the new notebook document and two cell documents + +# Run this test if you want to see the diagnostics messages of an LSP server def test_notebook_document__did_open(client_server): client_server._endpoint.request('initialize', { 'processId': 1234, @@ -183,6 +183,62 @@ def test_notebook_document__did_open(client_server): } ] }) -# time.sleep(0.1) -# # Assert that the documents are created in the workspace -# assert len(client_server.workspaces) == 1 + # TODO: assert on the content of diagnostics message + + +# Run this test if you want to see the diagnostics messages of an LSP server +def test_notebook_document__did_change(client_server): + client_server._endpoint.request('initialize', { + 'processId': 1234, + 'rootPath': os.path.dirname(__file__), + 'initializationOptions': {} + }).result(timeout=CALL_TIMEOUT) + client_server._endpoint.notify('notebookDocument/didOpen', { + 'notebookDocument': { + 'uri': 'notebook_uri', + 'notebookType': 'jupyter-notebook', + 'cells': [ + { + 'kind': NotebookCellKind.Code, + 'document': "cell_1_uri", + }, + { + 'kind': NotebookCellKind.Code, + 'document': "cell_2_uri", + } + ] + }, + 'cellTextDocuments': [ + { + 'uri': 'cell_1_uri', + 'languageId': 'python', + 'text': 'import sys', + }, + { + 'uri': 'cell_2_uri', + 'languageId': 'python', + 'text': '', + } + ] + }) + # TODO: assert diagnostics complains about sys being imported but not used + client_server._endpoint.notify('notebookDocument/didChange', { + 'notebookDocument': { + 'uri': 'notebook_uri', + }, + 'change': { + 'textContent': [ + { + 'document': { + 'uri': 'cell_2_uri', + }, + 'changes': [ + { + 'text': 'sys.path' + } + ] + } + ] + } + }) + # TODO: assert that diagnostics is gone From 500e1202696e09737aba8dd0ce9ab04ee5554141 Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Mon, 12 Jun 2023 16:48:15 +0200 Subject: [PATCH 06/27] support editing, adding, and removing cells --- pylsp/python_lsp.py | 32 +++---- pylsp/workspace.py | 1 - test/test_language_server.py | 175 +++++++++++++++++++++++++++++++++-- 3 files changed, 183 insertions(+), 25 deletions(-) diff --git a/pylsp/python_lsp.py b/pylsp/python_lsp.py index 556fb22a..e5dbd265 100644 --- a/pylsp/python_lsp.py +++ b/pylsp/python_lsp.py @@ -385,9 +385,7 @@ def lint(self, doc_uri, is_saved): # Since we're debounced, the document may no longer be open workspace = self._match_uri_to_workspace(doc_uri) document_object = workspace.documents.get(doc_uri, None) - if isinstance(document_object, Cell): - self._lint_notebook_document(document_object.parent, workspace) - elif isinstance(document_object, Document): + if isinstance(document_object, Document): self._lint_text_document(doc_uri, workspace, is_saved=is_saved) elif isinstance(document_object, Notebook): self._lint_notebook_document(document_object, workspace) @@ -413,7 +411,7 @@ def _lint_notebook_document(self, notebook_document, workspace): # cell_list helps us map the diagnostics back to the correct cell later. cell_list: List[Dict[str, Any]] = [] - + offset = 0 total_source = "" for cell in notebook_document.cells: @@ -435,10 +433,10 @@ def _lint_notebook_document(self, notebook_document, workspace): cell_list.append(data) total_source = total_source + "\n" + cell_document.source - - workspace.put_document(random_uri, total_source) + + workspace.put_document(random_uri, total_source) document_diagnostics = flatten(self._hook('pylsp_lint', random_uri, is_saved=True)) - + # Now we need to map the diagnostics back to the correct cell and # publish them. for cell in cell_list: @@ -452,7 +450,7 @@ def _lint_notebook_document(self, notebook_document, workspace): document_diagnostics.pop(0) workspace.publish_diagnostics(cell['uri'], cell_diagnostics) - + workspace.remove_document(random_uri) def references(self, doc_uri, position, exclude_declaration): @@ -485,7 +483,7 @@ def m_notebook_document__did_open(self, notebookDocument=None, cellTextDocuments # self._hook('pylsp_document_did_open', textDocument['uri']) # This hook seems only relevant for rope self.lint(notebookDocument['uri'], is_saved=True) - def m_notebook_document__did_change(self, notebookDocument=None, contentChanges=None, **_kwargs): + def m_notebook_document__did_change(self, notebookDocument=None, change=None, **_kwargs): """ Changes to the notebook document. @@ -499,12 +497,12 @@ def m_notebook_document__did_change(self, notebookDocument=None, contentChanges= """ workspace = self._match_uri_to_workspace(notebookDocument['uri']) - notebook_metadata = contentChanges.get('metadata') + notebook_metadata = change.get('metadata') if notebook_metadata: # Case 1 workspace.update_notebook_metadata(notebookDocument['uri'], notebook_metadata) - cells = contentChanges.get('cells') + cells = change.get('cells') if cells: # Change to cells structure = cells.get('structure') @@ -518,8 +516,9 @@ def m_notebook_document__did_change(self, notebookDocument=None, contentChanges= # Cell documents opened_cells = structure['didOpen'] for cell_document in opened_cells: - workspace.put_cell_document(cell_document['uri'], cell_document['languageId'], notebookDocument['uri'], - cell_document['text'], cell_document.get('version')) + workspace.put_cell_document(cell_document['uri'], cell_document['languageId'], + notebookDocument['uri'], cell_document['text'], + cell_document.get('version')) # Cell metadata which is added to Notebook opened_cells = notebook_cell_array_change['cells'] workspace.add_notebook_cells(notebookDocument['uri'], opened_cells, start) @@ -528,10 +527,10 @@ def m_notebook_document__did_change(self, notebookDocument=None, contentChanges= # Cell documents closed_cells = structure['didClose'] for cell_document in closed_cells: - workspace.rm_cell_document(cell_document['uri']) + workspace.rm_document(cell_document['uri']) # Cell metadata which is removed from Notebook workspace.remove_notebook_cells(notebookDocument['uri'], start, cell_delete_count) - + data = cells.get('data') if data: # Case 4.1 @@ -544,9 +543,10 @@ def m_notebook_document__did_change(self, notebookDocument=None, contentChanges= # Case 4.2 for cell in text_content: cell_uri = cell['document']['uri'] - # Even though the protocol says that changes is an array, we assume that it's always a single + # Even though the protocol says that `changes` is an array, we assume that it's always a single # element array that contains the last change to the cell source. workspace.update_document(cell_uri, cell['changes'][0]) + self.lint(notebookDocument['uri'], is_saved=True) def m_text_document__did_close(self, textDocument=None, **_kwargs): workspace = self._match_uri_to_workspace(textDocument['uri']) diff --git a/pylsp/workspace.py b/pylsp/workspace.py index 1e441639..343ec700 100644 --- a/pylsp/workspace.py +++ b/pylsp/workspace.py @@ -132,7 +132,6 @@ def put_cell_document(self, doc_uri, language_id, parent, source, version=None): def rm_document(self, doc_uri): self._docs.pop(doc_uri) - def update_document(self, doc_uri, change, version=None): self._docs[doc_uri].apply_change(change) self._docs[doc_uri].version = version diff --git a/test/test_language_server.py b/test/test_language_server.py index 9b5e0b25..a6e26e70 100644 --- a/test/test_language_server.py +++ b/test/test_language_server.py @@ -187,7 +187,7 @@ def test_notebook_document__did_open(client_server): # Run this test if you want to see the diagnostics messages of an LSP server -def test_notebook_document__did_change(client_server): +def test_notebook_document__did_change__edit_content(client_server): client_server._endpoint.request('initialize', { 'processId': 1234, 'rootPath': os.path.dirname(__file__), @@ -221,24 +221,183 @@ def test_notebook_document__did_change(client_server): } ] }) + time.sleep(1) # TODO: assert diagnostics complains about sys being imported but not used client_server._endpoint.notify('notebookDocument/didChange', { 'notebookDocument': { 'uri': 'notebook_uri', }, 'change': { - 'textContent': [ + 'cells': { + 'textContent': [ + { + 'document': { + 'uri': 'cell_2_uri', + }, + 'changes': [ + { + 'text': 'sys.path' + } + ] + } + ] + } + } + }) + time.sleep(1) + # TODO: assert that diagnostics is gone + # TODO: assert that the virtualized document has been removed to avoid memory leak + + +def test_notebook_document__did_change__add_new_cell(client_server): + client_server._endpoint.request('initialize', { + 'processId': 1234, + 'rootPath': os.path.dirname(__file__), + 'initializationOptions': {} + }).result(timeout=CALL_TIMEOUT) + client_server._endpoint.notify('notebookDocument/didOpen', { + 'notebookDocument': { + 'uri': 'notebook_uri', + 'notebookType': 'jupyter-notebook', + 'cells': [ { - 'document': { - 'uri': 'cell_2_uri', + 'kind': NotebookCellKind.Code, + 'document': "cell_1_uri", + }, + ] + }, + 'cellTextDocuments': [ + { + 'uri': 'cell_1_uri', + 'languageId': 'python', + 'text': 'import sys', + }, + ] + }) + time.sleep(1) + # TODO: assert diagnostics complains about sys being imported but not used + client_server._endpoint.notify('notebookDocument/didChange', { + 'notebookDocument': { + 'uri': 'notebook_uri', + }, + 'change': { + 'cells': { + 'structure': { + 'array': { + 'start': 1, + 'deleteCount': 0, + 'cells': [ + { + 'kind': NotebookCellKind.Code, + 'document': "cell_2_uri", + } + ] + }, + 'didOpen': [ + { + 'uri': 'cell_2_uri', + 'languageId': 'python', + 'text': 'sys.path', + } + ] + }, + } + } + }) + time.sleep(1) + # TODO: assert that diagnostics is gone + # TODO: assert that the virtualized document has been removed to avoid memory leak + # Inserting a cell between cell 1 and 2 + client_server._endpoint.notify('notebookDocument/didChange', { + 'notebookDocument': { + 'uri': 'notebook_uri', + }, + 'change': { + 'cells': { + 'structure': { + 'array': { + 'start': 1, + 'deleteCount': 0, + 'cells': [ + { + 'kind': NotebookCellKind.Code, + 'document': "cell_3_uri", + } + ] }, - 'changes': [ + 'didOpen': [ { - 'text': 'sys.path' + 'uri': 'cell_3_uri', + 'languageId': 'python', + 'text': 'x', } - ] + ] + }, + } + } + }) + time.sleep(1) + # TODO: assert that the cells are in the right order + + +def test_notebook_document__did_change__delete_cell(client_server): + client_server._endpoint.request('initialize', { + 'processId': 1234, + 'rootPath': os.path.dirname(__file__), + 'initializationOptions': {} + }).result(timeout=CALL_TIMEOUT) + client_server._endpoint.notify('notebookDocument/didOpen', { + 'notebookDocument': { + 'uri': 'notebook_uri', + 'notebookType': 'jupyter-notebook', + 'cells': [ + { + 'kind': NotebookCellKind.Code, + 'document': "cell_1_uri", + }, + { + 'kind': NotebookCellKind.Code, + 'document': "cell_2_uri", } ] + }, + 'cellTextDocuments': [ + { + 'uri': 'cell_1_uri', + 'languageId': 'python', + 'text': 'import sys', + }, + { + 'uri': 'cell_2_uri', + 'languageId': 'python', + 'text': 'sys.path', + } + ] + }) + time.sleep(1) + # TODO: assert diagnostics should not complain + # Delete cell 2 + client_server._endpoint.notify('notebookDocument/didChange', { + 'notebookDocument': { + 'uri': 'notebook_uri', + }, + 'change': { + 'cells': { + 'structure': { + 'array': { + 'start': 1, + 'deleteCount': 1, + }, + 'didClose': [ + { + 'uri': 'cell_2_uri', + } + ] + }, + } } }) - # TODO: assert that diagnostics is gone + time.sleep(1) + # TODO: assert that diagnostics comaplains about sys being imported but not used + # TODO: assert that the CellDocument has been removed from workspace.documents and notebook.cells + assert 0 From b407a31e4cf7a2cd2005aa79c41113b609c403ba Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Tue, 13 Jun 2023 09:51:36 +0200 Subject: [PATCH 07/27] add unit tests for notebook document messages --- pylsp/python_lsp.py | 2 +- test/test_language_server.py | 283 ----------------------------------- 2 files changed, 1 insertion(+), 284 deletions(-) diff --git a/pylsp/python_lsp.py b/pylsp/python_lsp.py index e5dbd265..e785b791 100644 --- a/pylsp/python_lsp.py +++ b/pylsp/python_lsp.py @@ -451,7 +451,7 @@ def _lint_notebook_document(self, notebook_document, workspace): workspace.publish_diagnostics(cell['uri'], cell_diagnostics) - workspace.remove_document(random_uri) + workspace.rm_document(random_uri) def references(self, doc_uri, position, exclude_declaration): return flatten(self._hook( diff --git a/test/test_language_server.py b/test/test_language_server.py index a6e26e70..92d1ea84 100644 --- a/test/test_language_server.py +++ b/test/test_language_server.py @@ -12,7 +12,6 @@ import pytest from pylsp.python_lsp import start_io_lang_server, PythonLSPServer -from pylsp.lsp import NotebookCellKind CALL_TIMEOUT = 10 RUNNING_IN_CI = bool(os.environ.get('CI')) @@ -119,285 +118,3 @@ def test_not_exit_without_check_parent_process_flag(client_server): # pylint: d def test_missing_message(client_server): # pylint: disable=redefined-outer-name with pytest.raises(JsonRpcMethodNotFound): client_server._endpoint.request('unknown_method').result(timeout=CALL_TIMEOUT) - - -# Run this test if you want to see the diagnostics messages of an LSP server -def test_text_document__did_open(client_server): - client_server._endpoint.request('initialize', { - 'processId': 1234, - 'rootPath': os.path.dirname(__file__), - 'initializationOptions': {} - }).result(timeout=CALL_TIMEOUT) - client_server._endpoint.notify('textDocument/didOpen', { - 'textDocument': { - 'uri': os.path.join(os.path.dirname(__file__)), - 'version': 1, - 'text': 'import sys\nx=2\ny=x+2' - } - }) - # TODO: assert on the content of diagnostics message - - -# Run this test if you want to see the diagnostics messages of an LSP server -def test_notebook_document__did_open(client_server): - client_server._endpoint.request('initialize', { - 'processId': 1234, - 'rootPath': os.path.dirname(__file__), - 'initializationOptions': {} - }).result(timeout=CALL_TIMEOUT) - client_server._endpoint.notify('notebookDocument/didOpen', { - 'notebookDocument': { - 'uri': 'notebook_uri', - 'notebookType': 'jupyter-notebook', - 'cells': [ - { - 'kind': NotebookCellKind.Code, - 'document': "cell_1_uri", - }, - # TODO: add markdown cell support later - # { - # 'kind': NotebookCellKind.Markup, - # 'document': "cell_2_uri", - # }, - { - 'kind': NotebookCellKind.Code, - 'document': "cell_3_uri", - } - ] - }, - 'cellTextDocuments': [ - { - 'uri': 'cell_1_uri', - 'languageId': 'python', - 'text': 'import sys', - }, - # { - # 'uri': 'cell_2_uri', - # 'languageId': 'markdown', - # 'text': '# Title\n\n Some text', - # }, - { - 'uri': 'cell_3_uri', - 'languageId': 'python', - 'text': 'x = 2\ny = x + 2\nprint(z)', - } - ] - }) - # TODO: assert on the content of diagnostics message - - -# Run this test if you want to see the diagnostics messages of an LSP server -def test_notebook_document__did_change__edit_content(client_server): - client_server._endpoint.request('initialize', { - 'processId': 1234, - 'rootPath': os.path.dirname(__file__), - 'initializationOptions': {} - }).result(timeout=CALL_TIMEOUT) - client_server._endpoint.notify('notebookDocument/didOpen', { - 'notebookDocument': { - 'uri': 'notebook_uri', - 'notebookType': 'jupyter-notebook', - 'cells': [ - { - 'kind': NotebookCellKind.Code, - 'document': "cell_1_uri", - }, - { - 'kind': NotebookCellKind.Code, - 'document': "cell_2_uri", - } - ] - }, - 'cellTextDocuments': [ - { - 'uri': 'cell_1_uri', - 'languageId': 'python', - 'text': 'import sys', - }, - { - 'uri': 'cell_2_uri', - 'languageId': 'python', - 'text': '', - } - ] - }) - time.sleep(1) - # TODO: assert diagnostics complains about sys being imported but not used - client_server._endpoint.notify('notebookDocument/didChange', { - 'notebookDocument': { - 'uri': 'notebook_uri', - }, - 'change': { - 'cells': { - 'textContent': [ - { - 'document': { - 'uri': 'cell_2_uri', - }, - 'changes': [ - { - 'text': 'sys.path' - } - ] - } - ] - } - } - }) - time.sleep(1) - # TODO: assert that diagnostics is gone - # TODO: assert that the virtualized document has been removed to avoid memory leak - - -def test_notebook_document__did_change__add_new_cell(client_server): - client_server._endpoint.request('initialize', { - 'processId': 1234, - 'rootPath': os.path.dirname(__file__), - 'initializationOptions': {} - }).result(timeout=CALL_TIMEOUT) - client_server._endpoint.notify('notebookDocument/didOpen', { - 'notebookDocument': { - 'uri': 'notebook_uri', - 'notebookType': 'jupyter-notebook', - 'cells': [ - { - 'kind': NotebookCellKind.Code, - 'document': "cell_1_uri", - }, - ] - }, - 'cellTextDocuments': [ - { - 'uri': 'cell_1_uri', - 'languageId': 'python', - 'text': 'import sys', - }, - ] - }) - time.sleep(1) - # TODO: assert diagnostics complains about sys being imported but not used - client_server._endpoint.notify('notebookDocument/didChange', { - 'notebookDocument': { - 'uri': 'notebook_uri', - }, - 'change': { - 'cells': { - 'structure': { - 'array': { - 'start': 1, - 'deleteCount': 0, - 'cells': [ - { - 'kind': NotebookCellKind.Code, - 'document': "cell_2_uri", - } - ] - }, - 'didOpen': [ - { - 'uri': 'cell_2_uri', - 'languageId': 'python', - 'text': 'sys.path', - } - ] - }, - } - } - }) - time.sleep(1) - # TODO: assert that diagnostics is gone - # TODO: assert that the virtualized document has been removed to avoid memory leak - # Inserting a cell between cell 1 and 2 - client_server._endpoint.notify('notebookDocument/didChange', { - 'notebookDocument': { - 'uri': 'notebook_uri', - }, - 'change': { - 'cells': { - 'structure': { - 'array': { - 'start': 1, - 'deleteCount': 0, - 'cells': [ - { - 'kind': NotebookCellKind.Code, - 'document': "cell_3_uri", - } - ] - }, - 'didOpen': [ - { - 'uri': 'cell_3_uri', - 'languageId': 'python', - 'text': 'x', - } - ] - }, - } - } - }) - time.sleep(1) - # TODO: assert that the cells are in the right order - - -def test_notebook_document__did_change__delete_cell(client_server): - client_server._endpoint.request('initialize', { - 'processId': 1234, - 'rootPath': os.path.dirname(__file__), - 'initializationOptions': {} - }).result(timeout=CALL_TIMEOUT) - client_server._endpoint.notify('notebookDocument/didOpen', { - 'notebookDocument': { - 'uri': 'notebook_uri', - 'notebookType': 'jupyter-notebook', - 'cells': [ - { - 'kind': NotebookCellKind.Code, - 'document': "cell_1_uri", - }, - { - 'kind': NotebookCellKind.Code, - 'document': "cell_2_uri", - } - ] - }, - 'cellTextDocuments': [ - { - 'uri': 'cell_1_uri', - 'languageId': 'python', - 'text': 'import sys', - }, - { - 'uri': 'cell_2_uri', - 'languageId': 'python', - 'text': 'sys.path', - } - ] - }) - time.sleep(1) - # TODO: assert diagnostics should not complain - # Delete cell 2 - client_server._endpoint.notify('notebookDocument/didChange', { - 'notebookDocument': { - 'uri': 'notebook_uri', - }, - 'change': { - 'cells': { - 'structure': { - 'array': { - 'start': 1, - 'deleteCount': 1, - }, - 'didClose': [ - { - 'uri': 'cell_2_uri', - } - ] - }, - } - } - }) - time.sleep(1) - # TODO: assert that diagnostics comaplains about sys being imported but not used - # TODO: assert that the CellDocument has been removed from workspace.documents and notebook.cells - assert 0 From 53e4ad6b8fc374bce228d2a4f50c6da627384fa7 Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Tue, 13 Jun 2023 09:55:37 +0200 Subject: [PATCH 08/27] add test_notebook_document --- test/test_notebook_document.py | 406 +++++++++++++++++++++++++++++++++ 1 file changed, 406 insertions(+) create mode 100644 test/test_notebook_document.py diff --git a/test/test_notebook_document.py b/test/test_notebook_document.py new file mode 100644 index 00000000..51449155 --- /dev/null +++ b/test/test_notebook_document.py @@ -0,0 +1,406 @@ +# Copyright 2021- Python Language Server Contributors. + +import os +import time +from threading import Thread +from unittest.mock import patch, call + +import pytest + +from pylsp.python_lsp import PythonLSPServer +from pylsp.lsp import NotebookCellKind + +CALL_TIMEOUT = 10 + + +def wait_for_condition(condition, timeout=CALL_TIMEOUT): + """Wait for a condition to be true, or timeout.""" + start_time = time.time() + while not condition(): + time.sleep(0.1) + if time.time() - start_time > timeout: + raise TimeoutError("Timeout waiting for condition") + + +def start(obj): + obj.start() + + +class ClientServerPair: + """A class to setup a client/server pair""" + + def __init__(self): + # Client to Server pipe + csr, csw = os.pipe() + # Server to client pipe + scr, scw = os.pipe() + + self.server = PythonLSPServer(os.fdopen(csr, "rb"), os.fdopen(scw, "wb")) + self.server_thread = Thread(target=start, args=[self.server]) + self.server_thread.start() + + self.client = PythonLSPServer(os.fdopen(scr, "rb"), os.fdopen(csw, "wb")) + self.client_thread = Thread(target=start, args=[self.client]) + self.client_thread.start() + + +@pytest.fixture +def client_server_pair(): + """A fixture that sets up a client/server pair and shuts down the server""" + client_server_pair = ClientServerPair() + + yield (client_server_pair.client, client_server_pair.server) + + shutdown_response = client_server_pair.client._endpoint.request("shutdown").result( + timeout=CALL_TIMEOUT + ) + assert shutdown_response is None + client_server_pair.client._endpoint.notify("exit") + + +def test_initialize(client_server_pair): + client, server = client_server_pair + response = client._endpoint.request( + "initialize", + { + "processId": 1234, + "rootPath": os.path.dirname(__file__), + "initializationOptions": {}, + }, + ).result(timeout=CALL_TIMEOUT) + assert server.workspace is not None + assert "capabilities" in response + # TODO: assert that notebook capabilities are in response + + +def test_notebook_document__did_open(client_server_pair): + client, server = client_server_pair + client._endpoint.request( + "initialize", + { + "processId": 1234, + "rootPath": os.path.dirname(__file__), + "initializationOptions": {}, + }, + ).result(timeout=CALL_TIMEOUT) + + with patch.object(server._endpoint, "notify") as mock_notify: + client._endpoint.notify( + "notebookDocument/didOpen", + { + "notebookDocument": { + "uri": "notebook_uri", + "notebookType": "jupyter-notebook", + "cells": [ + { + "kind": NotebookCellKind.Code, + "document": "cell_1_uri", + }, + { + "kind": NotebookCellKind.Code, + "document": "cell_2_uri", + }, + ], + }, + "cellTextDocuments": [ + { + "uri": "cell_1_uri", + "languageId": "python", + "text": "import sys", + }, + { + "uri": "cell_2_uri", + "languageId": "python", + "text": "x = 1", + }, + ], + }, + ) + wait_for_condition(lambda: mock_notify.call_count >= 2) + expected_call_args = [ + call( + "textDocument/publishDiagnostics", + params={ + "uri": "cell_1_uri", + "diagnostics": [ + { + "source": "pyflakes", + "range": { + "start": {"line": 1, "character": 0}, + "end": {"line": 1, "character": 11}, + }, + "message": "'sys' imported but unused", + "severity": 2, + } + ], + }, + ), + call( + "textDocument/publishDiagnostics", + params={ + "uri": "cell_2_uri", + "diagnostics": [ + { + "source": "pycodestyle", + "range": { + "start": {"line": 1, "character": 5}, + "end": {"line": 1, "character": 5}, + }, + "message": "W292 no newline at end of file", + "code": "W292", + "severity": 2, + } + ], + }, + ), + ] + mock_notify.assert_has_calls(expected_call_args) + + +def test_notebook_document__did_change(client_server_pair): + client, server = client_server_pair + client._endpoint.request( + "initialize", + { + "processId": 1234, + "rootPath": os.path.dirname(__file__), + "initializationOptions": {}, + }, + ).result(timeout=CALL_TIMEOUT) + + # Open notebook + with patch.object(server._endpoint, "notify") as mock_notify: + client._endpoint.notify( + "notebookDocument/didOpen", + { + "notebookDocument": { + "uri": "notebook_uri", + "notebookType": "jupyter-notebook", + "cells": [ + { + "kind": NotebookCellKind.Code, + "document": "cell_1_uri", + }, + { + "kind": NotebookCellKind.Code, + "document": "cell_2_uri", + }, + ], + }, + "cellTextDocuments": [ + { + "uri": "cell_1_uri", + "languageId": "python", + "text": "import sys", + }, + { + "uri": "cell_2_uri", + "languageId": "python", + "text": "", + }, + ], + }, + ) + wait_for_condition(lambda: mock_notify.call_count >= 2) + assert len(server.workspace.documents) == 3 + for uri in ["cell_1_uri", "cell_2_uri", "notebook_uri"]: + assert uri in server.workspace.documents + assert len(server.workspace.get_document("notebook_uri").cells) == 2 + expected_call_args = [ + call( + "textDocument/publishDiagnostics", + params={ + "uri": "cell_1_uri", + "diagnostics": [ + { + "source": "pyflakes", + "range": { + "start": {"line": 1, "character": 0}, + "end": {"line": 1, "character": 11}, + }, + "message": "'sys' imported but unused", + "severity": 2, + } + ], + }, + ), + call( + "textDocument/publishDiagnostics", + params={"uri": "cell_2_uri", "diagnostics": []}, + ), + ] + mock_notify.assert_has_calls(expected_call_args) + + # Remove second cell + with patch.object(server._endpoint, "notify") as mock_notify: + client._endpoint.notify( + "notebookDocument/didChange", + { + "notebookDocument": { + "uri": "notebook_uri", + }, + "change": { + "cells": { + "structure": { + "array": { + "start": 1, + "deleteCount": 1, + }, + "didClose": [ + { + "uri": "cell_2_uri", + } + ], + }, + } + }, + }, + ) + wait_for_condition(lambda: mock_notify.call_count >= 1) + assert len(server.workspace.documents) == 2 + assert not "cell_2_uri" in server.workspace.documents + assert len(server.workspace.get_document("notebook_uri").cells) == 1 + expected_call_args = [ + call( + "textDocument/publishDiagnostics", + params={ + "uri": "cell_1_uri", + "diagnostics": [ + { + "source": "pyflakes", + "range": { + "start": {"line": 1, "character": 0}, + "end": {"line": 1, "character": 10}, + }, + "message": "'sys' imported but unused", + "severity": 2, + } + ], + }, + ) + ] + mock_notify.assert_has_calls(expected_call_args) + + # Add second cell + with patch.object(server._endpoint, "notify") as mock_notify: + client._endpoint.notify( + "notebookDocument/didChange", + { + "notebookDocument": { + "uri": "notebook_uri", + }, + "change": { + "cells": { + "structure": { + "array": { + "start": 1, + "deleteCount": 0, + "cells": [ + { + "kind": NotebookCellKind.Code, + "document": "cell_3_uri", + } + ], + }, + "didOpen": [ + { + "uri": "cell_3_uri", + "languageId": "python", + "text": "x", + } + ], + }, + } + }, + }, + ) + wait_for_condition(lambda: mock_notify.call_count >= 2) + assert len(server.workspace.documents) == 3 + assert "cell_3_uri" in server.workspace.documents + assert len(server.workspace.get_document("notebook_uri").cells) == 2 + expected_call_args = [ + call( + "textDocument/publishDiagnostics", + params={ + "uri": "cell_1_uri", + "diagnostics": [ + { + "source": "pyflakes", + "range": { + "start": {"line": 1, "character": 0}, + "end": {"line": 1, "character": 11}, + }, + "message": "'sys' imported but unused", + "severity": 2, + } + ], + }, + ), + call( + "textDocument/publishDiagnostics", + params={ + "uri": "cell_3_uri", + "diagnostics": [ + { + "source": "pyflakes", + "range": { + "start": {"line": 1, "character": 0}, + "end": {"line": 1, "character": 1}, + }, + "message": "undefined name 'x'", + "severity": 1, + } + ], + }, + ), + ] + mock_notify.assert_has_calls(expected_call_args) + + # Edit second cell + with patch.object(server._endpoint, "notify") as mock_notify: + client._endpoint.notify( + "notebookDocument/didChange", + { + "notebookDocument": { + "uri": "notebook_uri", + }, + "change": { + "cells": { + "textContent": [ + { + "document": { + "uri": "cell_3_uri", + }, + "changes": [{"text": "sys.path"}], + } + ] + } + }, + }, + ) + wait_for_condition(lambda: mock_notify.call_count >= 2) + expected_call_args = [ + call( + "textDocument/publishDiagnostics", + params={"uri": "cell_1_uri", "diagnostics": []}, + ), + call( + "textDocument/publishDiagnostics", + params={ + "uri": "cell_3_uri", + "diagnostics": [ + { + "source": "pycodestyle", + "range": { + "start": {"line": 1, "character": 8}, + "end": {"line": 1, "character": 8}, + }, + "message": "W292 no newline at end of file", + "code": "W292", + "severity": 2, + } + ], + }, + ), + ] + mock_notify.assert_has_calls(expected_call_args) From c891cf0ef00fdda322fa3f5c60fde5011d360cb9 Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Tue, 13 Jun 2023 10:01:04 +0200 Subject: [PATCH 09/27] fix unit tests and pylint --- pylsp/python_lsp.py | 13 +++++-------- pylsp/workspace.py | 16 +++++++--------- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/pylsp/python_lsp.py b/pylsp/python_lsp.py index e785b791..c1ecdf92 100644 --- a/pylsp/python_lsp.py +++ b/pylsp/python_lsp.py @@ -6,17 +6,14 @@ import os import socketserver import threading -import ujson as json import uuid - +from typing import List, Dict, Any +import ujson as json from pylsp_jsonrpc.dispatchers import MethodDispatcher from pylsp_jsonrpc.endpoint import Endpoint from pylsp_jsonrpc.streams import JsonRpcStreamReader, JsonRpcStreamWriter - -from typing import List, Dict, Any - from . import lsp, _utils, uris from .config import config from .workspace import Workspace, Document, Cell, Notebook @@ -474,11 +471,11 @@ def m_completion_item__resolve(self, **completionItem): # TODO: add m_notebook_document__did_close def m_notebook_document__did_open(self, notebookDocument=None, cellTextDocuments=[], **_kwargs): workspace = self._match_uri_to_workspace(notebookDocument['uri']) - workspace.put_notebook_document(notebookDocument['uri'], notebookDocument['notebookType'], - cells=notebookDocument['cells'], version=notebookDocument.get('version'), + workspace.put_notebook_document(notebookDocument['uri'], notebookDocument['notebookType'], + cells=notebookDocument['cells'], version=notebookDocument.get('version'), metadata=notebookDocument.get('metadata')) for cell in cellTextDocuments: - workspace.put_cell_document(cell['uri'], cell['languageId'], notebookDocument['uri'], cell['text'], + workspace.put_cell_document(cell['uri'], cell['languageId'], notebookDocument['uri'], cell['text'], version=cell.get('version')) # self._hook('pylsp_document_did_open', textDocument['uri']) # This hook seems only relevant for rope self.lint(notebookDocument['uri'], is_saved=True) diff --git a/pylsp/workspace.py b/pylsp/workspace.py index 343ec700..8a17b77d 100644 --- a/pylsp/workspace.py +++ b/pylsp/workspace.py @@ -125,9 +125,9 @@ def remove_notebook_cells(self, doc_uri, start, delete_count): def update_notebook_metadata(self, doc_uri, metadata): self._docs[doc_uri].metadata = metadata - - def put_cell_document(self, doc_uri, language_id, parent, source, version=None): - self._docs[doc_uri] = self._create_cell_document(doc_uri, language_id, parent, source, version) + + def put_cell_document(self, doc_uri, language_id, source, version=None): + self._docs[doc_uri] = self._create_cell_document(doc_uri, language_id, source, version) def rm_document(self, doc_uri): self._docs.pop(doc_uri) @@ -275,7 +275,7 @@ def _create_document(self, doc_uri, source=None, version=None): extra_sys_path=self.source_roots(path), rope_project_builder=self._rope_project_builder, ) - + def _create_notebook_document(self, doc_uri, notebook_type, cells, version=None, metadata=None): return Notebook( doc_uri, @@ -286,14 +286,13 @@ def _create_notebook_document(self, doc_uri, notebook_type, cells, version=None, metadata=metadata ) - def _create_cell_document(self, doc_uri, language_id, parent, source=None, version=None): + def _create_cell_document(self, doc_uri, language_id, source=None, version=None): # TODO: remove what is unnecessary here. path = uris.to_fs_path(doc_uri) return Cell( doc_uri, language_id=language_id, workspace=self, - parent=parent, source=source, version=version, extra_sys_path=self.source_roots(path), @@ -497,7 +496,7 @@ def __init__(self, uri, notebook_type, workspace, cells=None, version=None, meta def __str__(self): return "Notebook with URI '%s'" % str(self.uri) - + def add_cells(self, new_cells: List, start: int) -> None: self.cells[start:start] = new_cells @@ -509,8 +508,7 @@ def remove_cells(self, start: int, delete_count: int) -> None: class Cell(Document): """Represents a cell in a notebook.""" - def __init__(self, uri, language_id, workspace, parent, source=None, version=None, local=True, extra_sys_path=None, + def __init__(self, uri, language_id, workspace, source=None, version=None, local=True, extra_sys_path=None, rope_project_builder=None): super().__init__(uri, workspace, source, version, local, extra_sys_path, rope_project_builder) self.language_id = language_id - self.parent = parent \ No newline at end of file From 3dcaf9f6feacf278257fa879b8692507c9ac10e0 Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Wed, 14 Jun 2023 17:20:58 +0200 Subject: [PATCH 10/27] fix pytest test_notebook_document.py --- pylsp/python_lsp.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/pylsp/python_lsp.py b/pylsp/python_lsp.py index c1ecdf92..723fbe88 100644 --- a/pylsp/python_lsp.py +++ b/pylsp/python_lsp.py @@ -16,7 +16,7 @@ from . import lsp, _utils, uris from .config import config -from .workspace import Workspace, Document, Cell, Notebook +from .workspace import Workspace, Document, Notebook from ._version import __version__ log = logging.getLogger(__name__) @@ -469,15 +469,13 @@ def m_completion_item__resolve(self, **completionItem): return self.completion_item_resolve(completionItem) # TODO: add m_notebook_document__did_close - def m_notebook_document__did_open(self, notebookDocument=None, cellTextDocuments=[], **_kwargs): + def m_notebook_document__did_open(self, notebookDocument=None, cellTextDocuments=None, **_kwargs): workspace = self._match_uri_to_workspace(notebookDocument['uri']) workspace.put_notebook_document(notebookDocument['uri'], notebookDocument['notebookType'], cells=notebookDocument['cells'], version=notebookDocument.get('version'), metadata=notebookDocument.get('metadata')) - for cell in cellTextDocuments: - workspace.put_cell_document(cell['uri'], cell['languageId'], notebookDocument['uri'], cell['text'], - version=cell.get('version')) - # self._hook('pylsp_document_did_open', textDocument['uri']) # This hook seems only relevant for rope + for cell in (cellTextDocuments or []): + workspace.put_cell_document(cell['uri'], cell['languageId'], cell['text'], version=cell.get('version')) self.lint(notebookDocument['uri'], is_saved=True) def m_notebook_document__did_change(self, notebookDocument=None, change=None, **_kwargs): @@ -514,8 +512,7 @@ def m_notebook_document__did_change(self, notebookDocument=None, change=None, ** opened_cells = structure['didOpen'] for cell_document in opened_cells: workspace.put_cell_document(cell_document['uri'], cell_document['languageId'], - notebookDocument['uri'], cell_document['text'], - cell_document.get('version')) + cell_document['text'], cell_document.get('version')) # Cell metadata which is added to Notebook opened_cells = notebook_cell_array_change['cells'] workspace.add_notebook_cells(notebookDocument['uri'], opened_cells, start) From ab3bcee66017033e55b1b5f2852305f29d81f2af Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Wed, 14 Jun 2023 17:39:23 +0200 Subject: [PATCH 11/27] Fix pylint issues: too-many-public-methods, dangerous-default-value, unused-import, too-many-locals, redefined-outer-name --- pylsp/lsp.py | 2 +- pylsp/python_lsp.py | 26 ++++++++++---------------- pylsp/workspace.py | 2 ++ test/test_notebook_document.py | 14 +++++++------- 4 files changed, 20 insertions(+), 24 deletions(-) diff --git a/pylsp/lsp.py b/pylsp/lsp.py index 49d8389c..d5f4737b 100644 --- a/pylsp/lsp.py +++ b/pylsp/lsp.py @@ -94,4 +94,4 @@ class TextDocumentSyncKind: class NotebookCellKind: Markup = 1 - Code = 2 \ No newline at end of file + Code = 2 diff --git a/pylsp/python_lsp.py b/pylsp/python_lsp.py index 723fbe88..9d50f033 100644 --- a/pylsp/python_lsp.py +++ b/pylsp/python_lsp.py @@ -415,22 +415,20 @@ def _lint_notebook_document(self, notebook_document, workspace): cell_uri = cell['document'] cell_document = workspace.get_cell_document(cell_uri) - lines = cell_document.lines - num_lines = len(lines) - start = offset + 1 - end = offset + num_lines - offset += num_lines + num_lines = len(cell_document.lines) data = { 'uri': cell_uri, - 'line_start': start, - 'line_end': end, + 'line_start': offset + 1, + 'line_end': offset + num_lines, 'source': cell_document.source } cell_list.append(data) total_source = total_source + "\n" + cell_document.source + offset += num_lines + workspace.put_document(random_uri, total_source) document_diagnostics = flatten(self._hook('pylsp_lint', random_uri, is_saved=True)) @@ -492,10 +490,9 @@ def m_notebook_document__did_change(self, notebookDocument=None, change=None, ** """ workspace = self._match_uri_to_workspace(notebookDocument['uri']) - notebook_metadata = change.get('metadata') - if notebook_metadata: + if change.get('metadata'): # Case 1 - workspace.update_notebook_metadata(notebookDocument['uri'], notebook_metadata) + workspace.update_notebook_metadata(notebookDocument['uri'], change.get('metadata')) cells = change.get('cells') if cells: @@ -509,18 +506,15 @@ def m_notebook_document__did_change(self, notebookDocument=None, change=None, ** if cell_delete_count == 0: # Case 2 # Cell documents - opened_cells = structure['didOpen'] - for cell_document in opened_cells: + for cell_document in structure['didOpen']: workspace.put_cell_document(cell_document['uri'], cell_document['languageId'], cell_document['text'], cell_document.get('version')) # Cell metadata which is added to Notebook - opened_cells = notebook_cell_array_change['cells'] - workspace.add_notebook_cells(notebookDocument['uri'], opened_cells, start) + workspace.add_notebook_cells(notebookDocument['uri'], notebook_cell_array_change['cells'], start) else: # Case 3 # Cell documents - closed_cells = structure['didClose'] - for cell_document in closed_cells: + for cell_document in structure['didClose']: workspace.rm_document(cell_document['uri']) # Cell metadata which is removed from Notebook workspace.remove_notebook_cells(notebookDocument['uri'], start, cell_delete_count) diff --git a/pylsp/workspace.py b/pylsp/workspace.py index 8a17b77d..61c7881a 100644 --- a/pylsp/workspace.py +++ b/pylsp/workspace.py @@ -33,8 +33,10 @@ def wrapper(self, *args, **kwargs): return wrapper +# pylint: disable=too-many-public-methods class Workspace: + M_PUBLISH_DIAGNOSTICS = 'textDocument/publishDiagnostics' M_PROGRESS = '$/progress' M_INITIALIZE_PROGRESS = 'window/workDoneProgress/create' diff --git a/test/test_notebook_document.py b/test/test_notebook_document.py index 51449155..88db7b48 100644 --- a/test/test_notebook_document.py +++ b/test/test_notebook_document.py @@ -47,18 +47,18 @@ def __init__(self): @pytest.fixture def client_server_pair(): """A fixture that sets up a client/server pair and shuts down the server""" - client_server_pair = ClientServerPair() + client_server_pair_obj = ClientServerPair() - yield (client_server_pair.client, client_server_pair.server) + yield (client_server_pair_obj.client, client_server_pair_obj.server) - shutdown_response = client_server_pair.client._endpoint.request("shutdown").result( + shutdown_response = client_server_pair_obj.client._endpoint.request("shutdown").result( timeout=CALL_TIMEOUT ) assert shutdown_response is None - client_server_pair.client._endpoint.notify("exit") + client_server_pair_obj.client._endpoint.notify("exit") -def test_initialize(client_server_pair): +def test_initialize(client_server_pair): # pylint: disable=redefined-outer-name client, server = client_server_pair response = client._endpoint.request( "initialize", @@ -73,7 +73,7 @@ def test_initialize(client_server_pair): # TODO: assert that notebook capabilities are in response -def test_notebook_document__did_open(client_server_pair): +def test_notebook_document__did_open(client_server_pair): # pylint: disable=redefined-outer-name client, server = client_server_pair client._endpoint.request( "initialize", @@ -157,7 +157,7 @@ def test_notebook_document__did_open(client_server_pair): mock_notify.assert_has_calls(expected_call_args) -def test_notebook_document__did_change(client_server_pair): +def test_notebook_document__did_change(client_server_pair): # pylint: disable=redefined-outer-name client, server = client_server_pair client._endpoint.request( "initialize", From 2dd71654aedac3ed9500fff6e8ac88c9d9e6c6cb Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Wed, 14 Jun 2023 21:00:20 +0200 Subject: [PATCH 12/27] support notebookDocument__did_close --- pylsp/python_lsp.py | 8 +++- pylsp/workspace.py | 2 +- test/test_notebook_document.py | 71 ++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 2 deletions(-) diff --git a/pylsp/python_lsp.py b/pylsp/python_lsp.py index 9d50f033..d1370cc3 100644 --- a/pylsp/python_lsp.py +++ b/pylsp/python_lsp.py @@ -466,7 +466,6 @@ def folding(self, doc_uri): def m_completion_item__resolve(self, **completionItem): return self.completion_item_resolve(completionItem) - # TODO: add m_notebook_document__did_close def m_notebook_document__did_open(self, notebookDocument=None, cellTextDocuments=None, **_kwargs): workspace = self._match_uri_to_workspace(notebookDocument['uri']) workspace.put_notebook_document(notebookDocument['uri'], notebookDocument['notebookType'], @@ -476,6 +475,13 @@ def m_notebook_document__did_open(self, notebookDocument=None, cellTextDocuments workspace.put_cell_document(cell['uri'], cell['languageId'], cell['text'], version=cell.get('version')) self.lint(notebookDocument['uri'], is_saved=True) + def m_notebook_document__did_close(self, notebookDocument=None, cellTextDocuments=None, **_kwargs): + workspace = self._match_uri_to_workspace(notebookDocument['uri']) + for cell in (cellTextDocuments or []): + workspace.publish_diagnostics(cell['uri'], []) + workspace.rm_document(cell['uri']) + workspace.rm_document(notebookDocument['uri']) + def m_notebook_document__did_change(self, notebookDocument=None, change=None, **_kwargs): """ Changes to the notebook document. diff --git a/pylsp/workspace.py b/pylsp/workspace.py index 61c7881a..574cdc5c 100644 --- a/pylsp/workspace.py +++ b/pylsp/workspace.py @@ -33,9 +33,9 @@ def wrapper(self, *args, **kwargs): return wrapper -# pylint: disable=too-many-public-methods class Workspace: + # pylint: disable=too-many-public-methods M_PUBLISH_DIAGNOSTICS = 'textDocument/publishDiagnostics' M_PROGRESS = '$/progress' diff --git a/test/test_notebook_document.py b/test/test_notebook_document.py index 88db7b48..8b6adfa6 100644 --- a/test/test_notebook_document.py +++ b/test/test_notebook_document.py @@ -404,3 +404,74 @@ def test_notebook_document__did_change(client_server_pair): # pylint: disable=re ), ] mock_notify.assert_has_calls(expected_call_args) + + +def test_notebook__did_close(client_server_pair): # pylint: disable=redefined-outer-name + client, server = client_server_pair + client._endpoint.request( + "initialize", + { + "processId": 1234, + "rootPath": os.path.dirname(__file__), + "initializationOptions": {}, + }, + ).result(timeout=CALL_TIMEOUT) + + # Open notebook + with patch.object(server._endpoint, "notify") as mock_notify: + client._endpoint.notify( + "notebookDocument/didOpen", + { + "notebookDocument": { + "uri": "notebook_uri", + "notebookType": "jupyter-notebook", + "cells": [ + { + "kind": NotebookCellKind.Code, + "document": "cell_1_uri", + }, + { + "kind": NotebookCellKind.Code, + "document": "cell_2_uri", + }, + ], + }, + "cellTextDocuments": [ + { + "uri": "cell_1_uri", + "languageId": "python", + "text": "import sys", + }, + { + "uri": "cell_2_uri", + "languageId": "python", + "text": "", + }, + ], + }, + ) + wait_for_condition(lambda: mock_notify.call_count >= 2) + assert len(server.workspace.documents) == 3 + for uri in ["cell_1_uri", "cell_2_uri", "notebook_uri"]: + assert uri in server.workspace.documents + + # Close notebook + with patch.object(server._endpoint, "notify") as mock_notify: + client._endpoint.notify( + "notebookDocument/didClose", + { + "notebookDocument": { + "uri": "notebook_uri", + }, + "cellTextDocuments": [ + { + "uri": "cell_1_uri", + }, + { + "uri": "cell_2_uri", + }, + ], + }, + ) + wait_for_condition(lambda: mock_notify.call_count >= 2) + assert len(server.workspace.documents) == 0 From 77222396250a71f8f7a9e72b0fb5814ba9384662 Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Thu, 15 Jun 2023 11:00:54 +0200 Subject: [PATCH 13/27] Add notebookDocumentSync to capabilities --- pylsp/python_lsp.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pylsp/python_lsp.py b/pylsp/python_lsp.py index d1370cc3..8979c48d 100644 --- a/pylsp/python_lsp.py +++ b/pylsp/python_lsp.py @@ -268,10 +268,11 @@ def capabilities(self): }, 'openClose': True, }, - # TODO: add notebookDocumentSync when we support it - # 'notebookDocumentSync' : { - # 'notebook': '*', - # }, + 'notebookDocumentSync' : { + 'notebookSelector': { + 'cells': [{ 'language': 'python' }] + } + }, 'workspace': { 'workspaceFolders': { 'supported': True, From 6c00bfc59a2f6d3b21b93dbbecdf0baeedbb2b12 Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Thu, 15 Jun 2023 11:00:54 +0200 Subject: [PATCH 14/27] Add notebookDocumentSync to capabilities --- pylsp/python_lsp.py | 8 ++++---- pylsp/workspace.py | 1 + test/test_notebook_document.py | 10 +++++----- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/pylsp/python_lsp.py b/pylsp/python_lsp.py index 8979c48d..f15dbd7a 100644 --- a/pylsp/python_lsp.py +++ b/pylsp/python_lsp.py @@ -268,9 +268,9 @@ def capabilities(self): }, 'openClose': True, }, - 'notebookDocumentSync' : { + 'notebookDocumentSync': { 'notebookSelector': { - 'cells': [{ 'language': 'python' }] + 'cells': [{'language': 'python'}] } }, 'workspace': { @@ -397,7 +397,7 @@ def _lint_text_document(self, doc_uri, workspace, is_saved): def _lint_notebook_document(self, notebook_document, workspace): """ Lint a notebook document. - + This is a bit more complicated than linting a text document, because we need to send the entire notebook document to the pylsp_lint hook, but we need to send the diagnostics back to the client on a per-cell basis. @@ -486,7 +486,7 @@ def m_notebook_document__did_close(self, notebookDocument=None, cellTextDocument def m_notebook_document__did_change(self, notebookDocument=None, change=None, **_kwargs): """ Changes to the notebook document. - + This could be one of the following: 1. Notebook metadata changed 2. Cell(s) added diff --git a/pylsp/workspace.py b/pylsp/workspace.py index 574cdc5c..074b6ad1 100644 --- a/pylsp/workspace.py +++ b/pylsp/workspace.py @@ -505,6 +505,7 @@ def add_cells(self, new_cells: List, start: int) -> None: def remove_cells(self, start: int, delete_count: int) -> None: del self.cells[start:start+delete_count] + # We inherit from Document for now to get the same API. However, cell document differ from the text documents in that # they have a language id. class Cell(Document): diff --git a/test/test_notebook_document.py b/test/test_notebook_document.py index 8b6adfa6..cc82575b 100644 --- a/test/test_notebook_document.py +++ b/test/test_notebook_document.py @@ -58,7 +58,7 @@ def client_server_pair(): client_server_pair_obj.client._endpoint.notify("exit") -def test_initialize(client_server_pair): # pylint: disable=redefined-outer-name +def test_initialize(client_server_pair): # pylint: disable=redefined-outer-name client, server = client_server_pair response = client._endpoint.request( "initialize", @@ -73,7 +73,7 @@ def test_initialize(client_server_pair): # pylint: disable=redefined-outer-name # TODO: assert that notebook capabilities are in response -def test_notebook_document__did_open(client_server_pair): # pylint: disable=redefined-outer-name +def test_notebook_document__did_open(client_server_pair): # pylint: disable=redefined-outer-name client, server = client_server_pair client._endpoint.request( "initialize", @@ -157,7 +157,7 @@ def test_notebook_document__did_open(client_server_pair): # pylint: disable=rede mock_notify.assert_has_calls(expected_call_args) -def test_notebook_document__did_change(client_server_pair): # pylint: disable=redefined-outer-name +def test_notebook_document__did_change(client_server_pair): # pylint: disable=redefined-outer-name client, server = client_server_pair client._endpoint.request( "initialize", @@ -258,7 +258,7 @@ def test_notebook_document__did_change(client_server_pair): # pylint: disable=re ) wait_for_condition(lambda: mock_notify.call_count >= 1) assert len(server.workspace.documents) == 2 - assert not "cell_2_uri" in server.workspace.documents + assert "cell_2_uri" not in server.workspace.documents assert len(server.workspace.get_document("notebook_uri").cells) == 1 expected_call_args = [ call( @@ -406,7 +406,7 @@ def test_notebook_document__did_change(client_server_pair): # pylint: disable=re mock_notify.assert_has_calls(expected_call_args) -def test_notebook__did_close(client_server_pair): # pylint: disable=redefined-outer-name +def test_notebook__did_close(client_server_pair): # pylint: disable=redefined-outer-name client, server = client_server_pair client._endpoint.request( "initialize", From 60517c14f77385208ce788119477225c36672627 Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Mon, 10 Jul 2023 15:10:14 +0200 Subject: [PATCH 15/27] fix: publishDiagnostics line starts at line 0 --- pylsp/python_lsp.py | 4 ++-- test/test_notebook_document.py | 28 ++++++++++++++-------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/pylsp/python_lsp.py b/pylsp/python_lsp.py index f15dbd7a..aa032829 100644 --- a/pylsp/python_lsp.py +++ b/pylsp/python_lsp.py @@ -440,8 +440,8 @@ def _lint_notebook_document(self, notebook_document, workspace): for diagnostic in document_diagnostics: if diagnostic['range']['start']['line'] > cell['line_end']: break - diagnostic['range']['start']['line'] = diagnostic['range']['start']['line'] - cell['line_start'] + 1 - diagnostic['range']['end']['line'] = diagnostic['range']['end']['line'] - cell['line_start'] + 1 + diagnostic['range']['start']['line'] = diagnostic['range']['start']['line'] - cell['line_start'] + diagnostic['range']['end']['line'] = diagnostic['range']['end']['line'] - cell['line_start'] cell_diagnostics.append(diagnostic) document_diagnostics.pop(0) diff --git a/test/test_notebook_document.py b/test/test_notebook_document.py index cc82575b..7fbba3ad 100644 --- a/test/test_notebook_document.py +++ b/test/test_notebook_document.py @@ -126,8 +126,8 @@ def test_notebook_document__did_open(client_server_pair): # pylint: disable=red { "source": "pyflakes", "range": { - "start": {"line": 1, "character": 0}, - "end": {"line": 1, "character": 11}, + "start": {"line": 0, "character": 0}, + "end": {"line": 0, "character": 11}, }, "message": "'sys' imported but unused", "severity": 2, @@ -143,8 +143,8 @@ def test_notebook_document__did_open(client_server_pair): # pylint: disable=red { "source": "pycodestyle", "range": { - "start": {"line": 1, "character": 5}, - "end": {"line": 1, "character": 5}, + "start": {"line": 0, "character": 5}, + "end": {"line": 0, "character": 5}, }, "message": "W292 no newline at end of file", "code": "W292", @@ -215,8 +215,8 @@ def test_notebook_document__did_change(client_server_pair): # pylint: disable=r { "source": "pyflakes", "range": { - "start": {"line": 1, "character": 0}, - "end": {"line": 1, "character": 11}, + "start": {"line": 0, "character": 0}, + "end": {"line": 0, "character": 11}, }, "message": "'sys' imported but unused", "severity": 2, @@ -269,8 +269,8 @@ def test_notebook_document__did_change(client_server_pair): # pylint: disable=r { "source": "pyflakes", "range": { - "start": {"line": 1, "character": 0}, - "end": {"line": 1, "character": 10}, + "start": {"line": 0, "character": 0}, + "end": {"line": 0, "character": 10}, }, "message": "'sys' imported but unused", "severity": 2, @@ -327,8 +327,8 @@ def test_notebook_document__did_change(client_server_pair): # pylint: disable=r { "source": "pyflakes", "range": { - "start": {"line": 1, "character": 0}, - "end": {"line": 1, "character": 11}, + "start": {"line": 0, "character": 0}, + "end": {"line": 0, "character": 11}, }, "message": "'sys' imported but unused", "severity": 2, @@ -344,8 +344,8 @@ def test_notebook_document__did_change(client_server_pair): # pylint: disable=r { "source": "pyflakes", "range": { - "start": {"line": 1, "character": 0}, - "end": {"line": 1, "character": 1}, + "start": {"line": 0, "character": 0}, + "end": {"line": 0, "character": 1}, }, "message": "undefined name 'x'", "severity": 1, @@ -392,8 +392,8 @@ def test_notebook_document__did_change(client_server_pair): # pylint: disable=r { "source": "pycodestyle", "range": { - "start": {"line": 1, "character": 8}, - "end": {"line": 1, "character": 8}, + "start": {"line": 0, "character": 8}, + "end": {"line": 0, "character": 8}, }, "message": "W292 no newline at end of file", "code": "W292", From ecc27ee1a4b12c7a03b3ca7530ebd126fa251b7f Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Mon, 10 Jul 2023 15:10:14 +0200 Subject: [PATCH 16/27] fix: publishDiagnostics starts at 0 and newlines are counted correctly --- pylsp/python_lsp.py | 22 +++-- pylsp/workspace.py | 20 +++++ test/test_notebook_document.py | 155 +++++++++++++++++++++++++++------ 3 files changed, 162 insertions(+), 35 deletions(-) diff --git a/pylsp/python_lsp.py b/pylsp/python_lsp.py index aa032829..15167a8c 100644 --- a/pylsp/python_lsp.py +++ b/pylsp/python_lsp.py @@ -416,34 +416,38 @@ def _lint_notebook_document(self, notebook_document, workspace): cell_uri = cell['document'] cell_document = workspace.get_cell_document(cell_uri) - num_lines = len(cell_document.lines) + num_lines = cell_document.line_count data = { 'uri': cell_uri, - 'line_start': offset + 1, - 'line_end': offset + num_lines, + 'line_start': offset, + 'line_end': offset + num_lines - 1, 'source': cell_document.source } cell_list.append(data) - total_source = total_source + "\n" + cell_document.source + if offset == 0: + total_source = cell_document.source + else: + maybe_newline = "" if total_source.endswith("\n") else "\n" + total_source += (maybe_newline + cell_document.source) offset += num_lines workspace.put_document(random_uri, total_source) document_diagnostics = flatten(self._hook('pylsp_lint', random_uri, is_saved=True)) - # Now we need to map the diagnostics back to the correct cell and - # publish them. + # Now we need to map the diagnostics back to the correct cell and publish them. + # Note: this is O(n*m) in the number of cells and diagnostics, respectively. for cell in cell_list: cell_diagnostics = [] for diagnostic in document_diagnostics: - if diagnostic['range']['start']['line'] > cell['line_end']: - break + if diagnostic['range']['start']['line'] > cell['line_end'] \ + or diagnostic['range']['end']['line'] < cell['line_start']: + continue diagnostic['range']['start']['line'] = diagnostic['range']['start']['line'] - cell['line_start'] diagnostic['range']['end']['line'] = diagnostic['range']['end']['line'] - cell['line_start'] cell_diagnostics.append(diagnostic) - document_diagnostics.pop(0) workspace.publish_diagnostics(cell['uri'], cell_diagnostics) diff --git a/pylsp/workspace.py b/pylsp/workspace.py index 074b6ad1..34545a1f 100644 --- a/pylsp/workspace.py +++ b/pylsp/workspace.py @@ -515,3 +515,23 @@ def __init__(self, uri, language_id, workspace, source=None, version=None, local rope_project_builder=None): super().__init__(uri, workspace, source, version, local, extra_sys_path, rope_project_builder) self.language_id = language_id + + @property + @lock + def line_count(self): + """" + Return the number of lines in the cell document. + + This function is used to combine all cell documents into one text document. Note that we need to count a + newline at the end of a non-empty text line as an extra line. + + Examples: + - "x\n" is a cell with two lines, "x" and "" + - "x" is a cell with one line, "x" + - "\n" is a cell with one line, "" + """ + if len(self.lines) == 0: + return 1 + + last_line = self.lines[-1] + return len(self.lines) + (last_line != "\n" and last_line.endswith("\n")) diff --git a/test/test_notebook_document.py b/test/test_notebook_document.py index 7fbba3ad..8f3c4a31 100644 --- a/test/test_notebook_document.py +++ b/test/test_notebook_document.py @@ -10,10 +10,10 @@ from pylsp.python_lsp import PythonLSPServer from pylsp.lsp import NotebookCellKind -CALL_TIMEOUT = 10 +CALL_TIMEOUT_IN_SECONDS = 10 -def wait_for_condition(condition, timeout=CALL_TIMEOUT): +def wait_for_condition(condition, timeout=CALL_TIMEOUT_IN_SECONDS): """Wait for a condition to be true, or timeout.""" start_time = time.time() while not condition(): @@ -51,9 +51,9 @@ def client_server_pair(): yield (client_server_pair_obj.client, client_server_pair_obj.server) - shutdown_response = client_server_pair_obj.client._endpoint.request("shutdown").result( - timeout=CALL_TIMEOUT - ) + shutdown_response = client_server_pair_obj.client._endpoint.request( + "shutdown" + ).result(timeout=CALL_TIMEOUT_IN_SECONDS) assert shutdown_response is None client_server_pair_obj.client._endpoint.notify("exit") @@ -67,13 +67,15 @@ def test_initialize(client_server_pair): # pylint: disable=redefined-outer-name "rootPath": os.path.dirname(__file__), "initializationOptions": {}, }, - ).result(timeout=CALL_TIMEOUT) + ).result(timeout=CALL_TIMEOUT_IN_SECONDS) assert server.workspace is not None assert "capabilities" in response # TODO: assert that notebook capabilities are in response -def test_notebook_document__did_open(client_server_pair): # pylint: disable=redefined-outer-name +def test_notebook_document__did_open( + client_server_pair, +): # pylint: disable=redefined-outer-name client, server = client_server_pair client._endpoint.request( "initialize", @@ -82,7 +84,7 @@ def test_notebook_document__did_open(client_server_pair): # pylint: disable=red "rootPath": os.path.dirname(__file__), "initializationOptions": {}, }, - ).result(timeout=CALL_TIMEOUT) + ).result(timeout=CALL_TIMEOUT_IN_SECONDS) with patch.object(server._endpoint, "notify") as mock_notify: client._endpoint.notify( @@ -100,28 +102,70 @@ def test_notebook_document__did_open(client_server_pair): # pylint: disable=red "kind": NotebookCellKind.Code, "document": "cell_2_uri", }, + { + "kind": NotebookCellKind.Code, + "document": "cell_3_uri", + }, + { + "kind": NotebookCellKind.Code, + "document": "cell_4_uri", + }, + { + "kind": NotebookCellKind.Code, + "document": "cell_5_uri", + }, ], }, + # Test as many edge cases as possible for the diagnostics message "cellTextDocuments": [ { "uri": "cell_1_uri", "languageId": "python", - "text": "import sys", + "text": "", }, { "uri": "cell_2_uri", "languageId": "python", - "text": "x = 1", + "text": "\n", + }, + { + "uri": "cell_3_uri", + "languageId": "python", + "text": "\nimport sys\n\nabc\n\n", + }, + { + "uri": "cell_4_uri", + "languageId": "python", + "text": "x", + }, + { + "uri": "cell_5_uri", + "languageId": "python", + "text": "y\n", }, ], }, ) - wait_for_condition(lambda: mock_notify.call_count >= 2) + wait_for_condition(lambda: mock_notify.call_count >= 5) expected_call_args = [ call( "textDocument/publishDiagnostics", params={ "uri": "cell_1_uri", + "diagnostics": [], + }, + ), + call( + "textDocument/publishDiagnostics", + params={ + "uri": "cell_2_uri", + "diagnostics": [], + }, + ), + call( + "textDocument/publishDiagnostics", + params={ + "uri": "cell_3_uri", "diagnostics": [ { "source": "pyflakes", @@ -131,25 +175,60 @@ def test_notebook_document__did_open(client_server_pair): # pylint: disable=red }, "message": "'sys' imported but unused", "severity": 2, - } + }, + { + "source": "pyflakes", + "range": { + "start": {"line": 3, "character": 0}, + "end": {"line": 3, "character": 4}, + }, + "message": "undefined name 'abc'", + "severity": 1, + }, + { + "source": "pycodestyle", + "range": { + "start": {"line": 1, "character": 0}, + "end": {"line": 1, "character": 11}, + }, + "message": "E303 too many blank lines (3)", + "code": "E303", + "severity": 2, + }, ], }, ), call( "textDocument/publishDiagnostics", params={ - "uri": "cell_2_uri", + "uri": "cell_4_uri", "diagnostics": [ { - "source": "pycodestyle", + "source": "pyflakes", "range": { - "start": {"line": 0, "character": 5}, - "end": {"line": 0, "character": 5}, + "start": {"line": 0, "character": 0}, + "end": {"line": 0, "character": 2}, }, - "message": "W292 no newline at end of file", - "code": "W292", - "severity": 2, - } + "message": "undefined name 'x'", + "severity": 1, + }, + ], + }, + ), + call( + "textDocument/publishDiagnostics", + params={ + "uri": "cell_5_uri", + "diagnostics": [ + { + "source": "pyflakes", + "range": { + "start": {"line": 0, "character": 0}, + "end": {"line": 0, "character": 2}, + }, + "message": "undefined name 'y'", + "severity": 1, + }, ], }, ), @@ -157,7 +236,9 @@ def test_notebook_document__did_open(client_server_pair): # pylint: disable=red mock_notify.assert_has_calls(expected_call_args) -def test_notebook_document__did_change(client_server_pair): # pylint: disable=redefined-outer-name +def test_notebook_document__did_change( + client_server_pair, +): # pylint: disable=redefined-outer-name client, server = client_server_pair client._endpoint.request( "initialize", @@ -166,7 +247,7 @@ def test_notebook_document__did_change(client_server_pair): # pylint: disable=r "rootPath": os.path.dirname(__file__), "initializationOptions": {}, }, - ).result(timeout=CALL_TIMEOUT) + ).result(timeout=CALL_TIMEOUT_IN_SECONDS) # Open notebook with patch.object(server._endpoint, "notify") as mock_notify: @@ -274,7 +355,17 @@ def test_notebook_document__did_change(client_server_pair): # pylint: disable=r }, "message": "'sys' imported but unused", "severity": 2, - } + }, + { + "source": "pycodestyle", + "range": { + "start": {"line": 0, "character": 10}, + "end": {"line": 0, "character": 10}, + }, + "message": "W292 no newline at end of file", + "code": "W292", + "severity": 2, + }, ], }, ) @@ -349,7 +440,17 @@ def test_notebook_document__did_change(client_server_pair): # pylint: disable=r }, "message": "undefined name 'x'", "severity": 1, - } + }, + { + "source": "pycodestyle", + "range": { + "start": {"line": 0, "character": 1}, + "end": {"line": 0, "character": 1}, + }, + "message": "W292 no newline at end of file", + "code": "W292", + "severity": 2, + }, ], }, ), @@ -406,7 +507,9 @@ def test_notebook_document__did_change(client_server_pair): # pylint: disable=r mock_notify.assert_has_calls(expected_call_args) -def test_notebook__did_close(client_server_pair): # pylint: disable=redefined-outer-name +def test_notebook__did_close( + client_server_pair, +): # pylint: disable=redefined-outer-name client, server = client_server_pair client._endpoint.request( "initialize", @@ -415,7 +518,7 @@ def test_notebook__did_close(client_server_pair): # pylint: disable=redefined- "rootPath": os.path.dirname(__file__), "initializationOptions": {}, }, - ).result(timeout=CALL_TIMEOUT) + ).result(timeout=CALL_TIMEOUT_IN_SECONDS) # Open notebook with patch.object(server._endpoint, "notify") as mock_notify: From 25cdfaba4ac5bab048510b6d02da35b98a1f66c0 Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Mon, 10 Jul 2023 15:10:14 +0200 Subject: [PATCH 17/27] fix: publishDiagnostics starts at 0 and newlines are counted correctly --- test/test_notebook_document.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/test_notebook_document.py b/test/test_notebook_document.py index 8f3c4a31..031d7cfc 100644 --- a/test/test_notebook_document.py +++ b/test/test_notebook_document.py @@ -69,8 +69,7 @@ def test_initialize(client_server_pair): # pylint: disable=redefined-outer-name }, ).result(timeout=CALL_TIMEOUT_IN_SECONDS) assert server.workspace is not None - assert "capabilities" in response - # TODO: assert that notebook capabilities are in response + assert "notebookDocumentSync" in response["capabilities"].keys() def test_notebook_document__did_open( From 2b35c9507ca79209e2ae5f5e1fc99f455b43c918 Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Mon, 10 Jul 2023 15:10:14 +0200 Subject: [PATCH 18/27] fix: publishDiagnostics starts at 0 and newlines are counted correctly --- pylsp/python_lsp.py | 2 +- test/test_notebook_document.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pylsp/python_lsp.py b/pylsp/python_lsp.py index 15167a8c..7cdb6702 100644 --- a/pylsp/python_lsp.py +++ b/pylsp/python_lsp.py @@ -394,7 +394,7 @@ def _lint_text_document(self, doc_uri, workspace, is_saved): flatten(self._hook('pylsp_lint', doc_uri, is_saved=is_saved)) ) - def _lint_notebook_document(self, notebook_document, workspace): + def _lint_notebook_document(self, notebook_document, workspace): # pylint: disable=too-many-locals """ Lint a notebook document. diff --git a/test/test_notebook_document.py b/test/test_notebook_document.py index 031d7cfc..34499267 100644 --- a/test/test_notebook_document.py +++ b/test/test_notebook_document.py @@ -10,7 +10,7 @@ from pylsp.python_lsp import PythonLSPServer from pylsp.lsp import NotebookCellKind -CALL_TIMEOUT_IN_SECONDS = 10 +CALL_TIMEOUT_IN_SECONDS = 30 def wait_for_condition(condition, timeout=CALL_TIMEOUT_IN_SECONDS): From 69a1621365a7e384056cd65a4a9157adc256f5d0 Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Mon, 10 Jul 2023 15:10:14 +0200 Subject: [PATCH 19/27] fix: publishDiagnostics starts at 0 and newlines are counted correctly --- pylsp/python_lsp.py | 3 +-- pylsp/workspace.py | 18 ++---------------- test/test_notebook_document.py | 6 +++--- 3 files changed, 6 insertions(+), 21 deletions(-) diff --git a/pylsp/python_lsp.py b/pylsp/python_lsp.py index 7cdb6702..5c089921 100644 --- a/pylsp/python_lsp.py +++ b/pylsp/python_lsp.py @@ -429,8 +429,7 @@ def _lint_notebook_document(self, notebook_document, workspace): # pylint: dis if offset == 0: total_source = cell_document.source else: - maybe_newline = "" if total_source.endswith("\n") else "\n" - total_source += (maybe_newline + cell_document.source) + total_source += ("\n" + cell_document.source) offset += num_lines diff --git a/pylsp/workspace.py b/pylsp/workspace.py index 34545a1f..0c868b6c 100644 --- a/pylsp/workspace.py +++ b/pylsp/workspace.py @@ -519,19 +519,5 @@ def __init__(self, uri, language_id, workspace, source=None, version=None, local @property @lock def line_count(self): - """" - Return the number of lines in the cell document. - - This function is used to combine all cell documents into one text document. Note that we need to count a - newline at the end of a non-empty text line as an extra line. - - Examples: - - "x\n" is a cell with two lines, "x" and "" - - "x" is a cell with one line, "x" - - "\n" is a cell with one line, "" - """ - if len(self.lines) == 0: - return 1 - - last_line = self.lines[-1] - return len(self.lines) + (last_line != "\n" and last_line.endswith("\n")) + """"Return the number of lines in the cell document.""" + return len(self.source.split('\n')) diff --git a/test/test_notebook_document.py b/test/test_notebook_document.py index 34499267..c334734f 100644 --- a/test/test_notebook_document.py +++ b/test/test_notebook_document.py @@ -169,8 +169,8 @@ def test_notebook_document__did_open( { "source": "pyflakes", "range": { - "start": {"line": 0, "character": 0}, - "end": {"line": 0, "character": 11}, + "start": {"line": 1, "character": 0}, + "end": {"line": 1, "character": 11}, }, "message": "'sys' imported but unused", "severity": 2, @@ -190,7 +190,7 @@ def test_notebook_document__did_open( "start": {"line": 1, "character": 0}, "end": {"line": 1, "character": 11}, }, - "message": "E303 too many blank lines (3)", + "message": "E303 too many blank lines (4)", "code": "E303", "severity": 2, }, From 1e9a51f2dc1bd2754dc198bb301431a2921c35ea Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Mon, 10 Jul 2023 15:10:14 +0200 Subject: [PATCH 20/27] fix: publishDiagnostics starts at 0 and newlines are counted correctly --- pylsp/python_lsp.py | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/pylsp/python_lsp.py b/pylsp/python_lsp.py index 5c089921..25907b88 100644 --- a/pylsp/python_lsp.py +++ b/pylsp/python_lsp.py @@ -434,23 +434,25 @@ def _lint_notebook_document(self, notebook_document, workspace): # pylint: dis offset += num_lines workspace.put_document(random_uri, total_source) - document_diagnostics = flatten(self._hook('pylsp_lint', random_uri, is_saved=True)) - - # Now we need to map the diagnostics back to the correct cell and publish them. - # Note: this is O(n*m) in the number of cells and diagnostics, respectively. - for cell in cell_list: - cell_diagnostics = [] - for diagnostic in document_diagnostics: - if diagnostic['range']['start']['line'] > cell['line_end'] \ - or diagnostic['range']['end']['line'] < cell['line_start']: - continue - diagnostic['range']['start']['line'] = diagnostic['range']['start']['line'] - cell['line_start'] - diagnostic['range']['end']['line'] = diagnostic['range']['end']['line'] - cell['line_start'] - cell_diagnostics.append(diagnostic) - - workspace.publish_diagnostics(cell['uri'], cell_diagnostics) - workspace.rm_document(random_uri) + try: + document_diagnostics = flatten(self._hook('pylsp_lint', random_uri, is_saved=True)) + + # Now we need to map the diagnostics back to the correct cell and publish them. + # Note: this is O(n*m) in the number of cells and diagnostics, respectively. + for cell in cell_list: + cell_diagnostics = [] + for diagnostic in document_diagnostics: + if diagnostic['range']['start']['line'] > cell['line_end'] \ + or diagnostic['range']['end']['line'] < cell['line_start']: + continue + diagnostic['range']['start']['line'] = diagnostic['range']['start']['line'] - cell['line_start'] + diagnostic['range']['end']['line'] = diagnostic['range']['end']['line'] - cell['line_start'] + cell_diagnostics.append(diagnostic) + + workspace.publish_diagnostics(cell['uri'], cell_diagnostics) + finally: + workspace.rm_document(random_uri) def references(self, doc_uri, position, exclude_declaration): return flatten(self._hook( From 4f140daabb3d21f8bc8d8733f3f37ce80004732a Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Mon, 10 Jul 2023 15:10:14 +0200 Subject: [PATCH 21/27] fix: publishDiagnostics starts at 0 and newlines are counted correctly --- pylsp/python_lsp.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylsp/python_lsp.py b/pylsp/python_lsp.py index 25907b88..844f449b 100644 --- a/pylsp/python_lsp.py +++ b/pylsp/python_lsp.py @@ -394,7 +394,7 @@ def _lint_text_document(self, doc_uri, workspace, is_saved): flatten(self._hook('pylsp_lint', doc_uri, is_saved=is_saved)) ) - def _lint_notebook_document(self, notebook_document, workspace): # pylint: disable=too-many-locals + def _lint_notebook_document(self, notebook_document, workspace): """ Lint a notebook document. From e79bbb2e0cfebce0101a79f68c9376acebee1298 Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Thu, 20 Jul 2023 13:31:02 +0200 Subject: [PATCH 22/27] feat: close cell if deleted --- pylsp/python_lsp.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/pylsp/python_lsp.py b/pylsp/python_lsp.py index 844f449b..7edbe82c 100644 --- a/pylsp/python_lsp.py +++ b/pylsp/python_lsp.py @@ -394,7 +394,7 @@ def _lint_text_document(self, doc_uri, workspace, is_saved): flatten(self._hook('pylsp_lint', doc_uri, is_saved=is_saved)) ) - def _lint_notebook_document(self, notebook_document, workspace): + def _lint_notebook_document(self, notebook_document, workspace): # pylint: disable=too-many-locals """ Lint a notebook document. @@ -443,11 +443,13 @@ def _lint_notebook_document(self, notebook_document, workspace): for cell in cell_list: cell_diagnostics = [] for diagnostic in document_diagnostics: - if diagnostic['range']['start']['line'] > cell['line_end'] \ - or diagnostic['range']['end']['line'] < cell['line_start']: + start_line = diagnostic['range']['start']['line'] + end_line = diagnostic['range']['end']['line'] + + if start_line > cell['line_end'] or end_line < cell['line_start']: continue - diagnostic['range']['start']['line'] = diagnostic['range']['start']['line'] - cell['line_start'] - diagnostic['range']['end']['line'] = diagnostic['range']['end']['line'] - cell['line_start'] + diagnostic['range']['start']['line'] = start_line - cell['line_start'] + diagnostic['range']['end']['line'] = end_line - cell['line_start'] cell_diagnostics.append(diagnostic) workspace.publish_diagnostics(cell['uri'], cell_diagnostics) @@ -528,6 +530,7 @@ def m_notebook_document__did_change(self, notebookDocument=None, change=None, ** # Cell documents for cell_document in structure['didClose']: workspace.rm_document(cell_document['uri']) + workspace.publish_diagnostics(cell_document['uri'], []) # Cell metadata which is removed from Notebook workspace.remove_notebook_cells(notebookDocument['uri'], start, cell_delete_count) From ce9f45881d499e8781ac0fc4f77fd88649be059f Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Sat, 22 Jul 2023 13:45:59 +0200 Subject: [PATCH 23/27] skip tests on windows as it's flaky on py3.7 --- test/test_notebook_document.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/test_notebook_document.py b/test/test_notebook_document.py index c334734f..ec3e5ee4 100644 --- a/test/test_notebook_document.py +++ b/test/test_notebook_document.py @@ -7,6 +7,7 @@ import pytest +from pylsp import IS_WIN from pylsp.python_lsp import PythonLSPServer from pylsp.lsp import NotebookCellKind @@ -58,6 +59,7 @@ def client_server_pair(): client_server_pair_obj.client._endpoint.notify("exit") +@pytest.mark.skipif(IS_WIN, reason="Flaky on Windows") def test_initialize(client_server_pair): # pylint: disable=redefined-outer-name client, server = client_server_pair response = client._endpoint.request( @@ -72,6 +74,7 @@ def test_initialize(client_server_pair): # pylint: disable=redefined-outer-name assert "notebookDocumentSync" in response["capabilities"].keys() +@pytest.mark.skipif(IS_WIN, reason="Flaky on Windows") def test_notebook_document__did_open( client_server_pair, ): # pylint: disable=redefined-outer-name @@ -235,6 +238,7 @@ def test_notebook_document__did_open( mock_notify.assert_has_calls(expected_call_args) +@pytest.mark.skipif(IS_WIN, reason="Flaky on Windows") def test_notebook_document__did_change( client_server_pair, ): # pylint: disable=redefined-outer-name @@ -506,6 +510,7 @@ def test_notebook_document__did_change( mock_notify.assert_has_calls(expected_call_args) +@pytest.mark.skipif(IS_WIN, reason="Flaky on Windows") def test_notebook__did_close( client_server_pair, ): # pylint: disable=redefined-outer-name From 5434d1f1b0ba301f8fdcc6539a1bfdb6a720884c Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Mon, 24 Jul 2023 23:10:48 +0200 Subject: [PATCH 24/27] fix test_notebook_document__did_change: need to wait for 2 calls to diagnostics notification since the first call is a n empty diagnostics for the closed cell --- test/test_notebook_document.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_notebook_document.py b/test/test_notebook_document.py index ec3e5ee4..a85ff931 100644 --- a/test/test_notebook_document.py +++ b/test/test_notebook_document.py @@ -340,7 +340,7 @@ def test_notebook_document__did_change( }, }, ) - wait_for_condition(lambda: mock_notify.call_count >= 1) + wait_for_condition(lambda: mock_notify.call_count >= 2) assert len(server.workspace.documents) == 2 assert "cell_2_uri" not in server.workspace.documents assert len(server.workspace.get_document("notebook_uri").cells) == 1 From ceb193db4d74ecdebb861382e916c0446d7268e8 Mon Sep 17 00:00:00 2001 From: tkrabel-db <91616041+tkrabel-db@users.noreply.github.com> Date: Sat, 29 Jul 2023 21:03:24 +0200 Subject: [PATCH 25/27] Update test/data/publish_diagnostics_message_examples/example_1.json Co-authored-by: Carlos Cordoba --- test/data/publish_diagnostics_message_examples/example_1.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/data/publish_diagnostics_message_examples/example_1.json b/test/data/publish_diagnostics_message_examples/example_1.json index 4fa425b9..25d43a17 100644 --- a/test/data/publish_diagnostics_message_examples/example_1.json +++ b/test/data/publish_diagnostics_message_examples/example_1.json @@ -33,4 +33,4 @@ } ], "uri": "/Users/.../code/python-lsp-server/test" -} \ No newline at end of file +} From e7df839b0301acb96a2aa8345167b85ced29d552 Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Sat, 29 Jul 2023 20:55:49 +0200 Subject: [PATCH 26/27] remove aliases --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 131fdf45..5016fd6b 100644 --- a/README.md +++ b/README.md @@ -165,8 +165,8 @@ Dev install ``` # create conda env -cc python-lsp-server -ca python-lsp-server +conda create --name python-lsp-server python=3.8 -y +conda activate python-lsp-server pip install ".[all]" pip install ".[websockets]" From 2dd05b75aff7343f53b8ed4a54baef4009bb5ecb Mon Sep 17 00:00:00 2001 From: Tobias Krabel Date: Sat, 29 Jul 2023 21:02:38 +0200 Subject: [PATCH 27/27] address comments --- pylsp/python_lsp.py | 18 +++++++++++++----- pylsp/workspace.py | 11 ++++++++--- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/pylsp/python_lsp.py b/pylsp/python_lsp.py index 7edbe82c..9302c4d2 100644 --- a/pylsp/python_lsp.py +++ b/pylsp/python_lsp.py @@ -476,9 +476,13 @@ def m_completion_item__resolve(self, **completionItem): def m_notebook_document__did_open(self, notebookDocument=None, cellTextDocuments=None, **_kwargs): workspace = self._match_uri_to_workspace(notebookDocument['uri']) - workspace.put_notebook_document(notebookDocument['uri'], notebookDocument['notebookType'], - cells=notebookDocument['cells'], version=notebookDocument.get('version'), - metadata=notebookDocument.get('metadata')) + workspace.put_notebook_document( + notebookDocument['uri'], + notebookDocument['notebookType'], + cells=notebookDocument['cells'], + version=notebookDocument.get('version'), + metadata=notebookDocument.get('metadata') + ) for cell in (cellTextDocuments or []): workspace.put_cell_document(cell['uri'], cell['languageId'], cell['text'], version=cell.get('version')) self.lint(notebookDocument['uri'], is_saved=True) @@ -521,8 +525,12 @@ def m_notebook_document__did_change(self, notebookDocument=None, change=None, ** # Case 2 # Cell documents for cell_document in structure['didOpen']: - workspace.put_cell_document(cell_document['uri'], cell_document['languageId'], - cell_document['text'], cell_document.get('version')) + workspace.put_cell_document( + cell_document['uri'], + cell_document['languageId'], + cell_document['text'], + cell_document.get('version') + ) # Cell metadata which is added to Notebook workspace.add_notebook_cells(notebookDocument['uri'], notebook_cell_array_change['cells'], start) else: diff --git a/pylsp/workspace.py b/pylsp/workspace.py index 0c868b6c..c97d97ad 100644 --- a/pylsp/workspace.py +++ b/pylsp/workspace.py @@ -506,10 +506,15 @@ def remove_cells(self, start: int, delete_count: int) -> None: del self.cells[start:start+delete_count] -# We inherit from Document for now to get the same API. However, cell document differ from the text documents in that -# they have a language id. class Cell(Document): - """Represents a cell in a notebook.""" + """ + Represents a cell in a notebook. + + Notes + ----- + We inherit from Document for now to get the same API. However, a cell document differs from text documents in that + they have a language id. + """ def __init__(self, uri, language_id, workspace, source=None, version=None, local=True, extra_sys_path=None, rope_project_builder=None):