From 84cb3a50e1677ad60a213a785ae5e47c499eba61 Mon Sep 17 00:00:00 2001 From: Shahriar Heidrich Date: Sun, 11 May 2025 17:41:59 +0200 Subject: [PATCH 1/7] Add basic Goto Implementation Request support --- CONFIGURATION.md | 1 + README.md | 4 +-- pylsp/config/schema.json | 5 ++++ pylsp/hookspecs.py | 5 ++++ pylsp/plugins/rope_implementation.py | 43 ++++++++++++++++++++++++++++ pylsp/python_lsp.py | 10 +++++++ pyproject.toml | 1 + 7 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 pylsp/plugins/rope_implementation.py diff --git a/CONFIGURATION.md b/CONFIGURATION.md index 0609169b..ee885081 100644 --- a/CONFIGURATION.md +++ b/CONFIGURATION.md @@ -72,6 +72,7 @@ This server can be configured using the `workspace/didChangeConfiguration` metho | `pylsp.plugins.rope_autoimport.memory` | `boolean` | Make the autoimport database memory only. Drastically increases startup time. | `false` | | `pylsp.plugins.rope_completion.enabled` | `boolean` | Enable or disable the plugin. | `false` | | `pylsp.plugins.rope_completion.eager` | `boolean` | Resolve documentation and detail eagerly. | `false` | +| `pylsp.plugins.rope_implementation.enabled` | `boolean` | Enable or disable the plugin. | `true` | | `pylsp.plugins.yapf.enabled` | `boolean` | Enable or disable the plugin. | `true` | | `pylsp.rope.extensionModules` | `string` | Builtin and c-extension modules that are allowed to be imported and inspected by rope. | `null` | | `pylsp.rope.ropeFolder` | `array` of unique `string` items | The name of the folder in which rope stores project configurations and data. Pass `null` for not using such a folder at all. | `null` | diff --git a/README.md b/README.md index 4cf305cc..ab5df91d 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ pip install python-lsp-server This will expose the command `pylsp` on your PATH. Confirm that installation succeeded by running `pylsp --help`. If the respective dependencies are found, the following optional providers will be enabled: -- [Rope](https://github.com/python-rope/rope) for Completions and renaming +- [Rope](https://github.com/python-rope/rope) for Completions, Goto Implementation, and renaming - [Pyflakes](https://github.com/PyCQA/pyflakes) linter to detect various errors - [McCabe](https://github.com/PyCQA/mccabe) linter for complexity checking - [pycodestyle](https://github.com/PyCQA/pycodestyle) linter for style checking @@ -151,7 +151,7 @@ pip install 'python-lsp-server[websockets]' * Code Linting * Code actions * Signature Help -* Go to definition +* Go to definition or implementation * Hover * Find References * Document Symbols diff --git a/pylsp/config/schema.json b/pylsp/config/schema.json index 18248384..b6204dc2 100644 --- a/pylsp/config/schema.json +++ b/pylsp/config/schema.json @@ -487,6 +487,11 @@ "default": false, "description": "Resolve documentation and detail eagerly." }, + "pylsp.plugins.rope_implementation.enabled": { + "type": "boolean", + "default": true, + "description": "Enable or disable the plugin." + }, "pylsp.plugins.yapf.enabled": { "type": "boolean", "default": true, diff --git a/pylsp/hookspecs.py b/pylsp/hookspecs.py index 41508be1..bf315211 100644 --- a/pylsp/hookspecs.py +++ b/pylsp/hookspecs.py @@ -93,6 +93,11 @@ def pylsp_hover(config, workspace, document, position) -> None: pass +@hookspec +def pylsp_implementations(config, workspace, document, position) -> None: + pass + + @hookspec def pylsp_initialize(config, workspace) -> None: pass diff --git a/pylsp/plugins/rope_implementation.py b/pylsp/plugins/rope_implementation.py new file mode 100644 index 00000000..221bb092 --- /dev/null +++ b/pylsp/plugins/rope_implementation.py @@ -0,0 +1,43 @@ +# Copyright 2017-2020 Palantir Technologies, Inc. +# Copyright 2021- Python Language Server Contributors. +import logging +import os + +from rope.contrib.findit import find_implementations + +from pylsp import hookimpl, uris + +log = logging.getLogger(__name__) + + +@hookimpl +def pylsp_settings(): + # Default to enabled (no reason not to) + return {"plugins": {"rope_implementation": {"enabled": True}}} + + +@hookimpl +def pylsp_implementations(config, workspace, document, position): + offset = document.offset_at_position(position) + rope_config = config.settings(document_path=document.path).get("rope", {}) + rope_project = workspace._rope_project_builder(rope_config) + rope_resource = document._rope_resource(rope_config) + + impls = find_implementations(rope_project, rope_resource, offset) + + return [ + { + "uri": uris.uri_with( + document.uri, + path=os.path.join(workspace.root_path, impl.resource.path), + ), + "range": { + # TODO: `impl.region` seems to be from the start of the file + # and offsets from the start of the line difficult to obtain, + # so we just return the whole line for now: + "start": {"line": impl.lineno - 1, "character": 0}, + "end": {"line": impl.lineno - 1, "character": 999}, + }, + } + for impl in impls + ] diff --git a/pylsp/python_lsp.py b/pylsp/python_lsp.py index ba41d6aa..1d7832c8 100644 --- a/pylsp/python_lsp.py +++ b/pylsp/python_lsp.py @@ -280,6 +280,7 @@ def capabilities(self): "commands": flatten(self._hook("pylsp_commands")) }, "hoverProvider": True, + "implementationProvider": True, # only when Rope is installed "referencesProvider": True, "renameProvider": True, "foldingRangeProvider": True, @@ -436,6 +437,9 @@ def highlight(self, doc_uri, position): def hover(self, doc_uri, position): return self._hook("pylsp_hover", doc_uri, position=position) or {"contents": ""} + def implementations(self, doc_uri, position): + return flatten(self._hook("pylsp_implementations", doc_uri, position=position)) + @_utils.debounce(LINT_DEBOUNCE_S, keyed_by="doc_uri") def lint(self, doc_uri, is_saved) -> None: # Since we're debounced, the document may no longer be open @@ -762,6 +766,12 @@ def m_text_document__definition(self, textDocument=None, position=None, **_kwarg return self._cell_document__definition(document, position, **_kwargs) return self.definitions(textDocument["uri"], position) + def m_text_document__implementation(self, textDocument=None, position=None, **_kwargs): + # textDocument here is just a dict with a uri + workspace = self._match_uri_to_workspace(textDocument["uri"]) + document = workspace.get_document(textDocument["uri"]) + return self.implementations(textDocument["uri"], position) + def m_text_document__document_highlight( self, textDocument=None, position=None, **_kwargs ): diff --git a/pyproject.toml b/pyproject.toml index f9c6a521..e8f344ff 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -80,6 +80,7 @@ pyflakes = "pylsp.plugins.pyflakes_lint" pylint = "pylsp.plugins.pylint_lint" rope_completion = "pylsp.plugins.rope_completion" rope_autoimport = "pylsp.plugins.rope_autoimport" +rope_implementation = "pylsp.plugins.rope_implementation" yapf = "pylsp.plugins.yapf_format" [project.scripts] From b7fb666a45f29a1cf6fc8eb5f3edb841ef26549b Mon Sep 17 00:00:00 2001 From: Shahriar Heidrich Date: Sun, 11 May 2025 21:27:20 +0200 Subject: [PATCH 2/7] Add test_implementation based on test_definition These don't work because Rope's find_implementations doesn't take a `source` argument (unlike e.g. `code_assist`) and hence *always* loads the file from disk, which means we can't use a "fake" module unless we want to use `unittest.mock.patch` to override some of this behavior, which is hacky & bad. --- test/plugins/test_implementations.py | 142 +++++++++++++++++++++++++++ 1 file changed, 142 insertions(+) create mode 100644 test/plugins/test_implementations.py diff --git a/test/plugins/test_implementations.py b/test/plugins/test_implementations.py new file mode 100644 index 00000000..66c7009f --- /dev/null +++ b/test/plugins/test_implementations.py @@ -0,0 +1,142 @@ +# Copyright 2017-2020 Palantir Technologies, Inc. +# Copyright 2021- Python Language Server Contributors. + +import os + +import pytest + +from pylsp import uris +from pylsp.plugins.rope_implementation import pylsp_implementations +from pylsp.workspace import Document + +DOC_URI = uris.from_fs_path(__file__) +DOC = """ +from abc import ABC, abstractmethod + +class Animal(ABC): + @abstractmethod + def breathe(self): + ... + + @property + @abstractmethod + def size(self) -> str: + ... + +class WingedAnimal(Animal): + @abstractmethod + def fly(self, destination): + ... + +class Bird(WingedAnimal): + def breathe(self): + print("*inhales like a bird*") + + def fly(self, destination): + print("*flies like a bird*") + + @property + def size(self) -> str: + return "bird-sized" + +print("not a method at all") +""" + + +def test_implementations(config, workspace) -> None: + # Over 'fly' in WingedAnimal.fly + cursor_pos = {"line": 15, "character": 9} + + # The implementation of 'Bird.fly' + def_range = { + "start": {"line": 22, "character": 0}, + "end": {"line": 22, "character": 999}, + } + + workspace.put_document(DOC_URI, DOC) + doc = workspace.get_document(DOC_URI) + assert [{"uri": DOC_URI, "range": def_range}] == pylsp_implementations( + config, workspace, doc, cursor_pos + ) + + +def test_implementations_skipping_one_class(config, workspace) -> None: + # Over 'Animal.breathe' + cursor_pos = {"line": 15, "character": 9} + + # The implementation of 'breathe', skipping intermediate classes + def_range = { + "start": {"line": 19, "character": 0}, + "end": {"line": 19, "character": 999}, + } + + doc = Document(DOC_URI, workspace, DOC) + assert [{"uri": DOC_URI, "range": def_range}] == pylsp_implementations( + config, workspace, doc, cursor_pos + ) + + +@pytest.mark.xfail(reason="not implemented upstream (Rope)", strict=True) +def test_property_implementations(config, workspace) -> None: + # Over 'Animal.size' + cursor_pos = {"line": 10, "character": 9} + + # The property implementation 'Bird.size' + def_range = { + "start": {"line": 26, "character": 0}, + "end": {"line": 26, "character": 999}, + } + + doc = Document(DOC_URI, workspace, DOC) + assert [{"uri": DOC_URI, "range": def_range}] == pylsp_implementations( + config, workspace, doc, cursor_pos + ) + + +def test_no_implementations_if_not_a_method(config, workspace) -> None: + # Over 'print(...)' call + cursor_pos = {"line": 26, "character": 0} + + doc = Document(DOC_URI, workspace, DOC) + assert [] == pylsp_implementations(config, workspace, doc, cursor_pos) + + +def test_document_path_implementations( + config, workspace, workspace_other_root_path, tmpdir +) -> None: + # Create a dummy module out of the workspace's root_path and try to get + # a implementation on it in another file placed next to it. + module_content = """ +class A: + def f(): +""" + + p = tmpdir.join("mymodule.py") + p.write(module_content) + + # Content of doc to test implementation + doc_content = """ +from mymodule import A +class B(A): + def f(): +""" + doc_path = str(tmpdir) + os.path.sep + "myfile.py" + doc_uri = uris.from_fs_path(doc_path) + doc = Document(doc_uri, workspace_other_root_path, doc_content) + + # The range where f is defined in mymodule.py + def_range = { + "start": {"line": 2, "character": 0}, + "end": {"line": 2, "character": 999}, + } + + # The position where foo is called in myfile.py + cursor_pos = {"line": 3, "character": 9} + + # The uri for mymodule.py + module_path = str(p) + module_uri = uris.from_fs_path(module_path) + + assert [{"uri": module_uri, "range": def_range}] == pylsp_implementations( + config, workspace, doc, cursor_pos + ) From 41a8852f1b644f209abd5ae07b963d844eaf9ec6 Mon Sep 17 00:00:00 2001 From: Shahriar Heidrich Date: Sun, 11 May 2025 22:10:08 +0200 Subject: [PATCH 3/7] Fix test_implementations by using real on-FS file --- test/data/implementations_examples/example.py | 29 ++++ test/plugins/test_implementations.py | 150 +++++++----------- 2 files changed, 85 insertions(+), 94 deletions(-) create mode 100644 test/data/implementations_examples/example.py diff --git a/test/data/implementations_examples/example.py b/test/data/implementations_examples/example.py new file mode 100644 index 00000000..ac724e73 --- /dev/null +++ b/test/data/implementations_examples/example.py @@ -0,0 +1,29 @@ +from abc import ABC, abstractmethod + +class Animal(ABC): + @abstractmethod + def breathe(self): + pass + + @property + @abstractmethod + def size(self) -> str: + pass + +class WingedAnimal(Animal): + @abstractmethod + def fly(self, destination): + pass + +class Bird(WingedAnimal): + def breathe(self): + print("*inhales like a bird*") + + def fly(self, destination): + print("*flies like a bird*") + + @property + def size(self) -> str: + return "bird-sized" + +print("not a method at all") diff --git a/test/plugins/test_implementations.py b/test/plugins/test_implementations.py index 66c7009f..47f3de7d 100644 --- a/test/plugins/test_implementations.py +++ b/test/plugins/test_implementations.py @@ -1,142 +1,104 @@ # Copyright 2017-2020 Palantir Technologies, Inc. # Copyright 2021- Python Language Server Contributors. -import os +from collections.abc import Iterable +from importlib.resources import as_file, files +from pathlib import Path import pytest +from rope.base.exceptions import BadIdentifierError from pylsp import uris +from pylsp.config.config import Config from pylsp.plugins.rope_implementation import pylsp_implementations -from pylsp.workspace import Document +from pylsp.workspace import Workspace -DOC_URI = uris.from_fs_path(__file__) -DOC = """ -from abc import ABC, abstractmethod -class Animal(ABC): - @abstractmethod - def breathe(self): - ... +# We use a real file because the part of Rope that this feature uses +# (`rope.findit.find_implementations`) *always* loads files from the +# filesystem, in contrast to e.g. `code_assist` which takes a `source` argument +# that can be more easily faked. +# An alternative to using real files would be `unittest.mock.patch`, but that +# ends up being more trouble than it's worth... +@pytest.fixture +def examples_dir_path() -> Iterable[Path]: + with as_file(files("test.data.implementations_examples")) as path: + yield path - @property - @abstractmethod - def size(self) -> str: - ... -class WingedAnimal(Animal): - @abstractmethod - def fly(self, destination): - ... +@pytest.fixture +def doc_uri(examples_dir_path: Path) -> str: + return uris.from_fs_path(str(examples_dir_path / "example.py")) -class Bird(WingedAnimal): - def breathe(self): - print("*inhales like a bird*") - def fly(self, destination): - print("*flies like a bird*") +# Similarly to the above, we need our workspace to point to the actual location +# on the filesystem containing the example modules, so we override the fixture: +@pytest.fixture +def workspace(examples_dir_path: Path, endpoint) -> None: + ws = Workspace(uris.from_fs_path(str(examples_dir_path)), endpoint) + ws._config = Config(ws.root_uri, {}, 0, {}) + yield ws + ws.close() - @property - def size(self) -> str: - return "bird-sized" -print("not a method at all") -""" - - -def test_implementations(config, workspace) -> None: +def test_implementations(config, workspace, doc_uri) -> None: # Over 'fly' in WingedAnimal.fly - cursor_pos = {"line": 15, "character": 9} + cursor_pos = {"line": 14, "character": 8} # The implementation of 'Bird.fly' def_range = { - "start": {"line": 22, "character": 0}, - "end": {"line": 22, "character": 999}, + "start": {"line": 21, "character": 0}, + "end": {"line": 21, "character": 999}, } - workspace.put_document(DOC_URI, DOC) - doc = workspace.get_document(DOC_URI) - assert [{"uri": DOC_URI, "range": def_range}] == pylsp_implementations( + doc = workspace.get_document(doc_uri) + assert [{"uri": doc_uri, "range": def_range}] == pylsp_implementations( config, workspace, doc, cursor_pos ) -def test_implementations_skipping_one_class(config, workspace) -> None: +def test_implementations_skipping_one_class(config, workspace, doc_uri) -> None: # Over 'Animal.breathe' - cursor_pos = {"line": 15, "character": 9} + cursor_pos = {"line": 4, "character": 8} # The implementation of 'breathe', skipping intermediate classes def_range = { - "start": {"line": 19, "character": 0}, - "end": {"line": 19, "character": 999}, + "start": {"line": 18, "character": 0}, + "end": {"line": 18, "character": 999}, } - doc = Document(DOC_URI, workspace, DOC) - assert [{"uri": DOC_URI, "range": def_range}] == pylsp_implementations( + doc = workspace.get_document(doc_uri) + assert [{"uri": doc_uri, "range": def_range}] == pylsp_implementations( config, workspace, doc, cursor_pos ) -@pytest.mark.xfail(reason="not implemented upstream (Rope)", strict=True) -def test_property_implementations(config, workspace) -> None: +@pytest.mark.xfail( + reason="not implemented upstream (Rope)", strict=True, raises=BadIdentifierError +) +def test_property_implementations(config, workspace, doc_uri) -> None: # Over 'Animal.size' - cursor_pos = {"line": 10, "character": 9} + cursor_pos = {"line": 9, "character": 9} # The property implementation 'Bird.size' def_range = { - "start": {"line": 26, "character": 0}, - "end": {"line": 26, "character": 999}, + "start": {"line": 25, "character": 0}, + "end": {"line": 25, "character": 999}, } - doc = Document(DOC_URI, workspace, DOC) - assert [{"uri": DOC_URI, "range": def_range}] == pylsp_implementations( + doc = workspace.get_document(doc_uri) + assert [{"uri": doc_uri, "range": def_range}] == pylsp_implementations( config, workspace, doc, cursor_pos ) -def test_no_implementations_if_not_a_method(config, workspace) -> None: - # Over 'print(...)' call - cursor_pos = {"line": 26, "character": 0} - - doc = Document(DOC_URI, workspace, DOC) - assert [] == pylsp_implementations(config, workspace, doc, cursor_pos) - +def test_implementations_not_a_method(config, workspace, doc_uri) -> None: + # Over 'print(...)' call => Rope error because not a method. + cursor_pos = {"line": 28, "character": 0} -def test_document_path_implementations( - config, workspace, workspace_other_root_path, tmpdir -) -> None: - # Create a dummy module out of the workspace's root_path and try to get - # a implementation on it in another file placed next to it. - module_content = """ -class A: - def f(): -""" + doc = workspace.get_document(doc_uri) - p = tmpdir.join("mymodule.py") - p.write(module_content) - - # Content of doc to test implementation - doc_content = """ -from mymodule import A -class B(A): - def f(): -""" - doc_path = str(tmpdir) + os.path.sep + "myfile.py" - doc_uri = uris.from_fs_path(doc_path) - doc = Document(doc_uri, workspace_other_root_path, doc_content) - - # The range where f is defined in mymodule.py - def_range = { - "start": {"line": 2, "character": 0}, - "end": {"line": 2, "character": 999}, - } - - # The position where foo is called in myfile.py - cursor_pos = {"line": 3, "character": 9} - - # The uri for mymodule.py - module_path = str(p) - module_uri = uris.from_fs_path(module_path) - - assert [{"uri": module_uri, "range": def_range}] == pylsp_implementations( - config, workspace, doc, cursor_pos - ) + # This exception is turned into an empty result set automatically in upper + # layers, so we just check that it is raised to document this behavior: + with pytest.raises(BadIdentifierError): + pylsp_implementations(config, workspace, doc, cursor_pos) From 9c715d1475857e143ee6c8c37d73ccd1d08fb852 Mon Sep 17 00:00:00 2001 From: Shahriar Heidrich Date: Sun, 11 May 2025 23:27:16 +0200 Subject: [PATCH 4/7] Determine columns for found implementations --- pylsp/plugins/rope_implementation.py | 43 ++++++++++++++++++++++------ test/plugins/test_implementations.py | 12 ++++---- 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/pylsp/plugins/rope_implementation.py b/pylsp/plugins/rope_implementation.py index 221bb092..038a3108 100644 --- a/pylsp/plugins/rope_implementation.py +++ b/pylsp/plugins/rope_implementation.py @@ -2,8 +2,11 @@ # Copyright 2021- Python Language Server Contributors. import logging import os +from typing import Any -from rope.contrib.findit import find_implementations +from rope.base.project import Project +from rope.base.resources import Resource +from rope.contrib.findit import Location, find_implementations from pylsp import hookimpl, uris @@ -31,13 +34,37 @@ def pylsp_implementations(config, workspace, document, position): document.uri, path=os.path.join(workspace.root_path, impl.resource.path), ), - "range": { - # TODO: `impl.region` seems to be from the start of the file - # and offsets from the start of the line difficult to obtain, - # so we just return the whole line for now: - "start": {"line": impl.lineno - 1, "character": 0}, - "end": {"line": impl.lineno - 1, "character": 999}, - }, + "range": _rope_location_to_range(impl, rope_project), } for impl in impls ] + + +def _rope_location_to_range( + location: Location, rope_project: Project +) -> dict[str, Any]: + # NOTE: This assumes the result is confined to a single line, which should + # always be the case here because Python doesn't allow splitting up + # identifiers across more than one line. + start_column, end_column = _rope_region_to_columns( + location.region, location.lineno, location.resource, rope_project + ) + return { + "start": {"line": location.lineno - 1, "character": start_column}, + "end": {"line": location.lineno - 1, "character": end_column}, + } + + +def _rope_region_to_columns( + offsets: tuple[int, int], line: int, rope_resource: Resource, rope_project: Project +) -> tuple[int, int]: + """ + Convert pair of offsets from start of file to columns within line. + + Assumes both offsets reside within the same line and will return nonsense + for the end offset if this isn't the case. + """ + line_start_offset = rope_project.get_pymodule(rope_resource).lines.get_line_start( + line + ) + return offsets[0] - line_start_offset, offsets[1] - line_start_offset diff --git a/test/plugins/test_implementations.py b/test/plugins/test_implementations.py index 47f3de7d..7c139fac 100644 --- a/test/plugins/test_implementations.py +++ b/test/plugins/test_implementations.py @@ -47,8 +47,8 @@ def test_implementations(config, workspace, doc_uri) -> None: # The implementation of 'Bird.fly' def_range = { - "start": {"line": 21, "character": 0}, - "end": {"line": 21, "character": 999}, + "start": {"line": 21, "character": 8}, + "end": {"line": 21, "character": 11}, } doc = workspace.get_document(doc_uri) @@ -63,8 +63,8 @@ def test_implementations_skipping_one_class(config, workspace, doc_uri) -> None: # The implementation of 'breathe', skipping intermediate classes def_range = { - "start": {"line": 18, "character": 0}, - "end": {"line": 18, "character": 999}, + "start": {"line": 18, "character": 8}, + "end": {"line": 18, "character": 15}, } doc = workspace.get_document(doc_uri) @@ -82,8 +82,8 @@ def test_property_implementations(config, workspace, doc_uri) -> None: # The property implementation 'Bird.size' def_range = { - "start": {"line": 25, "character": 0}, - "end": {"line": 25, "character": 999}, + "start": {"line": 25, "character": 8}, + "end": {"line": 25, "character": 12}, } doc = workspace.get_document(doc_uri) From c1437e859b0099d2619629c0064e06e1216392d0 Mon Sep 17 00:00:00 2001 From: Shahriar Heidrich Date: Mon, 12 May 2025 12:42:03 +0200 Subject: [PATCH 5/7] Add Python 3.8 support for rope_implementations --- pylsp/plugins/rope_implementation.py | 8 ++++---- test/plugins/test_implementations.py | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pylsp/plugins/rope_implementation.py b/pylsp/plugins/rope_implementation.py index 038a3108..0c8e0855 100644 --- a/pylsp/plugins/rope_implementation.py +++ b/pylsp/plugins/rope_implementation.py @@ -2,7 +2,7 @@ # Copyright 2021- Python Language Server Contributors. import logging import os -from typing import Any +from typing import Any, Dict, Tuple from rope.base.project import Project from rope.base.resources import Resource @@ -42,7 +42,7 @@ def pylsp_implementations(config, workspace, document, position): def _rope_location_to_range( location: Location, rope_project: Project -) -> dict[str, Any]: +) -> Dict[str, Any]: # NOTE: This assumes the result is confined to a single line, which should # always be the case here because Python doesn't allow splitting up # identifiers across more than one line. @@ -56,8 +56,8 @@ def _rope_location_to_range( def _rope_region_to_columns( - offsets: tuple[int, int], line: int, rope_resource: Resource, rope_project: Project -) -> tuple[int, int]: + offsets: Tuple[int, int], line: int, rope_resource: Resource, rope_project: Project +) -> Tuple[int, int]: """ Convert pair of offsets from start of file to columns within line. diff --git a/test/plugins/test_implementations.py b/test/plugins/test_implementations.py index 7c139fac..cec70f30 100644 --- a/test/plugins/test_implementations.py +++ b/test/plugins/test_implementations.py @@ -1,8 +1,7 @@ # Copyright 2017-2020 Palantir Technologies, Inc. # Copyright 2021- Python Language Server Contributors. -from collections.abc import Iterable -from importlib.resources import as_file, files +import test from pathlib import Path import pytest @@ -21,9 +20,10 @@ # An alternative to using real files would be `unittest.mock.patch`, but that # ends up being more trouble than it's worth... @pytest.fixture -def examples_dir_path() -> Iterable[Path]: - with as_file(files("test.data.implementations_examples")) as path: - yield path +def examples_dir_path() -> Path: + # In Python 3.12+, this should be obtained using `importlib.resources`, + # but as we need to support older versions, we do it the hacky way: + return Path(test.__file__).parent / "data/implementations_examples" @pytest.fixture From f38d3eff7229ae1e56fb54e68ba41f116075b26e Mon Sep 17 00:00:00 2001 From: Shahriar Heidrich Date: Mon, 12 May 2025 12:47:40 +0200 Subject: [PATCH 6/7] Handle Rope find_implementations errors explicitly --- pylsp/plugins/rope_implementation.py | 6 +++++- test/plugins/test_implementations.py | 12 +++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/pylsp/plugins/rope_implementation.py b/pylsp/plugins/rope_implementation.py index 0c8e0855..c89185a3 100644 --- a/pylsp/plugins/rope_implementation.py +++ b/pylsp/plugins/rope_implementation.py @@ -26,7 +26,11 @@ def pylsp_implementations(config, workspace, document, position): rope_project = workspace._rope_project_builder(rope_config) rope_resource = document._rope_resource(rope_config) - impls = find_implementations(rope_project, rope_resource, offset) + try: + impls = find_implementations(rope_project, rope_resource, offset) + except Exception as e: + log.debug("Failed to run Rope implementations finder: %s", e) + return [] return [ { diff --git a/test/plugins/test_implementations.py b/test/plugins/test_implementations.py index cec70f30..5e9d8a56 100644 --- a/test/plugins/test_implementations.py +++ b/test/plugins/test_implementations.py @@ -5,7 +5,6 @@ from pathlib import Path import pytest -from rope.base.exceptions import BadIdentifierError from pylsp import uris from pylsp.config.config import Config @@ -74,7 +73,7 @@ def test_implementations_skipping_one_class(config, workspace, doc_uri) -> None: @pytest.mark.xfail( - reason="not implemented upstream (Rope)", strict=True, raises=BadIdentifierError + reason="not implemented upstream (Rope)", strict=True, raises=AssertionError ) def test_property_implementations(config, workspace, doc_uri) -> None: # Over 'Animal.size' @@ -93,12 +92,11 @@ def test_property_implementations(config, workspace, doc_uri) -> None: def test_implementations_not_a_method(config, workspace, doc_uri) -> None: - # Over 'print(...)' call => Rope error because not a method. + # Over 'print(...)' call cursor_pos = {"line": 28, "character": 0} doc = workspace.get_document(doc_uri) - # This exception is turned into an empty result set automatically in upper - # layers, so we just check that it is raised to document this behavior: - with pytest.raises(BadIdentifierError): - pylsp_implementations(config, workspace, doc, cursor_pos) + # Rope produces an error because we're not over a method, which we then + # turn into an empty result list: + assert [] == pylsp_implementations(config, workspace, doc, cursor_pos) From b460f312e97de2dfd0f5c6c58559730ca94524c1 Mon Sep 17 00:00:00 2001 From: Shahriar Heidrich Date: Mon, 12 May 2025 12:53:28 +0200 Subject: [PATCH 7/7] Fix linting issues in rope_implementation & tests --- pylsp/python_lsp.py | 2 -- test/data/implementations_examples/example.py | 1 + test/plugins/test_implementations.py | 22 +++++++++---------- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/pylsp/python_lsp.py b/pylsp/python_lsp.py index 1d7832c8..7f97c414 100644 --- a/pylsp/python_lsp.py +++ b/pylsp/python_lsp.py @@ -768,8 +768,6 @@ def m_text_document__definition(self, textDocument=None, position=None, **_kwarg def m_text_document__implementation(self, textDocument=None, position=None, **_kwargs): # textDocument here is just a dict with a uri - workspace = self._match_uri_to_workspace(textDocument["uri"]) - document = workspace.get_document(textDocument["uri"]) return self.implementations(textDocument["uri"], position) def m_text_document__document_highlight( diff --git a/test/data/implementations_examples/example.py b/test/data/implementations_examples/example.py index ac724e73..3057a4cd 100644 --- a/test/data/implementations_examples/example.py +++ b/test/data/implementations_examples/example.py @@ -1,5 +1,6 @@ from abc import ABC, abstractmethod + class Animal(ABC): @abstractmethod def breathe(self): diff --git a/test/plugins/test_implementations.py b/test/plugins/test_implementations.py index 5e9d8a56..9b22c8fe 100644 --- a/test/plugins/test_implementations.py +++ b/test/plugins/test_implementations.py @@ -1,11 +1,11 @@ # Copyright 2017-2020 Palantir Technologies, Inc. # Copyright 2021- Python Language Server Contributors. -import test from pathlib import Path import pytest +import test from pylsp import uris from pylsp.config.config import Config from pylsp.plugins.rope_implementation import pylsp_implementations @@ -42,12 +42,12 @@ def workspace(examples_dir_path: Path, endpoint) -> None: def test_implementations(config, workspace, doc_uri) -> None: # Over 'fly' in WingedAnimal.fly - cursor_pos = {"line": 14, "character": 8} + cursor_pos = {"line": 15, "character": 8} # The implementation of 'Bird.fly' def_range = { - "start": {"line": 21, "character": 8}, - "end": {"line": 21, "character": 11}, + "start": {"line": 22, "character": 8}, + "end": {"line": 22, "character": 11}, } doc = workspace.get_document(doc_uri) @@ -58,12 +58,12 @@ def test_implementations(config, workspace, doc_uri) -> None: def test_implementations_skipping_one_class(config, workspace, doc_uri) -> None: # Over 'Animal.breathe' - cursor_pos = {"line": 4, "character": 8} + cursor_pos = {"line": 5, "character": 8} # The implementation of 'breathe', skipping intermediate classes def_range = { - "start": {"line": 18, "character": 8}, - "end": {"line": 18, "character": 15}, + "start": {"line": 19, "character": 8}, + "end": {"line": 19, "character": 15}, } doc = workspace.get_document(doc_uri) @@ -77,12 +77,12 @@ def test_implementations_skipping_one_class(config, workspace, doc_uri) -> None: ) def test_property_implementations(config, workspace, doc_uri) -> None: # Over 'Animal.size' - cursor_pos = {"line": 9, "character": 9} + cursor_pos = {"line": 10, "character": 9} # The property implementation 'Bird.size' def_range = { - "start": {"line": 25, "character": 8}, - "end": {"line": 25, "character": 12}, + "start": {"line": 26, "character": 8}, + "end": {"line": 26, "character": 12}, } doc = workspace.get_document(doc_uri) @@ -93,7 +93,7 @@ def test_property_implementations(config, workspace, doc_uri) -> None: def test_implementations_not_a_method(config, workspace, doc_uri) -> None: # Over 'print(...)' call - cursor_pos = {"line": 28, "character": 0} + cursor_pos = {"line": 29, "character": 0} doc = workspace.get_document(doc_uri)