Skip to content

Commit

Permalink
Handle empty tuples consistently. Fixes #136.
Browse files Browse the repository at this point in the history
It has now been documented (most recently in
https://bugs.python.org/issue37814) that the correct way to annotate an
empty tuple is `Tuple[()]`.

The runtime behavior of such an annotation is perhaps slightly
unexpected, in that `Tuple[()].__args__ == ((),)`, rather than `()` as
you might expect it would be (to parallel e.g. `Tuple[int, str].__args__
== (int, str)`).

On the other hand, this oddity is "useful" in that `Tuple.__args__ ==
()`, so this allows us to distinguish at runtime un-subscripted `Tuple`
(which I think must be interpreted as implicitly `Tuple[Any,...]` to
parallel e.g. `List` being equivalent to `List[Any]`?) from `Tuple[()]`.

This pull request makes MonkeyType's handling of tuple types consistent,
in that `Tuple` will round-trip to `Tuple` and `Tuple[()]` to
`Tuple[()]`, and in so doing fixes the crash with empty tuples in
rewriters exposed in #136. A traced empty tuple will now be correctly
represented in stubs as `Tuple[()]`.
  • Loading branch information
carljm committed Nov 7, 2019
1 parent 1581f30 commit fc53b69
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 6 deletions.
3 changes: 3 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ Changelog
master
------

* Fix crash with empty tuples. Thanks akayunov for the report, Christophe
Simonis for the simplest-case repro. Fixes #136.

* Don't add stringified annotations to type stubs. Thanks Łukasz Langa. Merge
of #148.

Expand Down
6 changes: 5 additions & 1 deletion monkeytype/encoding.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ def type_to_dict(typ: type) -> TypeDict:
}
elem_types = getattr(typ, '__args__', None)
if elem_types and is_generic(typ):
# empty typing.Tuple is weird; the spec says it should be Tuple[()],
# which results in __args__ of `((),)`
if elem_types == ((),):
elem_types = ()
d['elem_types'] = [type_to_dict(t) for t in elem_types]
return d

Expand Down Expand Up @@ -104,7 +108,7 @@ def type_from_dict(d: TypeDict) -> type:
f"is of type {type(typ)}, not type."
)
elem_type_dicts = d.get('elem_types')
if elem_type_dicts and is_generic(typ):
if elem_type_dicts is not None and is_generic(typ):
elem_types = tuple(type_from_dict(e) for e in elem_type_dicts)
# mypy complains that a value of type `type` isn't indexable. That's
# true, but we know typ is a subtype that is indexable. Even checking
Expand Down
10 changes: 6 additions & 4 deletions monkeytype/typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,6 @@ def get_type(obj):
val_type = shrink_types(get_type(v) for v in obj.values())
return Dict[key_type, val_type]
elif typ is tuple:
if not obj:
return Tuple
return Tuple[tuple(get_type(e) for e in obj)]
return typ

Expand Down Expand Up @@ -113,9 +111,13 @@ class TypeRewriter:
def _rewrite_container(self, cls, container):
if container.__module__ != "typing":
return container
if getattr(container, '__args__', None) is None:
args = getattr(container, '__args__', None)
if args is None:
return container
elems = tuple(self.rewrite(elem) for elem in container.__args__)
elif args == ((),): # special case of empty tuple `Tuple[()]`
elems = ()
else:
elems = tuple(self.rewrite(elem) for elem in container.__args__)
return cls[elems]

def rewrite_Dict(self, dct):
Expand Down
2 changes: 2 additions & 0 deletions tests/test_encoding.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ class TestTypeConversion:
Optional[str],
Set[int],
Tuple[int, str, str],
Tuple, # unparameterized tuple
Tuple[()], # empty tuple
Type[Outer],
Union[Outer.Inner, str, None],
# Nested generics
Expand Down
3 changes: 2 additions & 1 deletion tests/test_typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class TestGetType:
([1, True], List[Union[int, bool]]),
({'a': 1, 'b': 2}, Dict[str, int]),
({'a': 1, 2: 'b'}, Dict[Union[str, int], Union[str, int]]),
(tuple(), typing_Tuple),
(tuple(), typing_Tuple[()]),
(helper, Callable),
(lambda x: x, Callable),
(Dummy().an_instance_method, Callable),
Expand Down Expand Up @@ -128,6 +128,7 @@ class TestRemoveEmptyContainers:
(Dict[str, Union[List[str], List[Any]]], Dict[str, List[str]]),
(Union[List[Any], Set[Any]], Union[List[Any], Set[Any]]),
(Tuple, Tuple),
(typing_Tuple[()], typing_Tuple[()]),
],
)
def test_rewrite(self, typ, expected):
Expand Down

0 comments on commit fc53b69

Please sign in to comment.