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

Special-case type inference of empty collections #16122

Merged
merged 3 commits into from
Sep 26, 2023

Conversation

ilevkivskyi
Copy link
Member

Fixes #230
Fixes #6463
I bet it fixes some other duplicates, I closed couple yesterday, but likely there are more.

This may look a bit ad-hoc, but after some thinking this now starts to make sense to me for two reasons:

  • Unless I am missing something, this should be completely safe. Special-casing only applies to inferred types (i.e. empty collection literals etc).
  • Empty collections are actually special. Even if we solve some classes of issues with more principled solutions (e.g. I want to re-work type inference against unions in near future), there will always be some corner cases involving empty collections.

Similar issues keep coming, so I think it is a good idea to add this special-casing (especially taking into account how simple it is, and that it closer some "popular" issues).

@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

mypy (https://github.com/python/mypy)
+ mypy/test/testpep561.py:134: error: Unused "type: ignore" comment  [unused-ignore]

Tanjun (https://github.com/FasterSpeeding/Tanjun)
- tanjun/dependencies/reloaders.py:513: error: Incompatible return value type (got "tuple[Path, Union[tuple[None, set[Never]], tuple[str, set[Never]]]]", expected "tuple[Path, Union[tuple[str, set[str]], tuple[None, set[Path]]]]")  [return-value]

graphql-core (https://github.com/graphql-python/graphql-core)
+ src/graphql/type/definition.py:1010: error: Unused "type: ignore" comment  [unused-ignore]
+ tests/type/test_definition.py:735: error: Unused "type: ignore" comment  [unused-ignore]
+ tests/type/test_definition.py:886: error: Unused "type: ignore" comment  [unused-ignore]

koda-validate (https://github.com/keithasaurus/koda-validate)
+ koda_validate/union.py:171: error: Unused "type: ignore" comment  [unused-ignore]

discord.py (https://github.com/Rapptz/discord.py)
- discord/state.py:258: error: Incompatible types in assignment (expression has type "dict[Never, Never]", variable has type "dict[str, Callable[[Any], None]]")  [assignment]
- discord/message.py:1841: error: Incompatible types in assignment (expression has type "list[Never]", variable has type "list[User | Member]")  [assignment]
- discord/message.py:1841: note: "List" is invariant -- see https://mypy.readthedocs.io/en/stable/common_issues.html#variance
- discord/message.py:1841: note: Consider using "Sequence" instead, which is covariant
- discord/app_commands/tree.py:648: error: Incompatible return value type (got "list[Never]", expected "list[ContextMenu] | list[Command[Any, [VarArg(Any), KwArg(Any)], Any] | Group] | list[Command[Any, [VarArg(Any), KwArg(Any)], Any] | Group | ContextMenu]")  [return-value]

@ilevkivskyi
Copy link
Member Author

mypy_primer looks good (as expected).

@hauntsaninja hauntsaninja merged commit 4b66fa9 into python:master Sep 26, 2023
17 checks passed
@ilevkivskyi ilevkivskyi deleted the special-never branch September 26, 2023 23:39
@cdce8p
Copy link
Collaborator

cdce8p commented Sep 28, 2023

Noticed a small regression in Home Assistant. It seems to be related to the custom dict type in combination with :=.

from typing import TypeVar, Any

_KT = TypeVar("_KT")
_VT = TypeVar("_VT")

class CustomDictType(dict[_KT, _VT]):
    ...

class A:
    options: CustomDictType[str, CustomDictType[str, int]]

def ok(a: A) -> None:
    entry: dict[str, Any]
    entry = a.options.get("entry", {})
    if entry:
        reveal_type(entry)
        val = entry.get("name")

def bad(a: A) -> None:
    entry: dict[str, Any]
    if (
        entry := a.options.get("entry", {})
    ):
        reveal_type(entry)
        val = entry.get("name")
test.py:16: note: Revealed type is "Union[test.CustomDictType[builtins.str, builtins.int], builtins.dict[builtins.str, Any]]"
test.py:24: note: Revealed type is "Union[test.CustomDictType[builtins.str, builtins.int], builtins.dict[Never, Never]]"
test.py:25: error: No overload variant of "get" of "dict" matches argument type "str"  [call-overload]
test.py:25: note: Possible overload variants:
test.py:25: note:     def get(self, Never, /) -> None
test.py:25: note:     def get(self, Never, Never, /) -> Never
test.py:25: note:     def [_T] get(self, Never, _T, /) -> _T

@ilevkivskyi
Copy link
Member Author

Yeah, I guess walrus doesn't correctly "erases" inferred types, there is much simpler repro:

y: List[int]
if (y := []):
    reveal_type(y)  # builtins.list[Never]???

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

Successfully merging this pull request may close these issues.

No type inference for dicts in union Generic function type argument inference problem
3 participants