11178: Fix replace_match_with_if_let removing unsafe blocks r=bugadani a=bugadani

If the assist encounters an unsafe block in one of the match arms, the assist generated intermediate code like the following:

```rust
if let Foo(_) = foo {
    <then branch>
} else unsafe { ... }
```

Which was then parsed back and the unsafe branch got completely removed, removing in invalid code output:

```rust
if let Foo(_) = foo {
    <then branch>
} else
```

This PR fixes this issue.

However, I'm sure there is a better, more general solution here, but I lack familiarity with rust-analyzer. `make::expr_if` looks like it expects a `BlockExpr` that, when printed, is wrapped in braces correctly, but I'm sure changing the display impl for an `unsafe` `BlockExpr` would have caused problems. I could have changed `make::expr_if` instead to special case unsafe blocks, but that would have meant some expressions getting wrapped by the caller (as previously), and some others by the function.

Co-authored-by: Dániel Buga <bugadani@gmail.com>
This commit is contained in:
bors[bot] 2022-01-03 13:09:15 +00:00 committed by GitHub
commit 29fc022d85
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -207,21 +207,23 @@ pub(crate) fn replace_match_with_if_let(acc: &mut Assists, ctx: &AssistContext)
"Replace match with if let",
target,
move |edit| {
fn make_block_expr(expr: ast::Expr) -> ast::BlockExpr {
// Blocks with modifiers (unsafe, async, etc.) are parsed as BlockExpr, but are
// formatted without enclosing braces. If we encounter such block exprs,
// wrap them in another BlockExpr.
match expr {
ast::Expr::BlockExpr(block) if block.modifier().is_none() => block,
expr => make::block_expr(iter::empty(), Some(expr)),
}
}
let condition = make::condition(scrutinee, Some(if_let_pat));
let then_block = match then_expr.reset_indent() {
ast::Expr::BlockExpr(block) => block,
expr => make::block_expr(iter::empty(), Some(expr)),
};
let then_block = make_block_expr(then_expr.reset_indent());
let else_expr = if is_empty_expr(&else_expr) { None } else { Some(else_expr) };
let if_let_expr = make::expr_if(
condition,
then_block,
else_expr
.map(|expr| match expr {
ast::Expr::BlockExpr(block) => block,
expr => (make::block_expr(iter::empty(), Some(expr))),
})
.map(ast::ElseBranch::Block),
else_expr.map(make_block_expr).map(ast::ElseBranch::Block),
)
.indent(IndentLevel::from_node(match_expr.syntax()));
@ -917,4 +919,30 @@ fn foo() {
"#,
);
}
#[test]
fn test_replace_match_with_if_let_keeps_unsafe_block() {
check_assist(
replace_match_with_if_let,
r#"
impl VariantData {
pub fn is_struct(&self) -> bool {
$0match *self {
VariantData::Struct(..) => true,
_ => unsafe { unreachable_unchecked() },
}
}
} "#,
r#"
impl VariantData {
pub fn is_struct(&self) -> bool {
if let VariantData::Struct(..) = *self {
true
} else {
unsafe { unreachable_unchecked() }
}
}
} "#,
)
}
}