From 6159a8e532eadc5535cea0ae0343d6dd46f4b7fc Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Wed, 6 Mar 2024 09:58:20 +0100 Subject: [PATCH] [`pyupgrade`] Generate diagnostic for all valid f-string conversions regardless of line-length (`UP032`) (#10238) ## Summary Fixes https://github.com/astral-sh/ruff/issues/10235 This PR changes `UP032` to flag all `"".format` calls that can technically be rewritten to an f-string, even if rewritting it to an fstring, at least automatically, exceeds the line length (or increases the amount by which it goes over the line length). I looked at the Git history to understand whether the check prevents some false positives (reported by an issue), but i haven't found a compelling reason to limit the rule to only flag format calls that stay in the line length limit: * https://github.com/astral-sh/ruff/pull/7818 Changed the heuristic to determine if the fix fits to address https://github.com/astral-sh/ruff/discussions/7810 * https://github.com/astral-sh/ruff/pull/1905 first version of the rule I did take a look at pyupgrade and couldn't find a similar check, at least not in the rule code (maybe it's checked somewhere else?) https://github.com/asottile/pyupgrade/blob/main/pyupgrade/_plugins/fstrings.py ## Breaking Change? This could be seen as a breaking change according to ruff's [versioning policy](https://docs.astral.sh/ruff/versioning/): > The behavior of a stable rule is changed * The scope of a stable rule is significantly increased * The intent of the rule changes * Does not include bug fixes that follow the original intent of the rule It does increase the scope of the rule, but it is in the original intent of the rule (so it's not). ## Test Plan See changed test output --- .../src/rules/pyupgrade/rules/f_strings.rs | 27 +++++----- ...__rules__pyupgrade__tests__UP032_0.py.snap | 49 ++++++++++++++++++- ...__rules__pyupgrade__tests__UP032_1.py.snap | 2 - ...__rules__pyupgrade__tests__UP032_2.py.snap | 2 - 4 files changed, 60 insertions(+), 20 deletions(-) diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/f_strings.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/f_strings.rs index 7f3b9d7adf6c1c..30c1fdb829843c 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/f_strings.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/f_strings.rs @@ -447,17 +447,6 @@ pub(crate) fn f_strings( contents.insert(0, ' '); } - // Avoid refactors that exceed the line length limit. - if !fits_or_shrinks( - &contents, - template.into(), - checker.locator(), - checker.settings.pycodestyle.max_line_length, - checker.settings.tab_size, - ) { - return; - } - // Finally, avoid refactors that would introduce a runtime error. // For example, Django's `gettext` supports `format`-style arguments, but not f-strings. // See: https://docs.djangoproject.com/en/4.2/topics/i18n/translation @@ -479,17 +468,27 @@ 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( // 0, # 0 // ) // ``` - if !checker + let has_comments = checker .indexer() .comment_ranges() - .intersects(call.arguments.range()) - { + .intersects(call.arguments.range()); + + if f_string_fits_or_shrinks && !has_comments { diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( contents, call.range(), diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP032_0.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP032_0.py.snap index 1693cdb663508b..305e2ee42f0dc1 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP032_0.py.snap +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP032_0.py.snap @@ -945,6 +945,53 @@ 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 + | +158 | r'"\N{snowman} {}".format(a)' +159 | +160 | / "123456789 {}".format( +161 | | 11111111111111111111111111111111111111111111111111111111111111111111111111, +162 | | ) + | |_^ UP032 +163 | +164 | """ + | + = help: Convert to f-string + +UP032_0.py:164:1: UP032 Use f-string instead of `format` call + | +162 | ) +163 | +164 | / """ +165 | | {} +166 | | {} +167 | | {} +168 | | """.format( +169 | | 1, +170 | | 2, +171 | | 111111111111111111111111111111111111111111111111111111111111111111111111111111111111111, +172 | | ) + | |_^ UP032 +173 | +174 | aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = """{} + | + = help: Convert to f-string + +UP032_0.py:174:84: UP032 Use f-string instead of `format` call + | +172 | ) +173 | +174 | aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = """{} + | ____________________________________________________________________________________^ +175 | | """.format( +176 | | 111111 +177 | | ) + | |_^ UP032 +178 | +179 | "{}".format( + | + = help: Convert to f-string + UP032_0.py:202:1: UP032 Use f-string instead of `format` call | 200 | "{}".format(**c) @@ -1218,5 +1265,3 @@ UP032_0.py:254:1: UP032 [*] Use f-string instead of `format` call 253 253 | # The dictionary should be parenthesized. 254 |-"{}".format({0: 1}()) 254 |+f"{({0: 1}())}" - - diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP032_1.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP032_1.py.snap index 59cb45d8e46e8e..221526314e6fb3 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP032_1.py.snap +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP032_1.py.snap @@ -11,5 +11,3 @@ UP032_1.py:1:1: UP032 [*] Use f-string instead of `format` call ℹ Safe fix 1 |-"{} {}".format(a, b) # Intentionally at start-of-file, to ensure graceful handling. 1 |+f"{a} {b}" # Intentionally at start-of-file, to ensure graceful handling. - - diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP032_2.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP032_2.py.snap index 1a0736c7daf5d6..7ad4096da61828 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP032_2.py.snap +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP032_2.py.snap @@ -439,5 +439,3 @@ UP032_2.py:28:1: UP032 [*] Use f-string instead of `format` call 27 27 | "{.real}".format({1: 2, 3: 4}) 28 |-"{}".format((i for i in range(2))) 28 |+f"{(i for i in range(2))}" - -