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

Fix Github issue 855 - fail to parse with statement #861

Merged
merged 2 commits into from
Feb 16, 2023

Conversation

stroxler
Copy link
Contributor

@stroxler stroxler commented Feb 15, 2023

Fixes #855

Summary

When we added support for parenthesized with statements, the grammar on the with itself was correct (it's a right and left parenthesis around a comma-separated list of with-items, with a possible trailing comma).

But inside of the "as" variation of the with_item rule we have a peek at the next character, which was allowing for a comma or a colon. That peek needs to also accept right parentheses - otherwise, if the last item contains an as and has no trailing comma we fail to parse.

The bug is exercisecd by, for example, this code snippet:

with (foo, bar as bar,):
    pass

The with_wickedness test fixture has been revised to include both the plain and async variations of this example snippet with and without trailing comma, and tests pass after the peek rule fix.

Test Plan

cd native
cargo test

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 15, 2023
When we added support for parenthesized with statements, the
grammar on the with itself was correct (it's a right and left
parenthesis around a comma-separated list of with-items, with
a possible trailing comma).

But inside of the "as" variation of the with_item rule we have a peek at
the next character, which was allowing for a comma or a colon. That peek
needs to also accept right parentheses - otherwise, if the last item
contains an `as` and has no trailing comma we fail to parse.

The bug is exercisecd by, for example, this code snippet:
```
with (foo, bar as bar,):
    pass
```

The with_wickedness test fixture has been revised to include both
the plain and async variations of this example snippet with and without
trailing comma, and tests pass after the peek rule fix.
@codecov-commenter
Copy link

codecov-commenter commented Feb 15, 2023

Codecov Report

Base: 94.79% // Head: 94.79% // No change to project coverage 👍

Coverage data is based on head (df249a8) compared to base (1ee04c6).
Patch has no changes to coverable lines.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #861   +/-   ##
=======================================
  Coverage   94.79%   94.79%           
=======================================
  Files         249      249           
  Lines       25831    25831           
=======================================
  Hits        24487    24487           
  Misses       1344     1344           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@zsol zsol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

amazing :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fails to parse parenthesized with statement without trailing comma
4 participants