Skip to content

includeDeclaration is no longer respected in textDocument/references #439

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

Closed
krassowski opened this issue Sep 10, 2023 · 2 comments · Fixed by #440
Closed

includeDeclaration is no longer respected in textDocument/references #439

krassowski opened this issue Sep 10, 2023 · 2 comments · Fixed by #440
Labels
bug Something isn't working
Milestone

Comments

@krassowski
Copy link
Member

From manual debugging it seems that includeDeclaration gets handled correctly here:

def m_text_document__references(
self, textDocument=None, position=None, context=None, **_kwargs
):
exclude_declaration = not context["includeDeclaration"]
return self.references(textDocument["uri"], position, exclude_declaration)

and here:

def references(self, doc_uri, position, exclude_declaration):
return flatten(
self._hook(
"pylsp_references",
doc_uri,
position=position,
exclude_declaration=exclude_declaration,
)
)

but when it is passed to pylsp_references here:

@hookimpl
def pylsp_references(document, position, exclude_declaration=False):
code_position = _utils.position_to_jedi_linecolumn(document, position)
usages = document.jedi_script().get_references(**code_position)
if exclude_declaration:
# Filter out if the usage is the actual declaration of the thing
usages = [d for d in usages if not d.is_definition()]

the default value is always used. It looks like the pluggy weirdness with it failing to pass arguments correctly. Reproduced in 1.8.0 with pluggy 1.2.0 and 1.3.0 on Python 3.11.

Removing the default value (exclude_declaration=Falseexclude_declaration) fixes the issue.

@krassowski krassowski added the bug Something isn't working label Sep 10, 2023
@krassowski
Copy link
Member Author

Indeed this is what pluggy does:

import pluggy

hookspec = pluggy.HookspecMarker("test")
hookimpl = pluggy.HookimplMarker("test")

class MySpec:

    @hookspec
    def myhook(self, arg1, arg2):
        pass

class Plugin_1:

    @hookimpl
    def myhook(self, arg1, arg2=0):
        return arg1 + arg2

pm = pluggy.PluginManager("test")
pm.add_hookspecs(MySpec)
pm.register(Plugin_1())
results = pm.hook.myhook(arg1=1, arg2=2)
print(results)  # prints[1]

Compare with:

import pluggy

hookspec = pluggy.HookspecMarker("test")
hookimpl = pluggy.HookimplMarker("test")

class MySpec:

    @hookspec
    def myhook(self, arg1, arg2):
        pass

class Plugin_1:

    @hookimpl
    def myhook(self, arg1, arg2):
        return arg1 + arg2

pm = pluggy.PluginManager("test")
pm.add_hookspecs(MySpec)
pm.register(Plugin_1())
results = pm.hook.myhook(arg1=1, arg2=2)
print(results)  # prints[3]

@krassowski
Copy link
Member Author

I am unable to find a version of pluggy which worked differently (checked down to 0.x series). I am also unable to narrow down python-lsp-server version (checked down to 1.4.x) which worked correctly. I also see this behaviour on Python 3.8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants