Skip to content

Commit 2c9d993

Browse files
committed
fix: Improve label on add_missing_match_arms assist
"Fill match arms" is an extremely helpful assist, but the name is pretty opaque to new users (at least it was to me). We actually renamed the file in #10299 from `fill_match_arms.rs` to `add_missing_match_arms.rs` but the label hasn't changed from its original text in #733. Instead, show "Add missing match arms" if there are multiple missing match arms, and show "Add missing match arm `Foo::Bar`" when there's only a single missing match arm. Partially generated by Claude Opus.
1 parent 63b3eff commit 2c9d993

2 files changed

Lines changed: 80 additions & 6 deletions

File tree

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

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -223,9 +223,17 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>)
223223
return None;
224224
}
225225

226+
let first = missing_pats.next();
227+
let label = match (&first, missing_pats.peek()) {
228+
(Some((pat, false)), None) => format!("Add missing match arm `{pat}`"),
229+
(Some((_, true)), None) => "Add missing match arm `_`".to_owned(),
230+
_ => "Add missing match arms".to_owned(),
231+
};
232+
let missing_pats = first.into_iter().chain(missing_pats);
233+
226234
acc.add(
227235
AssistId::quick_fix("add_missing_match_arms"),
228-
"Fill match arms",
236+
label,
229237
ctx.sema.original_range(match_expr.syntax()).range,
230238
|builder| {
231239
// having any hidden variants means that we need a catch-all arm
@@ -635,7 +643,7 @@ mod tests {
635643
use crate::AssistConfig;
636644
use crate::tests::{
637645
TEST_CONFIG, check_assist, check_assist_not_applicable, check_assist_target,
638-
check_assist_unresolved, check_assist_with_config,
646+
check_assist_unresolved, check_assist_with_config, check_assist_with_label,
639647
};
640648

641649
use super::add_missing_match_arms;
@@ -1828,8 +1836,10 @@ fn foo(t: Test) {
18281836

18291837
#[test]
18301838
fn lazy_computation() {
1831-
// Computing a single missing arm is enough to determine applicability of the assist.
1832-
cov_mark::check_count!(add_missing_match_arms_lazy_computation, 1);
1839+
// We compute the first two arms to see if we need a plural in
1840+
// the label, but otherwise we're lazy.
1841+
cov_mark::check_count!(add_missing_match_arms_lazy_computation, 2);
1842+
18331843
check_assist_unresolved(
18341844
add_missing_match_arms,
18351845
r#"
@@ -1841,6 +1851,54 @@ fn foo(tuple: (A, A)) {
18411851
);
18421852
}
18431853

1854+
#[test]
1855+
fn label_single_missing_arm() {
1856+
check_assist_with_label(
1857+
add_missing_match_arms,
1858+
r#"
1859+
enum A { One, Two }
1860+
fn foo(a: A) {
1861+
match $0a {
1862+
A::One => {}
1863+
}
1864+
}
1865+
"#,
1866+
"Add missing match arm `A::Two`",
1867+
);
1868+
}
1869+
1870+
#[test]
1871+
fn label_multiple_missing_arms() {
1872+
check_assist_with_label(
1873+
add_missing_match_arms,
1874+
r#"
1875+
enum A { One, Two, Three }
1876+
fn foo(a: A) {
1877+
match $0a {}
1878+
}
1879+
"#,
1880+
"Add missing match arms",
1881+
);
1882+
}
1883+
1884+
#[test]
1885+
fn label_catch_all_only() {
1886+
check_assist_with_label(
1887+
add_missing_match_arms,
1888+
r#"
1889+
//- /main.rs crate:main deps:e
1890+
fn foo(t: ::e::E) {
1891+
match $0t {
1892+
e::E::A => {}
1893+
}
1894+
}
1895+
//- /e.rs crate:e
1896+
pub enum E { A, #[doc(hidden)] B, }
1897+
"#,
1898+
"Add missing match arm `_`",
1899+
);
1900+
}
1901+
18441902
#[test]
18451903
fn adds_comma_before_new_arms() {
18461904
check_assist(

crates/ide-assists/src/tests.rs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,15 @@ pub(crate) fn check_assist_target(
207207
check(assist, ra_fixture, ExpectedResult::Target(target), None);
208208
}
209209

210+
#[track_caller]
211+
pub(crate) fn check_assist_with_label(
212+
assist: Handler,
213+
#[rust_analyzer::rust_fixture] ra_fixture: &str,
214+
label: &str,
215+
) {
216+
check(assist, ra_fixture, ExpectedResult::Label(label), None);
217+
}
218+
210219
#[track_caller]
211220
pub(crate) fn check_assist_not_applicable(
212221
assist: Handler,
@@ -307,6 +316,7 @@ enum ExpectedResult<'a> {
307316
Unresolved,
308317
After(&'a str),
309318
Target(&'a str),
319+
Label(&'a str),
310320
}
311321

312322
#[track_caller]
@@ -335,7 +345,7 @@ fn check_with_config(
335345

336346
let ctx = AssistContext::new(sema, &config, frange);
337347
let resolve = match expected {
338-
ExpectedResult::Unresolved => AssistResolveStrategy::None,
348+
ExpectedResult::Unresolved | ExpectedResult::Label(_) => AssistResolveStrategy::None,
339349
_ => AssistResolveStrategy::All,
340350
};
341351
let mut acc = Assists::new(&ctx, resolve);
@@ -404,14 +414,20 @@ fn check_with_config(
404414
let range = assist.target;
405415
assert_eq_text!(&text_without_caret[range], target);
406416
}
417+
(Some(assist), ExpectedResult::Label(label)) => {
418+
assert_eq!(assist.label.to_string(), label);
419+
}
407420
(Some(assist), ExpectedResult::Unresolved) => assert!(
408421
assist.source_change.is_none(),
409422
"unresolved assist should not contain source changes"
410423
),
411424
(Some(_), ExpectedResult::NotApplicable) => panic!("assist should not be applicable!"),
412425
(
413426
None,
414-
ExpectedResult::After(_) | ExpectedResult::Target(_) | ExpectedResult::Unresolved,
427+
ExpectedResult::After(_)
428+
| ExpectedResult::Target(_)
429+
| ExpectedResult::Label(_)
430+
| ExpectedResult::Unresolved,
415431
) => {
416432
panic!("code action is not applicable")
417433
}

0 commit comments

Comments
 (0)