Skip to content

Commit

Permalink
Preserve quotes in F523 fixer (#4836)
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jun 3, 2023
1 parent 42c071d commit fcacd3c
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 6 deletions.
5 changes: 5 additions & 0 deletions crates/ruff/resources/test/fixtures/pyflakes/F523.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,8 @@
"{0}{1}".format(1, *args) # No issues
"{0}{1}".format(1, 2, *args) # No issues
"{0}{1}".format(1, 2, 3, *args) # F523

# With nested quotes
"{''1:{0}}".format(1, 2, 3) # F523
"{\"\"1:{0}}".format(1, 2, 3) # F523
'{""1:{0}}'.format(1, 2, 3) # F523
37 changes: 31 additions & 6 deletions crates/ruff/src/rules/pyflakes/fixes.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use anyhow::{bail, Ok, Result};
use anyhow::{anyhow, bail, Ok, Result};
use itertools::Itertools;
use libcst_native::{Codegen, CodegenState, DictElement, Expression};
use ruff_text_size::TextRange;
use rustpython_format::{
Expand All @@ -9,7 +10,7 @@ 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 ruff_python_ast::str::{leading_quote, raw_contents, trailing_quote};

use crate::cst::matchers::{
match_attribute, match_call_mut, match_dict, match_expression, match_simple_string,
Expand Down Expand Up @@ -144,20 +145,44 @@ pub(crate) fn remove_unused_positional_arguments_from_format_call(
});

let mut min_unused_index = 0;
for index in unused_arguments {
for index in unused_arguments.iter().sorted() {
if *index == min_unused_index {
min_unused_index += 1;
} else {
break;
}
}

let mut new_format_string;
// 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 new_format_string;
if min_unused_index > 0 {
// Extract the format string verbatim.
let func = match_attribute(&mut call.func)?;
let simple_string = match_simple_string(&mut func.value)?;
new_format_string = update_field_types(format_string, min_unused_index);
new_format_string = format!(r#""{new_format_string}""#);

// 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, min_unused_index),
trailing_quote
);

simple_string.value = new_format_string.as_str();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@ F523.py:19:1: F523 [*] `.format` call has unused arguments at position(s): 2
20 | "{0}{1}".format(1, 2, *args) # No issues
21 | "{0}{1}".format(1, 2, 3, *args) # F523
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ F523
22 |
23 | # With nested quotes
|
= help: Remove extra positional arguments at position(s): 2

Expand All @@ -178,5 +180,61 @@ F523.py:19:1: F523 [*] `.format` call has unused arguments at position(s): 2
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
23 | "{''1:{0}}".format(1, 2, 3) # F523
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ F523
24 | "{\"\"1:{0}}".format(1, 2, 3) # F523
25 | '{""1:{0}}'.format(1, 2, 3) # F523
|
= help: Remove extra positional arguments at position(s): 1, 2

Suggested fix
19 19 | "{0}{1}".format(1, 2, 3, *args) # F523
20 20 |
21 21 | # With nested quotes
22 |-"{''1:{0}}".format(1, 2, 3) # F523
22 |+"{''1:{0}}".format(1, ) # F523
23 23 | "{\"\"1:{0}}".format(1, 2, 3) # F523
24 24 | '{""1:{0}}'.format(1, 2, 3) # F523

F523.py:23:1: F523 [*] `.format` call has unused arguments at position(s): 1, 2
|
23 | # With nested quotes
24 | "{''1:{0}}".format(1, 2, 3) # F523
25 | "{\"\"1:{0}}".format(1, 2, 3) # F523
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ F523
26 | '{""1:{0}}'.format(1, 2, 3) # F523
|
= help: Remove extra positional arguments at position(s): 1, 2

Suggested fix
20 20 |
21 21 | # With nested quotes
22 22 | "{''1:{0}}".format(1, 2, 3) # F523
23 |-"{\"\"1:{0}}".format(1, 2, 3) # F523
23 |+"{\"\"1:{0}}".format(1, ) # F523
24 24 | '{""1:{0}}'.format(1, 2, 3) # F523

F523.py:24:1: F523 [*] `.format` call has unused arguments at position(s): 1, 2
|
24 | "{''1:{0}}".format(1, 2, 3) # F523
25 | "{\"\"1:{0}}".format(1, 2, 3) # F523
26 | '{""1:{0}}'.format(1, 2, 3) # F523
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ F523
|
= help: Remove extra positional arguments at position(s): 1, 2

Suggested fix
21 21 | # With nested quotes
22 22 | "{''1:{0}}".format(1, 2, 3) # F523
23 23 | "{\"\"1:{0}}".format(1, 2, 3) # F523
24 |-'{""1:{0}}'.format(1, 2, 3) # F523
24 |+'{""1:{0}}'.format(1, ) # F523


0 comments on commit fcacd3c

Please sign in to comment.