Skip to content

Commit ef56409

Browse files
mtshibaAlexWaygood
andauthored
[ty] support del statement and deletion of except handler names (astral-sh#18593)
## Summary This PR closes astral-sh/ty#238. Since `DefinitionState::Deleted` was introduced in astral-sh#18041, support for the `del` statement (and deletion of except handler names) is straightforward. However, it is difficult to determine whether references to attributes or subscripts are unresolved after they are deleted. This PR only invalidates narrowing by assignment if the attribute or subscript is deleted. ## Test Plan `mdtest/del.md` is added. --------- Co-authored-by: Alex Waygood <[email protected]>
1 parent 96171f4 commit ef56409

File tree

4 files changed

+193
-7
lines changed

4 files changed

+193
-7
lines changed
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
# `del` statement
2+
3+
## Basic
4+
5+
```py
6+
a = 1
7+
del a
8+
# error: [unresolved-reference]
9+
reveal_type(a) # revealed: Unknown
10+
11+
# error: [invalid-syntax] "Invalid delete target"
12+
del 1
13+
14+
# error: [unresolved-reference]
15+
del a
16+
17+
x, y = 1, 2
18+
del x, y
19+
# error: [unresolved-reference]
20+
reveal_type(x) # revealed: Unknown
21+
# error: [unresolved-reference]
22+
reveal_type(y) # revealed: Unknown
23+
24+
def cond() -> bool:
25+
return True
26+
27+
b = 1
28+
if cond():
29+
del b
30+
31+
# error: [possibly-unresolved-reference]
32+
reveal_type(b) # revealed: Literal[1]
33+
34+
c = 1
35+
if cond():
36+
c = 2
37+
else:
38+
del c
39+
40+
# error: [possibly-unresolved-reference]
41+
reveal_type(c) # revealed: Literal[2]
42+
43+
d = 1
44+
45+
def delete():
46+
# TODO: this results in `UnboundLocalError`; we should emit `unresolved-reference`
47+
del d
48+
49+
delete()
50+
reveal_type(d) # revealed: Literal[1]
51+
52+
def delete_global():
53+
global d
54+
del d
55+
56+
delete_global()
57+
# The variable should have been removed, but we won't check it for now.
58+
reveal_type(d) # revealed: Literal[1]
59+
```
60+
61+
## Delete attributes
62+
63+
If an attribute is referenced after being deleted, it will be an error at runtime. But we don't
64+
treat this as an error (because there may have been a redefinition by a method between the `del`
65+
statement and the reference). However, deleting an attribute invalidates type narrowing by
66+
assignment, and the attribute type will be the originally declared type.
67+
68+
### Invalidate narrowing
69+
70+
```py
71+
class C:
72+
x: int = 1
73+
74+
c = C()
75+
del c.x
76+
reveal_type(c.x) # revealed: int
77+
78+
# error: [unresolved-attribute]
79+
del c.non_existent
80+
81+
c.x = 1
82+
reveal_type(c.x) # revealed: Literal[1]
83+
del c.x
84+
reveal_type(c.x) # revealed: int
85+
```
86+
87+
### Delete an instance attribute definition
88+
89+
```py
90+
class C:
91+
x: int = 1
92+
93+
c = C()
94+
reveal_type(c.x) # revealed: int
95+
96+
del C.x
97+
c = C()
98+
# This attribute is unresolved, but we won't check it for now.
99+
reveal_type(c.x) # revealed: int
100+
```
101+
102+
## Delete items
103+
104+
Deleting an item also invalidates the narrowing by the assignment, but accessing the item itself is
105+
still valid.
106+
107+
```py
108+
def f(l: list[int]):
109+
del l[0]
110+
# If the length of `l` was 1, this will be a runtime error,
111+
# but if it was greater than that, it will not be an error.
112+
reveal_type(l[0]) # revealed: int
113+
114+
# error: [call-non-callable]
115+
del l["string"]
116+
117+
l[0] = 1
118+
reveal_type(l[0]) # revealed: Literal[1]
119+
del l[0]
120+
reveal_type(l[0]) # revealed: int
121+
```

crates/ty_python_semantic/resources/mdtest/exception/basic.md

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,3 +240,41 @@ def _(e: Exception | type[Exception] | None):
240240
def _(e: int | None):
241241
raise IndexError from e # error: [invalid-raise]
242242
```
243+
244+
## The caught exception is cleared at the end of the except clause
245+
246+
```py
247+
e = None
248+
reveal_type(e) # revealed: None
249+
250+
try:
251+
raise ValueError()
252+
except ValueError as e:
253+
reveal_type(e) # revealed: ValueError
254+
# error: [unresolved-reference]
255+
reveal_type(e) # revealed: Unknown
256+
257+
e = None
258+
259+
def cond() -> bool:
260+
return True
261+
262+
try:
263+
if cond():
264+
raise ValueError()
265+
except ValueError as e:
266+
reveal_type(e) # revealed: ValueError
267+
# error: [possibly-unresolved-reference]
268+
reveal_type(e) # revealed: None
269+
270+
def f(x: type[Exception]):
271+
e = None
272+
try:
273+
raise x
274+
except ValueError as e:
275+
pass
276+
except:
277+
pass
278+
# error: [possibly-unresolved-reference]
279+
reveal_type(e) # revealed: None
280+
```

crates/ty_python_semantic/src/semantic_index/builder.rs

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,12 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> {
449449
}
450450
}
451451

452+
fn delete_binding(&mut self, place: ScopedPlaceId) {
453+
let is_place_name = self.current_place_table().place_expr(place).is_name();
454+
self.current_use_def_map_mut()
455+
.delete_binding(place, is_place_name);
456+
}
457+
452458
/// Push a new [`Definition`] onto the list of definitions
453459
/// associated with the `definition_node` AST node.
454460
///
@@ -1817,7 +1823,7 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
18171823
// If `handled_exceptions` above was `None`, it's something like `except as e:`,
18181824
// which is invalid syntax. However, it's still pretty obvious here that the user
18191825
// *wanted* `e` to be bound, so we should still create a definition here nonetheless.
1820-
if let Some(symbol_name) = symbol_name {
1826+
let symbol = if let Some(symbol_name) = symbol_name {
18211827
let symbol = self.add_symbol(symbol_name.id.clone());
18221828

18231829
self.add_definition(
@@ -1827,9 +1833,16 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
18271833
is_star: *is_star,
18281834
}),
18291835
);
1830-
}
1836+
Some(symbol)
1837+
} else {
1838+
None
1839+
};
18311840

18321841
self.visit_body(handler_body);
1842+
// The caught exception is cleared at the end of the except clause
1843+
if let Some(symbol) = symbol {
1844+
self.delete_binding(symbol);
1845+
}
18331846
// Each `except` block is mutually exclusive with all other `except` blocks.
18341847
post_except_states.push(self.flow_snapshot());
18351848

@@ -1903,13 +1916,15 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
19031916
walk_stmt(self, stmt);
19041917
}
19051918
ast::Stmt::Delete(ast::StmtDelete { targets, range: _ }) => {
1919+
// We will check the target expressions and then delete them.
1920+
walk_stmt(self, stmt);
19061921
for target in targets {
19071922
if let Ok(target) = PlaceExpr::try_from(target) {
19081923
let place_id = self.add_place(target);
19091924
self.current_place_table().mark_place_used(place_id);
1925+
self.delete_binding(place_id);
19101926
}
19111927
}
1912-
walk_stmt(self, stmt);
19131928
}
19141929
ast::Stmt::Expr(ast::StmtExpr { value, range: _ }) if self.in_module_scope() => {
19151930
if let Some(expr) = dunder_all_extend_argument(value) {
@@ -1956,7 +1971,7 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
19561971
}
19571972
(ast::ExprContext::Load, _) => (true, false),
19581973
(ast::ExprContext::Store, _) => (false, true),
1959-
(ast::ExprContext::Del, _) => (false, true),
1974+
(ast::ExprContext::Del, _) => (true, true),
19601975
(ast::ExprContext::Invalid, _) => (false, false),
19611976
};
19621977
let place_id = self.add_place(place_expr);

crates/ty_python_semantic/src/types/infer.rs

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6145,7 +6145,11 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
61456145
fn infer_name_expression(&mut self, name: &ast::ExprName) -> Type<'db> {
61466146
match name.ctx {
61476147
ExprContext::Load => self.infer_name_load(name),
6148-
ExprContext::Store | ExprContext::Del => Type::Never,
6148+
ExprContext::Store => Type::Never,
6149+
ExprContext::Del => {
6150+
self.infer_name_load(name);
6151+
Type::Never
6152+
}
61496153
ExprContext::Invalid => Type::unknown(),
61506154
}
61516155
}
@@ -6254,10 +6258,14 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
62546258

62556259
match ctx {
62566260
ExprContext::Load => self.infer_attribute_load(attribute),
6257-
ExprContext::Store | ExprContext::Del => {
6261+
ExprContext::Store => {
62586262
self.infer_expression(value);
62596263
Type::Never
62606264
}
6265+
ExprContext::Del => {
6266+
self.infer_attribute_load(attribute);
6267+
Type::Never
6268+
}
62616269
ExprContext::Invalid => {
62626270
self.infer_expression(value);
62636271
Type::unknown()
@@ -7646,12 +7654,16 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
76467654

76477655
match ctx {
76487656
ExprContext::Load => self.infer_subscript_load(subscript),
7649-
ExprContext::Store | ExprContext::Del => {
7657+
ExprContext::Store => {
76507658
let value_ty = self.infer_expression(value);
76517659
let slice_ty = self.infer_expression(slice);
76527660
self.infer_subscript_expression_types(value, value_ty, slice_ty);
76537661
Type::Never
76547662
}
7663+
ExprContext::Del => {
7664+
self.infer_subscript_load(subscript);
7665+
Type::Never
7666+
}
76557667
ExprContext::Invalid => {
76567668
let value_ty = self.infer_expression(value);
76577669
let slice_ty = self.infer_expression(slice);

0 commit comments

Comments
 (0)