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

Bug when function return is typing.Type[] #821

Closed
nourselim0 opened this issue Jan 10, 2021 · 6 comments
Closed

Bug when function return is typing.Type[] #821

nourselim0 opened this issue Jan 10, 2021 · 6 comments
Labels
bug Something isn't working fixed in next version (main) A fix has been implemented and will appear in an upcoming version

Comments

@nourselim0
Copy link

Environment data

  • Language Server version: 20201.1.0
  • OS and version: Windows 10 Version 2004
  • Python version: 3.8.2

Expected behaviour

Let's say I have a function called get_cls and I explicitly specify it's return type to be Type[Example] (assume it uses importlib to get the actual class, so explicit hint is needed), and let's say Example has some member called num.
If I type get_cls().num I should find num in the intellisense list, and it should get a semantic token color, and pressing F12 (go to definition) on num should work. It should be identical to typing Example.num directly.

Actual behaviour

What happens is that num shows up in intellisense, but it doesn't get any semantic tokens, and F12 fails.
Doing the same for Example.num works as expected.

Code Snippet / Additional information

Screenshot showing "go to definition" error:
image
Notice the error and the lack of semantic color.

Full Code Sample:

from typing import Type

class Example:
    num = 42

Example.num  # works fine

def get_cls() -> Type[Example]:
    return Example

get_cls().num  # no token color, F12 fails

Removing the Type[Example] return hint fixes the issue, but only because the function is very basic.

@erictraut
Copy link
Contributor

Thanks for the clear and concise bug report. This will be fixed in the next release of pylance.

@erictraut erictraut added bug Something isn't working fixed in next version (main) A fix has been implemented and will appear in an upcoming version and removed triage labels Jan 10, 2021
@nourselim0
Copy link
Author

Thank you for the super-quick fix!

@jakebailey
Copy link
Member

This issue has been fixed in version 2021.1.1, which we've just released. You can find the changelog here: https://github.com/microsoft/pylance-release/blob/main/CHANGELOG.md#202111-13-january-2021

@nourselim0
Copy link
Author

The problem still happens if the inferred type is Type[x] | None or any union involving Type. To recreate this, you can edit my code sample so that the method returns Optional[Type[Example]].
I know that the correct thing to do is to check for none first (or an isinstance check), so afterwards the type is narrowed correctly, but this issue doesn't happen if the inferred type is any other Union like str | int (a call to upper will still be tokenized in that example). So there is an inconsistency here.
Should this be re-opened?

@erictraut
Copy link
Contributor

This is the intended behavior.

If you change your code to indicate that the return type is Optional[Type[Example]], it would be a programming error to access the num attribute of the result without first verifying that it's not None.

# pyright: strict

from typing import Optional, Type

class Example:
    num = 42

def get_cls() -> Optional[Type[Example]]:
    return Example

x = get_cls()
x.num  # Error reported in strict mode or with reportOptionalMemberAccess enabled

if x is not None:
    x.num # Does not report error and F12 works correctly

@nourselim0
Copy link
Author

I'm mainly talking about the case of having type checking off, here.
While I agree with the logic that you should always put conditions that narrow the type, but now there is an inconsistency with other Unions.
Maybe you should then change the behavior in this case:

from typing import Union

def get_str_or_int() -> Union[str, int]:
    pass

x = get_str_or_int()
x.upper()  # right now "upper" is tokenized and "go to definition" works on it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed in next version (main) A fix has been implemented and will appear in an upcoming version
Projects
None yet
Development

No branches or pull requests

3 participants