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

Take into account __future__ imports and pass flags to compile() #54

Closed
wants to merge 1 commit into from
Closed

Take into account __future__ imports and pass flags to compile() #54

wants to merge 1 commit into from

Conversation

flyte
Copy link

@flyte flyte commented Mar 28, 2015

Fixes #53 by parsing the source for future imports, then supplying the correct flags to compile().

This has the side-effect of changing the function signature for reformatter.Reformat() by allowing the unformatted_source to be passed in. This may not be ideal, so please let me know if you can see a more suitable way to get the compiler flags from where they're extracted from the source to the compile() calls in verifier.VerifyCode().

I have accepted the CLA.

@bwendling
Copy link
Member

Hi Flyte,

Thanks for the code. I looked over it and I'm not sure that we need all of this for what we're doing. Or at the least it should be done in a different place. :-)

The idea of the "verifier" module is that it will catch catastrophic errors. Note that there's a better way to validate that the semantics haven't changed (generating before and after ASTs and comparing them). So with that in mind, this seems like a bit of overkill. An easier way to do this is to generate all of the supported "from future import ..." statements and try to compile / parse them in the verifier module. We do something similar with the print statement already.

@flyte
Copy link
Author

flyte commented Mar 28, 2015

Understood. I shouldn't have stormed ahead with it without discussing it here first anyway.. I'll probably turn the code into a library since I couldn't find any to do this. Thanks for your responses.

@flyte flyte closed this Mar 28, 2015
@flyte
Copy link
Author

flyte commented Mar 29, 2015

Just if you're interested, I looked into the AST you mentioned and have used it to simplify my code massively. I did turn it into a library but it's hardly worth it now that it's so easy!
https://github.com/flyte/static-future/blob/develop/static_future/analysis.py

@bwendling
Copy link
Member

A side note, we're thinking about disabling the verification step for all but development (and maybe not even that).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Future imports aren't taken into account when checking syntax
3 participants