Skip to content

Add basic Goto Implementation Request support #644

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.

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

Draft
wants to merge 7 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CONFIGURATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -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` |
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions pylsp/config/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions pylsp/hookspecs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
74 changes: 74 additions & 0 deletions pylsp/plugins/rope_implementation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# Copyright 2017-2020 Palantir Technologies, Inc.
# Copyright 2021- Python Language Server Contributors.
import logging
import os
from typing import Any, Dict, Tuple

from rope.base.project import Project
from rope.base.resources import Resource
from rope.contrib.findit import Location, find_implementations
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it would also make sense to add find_definition?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you mean using rope.contrib.findit.find_definition to implement the LSP textDocument/definition request, that one is already implemented in python-lsp-server using Jedi, so I don't think adding an additional Rope-powered implementation makes sense. But maybe I misunderstand it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's what I meant. My reasoning is twofold:

  • I think that jedi's implementation will return both implementation and definitions
  • I believe that users who would want to use rope for this would likely want to disable the jedi implementation if they could have a matching rope implementation for consistency

Of note, rope is disabled by default in general because it is hard to guarantee it works well and does not conflict with jedi (e.g. by de-duplciating responses).

For the best UX for an average user I think it would be best to add textDocument/implementation using jedi, but I would not object to adding one with rope as in this PR.

I was just curious as to your reasoning for adding only textDocument/implementation and not also textDocument/definition which would be my choice if I was adding a rope version for it.

Copy link
Author

@smheidrich smheidrich May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Comment deleted because it doesn't make sense, give me a minute while I write a new one)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally wrote a response disagreeing but there was a glaring mistake in it because I overlooked that the current Rope find_implementations implementation is much buggier than I thought: E.g. for a file like

class A:
  def f(self): ...

class B(A):
  def f(self): ...

a: A = A()
a.f()

using Rope's find_implementations on the a.f() call will go to B.f for some reason 😕

It works well for moving between overridden methods within a class hierarchy, where it nicely complements the Jedi textDocument/definition, which always moves "up", by moving "down" instead.

But my guess is that it would take quite a lot of effort to fix Rope's find_implementations so it works correctly when used on method calls as well 😔

I'll open an issue but this PR should be on hold until it's fixed, no point merging something that's fundamentally buggy from day 1.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


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)

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 [
{
"uri": uris.uri_with(
document.uri,
path=os.path.join(workspace.root_path, impl.resource.path),
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Under certain circumstances I seem to get URIs like ///example.py here for a file that is actually in another directory - will have to investigate & fix.

),
"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
8 changes: 8 additions & 0 deletions pylsp/python_lsp.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -762,6 +766,10 @@ 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
return self.implementations(textDocument["uri"], position)

def m_text_document__document_highlight(
self, textDocument=None, position=None, **_kwargs
):
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
30 changes: 30 additions & 0 deletions test/data/implementations_examples/example.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
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")
102 changes: 102 additions & 0 deletions test/plugins/test_implementations.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
# Copyright 2017-2020 Palantir Technologies, Inc.
# Copyright 2021- Python Language Server Contributors.

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
from pylsp.workspace import Workspace


# 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() -> 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
def doc_uri(examples_dir_path: Path) -> str:
return uris.from_fs_path(str(examples_dir_path / "example.py"))


# 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()


def test_implementations(config, workspace, doc_uri) -> None:
# Over 'fly' in WingedAnimal.fly
cursor_pos = {"line": 15, "character": 8}

# The implementation of 'Bird.fly'
def_range = {
"start": {"line": 22, "character": 8},
"end": {"line": 22, "character": 11},
}

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, doc_uri) -> None:
# Over 'Animal.breathe'
cursor_pos = {"line": 5, "character": 8}

# The implementation of 'breathe', skipping intermediate classes
def_range = {
"start": {"line": 19, "character": 8},
"end": {"line": 19, "character": 15},
}

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, raises=AssertionError
)
def test_property_implementations(config, workspace, doc_uri) -> None:
# Over 'Animal.size'
cursor_pos = {"line": 10, "character": 9}

# The property implementation 'Bird.size'
def_range = {
"start": {"line": 26, "character": 8},
"end": {"line": 26, "character": 12},
}

doc = workspace.get_document(doc_uri)
assert [{"uri": doc_uri, "range": def_range}] == pylsp_implementations(
config, workspace, doc, cursor_pos
)


def test_implementations_not_a_method(config, workspace, doc_uri) -> None:
# Over 'print(...)' call
cursor_pos = {"line": 29, "character": 0}

doc = workspace.get_document(doc_uri)

# 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)
Loading