Add PEP701 support

[tusharsadhwani]

Description

Adds support for PEP 701: Syntactic formalization of f-strings to black's tokenizer.

Resolves #3746

PR is a work in progress

Given this Python file:

x = f"foo{2 + 2}bar"

Previous output:

$ python src/blib2to3/pgen2/tokenize.py asd.py
1,0-1,1:        NAME    'x'
1,2-1,3:        OP      '='
1,4-1,20:       STRING  'f"foo{2 + 2}bar"'
1,20-1,21:      NEWLINE '\n'
2,0-2,0:        ENDMARKER       ''

Current output

$ python src/blib2to3/pgen2/tokenize.py asd.py
1,0-1,1:        NAME    'x'
1,2-1,3:        OP      '='
1,4-1,2:        FSTRING_START   'f"'
1,2-1,5:        FSTRING_MIDDLE  'foo'
1,5-1,6:        LBRACE  '{'
1,6-1,7:        NUMBER  '2'
1,8-1,9:        OP      '+'
1,10-1,11:      NUMBER  '2'
1,12-1,13:      RBRACE  '}'
1,12-1,15:      FSTRING_MIDDLE  'bar'
1,15-1,20:      FSTRING_END     '"'
1,20-1,21:      NEWLINE '\n'
2,0-2,0:        ENDMARKER       ''

Checklist - did you ...

• [ ] Add an entry in CHANGES.md if necessary?
• [ ] Add / update tests if necessary?
• [ ] Add new / update outdated documentation?

[tusharsadhwani]

@ambv This is the approach that I'm thinking of: defining a tokenize_string generator, which in turn will call generate_tokens() inside it to be able to tokenize Python source inside it.

Does the approach seem ok? If yes, I'll go ahead and implement all the missing features.

[tusharsadhwani]

cc @JelleZijlstra as well, for the above comment.

[tusharsadhwani]

Also, although I don't think this change will be backwards incompatible, should it be possible to keep the old behaviour intact? Should this be behind the experimental flag?

[JelleZijlstra]

What aspect won't be backward compatible? Shouldn't we just start parsing code we previously failed on?

[tusharsadhwani]

@JelleZijlstra I thought so too. But I'm not 100% confident.

Do we want to preserve parsing failures for 3.11 and before? Or can we just drop the old tokenizer and parser?

[ambv]

I think the proposed approach is wrong. You can't use the existing understanding of strings and separately decompose them. You don't know the endquote up front anymore because this is now valid:

f = f"abc{"def"}"

You need to use a stack inside the existing generate_tokens().

[JelleZijlstra]

@tusharsadhwani It's fine if we parse this code in earlier Python versions too. Catching syntax errors is not a goal for Black.

[tusharsadhwani]

@ambv wouldn't the stack frames made by calling generate_tokens() recursively do the same job of maintaining a stack?

You can think of every call to tokenize_string() as pushing an fstring-mode to the stack, and the generate_tokens() call inside it to pushing a normal-mode to the stack.

[ambv]

No, the call stack isn't automatically compatible with the stack we need here.

You need to change how endprog in generate_tokens works for strings. This is where the new stack needs to be.

When you encounter f" then you switch to string literal tokenizing, looking for either " or { as endprog. When you encounter { you add } to the endprog stack and switch back to regular tokenizing... but since the stack is not empty, you watch out for a } to return back to string tokenizing.

That way you can encounter another ", switch to string tokenizing, and add a " as endprog to switch back to regular parsing. And when you reach that ", then you remove it from the stack, switch to regular parsing, but there's still } on the stack so you keep looking for that.

At no point are you iterating a counter like curly_brace_level. The endprog stack depth is the level.

Is that clearer now?

[tusharsadhwani]

Yes, it's clearer now.

I'll implement the rest.

[github-actions[bot]]

diff-shades reports zero changes comparing this PR (ab2f43c51f9385188ae952e5354c1d8955a9b8a0) to main (944b99aa91f0a5afadf48b3a90cacdb5c8e9f858).

---

What is this? | Workflow run | diff-shades documentation

[tusharsadhwani]

@ambv For code like this (which was invalid in Python3.11 but is valid in Python3.12):

f' \' {f"'"} \' '

Black should try to normalize it to something like this:

f" ' {f"'"} ' "

Right?

This I believe would require a well thought-out algorithm to figure out exactly which f-strings we normalize and which we don't. Because, depending on whether we normalize the inner one or not, the number of \' present in the complete output could change.

I'm not sure what the right approach is here, should I leave that work for later, and only worry about, say, the top level f-string normalization for now?

[tusharsadhwani]

cc @JelleZijlstra

[ambv]

Good point that we should do backslash normalization here as well for 3.12+ source code input. Sounds like a non-trivial design surface so it wouldn't be an issue if we don't ship this right away.

[tusharsadhwani]

This bug is fixed in the latest Python 3.12, and the comment can be ignored.

---

@ambv I'm trying to write logic to split/not split the line on a LBRACE inside an f-string. The tests with long lines containing f-strings fail right now, everything else passes,

But there is one edge case:

f"{
  x
}"

is valid syntax, but

f"{
  x:d
}"

~is not valid syntax in 3.12. Is this intentionally so? And if yes, where do I write the logic to not do RHS split if there's a format specifier?~

EDIT: This was actually a bug in CPython.

[tusharsadhwani]

Secondly, what's your opinion on RHS splitting inside f-strings?

-msg = (
-    lambda x: (
-        f"this is a very very very long lambda value {x} that doesn't fit on a single"
-        " line"
-    )
-)
+msg = lambda x: f"this is a very very very long lambda value {
+    x
+} that doesn't fit on a single line"

Is this the expected behaviour now?

[fredgido]

in this branch also didn't like it did this

info_text = (
    f"{a_count} a,"
    f"{b_count} b,"
    f"{c_count} c,"
    f"{d_count} d,"
    f"{e_count} e,"
    f"{f_count} f,"
    f"{g_count} g,"
    f"{h_count} h,"
)

becames

info_text = f"{a_count} a, " f"{b_count} b, " f"{c_count} c," f"{d_count} d," f"{e_count} e," f"{f_count} f," f"{
    g_count
} g," f"{h_count} h,"

Thanks for your work

[tusharsadhwani]

@fredgido noted, i'll handle that case.

[tusharsadhwani]

@ambv Small stuff and de-duplicating tokenizer code is left, can you do an initial review?

I've added a test file called pep_701.py.

There are 7 failing tests in test_format.py, and all of them seem like they are due to line wrapping inside long f-strings.

I'm unable to wrap my head around the right hand split code, and I need help with figuring out the logic for these two things:

• Not formatting f-string braces if Feature.FSTRING_PARSING is not available.
• Not formatting f-string braces if the brace ends in a =, eg. f'{x = }' shouldn't be formatted otherwise runtime behaviour changes.

cc @JelleZijlstra

[ambv]

AFK right now but I'll do a review tomorrow. Thanks for working on this!

[tusharsadhwani]

Here's one way that I thought to prevent fstring formatting in python 3.11 and below:

diff --git a/src/black/__init__.py b/src/black/__init__.py
index 7cb1e3b..38a66f9 100644
--- a/src/black/__init__.py
+++ b/src/black/__init__.py
@@ -1094,13 +1094,16 @@ def _format_str_once(src_contents: str, *, mode: Mode) -> str:
         future_imports = get_future_imports(src_node)
         versions = detect_target_versions(src_node, future_imports=future_imports)
 
-    context_manager_features = {
+    line_generator_features = {
         feature
-        for feature in {Feature.PARENTHESIZED_CONTEXT_MANAGERS}
+        for feature in {
+            Feature.PARENTHESIZED_CONTEXT_MANAGERS,
+            Feature.FSTRING_PARSING,
+        }
         if supports_feature(versions, feature)
     }
     normalize_fmt_off(src_node)
-    lines = LineGenerator(mode=mode, features=context_manager_features)
+    lines = LineGenerator(mode=mode, features=line_generator_features)
     elt = EmptyLineTracker(mode=mode)
     split_line_features = {
         feature
diff --git a/src/black/linegen.py b/src/black/linegen.py
index 5f346a2..b666245 100644
--- a/src/black/linegen.py
+++ b/src/black/linegen.py
@@ -501,6 +501,13 @@ class LineGenerator(Visitor[Line]):
         yield from self.visit_default(leaf)
 
     def visit_fstring(self, node: Node) -> Iterator[Line]:
+        if Feature.FSTRING_PARSING not in self.features:
+            string = str(node)
+            string_leaf = Leaf(token.STRING, string)
+            node.replace(string_leaf)
+            yield from self.visit_default(string_leaf)
+            return
+
         fstring_start = node.children[0]
         fstring_end = node.children[-1]
         assert isinstance(fstring_start, Leaf)

But this caused some tests to fail, mostly around empty lines not getting added/removed?

[tusharsadhwani]

@ambv small nudge

[tusharsadhwani]

@JelleZijlstra fair point about not splitting inside f strings! but I wasn't able to wrap my head around the right hand split logic enough to try out stuff with it.

Are there any conditions where you'd want to split fstrings on multiple lines? what if the line is very long?

[JelleZijlstra]

Possibly if the code inside the placeholder is very complex? It should certainly be uncommon.

It's fine to leave this PR focused purely on parsing PEP 701 syntax and leave decisions on how exactly to format them to another PR, though I recognize it may not be easy to separate those things out.

[tusharsadhwani]

@ambv I'll be at PyCon TH next month, maybe we could work on this then?

[tusharsadhwani]

@ambv bumping this, we had to take the conversation forward.

[JelleZijlstra]

I pushed a couple of small lint fixes. However, there is at least one crash that appears when formatting Black itself:

% black --unstable -c '''raise ValueError(
                "xxxxxxxxxxxIncorrect --line-ranges format, expect 'START-END', found"
                f" {lines_str!r}"
            )'''
raise ValueError(
                "xxxxxxxxxxxIncorrect --line-ranges format, expect START-END, found"
                f" {lines_str!r}"
            )
error: cannot format <string>: INTERNAL ERROR: Black produced invalid code: unterminated string literal (detected at line 2) (<unknown>, line 2). Please report a bug on https://github.com/psf/black/issues.  This invalid output might be helpful: /var/folders/hh/hdb5qd2x3v51v6l0gknpfjfr0000gp/T/blk_2on9v_hn.log

[dhruvmanila]

Hi 👋, we (Ruff) have moved ahead with f-string formatting with minimal style changes: https://github.com/astral-sh/ruff/pull/9642

Relevant discussion: https://github.com/astral-sh/ruff/discussions/9785

The style changes are kept to the minimal. Regarding breaking expressions across multiple lines, we've decided to use the heuristic to break only if at least one of the expression is multi-line i.e., the control of having multi-line expression or not is in the hands of the programmer. You can find more details in the PR description. I hope the maintainers can chime in on the style discussion if possible :)

[tusharsadhwani]

Picking this up again this weekend. Let's ship this soon 💪

[tusharsadhwani]

@JelleZijlstra I'm unable to reproduce that case on this branch as of now:

$ black --unstable -c '''raise ValueError(
                "xxxxxxxxxxxIncorrect --line-ranges format, expect 'START-END', found"
                f" {lines_str!r}"
            )'''
raise ValueError(
    "xxxxxxxxxxxIncorrect --line-ranges format, expect START-END, found" f" {
        lines_str!r
    }"
)