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

Walrus operator's left hand side now has STORE expression context #433

Merged
merged 6 commits into from
Dec 16, 2020

Conversation

zsol
Copy link
Member

@zsol zsol commented Dec 15, 2020

Summary

This PR makes scope provider assign a STORE expression context to the left hand side of a walrus operator. So, in an x := y expression, x should have ExpressionContext.STORE as its expression context metadata.

Test Plan

Added unit tests.

@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 Dec 15, 2020
@codecov-io
Copy link

codecov-io commented Dec 15, 2020

Codecov Report

Merging #433 (73df5d9) into master (88dd0c3) will decrease coverage by 0.00%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #433      +/-   ##
==========================================
- Coverage   94.32%   94.31%   -0.01%     
==========================================
  Files         232      232              
  Lines       22641    22657      +16     
==========================================
+ Hits        21355    21368      +13     
- Misses       1286     1289       +3     
Impacted Files Coverage Δ
libcst/metadata/tests/test_scope_provider.py 98.70% <50.00%> (-0.51%) ⬇️
libcst/metadata/expression_context_provider.py 98.16% <100.00%> (+0.06%) ⬆️
...metadata/tests/test_expression_context_provider.py 100.00% <100.00%> (ø)
libcst/_typed_visitor.py 96.75% <0.00%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 88dd0c3...73df5d9. Read the comment docs.

if x := y:
pass
"""
wrapper = MetadataWrapper(parse_module(dedent(code)))
Copy link
Contributor

Choose a reason for hiding this comment

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

The version can be decoupled; if you want, this will let the test run everywhere:

parse_module(
  dedent(code),
  config=cst.PartialParserConfig(python_version="3.8"),
)

@@ -1609,6 +1610,27 @@ def test_no_out_of_order_references_in_global_scope(self) -> None:
),
)

def test_walrus_accesses(self) -> None:
if sys.version_info < (3, 8):
Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing here?

Copy link
Member Author

Choose a reason for hiding this comment

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

This one is not as easy, I'd have to plumb the parser config through some helper functions, so I thought it's fine to keep it as is

Copy link
Contributor

@thatch thatch left a comment

Choose a reason for hiding this comment

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

Change is pretty good.

@zsol zsol merged commit 1571cdd into Instagram:master Dec 16, 2020
@zsol zsol deleted the walrus-expression-context branch December 16, 2020 19:58
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.

None yet

4 participants