diff --git a/crates/ruff_linter/src/rules/pylint/rules/unnecessary_dict_index_lookup.rs b/crates/ruff_linter/src/rules/pylint/rules/unnecessary_dict_index_lookup.rs index 88a842b7d6b4b..913279081fcc4 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/unnecessary_dict_index_lookup.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/unnecessary_dict_index_lookup.rs @@ -2,6 +2,7 @@ use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::visitor::Visitor; use ruff_python_ast::{self as ast, Expr, StmtFor}; +use ruff_text_size::Ranged; use crate::checkers::ast::Checker; use crate::rules::pylint::helpers::SequenceIndexVisitor; @@ -51,7 +52,7 @@ pub(crate) fn unnecessary_dict_index_lookup(checker: &mut Checker, stmt_for: &St }; let ranges = { - let mut visitor = SequenceIndexVisitor::new(dict_name, index_name, value_name); + let mut visitor = SequenceIndexVisitor::new(&dict_name.id, &index_name.id, &value_name.id); visitor.visit_body(&stmt_for.body); visitor.visit_body(&stmt_for.orelse); visitor.into_accesses() @@ -59,10 +60,10 @@ pub(crate) fn unnecessary_dict_index_lookup(checker: &mut Checker, stmt_for: &St for range in ranges { let mut diagnostic = Diagnostic::new(UnnecessaryDictIndexLookup, range); - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - value_name.to_string(), - range, - ))); + diagnostic.set_fix(Fix::safe_edits( + Edit::range_replacement(value_name.id.to_string(), range), + [noop(index_name), noop(value_name)], + )); checker.diagnostics.push(diagnostic); } } @@ -93,7 +94,8 @@ pub(crate) fn unnecessary_dict_index_lookup_comprehension(checker: &mut Checker, }; let ranges = { - let mut visitor = SequenceIndexVisitor::new(dict_name, index_name, value_name); + let mut visitor = + SequenceIndexVisitor::new(&dict_name.id, &index_name.id, &value_name.id); visitor.visit_expr(elt.as_ref()); for expr in &comp.ifs { visitor.visit_expr(expr); @@ -103,10 +105,10 @@ pub(crate) fn unnecessary_dict_index_lookup_comprehension(checker: &mut Checker, for range in ranges { let mut diagnostic = Diagnostic::new(UnnecessaryDictIndexLookup, range); - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - value_name.to_string(), - range, - ))); + diagnostic.set_fix(Fix::safe_edits( + Edit::range_replacement(value_name.id.to_string(), range), + [noop(index_name), noop(value_name)], + )); checker.diagnostics.push(diagnostic); } } @@ -115,7 +117,7 @@ pub(crate) fn unnecessary_dict_index_lookup_comprehension(checker: &mut Checker, fn dict_items<'a>( call_expr: &'a Expr, tuple_expr: &'a Expr, -) -> Option<(&'a str, &'a str, &'a str)> { +) -> Option<(&'a ast::ExprName, &'a ast::ExprName, &'a ast::ExprName)> { let ast::ExprCall { func, arguments, .. } = call_expr.as_call_expr()?; @@ -130,7 +132,7 @@ fn dict_items<'a>( return None; } - let Expr::Name(ast::ExprName { id: dict_name, .. }) = value.as_ref() else { + let Expr::Name(dict_name) = value.as_ref() else { return None; }; @@ -142,19 +144,24 @@ fn dict_items<'a>( }; // Grab the variable names. - let Expr::Name(ast::ExprName { id: index_name, .. }) = index else { + let Expr::Name(index_name) = index else { return None; }; - let Expr::Name(ast::ExprName { id: value_name, .. }) = value else { + let Expr::Name(value_name) = value else { return None; }; // If either of the variable names are intentionally ignored by naming them `_`, then don't // emit. - if index_name == "_" || value_name == "_" { + if index_name.id == "_" || value_name.id == "_" { return None; } Some((dict_name, index_name, value_name)) } + +/// Return a no-op edit for the given name. +fn noop(name: &ast::ExprName) -> Edit { + Edit::range_replacement(name.id.to_string(), name.range()) +} diff --git a/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs b/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs index 920d2442e9b5a..461a8d56c94c6 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs @@ -3,6 +3,7 @@ use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::visitor::Visitor; use ruff_python_ast::{self as ast, Expr, StmtFor}; use ruff_python_semantic::SemanticModel; +use ruff_text_size::Ranged; use crate::checkers::ast::Checker; use crate::rules::pylint::helpers::SequenceIndexVisitor; @@ -53,7 +54,7 @@ pub(crate) fn unnecessary_list_index_lookup(checker: &mut Checker, stmt_for: &St }; let ranges = { - let mut visitor = SequenceIndexVisitor::new(sequence, index_name, value_name); + let mut visitor = SequenceIndexVisitor::new(&sequence.id, &index_name.id, &value_name.id); visitor.visit_body(&stmt_for.body); visitor.visit_body(&stmt_for.orelse); visitor.into_accesses() @@ -61,10 +62,10 @@ pub(crate) fn unnecessary_list_index_lookup(checker: &mut Checker, stmt_for: &St for range in ranges { let mut diagnostic = Diagnostic::new(UnnecessaryListIndexLookup, range); - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - value_name.to_string(), - range, - ))); + diagnostic.set_fix(Fix::safe_edits( + Edit::range_replacement(value_name.id.to_string(), range), + [noop(index_name), noop(value_name)], + )); checker.diagnostics.push(diagnostic); } } @@ -97,17 +98,18 @@ pub(crate) fn unnecessary_list_index_lookup_comprehension(checker: &mut Checker, }; let ranges = { - let mut visitor = SequenceIndexVisitor::new(sequence, index_name, value_name); + let mut visitor = + SequenceIndexVisitor::new(&sequence.id, &index_name.id, &value_name.id); visitor.visit_expr(elt.as_ref()); visitor.into_accesses() }; for range in ranges { let mut diagnostic = Diagnostic::new(UnnecessaryListIndexLookup, range); - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - value_name.to_string(), - range, - ))); + diagnostic.set_fix(Fix::safe_edits( + Edit::range_replacement(value_name.id.to_string(), range), + [noop(index_name), noop(value_name)], + )); checker.diagnostics.push(diagnostic); } } @@ -117,7 +119,7 @@ fn enumerate_items<'a>( call_expr: &'a Expr, tuple_expr: &'a Expr, semantic: &SemanticModel, -) -> Option<(&'a str, &'a str, &'a str)> { +) -> Option<(&'a ast::ExprName, &'a ast::ExprName, &'a ast::ExprName)> { let ast::ExprCall { func, arguments, .. } = call_expr.as_call_expr()?; @@ -138,24 +140,29 @@ fn enumerate_items<'a>( }; // Grab the variable names. - let Expr::Name(ast::ExprName { id: index_name, .. }) = index else { + let Expr::Name(index_name) = index else { return None; }; - let Expr::Name(ast::ExprName { id: value_name, .. }) = value else { + let Expr::Name(value_name) = value else { return None; }; // If either of the variable names are intentionally ignored by naming them `_`, then don't // emit. - if index_name == "_" || value_name == "_" { + if index_name.id == "_" || value_name.id == "_" { return None; } // Get the first argument of the enumerate call. - let Some(Expr::Name(ast::ExprName { id: sequence, .. })) = arguments.args.first() else { + let Some(Expr::Name(sequence)) = arguments.args.first() else { return None; }; Some((sequence, index_name, value_name)) } + +/// Return a no-op edit for the given name. +fn noop(name: &ast::ExprName) -> Edit { + Edit::range_replacement(name.id.to_string(), name.range()) +}