diff --git a/crates/ruff/resources/test/fixtures/pyflakes/F504.py b/crates/ruff/resources/test/fixtures/pyflakes/F504.py index 6d33309eaf..8a4ab822e4 100644 --- a/crates/ruff/resources/test/fixtures/pyflakes/F504.py +++ b/crates/ruff/resources/test/fixtures/pyflakes/F504.py @@ -7,3 +7,5 @@ hidden = {"a": "!"} "%(a)s" % {"a": 1, r"b": "!"} # F504 ("b" not used) "%(a)s" % {'a': 1, u"b": "!"} # F504 ("b" not used) + +'' % {'a''b' : ''} # F504 ("ab" not used) diff --git a/crates/ruff/src/rules/pyflakes/fixes.rs b/crates/ruff/src/rules/pyflakes/fixes.rs index 56d2347cc4..24a7ef82e7 100644 --- a/crates/ruff/src/rules/pyflakes/fixes.rs +++ b/crates/ruff/src/rules/pyflakes/fixes.rs @@ -1,19 +1,17 @@ use anyhow::{bail, Ok, Result}; -use libcst_native::{DictElement, Expression}; use ruff_text_size::TextRange; use rustpython_parser::ast::{Excepthandler, Expr, Ranged}; use rustpython_parser::{lexer, Mode, Tok}; use ruff_diagnostics::Edit; use ruff_python_ast::source_code::{Locator, Stylist}; -use ruff_python_ast::str::raw_contents; use crate::autofix::codemods::CodegenStylist; use crate::cst::matchers::{match_call_mut, match_dict, match_expression}; /// Generate a [`Edit`] to remove unused keys from format dict. pub(crate) fn remove_unused_format_arguments_from_dict( - unused_arguments: &[&str], + unused_arguments: &[usize], stmt: &Expr, locator: &Locator, stylist: &Stylist, @@ -22,11 +20,12 @@ pub(crate) fn remove_unused_format_arguments_from_dict( let mut tree = match_expression(module_text)?; let dict = match_dict(&mut tree)?; - dict.elements.retain(|e| { - !matches!(e, DictElement::Simple { - key: Expression::SimpleString(name), - .. - } if raw_contents(name.value).map_or(false, |name| unused_arguments.contains(&name))) + // Remove the elements at the given indexes. + let mut index = 0; + dict.elements.retain(|_| { + let is_unused = unused_arguments.contains(&index); + index += 1; + !is_unused }); Ok(Edit::range_replacement( @@ -37,7 +36,7 @@ pub(crate) fn remove_unused_format_arguments_from_dict( /// Generate a [`Edit`] to remove unused keyword arguments from a `format` call. pub(crate) fn remove_unused_keyword_arguments_from_format_call( - unused_arguments: &[&str], + unused_arguments: &[usize], location: TextRange, locator: &Locator, stylist: &Stylist, @@ -46,8 +45,17 @@ pub(crate) fn remove_unused_keyword_arguments_from_format_call( let mut tree = match_expression(module_text)?; let call = match_call_mut(&mut tree)?; - call.args - .retain(|e| !matches!(&e.keyword, Some(kw) if unused_arguments.contains(&kw.value))); + // Remove the keyword arguments at the given indexes. + let mut index = 0; + call.args.retain(|arg| { + if arg.keyword.is_none() { + return true; + } + + let is_unused = unused_arguments.contains(&index); + index += 1; + !is_unused + }); Ok(Edit::range_replacement( tree.codegen_stylist(stylist), diff --git a/crates/ruff/src/rules/pyflakes/rules/strings.rs b/crates/ruff/src/rules/pyflakes/rules/strings.rs index 11edb1bc45..a89a6b602a 100644 --- a/crates/ruff/src/rules/pyflakes/rules/strings.rs +++ b/crates/ruff/src/rules/pyflakes/rules/strings.rs @@ -574,13 +574,15 @@ pub(crate) fn percent_format_extra_named_arguments( let Expr::Dict(ast::ExprDict { keys, .. }) = &right else { return; }; + // If any of the keys are spread, abort. if keys.iter().any(Option::is_none) { - return; // contains **x splat + return; } - let missing: Vec<&str> = keys + let missing: Vec<(usize, &str)> = keys .iter() - .filter_map(|k| match k { + .enumerate() + .filter_map(|(index, key)| match key { Some(Expr::Constant(ast::ExprConstant { value: Constant::Str(value), .. @@ -588,7 +590,7 @@ pub(crate) fn percent_format_extra_named_arguments( if summary.keywords.contains(value) { None } else { - Some(value.as_str()) + Some((index, value.as_str())) } } _ => None, @@ -599,16 +601,19 @@ pub(crate) fn percent_format_extra_named_arguments( return; } + let names: Vec = missing + .iter() + .map(|(_, name)| (*name).to_string()) + .collect(); let mut diagnostic = Diagnostic::new( - PercentFormatExtraNamedArguments { - missing: missing.iter().map(|&arg| arg.to_string()).collect(), - }, + PercentFormatExtraNamedArguments { missing: names }, location, ); if checker.patch(diagnostic.kind.rule()) { + let indexes: Vec = missing.iter().map(|(index, _)| *index).collect(); diagnostic.try_set_fix(|| { let edit = remove_unused_format_arguments_from_dict( - &missing, + &indexes, right, checker.locator, checker.stylist, @@ -742,21 +747,22 @@ pub(crate) fn string_dot_format_extra_named_arguments( keywords: &[Keyword], location: TextRange, ) { + // If there are any **kwargs, abort. if has_star_star_kwargs(keywords) { return; } - let keywords = keywords.iter().filter_map(|k| { - let Keyword { arg, .. } = &k; - arg.as_ref() - }); + let keywords = keywords + .iter() + .filter_map(|Keyword { arg, .. }| arg.as_ref()); - let missing: Vec<&str> = keywords - .filter_map(|arg| { - if summary.keywords.contains(arg.as_ref()) { + let missing: Vec<(usize, &str)> = keywords + .enumerate() + .filter_map(|(index, keyword)| { + if summary.keywords.contains(keyword.as_ref()) { None } else { - Some(arg.as_str()) + Some((index, keyword.as_str())) } }) .collect(); @@ -765,16 +771,19 @@ pub(crate) fn string_dot_format_extra_named_arguments( return; } + let names: Vec = missing + .iter() + .map(|(_, name)| (*name).to_string()) + .collect(); let mut diagnostic = Diagnostic::new( - StringDotFormatExtraNamedArguments { - missing: missing.iter().map(|&arg| arg.to_string()).collect(), - }, + StringDotFormatExtraNamedArguments { missing: names }, location, ); if checker.patch(diagnostic.kind.rule()) { + let indexes: Vec = missing.iter().map(|(index, _)| *index).collect(); diagnostic.try_set_fix(|| { let edit = remove_unused_keyword_arguments_from_format_call( - &missing, + &indexes, location, checker.locator, checker.stylist, diff --git a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F504_F504.py.snap b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F504_F504.py.snap index a8f8bc10aa..8428edeef4 100644 --- a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F504_F504.py.snap +++ b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F504_F504.py.snap @@ -38,20 +38,42 @@ F504.py:8:1: F504 [*] `%`-format string has unused named argument(s): b 8 |-"%(a)s" % {"a": 1, r"b": "!"} # F504 ("b" not used) 8 |+"%(a)s" % {"a": 1, } # F504 ("b" not used) 9 9 | "%(a)s" % {'a': 1, u"b": "!"} # F504 ("b" not used) +10 10 | +11 11 | '' % {'a''b' : ''} # F504 ("ab" not used) F504.py:9:1: F504 [*] `%`-format string has unused named argument(s): b | 9 | "%(a)s" % {"a": 1, r"b": "!"} # F504 ("b" not used) 10 | "%(a)s" % {'a': 1, u"b": "!"} # F504 ("b" not used) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ F504 +11 | +12 | '' % {'a''b' : ''} # F504 ("ab" not used) | = help: Remove extra named arguments: b ℹ Fix -6 6 | "%(a)s %(c)s" % {"x": 1, **hidden} # Ok (cannot see through splat) -7 7 | -8 8 | "%(a)s" % {"a": 1, r"b": "!"} # F504 ("b" not used) -9 |-"%(a)s" % {'a': 1, u"b": "!"} # F504 ("b" not used) - 9 |+"%(a)s" % {'a': 1, } # F504 ("b" not used) +6 6 | "%(a)s %(c)s" % {"x": 1, **hidden} # Ok (cannot see through splat) +7 7 | +8 8 | "%(a)s" % {"a": 1, r"b": "!"} # F504 ("b" not used) +9 |-"%(a)s" % {'a': 1, u"b": "!"} # F504 ("b" not used) + 9 |+"%(a)s" % {'a': 1, } # F504 ("b" not used) +10 10 | +11 11 | '' % {'a''b' : ''} # F504 ("ab" not used) + +F504.py:11:1: F504 [*] `%`-format string has unused named argument(s): ab + | +11 | "%(a)s" % {'a': 1, u"b": "!"} # F504 ("b" not used) +12 | +13 | '' % {'a''b' : ''} # F504 ("ab" not used) + | ^^^^^^^^^^^^^^^^^^ F504 + | + = help: Remove extra named arguments: ab + +ℹ Fix +8 8 | "%(a)s" % {"a": 1, r"b": "!"} # F504 ("b" not used) +9 9 | "%(a)s" % {'a': 1, u"b": "!"} # F504 ("b" not used) +10 10 | +11 |-'' % {'a''b' : ''} # F504 ("ab" not used) + 11 |+'' % {} # F504 ("ab" not used)