From f0d83f0278022987a5a2c01d0fa4663d4878da68 Mon Sep 17 00:00:00 2001 From: Michael Lee Date: Sat, 18 Jan 2020 19:09:39 -0800 Subject: [PATCH 1/4] Make isinstance/issubclass generate ad-hoc intersections This diff makes `isinstance(...)` and `issubclass(...)` try generating ad-hoc intersections of Instances when possible. For example, we previously concluded the if-branch is unreachable in the following program. This PR makes mypy infer an ad-hoc intersection instead. class A: pass class B: pass x: A if isinstance(x, B): reveal_type(x) # N: Revealed type is 'test.' If you try doing an `isinstance(...)` that legitimately is impossible due to conflicting method signatures or MRO issues, we continue to declare the branch unreachable. Passing in the `--warn-unreachable` flag will now also report an error about this: # flags: --warn-unreachable x: str # E: Subclass of "str" and "bytes" cannot exist: would have # incompatible method signatures if isinstance(x, bytes): reveal_type(x) # E: Statement is unreachable This error message has the same limitations as the other `--warn-unreachable` ones: we suppress them if the isinstance check is inside a function using TypeVars with multiple values. However, we *do* end up always inferring an intersection type when possible -- that logic is never suppressed. I initially thought we might have to suppress the new logic as well (see https://github.com/python/mypy/issues/3603#issuecomment-506996850), but it turns out this is a non-issue in practice once you add in the check that disallows impossible intersections. For example, when I tried running this PR on the larger of our two internal codebases, I found about 25 distinct errors, all of which were legitimate and unrelated to the problem discussed in the PR. (And if we don't suppress the extra error message, we get about 100-120 errors, mostly due to tests repeatdly doing `result = blah()` followed by `assert isinstance(result, X)` where X keeps changing.) --- mypy/checker.py | 151 ++++++++++-- mypy/messages.py | 23 +- mypy/nodes.py | 4 + mypy/semanal.py | 4 +- test-data/unit/check-isinstance.test | 299 ++++++++++++++++++++++- test-data/unit/check-protocols.test | 4 +- test-data/unit/check-typevar-values.test | 25 +- test-data/unit/deps.test | 16 ++ test-data/unit/pythoneval.test | 14 ++ 9 files changed, 500 insertions(+), 40 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 534c4bed24b9..90c675d328e1 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -38,7 +38,7 @@ get_proper_types, is_literal_type, TypeAliasType) from mypy.sametypes import is_same_type from mypy.messages import ( - MessageBuilder, make_inferred_type_note, append_invariance_notes, + MessageBuilder, make_inferred_type_note, append_invariance_notes, pretty_seq, format_type, format_type_bare, format_type_distinctly, SUGGESTED_TEST_FIXTURES ) import mypy.checkexpr @@ -63,7 +63,7 @@ from mypy.maptype import map_instance_to_supertype from mypy.typevars import fill_typevars, has_no_typevars, fill_typevars_with_any from mypy.semanal import set_callable_name, refers_to_fullname -from mypy.mro import calculate_mro +from mypy.mro import calculate_mro, MroError from mypy.erasetype import erase_typevars, remove_instance_last_known_values, erase_type from mypy.expandtype import expand_type, expand_type_by_instance from mypy.visitor import NodeVisitor @@ -1963,13 +1963,15 @@ def visit_block(self, b: Block) -> None: return for s in b.body: if self.binder.is_unreachable(): - if (self.options.warn_unreachable - and not self.binder.is_unreachable_warning_suppressed() - and not self.is_raising_or_empty(s)): + if self.should_report_unreachable_issues() and not self.is_raising_or_empty(s): self.msg.unreachable_statement(s) break self.accept(s) + def should_report_unreachable_issues(self) -> bool: + return (self.options.warn_unreachable + and not self.binder.is_unreachable_warning_suppressed()) + def is_raising_or_empty(self, s: Statement) -> bool: """Returns 'true' if the given statement either throws an error of some kind or is a no-op. @@ -3636,6 +3638,78 @@ def visit_continue_stmt(self, s: ContinueStmt) -> None: self.binder.handle_continue() return None + def make_fake_typeinfo(self, + curr_module_fullname: str, + class_gen_name: str, + class_short_name: str, + bases: List[Instance], + ) -> Tuple[ClassDef, TypeInfo]: + # Build the fake ClassDef and TypeInfo together. + # The ClassDef is full of lies and doesn't actually contain a body. + # Use format_bare to generate a nice name for error messages. + # We skip fully filling out a handful of TypeInfo fields because they + # should be irrelevant for a generated type like this: + # is_protocol, protocol_members, is_abstract + cdef = ClassDef(class_short_name, Block([])) + cdef.fullname = curr_module_fullname + '.' + class_gen_name + info = TypeInfo(SymbolTable(), cdef, curr_module_fullname) + cdef.info = info + info.bases = bases + calculate_mro(info) + info.calculate_metaclass_type() + return cdef, info + + def intersect_instances(self, + instances: Sequence[Instance], + ctx: Context, + ) -> Optional[Instance]: + curr_module = self.scope.stack[0] + assert isinstance(curr_module, MypyFile) + + base_classes = [] + formatted_names = [] + for inst in instances: + expanded = [inst] + if inst.type.is_intersection: + expanded = inst.type.bases + + for expanded_inst in expanded: + base_classes.append(expanded_inst) + formatted_names.append(format_type_bare(expanded_inst)) + + pretty_names_list = pretty_seq(format_type_distinctly(*base_classes, bare=True), "and") + short_name = ''.format(pretty_names_list) + full_name = gen_unique_name(short_name, curr_module.names) + + old_msg = self.msg + new_msg = self.msg.clean_copy() + self.msg = new_msg + try: + cdef, info = self.make_fake_typeinfo( + curr_module.fullname, + full_name, + short_name, + base_classes, + ) + self.check_multiple_inheritance(info) + info.is_intersection = True + except MroError: + if self.should_report_unreachable_issues(): + old_msg.impossible_intersection( + pretty_names_list, "inconsistent method resolution order", ctx) + return None + finally: + self.msg = old_msg + + if new_msg.is_errors(): + if self.should_report_unreachable_issues(): + self.msg.impossible_intersection( + pretty_names_list, "incompatible method signatures", ctx) + return None + + curr_module.names[full_name] = SymbolTableNode(GDEF, info) + return Instance(info, []) + def intersect_instance_callable(self, typ: Instance, callable_type: CallableType) -> Instance: """Creates a fake type that represents the intersection of an Instance and a CallableType. @@ -3650,20 +3724,9 @@ def intersect_instance_callable(self, typ: Instance, callable_type: CallableType gen_name = gen_unique_name("".format(typ.type.name), cur_module.names) - # Build the fake ClassDef and TypeInfo together. - # The ClassDef is full of lies and doesn't actually contain a body. - # Use format_bare to generate a nice name for error messages. - # We skip fully filling out a handful of TypeInfo fields because they - # should be irrelevant for a generated type like this: - # is_protocol, protocol_members, is_abstract + # Synthesize a fake TypeInfo short_name = format_type_bare(typ) - cdef = ClassDef(short_name, Block([])) - cdef.fullname = cur_module.fullname + '.' + gen_name - info = TypeInfo(SymbolTable(), cdef, cur_module.fullname) - cdef.info = info - info.bases = [typ] - calculate_mro(info) - info.calculate_metaclass_type() + cdef, info = self.make_fake_typeinfo(cur_module.fullname, gen_name, short_name, [typ]) # Build up a fake FuncDef so we can populate the symbol table. func_def = FuncDef('__call__', [], Block([]), callable_type) @@ -3828,9 +3891,11 @@ def find_isinstance_check_helper(self, node: Expression) -> Tuple[TypeMap, TypeM return {}, {} expr = node.args[0] if literal(expr) == LITERAL_TYPE: - vartype = type_map[expr] - type = get_isinstance_type(node.args[1], type_map) - return conditional_type_map(expr, vartype, type) + return self.conditional_type_map_with_intersection( + expr, + type_map[expr], + get_isinstance_type(node.args[1], type_map), + ) elif refers_to_fullname(node.callee, 'builtins.issubclass'): if len(node.args) != 2: # the error will be reported elsewhere return {}, {} @@ -4309,6 +4374,10 @@ def refine_identity_comparison_expression(self, if enum_name is not None: expr_type = try_expanding_enum_to_union(expr_type, enum_name) + + # We intentionally use 'conditional_type_map' directly here instead of + # 'self.conditional_type_map_with_intersection': we only compute ad-hoc + # intersections when working with pure instances. partial_type_maps.append(conditional_type_map(expr, expr_type, target_type)) return reduce_conditional_maps(partial_type_maps) @@ -4726,10 +4795,48 @@ def infer_issubclass_maps(self, node: CallExpr, # Any other object whose type we don't know precisely # for example, Any or a custom metaclass. return {}, {} # unknown type - yes_map, no_map = conditional_type_map(expr, vartype, type) + yes_map, no_map = self.conditional_type_map_with_intersection(expr, vartype, type) yes_map, no_map = map(convert_to_typetype, (yes_map, no_map)) return yes_map, no_map + def conditional_type_map_with_intersection(self, + expr: Expression, + expr_type: Type, + type_ranges: Optional[List[TypeRange]], + ) -> Tuple[TypeMap, TypeMap]: + yes_map, no_map = conditional_type_map(expr, expr_type, type_ranges) + + if yes_map is not None or type_ranges is None: + return yes_map, no_map + + # If we couldn't infer anything useful, try again by trying to compute an intersection + expr_type = get_proper_type(expr_type) + if isinstance(expr_type, UnionType): + possible_expr_types = get_proper_types(expr_type.relevant_items()) + else: + possible_expr_types = [expr_type] + + possible_target_types = [] + for tr in type_ranges: + item = get_proper_type(tr.item) + if not isinstance(item, Instance) or tr.is_upper_bound: + return yes_map, no_map + possible_target_types.append(item) + + out = [] + for v in possible_expr_types: + if not isinstance(v, Instance): + return yes_map, no_map + for t in possible_target_types: + intersection = self.intersect_instances([v, t], expr) + if intersection is None: + continue + out.append(intersection) + if len(out) == 0: + return None, {} + new_yes_type = make_simplified_union(out) + return {expr: new_yes_type}, {} + def conditional_type_map(expr: Expression, current_type: Optional[Type], diff --git a/mypy/messages.py b/mypy/messages.py index 011527b00fd7..8c4c3ba4d5eb 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -290,7 +290,11 @@ def has_no_attr(self, if matches: self.fail( '{} has no attribute "{}"; maybe {}?{}'.format( - format_type(original_type), member, pretty_or(matches), extra), + format_type(original_type), + member, + pretty_seq(matches, "or"), + extra, + ), context, code=codes.ATTR_DEFINED) failed = True @@ -623,7 +627,7 @@ def unexpected_keyword_argument(self, callee: CallableType, name: str, arg_type: if not matches: matches = best_matches(name, not_matching_type_args) if matches: - msg += "; did you mean {}?".format(pretty_or(matches[:3])) + msg += "; did you mean {}?".format(pretty_seq(matches[:3], "or")) self.fail(msg, context, code=codes.CALL_ARG) module = find_defining_module(self.modules, callee) if module: @@ -1263,6 +1267,14 @@ def redundant_condition_in_assert(self, truthiness: bool, context: Context) -> N def redundant_expr(self, description: str, truthiness: bool, context: Context) -> None: self.fail("{} is always {}".format(description, str(truthiness).lower()), context) + def impossible_intersection(self, + formatted_base_class_list: str, + reason: str, + context: Context, + ) -> None: + template = "Subclass of {} cannot exist: would have {}" + self.fail(template.format(formatted_base_class_list, reason), context) + def report_protocol_problems(self, subtype: Union[Instance, TupleType, TypedDictType], supertype: Instance, @@ -1995,13 +2007,14 @@ def best_matches(current: str, options: Iterable[str]) -> List[str]: reverse=True, key=lambda v: (ratios[v], v)) -def pretty_or(args: List[str]) -> str: +def pretty_seq(args: Sequence[str], conjunction: str) -> str: quoted = ['"' + a + '"' for a in args] if len(quoted) == 1: return quoted[0] if len(quoted) == 2: - return "{} or {}".format(quoted[0], quoted[1]) - return ", ".join(quoted[:-1]) + ", or " + quoted[-1] + return "{} {} {}".format(quoted[0], conjunction, quoted[1]) + last_sep = ", " + conjunction + " " + return ", ".join(quoted[:-1]) + last_sep + quoted[-1] def append_invariance_notes(notes: List[str], arg_type: Instance, diff --git a/mypy/nodes.py b/mypy/nodes.py index b2c7769580bd..e24a8887dd01 100644 --- a/mypy/nodes.py +++ b/mypy/nodes.py @@ -2379,6 +2379,9 @@ class is generic then it will be a type constructor of higher kind. # Is this a newtype type? is_newtype = False + # Is this a synthesized intersection type? + is_intersection = False + # This is a dictionary that will be serialized and un-serialized as is. # It is useful for plugins to add their data to save in the cache. metadata = None # type: Dict[str, JsonDict] @@ -2386,6 +2389,7 @@ class is generic then it will be a type constructor of higher kind. FLAGS = [ 'is_abstract', 'is_enum', 'fallback_to_any', 'is_named_tuple', 'is_newtype', 'is_protocol', 'runtime_protocol', 'is_final', + 'is_intersection', ] # type: Final[List[str]] def __init__(self, names: 'SymbolTable', defn: ClassDef, module_name: str) -> None: diff --git a/mypy/semanal.py b/mypy/semanal.py index 42770e1a0389..7e61bce9716b 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -81,7 +81,7 @@ from mypy.typevars import fill_typevars from mypy.visitor import NodeVisitor from mypy.errors import Errors, report_internal_error -from mypy.messages import best_matches, MessageBuilder, pretty_or, SUGGESTED_TEST_FIXTURES +from mypy.messages import best_matches, MessageBuilder, pretty_seq, SUGGESTED_TEST_FIXTURES from mypy.errorcodes import ErrorCode from mypy import message_registry, errorcodes as codes from mypy.types import ( @@ -1802,7 +1802,7 @@ def report_missing_module_attribute(self, import_id: str, source_id: str, import alternatives = set(module.names.keys()).difference({source_id}) matches = best_matches(source_id, alternatives)[:3] if matches: - suggestion = "; maybe {}?".format(pretty_or(matches)) + suggestion = "; maybe {}?".format(pretty_seq(matches, "or")) message += "{}".format(suggestion) self.fail(message, context, code=codes.ATTR_DEFINED) self.add_unknown_imported_symbol(imported_id, context) diff --git a/test-data/unit/check-isinstance.test b/test-data/unit/check-isinstance.test index 09c174a5d41a..953178bc84e9 100644 --- a/test-data/unit/check-isinstance.test +++ b/test-data/unit/check-isinstance.test @@ -1236,7 +1236,10 @@ else: [builtins fixtures/isinstancelist.pyi] [case testIsinstanceMultiAndSpecialCase] -class A: pass +class A: + # Ensure A.__add__ and int.__add__ are different to + # force 'isinstance(y, int)' checks below to never succeed. + def __add__(self, other: A) -> A: pass class B(A): flag = 1 @@ -1357,7 +1360,7 @@ class B: pass x = B() if isinstance(x, A): - reveal_type(x) # unreachable + reveal_type(x) # N: Revealed type is '__main__.' else: reveal_type(x) # N: Revealed type is '__main__.B' reveal_type(x) # N: Revealed type is '__main__.B' @@ -2158,7 +2161,7 @@ def foo2(x: Optional[str]) -> None: if x is None: reveal_type(x) # N: Revealed type is 'None' elif isinstance(x, A): - reveal_type(x) + reveal_type(x) # N: Revealed type is '__main__.' else: reveal_type(x) # N: Revealed type is 'builtins.str' [builtins fixtures/isinstance.pyi] @@ -2182,8 +2185,7 @@ def foo2(x: Optional[str]) -> None: if x is None: reveal_type(x) # N: Revealed type is 'None' elif isinstance(x, A): - # Mypy should, however, be able to skip impossible cases - reveal_type(x) + reveal_type(x) # N: Revealed type is '__main__.' else: reveal_type(x) # N: Revealed type is 'builtins.str' [builtins fixtures/isinstance.pyi] @@ -2284,3 +2286,290 @@ var = 'some string' if isinstance(var, *(str, int)): # E: Too many arguments for "isinstance" pass [builtins fixtures/isinstancelist.pyi] + +[case testIsInstanceAdHocIntersectionBasic] +class A: + def f1(self) -> int: ... +class B: + def f2(self) -> int: ... +class C: + def f3(self) -> int: ... + +x: A +if isinstance(x, B): + reveal_type(x) # N: Revealed type is '__main__.' + if isinstance(x, C): + reveal_type(x) # N: Revealed type is '__main__.' + reveal_type(x.f1()) # N: Revealed type is 'builtins.int' + reveal_type(x.f2()) # N: Revealed type is 'builtins.int' + reveal_type(x.f3()) # N: Revealed type is 'builtins.int' + x.bad() # E: "" has no attribute "bad" + else: + reveal_type(x) # N: Revealed type is '__main__.' +else: + reveal_type(x) # N: Revealed type is '__main__.A' +[builtins fixtures/isinstance.pyi] + +[case testIsInstanceAdHocIntersectionRepeatedChecks] +# flags: --warn-unreachable + +class A: pass +class B: pass + +x: A +if isinstance(x, B): + reveal_type(x) # N: Revealed type is '__main__.' + if isinstance(x, A): + reveal_type(x) # N: Revealed type is '__main__.' + if isinstance(x, B): + reveal_type(x) # N: Revealed type is '__main__.' +[builtins fixtures/isinstance.pyi] + +[case testIsInstanceAdHocIntersectionIncompatibleClasses] +# flags: --warn-unreachable +class A: + def f(self) -> int: ... +class B: + def f(self) -> str: ... +class C: + def f(self) -> str: ... + +class Example(A, B): pass # E: Definition of "f" in base class "A" is incompatible with definition in base class "B" +x: A +if isinstance(x, B): # E: Subclass of "A" and "B" cannot exist: would have incompatible method signatures + reveal_type(x) # E: Statement is unreachable +else: + reveal_type(x) # N: Revealed type is '__main__.A' + +y: C +if isinstance(y, B): + reveal_type(y) # N: Revealed type is '__main__.' + if isinstance(y, A): # E: Subclass of "C", "B", and "A" cannot exist: would have incompatible method signatures + reveal_type(y) # E: Statement is unreachable +[builtins fixtures/isinstance.pyi] + +[case testIsInstanceAdHocIntersectionGenerics] +# flags: --warn-unreachable +from typing import Generic, TypeVar + +class Parent: pass +class Child(Parent): pass + +T = TypeVar('T') +class A(Generic[T]): + def f(self) -> T: ... +class B: + def f(self) -> Parent: ... + +x: A[int] +if isinstance(x, B): # E: Subclass of "A[int]" and "B" cannot exist: would have incompatible method signatures + reveal_type(x) # E: Statement is unreachable +else: + reveal_type(x) # N: Revealed type is '__main__.A[builtins.int]' + +y: A[Parent] +if isinstance(y, B): + reveal_type(y) # N: Revealed type is '__main__.' + reveal_type(y.f()) # N: Revealed type is '__main__.Parent*' +else: + reveal_type(y) # N: Revealed type is '__main__.A[__main__.Parent]' + +z: A[Child] +if isinstance(z, B): + reveal_type(z) # N: Revealed type is '__main__.' + reveal_type(z.f()) # N: Revealed type is '__main__.Child*' +else: + reveal_type(z) # N: Revealed type is '__main__.A[__main__.Child]' +[builtins fixtures/isinstance.pyi] + +[case testIsInstanceAdHocIntersectionGenericsWithValues] +# flags: --warn-unreachable +from typing import TypeVar + +class A: + attr: int +class B: + attr: int +class C: + attr: str + +T1 = TypeVar('T1', A, B) +def f1(x: T1) -> T1: + if isinstance(x, A): + reveal_type(x) # N: Revealed type is '__main__.A*' \ + # N: Revealed type is '__main__.' + if isinstance(x, B): + reveal_type(x) # N: Revealed type is '__main__.' \ + # N: Revealed type is '__main__.' + else: + reveal_type(x) # N: Revealed type is '__main__.A*' + else: + reveal_type(x) # N: Revealed type is '__main__.B*' + return x + +T2 = TypeVar('T2', B, C) +def f2(x: T2) -> T2: + if isinstance(x, B): + reveal_type(x) # N: Revealed type is '__main__.B*' + # Note: even though --warn-unreachable is set, we don't report + # errors for the below: we don't yet have a way of filtering out + # reachability errors that occur for only one variation of the + # TypeVar yet. + if isinstance(x, C): + reveal_type(x) + else: + reveal_type(x) # N: Revealed type is '__main__.B*' + else: + reveal_type(x) # N: Revealed type is '__main__.C*' + return x +[builtins fixtures/isinstance.pyi] + +[case testIsInstanceAdHocIntersectionGenericsWithValuesDirectReturn] +# flags: --warn-unreachable +from typing import TypeVar + +class A: + attr: int +class B: + attr: int +class C: + attr: str + +T1 = TypeVar('T1', A, B) +def f1(x: T1) -> T1: + if isinstance(x, A): + # The error message is confusing, but we indeed do run into problems if + # 'x' is a subclass of A and B + return A() # E: Incompatible return value type (got "A", expected "B") + else: + return B() + +T2 = TypeVar('T2', B, C) +def f2(x: T2) -> T2: + if isinstance(x, B): + # In contrast, it's impossible for a subclass of "B" and "C" to + # exist, so this is fine + return B() + else: + return C() +[builtins fixtures/isinstance.pyi] + +[case testIsInstanceAdHocIntersectionUsage] +# flags: --warn-unreachable +class A: pass +class B: pass +class Concrete(A, B): pass + +def accept_a(a: A) -> None: pass +def accept_b(a: B) -> None: pass +def accept_concrete(c: Concrete) -> None: pass + +x: A +if isinstance(x, B): + var = x + reveal_type(var) # N: Revealed type is '__main__.' + accept_a(var) + accept_b(var) + accept_concrete(var) # E: Argument 1 to "accept_concrete" has incompatible type ""; expected "Concrete" +[builtins fixtures/isinstance.pyi] + +[case testIsInstanceAdHocIntersectionReinfer] +# flags: --warn-unreachable +class A: pass +class B: pass + +x: A +assert isinstance(x, B) +reveal_type(x) # N: Revealed type is '__main__.' + +y: A +assert isinstance(y, B) +reveal_type(y) # N: Revealed type is '__main__.1' + +x = y +reveal_type(x) # N: Revealed type is '__main__.1' +[builtins fixtures/isinstance.pyi] + +[case testIsInstanceAdHocIntersectionWithUnions] +# flags: --warn-unreachable +from typing import Type, Union +class A: pass +class B: pass +class C: pass +class D: pass + +v1: A +if isinstance(v1, (B, C)): + reveal_type(v1) # N: Revealed type is 'Union[__main__., __main__.]' + +v2: Union[A, B] +if isinstance(v2, C): + reveal_type(v2) # N: Revealed type is 'Union[__main__.1, __main__.]' + +v3: Union[A, B] +if isinstance(v3, (C, D)): + reveal_type(v3) # N: Revealed type is 'Union[__main__.2, __main__., __main__.1, __main__.]' +[builtins fixtures/isinstance.pyi] + +[case testIsInstanceAdHocIntersectionSameNames] +# flags: --warn-unreachable +from foo import A as A2 +class A: pass + +x: A +if isinstance(x, A2): + reveal_type(x) # N: Revealed type is '__main__.' + +[file foo.py] +class A: pass +[builtins fixtures/isinstance.pyi] + +[case testIsInstanceAdHocIntersectionBadMro] +# flags: --warn-unreachable +class X: pass +class Y: pass +class A(X, Y): pass +class B(Y, X): pass + +foo: A +if isinstance(foo, B): # E: Subclass of "A" and "B" cannot exist: would have inconsistent method resolution order + reveal_type(foo) # E: Statement is unreachable +[builtins fixtures/isinstance.pyi] + +[case testIsInstanceAdHocIntersectionAmbiguousClass] +# flags: --warn-unreachable +from typing import Any + +class Concrete: + x: int +class Ambiguous: + x: Any + +# We bias towards assuming these two classes could be overlapping +foo: Concrete +if isinstance(foo, Ambiguous): + reveal_type(foo) # N: Revealed type is '__main__.' + reveal_type(foo.x) # N: Revealed type is 'builtins.int' +[builtins fixtures/isinstance.pyi] + +[case testIsSubclassAdHocIntersection] +# flags: --warn-unreachable +from typing import Type + +class A: + x: int +class B: + x: int +class C: + x: str + +x: Type[A] +if issubclass(x, B): + reveal_type(x) # N: Revealed type is 'Type[__main__.]' + if issubclass(x, C): # E: Subclass of "A", "B", and "C" cannot exist: would have incompatible method signatures + reveal_type(x) # E: Statement is unreachable + else: + reveal_type(x) # N: Revealed type is 'Type[__main__.]' +else: + reveal_type(x) # N: Revealed type is 'Type[__main__.A]' +[builtins fixtures/isinstance.pyi] diff --git a/test-data/unit/check-protocols.test b/test-data/unit/check-protocols.test index 0081394541b0..8773e91d0840 100644 --- a/test-data/unit/check-protocols.test +++ b/test-data/unit/check-protocols.test @@ -1564,7 +1564,7 @@ if isinstance(c1i, P1): else: reveal_type(c1i) # Unreachable if isinstance(c1i, P): - reveal_type(c1i) # Unreachable + reveal_type(c1i) # N: Revealed type is '__main__.' else: reveal_type(c1i) # N: Revealed type is '__main__.C1[builtins.int]' @@ -1576,7 +1576,7 @@ else: c2: C2 if isinstance(c2, P): - reveal_type(c2) # Unreachable + reveal_type(c2) # N: Revealed type is '__main__.' else: reveal_type(c2) # N: Revealed type is '__main__.C2' diff --git a/test-data/unit/check-typevar-values.test b/test-data/unit/check-typevar-values.test index d70f7b240333..72993261a22f 100644 --- a/test-data/unit/check-typevar-values.test +++ b/test-data/unit/check-typevar-values.test @@ -178,20 +178,37 @@ def f(x: T) -> T: [out] [case testIsinstanceWithUserDefinedTypeAndTypeVarValues] +# flags: --warn-unreachable from typing import TypeVar class A: pass class B: pass -T = TypeVar('T', A, B) -def f(x: T) -> None: +T1 = TypeVar('T1', A, B) +def f1(x: T1) -> None: y = x if isinstance(x, A): - # This is only checked when x is A, since A and B are not considered overlapping. x = y - x = A() + x = A() # E: Incompatible types in assignment (expression has type "A", variable has type "B") else: x = B() x = y x.foo() # E: "B" has no attribute "foo" + +class C: + field: int +class D: + field: str +T2 = TypeVar('T2', C, D) +def f2(x: T2) -> None: + y = x + if isinstance(x, C): + # C and D are non-overlapping, so this branch is never checked + x = y + x = C() + else: + x = D() + x = y + x.foo() # E: "D" has no attribute "foo" + S = TypeVar('S', int, str) def g(x: S) -> None: y = x diff --git a/test-data/unit/deps.test b/test-data/unit/deps.test index f5224d9216fc..fdf288df9332 100644 --- a/test-data/unit/deps.test +++ b/test-data/unit/deps.test @@ -409,6 +409,21 @@ def ff(x: object) -> None: class A: x: int +class B: + x: str + +def f(x: A) -> None: + if isinstance(x, B): + x.y +[builtins fixtures/isinstancelist.pyi] +[out] + -> , m.A, m.f + -> m.B, m.f + +[case testAdHocIsInstance] +class A: + x: int + class B: y: int @@ -417,6 +432,7 @@ def f(x: A) -> None: x.y [builtins fixtures/isinstancelist.pyi] [out] +.y> -> m.f -> , m.A, m.f -> m.B, m.f diff --git a/test-data/unit/pythoneval.test b/test-data/unit/pythoneval.test index 7cda44c4e569..c07449a6f24b 100644 --- a/test-data/unit/pythoneval.test +++ b/test-data/unit/pythoneval.test @@ -1479,6 +1479,20 @@ def f_suppresses() -> int: _testUnreachableWithStdlibContextManagersNoStrictOptional.py:9: error: Statement is unreachable _testUnreachableWithStdlibContextManagersNoStrictOptional.py:15: error: Statement is unreachable +[case testIsInstanceAdHocIntersectionWithStrAndBytes] +# mypy: warn-unreachable +x: str +if isinstance(x, bytes): + reveal_type(x) +y: str +if isinstance(x, int): + reveal_type(x) +[out] +_testIsInstanceAdHocIntersectionWithStrAndBytes.py:3: error: Subclass of "str" and "bytes" cannot exist: would have incompatible method signatures +_testIsInstanceAdHocIntersectionWithStrAndBytes.py:4: error: Statement is unreachable +_testIsInstanceAdHocIntersectionWithStrAndBytes.py:6: error: Subclass of "str" and "int" cannot exist: would have incompatible method signatures +_testIsInstanceAdHocIntersectionWithStrAndBytes.py:7: error: Statement is unreachable + [case testAsyncioFutureWait] # mypy: strict-optional from asyncio import Future, wait From fa2dbb53eb4b18553021ed72d6891a11c77d9b59 Mon Sep 17 00:00:00 2001 From: Michael Lee Date: Tue, 21 Jan 2020 23:08:17 -0800 Subject: [PATCH 2/4] Add some incremental mode tests --- test-data/unit/check-incremental.test | 152 ++++++++++++++++++++++++++ test-data/unit/deps.test | 2 +- test-data/unit/fine-grained.test | 152 ++++++++++++++++++++++++++ 3 files changed, 305 insertions(+), 1 deletion(-) diff --git a/test-data/unit/check-incremental.test b/test-data/unit/check-incremental.test index edf536ac9306..0178226ea97f 100644 --- a/test-data/unit/check-incremental.test +++ b/test-data/unit/check-incremental.test @@ -5165,3 +5165,155 @@ class Foo: # type: ignore import a [file b.py.2] import a # a change + +[case testIsInstanceAdHocIntersectionIncrementalNoChange] +import b +[file a.py] +class A: pass +class B: pass + +class Foo: + def __init__(self) -> None: + x: A + assert isinstance(x, B) + self.x = x +[file b.py] +from a import Foo +[file b.py.2] +from a import Foo +reveal_type(Foo().x) +[builtins fixtures/isinstance.pyi] +[out] +[out2] +tmp/b.py:2: note: Revealed type is 'a.' + +[case testIsInstanceAdHocIntersectionIncrementalIsInstanceChange] +import c +[file a.py] +class A: pass +class B: pass +class C: pass + +class Foo: + def __init__(self) -> None: + x: A + assert isinstance(x, B) + self.x = x +[file a.py.2] +class A: pass +class B: pass +class C: pass + +class Foo: + def __init__(self) -> None: + x: A + assert isinstance(x, C) + self.x = x + +[file b.py] +from a import Foo +y = Foo().x + +[file c.py] +from b import y +reveal_type(y) +[builtins fixtures/isinstance.pyi] +[out] +tmp/c.py:2: note: Revealed type is 'a.' +[out2] +tmp/c.py:2: note: Revealed type is 'a.' + +[case testIsInstanceAdHocIntersectionIncrementalUnderlyingObjChang] +import c +[file a.py] +class A: pass +class B: pass +class C: pass +Extra = B +[file a.py.2] +class A: pass +class B: pass +class C: pass +Extra = C + +[file b.py] +from a import A, Extra +x: A +if isinstance(x, Extra): + y = x + +[file c.py] +from b import y +reveal_type(y) +[builtins fixtures/isinstance.pyi] +[out] +tmp/c.py:2: note: Revealed type is 'b.' +[out2] +tmp/c.py:2: note: Revealed type is 'b.' + +[case testIsInstanceAdHocIntersectionIncrementalIntersectionToUnreachable] +import c +[file a.py] +class A: + x: int +class B: + x: int +x: A +assert isinstance(x, B) +y = x + +[file a.py.2] +class A: + x: int +class B: + x: str +x: A +assert isinstance(x, B) +y = x + +[file b.py] +from a import y +z = y + +[file c.py] +from b import z +reveal_type(z) +[builtins fixtures/isinstance.pyi] +[out] +tmp/c.py:2: note: Revealed type is 'a.' +[out2] +tmp/c.py:2: note: Revealed type is 'a.A' + +[case testIsInstanceAdHocIntersectionIncrementalUnreachaableToIntersection] +import c +[file a.py] +class A: + x: int +class B: + x: str +x: A +assert isinstance(x, B) +y = x + +[file a.py.2] +class A: + x: int +class B: + x: int +x: A +assert isinstance(x, B) +y = x + +[file b.py] +from a import y +z = y + +[file c.py] +from b import z +reveal_type(z) +[builtins fixtures/isinstance.pyi] +[out] +tmp/c.py:2: note: Revealed type is 'a.A' +[out2] +tmp/c.py:2: note: Revealed type is 'a.' + diff --git a/test-data/unit/deps.test b/test-data/unit/deps.test index fdf288df9332..62ddeac07bc7 100644 --- a/test-data/unit/deps.test +++ b/test-data/unit/deps.test @@ -420,7 +420,7 @@ def f(x: A) -> None: -> , m.A, m.f -> m.B, m.f -[case testAdHocIsInstance] +[case testIsInstanceAdHocIntersectionDeps] class A: x: int diff --git a/test-data/unit/fine-grained.test b/test-data/unit/fine-grained.test index 4e2309b3c5cf..d09aaad614e1 100644 --- a/test-data/unit/fine-grained.test +++ b/test-data/unit/fine-grained.test @@ -9410,3 +9410,155 @@ x: List[C] = [a.f(), a.f()] == a.py:2: error: "C" expects 2 type arguments, but 1 given [builtins fixtures/list.pyi] + +[case testIsInstanceAdHocIntersectionFineGrainedIncrementalNoChange] +import b +[file a.py] +class A: pass +class B: pass + +class Foo: + def __init__(self) -> None: + x: A + assert isinstance(x, B) + self.x = x +[file b.py] +from a import Foo +[file b.py.2] +from a import Foo +reveal_type(Foo().x) +[builtins fixtures/isinstance.pyi] +[out] +== +b.py:2: note: Revealed type is 'a.' + +[case testIsInstanceAdHocIntersectionFineGrainedIncrementalIsInstanceChange] +import c +[file a.py] +class A: pass +class B: pass +class C: pass + +class Foo: + def __init__(self) -> None: + x: A + assert isinstance(x, B) + self.x = x +[file a.py.2] +class A: pass +class B: pass +class C: pass + +class Foo: + def __init__(self) -> None: + x: A + assert isinstance(x, C) + self.x = x + +[file b.py] +from a import Foo +y = Foo().x + +[file c.py] +from b import y +reveal_type(y) +[builtins fixtures/isinstance.pyi] +[out] +c.py:2: note: Revealed type is 'a.' +== +c.py:2: note: Revealed type is 'a.' + +[case testIsInstanceAdHocIntersectionFineGrainedIncrementalUnderlyingObjChang] +import c +[file a.py] +class A: pass +class B: pass +class C: pass +Extra = B +[file a.py.2] +class A: pass +class B: pass +class C: pass +Extra = C + +[file b.py] +from a import A, Extra +x: A +if isinstance(x, Extra): + y = x + +[file c.py] +from b import y +reveal_type(y) +[builtins fixtures/isinstance.pyi] +[out] +c.py:2: note: Revealed type is 'b.' +== +c.py:2: note: Revealed type is 'b.' + +[case testIsInstanceAdHocIntersectionFineGrainedIncrementalIntersectionToUnreachable] +import c +[file a.py] +class A: + x: int +class B: + x: int +x: A +assert isinstance(x, B) +y = x + +[file a.py.2] +class A: + x: int +class B: + x: str +x: A +assert isinstance(x, B) +y = x + +[file b.py] +from a import y +z = y + +[file c.py] +from b import z +reveal_type(z) +[builtins fixtures/isinstance.pyi] +[out] +c.py:2: note: Revealed type is 'a.' +== +c.py:2: note: Revealed type is 'a.A' + +[case testIsInstanceAdHocIntersectionFineGrainedIncrementalUnreachaableToIntersection] +import c +[file a.py] +class A: + x: int +class B: + x: str +x: A +assert isinstance(x, B) +y = x + +[file a.py.2] +class A: + x: int +class B: + x: int +x: A +assert isinstance(x, B) +y = x + +[file b.py] +from a import y +z = y + +[file c.py] +from b import z +reveal_type(z) +[builtins fixtures/isinstance.pyi] +[out] +c.py:2: note: Revealed type is 'a.A' +== +c.py:2: note: Revealed type is 'a.' + From 910135f5b78dc163bc721edd449a2eebee45019d Mon Sep 17 00:00:00 2001 From: Michael Lee Date: Tue, 21 Jan 2020 23:11:05 -0800 Subject: [PATCH 3/4] Add workaround for mypyc bug --- mypy/checker.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/mypy/checker.py b/mypy/checker.py index 90c675d328e1..2e77f349a851 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -4804,7 +4804,11 @@ def conditional_type_map_with_intersection(self, expr_type: Type, type_ranges: Optional[List[TypeRange]], ) -> Tuple[TypeMap, TypeMap]: - yes_map, no_map = conditional_type_map(expr, expr_type, type_ranges) + # For some reason, doing "yes_map, no_map = conditional_type_maps(...)" + # doesn't work: mypyc will decide that 'yes_map' is of type None if we try. + initial_maps = conditional_type_map(expr, expr_type, type_ranges) + yes_map = initial_maps[0] # type: TypeMap + no_map = initial_maps[1] # type: TypeMap if yes_map is not None or type_ranges is None: return yes_map, no_map From 76252579524767994195e2d23e1b6ddb9188ef1b Mon Sep 17 00:00:00 2001 From: Michael Lee Date: Thu, 23 Jan 2020 09:17:46 -0800 Subject: [PATCH 4/4] Respond to code review and add more comments --- mypy/checker.py | 27 ++++++++++++++++++++++++++- mypy/messages.py | 3 ++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 2e77f349a851..d80e0ec02b69 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -3663,6 +3663,28 @@ def intersect_instances(self, instances: Sequence[Instance], ctx: Context, ) -> Optional[Instance]: + """Try creating an ad-hoc intersection of the given instances. + + Note that this function does *not* try and create a full-fledged + intersection type. Instead, it returns an instance of a new ad-hoc + subclass of the given instances. + + This is mainly useful when you need a way of representing some + theoretical subclass of the instances the user may be trying to use + the generated intersection can serve as a placeholder. + + This function will create a fresh subclass every time you call it, + even if you pass in the exact same arguments. So this means calling + `self.intersect_intersection([inst_1, inst_2], ctx)` twice will result + in instances of two distinct subclasses of inst_1 and inst_2. + + This is by design: we want each ad-hoc intersection to be unique since + they're supposed represent some other unknown subclass. + + Returns None if creating the subclass is impossible (e.g. due to + MRO errors or incompatible signatures). If we do successfully create + a subclass, its TypeInfo will automatically be added to the global scope. + """ curr_module = self.scope.stack[0] assert isinstance(curr_module, MypyFile) @@ -4813,7 +4835,10 @@ def conditional_type_map_with_intersection(self, if yes_map is not None or type_ranges is None: return yes_map, no_map - # If we couldn't infer anything useful, try again by trying to compute an intersection + # If conditions_type_map was unable to successfully narrow the expr_type + # using the type_ranges and concluded if-branch is unreachable, we try + # computing it again using a different algorithm that tries to generate + # an ad-hoc intersection between the expr_type and the type_ranges. expr_type = get_proper_type(expr_type) if isinstance(expr_type, UnionType): possible_expr_types = get_proper_types(expr_type.relevant_items()) diff --git a/mypy/messages.py b/mypy/messages.py index 2050d522716d..14e1b146a82b 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -1275,7 +1275,8 @@ def impossible_intersection(self, context: Context, ) -> None: template = "Subclass of {} cannot exist: would have {}" - self.fail(template.format(formatted_base_class_list, reason), context) + self.fail(template.format(formatted_base_class_list, reason), context, + code=codes.UNREACHABLE) def report_protocol_problems(self, subtype: Union[Instance, TupleType, TypedDictType],