Skip to content

Commit 86ca176

Browse files
bors[bot]jDomantas
andauthored
Merge #8975
8975: Use todo!() as placeholder body for generated match arms r=matklad a=jDomantas `todo!()` seems to be a better fit for this than `{}`. Seeing that this assist predates stabilization of `todo` my guess is that simply no one bothered to change it yet. Also fixed the issue where if the last arm was not block-like, rust-analyzer would not add a comma after it and would generate invalid code. Co-authored-by: Domantas Jadenkus <[email protected]>
2 parents f5f24a9 + 3641abc commit 86ca176

File tree

4 files changed

+112
-62
lines changed

4 files changed

+112
-62
lines changed

crates/ide_assists/src/handlers/fill_match_arms.rs

Lines changed: 94 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ use crate::{
3131
//
3232
// fn handle(action: Action) {
3333
// match action {
34-
// $0Action::Move { distance } => {}
35-
// Action::Stop => {}
34+
// $0Action::Move { distance } => todo!(),
35+
// Action::Stop => todo!(),
3636
// }
3737
// }
3838
// ```
@@ -129,7 +129,7 @@ pub(crate) fn fill_match_arms(acc: &mut Assists, ctx: &AssistContext) -> Option<
129129
|builder| {
130130
let new_match_arm_list = match_arm_list.clone_for_update();
131131
let missing_arms = missing_pats
132-
.map(|pat| make::match_arm(iter::once(pat), make::expr_empty_block()))
132+
.map(|pat| make::match_arm(iter::once(pat), make::ext::expr_todo()))
133133
.map(|it| it.clone_for_update());
134134

135135
let catch_all_arm = new_match_arm_list
@@ -350,8 +350,8 @@ fn foo(a: bool) {
350350
r#"
351351
fn foo(a: bool) {
352352
match a {
353-
$0true => {}
354-
false => {}
353+
$0true => todo!(),
354+
false => todo!(),
355355
}
356356
}
357357
"#,
@@ -373,7 +373,7 @@ fn foo(a: bool) {
373373
fn foo(a: bool) {
374374
match a {
375375
true => {}
376-
$0false => {}
376+
$0false => todo!(),
377377
}
378378
}
379379
"#,
@@ -410,10 +410,10 @@ fn foo(a: bool) {
410410
r#"
411411
fn foo(a: bool) {
412412
match (a, a) {
413-
$0(true, true) => {}
414-
(true, false) => {}
415-
(false, true) => {}
416-
(false, false) => {}
413+
$0(true, true) => todo!(),
414+
(true, false) => todo!(),
415+
(false, true) => todo!(),
416+
(false, false) => todo!(),
417417
}
418418
}
419419
"#,
@@ -435,9 +435,9 @@ fn foo(a: bool) {
435435
fn foo(a: bool) {
436436
match (a, a) {
437437
(false, true) => {}
438-
$0(true, true) => {}
439-
(true, false) => {}
440-
(false, false) => {}
438+
$0(true, true) => todo!(),
439+
(true, false) => todo!(),
440+
(false, false) => todo!(),
441441
}
442442
}
443443
"#,
@@ -471,7 +471,7 @@ fn main() {
471471
match A::As {
472472
A::Bs { x, y: Some(_) } => {}
473473
A::Cs(_, Some(_)) => {}
474-
$0A::As => {}
474+
$0A::As => todo!(),
475475
}
476476
}
477477
"#,
@@ -499,7 +499,7 @@ use Option::*;
499499
fn main() {
500500
match None {
501501
None => {}
502-
Some(${0:_}) => {}
502+
Some(${0:_}) => todo!(),
503503
}
504504
}
505505
"#,
@@ -523,7 +523,7 @@ enum A { As, Bs, Cs(Option<i32>) }
523523
fn main() {
524524
match A::As {
525525
A::Cs(_) | A::Bs => {}
526-
$0A::As => {}
526+
$0A::As => todo!(),
527527
}
528528
}
529529
"#,
@@ -553,8 +553,8 @@ fn main() {
553553
A::Bs if 0 < 1 => {}
554554
A::Ds(_value) => { let x = 1; }
555555
A::Es(B::Xs) => (),
556-
$0A::As => {}
557-
A::Cs => {}
556+
$0A::As => todo!(),
557+
A::Cs => todo!(),
558558
}
559559
}
560560
"#,
@@ -580,7 +580,7 @@ fn main() {
580580
match A::As {
581581
A::As(_) => {}
582582
a @ A::Bs(_) => {}
583-
A::Cs(${0:_}) => {}
583+
A::Cs(${0:_}) => todo!(),
584584
}
585585
}
586586
"#,
@@ -605,11 +605,11 @@ enum A { As, Bs, Cs(String), Ds(String, String), Es { x: usize, y: usize } }
605605
fn main() {
606606
let a = A::As;
607607
match a {
608-
$0A::As => {}
609-
A::Bs => {}
610-
A::Cs(_) => {}
611-
A::Ds(_, _) => {}
612-
A::Es { x, y } => {}
608+
$0A::As => todo!(),
609+
A::Bs => todo!(),
610+
A::Cs(_) => todo!(),
611+
A::Ds(_, _) => todo!(),
612+
A::Es { x, y } => todo!(),
613613
}
614614
}
615615
"#,
@@ -638,10 +638,10 @@ fn main() {
638638
let a = A::One;
639639
let b = B::One;
640640
match (a, b) {
641-
$0(A::One, B::One) => {}
642-
(A::One, B::Two) => {}
643-
(A::Two, B::One) => {}
644-
(A::Two, B::Two) => {}
641+
$0(A::One, B::One) => todo!(),
642+
(A::One, B::Two) => todo!(),
643+
(A::Two, B::One) => todo!(),
644+
(A::Two, B::Two) => todo!(),
645645
}
646646
}
647647
"#,
@@ -670,10 +670,10 @@ fn main() {
670670
let a = A::One;
671671
let b = B::One;
672672
match (&a, &b) {
673-
$0(A::One, B::One) => {}
674-
(A::One, B::Two) => {}
675-
(A::Two, B::One) => {}
676-
(A::Two, B::Two) => {}
673+
$0(A::One, B::One) => todo!(),
674+
(A::One, B::Two) => todo!(),
675+
(A::Two, B::One) => todo!(),
676+
(A::Two, B::Two) => todo!(),
677677
}
678678
}
679679
"#,
@@ -705,9 +705,9 @@ fn main() {
705705
let b = B::One;
706706
match (a, b) {
707707
(A::Two, B::One) => {}
708-
$0(A::One, B::One) => {}
709-
(A::One, B::Two) => {}
710-
(A::Two, B::Two) => {}
708+
$0(A::One, B::One) => todo!(),
709+
(A::One, B::Two) => todo!(),
710+
(A::Two, B::Two) => todo!(),
711711
}
712712
}
713713
"#,
@@ -736,7 +736,7 @@ fn main() {
736736
match (a, b) {
737737
(Some(_), _) => {}
738738
(None, Some(_)) => {}
739-
$0(None, None) => {}
739+
$0(None, None) => todo!(),
740740
}
741741
}
742742
"#,
@@ -801,8 +801,8 @@ enum A { One, Two }
801801
fn main() {
802802
let a = A::One;
803803
match (a, ) {
804-
$0(A::One,) => {}
805-
(A::Two,) => {}
804+
$0(A::One,) => todo!(),
805+
(A::Two,) => todo!(),
806806
}
807807
}
808808
"#,
@@ -826,7 +826,7 @@ enum A { As }
826826
827827
fn foo(a: &A) {
828828
match a {
829-
$0A::As => {}
829+
$0A::As => todo!(),
830830
}
831831
}
832832
"#,
@@ -851,7 +851,7 @@ enum A {
851851
852852
fn foo(a: &mut A) {
853853
match a {
854-
$0A::Es { x, y } => {}
854+
$0A::Es { x, y } => todo!(),
855855
}
856856
}
857857
"#,
@@ -891,8 +891,8 @@ enum E { X, Y }
891891
892892
fn main() {
893893
match E::X {
894-
$0E::X => {}
895-
E::Y => {}
894+
$0E::X => todo!(),
895+
E::Y => todo!(),
896896
}
897897
}
898898
"#,
@@ -919,8 +919,8 @@ use foo::E::X;
919919
920920
fn main() {
921921
match X {
922-
$0X => {}
923-
foo::E::Y => {}
922+
$0X => todo!(),
923+
foo::E::Y => todo!(),
924924
}
925925
}
926926
"#,
@@ -947,7 +947,7 @@ fn foo(a: A) {
947947
match a {
948948
// foo bar baz
949949
A::One => {}
950-
$0A::Two => {}
950+
$0A::Two => todo!(),
951951
// This is where the rest should be
952952
}
953953
}
@@ -971,8 +971,8 @@ fn foo(a: A) {
971971
enum A { One, Two }
972972
fn foo(a: A) {
973973
match a {
974-
$0A::One => {}
975-
A::Two => {}
974+
$0A::One => todo!(),
975+
A::Two => todo!(),
976976
// foo bar baz
977977
}
978978
}
@@ -996,8 +996,8 @@ fn foo(a: A) {
996996
enum A { One, Two, }
997997
fn foo(a: A) {
998998
match a {
999-
$0A::One => {}
1000-
A::Two => {}
999+
$0A::One => todo!(),
1000+
A::Two => todo!(),
10011001
}
10021002
}
10031003
"#,
@@ -1021,8 +1021,8 @@ fn foo(opt: Option<i32>) {
10211021
r#"
10221022
fn foo(opt: Option<i32>) {
10231023
match opt {
1024-
Some(${0:_}) => {}
1025-
None => {}
1024+
Some(${0:_}) => todo!(),
1025+
None => todo!(),
10261026
}
10271027
}
10281028
"#,
@@ -1054,9 +1054,9 @@ enum Test {
10541054
10551055
fn foo(t: Test) {
10561056
m!(match t {
1057-
$0Test::A => {}
1058-
Test::B => {}
1059-
Test::C => {}
1057+
$0Test::A => todo!(),
1058+
Test::B => todo!(),
1059+
Test::C => todo!(),
10601060
});
10611061
}"#,
10621062
);
@@ -1076,4 +1076,44 @@ fn foo(tuple: (A, A)) {
10761076
"#,
10771077
);
10781078
}
1079+
1080+
#[test]
1081+
fn adds_comma_before_new_arms() {
1082+
check_assist(
1083+
fill_match_arms,
1084+
r#"
1085+
fn foo(t: bool) {
1086+
match $0t {
1087+
true => 1 + 2
1088+
}
1089+
}"#,
1090+
r#"
1091+
fn foo(t: bool) {
1092+
match t {
1093+
true => 1 + 2,
1094+
$0false => todo!(),
1095+
}
1096+
}"#,
1097+
);
1098+
}
1099+
1100+
#[test]
1101+
fn does_not_add_extra_comma() {
1102+
check_assist(
1103+
fill_match_arms,
1104+
r#"
1105+
fn foo(t: bool) {
1106+
match $0t {
1107+
true => 1 + 2,
1108+
}
1109+
}"#,
1110+
r#"
1111+
fn foo(t: bool) {
1112+
match t {
1113+
true => 1 + 2,
1114+
$0false => todo!(),
1115+
}
1116+
}"#,
1117+
);
1118+
}
10791119
}

crates/ide_assists/src/tests/generated.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -455,8 +455,8 @@ enum Action { Move { distance: u32 }, Stop }
455455
456456
fn handle(action: Action) {
457457
match action {
458-
$0Action::Move { distance } => {}
459-
Action::Stop => {}
458+
$0Action::Move { distance } => todo!(),
459+
Action::Stop => todo!(),
460460
}
461461
}
462462
"#####,

crates/syntax/src/ast/edit_in_place.rs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -356,25 +356,34 @@ impl ast::MatchArm {
356356
impl ast::MatchArmList {
357357
pub fn add_arm(&self, arm: ast::MatchArm) {
358358
normalize_ws_between_braces(self.syntax());
359+
let mut elements = Vec::new();
359360
let position = match self.arms().last() {
360361
Some(last_arm) => {
361-
let curly = last_arm
362+
let comma = last_arm
362363
.syntax()
363364
.siblings_with_tokens(Direction::Next)
364365
.find(|it| it.kind() == T![,]);
365-
Position::after(curly.unwrap_or_else(|| last_arm.syntax().clone().into()))
366+
if needs_comma(&last_arm) && comma.is_none() {
367+
elements.push(make::token(SyntaxKind::COMMA).into());
368+
}
369+
Position::after(comma.unwrap_or_else(|| last_arm.syntax().clone().into()))
366370
}
367371
None => match self.l_curly_token() {
368372
Some(it) => Position::after(it),
369373
None => Position::last_child_of(self.syntax()),
370374
},
371375
};
372376
let indent = IndentLevel::from_node(self.syntax()) + 1;
373-
let elements = vec![
374-
make::tokens::whitespace(&format!("\n{}", indent)).into(),
375-
arm.syntax().clone().into(),
376-
];
377+
elements.push(make::tokens::whitespace(&format!("\n{}", indent)).into());
378+
elements.push(arm.syntax().clone().into());
379+
if needs_comma(&arm) {
380+
elements.push(make::token(SyntaxKind::COMMA).into());
381+
}
377382
ted::insert_all(position, elements);
383+
384+
fn needs_comma(arm: &ast::MatchArm) -> bool {
385+
arm.expr().map_or(false, |e| !e.is_block_like())
386+
}
378387
}
379388
}
380389

xtask/src/tidy.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,7 @@ fn check_todo(path: &Path, text: &str) {
275275
// Some of our assists generate `todo!()`.
276276
"handlers/add_turbo_fish.rs",
277277
"handlers/generate_function.rs",
278+
"handlers/fill_match_arms.rs",
278279
// To support generating `todo!()` in assists, we have `expr_todo()` in
279280
// `ast::make`.
280281
"ast/make.rs",

0 commit comments

Comments
 (0)