Repository: python-lsp/python-lsp-server

Issue:
textDocument/rename reports positions outside of the document
When having a document that ends without a trailing newline 'textDocument/rename' reports a document change that contains a position outside of the document as the end position. For example a document just containing of:
`foo=12`

Ttriggering a rename on the beginning of foo with the replacement bar results in the following edits:
`
                "edits": [
                    {
                        "newText": "bar = 12",
                        "range": {
                            "end": {
                                "character": 0,
                                "line": 1
                            },
                            "start": {
                                "character": 0,
                                "line": 0
                            }
                        }
                    }
                ],
`

But the document just has one line, line 1 is out of the document scope. Expected would be 
`
                            "end": {
                                "character": 6,
                                "line": 0
                            }
`

Discussion:

Patch:
diff --git a/pylsp/plugins/jedi_rename.py b/pylsp/plugins/jedi_rename.py
index 700da50..9c89c1d 100644
--- a/pylsp/plugins/jedi_rename.py
+++ b/pylsp/plugins/jedi_rename.py
@@ -54,4 +54,6 @@ def pylsp_rename(
 
 def _num_lines(file_contents):
     "Count the number of lines in the given string."
-    return len(file_contents.splitlines())
+    if _utils.get_eol_chars(file_contents):
+        return len(file_contents.splitlines())
+    return 0
diff --git a/pylsp/plugins/rope_rename.py b/pylsp/plugins/rope_rename.py
index f59ba89..a4323a4 100644
--- a/pylsp/plugins/rope_rename.py
+++ b/pylsp/plugins/rope_rename.py
@@ -6,7 +6,7 @@ import logging
 from rope.base import libutils
 from rope.refactor.rename import Rename
 
-from pylsp import hookimpl, uris
+from pylsp import hookimpl, uris, _utils
 
 log = logging.getLogger(__name__)
 
@@ -59,4 +59,8 @@ def pylsp_rename(config, workspace, document, position, new_name):
 
 def _num_lines(resource):
     "Count the number of lines in a `File` resource."
-    return len(resource.read().splitlines())
+    text = resource.read()
+
+    if _utils.get_eol_chars(text):
+        return len(text.splitlines())
+    return 0


Test patch:
diff --git a/test/plugins/test_jedi_rename.py b/test/plugins/test_jedi_rename.py
index c3a1e48..d88a929 100644
--- a/test/plugins/test_jedi_rename.py
+++ b/test/plugins/test_jedi_rename.py
@@ -2,16 +2,12 @@
 # Copyright 2021- Python Language Server Contributors.
 
 import os
-import sys
 
 import pytest
 from pylsp import uris
 from pylsp.plugins.jedi_rename import pylsp_rename
 from pylsp.workspace import Document
 
-LT_PY36 = sys.version_info.major < 3 or (
-    sys.version_info.major == 3 and sys.version_info.minor < 6
-)
 
 DOC_NAME = "test1.py"
 DOC = """class Test1():
@@ -26,13 +22,17 @@ DOC_EXTRA = """from test1 import Test1
 x = Test1()
 """
 
+DOC_NAME_SIMPLE = "test3.py"
+DOC_SIMPLE = "foo = 12"
+
 
 @pytest.fixture
 def tmp_workspace(temp_workspace_factory):
-    return temp_workspace_factory({DOC_NAME: DOC, DOC_NAME_EXTRA: DOC_EXTRA})
+    return temp_workspace_factory(
+        {DOC_NAME: DOC, DOC_NAME_EXTRA: DOC_EXTRA, DOC_NAME_SIMPLE: DOC_SIMPLE}
+    )
 
 
-@pytest.mark.skipif(LT_PY36, reason="Jedi refactoring isnt supported on Python 2.x/3.5")
 def test_jedi_rename(tmp_workspace, config):  # pylint: disable=redefined-outer-name
     # rename the `Test1` class
     position = {"line": 0, "character": 6}
@@ -56,6 +56,7 @@ 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
@@ -63,6 +64,7 @@ def test_jedi_rename(tmp_workspace, config):  # pylint: disable=redefined-outer-
     # 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
@@ -77,3 +79,27 @@ def test_jedi_rename(tmp_workspace, config):  # pylint: disable=redefined-outer-
             "newText": expected,
         }
     ]
+
+    # Regression test for issue python-lsp/python-lsp-server#413
+    # rename foo
+    position = {"line": 0, "character": 0}
+    DOC_URI = uris.from_fs_path(os.path.join(tmp_workspace.root_path, DOC_NAME_SIMPLE))
+    doc = Document(DOC_URI, tmp_workspace)
+
+    result = pylsp_rename(config, tmp_workspace, doc, position, "bar")
+    assert len(result.keys()) == 1
+
+    changes = result.get("documentChanges")
+    assert len(changes) == 1
+
+    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},
+                "end": {"line": 0, "character": 0},
+            },
+            "newText": "bar = 12",
+        }
+    ]
diff --git a/test/plugins/test_rope_rename.py b/test/plugins/test_rope_rename.py
index 285a565..9b9039b 100644
--- a/test/plugins/test_rope_rename.py
+++ b/test/plugins/test_rope_rename.py
@@ -8,6 +8,7 @@ from pylsp import uris
 from pylsp.plugins.rope_rename import pylsp_rename
 from pylsp.workspace import Document
 
+
 DOC_NAME = "test1.py"
 DOC = """class Test1():
     pass
@@ -16,10 +17,13 @@ class Test2(Test1):
     pass
 """
 
+DOC_NAME_SIMPLE = "test2.py"
+DOC_SIMPLE = "foo = 12"
+
 
 @pytest.fixture
 def tmp_workspace(temp_workspace_factory):
-    return temp_workspace_factory({DOC_NAME: DOC})
+    return temp_workspace_factory({DOC_NAME: DOC, DOC_NAME_SIMPLE: DOC_SIMPLE})
 
 
 def test_rope_rename(tmp_workspace, config):  # pylint: disable=redefined-outer-name
@@ -45,3 +49,25 @@ def test_rope_rename(tmp_workspace, config):  # pylint: disable=redefined-outer-
             "newText": "class ShouldBeRenamed():\n    pass\n\nclass Test2(ShouldBeRenamed):\n    pass\n",
         }
     ]
+
+    # Regression test for issue python-lsp/python-lsp-server#413
+    # rename foo
+    position = {"line": 0, "character": 0}
+    DOC_URI = uris.from_fs_path(os.path.join(tmp_workspace.root_path, DOC_NAME_SIMPLE))
+    doc = Document(DOC_URI, tmp_workspace)
+
+    result = pylsp_rename(config, tmp_workspace, doc, position, "bar")
+    assert len(result.keys()) == 1
+
+    changes = result.get("documentChanges")
+    assert len(changes) == 1
+
+    assert changes[0].get("edits") == [
+        {
+            "range": {
+                "start": {"line": 0, "character": 0},
+                "end": {"line": 0, "character": 0},
+            },
+            "newText": "bar = 12",
+        }
+    ]