From f13212e0803214b1a67786afabf7292e90c88ea4 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Mon, 24 Jan 2022 10:05:46 +0000 Subject: [PATCH 1/4] Fix two type inference bugs related to enum value attributes A few bugs in type operations were exposed by #11962. This fixes them. Fix #12051, but other use cases are affected as well. First, when we erase last known values in an union with multiple Instance types, make sure that the resulting union doesn't have duplicate erased types. The duplicate items weren't incorrect as such, but they could cause overly complex error messages and potentially slow type checking performance. Second, make the join of a union type and Any commutative. Previously the result dependend on the order of the operands, which was clearly incorrect. --- mypy/erasetype.py | 9 +++++++++ mypy/join.py | 6 +++--- test-data/unit/check-enum.test | 18 ++++++++++++++++++ 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/mypy/erasetype.py b/mypy/erasetype.py index 3301325eb0a1..f8466c15bf03 100644 --- a/mypy/erasetype.py +++ b/mypy/erasetype.py @@ -161,3 +161,12 @@ def visit_type_alias_type(self, t: TypeAliasType) -> Type: # Type aliases can't contain literal values, because they are # always constructed as explicit types. return t + + def visit_union_type(self, t: UnionType) -> Type: + new = super().visit_union_type(t) + new = get_proper_type(new) + if isinstance(new, UnionType): + from mypy.typeops import make_simplified_union + # Erasure can result in many duplicate items; remove them. + new = make_simplified_union(new.items) + return new diff --git a/mypy/join.py b/mypy/join.py index d298b495fdc5..161c65ea800c 100644 --- a/mypy/join.py +++ b/mypy/join.py @@ -175,15 +175,15 @@ def join_types(s: Type, t: Type, instance_joiner: Optional[InstanceJoiner] = Non s = mypy.typeops.true_or_false(s) t = mypy.typeops.true_or_false(t) + if isinstance(s, UnionType) and not isinstance(t, UnionType): + s, t = t, s + if isinstance(s, AnyType): return s if isinstance(s, ErasedType): return t - if isinstance(s, UnionType) and not isinstance(t, UnionType): - s, t = t, s - if isinstance(s, NoneType) and not isinstance(t, NoneType): s, t = t, s diff --git a/test-data/unit/check-enum.test b/test-data/unit/check-enum.test index 2af57da9cbdc..a7c8ef15e474 100644 --- a/test-data/unit/check-enum.test +++ b/test-data/unit/check-enum.test @@ -1868,3 +1868,21 @@ class WithOverload(enum.IntEnum): class SubWithOverload(WithOverload): # Should pass pass [builtins fixtures/tuple.pyi] + +[case testEnumtValueUnionSimplification] +from enum import IntEnum +from typing import Any + +class C(IntEnum): + X = 0 + Y = 1 + Z = 2 + +def f1(c: C) -> None: + x = {'x': c.value} + reveal_type(x) # N: Revealed type is "builtins.dict[builtins.str*, builtins.int]" + +def f2(c: C, a: Any) -> None: + x = {'x': c.value, 'y': a} + reveal_type(x) # N: Revealed type is "builtins.dict[builtins.str*, Any]" +[builtins fixtures/dict.pyi] From 6f412d12f8f285ffc0ae3e68d87939b42bce1bbc Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 25 Jan 2022 10:49:48 +0000 Subject: [PATCH 2/4] Revert change to join (and related test case) --- mypy/join.py | 6 +++--- test-data/unit/check-enum.test | 4 ---- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/mypy/join.py b/mypy/join.py index 161c65ea800c..d298b495fdc5 100644 --- a/mypy/join.py +++ b/mypy/join.py @@ -175,15 +175,15 @@ def join_types(s: Type, t: Type, instance_joiner: Optional[InstanceJoiner] = Non s = mypy.typeops.true_or_false(s) t = mypy.typeops.true_or_false(t) - if isinstance(s, UnionType) and not isinstance(t, UnionType): - s, t = t, s - if isinstance(s, AnyType): return s if isinstance(s, ErasedType): return t + if isinstance(s, UnionType) and not isinstance(t, UnionType): + s, t = t, s + if isinstance(s, NoneType) and not isinstance(t, NoneType): s, t = t, s diff --git a/test-data/unit/check-enum.test b/test-data/unit/check-enum.test index a7c8ef15e474..92982ef30b4e 100644 --- a/test-data/unit/check-enum.test +++ b/test-data/unit/check-enum.test @@ -1881,8 +1881,4 @@ class C(IntEnum): def f1(c: C) -> None: x = {'x': c.value} reveal_type(x) # N: Revealed type is "builtins.dict[builtins.str*, builtins.int]" - -def f2(c: C, a: Any) -> None: - x = {'x': c.value, 'y': a} - reveal_type(x) # N: Revealed type is "builtins.dict[builtins.str*, Any]" [builtins fixtures/dict.pyi] From 6cbb9789b10ecee6fd7111e8f3c82e6c5b63c631 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 25 Jan 2022 13:33:43 +0000 Subject: [PATCH 3/4] Only simplify duplicate instance types in unions --- mypy/erasetype.py | 38 ++++++++++++++++++++++++------- mypy/test/testtypes.py | 48 ++++++++++++++++++++++++++++++++++++++-- mypy/test/typefixture.py | 2 ++ 3 files changed, 78 insertions(+), 10 deletions(-) diff --git a/mypy/erasetype.py b/mypy/erasetype.py index f8466c15bf03..f33b4f3a95cc 100644 --- a/mypy/erasetype.py +++ b/mypy/erasetype.py @@ -1,10 +1,10 @@ -from typing import Optional, Container, Callable +from typing import Optional, Container, Callable, List, Dict, cast from mypy.types import ( Type, TypeVisitor, UnboundType, AnyType, NoneType, TypeVarId, Instance, TypeVarType, CallableType, TupleType, TypedDictType, UnionType, Overloaded, ErasedType, PartialType, DeletedType, TypeTranslator, UninhabitedType, TypeType, TypeOfAny, LiteralType, ProperType, - get_proper_type, TypeAliasType, ParamSpecType + get_proper_type, get_proper_types, TypeAliasType, ParamSpecType ) from mypy.nodes import ARG_STAR, ARG_STAR2 @@ -163,10 +163,32 @@ def visit_type_alias_type(self, t: TypeAliasType) -> Type: return t def visit_union_type(self, t: UnionType) -> Type: - new = super().visit_union_type(t) - new = get_proper_type(new) - if isinstance(new, UnionType): - from mypy.typeops import make_simplified_union - # Erasure can result in many duplicate items; remove them. - new = make_simplified_union(new.items) + new = cast(UnionType, super().visit_union_type(t)) + # Erasure can result in many duplicate items; merge them. + # Call make_simplified_union only on lists of instance types + # that all have the same fullname, to avoid simplifying too + # much. + instances = [item for item in new.items + if isinstance(get_proper_type(item), Instance)] + # Avoid merge in simple cases such as optional types. + if len(instances) > 1: + instances_by_name: Dict[str, List[Instance]] = {} + new_items = get_proper_types(new.items) + for item in new_items: + if isinstance(item, Instance) and not item.args: + instances_by_name.setdefault(item.type.fullname, []).append(item) + merged: List[Type] = [] + for item in new_items: + if isinstance(item, Instance) and not item.args: + types = instances_by_name.get(item.type.fullname) + if types is not None: + if len(types) == 1: + merged.append(item) + else: + from mypy.typeops import make_simplified_union + merged.append(make_simplified_union(types)) + del instances_by_name[item.type.fullname] + else: + merged.append(item) + return UnionType.make_union(merged) return new diff --git a/mypy/test/testtypes.py b/mypy/test/testtypes.py index 0d37fe795bbc..6af1c18145cf 100644 --- a/mypy/test/testtypes.py +++ b/mypy/test/testtypes.py @@ -3,7 +3,7 @@ from typing import List, Tuple from mypy.test.helpers import Suite, assert_equal, assert_type, skip -from mypy.erasetype import erase_type +from mypy.erasetype import erase_type, remove_instance_last_known_values from mypy.expandtype import expand_type from mypy.join import join_types, join_simple from mypy.meet import meet_types, narrow_declared_type @@ -11,7 +11,8 @@ from mypy.indirection import TypeIndirectionVisitor from mypy.types import ( UnboundType, AnyType, CallableType, TupleType, TypeVarType, Type, Instance, NoneType, - Overloaded, TypeType, UnionType, UninhabitedType, TypeVarId, TypeOfAny, get_proper_type + Overloaded, TypeType, UnionType, UninhabitedType, TypeVarId, TypeOfAny, ProperType, + get_proper_type ) from mypy.nodes import ARG_POS, ARG_OPT, ARG_STAR, ARG_STAR2, CONTRAVARIANT, INVARIANT, COVARIANT from mypy.subtypes import is_subtype, is_more_precise, is_proper_subtype @@ -1092,3 +1093,46 @@ def assert_simple_is_same(self, s: Type, t: Type, expected: bool, strict: bool) '({} == {}) is {{}} ({{}} expected)'.format(s, t)) assert_equal(hash(s) == hash(t), expected, '(hash({}) == hash({}) is {{}} ({{}} expected)'.format(s, t)) + + +class RemoveLastKnownValueSuite(Suite): + def setUp(self) -> None: + self.fx = TypeFixture() + + def test_optional(self) -> None: + t = UnionType.make_union([self.fx.a, self.fx.nonet]) + self.assert_union_result(t, [self.fx.a, self.fx.nonet]) + + def test_two_instances(self) -> None: + t = UnionType.make_union([self.fx.a, self.fx.b]) + self.assert_union_result(t, [self.fx.a, self.fx.b]) + + def test_multiple_same_instances(self) -> None: + t = UnionType.make_union([self.fx.a, self.fx.a]) + assert remove_instance_last_known_values(t) == self.fx.a + t = UnionType.make_union([self.fx.a, self.fx.a, self.fx.b]) + self.assert_union_result(t, [self.fx.a, self.fx.b]) + t = UnionType.make_union([self.fx.a, self.fx.nonet, self.fx.a, self.fx.b]) + self.assert_union_result(t, [self.fx.a, self.fx.nonet, self.fx.b]) + + def test_single_last_known_value(self) -> None: + t = UnionType.make_union([self.fx.lit1_inst, self.fx.nonet]) + self.assert_union_result(t, [self.fx.a, self.fx.nonet]) + + def test_last_known_values_with_merge(self) -> None: + t = UnionType.make_union([self.fx.lit1_inst, self.fx.lit2_inst, self.fx.lit4_inst]) + assert remove_instance_last_known_values(t) == self.fx.a + t = UnionType.make_union([self.fx.lit1_inst, + self.fx.b, + self.fx.lit2_inst, + self.fx.lit4_inst]) + self.assert_union_result(t, [self.fx.a, self.fx.b]) + + def test_generics(self) -> None: + t = UnionType.make_union([self.fx.ga, self.fx.gb]) + self.assert_union_result(t, [self.fx.ga, self.fx.gb]) + + def assert_union_result(self, t: ProperType, expected: List[Type]) -> None: + t2 = remove_instance_last_known_values(t) + assert type(t2) is UnionType + assert t2.items == expected diff --git a/mypy/test/typefixture.py b/mypy/test/typefixture.py index 1a5dd8164136..f1cb62065eee 100644 --- a/mypy/test/typefixture.py +++ b/mypy/test/typefixture.py @@ -157,9 +157,11 @@ def make_type_var(name: str, id: int, values: List[Type], upper_bound: Type, self.lit1 = LiteralType(1, self.a) self.lit2 = LiteralType(2, self.a) self.lit3 = LiteralType("foo", self.d) + self.lit4 = LiteralType(3, self.a) self.lit1_inst = Instance(self.ai, [], last_known_value=self.lit1) self.lit2_inst = Instance(self.ai, [], last_known_value=self.lit2) self.lit3_inst = Instance(self.di, [], last_known_value=self.lit3) + self.lit4_inst = Instance(self.ai, [], last_known_value=self.lit4) self.type_a = TypeType.make_normalized(self.a) self.type_b = TypeType.make_normalized(self.b) From 6e147e702a4c961ff513723ff2be1d14d0830d80 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 25 Jan 2022 14:18:27 +0000 Subject: [PATCH 4/4] Update mypy/test/typefixture.py Co-authored-by: Nikita Sobolev --- mypy/test/typefixture.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/test/typefixture.py b/mypy/test/typefixture.py index f1cb62065eee..7d4faeccf432 100644 --- a/mypy/test/typefixture.py +++ b/mypy/test/typefixture.py @@ -157,7 +157,7 @@ def make_type_var(name: str, id: int, values: List[Type], upper_bound: Type, self.lit1 = LiteralType(1, self.a) self.lit2 = LiteralType(2, self.a) self.lit3 = LiteralType("foo", self.d) - self.lit4 = LiteralType(3, self.a) + self.lit4 = LiteralType(4, self.a) self.lit1_inst = Instance(self.ai, [], last_known_value=self.lit1) self.lit2_inst = Instance(self.ai, [], last_known_value=self.lit2) self.lit3_inst = Instance(self.di, [], last_known_value=self.lit3)