Skip to content

Commit

Permalink
Support concatenated string key removals (#4976)
Browse files Browse the repository at this point in the history
  • Loading branch information
@charliermarsh
charliermarsh committed Jun 9, 2023
1 parent 63fdcea commit d647105
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 36 deletions.
2 changes: 2 additions & 0 deletions crates/ruff/resources/test/fixtures/pyflakes/F504.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@

"%(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)
30 changes: 19 additions & 11 deletions crates/ruff/src/rules/pyflakes/fixes.rs
View file
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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(
Expand All @@ -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,
Expand All @@ -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),
Expand Down
49 changes: 29 additions & 20 deletions crates/ruff/src/rules/pyflakes/rules/strings.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -574,21 +574,23 @@ 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),
..
})) => {
if summary.keywords.contains(value) {
None
} else {
Some(value.as_str())
Some((index, value.as_str()))
}
}
_ => None,
Expand All @@ -599,16 +601,19 @@ pub(crate) fn percent_format_extra_named_arguments(
return;
}

let names: Vec<String> = 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<usize> = 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,
Expand Down Expand Up @@ -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();
Expand All @@ -765,16 +771,19 @@ pub(crate) fn string_dot_format_extra_named_arguments(
return;
}

let names: Vec<String> = 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<usize> = 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,
Expand Down
32 changes: 27 additions & 5 deletions crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F504_F504.py.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)


0 comments on commit d647105

Please sign in to comment.