From d56994f2ae05f10aac81eb200e2a8433d7ca5a4f Mon Sep 17 00:00:00 2001 From: Michael Lee Date: Sun, 23 Jun 2019 23:27:48 -0700 Subject: [PATCH 1/2] Fix reachability inference with 'isinstance(Any, Any)' While experimenting with some reachability inference logic, I discovered the following program does not seem to behave as expected: ```python from typing import Any from foo import A # type: ignore x: Any if isinstance(x, A): reveal_type(x) else: reveal_type(x) ``` Both branches really ought to be reachable: since both `x` and `A` are of type `Any`, we really can't say whether or not any particular branch will run. However, mypy currently instead assumes that only the first branch is reachable and does not type-check the second branch. So in this example, only the first `reveal_type(...)` outputs a note. This pull request fixes this bug. --- mypy/checker.py | 7 ++++++- test-data/unit/check-isinstance.test | 23 +++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/mypy/checker.py b/mypy/checker.py index 8720b4995f36..06357caeb1a1 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -3900,7 +3900,12 @@ def conditional_type_map(expr: Expression, else: proposed_type = UnionType([type_range.item for type_range in proposed_type_ranges]) if current_type: - if (not any(type_range.is_upper_bound for type_range in proposed_type_ranges) + if isinstance(proposed_type, AnyType): + # We don't really know much about the proposed type, so we shouldn't + # attempt to narrow anything. Instead, we broaden the expr to Any to + # avoid false positives + return {expr: proposed_type}, {} + elif (not any(type_range.is_upper_bound for type_range in proposed_type_ranges) and is_proper_subtype(current_type, proposed_type)): # Expression is always of one of the types in proposed_type_ranges return {}, None diff --git a/test-data/unit/check-isinstance.test b/test-data/unit/check-isinstance.test index 0b80a74c0110..e09e7f8ac41f 100644 --- a/test-data/unit/check-isinstance.test +++ b/test-data/unit/check-isinstance.test @@ -2101,6 +2101,29 @@ def f(x: Union[A, str]) -> None: x.method_only_in_a() [builtins fixtures/isinstance.pyi] +[case testIsinstanceIgnoredImport2] +from typing import Any +from foo import Bad # type: ignore +x: Any +if isinstance(x, Bad): + reveal_type(x) # N: Revealed type is 'Any' +else: + reveal_type(x) # N: Revealed type is 'Any' + +y: object +if isinstance(y, Bad): + reveal_type(y) # N: Revealed type is 'Any' +else: + reveal_type(y) # N: Revealed type is 'builtins.object' + +class Ok: pass +z: Any +if isinstance(z, Ok): + reveal_type(z) # N: Revealed type is '__main__.Ok' +else: + reveal_type(z) # N: Revealed type is 'Any' +[builtins fixtures/isinstance.pyi] + [case testIsInstanceInitialNoneCheckSkipsImpossibleCasesNoStrictOptional] # flags: --strict-optional from typing import Optional, Union From 88ed77a26efc6a1de4005fb2e866a1176cdf2703 Mon Sep 17 00:00:00 2001 From: Michael Lee Date: Sun, 23 Jun 2019 23:58:26 -0700 Subject: [PATCH 2/2] Generalize to isinstance with multiple bad types --- mypy/checker.py | 6 ++---- test-data/unit/check-isinstance.test | 9 +++++++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 06357caeb1a1..842a4a3d25a5 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -3895,10 +3895,8 @@ def conditional_type_map(expr: Expression, second element is a map from the expression to the type it would hold if it was not the proposed type, if any. None means bot, {} means top""" if proposed_type_ranges: - if len(proposed_type_ranges) == 1: - proposed_type = proposed_type_ranges[0].item # Union with a single type breaks tests - else: - proposed_type = UnionType([type_range.item for type_range in proposed_type_ranges]) + proposed_items = [type_range.item for type_range in proposed_type_ranges] + proposed_type = UnionType.make_simplified_union(proposed_items) if current_type: if isinstance(proposed_type, AnyType): # We don't really know much about the proposed type, so we shouldn't diff --git a/test-data/unit/check-isinstance.test b/test-data/unit/check-isinstance.test index e09e7f8ac41f..ca3b2fb176ea 100644 --- a/test-data/unit/check-isinstance.test +++ b/test-data/unit/check-isinstance.test @@ -2101,15 +2101,20 @@ def f(x: Union[A, str]) -> None: x.method_only_in_a() [builtins fixtures/isinstance.pyi] -[case testIsinstanceIgnoredImport2] +[case testIsinstanceIgnoredImportDualAny] from typing import Any -from foo import Bad # type: ignore +from foo import Bad, OtherBad # type: ignore x: Any if isinstance(x, Bad): reveal_type(x) # N: Revealed type is 'Any' else: reveal_type(x) # N: Revealed type is 'Any' +if isinstance(x, (Bad, OtherBad)): + reveal_type(x) # N: Revealed type is 'Any' +else: + reveal_type(x) # N: Revealed type is 'Any' + y: object if isinstance(y, Bad): reveal_type(y) # N: Revealed type is 'Any'