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 produced invalid code: assert (*x) #921

Closed
dizzy57 opened this issue Jul 9, 2019 · 4 comments
Closed

Black produced invalid code: assert (*x) #921

dizzy57 opened this issue Jul 9, 2019 · 4 comments
Labels
C: crash Black is crashing

Comments

@dizzy57
Copy link

dizzy57 commented Jul 9, 2019

Operating system: macOS Mojave 10.14.5 (18F132)
Python version: Python 3.7.3
Black version: 19.3b0 (f3bb22a)
Does also happen on master: yes

Minified reproducer:

assert (*x)

Stack trace:

  File "black.py", line 3547, in assert_equivalent
    dst_ast = parse_ast(dst)
  File "black.py", line 3490, in parse_ast
    return ast27.parse(src)
  File "/Users/apreobrazhen/miniconda3/lib/python3.7/site-packages/typed_ast/ast27.py", line 51, in parse
    return _ast27.parse(source, filename, mode)
assert *x

This is invalid python code that gets accepted by blib2to3 grammar. I think black should throw InvalidInput exception in this case, not AssertionError.

Maybe it's worth raising an upstream bug for lib2to3?

@zsol zsol added T: bug Something isn't working lib2to3 and removed T: bug Something isn't working labels Jul 9, 2019
@zsol
Copy link
Collaborator

zsol commented Jul 9, 2019

I agree lib2to3 shouldn't accept this. There's a certain class of these python programs that are technically invalid python to start with, so Black is at a severe disadvantage of dealing with. I don't think this is super important to fix in Black as it will usually generate invalid code (like above) and then fail promptly.

I can't come up with an example that's all of:

  • invalid python
  • Black accepts it
  • Black generates valid python at the end

This would be bad as Black would hide the fact that the original code was not valid Python and thus all bets are off, but as a user of Black it's very hard to tell there was an error in the first place.

@dizzy57
Copy link
Author

dizzy57 commented Jul 10, 2019

I think it still would be nice to show a "Cannot parse" message instead of "INTERNAL ERROR, please report a bug".

@isidentical
Copy link
Collaborator

I agree with @zsol, there are numerous of examples where black can parse but CPython can't parse (or rather don't accept as input). The current error message seems to be better though:

 $ black -c "assert (*x)"                                                                                                                              19ms
assert (*x)
error: cannot format <string>: cannot use --safe with this file; failed to parse source file: can't use starred expression here (<unknown>, line 1)

@JelleZijlstra
Copy link
Collaborator

Thanks, closing as the current behavior seems fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: crash Black is crashing
Projects
None yet
Development

No branches or pull requests

4 participants