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

Inference for empty in-place container types #1736

Closed
cdce8p opened this issue Aug 28, 2021 · 6 comments
Closed

Inference for empty in-place container types #1736

cdce8p opened this issue Aug 28, 2021 · 6 comments
Labels
enhancement New feature or request fixed in next version (main) A fix has been implemented and will appear in an upcoming version type checking

Comments

@cdce8p
Copy link

cdce8p commented Aug 28, 2021

Environment data

  • Language Server version: 2021.8.3
  • OS and version: darwin x64
  • Python version (and distribution if applicable, e.g. Anaconda): 3.10
  • python.analysis.indexing: undefined
  • python.analysis.typeCheckingMode: basic

Expected behaviour

Revealed type should be item: str

Actual behaviour

item: str | Any

Logs

--

Code Snippet / Additional information

def func(lst: list[str] | None = None) -> None:
    for item in lst or ():
        reveal_type(item)  # str | Any

    for item in lst or tuple():
        reveal_type(item)  # str | _T_co@tuple


def func(lst: tuple[str, ...] | None = None) -> None:
    for item in lst or []:
        reveal_type(item)  # str | Any

def func(lst: list[str] | None = None) -> None:
    for item in lst or {}:
        reveal_type(item)  # str | Any

Mypy will infer builtins.str* in all cases.

--
Note: It does work correctly if an empty list is used.

def func(lst: list[str] | None = None) -> None:
    for item in lst or []:
        reveal_type(item)  # str
@erictraut
Copy link
Contributor

Here's what's going on. The type of the subexpression lst or () evaluates to list[str] | tuple[()]. The iterated type is determined by looking at the __iter__ and __next__ methods of these respective specialized classes. This results in the type str | Any. The same is true for lists and dictionaries. So, from a type checking perspective, I think the results are correct based on the type information provided in the typeshed stubs.

The reason this works with a combination of lists is due to special-case handling in the type inference code for or operators. If an empty list or dictionary expression is found on the RHS of an or, the type of the LHS operand is used for bidirectional type inference on the RHS. So in your example above, [] would normally be inferred to have type list[Unknown], but in this case, it is inferred as list[str]. This allows the expression lst or [] to be evaluated as list[str].

There is one case above that could be made to work with some straightforward special-casing logic: the lst or () expression. In that particular case, the iterated type of tuple[()] can be evaluated as Never, so the expression lst or () would then evaluate to list[str]. I've updated the type checking logic to handle this special case.

The other cases above would require much hackier special casing, embedding deep semantic knowledge of these types in the type checker. We generally don't do this unless it's a really common use case because these special cases make the code less maintainable and slower — and often break other edge cases within the type analysis. We can see if we get signal from other users that this is justified here, but for now I'm going to leave the behavior as is for the non-tuple case. This gives you a couple of viable workarounds to handle your use case.

@cdce8p
Copy link
Author

cdce8p commented Aug 28, 2021

Thanks for looking into it! Usually, I only come across lst or [] or lst or (), so it should be enough.

I found the other cases while investigating under which circumstances this happens. Thought I mention them as well, since I already found them.

@jakebailey
Copy link
Member

@erictraut Was this fixed in microsoft/pyright@49d9c7c?

@jakebailey
Copy link
Member

Ah, I see, you said it was special cased just for that one case.

@jakebailey jakebailey added the enhancement New feature or request label Aug 30, 2021
@cdce8p
Copy link
Author

cdce8p commented Aug 30, 2021

With the commit, I would consider it fixed. As mentioned before, the other cases have only really been for debugging purposes.

@jakebailey jakebailey added the fixed in next version (main) A fix has been implemented and will appear in an upcoming version label Aug 30, 2021
@jakebailey
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fixed in next version (main) A fix has been implemented and will appear in an upcoming version type checking
Projects
None yet
Development

No branches or pull requests

3 participants