Repository navigation
Preserve whitespace before : in except handlers without as - #1473
Open
Thanatos9404 wants to merge 1 commit into
Open
Thanatos9404 wants to merge 1 commit into
Thanatos9404 wants to merge 1 commit into
Conversation
The native parser only read the whitespace before the colon of an ExceptHandler or ExceptStarHandler when the handler had an `as` name. Without a name that whitespace was not stored anywhere, so `except A :` and `except* A :` came back as `except A:` and `except* A:` after a parse and codegen round trip. Always parse whitespace_before_colon from the colon token. A bare `except :` is unchanged: its whitespace is shared with the colon token and is still consumed first by whitespace_after_except. Add round-trip fixtures for both handler types, and node tests that check whitespace_before_colon is populated when there is no name. Fixes Instagram#1457
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1457.
The native parser only read the whitespace before the colon of an
exceptorexcept*handler when the handler had anasname. Without one,whitespace_before_colonwas left at its default and the whitespace was dropped, soexcept A :came back asexcept A:. TheExceptHandlerandExceptStarHandlernodes already have the field and emit it in codegen, so the fix is only in the twoInflateimpls innative/libcst/src/nodes/statement.rs: always parsewhitespace_before_colonfrom the colon token.A bare
except :parses the same as before (whitespace_after_except=" ",whitespace_before_colon=""). The whitespace afterexceptand before:is the same token whitespace there, andwhitespace_after_exceptconsumes it first, so it is not duplicated. There is a test for that case.Test Plan
New tests:
native/libcst/tests/fixtures/terrible_tries.pyandstarry_tries.py: whitespace before:after a name, a parenthesized tuple, an unparenthesized tuple (PEP 758) and anasname, plus a bareexcept :. Bothparser_roundtrip.rsandlibcst/tests/test_roundtrip.pyround-trip these fixtures.libcst/_nodes/tests/test_try.py: parse and codegen cases checking thatwhitespace_before_colonis populated forexcept Exception :andexcept (IOError, ImportError) :, the same two forexcept*, and that a bareexcept :still parses as before.On
mainplus only the new tests, the four new whitespace cases and both fixture round trips fail (except foo :comes back asexcept foo:); the bareexcept :case passes. With the fix (Ubuntu 24.04 on WSL2, Python 3.10, debug build) these pass:uv run poe testpyre checkcargo test --manifest-path=native/Cargo.toml, with and without--no-default-featurescargo fmt --all --manifest-path=native/Cargo.toml -- --checkcargo clippy --manifest-path=native/Cargo.toml --all-targets --all-features(no new warnings)flake8andufmt checkontest_try.py,scripts/check_copyright.py