bpo-28002: Roundtrip f-strings with ast.unparse better

[hauntsaninja]

By attempting to avoid backslashes in f-string expressions.
We also now proactively raise errors for some backslashes we can't
avoid while unparsing FormattedValues

https://bugs.python.org/issue28002

Automerge-Triggered-By: GH:isidentical

[hauntsaninja]

Some notes:

• I'm not particularly familiar with unicode intricacies, so it's possible there is a simpler, more robust or more performant way to do this.
• Let me know if there are more tests that should be added.
• Maybe we can skip-news, since ast.unparse is new in Python 3.9 anyway?
• Part of this patch is based on @simonpercivall's work in astunparse. If there is credit-giving to be given, please let me know how he can be credited for that.

cc @isidentical

[hauntsaninja]

The test failure in test_concurrent_futures on Windows x86 seems unrelated to this PR

[hauntsaninja]

One issue with the PR as it stands is the perhaps undesirable unparsing of the following, probably reasonably common f-string:

>>> import ast >>> print(ast.unparse(ast.parse(r'''f"{x}\n" '''))) f'''{x} ''' 

For this to pass a src roundtrip, we'd need to process the buffer a little more intelligently when self.avoid_backslashes is False.

[isidentical]

@hauntsaninja would you rebase and (if you liked it) apply the suggestions. So we can move forward, and try to improve it. I have a script that I'll plan to test this PR against top PyPI packages, so we might find more problems / or verify that it works well at the end.

[isidentical]

FYI, this patch fails on roundtripping this file https://github.com/numpy/numpy/blob/master/tools/refguide_check.py

[isidentical]

What about not including the \n into the exceptions list in should_use_repr?

[hauntsaninja]

Thanks, updated to include your two comments!

I think including \n into the exception list is reasonable, and would fix the common f"{x}\n" case. There would be a regression in being able to unparse some of the test cases I added in test_fstrings_complicated, but those are less common, so it would still be the pragmatic thing to do.

Let me see if I can come up with a fix over the weekend, otherwise we can go ahead and make the should_use_repr change.

[isidentical]

A gentle ping @hauntsaninja

[hauntsaninja]

Thanks for the reminder!

Made changes that should result in better cosmetic unparsing of cases like f"{x}\n" and fixed issues where we'd struggle because we might need to escape the final quote within triple quotes (basically more complicated versions of """\""""). These changes also don't regress against any existing test cases added in this PR.

[hauntsaninja]

@isidentical Thanks for taking another look! I made the changes as suggested, except for the ones I replied to

[isidentical]

@pablogsal would you mind checking out this?

[isidentical]

Thanks for the continuous work @hauntsaninja! If you have time, please try the latest version on the most downloaded PyPI packages set, if not please let me know (so I can test it by myself).

[isidentical]

Also, you should be able to collapse my reviews (mark them as resolved) so that it won't distract people when reading the history.

[hauntsaninja]

The CI failure here looks like a flake. I re-ran AST roundtripping over 4k packages and everything was clear (apart from some pre-existing RecursionErrors)

[isidentical]

A gentle ping @ericvsmith. Do you plan to take a look?

[hauntsaninja]

I'd love for this to make 3.9.1, if possible! :-)

[isidentical]

closing and re-opening for the CI

[ericvsmith]

Sorry for the delay. This looks reasonable enough to me, but someone who's more familiar with the AST and/or unparsing should take a look. Maybe @ambv?

[isidentical]

re-triggering the CI (some irrelevant macos failure)

[bedevere-bot]

@isidentical: Please replace # with GH- in the commit message next time. Thanks!

[miss-islington]

Thanks @hauntsaninja for the PR, and @isidentical for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

[bedevere-bot]

GH-23430 is a backport of this pull request to the 3.9 branch.

[isidentical]

Thank you so much for your continuous efforts on this issue @hauntsaninja.

[hauntsaninja]

Thank you for making this PR happen! :-)