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

Unable to autosuggest members with typing.Literal types when used in Generic classes #4938

Closed
girip11 opened this issue Oct 7, 2023 · 8 comments
Assignees
Labels
enhancement New feature or request

Comments

@girip11
Copy link

girip11 commented Oct 7, 2023

Environment

Language Server version: v2023.10.11 (pre-release version)
OS and version: macOS 13.2.1 Ventura
Python version: 3.11.5

Issue

Consider the below snippet.

from typing import Any, Generic, Literal, TypeAlias, TypeVar

T = TypeVar("T", bound=str)

DS1FieldNames = Literal["col1", "col2", "col3"]


class GenericType(Generic[T]):
    def calculate(self, value: T) -> None:
        pass

    def __getattr__(self, value: T) -> Any:
        pass


ConcreteType: TypeAlias = GenericType[DS1FieldNames]

c = ConcreteType()

In the above example, when I invoke the method c.calculate(), I get the field suggestion as in the image below.

image

But when I type c., I expected that the literal values should be populated in the list of attributes for the object c. But this wasn't the case.

If I change the signature of the __getattr__, it works as expected, as you could see in the image below.
image

NOTE - If I invoke the magic methods explicitly, the auto suggestion works.
image

This applies to the __getitem__ method too. I assume this is the case for all the dunder methods overridden in the class.

I ran pyright against this code and it didn't complain, so I assume it passed the type checking rules. When I used the reveal_type for all these magic methods, pyright gave me the correct types.
image

Am I doing something incorrect here? Or currently such autosuggestions are not supported on magic dunder methods implemented using generic types?

I am not sure if this would qualify for a feature request? Getting these members through auto suggestion helps greatly minimizing the member spelling lookups. Currently to make it work, I would need to duplicate the class implementation if I need to get the auto suggestions for different typing.Literal values.

@github-actions github-actions bot added the needs repro Issue has not been reproduced yet label Oct 7, 2023
@rchiodo
Copy link
Contributor

rchiodo commented Oct 9, 2023

Thanks for the issue. This looks like a bug to me. We have support for __getattr__ but I don't believe we save the type argument for the TypeAlias as you've shown here.

@rchiodo rchiodo removed the needs repro Issue has not been reproduced yet label Oct 9, 2023
@judej judej assigned heejaechang and unassigned rchiodo Oct 9, 2023
@judej judej added the enhancement New feature or request label Oct 9, 2023
@girip11
Copy link
Author

girip11 commented Oct 10, 2023

Thanks so much for acknowledging.

I will use this issue to document other related ones on the same snippet I have posted above. I have found similar issues with other magic methods like __getitem__, __setitem__ too.

My expectation is that, the way Intellisense works for calculate() method, it should also work the same way for the magic methods.

@girip11
Copy link
Author

girip11 commented Oct 10, 2023

Intellisense DOES NOT work for the __getitem__

class GenericType(Generic[T]):
    def calculate(self, value:  T) -> None:
        pass

    # Intellisense DOES NOT work
    def __getitem__(self, value: T) -> Any:
        pass

Intellisense WORKS for the below signature of __getitem__

class GenericType(Generic[T]):
    def calculate(self, value:  T) -> None:
        pass

    # Intellisense WORKS
    def __getitem__(self, value: DS1FieldNames) -> Any:
        pass

@girip11
Copy link
Author

girip11 commented Oct 10, 2023

Intellisense DOES NOT work for the __getitem__

class GenericType(Generic[T]):
    # intellisense works very nicely for this method
    # It autosuggests for both single value or when I pass a list of values
    def calculate(self, value:  T | list[T]) -> None:
        pass

    # Intellisense suggests only for DS1FieldNames but when I try to pass a list
    # it doesnt suggest anything.
    def __getitem__(self, value: DS1FieldNames | list[DS1FieldNames]) -> Any:
        pass

I will capture the screenshot so that its easier to understand. As you can see in the below image, intellisense works WELL for the calculate method
image

Intellisense doesn't work for __getitem__ when passing a list

image

erictraut pushed a commit to microsoft/pyright that referenced this issue Oct 10, 2023
… with literal types. The old logic didn't properly handle generics or overloads. This addresses microsoft/pylance-release#4938.
@erictraut
Copy link
Contributor

erictraut commented Oct 10, 2023

There is currently no support for __getattr__ in the completion provider logic. That's not a bug. It's just functionality that doesn't exist. A __getattr__ method that limits its input to literal types is unusual, so the pylance team may want to wait for additional user input before deciding how to prioritize this request. Handling all of the edge cases for such a feature will be tricky because it would need to handle generics, overloads, and metaclasses. It will also need to distinguish between __getattr__, __setattr__ and __delattr__ depending on how the expression is used in the code.

The completion provider does have support for a __getitem__ method that limits its inputs to a literal type, but the current code doesn't handle generics or overloads. I've addressed both of these limitations. This will be included in the next release of pyright and a future release of pylance.

Here's how it looks with the new logic in place:

image

@girip11
Copy link
Author

girip11 commented Oct 10, 2023

Thanks Eric. Regarding the __getitem__ fix, could you confirm if it works for the signature def __getitem__(self, value: T| list[T]): and when I try to pass c.[[""]]. Previously it wasn't showing up in the auto suggestions.

On the __getattr__ support, currently if I explicitly provide the parameter type, I do get the right value suggestions when I type c. . So I assumed its supported but only the support for generic type parameters was missing.

When it comes to __getitem__, we do have __setitem__ and __delitem__, so I thought if it autosuggests for __getitem__, then on the similar lines, __getattr__ can also be supported.

@erictraut
Copy link
Contributor

erictraut commented Oct 10, 2023

The logic works only with a __getitem__ that accepts a literal type or a TypeVar that is specialized to a literal type. It doesn't work for more complex types like T | list[T]. If you want that to work, please file a separate feature request, and the pylance team can prioritize it based on user feedback.

@girip11
Copy link
Author

girip11 commented Oct 11, 2023

Thanks Eric. I will raise a new feature request and I will close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants