Skip to content

Commit

Permalink
[pyupgrade] Allow fixes for f-string rule regardless of line length…
Browse files Browse the repository at this point in the history
… (`UP032`) (#10263)

## Summary

This is a follow-up to #10238 to
offer fixes for the f-string rule regardless of the line length of the
resulting fix. To quote Alex in the linked PR:

> Yes, from the user's perspective I'd rather have a fix that may lead
to line length issues than have to fix them myself :-) Cleaning up line
lengths is easier than changing from `"".format()` to `f""`

I agree with this position, which is that if we're going to offer a
diagnostic, we should really be offering the user the ability to fix it
-- otherwise, we're just inconveniencing them.
  • Loading branch information
charliermarsh authored Mar 7, 2024
1 parent 461cdad commit 91af5a4
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 44 deletions.
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
pyupgrade::rules::format_literals(checker, call, &summary);
}
if checker.enabled(Rule::FString) {
pyupgrade::rules::f_strings(checker, call, &summary, value);
pyupgrade::rules::f_strings(checker, call, &summary);
}
}
}
Expand Down
23 changes: 0 additions & 23 deletions crates/ruff_linter/src/fix/edits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,29 +418,6 @@ pub(crate) fn fits(
all_lines_fit(fix, node, locator, line_length.value() as usize, tab_size)
}

/// Returns `true` if the fix fits within the maximum configured line length, or produces lines that
/// are shorter than the maximum length of the existing AST node.
pub(crate) fn fits_or_shrinks(
fix: &str,
node: AnyNodeRef,
locator: &Locator,
line_length: LineLength,
tab_size: IndentWidth,
) -> bool {
// Use the larger of the line length limit, or the longest line in the existing AST node.
let line_length = std::iter::once(line_length.value() as usize)
.chain(
locator
.slice(locator.lines_range(node.range()))
.universal_newlines()
.map(|line| LineWidthBuilder::new(tab_size).add_str(&line).get()),
)
.max()
.unwrap_or(line_length.value() as usize);

all_lines_fit(fix, node, locator, line_length, tab_size)
}

/// Returns `true` if all lines in the fix are shorter than the given line length.
fn all_lines_fit(
fix: &str,
Expand Down
19 changes: 2 additions & 17 deletions crates/ruff_linter/src/rules/pyupgrade/rules/f_strings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use ruff_source_file::Locator;
use ruff_text_size::{Ranged, TextRange};

use crate::checkers::ast::Checker;
use crate::fix::edits::fits_or_shrinks;

use crate::rules::pyflakes::format::FormatSummary;
use crate::rules::pyupgrade::helpers::{curly_escape, curly_unescape};
Expand Down Expand Up @@ -392,12 +391,7 @@ impl FStringConversion {
}

/// UP032
pub(crate) fn f_strings(
checker: &mut Checker,
call: &ast::ExprCall,
summary: &FormatSummary,
template: &Expr,
) {
pub(crate) fn f_strings(checker: &mut Checker, call: &ast::ExprCall, summary: &FormatSummary) {
if summary.has_nested_parts {
return;
}
Expand Down Expand Up @@ -520,15 +514,6 @@ pub(crate) fn f_strings(

let mut diagnostic = Diagnostic::new(FString, call.range());

// Avoid refactors that exceed the line length limit or make it exceed by more.
let f_string_fits_or_shrinks = fits_or_shrinks(
&contents,
template.into(),
checker.locator(),
checker.settings.pycodestyle.max_line_length,
checker.settings.tab_size,
);

// Avoid fix if there are comments within the call:
// ```
// "{}".format(
Expand All @@ -540,7 +525,7 @@ pub(crate) fn f_strings(
.comment_ranges()
.intersects(call.arguments.range());

if f_string_fits_or_shrinks && !has_comments {
if !has_comments {
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
contents,
call.range(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -945,7 +945,7 @@ UP032_0.py:129:1: UP032 [*] Use f-string instead of `format` call
131 131 | ###
132 132 | # Non-errors

UP032_0.py:160:1: UP032 Use f-string instead of `format` call
UP032_0.py:160:1: UP032 [*] Use f-string instead of `format` call
|
158 | r'"\N{snowman} {}".format(a)'
159 |
Expand All @@ -958,7 +958,19 @@ UP032_0.py:160:1: UP032 Use f-string instead of `format` call
|
= help: Convert to f-string

UP032_0.py:164:1: UP032 Use f-string instead of `format` call
Safe fix
157 157 |
158 158 | r'"\N{snowman} {}".format(a)'
159 159 |
160 |-"123456789 {}".format(
161 |- 11111111111111111111111111111111111111111111111111111111111111111111111111,
162 |-)
160 |+f"123456789 {11111111111111111111111111111111111111111111111111111111111111111111111111}"
163 161 |
164 162 | """
165 163 | {}

UP032_0.py:164:1: UP032 [*] Use f-string instead of `format` call
|
162 | )
163 |
Expand All @@ -977,7 +989,28 @@ UP032_0.py:164:1: UP032 Use f-string instead of `format` call
|
= help: Convert to f-string

UP032_0.py:174:84: UP032 Use f-string instead of `format` call
Safe fix
161 161 | 11111111111111111111111111111111111111111111111111111111111111111111111111,
162 162 | )
163 163 |
164 |+f"""
165 |+{1}
166 |+{2}
167 |+{111111111111111111111111111111111111111111111111111111111111111111111111111111111111111}
164 168 | """
165 |-{}
166 |-{}
167 |-{}
168 |-""".format(
169 |-1,
170 |-2,
171 |-111111111111111111111111111111111111111111111111111111111111111111111111111111111111111,
172 |-)
173 169 |
174 170 | aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = """{}
175 171 | """.format(

UP032_0.py:174:84: UP032 [*] Use f-string instead of `format` call
|
172 | )
173 |
Expand All @@ -992,6 +1025,20 @@ UP032_0.py:174:84: UP032 Use f-string instead of `format` call
|
= help: Convert to f-string

Safe fix
171 171 | 111111111111111111111111111111111111111111111111111111111111111111111111111111111111111,
172 172 | )
173 173 |
174 |-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = """{}
175 |-""".format(
176 |- 111111
177 |-)
174 |+aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = f"""{111111}
175 |+"""
178 176 |
179 177 | "{}".format(
180 178 | [

UP032_0.py:202:1: UP032 Use f-string instead of `format` call
|
200 | "{}".format(**c)
Expand Down

0 comments on commit 91af5a4

Please sign in to comment.