mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-10-01 14:21:24 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

Apply unnecessary index rule prior to enumerate rewrite (#9012)

Browse source 
This PR adds synthetic edits to `PLR1736` to avoid removing the
referenced value as part of `FURB148`.

Closes https://github.com/astral-sh/ruff/issues/9010.
This commit is contained in:
Charlie Marsh 2023-12-05 15:25:28 -05:00 • committed by GitHub
parent 3def18fc21
commit 268d95e911
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 44 additions and 30 deletions
Download patch file Download diff file Expand all files Collapse all files

37
crates/ruff_linter/src/rules/pylint/rules/unnecessary_dict_index_lookup.rs
View file

@ -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())
}

37
crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs
View file

@ -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())
}