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

Failing parse on nested with-statements. #104

Closed
Ferada opened this issue Jan 11, 2015 · 4 comments
Closed

Failing parse on nested with-statements. #104

Ferada opened this issue Jan 11, 2015 · 4 comments
Milestone

Comments

@Ferada
Copy link
Contributor

Ferada commented Jan 11, 2015

I've uploaded this commit with two failing teststhis commit with one failing test case which, one just fails to refactor correctly, the other raises an exception because(?) the source code doesn't exactly match the AST, i.e.:

    with open("test") as file1, open("test") as file2:
        print(file1, file2)

fails to parse; there is a second (nested) with node in the AST, but in _Source.consume/patchedast.py:643 the self.offset is at the end of the line and couldn't match a second, non-existent with anyway.

The other, simpler case is a regular nested `with`, i.e. extracting a method from the following code (the `print` line):
    with open("test") as file1:
        with open("test") as file2:
            print(file1, file2)

will refactor (and miss the two variables) to:

    def extracted():
        print(file1, file2)

    with open("test") as file1:
        with open("test") as file2:
            extracted()

I'm pretty sure I can figure out the second case, but for the first I'm at a loss how to start fixing it. Any pointers?

Edit: I've changed the one test case to reflect the working behaviour with the added global_ flag.

@aligrudi
Copy link
Member

Olof-Joachim Frahm notifications@github.com wrote:

I've uploaded this commit with two failing tests, one just fails to refactor correctly, the other raises an exception because(?) the source code doesn't exactly match the AST, i.e.:

    with open("test") as file1, open("test") as file2:
        print(file1, file2)

fails to parse; there is a second (nested) with node in the AST, but in _Source.consume/patchedast.py:643 the self.offset is at the end of the line and couldn't match a second, non-existent with anyway.

The other, simpler case is a regular nested with, i.e. extracting a method from the following code (the print line):

    with open("test") as file1:
        with open("test") as file2:
            print(file1, file2)

will refactor (and miss the two variables) to:

    def extracted():
        print(file1, file2)

    with open("test") as file1:
        with open("test") as file2:
            extracted()

I'm pretty sure I can figure out the second case, but for the first I'm at a loss how to start fixing it. Any pointers?

I guess both of these cases are due to rope/refactor/patchedast.py.
The file tries to match the nodes of an AST with the source code,
which is necessary for some refactorings that modify the AST
directly. The _handle() method of _PatchingASTWalker matches
a list of ast nodes and strings (like keywords) recursively.

Ali

@Ferada
Copy link
Contributor Author

Ferada commented Jan 11, 2015

Actually since I've now stepped through it, the second one works if global_ is set, which is still a bit unexpected, but okay, both variables are global, so I kind of understand why now.

I guess both of these cases are due to rope/refactor/patchedast.py.
The file tries to match the nodes of an AST with the source code,
which is necessary for some refactorings that modify the AST
directly. The _handle() method of _PatchingASTWalker
matches a list of ast nodes and strings (like keywords) recursively.

Yes, so what if there is no corresponding part of the source code? The second with isn't actually anywhere in the source code. Maybe it would work if the region for the first one is reused; or alternatively it could be discarded and only the part from the comma, open("test") as file2 is used instead?

@aligrudi
Copy link
Member

Olof-Joachim Frahm notifications@github.com wrote:

The file tries to match the nodes of an AST with the source code,
which is necessary for some refactorings that modify the AST
directly. The _handle() method of _PatchingASTWalker
matches a list of ast nodes and strings (like keywords) recursively.

Yes, so what if there is no corresponding part of the source code? The second with isn't actually anywhere in the source code. Maybe it would work if the region for the first one is reused; or alternatively it could be discarded and only the part from the comma, open("test") as file2 is used instead?

Sorry, I did not read your first message fully. Do you
mean both:

with open("test") as file1, open("test") as file2:

and:

with open("test") as file1:
    with open("test") as file2:

are parsed exactly the same way (node.optional_vars in _With
containing only the first and the second being nested)?

If so, a workaround may be to do something like semicolon_or_as_in_except
to handle two cases that produce the same AST.

Ali

@mcepl mcepl added bug Unexpected or incorrect user-visible behavior needinfo labels Aug 9, 2015
@lieryan lieryan added this to the 0.21.0 milestone Sep 13, 2021
lieryan added a commit that referenced this issue Sep 26, 2021
Nested with statements. Fix issue #104
@lieryan
Copy link
Member

lieryan commented Sep 26, 2021

This issue is fixed in #398. Thanks @climbus contributing the fix.

@lieryan lieryan closed this as completed Sep 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants