[pyupgrade] Automatically rewrite format-strings to f-strings #1905

colin99d · 2023-01-16T03:11:34Z

Draft opened for visibility. This is a part of #827. This PR is currently affected by the \N{emoji} bug that we are searching for a fix for.

colin99d · 2023-01-16T20:33:32Z

@charliermarsh this pr is ready except for the questions below, so I will move it out of draft. Before this is merged we need to consider or resolve the following items:

The /N{emoji} issue from UP031 remains unsolved (whenever we solve it I will integrate the solution here.
F strings introduce double bracket syntax {{}}, which is throwing my regexs for a loop. Currently, the best solution I have is \{(?P<name>[^\W0-9]\w*)?(?P<fmt>[^{]*?)} (this does NOT work fully). I tried using libcst_native but it just gives a normal string, and not a formatted one. I believe we could use fancy_regex here; however, that adds a whole dependency for one small functionality. As always, we could just only check for this and not fix it.
Similar to my PR for UP031, there is a negative test case I disagree with.

"{}" . format(x)

This is not supposed to be formatted because of its "weird syntax", but our AST parser ignores the spaces. This means ignoring this is more work for us, and results in a worse user experience.
4. Lastly (and this one is kinda embarrassing). I cannot for the life of me get the proper .snap file to generate for tests. I have never had an issue before, but it keeps suggesting that the snap just be an empty list, even though running ruff on those files produces a lot of changes. I am sure its something small I am missing.

src/checkers/ast.rs

charliermarsh · 2023-01-17T02:15:19Z

src/rules/pyupgrade/rules/f_strings.rs

+    }
+
+    /// Returns true if the statement and function call match, and false if not
+    fn check_with_summary(&self, summary: &FormatSummary) -> bool {


@colin99d - Is this strictly necessary? (Do you know of any case where it would trigger?)

charliermarsh · 2023-01-17T02:52:04Z

@colin99d - I'm fine to ignore the unicode case. What else needs solving here? The {{}} case?

colin99d · 2023-01-17T02:57:14Z

That should be the only thing!

charliermarsh · 2023-01-17T02:59:53Z

I have one idea I'd like to try, which may let us avoid regexes.

charliermarsh · 2023-01-17T03:00:07Z

(As-is, what happens in those cases? We just ignore them?)

colin99d · 2023-01-17T03:19:54Z

(As-is, what happens in those cases? We just ignore them?)

Right now it is ignored in a way. For example: below rust thinks it needs three arguments but only finds 2, so it exits. If we are going to ignore these, we should probably implement some better logic for ignoring them.
{}{{}}{}".format(escaped, y)

charliermarsh · 2023-01-17T03:22:11Z

I think we can leverage RustPython's actual string parser and forego regular expressions. Trying it now...

charliermarsh · 2023-01-17T03:43:14Z

@colin99d - I was able to use RustPython's own parser, and I think it's properly handling all test-cases now.

This might be a useful approach to use in the other PR too? I haven't looked into it deeply, but generally, it's best to use the RustPython parsers where we can.

…regardless of line-length (`UP032`) (#10238) ## Summary Fixes #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: * #7818 Changed the heuristic to determine if the fix fits to address #7810 * #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

…regardless of line-length (`UP032`) (astral-sh#10238) ## Summary Fixes astral-sh#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: * astral-sh#7818 Changed the heuristic to determine if the fix fits to address astral-sh#7810 * astral-sh#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

colin99d added 8 commits January 15, 2023 16:45

Layed a foundation

83319f1

Made progress

b10ad54

Added unit tests

c7c5056

Got something working

6b3430b

Got all positive tests in

0da7543

Got all positive tests in

47454c7

Added error handling

d516a63

Improved error handling

f56f682

colin99d changed the title ~~Fstrings~~ Pyupgrade: Fstrings Jan 16, 2023

colin99d added 7 commits January 15, 2023 22:17

Actually make a string

220bf74

Last little change for tonight

0e95588

Made a lot of progress

c518410

Added handling for a lot of negative cases

62e8a41

Typos and fmt

3f16a2a

Fixed some small things

e48cf15

Fixed some small things

d30ebc9

colin99d marked this pull request as ready for review January 16, 2023 20:33

colin99d and others added 7 commits January 16, 2023 15:35

two more rules

b4624ef

two more rules

aadf364

Fixed clippy rule

ed871cc

Merge branch 'main' into fstrings

be4bb47

Fixed more clippy issues:

267f506

Merge branch 'main' into fstrings

c1a07f0

Fix fixtures

ffbbd43

charliermarsh reviewed Jan 17, 2023

View reviewed changes

src/checkers/ast.rs Show resolved Hide resolved

charliermarsh reviewed Jan 17, 2023

View reviewed changes

charliermarsh added 2 commits January 16, 2023 21:21

Minor refactors

be5029b

Minor refactors

0e7fcd8

charliermarsh added 2 commits January 16, 2023 22:33

Use RustPython parser

bf83d2e

Use RustPython parser

d87da2e

charliermarsh changed the title ~~Pyupgrade: Fstrings~~ Pyupgrade: automatically rewrite format-strings to f-strings Jan 17, 2023

charliermarsh changed the title ~~Pyupgrade: automatically rewrite format-strings to f-strings~~ [pyupgrade] Automatically rewrite format-strings to f-strings Jan 17, 2023

charliermarsh added 2 commits January 16, 2023 22:51

Avoid implicit string concats

2692121

Change format

87c31e6

charliermarsh merged commit 1730f2a into astral-sh:main Jan 17, 2023

nstarman mentioned this pull request Jan 18, 2023

Apply the flynt tool astropy/astropy#14278

Merged

10 tasks

akx mentioned this pull request Jan 23, 2023

Implement flynt's "join and concat to f-string" transforms #2102

Open

2 tasks

MichaReiser mentioned this pull request Mar 5, 2024

[pyupgrade] Generate diagnostic for all valid f-string conversions regardless of line-length (UP032) #10238

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[pyupgrade] Automatically rewrite format-strings to f-strings #1905

[pyupgrade] Automatically rewrite format-strings to f-strings #1905

colin99d commented Jan 16, 2023

colin99d commented Jan 16, 2023 •

edited

Loading

charliermarsh Jan 17, 2023

charliermarsh commented Jan 17, 2023

colin99d commented Jan 17, 2023

charliermarsh commented Jan 17, 2023 •

edited

Loading

charliermarsh commented Jan 17, 2023

colin99d commented Jan 17, 2023 •

edited

Loading

charliermarsh commented Jan 17, 2023

charliermarsh commented Jan 17, 2023

[pyupgrade] Automatically rewrite format-strings to f-strings #1905

[pyupgrade] Automatically rewrite format-strings to f-strings #1905

Conversation

colin99d commented Jan 16, 2023

colin99d commented Jan 16, 2023 • edited Loading

charliermarsh Jan 17, 2023

Choose a reason for hiding this comment

charliermarsh commented Jan 17, 2023

colin99d commented Jan 17, 2023

charliermarsh commented Jan 17, 2023 • edited Loading

charliermarsh commented Jan 17, 2023

colin99d commented Jan 17, 2023 • edited Loading

charliermarsh commented Jan 17, 2023

charliermarsh commented Jan 17, 2023

colin99d commented Jan 16, 2023 •

edited

Loading

charliermarsh commented Jan 17, 2023 •

edited

Loading

colin99d commented Jan 17, 2023 •

edited

Loading