8000 Do not consider bare TypeVar not overlapping with None for reachabili… · python/mypy@08340c2 · GitHub
[go: up one dir, main page]
Skip to content

Commit 08340c2

Browse files
sterliakovsterliakov
authored
Do not consider bare TypeVar not overlapping with None for reachability analysis (#18138)
Fixes #18126. Simply allowing such intersection was insufficient: existing binder logic widened the type to `T | None` after the `is None` check. This PR extends the binder logic to prevent constructing a union type when all conditional branches are reachable and contain no assignments: checking `if isinstance(something, Something)` does not change the type of `something` after the end of the `if` block.
1 parent e840275 commit 08340c2

7 files changed

+116
-42
lines changed

‎mypy/binder.py

Lines changed: 39 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
from collections import defaultdict
44
from contextlib import contextmanager
5-
from typing import DefaultDict, Iterator, List, Optional, Tuple, Union, cast
5+
from typing import DefaultDict, Iterator, List, NamedTuple, Optional, Tuple, Union
66
from typing_extensions import TypeAlias as _TypeAlias
77

88
from mypy.erasetype import remove_instance_last_known_values
@@ -30,6 +30,11 @@
3030
BindableExpression: _TypeAlias = Union[IndexExpr, MemberExpr, NameExpr]
3131

3232

33+
class CurrentType(NamedTuple):
34+
type: Type
35+
from_assignment: bool
36+
37+
3338
class Frame:
3439
"""A Frame represents a specific point in the execution of a program.
3540
It carries information about the current types of expressions at
@@ -44,7 +49,7 @@ class Frame:
4449

4550
def __init__(self, id: int, conditional_frame: bool = False) -> None:
4651
self.id = id
47-
self.types: dict[Key, Type] = {}
52+
self.types: dict[Key, CurrentType] = {}
4853
self.unreachable = False
4954
self.conditional_frame = conditional_frame
5055
self.suppress_unreachable_warnings = False
@@ -132,18 +137,18 @@ def push_frame(self, conditional_frame: bool = False) -> Frame:
132137
self.options_on_return.append([])
133138
return f
134139

135-
def _put(self, key: Key, type: Type, index: int = -1) -> None:
136-
self.frames[index].types[key] = type
140+
def _put(self, key: Key, type: Type, from_assignment: bool, index: int = -1) -> None:
141+
self.frames[index].types[key] = CurrentType(type, from_assignment)
137142

138-
def _get(self, key: Key, index: int = -1) -> Type | None:
143+
def _get(self, key: Key, index: int = -1) -> CurrentType | None:
139144
if index < 0:
140145
index += len(self.frames)
141146
for i in range(index, -1, -1):
142147
if key in self.frames[i].types:
143148
return self.frames[i].types[key]
144149
return None
145150

146-
def put(self, expr: Expression, typ: Type) -> None:
151+
def put(self, expr: Expression, typ: Type, *, from_assignment: bool = True) -> None:
147152
if not isinstance(expr, (IndexExpr, MemberExpr, NameExpr)):
148153
return
149154
if not literal(expr):
@@ -153,7 +158,7 @@ def put(self, expr: Expression, typ: Type) -> None:
153158
if key not in self.declarations:
154159
self.declarations[key] = get_declaration(expr)
155160
self._add_dependencies(key)
156-
self._put(key, typ)
161+
self._put(key, typ, from_assignment)
157162

158163
def unreachable(self) -> None:
159164
self.frames[-1].unreachable = True
@@ -164,7 +169,10 @@ def suppress_unreachable_warnings(self) -> None:
164169
def get(self, expr: Expression) -> Type | None:
165170
key = literal_hash(expr)
166171
assert key is not None, "Internal error: binder tried to get non-literal"
167-
return self._get(key)
172+
found = self._get(key)
173+
if found is None:
174+
return None
175+
return found.type
168176

169177
def is_unreachable(self) -> bool:
170178
# TODO: Copy the value of unreachable into new frames to avoid
@@ -193,7 +201,7 @@ def update_from_options(self, frames: list[Frame]) -> bool:
193201
If a key is declared as AnyType, only update it if all the
194202
options are the same.
195203
"""
196-
204+
all_reachable = all(not f.unreachable for f in frames)
197205
frames = [f for f in frames if not f.unreachable]
198206
changed = False
199207
keys = {key for f in frames for key in f.types}
@@ -207,17 +215,30 @@ def update_from_options(self, frames: list[Frame]) -> bool:
207215
# know anything about key in at least one possible frame.
208216
continue
209217

210-
type = resulting_values[0]
211-
assert type is not None
218+
if all_reachable and all(
219+
x is not None and not x.from_assignment for x in resulting_values
220+
):
221+
# Do not synthesize a new type if we encountered a conditional block
222+
# (if, while or match-case) without assignments.
223+
# See check-isinstance.test::testNoneCheckDoesNotMakeTypeVarOptional
224+
# This is a safe assumption: the fact that we checked something with `is`
225+
# or `isinstance` does not change the type of the value.
226+
continue
227+
228+
current_type = resulting_values[0]
229+
assert current_type is not None
230+
type = current_type.type
212231
declaration_type = get_proper_type(self.declarations.get(key))
213232
if isinstance(declaration_type, AnyType):
214233
# At this point resulting values can't contain None, see continue above
215-
if not all(is_same_type(type, cast(Type, t)) for t in resulting_values[1:]):
234+
if not all(
235+
t is not None and is_same_type(type, t.type) for t in resulting_values[1:]
236+
):
216237
type = AnyType(TypeOfAny.from_another_any, source_any=declaration_type)
217238
else:
218239
for other in resulting_values[1:]:
219240
assert other is not None
220-
type = join_simple(self.declarations[key], type, other)
241+
type = join_simple(self.declarations[key], type, other.type)
221242
# Try simplifying resulting type for unions involving variadic tuples.
222243
# Technically, everything is still valid without this step, but if we do
223244
# not do this, this may create long unions after exiting an if check like:
@@ -236,8 +257,8 @@ def update_from_options(self, frames: list[Frame]) -> bool:
236257
)
237258
if simplified == self.declarations[key]:
238259
type = simplified
239-
if current_value is None or not is_same_type(type, current_value):
240-
self._put(key, type)
260+
if current_value is None or not is_same_type(type, current_value[0]):
261+
self._put(key, type, from_assignment=True)
241262
changed = True
242263

243264
self.frames[-1].unreachable = not frames
@@ -374,7 +395,9 @@ def most_recent_enclosing_type(self, expr: BindableExpression, type: Type) -> Ty
374395
key = literal_hash(expr)
375396
assert key is not None
376397
enclosers = [get_declaration(expr)] + [
377-
f.types[key] for f in self.frames if key in f.types and is_subtype(type, f.types[key])
398+
f.types[key].type
399+
for f in self.frames
400+
if key in f.types and is_subtype(type, f.types[key][0])
378401
]
379402
return enclosers[-1]
380403

‎mypy/checker.py

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4725,11 +4725,11 @@ def visit_if_stmt(self, s: IfStmt) -> None:
47254725

47264726
# XXX Issue a warning if condition is always False?
47274727
with self.binder.frame_context(can_skip=True, fall_through=2):
4728-
self.push_type_map(if_map)
4728+
self.push_type_map(if_map, from_assignment=False)
47294729
self.accept(b)
47304730

47314731
# XXX Issue a warning if condition is always True?
4732-
self.push_type_map(else_map)
4732+
self.push_type_map(else_map, from_assignment=False)
47334733

47344734
with self.binder.frame_context(can_skip=False, fall_through=2):
47354735
if s.else_body:
@@ -5310,18 +5310,21 @@ def visit_match_stmt(self, s: MatchStmt) -> None:
53105310
if b.is_unreachable or isinstance(
53115311
get_proper_type(pattern_type.type), UninhabitedType
53125312
):
5313-
self.push_type_map(None)
5313+
self.push_type_map(None, from_assignment=False)
53145314
else_map: TypeMap = {}
53155315
else:
53165316
pattern_map, else_map = conditional_types_to_typemaps(
53175317
named_subject, pattern_type.type, pattern_type.rest_type
53185318
)
53195319
self.remove_capture_conflicts(pattern_type.captures, inferred_types)
5320-
self.push_type_map(pattern_map)
5320+
self.push_type_map(pattern_map, from_assignment=False)
53215321
if pattern_map:
53225322
for expr, typ in pattern_map.items():
5323-
self.push_type_map(self._get_recursive_sub_patterns_map(expr, typ))
5324-
self.push_type_map(pattern_type.captures)
5323+
self.push_type_map(
5324+
self._get_recursive_sub_patterns_map(expr, typ),
5325+
from_assignment=False,
5326+
)
5327+
self.push_type_map(pattern_type.captures, from_assignment=False)
53255328
if g is not None:
53265329
with self.binder.frame_context(can_skip=False, fall_through=3):
53275330
gt = get_proper_type(self.expr_checker.accept(g))
@@ -5347,11 +5350,11 @@ def visit_match_stmt(self, s: MatchStmt) -> None:
53475350
continue
53485351
type_map[named_subject] = type_map[expr]
53495352

5350-
self.push_type_map(guard_map)
5353+
self.push_type_map(guard_map, from_assignment=False)
53515354
self.accept(b)
53525355
else:
53535356
self.accept(b)
5354-
self.push_type_map(else_map)
5357+
self.push_type_map(else_map, from_assignment=False)
53555358

53565359
# This is needed due to a quirk in frame_context. Without it types will stay narrowed
53575360
# after the match.
@@ -7372,12 +7375,12 @@ def iterable_item_type(
73727375
def function_type(self, func: FuncBase) -> FunctionLike:
73737376
return function_type(func, self.named_type("builtins.function"))
73747377

7375-
def push_type_map(self, type_map: TypeMap) -> None:
7378+
def push_type_map(self, type_map: TypeMap, *, from_assignment: bool = True) -> None:
73767379
if type_map is None:
73777380
self.binder.unreachable()
73787381
else:
73797382
for expr, type in type_map.items():
7380-
self.binder.put(expr, type)
7383+
self.binder.put(expr, type, from_assignment=from_assignment)
73817384

73827385
def infer_issubclass_maps(self, node: CallExpr, expr: Expression) -> tuple[TypeMap, TypeMap]:
73837386
"""Infer type restrictions for an expression in issubclass call."""
@@ -7750,9 +7753,7 @@ def conditional_types(
77507753
) and is_proper_subtype(current_type, proposed_type, ignore_promotions=True):
77517754
# Expression is always of one of the types in proposed_type_ranges
77527755
return default, UninhabitedType()
7753-
elif not is_overlapping_types(
7754-
current_type, proposed_type, prohibit_none_typevar_overlap=True, ignore_promotions=True
7755-
):
7756+
elif not is_overlapping_types(current_type, proposed_type, ignore_promotions=True):
77567757
# Expression is never of any type in proposed_type_ranges
77577758
return UninhabitedType(), default
77587759
else:

‎test-data/unit/check-enum.test

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -815,7 +815,7 @@ elif x is Foo.C:
815815
reveal_type(x) # N: Revealed type is "Literal[__main__.Foo.C]"
816816
else:
817817
reveal_type(x) # No output here: this branch is unreachable
818-
reveal_type(x) # N: Revealed type is "__main__.Foo"
818+
reveal_type(x) # N: Revealed type is "Union[Literal[__main__.Foo.A], Literal[__main__.Foo.B], Literal[__main__.Foo.C]]"
819819

820820
if Foo.A is x:
821821
reveal_type(x) # N: Revealed type is "Literal[__main__.Foo.A]"
@@ -825,7 +825,7 @@ elif Foo.C is x:
825825
reveal_type(x) # N: Revealed type is "Literal[__main__.Foo.C]"
826826
else:
827827
reveal_type(x) # No output here: this branch is unreachable
828-
reveal_type(x) # N: Revealed type is "__main__.Foo"
828+
reveal_type(x) # N: Revealed type is "Union[Literal[__main__.Foo.A], Literal[__main__.Foo.B], Literal[__main__.Foo.C]]"
829829

830830
y: Foo
831831
if y is Foo.A:

‎test-data/unit/check-isinstance.test

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2207,23 +2207,24 @@ def foo2(x: Optional[str]) -> None:
22072207
reveal_type(x) # N: Revealed type is "builtins.str"
22082208
[builtins fixtures/isinstance.pyi]
22092209

2210-
[case testNoneCheckDoesNotNarrowWhenUsingTypeVars]
2211-
2212-
# Note: this test (and the following one) are testing checker.conditional_type_map:
2213-
# if you set the 'prohibit_none_typevar_overlap' keyword argument to False when calling
2214-
# 'is_overlapping_types', the binder will incorrectly infer that 'out' has a type of
2215-
# Union[T, None] after the if statement.
2216-
2210+
[case testNoneCheckDoesNotMakeTypeVarOptional]
22172211
from typing import TypeVar
22182212

22192213
T = TypeVar('T')
22202214

2221-
def foo(x: T) -> T:
2215+
def foo_if(x: T) -> T:
22222216
out = None
22232217
out = x
22242218
if out is None:
22252219
pass
22262220
return out
2221+
2222+
def foo_while(x: T) -> T:
2223+
out = None
2224+
out = x
2225+
while out is None:
2226+
pass
2227+
return out
22272228
[builtins fixtures/isinstance.pyi]
22282229

22292230
[case testNoneCheckDoesNotNarrowWhenUsingTypeVarsNoStrictOptional]

‎test-data/unit/check-narrowing.test

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2333,3 +2333,22 @@ def f(x: C) -> None:
23332333

23342334
f(C(5))
23352335
[builtins fixtures/primitives.pyi]
2336+
2337+
[case testNarrowingTypeVarNone]
2338+
# flags: --warn-unreachable
2339+
2340+
# https://github.com/python/mypy/issues/18126
2341+
from typing import TypeVar
2342+
2343+
T = TypeVar("T")
2344+
2345+
def fn_if(arg: T) -> None:
2346+
if arg is None:
2347+
return None
2348+
return None
2349+
2350+
def fn_while(arg: T) -> None:
2351+
while arg is None:
2352+
return None
2353+
return None
2354+
[builtins fixtures/primitives.pyi]

‎test-data/unit/check-python310.test

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2409,3 +2409,33 @@ def f(x: T) -> None:
24092409
case _:
24102410
accept_seq_int(x) # E: Argument 1 to "accept_seq_int" has incompatible type "T"; expected "Sequence[int]"
24112411
[builtins fixtures/tuple.pyi]
2412+
2413+
[case testNarrowingTypeVarMatch]
2414+
# flags: --warn-unreachable
2415+
2416+
# https://github.com/python/mypy/issues/18126
2417+
from typing import TypeVar
2418+
2419+
T = TypeVar("T")
2420+
2421+
def fn_case(arg: T) -> None:
2422+
match arg:
2423+
case None:
2424+
return None
2425+
return None
2426+
[builtins fixtures/primitives.pyi]
2427+
2428+
[case testNoneCheckDoesNotMakeTypeVarOptionalMatch]
2429+
from typing import TypeVar
2430+
2431+
T = TypeVar('T')
2432+
2433+
def foo(x: T) -> T:
2434+
out = None
2435+
out = x
2436+
match out:
2437+
case None:
2438+
pass
2439+
return out
2440+
2441+
[builtins fixtures/isinstance.pyi]

‎test-data/unit/check-type-promotion.test

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ else:
9191
reveal_type(x) # N: Revealed type is "builtins.complex"
9292

9393
# Note we make type precise, since type promotions are involved
94-
reveal_type(x) # N: Revealed type is "Union[builtins.complex, builtins.int, builtins.float]"
94+
reveal_type(x) # N: Revealed type is "builtins.complex"
9595
[builtins fixtures/primitives.pyi]
9696

9797
[case testIntersectionUsingPromotion3]
@@ -127,7 +127,7 @@ if isinstance(x, int):
127127
reveal_type(x) # N: Revealed type is "builtins.int"
128128
else:
129129
reveal_type(x) # N: Revealed type is "Union[builtins.float, builtins.complex]"
130-
reveal_type(x) # N: Revealed type is "Union[builtins.int, builtins.float, builtins.complex]"
130+
reveal_type(x) # N: Revealed type is "Union[builtins.float, builtins.complex]"
131131
[builtins fixtures/primitives.pyi]
132132

133133
[case testIntersectionUsingPromotion6]
@@ -139,7 +139,7 @@ if isinstance(x, int):
139139
reveal_type(x) # N: Revealed type is "builtins.int"
140140
else:
141141
reveal_type(x) # N: Revealed type is "Union[builtins.str, builtins.complex]"
142-
reveal_type(x) # N: Revealed type is "Union[builtins.str, builtins.int, builtins.complex]"
142+
reveal_type(x) # N: Revealed type is "Union[builtins.str, builtins.complex]"
143143
[builtins fixtures/primitives.pyi]
144144

145145
[case testIntersectionUsingPromotion7]

0 commit comments

Comments
 (0)
0