-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Infer types from issubclass() calls #3005
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
Changes from 1 commit
158c889
c5525bd
1776e90
3334b8e
3587549
81e1923
e4ffdc5
9440a52
c831401
a4a7214
f94ca45
a260b79
fbd0b91
e48d983
b0ce683
aca05c7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,7 @@ | |
import sys | ||
|
||
from typing import ( | ||
Dict, Set, List, cast, Tuple, TypeVar, Union, Optional, NamedTuple, Iterator, MutableMapping | ||
Dict, Set, List, cast, Tuple, TypeVar, Union, Optional, NamedTuple, Iterator | ||
) | ||
|
||
from mypy.errors import Errors, report_internal_error | ||
|
@@ -2616,18 +2616,18 @@ def or_conditional_maps(m1: TypeMap, m2: TypeMap) -> TypeMap: | |
return result | ||
|
||
|
||
def convert_to_types(m: MutableMapping[Expression, Type]) -> None: | ||
for k in m: | ||
x = m[k] | ||
if isinstance(x, UnionType): | ||
m[k] = UnionType([TypeType(t) for t in x.items]) | ||
elif isinstance(x, Instance): | ||
m[k] = TypeType(m[k]) | ||
def convert_to_typetype(type_map: TypeMap) -> TypeMap: | ||
converted_type_map: TypeMap = {} | ||
for expr, typ in type_map.items(): | ||
if isinstance(typ, UnionType): | ||
converted_type_map[expr] = UnionType([TypeType(t) for t in typ.items]) | ||
elif isinstance(typ, Instance): | ||
converted_type_map[expr] = TypeType(typ) | ||
else: | ||
# TODO: verify this is ok | ||
# unknown object, don't know how to convert | ||
m.clear() | ||
return | ||
return {} | ||
return converted_type_map | ||
|
||
|
||
def find_isinstance_check(node: Expression, | ||
|
@@ -2659,17 +2659,23 @@ def find_isinstance_check(node: Expression, | |
expr = node.args[0] | ||
if expr.literal == LITERAL_TYPE: | ||
vartype = type_map[expr] | ||
type = get_issubclass_type(node.args[1], type_map) | ||
type = get_isinstance_type(node.args[1], type_map) | ||
if isinstance(vartype, UnionType): | ||
vartype = UnionType([t.item for t in vartype.items]) # type: ignore | ||
union_list = [] | ||
for t in vartype.items: | ||
if isinstance(t, TypeType): | ||
union_list.append(t.item) | ||
else: | ||
# this an error; should be caught earlier, so we should never be here | ||
return {}, {} | ||
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. If you are sure this is an unreachable branch, then you could put |
||
vartype = UnionType(union_list) | ||
elif isinstance(vartype, TypeType): | ||
vartype = vartype.item | ||
else: | ||
# TODO: verify this is ok | ||
return {}, {} # unknown type | ||
yes_map, no_map = conditional_type_map(expr, vartype, type) | ||
convert_to_types(yes_map) | ||
convert_to_types(no_map) | ||
yes_map, no_map = map(convert_to_typetype, (yes_map, no_map)) | ||
return yes_map, no_map | ||
elif refers_to_fullname(node.callee, 'builtins.callable'): | ||
expr = node.args[0] | ||
|
@@ -2777,30 +2783,6 @@ def get_isinstance_type(expr: Expression, type_map: Dict[Expression, Type]) -> L | |
return types | ||
|
||
|
||
def get_issubclass_type(expr: Expression, type_map: Dict[Expression, Type]) -> Type: | ||
all_types = [type_map[e] for e in flatten(expr)] | ||
|
||
types = [] # type: List[Type] | ||
|
||
for type in all_types: | ||
if isinstance(type, FunctionLike): | ||
if type.is_type_obj(): | ||
# Type variables may be present -- erase them, which is the best | ||
# we can do (outside disallowing them here). | ||
type = erase_typevars(type.items()[0].ret_type) | ||
types.append(type) | ||
elif isinstance(type, TypeType): | ||
types.append(type.item) | ||
else: # we didn't see an actual type, but rather a variable whose value is unknown to us | ||
return None | ||
|
||
assert len(types) != 0 | ||
if len(types) == 1: | ||
return types[0] | ||
else: | ||
return UnionType(types) | ||
|
||
|
||
def expand_func(defn: FuncItem, map: Dict[TypeVarId, Type]) -> FuncItem: | ||
visitor = TypeTransformVisitor(map) | ||
ret = defn.accept(visitor) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1412,9 +1412,41 @@ def f(x: Union[int, A], a: Type[A]) -> None: | |
|
||
[builtins fixtures/isinstancelist.pyi] | ||
|
||
|
||
[case testIssubclass] | ||
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. Is it possible to split this test into few smaller unit tests? |
||
from typing import Type, ClassVar | ||
|
||
class Goblin: | ||
aggressive: bool = True | ||
level: int | ||
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. Do you need both attributes here? Some test cases could be simplified if you have only one instance variable. |
||
|
||
class GoblinAmbusher(Goblin): | ||
job: ClassVar[str] = 'Ranger' | ||
|
||
|
||
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. One blanc line is enough here |
||
def test_issubclass(cls: Type[Goblin]) -> None: | ||
if issubclass(cls, GoblinAmbusher): | ||
cls.aggressive | ||
cls.level | ||
cls.job | ||
ga = cls() | ||
ga.level = 15 | ||
ga.job | ||
ga.job = "Warrior" # E: Cannot assign to class variable "job" via instance | ||
else: | ||
cls.aggressive | ||
cls.level | ||
cls.job # E: Type[Goblin] has no attribute "job" | ||
g = cls() | ||
g.level = 15 | ||
g.job # E: "Goblin" has no attribute "job" | ||
|
||
|
||
[builtins fixtures/isinstancelist.pyi] | ||
|
||
|
||
[case testIssubclassDeepHierarchy] | ||
from typing import Type, ClassVar | ||
|
||
class Mob: | ||
pass | ||
|
@@ -1426,11 +1458,7 @@ class Goblin(Mob): | |
class GoblinAmbusher(Goblin): | ||
job: ClassVar[str] = 'Ranger' | ||
|
||
class GoblinDigger(Goblin): | ||
job: ClassVar[str] = 'Thief' | ||
|
||
|
||
def test_issubclass1(cls: Type[Mob]) -> None: | ||
def test_issubclass(cls: Type[Mob]) -> None: | ||
if issubclass(cls, Goblin): | ||
cls.aggressive | ||
cls.level | ||
|
@@ -1471,6 +1499,27 @@ def test_issubclass1(cls: Type[Mob]) -> None: | |
ga.job | ||
ga.job = "Warrior" # E: Cannot assign to class variable "job" via instance | ||
|
||
[builtins fixtures/isinstancelist.pyi] | ||
|
||
|
||
[case testIssubclassTuple] | ||
from typing import Type, ClassVar | ||
|
||
class Mob: | ||
pass | ||
|
||
class Goblin(Mob): | ||
aggressive: bool = True | ||
level: int | ||
|
||
class GoblinAmbusher(Goblin): | ||
job: ClassVar[str] = 'Ranger' | ||
|
||
class GoblinDigger(Goblin): | ||
job: ClassVar[str] = 'Thief' | ||
|
||
|
||
def test_issubclass(cls: Type[Mob]) -> None: | ||
if issubclass(cls, (Goblin, GoblinAmbusher)): | ||
cls.aggressive | ||
cls.level | ||
|
@@ -1512,23 +1561,22 @@ def test_issubclass1(cls: Type[Mob]) -> None: | |
g.job | ||
g.job = "Warrior" # E: Cannot assign to class variable "job" via instance | ||
|
||
[builtins fixtures/isinstancelist.pyi] | ||
|
||
def test_issubclass2(cls: Type[Goblin]) -> None: | ||
if issubclass(cls, GoblinAmbusher): | ||
cls.aggressive | ||
cls.level | ||
cls.job | ||
ga = cls() | ||
ga.level = 15 | ||
ga.job | ||
ga.job = "Warrior" # E: Cannot assign to class variable "job" via instance | ||
|
||
[case testIssubclassBuiltins] | ||
from typing import List, Type | ||
|
||
class MyList(List): pass | ||
class MyIntList(List[int]): pass | ||
|
||
def f(cls: Type): | ||
if issubclass(cls, MyList): | ||
cls()[0] | ||
else: | ||
cls.aggressive | ||
cls.level | ||
cls.job # E: Type[Goblin] has no attribute "job" | ||
g = cls() | ||
g.level = 15 | ||
g.job # E: "Goblin" has no attribute "job" | ||
cls()[0] # E: | ||
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. What is the error message here? Why didn't you write it here? 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. Oh, no error message I just accidentally left 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. But shouldn't this actually be an error? In the 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. No, it's my bad. I confused this test with another where I left The reason it doesn't complain is that Btw, I would prefer |
||
|
||
if issubclass(cls, MyIntList): | ||
cls()[0] + 1 | ||
|
||
[builtins fixtures/isinstancelist.pyi] | ||
[builtins fixtures/isinstancelist.pyi] |
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.
typo: "this an error;" -> "this is an error;"