Jedi: Incorrect parent when methods of class are annotated by decorator

[Jma353]

The Bug

I have a stable repro of an issue I've observed in the context of the "Outline" pane in IDEs. Basically, Jedi is reflecting the wrong parent for a method within a class.

I have added the following code to the test_classes.py test case locally:

def test_parent_on_decorator(Script):
    script = Script(dedent('''\
        class AClass:
            @decorator()
                def b_test(self):
                print("Hello")
                print("world")

            def a_test(self):
                pass'''))
    names = script.get_names(all_scopes=True)

    a_test, = filter(lambda d: d.name == 'a_test', names)

    assert a_test.parent().name == 'AClass'

    script = Script(dedent('''\
        class AClass:
            @decorator()
            def b_test(self):
                print("Hello")
                print("world")

            def a_test(self):
                pass'''))
    names = script.get_names(all_scopes=True)

    b_test, = filter(lambda d: d.name == 'b_test', names)
    a_test, = filter(lambda d: d.name == 'a_test', names)

    assert b_test.parent().name == 'AClass'
    assert a_test.parent().name == 'AClass'

This test-case reliably fails, indicating that the SECOND time around, Jedi thinks a_test's parent is b_test. I suspect that this has to do with some global state Jedi is maintaining? In general, this test case simulates the case where a user has improper indentation, and then fixes it. This does not occur when indentation is correct off the bat.

Suggestions / Code Pointers?

I would love to fix this bug myself. However, given the complexity of Jedi, I was wondering if @davidhalter or others had an initial notion as to where this bug might be occurring. I'd love to scope down my search for the issue as much as possible. Thanks!

[Jma353]

NOTE: I was able to repro this bug in the VS Code Python extension from MSFT. It manifests itself in the Outline view. cc: @DonJayamanne

[davidhalter]

I have a suspicion. Will look into it ASAP and then try to not spoil you with all the details, so you can solve it yourself :)

[davidhalter]

Thanks a lot for finding this and writing a good reproduction case!! You wouldn't understand how good this makes me feel to finally find another one of these. I wrote a fuzzer for parso to find all of these, but this is one of the issues that the fuzzer didn't find. I've always known there was at least one remaining issue (probably not much more and this one might have been it) that was causing issues.

This is a parso issue. That being said, it's a diff parser issue (parso/python/diff.py). Unfortunately it's extremely hard to fix these. It's very messy code designed to speed up parsing if most of the code remains the same. So I'm really warning you, the code in parso is probably the ugliest of all of the code in Jedi/Parso combined :).

Generally you get rid of this issue if you write

import jedi                      
jedi.settings.fast_parser = False

but this will slow down completions substantially in some cases.

If you want to fix this, I've written a failing parso test (derived from your test) in parso/test/test_diff_parser.py:

def test_parent_on_decorator(differ):
    code1 = dedent('''\
        class AClass:
            @decorator()
                def b_test(self):
                print("Hello")
                print("world")

            def a_test(self):
                pass''')
    code2 = dedent('''\
        class AClass:
            @decorator()
            def b_test(self):
                print("Hello")
                print("world")

            def a_test(self):
                pass''')
    differ.initialize(code1)
    module_node = differ.parse(code2, copies=1, parsers=1)
    cls = module_node.children[0]
    cls_suite = cls.children[-1]
    assert len(cls_suite.children) == 3

At that point you would need to start working on parso/python/diff.py and trying to find the reason why this is happening. This is pretty hard and I'm happy to try to help you, but I'd also have to experiment quite a bit to find what exactly in that file is causing it. So are you going to try? :)

[Jma353]

I can try; thanks for your context! Let me get acclimated with the parso codebase and I'll create a separate parso issue that points back to this issue.

[davidhalter]

You don't have to create a parso issue, we can also discuss this here, but do whatever you like :)

[Jma353]

If that's what you prefer, we can keep it all here :)

[Jma353]

I believe I've localized the issue to the following function.

Basically, the top of the stack tos is the b_test function for the above failing test case. If I, say, setup the following:

class AClass:
    @decorator()
    def b_test(self):
        a = 1
    b = 2

    def a_test(self):
        pass

and toggled the b_test indentation, the diffed parser properly reflects the parent of a_test b/c the tos is the class (since the simple statement of the b = 2 is outside the suite of b_test).

I'm working on a way to tweak this function to properly recognize that a_test is outside the scope of b_test when there's multiple statements within b_test.

[davidhalter]

My guess is that it has to do with

            @decorator()
                def b_test(self):

Where the decorator doesn't have the same indentation as b_test. This is really a special case that I really don't understand at the moment.

[Jma353]

Yep, when I get more cycles, I'll dig in more. Your assumption is correct in the sense that the parent-child relationship gets mixed up when there's a @decorator available, due to how the parser recovers.

Just figured I'd keep you up to date on the issue investigation :).

[davidhalter]

Are you still working on this? :)

[davidhalter]

I ended up fixing it :). This was a relatively high priority bug and it was important to me that it fixed at some point. So I'm sorry for not waiting with it :).

https://github.com/davidhalter/parso/commit/51a044cc702d842c28748fb448d063511cfc1ab0 is the relevant commit.

[davidhalter]

Thanks again for the reproduction case. That was extremely valuable! You probably wouldn't understand how hard it is for me to reproduce this kind of stuff :).

I'm going to work a bit on fuzzing to make it possible to find such issues. I'm going to tag the commits, so you can see them here.

[Jma353]

Sweet; sorry for the slow response; a few other things came onto my plate that delayed me from working on this; I'm glad my repro and directives helped you fix this!

[Jma353]

Btw, is there an ETA on 0.17 stabilizing and being available through PyPI? I'm excited for the crazy # of new features! Awesome job!

[davidhalter]

No ETA, yet. I'm trying to fix the last parso issues and will probably release at that point. It will happen soon. There's a great pull request open about generics that I want to merge and then I might do the Django stuff as well, which is probably not a lot of work.

[davidhalter]

After 70 commits more and 3 weeks of work I'm finally finished with that parso rework. :) Thanks again for finding this reproduction. I went down the rabbit whole because of it, but I guess it made sense and now stuff like this shouldn't really happen anymore.

Jedi release will be available in the next few days hopefully. I was really just trying to get the parso release completed. Now it's just a matter of reviewing the API and maybe some minor adjustments.