From ecefb7318ae31451ed6d4a7f7cc55583fff7049f Mon Sep 17 00:00:00 2001 From: Carlos Cordoba Date: Wed, 4 Oct 2023 13:18:21 -0400 Subject: [PATCH] Fix renaming when file has no EOLs (#450) --- pylsp/plugins/jedi_rename.py | 4 +++- pylsp/plugins/rope_rename.py | 8 +++++-- test/plugins/test_jedi_rename.py | 38 +++++++++++++++++++++++++++----- test/plugins/test_rope_rename.py | 28 ++++++++++++++++++++++- 4 files changed, 68 insertions(+), 10 deletions(-) diff --git a/pylsp/plugins/jedi_rename.py b/pylsp/plugins/jedi_rename.py index 700da508..9c89c1de 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 f59ba890..a4323a42 100644 --- a/pylsp/plugins/rope_rename.py +++ b/pylsp/plugins/rope_rename.py @@ -6,7 +6,7 @@ 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 diff --git a/test/plugins/test_jedi_rename.py b/test/plugins/test_jedi_rename.py index c3a1e485..d88a9297 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 @@ class Test2(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 285a565e..9b9039ba 100644 --- a/test/plugins/test_rope_rename.py +++ b/test/plugins/test_rope_rename.py @@ -8,6 +8,7 @@ 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", + } + ]