Skip to content

Commit 4141d20

Browse files
JukkaLilevkivskyi
JukkaL
authored and
ilevkivskyi
committed
Fixes to union simplification, isinstance and more (try 2) (#3086)
* Fixes to union simplification, isinstance and more (#3025) The main change is that unions containing Any are no longer simplified to just Any. Also union simplification now has a deterministic result unlike previously, when result could depend on the order of items in a union (this is true modulo remaining bugs). This required changes in various other places to keep the existing semantics, and resulted in some fixes to existing test cases. I also had to fix some tangentially related minor bugs that were triggered by the other changes. We generally don't have fully constructed TypeInfos so we can't do proper union simplification during semantic analysis. Just implement simple-minded simplification that deals with the cases we care about. Fixes #2978. Fixes #1914. * Fix strict optional and partial type special case and fix test * Fix lint * Make is_subtype and is_proper_subtype do promotion the same way Fixes #1850. * Drop reference to ErrorType (recently expunged) * Fix merge mistake * Fix another merge mistake * Update based on review * Add test case * Fix redundant TODO due to copy paste
1 parent 1316c4e commit 4141d20

22 files changed

Lines changed: 753 additions & 133 deletions

‎mypy/binder.py‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
from typing import (Dict, List, Set, Iterator, Union)
22
from contextlib import contextmanager
33

4-
from mypy.types import Type, AnyType, PartialType
4+
from mypy.types import Type, AnyType, PartialType, UnionType, NoneTyp
55
from mypy.nodes import (Key, Node, Expression, Var, RefExpr, SymbolTableNode)
66

77
from mypy.subtypes import is_subtype
@@ -227,7 +227,11 @@ def assign_type(self, expr: Expression,
227227
if (isinstance(self.most_recent_enclosing_type(expr, type), AnyType)
228228
and not restrict_any):
229229
pass
230-
elif isinstance(type, AnyType):
230+
elif (isinstance(type, AnyType)
231+
and not (isinstance(declared_type, UnionType)
232+
and any(isinstance(item, AnyType) for item in declared_type.items))):
233+
# Assigning an Any value doesn't affect the type to avoid false negatives, unless
234+
# there is an Any item in a declared union type.
231235
self.put(expr, declared_type)
232236
else:
233237
self.put(expr, type)

‎mypy/checker.py‎

Lines changed: 39 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
Type, AnyType, CallableType, FunctionLike, Overloaded, TupleType, TypedDictType,
3434
Instance, NoneTyp, strip_type, TypeType,
3535
UnionType, TypeVarId, TypeVarType, PartialType, DeletedType, UninhabitedType, TypeVarDef,
36-
true_only, false_only, function_type, is_named_instance
36+
true_only, false_only, function_type, is_named_instance, union_items
3737
)
3838
from mypy.sametypes import is_same_type, is_same_types
3939
from mypy.messages import MessageBuilder
@@ -817,44 +817,45 @@ def check_overlapping_op_methods(self,
817817
# of x in __radd__ would not be A, the methods could be
818818
# non-overlapping.
819819

820-
if isinstance(forward_type, CallableType):
821-
# TODO check argument kinds
822-
if len(forward_type.arg_types) < 1:
823-
# Not a valid operator method -- can't succeed anyway.
824-
return
820+
for forward_item in union_items(forward_type):
821+
if isinstance(forward_item, CallableType):
822+
# TODO check argument kinds
823+
if len(forward_item.arg_types) < 1:
824+
# Not a valid operator method -- can't succeed anyway.
825+
return
825826

826-
# Construct normalized function signatures corresponding to the
827-
# operator methods. The first argument is the left operand and the
828-
# second operand is the right argument -- we switch the order of
829-
# the arguments of the reverse method.
830-
forward_tweaked = CallableType(
831-
[forward_base, forward_type.arg_types[0]],
832-
[nodes.ARG_POS] * 2,
833-
[None] * 2,
834-
forward_type.ret_type,
835-
forward_type.fallback,
836-
name=forward_type.name)
837-
reverse_args = reverse_type.arg_types
838-
reverse_tweaked = CallableType(
839-
[reverse_args[1], reverse_args[0]],
840-
[nodes.ARG_POS] * 2,
841-
[None] * 2,
842-
reverse_type.ret_type,
843-
fallback=self.named_type('builtins.function'),
844-
name=reverse_type.name)
845-
846-
if is_unsafe_overlapping_signatures(forward_tweaked,
847-
reverse_tweaked):
848-
self.msg.operator_method_signatures_overlap(
849-
reverse_class.name(), reverse_name,
850-
forward_base.type.name(), forward_name, context)
851-
elif isinstance(forward_type, Overloaded):
852-
for item in forward_type.items():
853-
self.check_overlapping_op_methods(
854-
reverse_type, reverse_name, reverse_class,
855-
item, forward_name, forward_base, context)
856-
elif not isinstance(forward_type, AnyType):
857-
self.msg.forward_operator_not_callable(forward_name, context)
827+
# Construct normalized function signatures corresponding to the
828+
# operator methods. The first argument is the left operand and the
829+
# second operand is the right argument -- we switch the order of
830+
# the arguments of the reverse method.
831+
forward_tweaked = CallableType(
832+
[forward_base, forward_item.arg_types[0]],
833+
[nodes.ARG_POS] * 2,
834+
[None] * 2,
835+
forward_item.ret_type,
836+
forward_item.fallback,
837+
name=forward_item.name)
838+
reverse_args = reverse_type.arg_types
839+
reverse_tweaked = CallableType(
840+
[reverse_args[1], reverse_args[0]],
841+
[nodes.ARG_POS] * 2,
842+
[None] * 2,
843+
reverse_type.ret_type,
844+
fallback=self.named_type('builtins.function'),
845+
name=reverse_type.name)
846+
847+
if is_unsafe_overlapping_signatures(forward_tweaked,
848+
reverse_tweaked):
849+
self.msg.operator_method_signatures_overlap(
850+
reverse_class.name(), reverse_name,
851+
forward_base.type.name(), forward_name, context)
852+
elif isinstance(forward_item, Overloaded):
853+
for item in forward_item.items():
854+
self.check_overlapping_op_methods(
855+
reverse_type, reverse_name, reverse_class,
856+
item, forward_name, forward_base, context)
857+
elif not isinstance(forward_item, AnyType):
858+
self.msg.forward_operator_not_callable(forward_name, context)
858859

859860
def check_inplace_operator_method(self, defn: FuncBase) -> None:
860861
"""Check an inplace operator method such as __iadd__.

‎mypy/checkexpr.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
from mypy import messages
3434
from mypy.infer import infer_type_arguments, infer_function_type_arguments
3535
from mypy import join
36-
from mypy.meet import meet_simple
36+
from mypy.meet import narrow_declared_type
3737
from mypy.maptype import map_instance_to_supertype
3838
from mypy.subtypes import is_subtype, is_equivalent
3939
from mypy import applytype
@@ -2256,7 +2256,7 @@ def narrow_type_from_binder(self, expr: Expression, known_type: Type) -> Type:
22562256
if expr.literal >= LITERAL_TYPE:
22572257
restriction = self.chk.binder.get(expr)
22582258
if restriction:
2259-
ans = meet_simple(known_type, restriction)
2259+
ans = narrow_declared_type(known_type, restriction)
22602260
return ans
22612261
return known_type
22622262

‎mypy/erasetype.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ def visit_typeddict_type(self, t: TypedDictType) -> Type:
7373
return t.fallback.accept(self)
7474

7575
def visit_union_type(self, t: UnionType) -> Type:
76-
return AnyType() # XXX: return underlying type if only one?
76+
erased_items = [erase_type(item) for item in t.items]
77+
return UnionType.make_simplified_union(erased_items)
7778

7879
def visit_type_type(self, t: TypeType) -> Type:
7980
return TypeType(t.item.accept(self), line=t.line)

‎mypy/join.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
UninhabitedType, TypeType, true_or_false
1111
)
1212
from mypy.maptype import map_instance_to_supertype
13-
from mypy.subtypes import is_subtype, is_equivalent, is_subtype_ignoring_tvars
13+
from mypy.subtypes import is_subtype, is_equivalent, is_subtype_ignoring_tvars, is_proper_subtype
1414

1515
from mypy import experiments
1616

@@ -29,10 +29,10 @@ def join_simple(declaration: Type, s: Type, t: Type) -> Type:
2929
if isinstance(s, ErasedType):
3030
return t
3131

32-
if is_subtype(s, t):
32+
if is_proper_subtype(s, t):
3333
return t
3434

35-
if is_subtype(t, s):
35+
if is_proper_subtype(t, s):
3636
return s
3737

3838
if isinstance(declaration, UnionType):

‎mypy/meet.py‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,21 +25,26 @@ def meet_types(s: Type, t: Type) -> Type:
2525
return t.accept(TypeMeetVisitor(s))
2626

2727

28-
def meet_simple(s: Type, t: Type, default_right: bool = True) -> Type:
29-
if s == t:
30-
return s
31-
if isinstance(s, UnionType):
32-
return UnionType.make_simplified_union([meet_types(x, t) for x in s.items])
33-
elif not is_overlapping_types(s, t, use_promotions=True):
28+
def narrow_declared_type(declared: Type, narrowed: Type) -> Type:
29+
"""Return the declared type narrowed down to another type."""
30+
if declared == narrowed:
31+
return declared
32+
if isinstance(declared, UnionType):
33+
return UnionType.make_simplified_union([narrow_declared_type(x, narrowed)
34+
for x in declared.items])
35+
elif not is_overlapping_types(declared, narrowed, use_promotions=True):
3436
if experiments.STRICT_OPTIONAL:
3537
return UninhabitedType()
3638
else:
3739
return NoneTyp()
38-
else:
39-
if default_right:
40-
return t
41-
else:
42-
return s
40+
elif isinstance(narrowed, UnionType):
41+
return UnionType.make_simplified_union([narrow_declared_type(declared, x)
42+
for x in narrowed.items])
43+
elif isinstance(narrowed, AnyType):
44+
return narrowed
45+
elif isinstance(declared, (Instance, TupleType)):
46+
return meet_types(declared, narrowed)
47+
return narrowed
4348

4449

4550
def is_overlapping_types(t: Type, s: Type, use_promotions: bool = False) -> bool:
@@ -241,6 +246,10 @@ def visit_tuple_type(self, t: TupleType) -> Type:
241246
elif (isinstance(self.s, Instance) and
242247
self.s.type.fullname() == 'builtins.tuple' and self.s.args):
243248
return t.copy_modified(items=[meet_types(it, self.s.args[0]) for it in t.items])
249+
elif (isinstance(self.s, Instance) and t.fallback.type == self.s.type):
250+
# Uh oh, a broken named tuple type (https://github.com/python/mypy/issues/3016).
251+
# Do something reasonable until that bug is fixed.
252+
return t
244253
else:
245254
return self.default(self.s)
246255

0 commit comments

Comments
 (0)