From 10cd98c898c89a8c9961b25725a6cd913485e1d0 Mon Sep 17 00:00:00 2001 From: Raoul Wols Date: Thu, 18 Jun 2020 20:25:55 +0200 Subject: [PATCH] Do not create documents from a textDocument/rename (#818) --- pyls/plugins/jedi_rename.py | 45 ++++++++++++++++---------------- pyls/plugins/rope_rename.py | 37 ++++++++++++++------------ pyls/workspace.py | 3 +++ test/plugins/test_jedi_rename.py | 37 +++++++++++++++++++++++--- test/plugins/test_rope_rename.py | 2 ++ 5 files changed, 81 insertions(+), 43 deletions(-) diff --git a/pyls/plugins/jedi_rename.py b/pyls/plugins/jedi_rename.py index 2e633d71..a17b46ec 100644 --- a/pyls/plugins/jedi_rename.py +++ b/pyls/plugins/jedi_rename.py @@ -17,30 +17,29 @@ def pyls_rename(config, workspace, document, position, new_name): # pylint: dis raise Exception('No support for renaming in Python 2/3.5 with Jedi. ' 'Consider using the rope_rename plugin instead') log.debug('Finished rename: %s', refactoring.get_diff()) - - return { - 'documentChanges': [ - { - 'textDocument': { - 'uri': uris.uri_with(document.uri, path=file_path), - 'version': workspace.get_document(document.uri).version, - }, - 'edits': [ - { - 'range': { - 'start': {'line': 0, 'character': 0}, - 'end': { - 'line': _num_lines(changed_file.get_new_code()), - 'character': 0, - }, + changes = [] + for file_path, changed_file in refactoring.get_changed_files().items(): + uri = uris.from_fs_path(file_path) + doc = workspace.get_maybe_document(uri) + changes.append({ + 'textDocument': { + 'uri': uri, + 'version': doc.version if doc else None + }, + 'edits': [ + { + 'range': { + 'start': {'line': 0, 'character': 0}, + 'end': { + 'line': _num_lines(changed_file.get_new_code()), + 'character': 0, }, - 'newText': changed_file.get_new_code(), - } - ], - } - for file_path, changed_file in refactoring.get_changed_files().items() - ], - } + }, + 'newText': changed_file.get_new_code(), + } + ], + }) + return {'documentChanges': changes} def _num_lines(file_contents): diff --git a/pyls/plugins/rope_rename.py b/pyls/plugins/rope_rename.py index 80091c0d..730ae333 100644 --- a/pyls/plugins/rope_rename.py +++ b/pyls/plugins/rope_rename.py @@ -1,6 +1,5 @@ # Copyright 2017 Palantir Technologies, Inc. import logging -import os from rope.base import libutils from rope.refactor.rename import Rename @@ -30,23 +29,29 @@ def pyls_rename(config, workspace, document, position, new_name): log.debug("Executing rename of %s to %s", document.word_at_position(position), new_name) changeset = rename.get_changes(new_name, in_hierarchy=True, docs=True) log.debug("Finished rename: %s", changeset.changes) - return { - 'documentChanges': [{ + changes = [] + for change in changeset.changes: + uri = uris.from_fs_path(change.resource.path) + doc = workspace.get_maybe_document(uri) + changes.append({ 'textDocument': { - 'uri': uris.uri_with( - document.uri, path=os.path.join(workspace.root_path, change.resource.path) - ), - 'version': workspace.get_document(document.uri).version + 'uri': uri, + 'version': doc.version if doc else None }, - 'edits': [{ - 'range': { - 'start': {'line': 0, 'character': 0}, - 'end': {'line': _num_lines(change.resource), 'character': 0}, - }, - 'newText': change.new_contents - }] - } for change in changeset.changes] - } + 'edits': [ + { + 'range': { + 'start': {'line': 0, 'character': 0}, + 'end': { + 'line': _num_lines(change.resource), + 'character': 0, + }, + }, + 'newText': change.new_contents, + } + ] + }) + return {'documentChanges': changes} def _num_lines(resource): diff --git a/pyls/workspace.py b/pyls/workspace.py index 09acc8e2..cf582c34 100644 --- a/pyls/workspace.py +++ b/pyls/workspace.py @@ -71,6 +71,9 @@ def get_document(self, doc_uri): """ return self._docs.get(doc_uri) or self._create_document(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) diff --git a/test/plugins/test_jedi_rename.py b/test/plugins/test_jedi_rename.py index 1d82d954..034f4a18 100644 --- a/test/plugins/test_jedi_rename.py +++ b/test/plugins/test_jedi_rename.py @@ -17,10 +17,18 @@ class Test2(Test1): pass ''' +DOC_NAME_EXTRA = 'test2.py' +DOC_EXTRA = '''from test1 import Test1 +x = Test1() +''' + @pytest.fixture def tmp_workspace(temp_workspace_factory): - return temp_workspace_factory({DOC_NAME: DOC}) + return temp_workspace_factory({ + DOC_NAME: DOC, + DOC_NAME_EXTRA: DOC_EXTRA + }) @pytest.mark.skipif(LT_PY36, reason='Jedi refactoring isnt supported on Python 2.x/3.5') @@ -34,10 +42,11 @@ def test_jedi_rename(tmp_workspace, config): # pylint: disable=redefined-outer- assert len(result.keys()) == 1 changes = result.get('documentChanges') - assert len(changes) == 1 - changes = changes[0] + assert len(changes) == 2 - assert changes.get('edits') == [ + assert changes[0]['textDocument']['uri'] == doc.uri + assert changes[0]['textDocument']['version'] == doc.version + assert changes[0].get('edits') == [ { 'range': { 'start': {'line': 0, 'character': 0}, @@ -46,3 +55,23 @@ def test_jedi_rename(tmp_workspace, config): # pylint: disable=redefined-outer- 'newText': 'class ShouldBeRenamed():\n pass\n\nclass Test2(ShouldBeRenamed):\n pass\n', } ] + path = os.path.join(tmp_workspace.root_path, DOC_NAME_EXTRA) + uri_extra = uris.from_fs_path(path) + assert changes[1]['textDocument']['uri'] == uri_extra + # This also checks whether documents not yet added via textDocument/didOpen + # but that do need to be renamed in the project have a `null` version + # number. + assert changes[1]['textDocument']['version'] is None + expected = 'from test1 import ShouldBeRenamed\nx = ShouldBeRenamed()\n' + if os.name == 'nt': + # The .write method in the temp_workspace_factory functions writes + # Windows-style line-endings. + expected = expected.replace('\n', '\r\n') + assert changes[1].get('edits') == [ + { + 'range': { + 'start': {'line': 0, 'character': 0}, + 'end': {'line': 2, 'character': 0}}, + 'newText': expected + } + ] diff --git a/test/plugins/test_rope_rename.py b/test/plugins/test_rope_rename.py index 45bd6bbd..1fc32226 100644 --- a/test/plugins/test_rope_rename.py +++ b/test/plugins/test_rope_rename.py @@ -31,6 +31,8 @@ def test_rope_rename(tmp_workspace, config): # pylint: disable=redefined-outer- assert len(changes) == 1 changes = changes[0] + # Note that this test differs from test_jedi_rename, because rope does not + # seem to modify files that haven't been opened with textDocument/didOpen. assert changes.get("edits") == [ { "range": {