Skip to content

Commit 927d56a

Browse files
committed
Auto merge of rust-lang#13764 - WaffleLapkin:badassexprs, r=Veykril
fix: Correctly check for parentheses redundancy in `remove_parentheses` assist This is quite a bunch of code and some hacks, but I _think_ this time it's correct. I've added a lot of tests, most of which fail with the assist impl from rust-lang#13733 :')
2 parents 5c8f00f + babd4c7 commit 927d56a

File tree

2 files changed

+440
-97
lines changed

2 files changed

+440
-97
lines changed

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

+133-3
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,8 @@ pub(crate) fn remove_parentheses(acc: &mut Assists, ctx: &AssistContext<'_>) ->
2929

3030
let expr = parens.expr()?;
3131

32-
let parent = ast::Expr::cast(parens.syntax().parent()?);
33-
let is_ok_to_remove = expr.precedence() >= parent.as_ref().and_then(ast::Expr::precedence);
34-
if !is_ok_to_remove {
32+
let parent = parens.syntax().parent()?;
33+
if expr.needs_parens_in(parent) {
3534
return None;
3635
}
3736

@@ -58,6 +57,31 @@ mod tests {
5857
check_assist(remove_parentheses, r#"fn f() { (2$0) + 2; }"#, r#"fn f() { 2 + 2; }"#);
5958
}
6059

60+
#[test]
61+
fn remove_parens_closure() {
62+
check_assist(remove_parentheses, r#"fn f() { &$0(|| 42) }"#, r#"fn f() { &|| 42 }"#);
63+
64+
check_assist_not_applicable(remove_parentheses, r#"fn f() { $0(|| 42).f() }"#);
65+
}
66+
67+
#[test]
68+
fn remove_parens_if_let_chains() {
69+
check_assist_not_applicable(
70+
remove_parentheses,
71+
r#"fn f() { if let true = $0(true && true) {} }"#,
72+
);
73+
}
74+
75+
#[test]
76+
fn remove_parens_associativity() {
77+
check_assist(
78+
remove_parentheses,
79+
r#"fn f() { $0(2 + 2) + 2; }"#,
80+
r#"fn f() { 2 + 2 + 2; }"#,
81+
);
82+
check_assist_not_applicable(remove_parentheses, r#"fn f() { 2 + $0(2 + 2); }"#);
83+
}
84+
6185
#[test]
6286
fn remove_parens_precedence() {
6387
check_assist(
@@ -88,4 +112,110 @@ mod tests {
88112
check_assist_not_applicable(remove_parentheses, r#"fn f() { (2 +$0 2) }"#);
89113
check_assist_not_applicable(remove_parentheses, r#"fn f() {$0 (2 + 2) }"#);
90114
}
115+
116+
#[test]
117+
fn remove_parens_doesnt_apply_when_expr_would_be_turned_into_a_statement() {
118+
check_assist_not_applicable(remove_parentheses, r#"fn x() -> u8 { $0({ 0 } + 1) }"#);
119+
check_assist_not_applicable(
120+
remove_parentheses,
121+
r#"fn x() -> u8 { $0(if true { 0 } else { 1 } + 1) }"#,
122+
);
123+
check_assist_not_applicable(remove_parentheses, r#"fn x() -> u8 { $0(loop {} + 1) }"#);
124+
}
125+
126+
#[test]
127+
fn remove_parens_doesnt_apply_weird_syntax_and_adge_cases() {
128+
// removing `()` would break code because {} would be counted as the loop/if body
129+
check_assist_not_applicable(remove_parentheses, r#"fn f() { for _ in $0(0..{3}) {} }"#);
130+
check_assist_not_applicable(remove_parentheses, r#"fn f() { for _ in $0(S {}) {} }"#);
131+
check_assist_not_applicable(remove_parentheses, r#"fn f() { if $0(S {} == 2) {} }"#);
132+
check_assist_not_applicable(remove_parentheses, r#"fn f() { if $0(return) {} }"#);
133+
}
134+
135+
#[test]
136+
fn remove_parens_return_with_value_followed_by_block() {
137+
check_assist(
138+
remove_parentheses,
139+
r#"fn f() { if $0(return ()) {} }"#,
140+
r#"fn f() { if return () {} }"#,
141+
);
142+
}
143+
144+
#[test]
145+
fn remove_exprs_let_else_restrictions() {
146+
// `}` is not allowed before `else` here
147+
check_assist_not_applicable(
148+
remove_parentheses,
149+
r#"fn f() { let _ = $0(S{}) else { return }; }"#,
150+
);
151+
152+
// logic operators can't directly appear in the let-else
153+
check_assist_not_applicable(
154+
remove_parentheses,
155+
r#"fn f() { let _ = $0(false || false) else { return }; }"#,
156+
);
157+
check_assist_not_applicable(
158+
remove_parentheses,
159+
r#"fn f() { let _ = $0(true && true) else { return }; }"#,
160+
);
161+
}
162+
163+
#[test]
164+
fn remove_parens_weird_places() {
165+
check_assist(
166+
remove_parentheses,
167+
r#"fn f() { match () { _=>$0(()) } }"#,
168+
r#"fn f() { match () { _=>() } }"#,
169+
);
170+
171+
check_assist(
172+
remove_parentheses,
173+
r#"fn x() -> u8 { { [$0({ 0 } + 1)] } }"#,
174+
r#"fn x() -> u8 { { [{ 0 } + 1] } }"#,
175+
);
176+
}
177+
178+
#[test]
179+
fn remove_parens_return_dot_f() {
180+
check_assist(
181+
remove_parentheses,
182+
r#"fn f() { $0(return).f() }"#,
183+
r#"fn f() { return.f() }"#,
184+
);
185+
}
186+
187+
#[test]
188+
fn remove_parens_prefix_then_return_something() {
189+
check_assist(
190+
remove_parentheses,
191+
r#"fn f() { &$0(return ()) }"#,
192+
r#"fn f() { &return () }"#,
193+
);
194+
}
195+
196+
#[test]
197+
fn remove_parens_double_paren_stmt() {
198+
check_assist(
199+
remove_parentheses,
200+
r#"fn x() -> u8 { $0(({ 0 } + 1)) }"#,
201+
r#"fn x() -> u8 { ({ 0 } + 1) }"#,
202+
);
203+
204+
check_assist(
205+
remove_parentheses,
206+
r#"fn x() -> u8 { (($0{ 0 } + 1)) }"#,
207+
r#"fn x() -> u8 { ({ 0 } + 1) }"#,
208+
);
209+
}
210+
211+
#[test]
212+
fn remove_parens_im_tired_of_naming_tests() {
213+
check_assist(
214+
remove_parentheses,
215+
r#"fn f() { 2 + $0(return 2) }"#,
216+
r#"fn f() { 2 + return 2 }"#,
217+
);
218+
219+
check_assist_not_applicable(remove_parentheses, r#"fn f() { $0(return 2) + 2 }"#);
220+
}
91221
}

0 commit comments

Comments
 (0)