Skip to content
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

Go to method definition in Python modules fails with "No definition found for '<method_name>'" message #1402

Closed
karrtikr opened this issue Sep 16, 2019 · 7 comments
Labels
database-index Needs a database index/Rewrite in Rust (#1059) feature

Comments

@karrtikr
Copy link

Possible enhancement

@marius-nicolae commented on Sat Sep 14 2019

Hi,

First of all, thank you, for your wonderful work. I like VS Code a lot but there is an issue I'd like to report.

Usually, in Python modules, there are classes definitions with methods references but without calls with actual types. In this case, when trying to go to a method definition the VS Code displays a "No definition found for '<method_name>'" message as opposed to Pycharm which displays a menu with all possible methods definitions. Even if the Python file contains calls with actual objects the VS Code will go the first definition even if there are multiple possibilities.

Environment data

Version: 1.38.1
Commit: b37e54c98e1a74ba89e03073e5a3761284e3ffb0
Date: 2019-09-11T13:30:08.229Z
Electron: 4.2.10
Chrome: 69.0.3497.128
Node.js: 10.11.0
V8: 6.9.427.31-electron.0
OS: Linux x64 4.15.0-58-generic

  • Extension version (available under the Extensions sidebar): ms-python.python-2019.9.34911
  • OS and version: Xubuntu 18.04
  • Python version (& distribution if applicable, e.g. Anaconda): 3.6.8 64 bits
  • Type of virtual environment used (N/A | venv | virtualenv | conda | ...): N/A
  • Relevant/affected Python packages and their versions: XXX
  • Jedi or Language Server? (i.e. what is "python.jediEnabled" set to; more info #3977): Jedi enabled

Expected behaviour

VS Code should display a menu with all possible method definitions, with the ones defined in the current .py file on top

Actual behaviour

Displays a "No definition found for '<method_name>'" message in case no calls with actual objects are found in the current file or goes to the first implementation otherwise

Steps to reproduce:

  1. Unpack the attached archive - test.tar.gz
  2. Open the project folder
  3. Open to the Test/test.py file
  4. F12 on the Foo method reference from line 10

More details, on the attached animated gif.
Go to method definition-2019-09-14 11-13

@davidhalter
Copy link
Owner

Hey @karrtikr Thanks for the detailed report. I quickly copied the files here, because I don't have VScode and it's easier to reason about them here:

app.py

from Test import test
                     
a = test.A()         
b = test.B()         
                     
test.CallFoo(a)      
test.CallFoo(b)      

Test/test.py

class A:
    def Foo(self):
        print("A.Foo method")

class B:
    def Foo(self):
        print("B.Foo method")

def CallFoo(obj):
    obj.Foo()

# a = A()
# b = B()

# CallFoo(a)
# CallFoo(b)

@davidhalter
Copy link
Owner

Thanks for the detailed report. I assume PyCharm only works when

test.CallFoo(a)      
test.CallFoo(b)

is used in app.py. Otherwise it wouldn't know enough, right?

Nested folder structures are a bit of an issue in Jedi sometimes. I'm definitely keeping your use case in mind when I'm tackling #1059.

So don't expect a quick fix, it might take a while (a few months to years), but I'm trying to work on it.

@karrtikr
Copy link
Author

@marius-nicolae originally reported that issue, but from the screenshot it is evident that no matter if test.CallFoo(a) or test.CallFoo(b) is called in the file, PyCharm always points out both the definitions.

In our case we only point to the definitions if function is called (which is probably better). But I think the enhancement is in case when we have multiple function calls, we should have a pop up of both definitions instead of going to the first one.

@marius-nicolae You can correct me if that's not what you asked for.

@davidhalter
Copy link
Owner

If the full code is provided (without comments), Jedi lists both definition. I feel like this was VScode's decision to jump to the first one. For me with jedi-vim, it shows two definitions. However this only happens if you uncomment all the code.

@karrtikr
Copy link
Author

I see, maybe Go to definition by design only points to one implementation.

@marius-nicolae
Copy link

marius-nicolae commented Sep 20, 2019

The idea is that often a developer will have to browse a lot of Python code, including modules, where there are many classes methods and functions exposed without calls with actual objects or calls which covers only partially all the possibilities (i.e. in our example, only a test.CallFoo(a) or test.CallFoo(b) would be present). There will be a negative user experience if go to the method definition fails but it's found through grep (i.e. find in files).

I would say that, in case the object type can't be determined at a particular point (no method call with an actual object, no types hinting, etc), simply display all the method definitions from all classes available at that point, as currently PyCharm appears to do. I find it much more powerful this way.

I also do agree that the information exposed shouldn't be too verbose (all the time) and for cases like:

a = A()
a.Foo()

or when there were type hints ( in our case the function would have been declared as def CallFoo(obj: A):) the go to method definition would go stright to the class method implementation (i.e. in A class, in our example).

@davidhalter davidhalter added the database-index Needs a database index/Rewrite in Rust (#1059) label Dec 22, 2019
@davidhalter
Copy link
Owner

This example is working now. See also my comment in #1047 (comment).

This only works under some circumstances.

  1. The name must be somewhat unique (i.e. not appear in lots of files).
  2. Bigger projects might generally lead to some names not being found. It should work in small projects pretty much consistently, because Jedi searches directory structures now (and not only the current directory).

So if you want this to work 99% of the time and not just 90%, subscribe to #1059.

@marius-nicolae I still consider random completions without type inference to be pretty bad. But I'm not generally against having an optional feature that people can use (I don't have to use it, but others can of course). If you want that, please open a new issue and describe how exactly you imagine it, with a few examples. That way it's easy to write tests that make your use case possible. I would also be very interested in a detailed description of how pycharm does it, if you like it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database-index Needs a database index/Rewrite in Rust (#1059) feature
Projects
None yet
Development

No branches or pull requests

3 participants