8000 Infer types from issubclass() calls by pkch · Pull Request #3005 · python/mypy · GitHub
[go: up one dir, main page]

Skip to content

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

Merged
merged 16 commits into from
Apr 21, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Apply CR comments
  • Loading branch information
pkch committed Mar 20, 2017
commit 3334b8e67151f80cc62cdad31d4408bc9b01c32d
58 changes: 20 additions & 38 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Copy link
Member

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;"

return {}, {}
Copy link
Member

Choose a reason for hiding this comment

The 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 assert False here. If this could happen as a result of an error that is reported elsewhere, then adjust the comment correspondingly.

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]
Expand Down Expand Up @@ -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)
Expand Down
90 changes: 69 additions & 21 deletions test-data/unit/check-isinstance.test
Original file line number Diff line number Diff line change
Expand Up @@ -1412,9 +1412,41 @@ def f(x: Union[int, A], a: Type[A]) -> None:

[builtins fixtures/isinstancelist.pyi]


[case testIssubclass]
Copy link
Member

Choose a reason for hiding this comment

The 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
Copy link
Member

Choose a reason for hiding this comment

The 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.
Also you probably don't need to check both access and assignment to instance variables in all tests, just checking assignment is enough.


class GoblinAmbusher(Goblin):
job: ClassVar[str] = 'Ranger'


Copy link
Member

Choose a reason for hiding this comment

The 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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, no error message I just accidentally left E:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But shouldn't this actually be an error? In the else branch, cls is not List and therefore not subscriptable. Or am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 E:; in this case I did mean for it to return an error, and it doesn't, and I forgot about it.

The reason it doesn't complain is that cls() is revealed as Any, since cls is defined as Type, which is translated to Type[Any]. So the entire test was useless.

Btw, I would prefer Type to be expanded to Type[object] rather than Type[Any], but I suppose it would be too inconsistent.


if issubclass(cls, MyIntList):
cls()[0] + 1

[builtins fixtures/isinstancelist.pyi]
[builtins fixtures/isinstancelist.pyi]
2 changes: 1 addition & 1 deletion typeshed
Submodule typeshed updated 54 files
+2 −2 .travis.yml
+1 −0 CONTRIBUTING.md
+5 −5 requirements-tests-py3.txt
+11 −11 stdlib/2/ConfigParser.pyi
+19 −19 stdlib/2/__builtin__.pyi
+20 −20 stdlib/2/array.pyi
+6 −9 stdlib/2/datetime.pyi
+5 −5 stdlib/2/pickle.pyi
+3 −1 stdlib/2/socket.pyi
+3 −3 stdlib/2/tempfile.pyi
+7 −7 stdlib/2and3/_bisect.pyi
+1 −1 stdlib/2and3/asynchat.pyi
+12 −10 stdlib/2and3/codecs.pyi
+3 −3 stdlib/2and3/fractions.pyi
+134 −0 stdlib/2and3/ftplib.pyi
+3 −3 stdlib/2and3/logging/__init__.pyi
+1 −1 stdlib/2and3/logging/handlers.pyi
+65 −28 stdlib/3.4/asyncio/events.pyi
+5 −5 stdlib/3.4/asyncio/locks.pyi
+8 −8 stdlib/3.4/asyncio/queues.pyi
+1 −1 stdlib/3.4/asyncio/tasks.pyi
+2 −2 stdlib/3.4/pathlib.pyi
+5 −4 stdlib/3/_posixsubprocess.pyi
+1 −1 stdlib/3/base64.pyi
+20 −17 stdlib/3/builtins.pyi
+0 −194 stdlib/3/codecs.pyi
+13 −12 stdlib/3/collections/__init__.pyi
+1 −1 stdlib/3/datetime.pyi
+1 −1 stdlib/3/email/message.pyi
+3 −2 stdlib/3/http/client.pyi
+2 −2 stdlib/3/shlex.pyi
+3 −1 stdlib/3/socket.pyi
+0 −2 stdlib/3/ssl.pyi
+87 −41 stdlib/3/subprocess.pyi
+78 −39 stdlib/3/tempfile.pyi
+10 −3 stdlib/3/typing.pyi
+29 −30 stdlib/3/unicodedata.pyi
+2 −2 third_party/2/concurrent/futures/__init__.pyi
+62 −55 third_party/2/werkzeug/wrappers.pyi
+2 −2 third_party/2and3/characteristic/__init__.pyi
+2 −2 third_party/2and3/mypy_extensions.pyi
+5 −5 third_party/3.6/click/core.pyi
+13 −13 third_party/3.6/click/decorators.pyi
+4 −4 third_party/3.6/click/termui.pyi
+11 −11 third_party/3.6/click/types.pyi
+3 −3 third_party/3.6/click/utils.pyi
+1 −1 third_party/3/dateutil/parser.pyi
+1 −1 third_party/3/dateutil/tz/_common.pyi
+1 −1 third_party/3/dateutil/tz/tz.pyi
+49 −48 third_party/3/itsdangerous.pyi
+2 −2 third_party/3/requests/adapters.pyi
+5 −4 third_party/3/requests/api.pyi
+19 −17 third_party/3/requests/sessions.pyi
+62 −55 third_party/3/werkzeug/wrappers.pyi
0