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

Complex decorators break safe mode #2782

Closed
emfdavid opened this issue Jan 18, 2022 · 7 comments
Closed

Complex decorators break safe mode #2782

emfdavid opened this issue Jan 18, 2022 · 7 comments
Labels
C: parser How we parse code. Or fail to parse it. T: bug Something isn't working

Comments

@emfdavid
Copy link
Contributor

Using decorator syntax with a chained function call breaks the AST validation in safe mode.
This is a pattern for prometheus client

Use black playground repro case

Screen Shot 2022-01-18 at 6 28 09 PM

If I comment out the chained labels - it works.
Screen Shot 2022-01-18 at 6 30 16 PM

Currently my best work around is to ignore the files using this syntax in my config and run black manually with fast for those files.

@emfdavid emfdavid added the T: bug Something isn't working label Jan 18, 2022
@JelleZijlstra
Copy link
Collaborator

We may never have properly added support for https://www.python.org/dev/peps/pep-0614/.

@JelleZijlstra JelleZijlstra added the C: parser How we parse code. Or fail to parse it. label Jan 18, 2022
@isidentical
Copy link
Collaborator

isidentical commented Jan 19, 2022

AFAIK the PEP 614 support is there:

decorator: '@' namedexpr_test NEWLINE

This seems like a problem with Black Playground, since they are still using Python 3.8. Created jpadilla/black-playground#53

@emfdavid what is the version of the python interpreter you are running the black with?

@emfdavid
Copy link
Contributor Author

Excellent catch @isidentical
I set our commit hook to use py37 as the lowest level supported and didn't think twice about it when I could repro the issue in playground.
Are issues with ast validation rare enough that it would be helpful to add try checking your python version? to the error message?

@JelleZijlstra
Copy link
Collaborator

@emfdavid good idea, do you want to submit a PR?

@isidentical
Copy link
Collaborator

Are issues with ast validation rare enough that it would be helpful to add try checking your python version? to the error message?

I would assume if people are using the newer syntax, their python version would be higher. Though in case of pre-commit, it might go unnoticed considering it is using a separate environment. So I think it makes sense.

@isidentical
Copy link
Collaborator

This has been now fixed on the playground as well!

@emfdavid
Copy link
Contributor Author

@JelleZijlstra #2786

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. T: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants