[refurb] Implement delete-full-slice rule (FURB131)

crates/ruff/resources/test/fixtures/refurb/FURB131.py

@@ -0,0 +1,64 @@
+from typing import Dict, List
+
+names = {"key": "value"}
+nums = [1, 2, 3]
+x = 42
+y = "hello"
+
+# these should match
+
+# FURB131
+del nums[:]
+
+
+# FURB131
+del names[:]
+
+
+# FURB131
+del x, nums[:]
+
+
+# FURB131
+del y, names[:], x
+
+
+def yes_one(x: list[int]):
+    # FURB131
+    del x[:]
+
+
+def yes_two(x: dict[int, str]):
+    # FURB131
+    del x[:]
+
+
+def yes_three(x: List[int]):
+    # FURB131
+    del x[:]
+
+
+def yes_four(x: Dict[int, str]):
+    # FURB131
+    del x[:]
+
+
+# these should not
+
+del names["key"]
+del nums[0]
+
+x = 1
+del x
+
+
+del nums[1:2]
+
+
+del nums[:2]
+del nums[1:]
+del nums[::2]
+
+
+def no_one(param):
+    del param[:]

crates/ruff/src/checkers/ast/analyze/statement.rs

@@ -1460,14 +1460,17 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
                 }
             }
         }
-        Stmt::Delete(ast::StmtDelete { targets, range: _ }) => {
+        Stmt::Delete(delete @ ast::StmtDelete { targets, range: _ }) => {
             if checker.enabled(Rule::GlobalStatement) {
                 for target in targets {
                     if let Expr::Name(ast::ExprName { id, .. }) = target {
                         pylint::rules::global_statement(checker, id);
                     }
                 }
             }
+            if checker.enabled(Rule::DeleteFullSlice) {
+                refurb::rules::delete_full_slice(checker, delete);
+            }
         }
         Stmt::Expr(ast::StmtExpr { value, range: _ }) => {
             if checker.enabled(Rule::UselessComparison) {

crates/ruff/src/codes.rs

@@ -867,6 +867,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
 
         // refurb
         (Refurb, "113") => (RuleGroup::Unspecified, rules::refurb::rules::RepeatedAppend),
+        (Refurb, "131") => (RuleGroup::Unspecified, rules::refurb::rules::DeleteFullSlice),
 
         _ => return None,
     })

crates/ruff/src/rules/refurb/helpers.rs

@@ -0,0 +1,134 @@
+use ast::{ParameterWithDefault, Parameters};
+use ruff_python_ast::{self as ast, Expr, Stmt};
+use ruff_python_semantic::analyze::type_inference::{PythonType, ResolvedPythonType};
+use ruff_python_semantic::{Binding, BindingKind};
+
+use crate::checkers::ast::Checker;
+
+trait TypeChecker {
+    const EXPR_TYPE: PythonType;
+    fn match_annotation(checker: &Checker, annotation: &Expr) -> bool;
+}
+
+fn check_type<'a, T: TypeChecker>(checker: &'a Checker, binding: &'a Binding, name: &str) -> bool {
+    assert!(binding.source.is_some());
+    let stmt = checker.semantic().statement(binding.source.unwrap());
+
+    match binding.kind {
+        BindingKind::Assignment => match stmt {
+            Stmt::Assign(ast::StmtAssign { value, .. }) => {
+                let value_type: ResolvedPythonType = value.as_ref().into();
+                let ResolvedPythonType::Atom(candidate) = value_type
+                else {
+                    return false;
+                };
+                candidate == T::EXPR_TYPE
+            }
+            Stmt::AnnAssign(ast::StmtAnnAssign { annotation, .. }) => {
+                T::match_annotation(checker, annotation.as_ref())
+            }
+            _ => false,
+        },
+        BindingKind::Argument => match stmt {
+            Stmt::FunctionDef(ast::StmtFunctionDef { parameters, .. }) => {
+                let Some(parameter) = find_parameter_by_name(parameters.as_ref(), name)
+                else {
+                    return false;
+                };
+                let Some(ref annotation) = parameter.parameter.annotation
+                else {
+                    return false;
+                };
+                T::match_annotation(checker, annotation.as_ref())
+            }
+            _ => false,
+        },
+        BindingKind::Annotation => match stmt {
+            Stmt::AnnAssign(ast::StmtAnnAssign { annotation, .. }) => {
+                T::match_annotation(checker, annotation.as_ref())
+            }
+            _ => false,
+        },
+        _ => false,
+    }
+}
+
+trait AnnotationTypeChecker {
+    const TYPING_NAME: &'static str;
+    const BUILTIN_TYPE_NAME: &'static str;
+
+    fn match_annotation(checker: &Checker, annotation: &Expr) -> bool {
+        let Expr::Subscript(ast::ExprSubscript { value, .. }) = annotation
+        else {
+            return false;
+        };
+        Self::match_builtin_type(checker, value)
+            || checker
+                .semantic()
+                .match_typing_expr(value, Self::TYPING_NAME)
+    }
+
+    fn match_builtin_type(checker: &Checker, type_expr: &Expr) -> bool {
+        let Expr::Name(ast::ExprName { id, .. }) = type_expr
+        else {
+            return false;
+        };
+        id == Self::BUILTIN_TYPE_NAME && checker.semantic().is_builtin(Self::BUILTIN_TYPE_NAME)
+    }
+}
+
+struct ListChecker;
+
+impl AnnotationTypeChecker for ListChecker {
+    const TYPING_NAME: &'static str = "List";
+    const BUILTIN_TYPE_NAME: &'static str = "list";
+}
+
+impl TypeChecker for ListChecker {
+    const EXPR_TYPE: PythonType = PythonType::List;
+    fn match_annotation(checker: &Checker, annotation: &Expr) -> bool {
+        <Self as AnnotationTypeChecker>::match_annotation(checker, annotation)
+    }
+}
+
+struct DictChecker;
+
+impl AnnotationTypeChecker for DictChecker {
+    const TYPING_NAME: &'static str = "Dict";
+    const BUILTIN_TYPE_NAME: &'static str = "dict";
+}
+
+impl TypeChecker for DictChecker {
+    const EXPR_TYPE: PythonType = PythonType::Dict;
+    fn match_annotation(checker: &Checker, annotation: &Expr) -> bool {
+        <Self as AnnotationTypeChecker>::match_annotation(checker, annotation)
+    }
+}
+
+pub(super) fn is_list<'a>(checker: &'a Checker, binding: &'a Binding, name: &str) -> bool {
+    check_type::<ListChecker>(checker, binding, name)
+}
+
+pub(super) fn is_dict<'a>(checker: &'a Checker, binding: &'a Binding, name: &str) -> bool {
+    check_type::<DictChecker>(checker, binding, name)
+}
+
+#[inline]
+fn find_parameter_by_name<'a>(
+    parameters: &'a Parameters,
+    name: &'a str,
+) -> Option<&'a ParameterWithDefault> {
+    find_parameter_by_name_impl(&parameters.args, name)
+        .or_else(|| find_parameter_by_name_impl(&parameters.posonlyargs, name))
+        .or_else(|| find_parameter_by_name_impl(&parameters.kwonlyargs, name))
+}
+
+#[inline]
+fn find_parameter_by_name_impl<'a>(
+    parameters: &'a [ParameterWithDefault],
+    name: &'a str,
+) -> Option<&'a ParameterWithDefault> {
+    parameters
+        .iter()
+        .find(|arg| arg.parameter.name.as_str() == name)
+}

crates/ruff/src/rules/refurb/mod.rs

@@ -1,4 +1,5 @@
 //! Rules from [refurb](https://pypi.org/project/refurb/)/
+mod helpers;
 pub(crate) mod rules;
 
 #[cfg(test)]
@@ -13,6 +14,7 @@ mod tests {
     use crate::{assert_messages, settings};
 
     #[test_case(Rule::RepeatedAppend, Path::new("FURB113.py"))]
+    #[test_case(Rule::DeleteFullSlice, Path::new("FURB131.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
         let diagnostics = test_path(

crates/ruff/src/rules/refurb/rules/delete_full_slice.rs

@@ -0,0 +1,148 @@
+use ast::Ranged;
+use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::{self as ast, Expr};
+use ruff_python_codegen::Generator;
+use ruff_python_semantic::Binding;
+use ruff_text_size::TextRange;
+
+use crate::checkers::ast::Checker;
+use crate::registry::AsRule;
+use crate::rules::refurb::helpers::{is_dict, is_list};
+
+/// ## What it does
+/// Checks for delete statements with full slice on lists and dictionaries.
+///
+/// ## Why is this bad?
+/// It is faster and more succinct to remove all items via the `clear()` method.
+///
+/// ## Example
+/// ```python
+/// names = {"key": "value"}
+/// nums = [1, 2, 3]
+///
+/// del names[:]
+/// del nums[:]
+/// ```
+///
+/// Use instead:
+/// ```python
+/// names = {"key": "value"}
+/// nums = [1, 2, 3]
+///
+/// names.clear()
+/// nums.clear()
+/// ```
+#[violation]
+pub struct DeleteFullSlice;
+
+impl Violation for DeleteFullSlice {
+    const AUTOFIX: AutofixKind = AutofixKind::Sometimes;
+
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Prefer `clear` over deleting the full slice")
+    }
+
+    fn autofix_title(&self) -> Option<String> {
+        Some("Replace with `clear()`".to_string())
+    }
+}
+
+// FURB131
+pub(crate) fn delete_full_slice(checker: &mut Checker, delete: &ast::StmtDelete) {
+    // ATM, we can only auto-fix when delete has a single target.
+    let only_target = delete.targets.len() == 1;
+    for target in &delete.targets {
+        let Some(name) = match_full_slice(checker, target) else {
+            continue;
+        };
+        let mut diagnostic = Diagnostic::new(DeleteFullSlice {}, delete.range);
+
+        if checker.patch(diagnostic.kind.rule()) && only_target {
+            let replacement = make_suggestion(name, checker.generator());
+            diagnostic.set_fix(Fix::suggested(Edit::replacement(
+                replacement,
+                delete.start(),
+                delete.end(),
+            )));
+        }
+
+        checker.diagnostics.push(diagnostic);
+    }
+}
+
+fn make_suggestion(name: &str, generator: Generator) -> String {
+    // Here we construct `var.clear()`
+    //
+    // And start with construction of `var`
+    let var = ast::ExprName {
+        id: name.to_string(),
+        ctx: ast::ExprContext::Load,
+        range: TextRange::default(),
+    };
+    // Make `var.clear`.
+    // NOTE: receiver is the same for all appends and that's why we can take the first.
+    let attr = ast::ExprAttribute {
+        value: Box::new(var.into()),
+        attr: ast::Identifier::new("clear".to_string(), TextRange::default()),
+        ctx: ast::ExprContext::Load,
+        range: TextRange::default(),
+    };
+    // Make it into a call `var.clear()`
+    let call = ast::ExprCall {
+        func: Box::new(attr.into()),
+        arguments: ast::Arguments {
+            args: vec![],
+            keywords: vec![],
+            range: TextRange::default(),
+        },
+        range: TextRange::default(),
+    };
+    // And finally, turn it into a statement.
+    let stmt = ast::StmtExpr {
+        value: Box::new(call.into()),
+        range: TextRange::default(),
+    };
+    generator.stmt(&stmt.into())
+}
+
+fn match_full_slice<'a>(checker: &mut Checker, expr: &'a Expr) -> Option<&'a str> {
+    // Check that it is del expr[...]
+    let Expr::Subscript(subscript) = expr else {
+        return None;
+    };
+
+    // Check that it is del expr[:]
+    let Expr::Slice(ast::ExprSlice{ lower: None, upper: None, step: None, ..}) = subscript.slice.as_ref() else {
+        return None;
+    };
+
+    // Check that it is del var[:]
+    let Expr::Name(ast::ExprName { id: name, .. }) = subscript.value.as_ref() else {
+        return None;
+    };
+
+    // Let's find definition for var
+    let scope = checker.semantic().current_scope();
+    let bindings: Vec<&Binding> = scope
+        .get_all(name)
+        .map(|binding_id| checker.semantic().binding(binding_id))
+        .collect();
+
+    // NOTE: Maybe it is too strict of a limitation, but it seems reasonable.
+    if bindings.len() != 1 {
+        return None;
+    }
+
+    let binding = bindings[0];
+    // It should only apply to variables that are known to be lists or dicts.
+    if binding.source.is_none()
+        || !(is_dict(checker, binding, name) || is_list(checker, binding, name))
+    {
+        return None;
+    }
+
+    // Name is needed for the fix suggestion.
+    Some(name)
+}

crates/ruff/src/rules/refurb/rules/mod.rs

@@ -1,3 +1,5 @@
+pub(crate) use delete_full_slice::*;
 pub(crate) use repeated_append::*;
 
+mod delete_full_slice;
 mod repeated_append;