Fix inference logic for isinstance #2997
Conversation
mypy/checker.py
Outdated
| type = erase_typevars(type.items()[0].ret_type) | ||
|
|
||
| types.append(type) | ||
|
|
There was a problem hiding this comment.
I think there are too many blank lines here. IMO the only blank line should be outside the for-loop, before the assert on line 2720 below.
pkch
force-pushed
the
fix-isinstance-inference
branch
from
a61bf32 to
79a8085
Compare
mypy/semanal.py
Outdated
| # bar in its namespace. This must be done for all types of bar. | ||
| file = base.node | ||
| assert isinstance(file, (MypyFile, type(None))) | ||
| file = cast(Optional[MypyFile], base.node) |
There was a problem hiding this comment.
Add comment describing why we don't use assert isinstance here.
mypy/semanal.py
Outdated
| # the build would terminate after semantic analysis | ||
| # and we wouldn't be able to report any type errors. | ||
| full_name = '%s.%s' % (file.fullname() if file is not None else None, expr.name) | ||
| full_name = '%s.%s' % (file.fullname() if file is not None |
There was a problem hiding this comment.
This change seems unnecessary now?
| from typing import * | ||
| def f(x: Union[int, str], typ: type) -> None: | ||
| if isinstance(x, (typ, int)): | ||
| x + 1 # E: Unsupported operand types for + (likely involving Union) |
There was a problem hiding this comment.
Add also an else: block and do a reveal_type(x) in that block. Add another reveal_type(x) just after the if statement.
| if isinstance(x, (typ, int)): | ||
| x + 1 # E: Unsupported operand types for + (likely involving Union) | ||
| [builtins fixtures/isinstancelist.pyi] | ||
|
|
There was a problem hiding this comment.
Add test for the type object that is Type[A] or similar. Example:
class A: pass
def f(x: Union[int, A], a: Type[A]) -> None:
if isinstance(x, a):
reveal_type(x)
elif isinstance(x, int):
reveal_type(x)
else:
reveal_type(x)
reveal_type(x)
bca997d to
6ea4f4d
Compare
pkch
force-pushed
the
fix-isinstance-inference
branch
from
2fa26d1 to
be30faa
Compare
| return UnionType(types) | ||
| types.append(TypeRange(type, is_upper_bound=False)) | ||
| elif isinstance(type, TypeType): | ||
| types.append(TypeRange(type.item, is_upper_bound=True)) |
There was a problem hiding this comment.
Add comment here describing why is_upper_bound=True.
mypy/checker.py
Outdated
| types.append(TypeRange(type.item, is_upper_bound=True)) | ||
| else: # we didn't see an actual type, but rather a variable whose value is unknown to us | ||
| return None | ||
| assert len(types) != 0 |
There was a problem hiding this comment.
I wonder if we should return None if the length if 0? For example, what happens if we have isinstance(x, ())?
| x + 1 # E: Unsupported operand types for + (likely involving Union) | ||
| reveal_type(x) # E: Revealed type is 'Union[builtins.int, builtins.str]' | ||
| else: | ||
| reveal_type(x) # E: Revealed type is 'Union[builtins.int, builtins.str]' |
There was a problem hiding this comment.
Here we could actually infer the type to be builtins.str, since we know that it can't be builtins.int due to the isinstance check.
| reveal_type(x) # E: Revealed type is '__main__.A' | ||
| reveal_type(x) # E: Revealed type is 'Union[builtins.int, __main__.A]' | ||
|
|
||
| [builtins fixtures/isinstancelist.pyi] |
There was a problem hiding this comment.
Not directly related, but can you add a test case that does something like isinstance(x, ())?
mypy/checker.py
Outdated
| if current_type: | ||
| if is_proper_subtype(current_type, proposed_type): | ||
| # Expression is always of type proposed_type | ||
| if not any(type_range.is_upper_bound for type_range in proposed_type_ranges) \ |
There was a problem hiding this comment.
Use if (...)" to avoid the use of \.
4 participants
Fixes #2993