Do not create documents from a textDocument/rename (#818)

Author: rwols

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):

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):

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)
 

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
+        }
+    ]

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": {