-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Narrow literals in the negative case even with custom equality #18574
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -845,7 +845,7 @@ x1: Union[Custom, Literal[1], Literal[2]] | |
if x1 == 1: | ||
reveal_type(x1) # N: Revealed type is "Union[__main__.Custom, Literal[1], Literal[2]]" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why isn't There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah we note that custom equality exists and if so give up on narrowing currently (iirc, it's been a few months...). I guess it could be fine grained, this PR just notes that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, iterating over every component type of a union and applying this logic might make sense, and would also fix this bug. But that sounds more complicated than just this. |
||
else: | ||
reveal_type(x1) # N: Revealed type is "Union[__main__.Custom, Literal[1], Literal[2]]" | ||
reveal_type(x1) # N: Revealed type is "Union[__main__.Custom, Literal[2]]" | ||
|
||
x2: Union[Default, Literal[1], Literal[2]] | ||
if x2 == 1: | ||
|
@@ -2417,6 +2417,31 @@ while x is not None and b(): | |
|
||
[builtins fixtures/primitives.pyi] | ||
|
||
|
||
[case testNegativeNarrowingWithCustomEq] | ||
from typing import Union | ||
from typing_extensions import Literal | ||
|
||
class A: | ||
def __eq__(self, other: object) -> bool: ... # necessary | ||
|
||
def f(v: Union[A, Literal["text"]]) -> Union[A, None]: | ||
if v == "text": | ||
reveal_type(v) # N: Revealed type is "Union[__main__.A, Literal['text']]" | ||
return None | ||
else: | ||
reveal_type(v) # N: Revealed type is "__main__.A" | ||
return v # no error | ||
|
||
def g(v: Union[A, Literal["text"]]) -> Union[A, None]: | ||
if v != "text": | ||
reveal_type(v) # N: Revealed type is "__main__.A" | ||
return None | ||
else: | ||
reveal_type(v) # N: Revealed type is "Union[__main__.A, Literal['text']]" | ||
return v # E: Incompatible return value type (got "Union[A, Literal['text']]", expected "Optional[A]") | ||
[builtins fixtures/primitives.pyi] | ||
|
||
[case testNarrowingTypeVarMultiple] | ||
from typing import TypeVar | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternative: I could try
has_no_custom_eq_checks(p_t.fallback)
maybe?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know any other literals that could possibly have custom
__eq__
, amd enums are complex enough to have some unexpected corner cases - this check looks reasonable to me as-is. Maybe rewrite asreturn should_narrow_by_identity or not p_t.fallback.type.is_enum
to make it more clear than if/else? And maybe even the whole ladder as justreturn isinstance(p_t, LiteralType) and (... or ...)
?