mirror of
https://github.com/astral-sh/ruff
synced 2026-01-21 13:30:49 -05:00
[ty] Check __delitem__ instead of __getitem__ for del x[k] (#22121)
## Summary Previously, `del x[k]` incorrectly required the object to have a `__getitem__` method. This was wrong because deletion only needs `__delitem__`, which is independent of `__getitem__`. Closes https://github.com/astral-sh/ty/issues/1799.
This commit is contained in:
@@ -171,6 +171,8 @@ reveal_type(c.x) # revealed: int
|
||||
|
||||
## Delete items
|
||||
|
||||
### Basic item deletion
|
||||
|
||||
Deleting an item also invalidates the narrowing by the assignment, but accessing the item itself is
|
||||
still valid.
|
||||
|
||||
@@ -189,3 +191,69 @@ def f(l: list[int]):
|
||||
del l[0]
|
||||
reveal_type(l[0]) # revealed: int
|
||||
```
|
||||
|
||||
### `__delitem__` without `__getitem__`
|
||||
|
||||
A class or protocol that only defines `__delitem__` (without `__getitem__`) should still support
|
||||
item deletion. The `__delitem__` method is independent of `__getitem__`.
|
||||
|
||||
```py
|
||||
from typing import Protocol, TypeVar
|
||||
|
||||
KT = TypeVar("KT")
|
||||
|
||||
class CanDelItem(Protocol[KT]):
|
||||
def __delitem__(self, k: KT, /) -> None: ...
|
||||
|
||||
def f(x: CanDelItem[int], k: int):
|
||||
# This should be valid - the object has __delitem__
|
||||
del x[k]
|
||||
|
||||
class OnlyDelItem:
|
||||
def __delitem__(self, key: int) -> None:
|
||||
pass
|
||||
|
||||
d = OnlyDelItem()
|
||||
del d[0] # OK
|
||||
|
||||
# error: [non-subscriptable] "Cannot subscript object of type `OnlyDelItem` with no `__getitem__` method"
|
||||
d[0]
|
||||
```
|
||||
|
||||
### `__getitem__` without `__delitem__`
|
||||
|
||||
A class that only defines `__getitem__` (without `__delitem__`) should not support item deletion.
|
||||
|
||||
```py
|
||||
class OnlyGetItem:
|
||||
def __getitem__(self, key: int) -> str:
|
||||
return "value"
|
||||
|
||||
g = OnlyGetItem()
|
||||
reveal_type(g[0]) # revealed: str
|
||||
|
||||
# error: [non-subscriptable] "Cannot delete subscript on object of type `OnlyGetItem` with no `__delitem__` method"
|
||||
del g[0]
|
||||
```
|
||||
|
||||
### TypedDict deletion
|
||||
|
||||
Deleting a required key from a TypedDict is a type error because it would make the object no longer
|
||||
a valid instance of that TypedDict type.
|
||||
|
||||
```py
|
||||
from typing_extensions import TypedDict
|
||||
|
||||
class Movie(TypedDict):
|
||||
name: str
|
||||
year: int
|
||||
|
||||
m: Movie = {"name": "Blade Runner", "year": 1982}
|
||||
|
||||
# error: [invalid-argument-type]
|
||||
del m["name"]
|
||||
```
|
||||
|
||||
TODO: Deletion of `NotRequired` keys (or keys in `total=False` TypedDicts) should be allowed, but ty
|
||||
currently uses the typeshed stub `__delitem__(k: Never)` which rejects all deletions. See
|
||||
mypy/pyright behavior for reference.
|
||||
|
||||
@@ -2385,10 +2385,17 @@ pub(super) fn report_non_subscriptable(
|
||||
let Some(builder) = context.report_lint(&NON_SUBSCRIPTABLE, node) else {
|
||||
return;
|
||||
};
|
||||
builder.into_diagnostic(format_args!(
|
||||
"Cannot subscript object of type `{}` with no `{method}` method",
|
||||
non_subscriptable_ty.display(context.db())
|
||||
));
|
||||
if method == "__delitem__" {
|
||||
builder.into_diagnostic(format_args!(
|
||||
"Cannot delete subscript on object of type `{}` with no `{method}` method",
|
||||
non_subscriptable_ty.display(context.db())
|
||||
));
|
||||
} else {
|
||||
builder.into_diagnostic(format_args!(
|
||||
"Cannot subscript object of type `{}` with no `{method}` method",
|
||||
non_subscriptable_ty.display(context.db())
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn report_slice_step_size_zero(context: &InferContext, node: AnyNodeRef) {
|
||||
|
||||
@@ -4304,6 +4304,161 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
||||
}
|
||||
}
|
||||
|
||||
/// Validate a subscript deletion of the form `del object[key]`.
|
||||
fn validate_subscript_deletion(
|
||||
&self,
|
||||
target: &ast::ExprSubscript,
|
||||
object_ty: Type<'db>,
|
||||
slice_ty: Type<'db>,
|
||||
) {
|
||||
self.validate_subscript_deletion_impl(target, None, object_ty, slice_ty);
|
||||
}
|
||||
|
||||
fn validate_subscript_deletion_impl(
|
||||
&self,
|
||||
target: &'ast ast::ExprSubscript,
|
||||
full_object_ty: Option<Type<'db>>,
|
||||
object_ty: Type<'db>,
|
||||
slice_ty: Type<'db>,
|
||||
) {
|
||||
let db = self.db();
|
||||
|
||||
let attach_original_type_info = |diagnostic: &mut LintDiagnosticGuard| {
|
||||
if let Some(full_object_ty) = full_object_ty {
|
||||
diagnostic.info(format_args!(
|
||||
"The full type of the subscripted object is `{}`",
|
||||
full_object_ty.display(db)
|
||||
));
|
||||
}
|
||||
};
|
||||
|
||||
match object_ty {
|
||||
Type::Union(union) => {
|
||||
for element_ty in union.elements(db) {
|
||||
self.validate_subscript_deletion_impl(
|
||||
target,
|
||||
full_object_ty.or(Some(object_ty)),
|
||||
*element_ty,
|
||||
slice_ty,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Type::Intersection(intersection) => {
|
||||
// Check if any positive element supports deletion
|
||||
let mut any_valid = false;
|
||||
for element_ty in intersection.positive(db) {
|
||||
if self.can_delete_subscript(*element_ty, slice_ty) {
|
||||
any_valid = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
// If none are valid, emit a diagnostic for the first failing element
|
||||
if !any_valid {
|
||||
if let Some(element_ty) = intersection.positive(db).first() {
|
||||
self.validate_subscript_deletion_impl(
|
||||
target,
|
||||
full_object_ty.or(Some(object_ty)),
|
||||
*element_ty,
|
||||
slice_ty,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
_ => {
|
||||
match object_ty.try_call_dunder(
|
||||
db,
|
||||
"__delitem__",
|
||||
CallArguments::positional([slice_ty]),
|
||||
TypeContext::default(),
|
||||
) {
|
||||
Ok(_) => {}
|
||||
Err(err) => match err {
|
||||
CallDunderError::PossiblyUnbound { .. } => {
|
||||
if let Some(builder) = self
|
||||
.context
|
||||
.report_lint(&POSSIBLY_MISSING_IMPLICIT_CALL, target)
|
||||
{
|
||||
let mut diagnostic = builder.into_diagnostic(format_args!(
|
||||
"Method `__delitem__` of type `{}` may be missing",
|
||||
object_ty.display(db),
|
||||
));
|
||||
attach_original_type_info(&mut diagnostic);
|
||||
}
|
||||
}
|
||||
CallDunderError::CallError(call_error_kind, bindings) => {
|
||||
match call_error_kind {
|
||||
CallErrorKind::NotCallable => {
|
||||
if let Some(builder) =
|
||||
self.context.report_lint(&CALL_NON_CALLABLE, target)
|
||||
{
|
||||
let mut diagnostic = builder.into_diagnostic(format_args!(
|
||||
"Method `__delitem__` of type `{}` is not callable \
|
||||
on object of type `{}`",
|
||||
bindings.callable_type().display(db),
|
||||
object_ty.display(db),
|
||||
));
|
||||
attach_original_type_info(&mut diagnostic);
|
||||
}
|
||||
}
|
||||
CallErrorKind::BindingError => {
|
||||
if let Some(builder) =
|
||||
self.context.report_lint(&INVALID_ARGUMENT_TYPE, target)
|
||||
{
|
||||
let mut diagnostic = builder.into_diagnostic(format_args!(
|
||||
"Method `__delitem__` of type `{}` cannot be called \
|
||||
with key of type `{}` on object of type `{}`",
|
||||
bindings.callable_type().display(db),
|
||||
slice_ty.display(db),
|
||||
object_ty.display(db),
|
||||
));
|
||||
attach_original_type_info(&mut diagnostic);
|
||||
}
|
||||
}
|
||||
CallErrorKind::PossiblyNotCallable => {
|
||||
if let Some(builder) =
|
||||
self.context.report_lint(&CALL_NON_CALLABLE, target)
|
||||
{
|
||||
let mut diagnostic = builder.into_diagnostic(format_args!(
|
||||
"Method `__delitem__` of type `{}` may not be callable \
|
||||
on object of type `{}`",
|
||||
bindings.callable_type().display(db),
|
||||
object_ty.display(db),
|
||||
));
|
||||
attach_original_type_info(&mut diagnostic);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
CallDunderError::MethodNotAvailable => {
|
||||
report_non_subscriptable(
|
||||
&self.context,
|
||||
target,
|
||||
object_ty,
|
||||
"__delitem__",
|
||||
);
|
||||
}
|
||||
},
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Check if a type supports subscript deletion (has `__delitem__`).
|
||||
fn can_delete_subscript(&self, object_ty: Type<'db>, slice_ty: Type<'db>) -> bool {
|
||||
let db = self.db();
|
||||
object_ty
|
||||
.try_call_dunder(
|
||||
db,
|
||||
"__delitem__",
|
||||
CallArguments::positional([slice_ty]),
|
||||
TypeContext::default(),
|
||||
)
|
||||
.is_ok()
|
||||
}
|
||||
|
||||
/// Make sure that the attribute assignment `obj.attribute = value` is valid.
|
||||
///
|
||||
/// `target` is the node for the left-hand side, `object_ty` is the type of `obj`, `attribute` is
|
||||
@@ -11385,7 +11540,9 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
||||
Type::Never
|
||||
}
|
||||
ExprContext::Del => {
|
||||
self.infer_subscript_load(subscript);
|
||||
let value_ty = self.infer_expression(value, TypeContext::default());
|
||||
let slice_ty = self.infer_expression(slice, TypeContext::default());
|
||||
self.validate_subscript_deletion(subscript, value_ty, slice_ty);
|
||||
Type::Never
|
||||
}
|
||||
ExprContext::Invalid => {
|
||||
|
||||
Reference in New Issue
Block a user