Misc. minor refactors to incorrect-dict-iterator (#5762)

## Summary

Mostly a no-op: use a single match for key-value, use identifier range
rather than re-lexing, respect our `dummy-variable-rgx` setting.
This commit is contained in:
Charlie Marsh 2023-07-14 13:29:25 -04:00 committed by GitHub
parent 8187bf9f7e
commit 81b88dcfb9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 34 additions and 48 deletions

View file

@ -1,13 +1,12 @@
use std::fmt; use std::fmt;
use ruff_text_size::{TextRange, TextSize}; use regex::Regex;
use rustpython_parser::ast;
use rustpython_parser::ast::Expr; use rustpython_parser::ast::Expr;
use rustpython_parser::{ast, lexer, Mode, Tok}; use rustpython_parser::ast::Ranged;
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::source_code::Locator;
use rustpython_parser::ast::Ranged;
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
use crate::registry::AsRule; use crate::registry::AsRule;
@ -63,26 +62,26 @@ pub(crate) fn incorrect_dict_iterator(checker: &mut Checker, target: &Expr, iter
let Expr::Tuple(ast::ExprTuple { elts, .. }) = target else { let Expr::Tuple(ast::ExprTuple { elts, .. }) = target else {
return; return;
}; };
if elts.len() != 2 { let [key, value] = elts.as_slice() else {
return; return;
} };
let Expr::Call(ast::ExprCall { func, args, .. }) = iter else { let Expr::Call(ast::ExprCall { func, args, .. }) = iter else {
return; return;
}; };
if !args.is_empty() { if !args.is_empty() {
return; return;
} }
let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = func.as_ref() else { let Expr::Attribute(ast::ExprAttribute { attr, .. }) = func.as_ref() else {
return; return;
}; };
if attr != "items" { if attr != "items" {
return; return;
} }
let unused_key = is_ignored_tuple_or_name(&elts[0]); match (
let unused_value = is_ignored_tuple_or_name(&elts[1]); is_ignored_tuple_or_name(key, &checker.settings.dummy_variable_rgx),
is_ignored_tuple_or_name(value, &checker.settings.dummy_variable_rgx),
match (unused_key, unused_value) { ) {
(true, true) => { (true, true) => {
// Both the key and the value are unused. // Both the key and the value are unused.
} }
@ -98,14 +97,12 @@ pub(crate) fn incorrect_dict_iterator(checker: &mut Checker, target: &Expr, iter
func.range(), func.range(),
); );
if checker.patch(diagnostic.kind.rule()) { if checker.patch(diagnostic.kind.rule()) {
if let Some(range) = attribute_range(value.end(), checker.locator) { let replace_attribute = Edit::range_replacement("values".to_string(), attr.range());
let replace_attribute = Edit::range_replacement("values".to_string(), range); let replace_target = Edit::range_replacement(
let replace_target = Edit::range_replacement( checker.locator.slice(value.range()).to_string(),
checker.locator.slice(elts[1].range()).to_string(), target.range(),
target.range(), );
); diagnostic.set_fix(Fix::suggested_edits(replace_attribute, [replace_target]));
diagnostic.set_fix(Fix::suggested_edits(replace_attribute, [replace_target]));
}
} }
checker.diagnostics.push(diagnostic); checker.diagnostics.push(diagnostic);
} }
@ -118,14 +115,12 @@ pub(crate) fn incorrect_dict_iterator(checker: &mut Checker, target: &Expr, iter
func.range(), func.range(),
); );
if checker.patch(diagnostic.kind.rule()) { if checker.patch(diagnostic.kind.rule()) {
if let Some(range) = attribute_range(value.end(), checker.locator) { let replace_attribute = Edit::range_replacement("keys".to_string(), attr.range());
let replace_attribute = Edit::range_replacement("keys".to_string(), range); let replace_target = Edit::range_replacement(
let replace_target = Edit::range_replacement( checker.locator.slice(key.range()).to_string(),
checker.locator.slice(elts[0].range()).to_string(), target.range(),
target.range(), );
); diagnostic.set_fix(Fix::suggested_edits(replace_attribute, [replace_target]));
diagnostic.set_fix(Fix::suggested_edits(replace_attribute, [replace_target]));
}
} }
checker.diagnostics.push(diagnostic); checker.diagnostics.push(diagnostic);
} }
@ -148,23 +143,12 @@ impl fmt::Display for DictSubset {
} }
/// Returns `true` if the given expression is either an ignored value or a tuple of ignored values. /// Returns `true` if the given expression is either an ignored value or a tuple of ignored values.
fn is_ignored_tuple_or_name(expr: &Expr) -> bool { fn is_ignored_tuple_or_name(expr: &Expr, dummy_variable_rgx: &Regex) -> bool {
match expr { match expr {
Expr::Tuple(ast::ExprTuple { elts, .. }) => elts.iter().all(is_ignored_tuple_or_name), Expr::Tuple(ast::ExprTuple { elts, .. }) => elts
Expr::Name(ast::ExprName { id, .. }) => id == "_", .iter()
.all(|expr| is_ignored_tuple_or_name(expr, dummy_variable_rgx)),
Expr::Name(ast::ExprName { id, .. }) => dummy_variable_rgx.is_match(id.as_str()),
_ => false, _ => false,
} }
} }
/// Returns the range of the attribute identifier after the given location, if any.
fn attribute_range(at: TextSize, locator: &Locator) -> Option<TextRange> {
lexer::lex_starts_at(locator.after(at), Mode::Expression, at)
.flatten()
.find_map(|(tok, range)| {
if matches!(tok, Tok::Name { .. }) {
Some(range)
} else {
None
}
})
}

View file

@ -1,4 +1,4 @@
use rustpython_parser::ast::Stmt; use rustpython_parser::ast::{Expr, Stmt};
use ruff_diagnostics::{Diagnostic, Violation}; use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
@ -55,10 +55,12 @@ pub(crate) fn reraise_no_cause(checker: &mut Checker, body: &[Stmt]) {
}; };
for (range, exc, cause) in raises { for (range, exc, cause) in raises {
if exc.map_or(false, |expr| expr.is_call_expr() && cause.is_none()) { if cause.is_none() {
checker if exc.map_or(false, Expr::is_call_expr) {
.diagnostics checker
.push(Diagnostic::new(ReraiseNoCause, range)); .diagnostics
.push(Diagnostic::new(ReraiseNoCause, range));
}
} }
} }
} }