8000 Fix renaming when file has no EOLs by ccordoba12 · Pull Request #450 · python-lsp/python-lsp-server · GitHub
[go: up one dir, main page]
Skip to content
8000

Fix renaming when file has no EOLs #450

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 4, 2023
Merged

Fix renaming when file has no EOLs #450

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion pylsp/plugins/jedi_rename.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 6 additions & 2 deletions pylsp/plugins/rope_rename.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from rope.base import libutils
from rope.refactor.rename import Ren 8000 ame

from pylsp import hookimpl, uris
from pylsp import hookimpl, uris, _utils

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -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
38 changes: 32 additions & 6 deletions test/plugins/test_jedi_rename.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand All @@ -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}
Expand All @@ -56,13 +56,15 @@ 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
Expand All @@ -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",
}
]
28 changes: 27 additions & 1 deletion test/plugins/test_rope_rename.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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",
}
]
0