elpy-goto-definition on decorated function will jump to definition of decorator

[TauPan]

Virtualenv........: redacted (/home/user/.virtualenvs/redacted)
RPC Python........: 2.6.6 (/home/user/.virtualenvs/redacted/bin/python)
Interactive Python: python (/home/user/.virtualenvs/redacted/bin/python)
Emacs.............: 24.3.1
Elpy..............: 1.6.0
Jedi..............: 0.8.1-final0
Rope..............: 0.9.4 (0.10.2 available)
Syntax checker....: flake8 (/home/user/.virtualenvs/redacted/bin/flake8)

The directory ~/.local/bin/ is not in your PATH, even though you do
not have an active virtualenv. Installing Python packages locally will
create executables in that directory, so Emacs won't find them. If you
are missing some commands, do add this directory to your PATH.

There is a newer version of Rope available.

[run] pip install --upgrade rope

Using jedi as rpc backend.

(tried upgrading rope after pasting this, didn't help, not surprised... ;) )

test.py

from contextlib import contextmanager

@contextmanager
def stuff():
    yield

with stuff():
    print "Bla"

elpy-goto-definition on the call to stuff() will jump to the 'helper' function in the definition of contextmanager.

Maybe (probably) this is a bug in jedi (again), but I don't know how to use jedi directly to check.

Kind regards (and happy New Year)! Friedel

[jorgenschaefer]

Happy new year! Sorry for the long delay, I was on vacation.

This would indeed be a misfeature in Jedi. Reproduction:

import jedi

source = '''\                                                                   
from contextlib import contextmanager                                           

@contextmanager                                                               
def stuff():                                                                    
    yield                                                                       

with stuff():                                                                   
    print "Bla"                                                                 
'''

script = jedi.Script(source=source, line=8, column=5)
(loc,) = script.goto_definitions()
print(loc.module_path, loc.line, loc.column)

[TauPan]

I've just tried to reproduce this at home on python 2.7.6, and elpy jumps to the correct line, also your script prints:

(None, 5, 0)

(elpy-config is identical, I have the same .emacs at home and use (use-package elpy :ensure t ...))

I'll try at work on monday again (since I can't be bothered to set up python 2.6 via pyenv here) but it looks like this is a bug only with python 2.6.

[jorgenschaefer]

Make sure to use the code in the comment above, not any mail you might have received, as I had a minor "bug" in the first post (I had commented out the decorator to make sure it actually works without that).

[TauPan]

Thanks, I didn't notice that:

Testcase prints:

('/usr/lib/python2.7/contextlib.py', 83, 4)

On python 2.7.6.

So it has nothing to do with the python version, I just overlooked that the decorator was commented out.

[jorgenschaefer]

Hm. goto_assignments from Jedi does the right thing in this case. This is a bit painful.

Jedi can provide different locations for definitions. The two methods give different results, but it's not clear to me how to pick the right one.

[jorgenschaefer]

I'll push this back to 1.8. I'm still rather confused when to use which of the two there. It's a non-trivial question. The "right" thing might be to display a selection box. I'm unsure about that, though. Half the time, it's pretty clear that goto_assignment does the wrong thing. Hrmph.

[TauPan]

Guessing which is intended doesn't seem smart.

What about a different binding altogether? In this case I know that I want to jump to a decorated function, so I would use the other binding. In other cases, goto_assignment might be useful too. In any case, a selection box should be useful iff there are multiple results (for definitions xor assignments).

[jorgenschaefer]

Hm. C-u M-. might work, actually.

[ChillarAnand]

Since behaviour of both functions is not consistent, Elpy should use both, get both functions and compare them with function name at cursor and move accordingly. With C-u M-. we can move to decorator/alternate function if available?

[jorgenschaefer]

The new xref code in Emacs can offer multiple possible locations. If that was more widely available (i.e. in a few years), we could use that.

Until then, C-u M-. probably should display a selection, while M-. probably should select the sanest candidate, yes.

[colonelpanic8]

Is anyone actively working on this? I'd be willing to make the changes.

[ChillarAnand]

@IvanMalison I am working on this. I am still looking for a better idea to solve this.

@jorgenschaefer Any workarounds to make elpy select the right candidate?

[jorgenschaefer]

@IvanMalison, if you have good ideas on how to solve this, you are very welcome to contribute them! :-)

@ChillarAnand, I have no idea what you are asking, I am afraid. The problem is that Jedi returns an unexpected result in this particular case, so the logic in Elpy's jedibackend needs to improve. I have no idea what kind of "workaround" you might have in mind for this.

A few notes on possible fixes for this:

• M-. should continue to jump to the most likely candidate. This means that M-. should not pop up a selection box unless there is something really weird and unusual going on. In particular, M-. should continue to not jump to assignments.
• C-u M-. should likely pop up a selection box of some kind with possible results. I would not mind an rgrep-like, non-modal window, even.

[ChillarAnand]

import jedi

source = '''
from django.contrib.auth.decorators import login_required

@login_required
def bar():
    pass

z = bar()'''

script = jedi.Script(source, column=5)
print(script.goto_definitions())
print(script.goto_assignments())

outputs

[<Definition def _wrapped_view>]
[<Definition def bar>]

Sometimes goto_assignments might take to right location.

So if jedi returns multiple locations, we look at thing at point and then look at both locations and go to relevant location for M-.?

[jorgenschaefer]

Both method calls can return multiple locations. Yes, the problem is figuring out which of the results is the most relevant.

[steve-the-bayesian]

Has anything changed in the past 5 years to make a solution more feasible. I hit this bug daily. Thanks.

[galaunay]

I just ran the example from above:

from django.contrib.auth.decorators import login_required

@login_required
def bar():
    pass

z = bar()

and it works as expected (jumping to def bar():).

Do you have a non-working example ?

[steve-the-bayesian]

Here's what breaks for me. Let me know what I can send you from my environment (and how to get the information)

def printme(f):
    """
    A decorator that prints the name of the function being called.
    """
    def printed(*args, **kw):

        print(f"begin {f.__name__}")
        result = f(*args, **kw)

        print(f"Done with function: {f.__name__} ")
        return result

    return printed

@printme
def bar():
    print("Hey, I'm in the 'bar'.  WooHoo!!")

z = bar()

Expected behavior: With cursor on the word 'bar' in the z = bar() call, press Command-. (I'm on a mac) Cursor should jump to 'def bar():'

Observed behavior: Cursor jumps to 'def printed'

[galaunay]

Thanks,

There was some discussions on jedi repository about this (this issue and that PR).

Their solution was to use goto_assignment instead of goto_definitions (it is not bound in elpy, but there is the function elpy-goto-assignment that you can bind to something).

[steve-the-bayesian]

If I bind it to M-. will M-, provide the return journey?

On Sun, Mar 22, 2020, 10:18 AM galaunay notifications@github.com wrote:

Thanks,

There was some discussions on jedi repository about this (this https://github.com/davidhalter/jedi-vim/issues/348 issue and that https://github.com/davidhalter/jedi-vim/pull/808 PR).

Their solution was to use goto_assignment instead of goto_definitions (it is not bound in elpy, but there is the function elpy-goto-assignment that you can bind to something).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/jorgenschaefer/elpy/issues/433#issuecomment-602242082, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMVDVJIYW6WMNCEZVZP63LRIZB7HANCNFSM4AZ3MWJA .

[galaunay]

It should, yes.

[steve-the-bayesian]

Seems to work. Thank you!

On Sun, Mar 22, 2020 at 10:24 AM galaunay notifications@github.com wrote:

It should, yes.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/jorgenschaefer/elpy/issues/433#issuecomment-602242871, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMVDVMI22TLMD2OP6JN2RLRIZCTZANCNFSM4AZ3MWJA .

[TauPan]

Works fine! Finally ;)

How should we proceed? The issue is fixed by using elpy-goto-assignment instead of elpy-goto-definition for me, but I feel that this is something that should be documented.

Also I'm wondering if it still applies that goto-definition will sometimes go to the unexpected place.

[galaunay]

We can't really blame jedi for the behavior of goto-definition, as it does jump to the real function definition (which is inside the decorator definition).

On the other hand, goto-assigment cannot be used as a replacement of goto-definition, as it won't always work. For example in this snippet it will jump to the assignment of a, which is a = foo:

def foo(a, b):
  return a + b

a = foo
a   #  <== cursor here

The ideal solution would be to make goto-definition a bit smarter and avoid jumping inside decorator definitions (I agree that this is rarely what we want...).

Jedi does not provide a way to do that out-of-the-box, but we should be able to tweak rpc_get_definition() to be a bit smarter about decorators.

[TauPan]

FTR, this is my current workaround right now:

;; from
;; https://github.com/jorgenschaefer/elpy/wiki/Customizations#an-alternative-to-elpy-goto-definition
(defun elpy-goto-definition-or-rgrep (arg)
  "Go to the definition of the symbol at point, if found. Otherwise, run `elpy-rgrep-symbol'."
  (interactive "p")
  (ring-insert find-tag-marker-ring (point-marker))
  ;; elpy-goto-definition does not work with decorated
  ;; functions see
  ;; https://github.com/jorgenschaefer/elpy/issues/433#issuecomment-602242082
  (condition-case nil (if (> arg 1)
                          (elpy-goto-definition)
                        (elpy-goto-assignment))
    (error (elpy-rgrep-symbol
            (concat "\\(def\\|class\\)\s" (thing-at-point 'symbol) "(")))))

The case mentioned above that there is an assignment in outer scope with identical name to the inner scope should not happen very often, as you're supposed to use longer identifiers with larger scopes or longer lifetimes, so 'elpy-goto-assignment` will be what I expect more often than not. (Unless there's another case I'm not seeing here.)

[galaunay]

Thanks.

I couldn't think of a "smart" way of using both elpy-goto-definition and elpy-goto-assignement in parallel. So I just updated the documentation (in customization tips) to mention the alternative.