mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-01 14:21:24 +00:00
[ty] support del statement and deletion of except handler names (#18593)
## Summary This PR closes https://github.com/astral-sh/ty/issues/238. Since `DefinitionState::Deleted` was introduced in #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 <Alex.Waygood@Gmail.com>
This commit is contained in:
parent
96171f41c2
commit
ef564094a9
4 changed files with 193 additions and 7 deletions
121
crates/ty_python_semantic/resources/mdtest/del.md
Normal file
121
crates/ty_python_semantic/resources/mdtest/del.md
Normal file
|
@ -0,0 +1,121 @@
|
||||||
|
# `del` statement
|
||||||
|
|
||||||
|
## Basic
|
||||||
|
|
||||||
|
```py
|
||||||
|
a = 1
|
||||||
|
del a
|
||||||
|
# error: [unresolved-reference]
|
||||||
|
reveal_type(a) # revealed: Unknown
|
||||||
|
|
||||||
|
# error: [invalid-syntax] "Invalid delete target"
|
||||||
|
del 1
|
||||||
|
|
||||||
|
# error: [unresolved-reference]
|
||||||
|
del a
|
||||||
|
|
||||||
|
x, y = 1, 2
|
||||||
|
del x, y
|
||||||
|
# error: [unresolved-reference]
|
||||||
|
reveal_type(x) # revealed: Unknown
|
||||||
|
# error: [unresolved-reference]
|
||||||
|
reveal_type(y) # revealed: Unknown
|
||||||
|
|
||||||
|
def cond() -> bool:
|
||||||
|
return True
|
||||||
|
|
||||||
|
b = 1
|
||||||
|
if cond():
|
||||||
|
del b
|
||||||
|
|
||||||
|
# error: [possibly-unresolved-reference]
|
||||||
|
reveal_type(b) # revealed: Literal[1]
|
||||||
|
|
||||||
|
c = 1
|
||||||
|
if cond():
|
||||||
|
c = 2
|
||||||
|
else:
|
||||||
|
del c
|
||||||
|
|
||||||
|
# error: [possibly-unresolved-reference]
|
||||||
|
reveal_type(c) # revealed: Literal[2]
|
||||||
|
|
||||||
|
d = 1
|
||||||
|
|
||||||
|
def delete():
|
||||||
|
# TODO: this results in `UnboundLocalError`; we should emit `unresolved-reference`
|
||||||
|
del d
|
||||||
|
|
||||||
|
delete()
|
||||||
|
reveal_type(d) # revealed: Literal[1]
|
||||||
|
|
||||||
|
def delete_global():
|
||||||
|
global d
|
||||||
|
del d
|
||||||
|
|
||||||
|
delete_global()
|
||||||
|
# The variable should have been removed, but we won't check it for now.
|
||||||
|
reveal_type(d) # revealed: Literal[1]
|
||||||
|
```
|
||||||
|
|
||||||
|
## Delete attributes
|
||||||
|
|
||||||
|
If an attribute is referenced after being deleted, it will be an error at runtime. But we don't
|
||||||
|
treat this as an error (because there may have been a redefinition by a method between the `del`
|
||||||
|
statement and the reference). However, deleting an attribute invalidates type narrowing by
|
||||||
|
assignment, and the attribute type will be the originally declared type.
|
||||||
|
|
||||||
|
### Invalidate narrowing
|
||||||
|
|
||||||
|
```py
|
||||||
|
class C:
|
||||||
|
x: int = 1
|
||||||
|
|
||||||
|
c = C()
|
||||||
|
del c.x
|
||||||
|
reveal_type(c.x) # revealed: int
|
||||||
|
|
||||||
|
# error: [unresolved-attribute]
|
||||||
|
del c.non_existent
|
||||||
|
|
||||||
|
c.x = 1
|
||||||
|
reveal_type(c.x) # revealed: Literal[1]
|
||||||
|
del c.x
|
||||||
|
reveal_type(c.x) # revealed: int
|
||||||
|
```
|
||||||
|
|
||||||
|
### Delete an instance attribute definition
|
||||||
|
|
||||||
|
```py
|
||||||
|
class C:
|
||||||
|
x: int = 1
|
||||||
|
|
||||||
|
c = C()
|
||||||
|
reveal_type(c.x) # revealed: int
|
||||||
|
|
||||||
|
del C.x
|
||||||
|
c = C()
|
||||||
|
# This attribute is unresolved, but we won't check it for now.
|
||||||
|
reveal_type(c.x) # revealed: int
|
||||||
|
```
|
||||||
|
|
||||||
|
## Delete items
|
||||||
|
|
||||||
|
Deleting an item also invalidates the narrowing by the assignment, but accessing the item itself is
|
||||||
|
still valid.
|
||||||
|
|
||||||
|
```py
|
||||||
|
def f(l: list[int]):
|
||||||
|
del l[0]
|
||||||
|
# If the length of `l` was 1, this will be a runtime error,
|
||||||
|
# but if it was greater than that, it will not be an error.
|
||||||
|
reveal_type(l[0]) # revealed: int
|
||||||
|
|
||||||
|
# error: [call-non-callable]
|
||||||
|
del l["string"]
|
||||||
|
|
||||||
|
l[0] = 1
|
||||||
|
reveal_type(l[0]) # revealed: Literal[1]
|
||||||
|
del l[0]
|
||||||
|
reveal_type(l[0]) # revealed: int
|
||||||
|
```
|
|
@ -240,3 +240,41 @@ def _(e: Exception | type[Exception] | None):
|
||||||
def _(e: int | None):
|
def _(e: int | None):
|
||||||
raise IndexError from e # error: [invalid-raise]
|
raise IndexError from e # error: [invalid-raise]
|
||||||
```
|
```
|
||||||
|
|
||||||
|
## The caught exception is cleared at the end of the except clause
|
||||||
|
|
||||||
|
```py
|
||||||
|
e = None
|
||||||
|
reveal_type(e) # revealed: None
|
||||||
|
|
||||||
|
try:
|
||||||
|
raise ValueError()
|
||||||
|
except ValueError as e:
|
||||||
|
reveal_type(e) # revealed: ValueError
|
||||||
|
# error: [unresolved-reference]
|
||||||
|
reveal_type(e) # revealed: Unknown
|
||||||
|
|
||||||
|
e = None
|
||||||
|
|
||||||
|
def cond() -> bool:
|
||||||
|
return True
|
||||||
|
|
||||||
|
try:
|
||||||
|
if cond():
|
||||||
|
raise ValueError()
|
||||||
|
except ValueError as e:
|
||||||
|
reveal_type(e) # revealed: ValueError
|
||||||
|
# error: [possibly-unresolved-reference]
|
||||||
|
reveal_type(e) # revealed: None
|
||||||
|
|
||||||
|
def f(x: type[Exception]):
|
||||||
|
e = None
|
||||||
|
try:
|
||||||
|
raise x
|
||||||
|
except ValueError as e:
|
||||||
|
pass
|
||||||
|
except:
|
||||||
|
pass
|
||||||
|
# error: [possibly-unresolved-reference]
|
||||||
|
reveal_type(e) # revealed: None
|
||||||
|
```
|
||||||
|
|
|
@ -449,6 +449,12 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn delete_binding(&mut self, place: ScopedPlaceId) {
|
||||||
|
let is_place_name = self.current_place_table().place_expr(place).is_name();
|
||||||
|
self.current_use_def_map_mut()
|
||||||
|
.delete_binding(place, is_place_name);
|
||||||
|
}
|
||||||
|
|
||||||
/// Push a new [`Definition`] onto the list of definitions
|
/// Push a new [`Definition`] onto the list of definitions
|
||||||
/// associated with the `definition_node` AST node.
|
/// associated with the `definition_node` AST node.
|
||||||
///
|
///
|
||||||
|
@ -1817,7 +1823,7 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
|
||||||
// If `handled_exceptions` above was `None`, it's something like `except as e:`,
|
// If `handled_exceptions` above was `None`, it's something like `except as e:`,
|
||||||
// which is invalid syntax. However, it's still pretty obvious here that the user
|
// which is invalid syntax. However, it's still pretty obvious here that the user
|
||||||
// *wanted* `e` to be bound, so we should still create a definition here nonetheless.
|
// *wanted* `e` to be bound, so we should still create a definition here nonetheless.
|
||||||
if let Some(symbol_name) = symbol_name {
|
let symbol = if let Some(symbol_name) = symbol_name {
|
||||||
let symbol = self.add_symbol(symbol_name.id.clone());
|
let symbol = self.add_symbol(symbol_name.id.clone());
|
||||||
|
|
||||||
self.add_definition(
|
self.add_definition(
|
||||||
|
@ -1827,9 +1833,16 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
|
||||||
is_star: *is_star,
|
is_star: *is_star,
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
}
|
Some(symbol)
|
||||||
|
} else {
|
||||||
|
None
|
||||||
|
};
|
||||||
|
|
||||||
self.visit_body(handler_body);
|
self.visit_body(handler_body);
|
||||||
|
// The caught exception is cleared at the end of the except clause
|
||||||
|
if let Some(symbol) = symbol {
|
||||||
|
self.delete_binding(symbol);
|
||||||
|
}
|
||||||
// Each `except` block is mutually exclusive with all other `except` blocks.
|
// Each `except` block is mutually exclusive with all other `except` blocks.
|
||||||
post_except_states.push(self.flow_snapshot());
|
post_except_states.push(self.flow_snapshot());
|
||||||
|
|
||||||
|
@ -1903,13 +1916,15 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
|
||||||
walk_stmt(self, stmt);
|
walk_stmt(self, stmt);
|
||||||
}
|
}
|
||||||
ast::Stmt::Delete(ast::StmtDelete { targets, range: _ }) => {
|
ast::Stmt::Delete(ast::StmtDelete { targets, range: _ }) => {
|
||||||
|
// We will check the target expressions and then delete them.
|
||||||
|
walk_stmt(self, stmt);
|
||||||
for target in targets {
|
for target in targets {
|
||||||
if let Ok(target) = PlaceExpr::try_from(target) {
|
if let Ok(target) = PlaceExpr::try_from(target) {
|
||||||
let place_id = self.add_place(target);
|
let place_id = self.add_place(target);
|
||||||
self.current_place_table().mark_place_used(place_id);
|
self.current_place_table().mark_place_used(place_id);
|
||||||
|
self.delete_binding(place_id);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
walk_stmt(self, stmt);
|
|
||||||
}
|
}
|
||||||
ast::Stmt::Expr(ast::StmtExpr { value, range: _ }) if self.in_module_scope() => {
|
ast::Stmt::Expr(ast::StmtExpr { value, range: _ }) if self.in_module_scope() => {
|
||||||
if let Some(expr) = dunder_all_extend_argument(value) {
|
if let Some(expr) = dunder_all_extend_argument(value) {
|
||||||
|
@ -1956,7 +1971,7 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
|
||||||
}
|
}
|
||||||
(ast::ExprContext::Load, _) => (true, false),
|
(ast::ExprContext::Load, _) => (true, false),
|
||||||
(ast::ExprContext::Store, _) => (false, true),
|
(ast::ExprContext::Store, _) => (false, true),
|
||||||
(ast::ExprContext::Del, _) => (false, true),
|
(ast::ExprContext::Del, _) => (true, true),
|
||||||
(ast::ExprContext::Invalid, _) => (false, false),
|
(ast::ExprContext::Invalid, _) => (false, false),
|
||||||
};
|
};
|
||||||
let place_id = self.add_place(place_expr);
|
let place_id = self.add_place(place_expr);
|
||||||
|
|
|
@ -6145,7 +6145,11 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
||||||
fn infer_name_expression(&mut self, name: &ast::ExprName) -> Type<'db> {
|
fn infer_name_expression(&mut self, name: &ast::ExprName) -> Type<'db> {
|
||||||
match name.ctx {
|
match name.ctx {
|
||||||
ExprContext::Load => self.infer_name_load(name),
|
ExprContext::Load => self.infer_name_load(name),
|
||||||
ExprContext::Store | ExprContext::Del => Type::Never,
|
ExprContext::Store => Type::Never,
|
||||||
|
ExprContext::Del => {
|
||||||
|
self.infer_name_load(name);
|
||||||
|
Type::Never
|
||||||
|
}
|
||||||
ExprContext::Invalid => Type::unknown(),
|
ExprContext::Invalid => Type::unknown(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -6254,10 +6258,14 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
||||||
|
|
||||||
match ctx {
|
match ctx {
|
||||||
ExprContext::Load => self.infer_attribute_load(attribute),
|
ExprContext::Load => self.infer_attribute_load(attribute),
|
||||||
ExprContext::Store | ExprContext::Del => {
|
ExprContext::Store => {
|
||||||
self.infer_expression(value);
|
self.infer_expression(value);
|
||||||
Type::Never
|
Type::Never
|
||||||
}
|
}
|
||||||
|
ExprContext::Del => {
|
||||||
|
self.infer_attribute_load(attribute);
|
||||||
|
Type::Never
|
||||||
|
}
|
||||||
ExprContext::Invalid => {
|
ExprContext::Invalid => {
|
||||||
self.infer_expression(value);
|
self.infer_expression(value);
|
||||||
Type::unknown()
|
Type::unknown()
|
||||||
|
@ -7646,12 +7654,16 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
||||||
|
|
||||||
match ctx {
|
match ctx {
|
||||||
ExprContext::Load => self.infer_subscript_load(subscript),
|
ExprContext::Load => self.infer_subscript_load(subscript),
|
||||||
ExprContext::Store | ExprContext::Del => {
|
ExprContext::Store => {
|
||||||
let value_ty = self.infer_expression(value);
|
let value_ty = self.infer_expression(value);
|
||||||
let slice_ty = self.infer_expression(slice);
|
let slice_ty = self.infer_expression(slice);
|
||||||
self.infer_subscript_expression_types(value, value_ty, slice_ty);
|
self.infer_subscript_expression_types(value, value_ty, slice_ty);
|
||||||
Type::Never
|
Type::Never
|
||||||
}
|
}
|
||||||
|
ExprContext::Del => {
|
||||||
|
self.infer_subscript_load(subscript);
|
||||||
|
Type::Never
|
||||||
|
}
|
||||||
ExprContext::Invalid => {
|
ExprContext::Invalid => {
|
||||||
let value_ty = self.infer_expression(value);
|
let value_ty = self.infer_expression(value);
|
||||||
let slice_ty = self.infer_expression(slice);
|
let slice_ty = self.infer_expression(slice);
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue