Skip to content
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.

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

Black fails to format an un-formatted file with walrus operator #3143

Closed
michcio1234 opened this issue Jun 28, 2022 · 6 comments
Closed

Black fails to format an un-formatted file with walrus operator #3143

michcio1234 opened this issue Jun 28, 2022 · 6 comments
Labels
C: parser How we parse code. Or fail to parse it. R: not a bug This is deliberate behavior of Black. T: bug Something isn't working

Comments

@michcio1234
Copy link

michcio1234 commented Jun 28, 2022

Describe the bug

If a file is not black-formatted and it contains a walrus operator, Black fails.
If a file is already black-formatted and it contains a walrus operator, Black succeeds.

It happens on my machine on version 22.6.0. It doesn't happen in playground.

To Reproduce

With this code, which is already black-formatted, all is good:

if 3 > (a := 3):
    pass

x = [1, 2]
$ black black_walrus.py 
All done! ✨ 🍰 ✨
1 file left unchanged.

But with this code, which is not black-formatted (notice the magic comma), Black fails:

if 3 > (a := 3):
    pass

x = [1, 2,]
$ black white_walrus.py 
error: cannot format white_walrus.py: cannot use --safe with this file; failed to parse source file AST: invalid syntax (<unknown>, line 1)
This could be caused by running Black with an older Python version that does not support new syntax used in your source file.

Oh no! 💥 💔 💥
1 file failed to reformat.

Expected behavior

In the second example, Black should re-format the file with no errors.

Environment

$ black --version
black, 22.6.0 (compiled: yes)
Python (CPython) 3.6.10

$ python --version
Python 3.6.10 :: Anaconda, Inc.

OS: Linux Mint 20.3, kernel 5.4.0-120-generic

Additional context

Since it works correctly in the playground, I suppose something is wrong with my local installation, but I have no idea what it can be.

@michcio1234 michcio1234 added the T: bug Something isn't working label Jun 28, 2022
@felix-hilden
Copy link
Collaborator

Thanks for reporting in detail! If the problem is in the trailing comma, does the previous statement (walrus) affect it? So, does it fail if it's removed?

@Jackenmen
Copy link
Contributor

You're using Python 3.6 to format a file using Python 3.8 feature. I don't think that's supported.

@michcio1234
Copy link
Author

@jack1142 Hahh, of course. 🤦🏻 Updating to Python 3.9 fixed the issue. Thank you.

@felix-hilden
Copy link
Collaborator

I think there's something to this. For example, I have 3.9 but I'm able to run Black on match statements with --target-version py310. However, it only works when there's nothing to change, but fails with "cannot parse" when the target version is left out and "invalid syntax" when there are changes.

I'm unsure how the parsing works exactly, but having it work with no changes seems odd to me.

@ichard26 ichard26 added R: not a bug This is deliberate behavior of Black. C: parser How we parse code. Or fail to parse it. labels Jun 28, 2022
@michcio1234
Copy link
Author

michcio1234 commented Jun 28, 2022

Yeah, that's what was happening to me. It was not about the magic comma; it was about any reformatting at all. I think I even tried with --target-version py39 and got the same behaviour (I'm not 100% sure though).

It would be good if Black informed that targeting a higher version of Python that the Black itself is running on is not supported.
Actually, it could also work for auto-detected target Python version.

@ichard26
Copy link
Collaborator

ichard26 commented Jun 28, 2022

The reason is because our main parser (blib2to3) is extremely lax (mostly to support Python 3.10 syntax without rewriting the whole parser and therefore codebase) so it doesn't care if the Python version is too low, it'll happily parse newer syntax. For the AST equivalence safety check though, it uses the built-in ast module which obviously does not support syntax beyond the running Python. We only run the safety check if changes were made so that's why if no changes are made, Black "works."

I think there's something to this. For example, I have 3.9 but I'm able to run Black on match statements with --target-version py310. However, it only works when there's nothing to change, but fails with "cannot parse" when the target version is left out and "invalid syntax" when there are changes.

Yeah we do lock the very coarse Python version configuration of blib2to3 to the target-version, but by default without a target-version specified, it should select all grammars. Are you sure you're running Black in a clean directory (without pyproject.toml?)

It would be good if Black informed that targeting a higher version of Python that the Black itself is running on is not supported.
Actually, it could also work for auto-detected target Python version.

I like the idea, but doing it for auto-detected Python is a bit harder since we would want to warn the user only once. That's difficult to do when the files are being processed in different processes at the same time thanks to multiprocessing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: parser How we parse code. Or fail to parse it. R: not a bug This is deliberate behavior of Black. T: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants