Support autofix for some multiline str.format calls (#5638)

## Summary Fixes #5531 ## Test Plan New test cases
astral-sh · Jul 10, 2023 · 82317ba · 82317ba
1 parent 24bcbb8
commit 82317ba
Show file tree

Hide file tree

Showing 4 changed files with 117 additions and 43 deletions.
diff --git a/crates/ruff/resources/test/fixtures/pyupgrade/UP032_0.py b/crates/ruff/resources/test/fixtures/pyupgrade/UP032_0.py
@@ -54,6 +54,14 @@
 
 '''{[b]}'''.format(a)
 
+"{}".format(
+    1
+)
+
+"123456789 {}".format(
+    1111111111111111111111111111111111111111111111111111111111111111111111111,
+)
+
 ###
 # Non-errors
 ###
@@ -87,6 +95,9 @@
 
 "{a}" "{b}".format(a=1, b=1)
 
+"123456789 {}".format(
+    11111111111111111111111111111111111111111111111111111111111111111111111111,
+)
 
 async def c():
     return "{}".format(await 3)

diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs
@@ -2436,19 +2436,19 @@ where
                     if let Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = func.as_ref() {
                         let attr = attr.as_str();
                         if let Expr::Constant(ast::ExprConstant {
-                            value: Constant::Str(value),
+                            value: Constant::Str(val),
                             ..
                         }) = value.as_ref()
                         {
                             if attr == "join" {
                                 // "...".join(...) call
                                 if self.enabled(Rule::StaticJoinToFString) {
-                                    flynt::rules::static_join_to_fstring(self, expr, value);
+                                    flynt::rules::static_join_to_fstring(self, expr, val);
                                 }
                             } else if attr == "format" {
                                 // "...".format(...) call
                                 let location = expr.range();
-                                match pyflakes::format::FormatSummary::try_from(value.as_ref()) {
+                                match pyflakes::format::FormatSummary::try_from(val.as_ref()) {
                                     Err(e) => {
                                         if self.enabled(Rule::StringDotFormatInvalidFormat) {
                                             self.diagnostics.push(Diagnostic::new(
@@ -2492,7 +2492,13 @@ where
                                         }
 
                                         if self.enabled(Rule::FString) {
-                                            pyupgrade::rules::f_strings(self, &summary, expr);
+                                            pyupgrade::rules::f_strings(
+                                                self,
+                                                &summary,
+                                                expr,
+                                                value,
+                                                self.settings.line_length,
+                                            );
                                         }
                                     }
                                 }

diff --git a/crates/ruff/src/rules/pyupgrade/rules/f_strings.rs b/crates/ruff/src/rules/pyupgrade/rules/f_strings.rs
@@ -13,6 +13,7 @@ use ruff_python_ast::source_code::Locator;
 use ruff_python_ast::str::{is_implicit_concatenation, leading_quote, trailing_quote};
 
 use crate::checkers::ast::Checker;
+use crate::line_width::LineLength;
 use crate::registry::AsRule;
 use crate::rules::pyflakes::format::FormatSummary;
 use crate::rules::pyupgrade::helpers::curly_escape;
@@ -313,13 +314,19 @@ fn try_convert_to_f_string(expr: &Expr, locator: &Locator) -> Option<String> {
 }
 
 /// UP032
-pub(crate) fn f_strings(checker: &mut Checker, summary: &FormatSummary, expr: &Expr) {
+pub(crate) fn f_strings(
+    checker: &mut Checker,
+    summary: &FormatSummary,
+    expr: &Expr,
+    template: &Expr,
+    line_length: LineLength,
+) {
     if summary.has_nested_parts {
         return;
     }
 
     // Avoid refactoring multi-line strings.
-    if checker.locator.contains_line_break(expr.range()) {
+    if checker.locator.contains_line_break(template.range()) {
         return;
     }
 
@@ -329,9 +336,9 @@ pub(crate) fn f_strings(checker: &mut Checker, summary: &FormatSummary, expr: &E
         return;
     };
 
-    // Avoid refactors that increase the resulting string length.
-    let existing = checker.locator.slice(expr.range());
-    if contents.len() > existing.len() {
+    // Avoid refactors that exceed the line length limit.
+    let col_offset = template.start() - checker.locator.line_start(template.start());
+    if col_offset.to_usize() + contents.len() > line_length.get() {
         return;
     }
 

diff --git a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP032_0.py.snap b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP032_0.py.snap
@@ -533,7 +533,7 @@ UP032_0.py:55:1: UP032 [*] Use f-string instead of `format` call
 55 | '''{[b]}'''.format(a)
    | ^^^^^^^^^^^^^^^^^^^^^ UP032
 56 | 
-57 | ###
+57 | "{}".format(
    |
    = help: Convert to f-string
 
@@ -544,57 +544,107 @@ UP032_0.py:55:1: UP032 [*] Use f-string instead of `format` call
 55    |-'''{[b]}'''.format(a)
    55 |+f'''{a["b"]}'''
 56 56 | 
-57 57 | ###
-58 58 | # Non-errors
+57 57 | "{}".format(
+58 58 |     1
 
-UP032_0.py:100:11: UP032 [*] Use f-string instead of `format` call
+UP032_0.py:57:1: UP032 [*] Use f-string instead of `format` call
+   |
+55 |   '''{[b]}'''.format(a)
+56 |   
+57 | / "{}".format(
+58 | |     1
+59 | | )
+   | |_^ UP032
+60 |   
+61 |   "123456789 {}".format(
+   |
+   = help: Convert to f-string
+
+ℹ Suggested fix
+54 54 | 
+55 55 | '''{[b]}'''.format(a)
+56 56 | 
+57    |-"{}".format(
+58    |-    1
+59    |-)
+   57 |+f"{1}"
+60 58 | 
+61 59 | "123456789 {}".format(
+62 60 |     1111111111111111111111111111111111111111111111111111111111111111111111111,
+
+UP032_0.py:61:1: UP032 [*] Use f-string instead of `format` call
+   |
+59 |   )
+60 |   
+61 | / "123456789 {}".format(
+62 | |     1111111111111111111111111111111111111111111111111111111111111111111111111,
+63 | | )
+   | |_^ UP032
+64 |   
+65 |   ###
+   |
+   = help: Convert to f-string
+
+ℹ Suggested fix
+58 58 |     1
+59 59 | )
+60 60 | 
+61    |-"123456789 {}".format(
+62    |-    1111111111111111111111111111111111111111111111111111111111111111111111111,
+63    |-)
+   61 |+f"123456789 {1111111111111111111111111111111111111111111111111111111111111111111111111}"
+64 62 | 
+65 63 | ###
+66 64 | # Non-errors
+
+UP032_0.py:111:11: UP032 [*] Use f-string instead of `format` call
     |
- 99 | def d(osname, version, release):
-100 |     return"{}-{}.{}".format(osname, version, release)
+110 | def d(osname, version, release):
+111 |     return"{}-{}.{}".format(osname, version, release)
     |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP032
     |
     = help: Convert to f-string
 
 ℹ Suggested fix
-97  97  | 
-98  98  | 
-99  99  | def d(osname, version, release):
-100     |-    return"{}-{}.{}".format(osname, version, release)
-    100 |+    return f"{osname}-{version}.{release}"
-101 101 | 
-102 102 | 
-103 103 | def e():
-
-UP032_0.py:104:10: UP032 [*] Use f-string instead of `format` call
+108 108 | 
+109 109 | 
+110 110 | def d(osname, version, release):
+111     |-    return"{}-{}.{}".format(osname, version, release)
+    111 |+    return f"{osname}-{version}.{release}"
+112 112 | 
+113 113 | 
+114 114 | def e():
+
+UP032_0.py:115:10: UP032 [*] Use f-string instead of `format` call
     |
-103 | def e():
-104 |     yield"{}".format(1)
+114 | def e():
+115 |     yield"{}".format(1)
     |          ^^^^^^^^^^^^^^ UP032
     |
     = help: Convert to f-string
 
 ℹ Suggested fix
-101 101 | 
-102 102 | 
-103 103 | def e():
-104     |-    yield"{}".format(1)
-    104 |+    yield f"{1}"
-105 105 | 
-106 106 | 
-107 107 | assert"{}".format(1)
-
-UP032_0.py:107:7: UP032 [*] Use f-string instead of `format` call
+112 112 | 
+113 113 | 
+114 114 | def e():
+115     |-    yield"{}".format(1)
+    115 |+    yield f"{1}"
+116 116 | 
+117 117 | 
+118 118 | assert"{}".format(1)
+
+UP032_0.py:118:7: UP032 [*] Use f-string instead of `format` call
     |
-107 | assert"{}".format(1)
+118 | assert"{}".format(1)
     |       ^^^^^^^^^^^^^^ UP032
     |
     = help: Convert to f-string
 
 ℹ Suggested fix
-104 104 |     yield"{}".format(1)
-105 105 | 
-106 106 | 
-107     |-assert"{}".format(1)
-    107 |+assert f"{1}"
+115 115 |     yield"{}".format(1)
+116 116 | 
+117 117 | 
+118     |-assert"{}".format(1)
+    118 |+assert f"{1}"