Remove destructive fixes for F523 (#4883)

This commit is contained in:
Charlie Marsh 2023-06-05 20:44:30 -04:00 committed by GitHub
parent c67029ded9
commit 0c7ea800af
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 73 additions and 230 deletions

View file

@ -3,7 +3,7 @@ use libcst_native::{
Arg, Attribute, Call, Comparison, CompoundStatement, Dict, Expression, FormattedString,
FormattedStringContent, FormattedStringExpression, FunctionDef, GeneratorExp, If, Import,
ImportAlias, ImportFrom, ImportNames, IndentedBlock, Lambda, ListComp, Module, Name,
SimpleString, SmallStatement, Statement, Suite, Tuple, With,
SmallStatement, Statement, Suite, Tuple, With,
};
pub(crate) fn match_module(module_text: &str) -> Result<Module> {
@ -109,16 +109,6 @@ pub(crate) fn match_attribute<'a, 'b>(
}
}
pub(crate) fn match_simple_string<'a, 'b>(
expression: &'a mut Expression<'b>,
) -> Result<&'a mut SimpleString<'b>> {
if let Expression::SimpleString(simple_string) = expression {
Ok(simple_string)
} else {
bail!("Expected Expression::SimpleString")
}
}
pub(crate) fn match_formatted_string<'a, 'b>(
expression: &'a mut Expression<'b>,
) -> Result<&'a mut FormattedString<'b>> {

View file

@ -1,20 +1,15 @@
use anyhow::{anyhow, bail, Ok, Result};
use anyhow::{bail, Ok, Result};
use libcst_native::{DictElement, Expression};
use ruff_text_size::TextRange;
use rustpython_format::{
FieldName, FieldNamePart, FieldType, FormatPart, FormatString, FromTemplate,
};
use rustpython_parser::ast::{Excepthandler, Expr, Ranged};
use rustpython_parser::{lexer, Mode, Tok};
use crate::autofix::codemods::CodegenStylist;
use ruff_diagnostics::Edit;
use ruff_python_ast::source_code::{Locator, Stylist};
use ruff_python_ast::str::{leading_quote, raw_contents, trailing_quote};
use ruff_python_ast::str::raw_contents;
use crate::cst::matchers::{
match_attribute, match_call_mut, match_dict, match_expression, match_simple_string,
};
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(
@ -60,132 +55,25 @@ pub(crate) fn remove_unused_keyword_arguments_from_format_call(
))
}
fn unparse_format_part(format_part: FormatPart) -> String {
match format_part {
FormatPart::Literal(literal) => literal,
FormatPart::Field {
field_name,
conversion_spec,
format_spec,
} => {
let mut field_name = field_name;
if let Some(conversion) = conversion_spec {
field_name.push_str(&format!("!{conversion}"));
}
if !format_spec.is_empty() {
field_name.push_str(&format!(":{format_spec}"));
}
format!("{{{field_name}}}")
}
}
}
fn update_field_types(format_string: &FormatString, index_map: &[usize]) -> String {
format_string
.format_parts
.iter()
.map(|part| match part {
FormatPart::Literal(literal) => FormatPart::Literal(literal.to_string()),
FormatPart::Field {
field_name,
conversion_spec,
format_spec,
} => {
// SAFETY: We've already parsed this string before.
let new_field_name = FieldName::parse(field_name).unwrap();
let mut new_field_name_string = match new_field_name.field_type {
FieldType::Auto => String::new(),
FieldType::Index(i) => index_map.get(i).unwrap_or(&i).to_string(),
FieldType::Keyword(keyword) => keyword,
};
for field_name_part in &new_field_name.parts {
let field_name_part_string = match field_name_part {
FieldNamePart::Attribute(attribute) => format!(".{attribute}"),
FieldNamePart::Index(i) => format!("[{i}]"),
FieldNamePart::StringIndex(s) => format!("[{s}]"),
};
new_field_name_string.push_str(&field_name_part_string);
}
// SAFETY: We've already parsed this string before.
let new_format_spec = FormatString::from_str(format_spec).unwrap();
let new_format_spec_string = update_field_types(&new_format_spec, index_map);
FormatPart::Field {
field_name: new_field_name_string,
conversion_spec: *conversion_spec,
format_spec: new_format_spec_string,
}
}
})
.map(unparse_format_part)
.collect()
}
/// Generate a [`Edit`] to remove unused positional arguments from a `format` call.
pub(crate) fn remove_unused_positional_arguments_from_format_call(
unused_arguments: &[usize],
location: TextRange,
locator: &Locator,
stylist: &Stylist,
format_string: &FormatString,
) -> Result<Edit> {
let module_text = locator.slice(location);
let mut tree = match_expression(module_text)?;
let call = match_call_mut(&mut tree)?;
// Remove any unused arguments, and generate a map from previous index to new index.
// Remove any unused arguments.
let mut index = 0;
let mut offset = 0;
let mut index_map = Vec::with_capacity(call.args.len());
call.args.retain(|_| {
index_map.push(index - offset);
let is_unused = unused_arguments.contains(&index);
index += 1;
if is_unused {
offset += 1;
}
!is_unused
});
// If we removed an argument, we may need to rewrite the positional themselves.
// Ex) `"{1}{2}".format(a, b, c)` to `"{0}{1}".format(b, c)`
let rewrite_arguments = index_map
.iter()
.enumerate()
.filter(|&(prev_index, _)| !unused_arguments.contains(&prev_index))
.any(|(prev_index, &new_index)| prev_index != new_index);
let new_format_string;
if rewrite_arguments {
// Extract the format string verbatim.
let func = match_attribute(&mut call.func)?;
let simple_string = match_simple_string(&mut func.value)?;
// Extract existing quotes from the format string.
let leading_quote = leading_quote(simple_string.value).ok_or_else(|| {
anyhow!(
"Could not find leading quote for format string: {}",
simple_string.value
)
})?;
let trailing_quote = trailing_quote(simple_string.value).ok_or_else(|| {
anyhow!(
"Could not find trailing quote for format string: {}",
simple_string.value
)
})?;
// Update the format string, preserving the quotes.
new_format_string = format!(
"{}{}{}",
leading_quote,
update_field_types(format_string, &index_map),
trailing_quote
);
simple_string.value = new_format_string.as_str();
}
Ok(Edit::range_replacement(
tree.codegen_stylist(stylist),
location,

View file

@ -26,7 +26,6 @@ pub(crate) struct FormatSummary {
pub(crate) indices: Vec<usize>,
pub(crate) keywords: Vec<String>,
pub(crate) has_nested_parts: bool,
pub(crate) format_string: FormatString,
}
impl TryFrom<&str> for FormatSummary {
@ -75,7 +74,6 @@ impl TryFrom<&str> for FormatSummary {
indices,
keywords,
has_nested_parts,
format_string,
})
}
}

View file

@ -4,7 +4,7 @@ use ruff_text_size::TextRange;
use rustc_hash::FxHashSet;
use rustpython_parser::ast::{self, Constant, Expr, Identifier, Keyword};
use ruff_diagnostics::{AlwaysAutofixableViolation, AutofixKind, Diagnostic, Violation};
use ruff_diagnostics::{AlwaysAutofixableViolation, AutofixKind, Diagnostic, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use crate::checkers::ast::Checker;
@ -604,14 +604,14 @@ pub(crate) fn percent_format_extra_named_arguments(
location,
);
if checker.patch(diagnostic.kind.rule()) {
#[allow(deprecated)]
diagnostic.try_set_fix_from_edit(|| {
remove_unused_format_arguments_from_dict(
diagnostic.try_set_fix(|| {
let edit = remove_unused_format_arguments_from_dict(
&missing,
right,
checker.locator,
checker.stylist,
)
)?;
Ok(Fix::automatic(edit))
});
}
checker.diagnostics.push(diagnostic);
@ -770,14 +770,14 @@ pub(crate) fn string_dot_format_extra_named_arguments(
location,
);
if checker.patch(diagnostic.kind.rule()) {
#[allow(deprecated)]
diagnostic.try_set_fix_from_edit(|| {
remove_unused_keyword_arguments_from_format_call(
diagnostic.try_set_fix(|| {
let edit = remove_unused_keyword_arguments_from_format_call(
&missing,
location,
checker.locator,
checker.stylist,
)
)?;
Ok(Fix::automatic(edit))
});
}
checker.diagnostics.push(diagnostic);
@ -815,16 +815,42 @@ pub(crate) fn string_dot_format_extra_positional_arguments(
location,
);
if checker.patch(diagnostic.kind.rule()) {
#[allow(deprecated)]
diagnostic.try_set_fix_from_edit(|| {
remove_unused_positional_arguments_from_format_call(
&missing,
location,
checker.locator,
checker.stylist,
&summary.format_string,
)
});
// We can only fix if the positional arguments we're removing don't require re-indexing
// the format string itself. For example, we can't fix `"{1}{2}".format(0, 1, 2)"`, since
// this requires changing the format string to `"{0}{1}"`. But we can fix
// `"{0}{1}".format(0, 1, 2)`, since this only requires modifying the call arguments.
fn is_contiguous_from_end<T>(indexes: &[usize], target: &[T]) -> bool {
if indexes.is_empty() {
return true;
}
let mut expected_index = target.len() - 1;
for &index in indexes.iter().rev() {
if index != expected_index {
return false;
}
if expected_index == 0 {
break;
}
expected_index -= 1;
}
true
}
if is_contiguous_from_end(&missing, args) {
diagnostic.try_set_fix(|| {
let edit = remove_unused_positional_arguments_from_format_call(
&missing,
location,
checker.locator,
checker.stylist,
)?;
Ok(Fix::automatic(edit))
});
}
}
checker.diagnostics.push(diagnostic);
}

View file

@ -12,7 +12,7 @@ F504.py:3:1: F504 [*] `%`-format string has unused named argument(s): b
|
= help: Remove extra named arguments: b
Suggested fix
Fix
1 1 | # Ruff has no way of knowing if the following are F505s
2 2 | a = "wrong"
3 |-"%(a)s %(c)s" % {a: "?", "b": "!"} # F504 ("b" not used)
@ -31,7 +31,7 @@ F504.py:8:1: F504 [*] `%`-format string has unused named argument(s): b
|
= help: Remove extra named arguments: b
Suggested fix
Fix
5 5 | hidden = {"a": "!"}
6 6 | "%(a)s %(c)s" % {"x": 1, **hidden} # Ok (cannot see through splat)
7 7 |
@ -47,7 +47,7 @@ F504.py:9:1: F504 [*] `%`-format string has unused named argument(s): b
|
= help: Remove extra named arguments: b
Suggested fix
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)

View file

@ -12,7 +12,7 @@ F50x.py:8:1: F504 [*] `%`-format string has unused named argument(s): baz
|
= help: Remove extra named arguments: baz
Suggested fix
Fix
5 5 | '%s %s' % (1,) # F507
6 6 | '%s %s' % (1, 2, 3) # F507
7 7 | '%(bar)s' % {} # F505

View file

@ -10,7 +10,7 @@ F522.py:1:1: F522 [*] `.format` call has unused named argument(s): bar
|
= help: Remove extra named arguments: bar
Suggested fix
Fix
1 |-"{}".format(1, bar=2) # F522
1 |+"{}".format(1, ) # F522
2 2 | "{bar}{}".format(1, bar=2, spam=3) # F522
@ -27,7 +27,7 @@ F522.py:2:1: F522 [*] `.format` call has unused named argument(s): spam
|
= help: Remove extra named arguments: spam
Suggested fix
Fix
1 1 | "{}".format(1, bar=2) # F522
2 |-"{bar}{}".format(1, bar=2, spam=3) # F522
2 |+"{bar}{}".format(1, bar=2, ) # F522
@ -43,7 +43,7 @@ F522.py:4:1: F522 [*] `.format` call has unused named argument(s): eggs, ham
|
= help: Remove extra named arguments: eggs, ham
Suggested fix
Fix
1 1 | "{}".format(1, bar=2) # F522
2 2 | "{bar}{}".format(1, bar=2, spam=3) # F522
3 3 | "{bar:{spam}}".format(bar=2, spam=3) # No issues

View file

@ -11,7 +11,7 @@ F523.py:2:1: F523 [*] `.format` call has unused arguments at position(s): 1
|
= help: Remove extra positional arguments at position(s): 1
Suggested fix
Fix
1 1 | # With indexes
2 |-"{0}".format(1, 2) # F523
2 |+"{0}".format(1, ) # F523
@ -19,7 +19,7 @@ F523.py:2:1: F523 [*] `.format` call has unused arguments at position(s): 1
4 4 | "{1:{0}}".format(1, 2) # No issues
5 5 | "{1:{0}}".format(1, 2, 3) # F523
F523.py:3:1: F523 [*] `.format` call has unused arguments at position(s): 0, 2
F523.py:3:1: F523 `.format` call has unused arguments at position(s): 0, 2
|
3 | # With indexes
4 | "{0}".format(1, 2) # F523
@ -30,15 +30,6 @@ F523.py:3:1: F523 [*] `.format` call has unused arguments at position(s): 0, 2
|
= help: Remove extra positional arguments at position(s): 0, 2
Suggested fix
1 1 | # With indexes
2 2 | "{0}".format(1, 2) # F523
3 |-"{1}".format(1, 2, 3) # F523
3 |+"{0}".format(2, ) # F523
4 4 | "{1:{0}}".format(1, 2) # No issues
5 5 | "{1:{0}}".format(1, 2, 3) # F523
6 6 | "{0}{2}".format(1, 2) # F523, # F524
F523.py:5:1: F523 [*] `.format` call has unused arguments at position(s): 2
|
5 | "{1}".format(1, 2, 3) # F523
@ -50,7 +41,7 @@ F523.py:5:1: F523 [*] `.format` call has unused arguments at position(s): 2
|
= help: Remove extra positional arguments at position(s): 2
Suggested fix
Fix
2 2 | "{0}".format(1, 2) # F523
3 3 | "{1}".format(1, 2, 3) # F523
4 4 | "{1:{0}}".format(1, 2) # No issues
@ -70,7 +61,7 @@ F523.py:6:1: F523 [*] `.format` call has unused arguments at position(s): 1
|
= help: Remove extra positional arguments at position(s): 1
Suggested fix
Fix
3 3 | "{1}".format(1, 2, 3) # F523
4 4 | "{1:{0}}".format(1, 2) # No issues
5 5 | "{1:{0}}".format(1, 2, 3) # F523
@ -80,7 +71,7 @@ F523.py:6:1: F523 [*] `.format` call has unused arguments at position(s): 1
8 8 |
9 9 | # With no indexes
F523.py:7:1: F523 [*] `.format` call has unused arguments at position(s): 0, 3
F523.py:7:1: F523 `.format` call has unused arguments at position(s): 0, 3
|
7 | "{1:{0}}".format(1, 2, 3) # F523
8 | "{0}{2}".format(1, 2) # F523, # F524
@ -91,16 +82,6 @@ F523.py:7:1: F523 [*] `.format` call has unused arguments at position(s): 0, 3
|
= help: Remove extra positional arguments at position(s): 0, 3
Suggested fix
4 4 | "{1:{0}}".format(1, 2) # No issues
5 5 | "{1:{0}}".format(1, 2, 3) # F523
6 6 | "{0}{2}".format(1, 2) # F523, # F524
7 |-"{1.arg[1]!r:0{2['arg']}{1}}".format(1, 2, 3, 4) # F523
7 |+"{0.arg[1]!r:0{1['arg']}{0}}".format(2, 3, ) # F523
8 8 |
9 9 | # With no indexes
10 10 | "{}".format(1, 2) # F523
F523.py:10:1: F523 [*] `.format` call has unused arguments at position(s): 1
|
10 | # With no indexes
@ -111,7 +92,7 @@ F523.py:10:1: F523 [*] `.format` call has unused arguments at position(s): 1
|
= help: Remove extra positional arguments at position(s): 1
Suggested fix
Fix
7 7 | "{1.arg[1]!r:0{2['arg']}{1}}".format(1, 2, 3, 4) # F523
8 8 |
9 9 | # With no indexes
@ -132,7 +113,7 @@ F523.py:11:1: F523 [*] `.format` call has unused arguments at position(s): 1, 2
|
= help: Remove extra positional arguments at position(s): 1, 2
Suggested fix
Fix
8 8 |
9 9 | # With no indexes
10 10 | "{}".format(1, 2) # F523
@ -153,7 +134,7 @@ F523.py:13:1: F523 [*] `.format` call has unused arguments at position(s): 2
|
= help: Remove extra positional arguments at position(s): 2
Suggested fix
Fix
10 10 | "{}".format(1, 2) # F523
11 11 | "{}".format(1, 2, 3) # F523
12 12 | "{:{}}".format(1, 2) # No issues
@ -163,7 +144,7 @@ F523.py:13:1: F523 [*] `.format` call has unused arguments at position(s): 2
15 15 | # With *args
16 16 | "{0}{1}".format(*args) # No issues
F523.py:19:1: F523 [*] `.format` call has unused arguments at position(s): 2
F523.py:19:1: F523 `.format` call has unused arguments at position(s): 2
|
19 | "{0}{1}".format(1, *args) # No issues
20 | "{0}{1}".format(1, 2, *args) # No issues
@ -174,16 +155,6 @@ F523.py:19:1: F523 [*] `.format` call has unused arguments at position(s): 2
|
= help: Remove extra positional arguments at position(s): 2
Suggested fix
16 16 | "{0}{1}".format(*args) # No issues
17 17 | "{0}{1}".format(1, *args) # No issues
18 18 | "{0}{1}".format(1, 2, *args) # No issues
19 |-"{0}{1}".format(1, 2, 3, *args) # F523
19 |+"{0}{1}".format(1, 2, *args) # F523
20 20 |
21 21 | # With nested quotes
22 22 | "''1{0}".format(1, 2, 3) # F523
F523.py:22:1: F523 [*] `.format` call has unused arguments at position(s): 1, 2
|
22 | # With nested quotes
@ -194,7 +165,7 @@ F523.py:22:1: F523 [*] `.format` call has unused arguments at position(s): 1, 2
|
= help: Remove extra positional arguments at position(s): 1, 2
Suggested fix
Fix
19 19 | "{0}{1}".format(1, 2, 3, *args) # F523
20 20 |
21 21 | # With nested quotes
@ -214,7 +185,7 @@ F523.py:23:1: F523 [*] `.format` call has unused arguments at position(s): 2
|
= help: Remove extra positional arguments at position(s): 2
Suggested fix
Fix
20 20 |
21 21 | # With nested quotes
22 22 | "''1{0}".format(1, 2, 3) # F523
@ -235,7 +206,7 @@ F523.py:24:1: F523 [*] `.format` call has unused arguments at position(s): 2
|
= help: Remove extra positional arguments at position(s): 2
Suggested fix
Fix
21 21 | # With nested quotes
22 22 | "''1{0}".format(1, 2, 3) # F523
23 23 | "\"\"{1}{0}".format(1, 2, 3) # F523
@ -245,7 +216,7 @@ F523.py:24:1: F523 [*] `.format` call has unused arguments at position(s): 2
26 26 | # With modified indexes
27 27 | "{1}{2}".format(1, 2, 3) # F523, # F524
F523.py:27:1: F523 [*] `.format` call has unused arguments at position(s): 0
F523.py:27:1: F523 `.format` call has unused arguments at position(s): 0
|
27 | # With modified indexes
28 | "{1}{2}".format(1, 2, 3) # F523, # F524
@ -255,17 +226,7 @@ F523.py:27:1: F523 [*] `.format` call has unused arguments at position(s): 0
|
= help: Remove extra positional arguments at position(s): 0
Suggested fix
24 24 | '""{1}{0}'.format(1, 2, 3) # F523
25 25 |
26 26 | # With modified indexes
27 |-"{1}{2}".format(1, 2, 3) # F523, # F524
27 |+"{0}{1}".format(2, 3) # F523, # F524
28 28 | "{1}{3}".format(1, 2, 3, 4) # F523, # F524
29 29 | "{1} {8}".format(0, 1) # F523, # F524
30 30 |
F523.py:28:1: F523 [*] `.format` call has unused arguments at position(s): 0, 2
F523.py:28:1: F523 `.format` call has unused arguments at position(s): 0, 2
|
28 | # With modified indexes
29 | "{1}{2}".format(1, 2, 3) # F523, # F524
@ -275,17 +236,7 @@ F523.py:28:1: F523 [*] `.format` call has unused arguments at position(s): 0, 2
|
= help: Remove extra positional arguments at position(s): 0, 2
Suggested fix
25 25 |
26 26 | # With modified indexes
27 27 | "{1}{2}".format(1, 2, 3) # F523, # F524
28 |-"{1}{3}".format(1, 2, 3, 4) # F523, # F524
28 |+"{0}{1}".format(2, 4) # F523, # F524
29 29 | "{1} {8}".format(0, 1) # F523, # F524
30 30 |
31 31 | # Not fixable
F523.py:29:1: F523 [*] `.format` call has unused arguments at position(s): 0
F523.py:29:1: F523 `.format` call has unused arguments at position(s): 0
|
29 | "{1}{2}".format(1, 2, 3) # F523, # F524
30 | "{1}{3}".format(1, 2, 3, 4) # F523, # F524
@ -296,16 +247,6 @@ F523.py:29:1: F523 [*] `.format` call has unused arguments at position(s): 0
|
= help: Remove extra positional arguments at position(s): 0
Suggested fix
26 26 | # With modified indexes
27 27 | "{1}{2}".format(1, 2, 3) # F523, # F524
28 28 | "{1}{3}".format(1, 2, 3, 4) # F523, # F524
29 |-"{1} {8}".format(0, 1) # F523, # F524
29 |+"{0} {8}".format(1) # F523, # F524
30 30 |
31 31 | # Not fixable
32 32 | (''
F523.py:32:2: F523 `.format` call has unused arguments at position(s): 0
|
32 | # Not fixable