Add dedicated method to find typed binding

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

@@ -43,6 +43,12 @@ def yes_four(x: Dict[int, str]):
     del x[:]
 
 
+def yes_five(x: Dict[int, str]):
+    # FURB131
+    del x[:]
+
+    x = 1
+
 # these should not
 
 del names["key"]

crates/ruff_linter/src/rules/perflint/rules/manual_dict_comprehension.rs

@@ -4,7 +4,6 @@ use ruff_python_ast::comparable::ComparableExpr;
 use ruff_python_ast::helpers::any_over_expr;
 use ruff_python_ast::{self as ast, Expr, Stmt};
 use ruff_python_semantic::analyze::typing::is_dict;
-use ruff_python_semantic::Binding;
 
 use crate::checkers::ast::Checker;
 
@@ -129,22 +128,16 @@ pub(crate) fn manual_dict_comprehension(checker: &mut Checker, target: &Expr, bo
     }
 
     // Exclude non-dictionary value.
-    let Expr::Name(ast::ExprName {
-        id: subscript_name, ..
-    }) = subscript_value.as_ref()
-    else {
+    let Some(name) = subscript_value.as_name_expr() else {
         return;
     };
-    let scope = checker.semantic().current_scope();
-    let bindings: Vec<&Binding> = scope
-        .get_all(subscript_name)
-        .map(|binding_id| checker.semantic().binding(binding_id))
-        .collect();
-
-    let [binding] = bindings.as_slice() else {
+    let Some(binding) = checker
+        .semantic()
+        .only_binding(name)
+        .map(|id| checker.semantic().binding(id))
+    else {
         return;
     };
-
     if !is_dict(binding, checker.semantic()) {
         return;
     }
@@ -165,8 +158,7 @@ pub(crate) fn manual_dict_comprehension(checker: &mut Checker, target: &Expr, bo
     // ```
     if if_test.is_some_and(|test| {
         any_over_expr(test, &|expr| {
-            expr.as_name_expr()
-                .is_some_and(|expr| expr.id == *subscript_name)
+            ComparableExpr::from(expr) == ComparableExpr::from(name)
         })
     }) {
         return;

crates/ruff_linter/src/rules/perflint/rules/manual_list_comprehension.rs

@@ -5,7 +5,6 @@ use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::comparable::ComparableExpr;
 use ruff_python_ast::helpers::any_over_expr;
 use ruff_python_semantic::analyze::typing::is_list;
-use ruff_python_semantic::Binding;
 
 use crate::checkers::ast::Checker;
 
@@ -144,20 +143,16 @@ pub(crate) fn manual_list_comprehension(checker: &mut Checker, target: &Expr, bo
     }
 
     // Avoid non-list values.
-    let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() else {
+    let Some(name) = value.as_name_expr() else {
         return;
     };
-    let bindings: Vec<&Binding> = checker
+    let Some(binding) = checker
         .semantic()
-        .current_scope()
-        .get_all(id)
-        .map(|binding_id| checker.semantic().binding(binding_id))
-        .collect();
-
-    let [binding] = bindings.as_slice() else {
+        .only_binding(name)
+        .map(|id| checker.semantic().binding(id))
+    else {
         return;
     };
-
     if !is_list(binding, checker.semantic()) {
         return;
     }
@@ -176,15 +171,12 @@ pub(crate) fn manual_list_comprehension(checker: &mut Checker, target: &Expr, bo
     // ```python
     // filtered = [x for x in y if x in filtered]
     // ```
-    if let Some(value_name) = value.as_name_expr() {
-        if if_test.is_some_and(|test| {
-            any_over_expr(test, &|expr| {
-                expr.as_name_expr()
-                    .is_some_and(|expr| expr.id == value_name.id)
-            })
-        }) {
-            return;
-        }
+    if if_test.is_some_and(|test| {
+        any_over_expr(test, &|expr| {
+            expr.as_name_expr().is_some_and(|expr| expr.id == name.id)
+        })
+    }) {
+        return;
     }
 
     checker

crates/ruff_linter/src/rules/perflint/rules/manual_list_copy.rs

@@ -3,7 +3,6 @@ use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::helpers::any_over_expr;
 use ruff_python_ast::{self as ast, Arguments, Expr, Stmt};
 use ruff_python_semantic::analyze::typing::is_list;
-use ruff_python_semantic::Binding;
 
 use crate::checkers::ast::Checker;
 
@@ -102,20 +101,16 @@ pub(crate) fn manual_list_copy(checker: &mut Checker, target: &Expr, body: &[Stm
     }
 
     // Avoid non-list values.
-    let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() else {
+    let Some(name) = value.as_name_expr() else {
         return;
     };
-    let bindings: Vec<&Binding> = checker
+    let Some(binding) = checker
         .semantic()
-        .current_scope()
-        .get_all(id)
-        .map(|binding_id| checker.semantic().binding(binding_id))
-        .collect();
-
-    let [binding] = bindings.as_slice() else {
+        .only_binding(name)
+        .map(|id| checker.semantic().binding(id))
+    else {
         return;
     };
-
     if !is_list(binding, checker.semantic()) {
         return;
     }

crates/ruff_linter/src/rules/refurb/rules/check_and_remove_from_set.rs

@@ -97,9 +97,9 @@ pub(crate) fn check_and_remove_from_set(checker: &mut Checker, if_stmt: &ast::St
     // Check if what we assume is set is indeed a set.
     if !checker
         .semantic()
-        .resolve_name(check_set)
-        .map(|binding_id| checker.semantic().binding(binding_id))
-        .map_or(false, |binding| is_set(binding, checker.semantic()))
+        .only_binding(check_set)
+        .map(|id| checker.semantic().binding(id))
+        .is_some_and(|binding| is_set(binding, checker.semantic()))
     {
         return;
     };

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

@@ -2,7 +2,7 @@ use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::{self as ast, Expr};
 use ruff_python_semantic::analyze::typing::{is_dict, is_list};
-use ruff_python_semantic::{Binding, SemanticModel};
+use ruff_python_semantic::SemanticModel;
 use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
@@ -70,7 +70,7 @@ pub(crate) fn delete_full_slice(checker: &mut Checker, delete: &ast::StmtDelete)
 
         // Fix is only supported for single-target deletions.
         if delete.targets.len() == 1 {
-            let replacement = generate_method_call(name, "clear", checker.generator());
+            let replacement = generate_method_call(&name.id, "clear", checker.generator());
             diagnostic.set_fix(Fix::unsafe_edit(Edit::replacement(
                 replacement,
                 delete.start(),
@@ -83,7 +83,7 @@ pub(crate) fn delete_full_slice(checker: &mut Checker, delete: &ast::StmtDelete)
 }
 
 /// Match `del expr[:]` where `expr` is a list or a dict.
-fn match_full_slice<'a>(expr: &'a Expr, semantic: &SemanticModel) -> Option<&'a str> {
+fn match_full_slice<'a>(expr: &'a Expr, semantic: &SemanticModel) -> Option<&'a ast::ExprName> {
     // Check that it is `del expr[...]`.
     let subscript = expr.as_subscript_expr()?;
 
@@ -100,22 +100,9 @@ fn match_full_slice<'a>(expr: &'a Expr, semantic: &SemanticModel) -> Option<&'a
         return None;
     }
 
-    // Check that it is del var[:]
-    let ast::ExprName { id: name, .. } = subscript.value.as_name_expr()?;
-
-    // Let's find definition for var
-    let scope = semantic.current_scope();
-    let bindings: Vec<&Binding> = scope
-        .get_all(name)
-        .map(|binding_id| semantic.binding(binding_id))
-        .collect();
-
-    // NOTE: Maybe it is too strict of a limitation, but it seems reasonable.
-    let [binding] = bindings.as_slice() else {
-        return None;
-    };
-
     // It should only apply to variables that are known to be lists or dicts.
+    let name = subscript.value.as_name_expr()?;
+    let binding = semantic.binding(semantic.only_binding(name)?);
     if !(is_dict(binding, semantic) || is_list(binding, semantic)) {
         return None;
     }

crates/ruff_linter/src/rules/refurb/rules/repeated_append.rs

@@ -76,7 +76,7 @@ impl Violation for RepeatedAppend {
 
 /// FURB113
 pub(crate) fn repeated_append(checker: &mut Checker, stmt: &Stmt) {
-    let Some(appends) = match_consecutive_appends(checker.semantic(), stmt) else {
+    let Some(appends) = match_consecutive_appends(stmt, checker.semantic()) else {
         return;
     };
 
@@ -163,8 +163,8 @@ impl Ranged for AppendGroup<'_> {
 
 /// Match consecutive calls to `append` on list variables starting from the given statement.
 fn match_consecutive_appends<'a>(
-    semantic: &'a SemanticModel,
     stmt: &'a Stmt,
+    semantic: &'a SemanticModel,
 ) -> Option<Vec<Append<'a>>> {
     // Match the current statement, to see if it's an append.
     let append = match_append(semantic, stmt)?;

crates/ruff_linter/src/rules/refurb/rules/unnecessary_enumerate.rs

@@ -6,7 +6,7 @@ use ruff_python_ast as ast;
 use ruff_python_ast::{Arguments, Expr, Int};
 use ruff_python_codegen::Generator;
 use ruff_python_semantic::analyze::typing::{is_dict, is_list, is_set, is_tuple};
-use ruff_python_semantic::Binding;
+
 use ruff_text_size::{Ranged, TextRange};
 
 use crate::checkers::ast::Checker;
@@ -114,7 +114,7 @@ pub(crate) fn unnecessary_enumerate(checker: &mut Checker, stmt_for: &ast::StmtF
     };
 
     // Get the first argument, which is the sequence to iterate over.
-    let Some(Expr::Name(ast::ExprName { id: sequence, .. })) = arguments.args.first() else {
+    let Some(Expr::Name(sequence)) = arguments.args.first() else {
         return;
     };
 
@@ -138,7 +138,8 @@ pub(crate) fn unnecessary_enumerate(checker: &mut Checker, stmt_for: &ast::StmtF
             );
 
             // The index is unused, so replace with `for value in sequence`.
-            let replace_iter = Edit::range_replacement(sequence.into(), stmt_for.iter.range());
+            let replace_iter =
+                Edit::range_replacement(sequence.id.to_string(), stmt_for.iter.range());
             let replace_target = Edit::range_replacement(
                 pad(
                     checker.locator().slice(value).to_string(),
@@ -154,12 +155,11 @@ pub(crate) fn unnecessary_enumerate(checker: &mut Checker, stmt_for: &ast::StmtF
         (false, true) => {
             // Ensure the sequence object works with `len`. If it doesn't, the
             // fix is unclear.
-            let scope = checker.semantic().current_scope();
-            let bindings: Vec<&Binding> = scope
-                .get_all(sequence)
-                .map(|binding_id| checker.semantic().binding(binding_id))
-                .collect();
-            let [binding] = bindings.as_slice() else {
+            let Some(binding) = checker
+                .semantic()
+                .only_binding(sequence)
+                .map(|id| checker.semantic().binding(id))
+            else {
                 return;
             };
             // This will lead to a lot of false negatives, but it is the best
@@ -193,7 +193,7 @@ pub(crate) fn unnecessary_enumerate(checker: &mut Checker, stmt_for: &ast::StmtF
                     )
                 }) {
                     let replace_iter = Edit::range_replacement(
-                        generate_range_len_call(sequence, checker.generator()),
+                        generate_range_len_call(&sequence.id, checker.generator()),
                         stmt_for.iter.range(),
                     );
 

crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB131_FURB131.py.snap

@@ -127,6 +127,27 @@ FURB131.py:43:5: FURB131 [*] Prefer `clear` over deleting a full slice
    43 |+    x.clear()
 44 44 | 
 45 45 | 
-46 46 | # these should not
+46 46 | def yes_five(x: Dict[int, str]):
+
+FURB131.py:48:5: FURB131 [*] Prefer `clear` over deleting a full slice
+   |
+46 | def yes_five(x: Dict[int, str]):
+47 |     # FURB131
+48 |     del x[:]
+   |     ^^^^^^^^ FURB131
+49 | 
+50 |     x = 1
+   |
+   = help: Replace with `clear()`
+
+ℹ Suggested fix
+45 45 | 
+46 46 | def yes_five(x: Dict[int, str]):
+47 47 |     # FURB131
+48    |-    del x[:]
+   48 |+    x.clear()
+49 49 | 
+50 50 |     x = 1
+51 51 | 
 
 

crates/ruff_python_semantic/src/model.rs

@@ -616,6 +616,16 @@ impl<'a> SemanticModel<'a> {
         self.resolved_names.get(&name.into()).copied()
     }
 
+    /// Resolves the [`ast::ExprName`] to the [`BindingId`] of the symbol it refers to, if it's the
+    /// only binding to that name in its scope.
+    pub fn only_binding(&self, name: &ast::ExprName) -> Option<BindingId> {
+        self.resolve_name(name).filter(|id| {
+            let binding = self.binding(*id);
+            let scope = &self.scopes[binding.scope];
+            scope.shadowed_binding(*id).is_none()
+        })
+    }
+
     /// Resolves the [`Expr`] to a fully-qualified symbol-name, if `value` resolves to an imported
     /// or builtin symbol.
     ///