Skip to content

Commit f2b8b59

Browse files
committed
fix: refine remove_parentheses for return-like expressions
1 parent c9892f6 commit f2b8b59

File tree

1 file changed

+42
-11
lines changed

1 file changed

+42
-11
lines changed

crates/ide-assists/src/handlers/remove_parentheses.rs

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,7 @@ pub(crate) fn remove_parentheses(acc: &mut Assists, ctx: &AssistContext<'_>) ->
3434

3535
let expr = parens.expr()?;
3636

37-
if let ast::Expr::ReturnExpr(ret) = &expr
38-
&& ret.expr().is_none()
37+
if let ast::Expr::ReturnExpr(_) = &expr
3938
&& let Some(r_paren) = parens.r_paren_token()
4039
&& let Some(next) = r_paren.next_token().and_then(|t| skip_trivia_token(t, Direction::Next))
4140
&& next.kind() == T![||]
@@ -45,7 +44,11 @@ pub(crate) fn remove_parentheses(acc: &mut Assists, ctx: &AssistContext<'_>) ->
4544
}
4645

4746
let parent = parens.syntax().parent()?;
48-
if expr.needs_parens_in(&parent) {
47+
let allow_prefix_ret_like = matches!(
48+
(&expr, ast::Expr::cast(parent.clone())),
49+
(e, Some(ast::Expr::PrefixExpr(_))) if matches!(e, ast::Expr::ReturnExpr(_) | ast::Expr::BreakExpr(_) | ast::Expr::ContinueExpr(_))
50+
);
51+
if !allow_prefix_ret_like && expr.needs_parens_in(&parent) {
4952
return None;
5053
}
5154

@@ -174,15 +177,16 @@ mod tests {
174177
}
175178

176179
#[test]
177-
fn remove_parens_prefix_with_return_no_value() {
178-
// removing `()` from !(return) would make `!return` which is invalid syntax
179-
check_assist_not_applicable(
180+
fn remove_parens_prefix_with_ret_like_prefix() {
181+
check_assist(remove_parentheses, r#"fn f() { !$0(return) }"#, r#"fn f() { !return }"#);
182+
// `break`, `continue` behave the same under prefix operators
183+
check_assist(remove_parentheses, r#"fn f() { !$0(break) }"#, r#"fn f() { !break }"#);
184+
check_assist(remove_parentheses, r#"fn f() { !$0(continue) }"#, r#"fn f() { !continue }"#);
185+
check_assist(
180186
remove_parentheses,
181-
r#"fn main() { let _x = true && !$0(return) || true; }"#,
187+
r#"fn f() { !$0(return false) }"#,
188+
r#"fn f() { !return false }"#,
182189
);
183-
check_assist_not_applicable(remove_parentheses, r#"fn f() { !$0(return) }"#);
184-
check_assist_not_applicable(remove_parentheses, r#"fn f() { !$0(break) }"#);
185-
check_assist_not_applicable(remove_parentheses, r#"fn f() { !$0(continue) }"#);
186190

187191
// Binary operators should still allow removal
188192
check_assist(
@@ -260,10 +264,37 @@ mod tests {
260264

261265
#[test]
262266
fn remove_parens_return_in_unary_not() {
267+
check_assist(
268+
remove_parentheses,
269+
r#"fn f() { cond && !$0(return) }"#,
270+
r#"fn f() { cond && !return }"#,
271+
);
272+
check_assist(
273+
remove_parentheses,
274+
r#"fn f() { cond && !$0(return false) }"#,
275+
r#"fn f() { cond && !return false }"#,
276+
);
277+
}
278+
279+
#[test]
280+
fn remove_parens_return_in_disjunction_with_closure_risk() {
281+
// `return` may only be blocked when it would form `return ||`
263282
cov_mark::check!(remove_parens_return_closure);
264283
check_assist_not_applicable(
265284
remove_parentheses,
266-
r#"fn main() { let _x = true && !$0(return) || true; }"#,
285+
r#"fn f() { let _x = true && $0(return) || true; }"#,
286+
);
287+
check_assist_not_applicable(
288+
remove_parentheses,
289+
r#"fn f() { let _x = true && !$0(return) || true; }"#,
290+
);
291+
check_assist_not_applicable(
292+
remove_parentheses,
293+
r#"fn f() { let _x = true && $0(return false) || true; }"#,
294+
);
295+
check_assist_not_applicable(
296+
remove_parentheses,
297+
r#"fn f() { let _x = true && !$0(return false) || true; }"#,
267298
);
268299
}
269300

0 commit comments

Comments
 (0)