davidhalter / jedi

Awesome autocompletion, static analysis and refactoring library for python
http://jedi.readthedocs.io
Other
5.77k stars 508 forks source link

No completions for method returning class generic type when __init__ contains nested tuple type #1560

Closed PeterJCLaw closed 4 years ago

PeterJCLaw commented 4 years ago

(Discovered while exploring a issue with NamedTuples)

from typing import List, Tuple, TypeVar, Generic, NewType

T = TypeVar('T')

class That(Generic[T]):
    # It doesn't seem to matter where the `T` comes in this signature - just
    # somewhere so that it's valid from an annotation perspective.
    def __init__(self, items: List[Tuple[str, T]]) -> None:
        pass

    # I don't think it matters that this is `T` specifically - it just needs to
    # be something non-concrete, so that jedi is forced to evaluate the class
    # rather than being able to short-circuit that by looking only at the annotation.
    def get(self) -> T:
        pass

inst = That([("abc", 2)])

# No completions here, but should have completions for `int`
inst.get().
PeterJCLaw commented 4 years ago

Where I've got to so far with this is that jedi rejects the signature of the __init__ method when trying to resolve the creation of inst, due to [("abc", 2)] not looking enough like List[Tuple[str, T]].

Specifically we end up in:

At the bottom level we're comparing two similar but not sufficiently equal types:

I might carry on with this at some point, however I feel I'm hitting a similar issue here to the one discussed on https://github.com/davidhalter/jedi/pull/1554#discussion_r410908948. Not because this is nested tuples per-se, but rather that jedi has two representations for a very similar concept and I've not really understood why that's the case.

davidhalter commented 4 years ago

At the bottom level we're comparing two similar but not sufficiently equal types:

Yeah, I think this is a fundamental issue with tuples that I also dislike. I think there's at least two potential solutions, of which I'm not sure they if they actually work:

  1. Patch DefineGenericBase.is_same_class for Tuple[] and maybe also tuple[]?!
  2. Bring Tuple and tuple closer together. Maybe tuple[int, ...] would then also be homogenous.

But I think we will be smarter after merging your pull requests. It's a recurring theme in there :).

davidhalter commented 4 years ago

After working on #1572 and testing this briefly, I feel like the main issue is that __init__ is never properly analyzed. The problem is basically that __init__ behaves differently than a normal method: It's really hasn't been necessary to look at it at all if you don't use overload, because people could just use That[int]([("abc", 2)]) to make it work.

So if we want to make it work, we probably have to check the passed arguments against __init__ and get type vars from there. That's at least my suspicion. I think I tested it with items: T instead of items: List[Tuple[str, T]], which is way simpler. It still did not work.

davidhalter commented 4 years ago

@PeterJCLaw Before I release 0.17.1 will you work on this or should I?

davidhalter commented 4 years ago

I just realized that you were right and it's only a problem with that specific example, items: T works.

davidhalter commented 4 years ago

This was a small issue with class matching. IMO class matching is not in a great shape at all and should probably be refactored. It's however not that easy to do it better :).