Skip to content

t-strings: AST unparsing does not reproduce original formatting #133581

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
JelleZijlstra opened this issue May 7, 2025 · 7 comments
Closed

t-strings: AST unparsing does not reproduce original formatting #133581

JelleZijlstra opened this issue May 7, 2025 · 7 comments
Labels
3.14 bugs and security fixes stdlib Python modules in the Lib dir topic-parser type-bug An unexpected behavior, bug, or error

Comments

@JelleZijlstra
Copy link
Member

JelleZijlstra commented May 7, 2025 •
edited by bedevere-app bot
Loading

Bug report

Bug description:

The AST unparsing of Interpolation objects does not reproduce formatting within interpolations.

This is true both for ast.unparse in Python:

>>> ast.unparse(ast.parse('t"{a   + b}"'))
"t'{a + b}'"

And for the C unparse that is used by from __future__ import annotations:

>>> ns = {}
>>> exec("""
... from __future__ import annotations
... def f(x: t"{a    + b}"): ...
... """, ns)
>>> ns["f"].__annotations__
{'x': "t'{a + b}'"}

In general, we can't recover the original source exactly, but in this case we do have the original source available:

>>> ast.parse('t"{a   + b}"').body[0].value.values[0].str
'a   + b'

Let's use it.

CPython versions tested on:

3.14

Operating systems tested on:

No response

Linked PRs
@JelleZijlstra JelleZijlstra added type-bug An unexpected behavior, bug, or error 3.14 bugs and security fixes labels May 7, 2025
@picnixz picnixz added the stdlib Python modules in the Lib dir label May 7, 2025
@JelleZijlstra
Copy link
Member Author

This leads to AST roundtrip failures (showing up in test cases in #133553).

@JelleZijlstra
Copy link
Member Author

cc @lysnikolaou

@lysnikolaou
Copy link
Member

Hmm, what exactly is failing in the AST roundtrip tests? test_unparse only checks ASTs, which should be the same for all of these, right?

@JelleZijlstra
Copy link
Member Author

The AST changes on roundtrip because the AST for an interpolation object includes the raw text, which changes.

@lysnikolaou
Copy link
Member

Oh right, I hit that while working on the PEP PR and thought I'd implemented skipping checking ast.Interpolation.str in test_unparse.

Not sure whether we should be using it for test_unparse, that'd be breaking assumptions for the formatting of the code. For example, a + b roundtrips to a + b. Why should t-string be different in that regard?

@JelleZijlstra
Copy link
Member Author

Why should t-string be different in that regard?

Because in t-strings the original source code is part of the AST, and in the rest of the grammar it isn't.

In general I feel ast.unparse(ast.parse(...)) should roundtrip as much as possible, to the extent that the original source code is still recoverable from the AST.

@bedevere-app bedevere-app bot mentioned this issue May 8, 2025
Merged
JelleZijlstra added a commit that referenced this issue May 8, 2025
@JelleZijlstra
gh-133581: Improve AST unparsing of t-strings (#133635)
bfac7d2
miss-islington pushed a commit to miss-islington/cpython that referenced this issue May 8, 2025
@JelleZijlstra @miss-islington
pythongh-133581: Improve AST unparsing of t-strings (pythonGH-133635)
bfaea03 
(cherry picked from commit bfac7d2)

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@bedevere-app bedevere-app bot mentioned this issue May 8, 2025
Merged
JelleZijlstra added a commit that referenced this issue May 8, 2025
@miss-islington @JelleZijlstra
[3.14] gh-133581: Improve AST unparsing of t-strings (GH-133635) (#13…
1534969 
…3666)

gh-133581: Improve AST unparsing of t-strings (GH-133635)
(cherry picked from commit bfac7d2)

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
JelleZijlstra added a commit to JelleZijlstra/cpython that referenced this issue May 9, 2025
@JelleZijlstra
pythongh-133581: Fix refleak in AST unparsing
7057e79
@bedevere-app bedevere-app bot mentioned this issue May 9, 2025
Merged
lysnikolaou pushed a commit that referenced this issue May 9, 2025
@JelleZijlstra
gh-133581: Fix refleak in t-string AST unparsing (#133724)
308ceff
miss-islington pushed a commit to miss-islington/cpython that referenced this issue May 9, 2025
@JelleZijlstra @miss-islington
pythongh-133581: Fix refleak in t-string AST unparsing (pythonGH-133724)
f02c29d 
(cherry picked from commit 308ceff)

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@bedevere-app bedevere-app bot mentioned this issue May 9, 2025
Merged
lysnikolaou pushed a commit that referenced this issue May 9, 2025
@miss-islington @JelleZijlstra
[3.14] gh-133581: Fix refleak in t-string AST unparsing (GH-133724) (#…
92d5677 
…133731)

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@lysnikolaou
Copy link
Member

This has been fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
3.14 bugs and security fixes stdlib Python modules in the Lib dir topic-parser type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@JelleZijlstra @tomasr8 @picnixz @lysnikolaou and others