fix: Panic on move_guard_to_arm_body with struct literals - #23306
Open
Wilfred wants to merge 1 commit into
Open
Conversation
Wilfred
force-pushed
the
fix/move-guard-struct-literal-parens
branch
from
September 7, 2026 13:43
c4a8fd2 to
a867960
Compare
ChayimFriedman2
requested changes
Sep 7, 2026
| let guard_condition = guard.condition()?.reset_indent(); | ||
|
|
||
| let mut guard_condition = guard.condition()?.reset_indent(); | ||
| if guard_condition.contains_exterior_struct_lit() { |
Contributor
There was a problem hiding this comment.
You should use needs_parens_in() instead.
Contributor
Author
There was a problem hiding this comment.
Using needs_parens_in_place_of() made the changes a bit simpler, so I went with that instead. Let me know what you think :)
Rust syntax requires parentheses on struct literals in if statements.
Previously, we'd try to compute the edit for the
move_guard_to_arm_body assist, try to parse `if foo == SomeStruct { x:
123 }` and panic.
Instead, wrap the expression in parentheses so it's legal Rust. This
fixes the panic and ensures the assist produces well-formed code.
This assist doesn't exactly match the suggestions in rustc, but it's
still valid output and much simpler to implement.
(rustc suggests `if foo == (SomeStruct { x: 123 })` whereas we now produce
`if (foo == SomeStruct { x: 123 })`.)
AI disclosure: Code written with assistance by Opus 5, but comments,
commit message and review by me.
Wilfred
force-pushed
the
fix/move-guard-struct-literal-parens
branch
from
September 7, 2026 15:01
a867960 to
a6040d3
Compare
ChayimFriedman2
requested changes
Sep 7, 2026
|
|
||
| let guard_condition = guard.condition()?.reset_indent(); | ||
| let guard_condition = if guard_condition | ||
| .needs_parens_in_place_of(empty_if.syntax(), empty_condition.syntax()) |
Contributor
There was a problem hiding this comment.
Ugh. I forgot that we don't have the if available.
So I'm torn between two options: creating the if first and using the SyntaxEditor to wrap in parens if needed, or the previous way. The current way is not good IMO.
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.
Rust syntax requires parentheses on struct literals ("exterior struct literals") in if statements.
Previously, we'd try to compute the edit for the move_guard_to_arm_body assist, try to parse
if foo == SomeStruct { x: 123 }and panic.Instead, wrap the expression in parentheses so it's legal Rust. This fixes the panic and ensures the assist produces well-formed code.
This assist doesn't exactly match the suggestions in rustc, but it's still valid output and much simpler to implement.
(rustc suggests
if foo == (SomeStruct { x: 123 })whereas we now produceif (foo == SomeStruct { x: 123 }).)AI disclosure: Code written with assistance by Opus 5, but comments, commit message and review by me.