Skip to content

internal: Migrate assists to the structured snippet API, part 3 #15260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jul 12, 2023
Merged

internal: Migrate assists to the structured snippet API, part 3 #15260

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
35f2e82
Don't show `add_missing_match_arms` when not needed
DropDemBits Jul 9, 2023
6ab2788
Migrate `add_missing_match_arms` to mutable ast
DropDemBits Jul 9, 2023
27444ed
Migrate `promote_local_to_const` to mutable ast
DropDemBits Jul 9, 2023
8c40fa3
Add `HasVisibilityEdit::set_visibility`
DropDemBits Jul 10, 2023
11debaf
Migrate `fix_visibility` to mutable ast
DropDemBits Jul 10, 2023
99abcdc
format let-else in `promote_local_to_const`
DropDemBits Jul 10, 2023
f9a144f
Hoist out `make::name_ref`
DropDemBits Jul 11, 2023
a9889a0
fix the rest of the nits
DropDemBits Jul 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 79 additions & 54 deletions crates/ide-assists/src/handlers/add_missing_match_arms.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@ use itertools::Itertools;
use syntax::ast::edit_in_place::Removable;
use syntax::ast::{self, make, AstNode, HasName, MatchArmList, MatchExpr, Pat};

use crate::{
utils::{self, render_snippet, Cursor},
AssistContext, AssistId, AssistKind, Assists,
};
use crate::{utils, AssistContext, AssistId, AssistKind, Assists};

// Assist: add_missing_match_arms
//
Expand Down Expand Up @@ -75,14 +72,18 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>)
.collect();

let module = ctx.sema.scope(expr.syntax())?.module();
let (mut missing_pats, is_non_exhaustive): (
let (mut missing_pats, is_non_exhaustive, has_hidden_variants): (
Peekable<Box<dyn Iterator<Item = (ast::Pat, bool)>>>,
bool,
bool,
) = if let Some(enum_def) = resolve_enum_def(&ctx.sema, &expr) {
let is_non_exhaustive = enum_def.is_non_exhaustive(ctx.db(), module.krate());

let variants = enum_def.variants(ctx.db());

let has_hidden_variants =
variants.iter().any(|variant| variant.should_be_hidden(ctx.db(), module.krate()));

let missing_pats = variants
.into_iter()
.filter_map(|variant| {
Expand All @@ -101,7 +102,7 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>)
} else {
Box::new(missing_pats)
};
(missing_pats.peekable(), is_non_exhaustive)
(missing_pats.peekable(), is_non_exhaustive, has_hidden_variants)
} else if let Some(enum_defs) = resolve_tuple_of_enum_def(&ctx.sema, &expr) {
let is_non_exhaustive =
enum_defs.iter().any(|enum_def| enum_def.is_non_exhaustive(ctx.db(), module.krate()));
Expand All @@ -124,6 +125,12 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>)
if n_arms > 256 {
return None;
}

let has_hidden_variants = variants_of_enums
.iter()
.flatten()
.any(|variant| variant.should_be_hidden(ctx.db(), module.krate()));

let missing_pats = variants_of_enums
.into_iter()
.multi_cartesian_product()
Expand All @@ -139,7 +146,11 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>)
(ast::Pat::from(make::tuple_pat(patterns)), is_hidden)
})
.filter(|(variant_pat, _)| is_variant_missing(&top_lvl_pats, variant_pat));
((Box::new(missing_pats) as Box<dyn Iterator<Item = _>>).peekable(), is_non_exhaustive)
(
(Box::new(missing_pats) as Box<dyn Iterator<Item = _>>).peekable(),
is_non_exhaustive,
has_hidden_variants,
)
} else if let Some((enum_def, len)) = resolve_array_of_enum_def(&ctx.sema, &expr) {
let is_non_exhaustive = enum_def.is_non_exhaustive(ctx.db(), module.krate());
let variants = enum_def.variants(ctx.db());
Expand All @@ -148,6 +159,9 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>)
return None;
}

let has_hidden_variants =
variants.iter().any(|variant| variant.should_be_hidden(ctx.db(), module.krate()));

let variants_of_enums = vec![variants; len];

let missing_pats = variants_of_enums
Expand All @@ -164,28 +178,42 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>)
(ast::Pat::from(make::slice_pat(patterns)), is_hidden)
})
.filter(|(variant_pat, _)| is_variant_missing(&top_lvl_pats, variant_pat));
((Box::new(missing_pats) as Box<dyn Iterator<Item = _>>).peekable(), is_non_exhaustive)
(
(Box::new(missing_pats) as Box<dyn Iterator<Item = _>>).peekable(),
is_non_exhaustive,
has_hidden_variants,
)
} else {
return None;
};

let mut needs_catch_all_arm = is_non_exhaustive && !has_catch_all_arm;

if !needs_catch_all_arm && missing_pats.peek().is_none() {
if !needs_catch_all_arm
&& ((has_hidden_variants && has_catch_all_arm) || missing_pats.peek().is_none())
{
return None;
}

acc.add(
AssistId("add_missing_match_arms", AssistKind::QuickFix),
"Fill match arms",
target_range,
|builder| {
|edit| {
let new_match_arm_list = match_arm_list.clone_for_update();

// having any hidden variants means that we need a catch-all arm
needs_catch_all_arm |= has_hidden_variants;

let missing_arms = missing_pats
.map(|(pat, hidden)| {
(make::match_arm(iter::once(pat), None, make::ext::expr_todo()), hidden)
.filter(|(_, hidden)| {
// filter out hidden patterns because they're handled by the catch-all arm
!hidden
})
.map(|(it, hidden)| (it.clone_for_update(), hidden));
.map(|(pat, _)| {
make::match_arm(iter::once(pat), None, make::ext::expr_todo())
.clone_for_update()
});

let catch_all_arm = new_match_arm_list
.arms()
Expand All @@ -204,15 +232,13 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>)
cov_mark::hit!(add_missing_match_arms_empty_expr);
}
}

let mut first_new_arm = None;
for (arm, hidden) in missing_arms {
if hidden {
needs_catch_all_arm = !has_catch_all_arm;
} else {
first_new_arm.get_or_insert_with(|| arm.clone());
new_match_arm_list.add_arm(arm);
}
for arm in missing_arms {
first_new_arm.get_or_insert_with(|| arm.clone());
new_match_arm_list.add_arm(arm);
}

if needs_catch_all_arm && !has_catch_all_arm {
cov_mark::hit!(added_wildcard_pattern);
let arm = make::match_arm(
Expand All @@ -225,24 +251,39 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>)
new_match_arm_list.add_arm(arm);
}

let old_range = ctx.sema.original_range(match_arm_list.syntax()).range;
match (first_new_arm, ctx.config.snippet_cap) {
(Some(first_new_arm), Some(cap)) => {
let extend_lifetime;
let cursor =
match first_new_arm.syntax().descendants().find_map(ast::WildcardPat::cast)
{
Some(it) => {
extend_lifetime = it.syntax().clone();
Cursor::Replace(&extend_lifetime)
}
None => Cursor::Before(first_new_arm.syntax()),
};
let snippet = render_snippet(cap, new_match_arm_list.syntax(), cursor);
builder.replace_snippet(cap, old_range, snippet);
if let (Some(first_new_arm), Some(cap)) = (first_new_arm, ctx.config.snippet_cap) {
match first_new_arm.syntax().descendants().find_map(ast::WildcardPat::cast) {
Some(it) => edit.add_placeholder_snippet(cap, it),
None => edit.add_tabstop_before(cap, first_new_arm),
}
_ => builder.replace(old_range, new_match_arm_list.to_string()),
}

// FIXME: Hack for mutable syntax trees not having great support for macros
// Just replace the element that the original range came from
let old_place = {
// Find the original element
let old_file_range = ctx.sema.original_range(match_arm_list.syntax());
let file = ctx.sema.parse(old_file_range.file_id);
let old_place = file.syntax().covering_element(old_file_range.range);

// Make `old_place` mut
match old_place {
syntax::SyntaxElement::Node(it) => {
syntax::SyntaxElement::from(edit.make_syntax_mut(it))
}
syntax::SyntaxElement::Token(it) => {
// Don't have a way to make tokens mut, so instead make the parent mut
// and find the token again
let parent = edit.make_syntax_mut(it.parent().unwrap());
let mut_token =
parent.covering_element(it.text_range()).into_token().unwrap();

syntax::SyntaxElement::from(mut_token)
}
}
};

syntax::ted::replace(old_place, new_match_arm_list.syntax());
},
)
}
Expand Down Expand Up @@ -1621,10 +1662,9 @@ pub enum E { #[doc(hidden)] A, }
);
}

// FIXME: I don't think the assist should be applicable in this case
#[test]
fn does_not_fill_wildcard_with_wildcard() {
check_assist(
check_assist_not_applicable(
add_missing_match_arms,
r#"
//- /main.rs crate:main deps:e
Expand All @@ -1635,13 +1675,6 @@ fn foo(t: ::e::E) {
}
//- /e.rs crate:e
pub enum E { #[doc(hidden)] A, }
"#,
r#"
fn foo(t: ::e::E) {
match t {
_ => todo!(),
}
}
"#,
);
}
Expand Down Expand Up @@ -1777,7 +1810,7 @@ fn foo(t: ::e::E, b: bool) {

#[test]
fn does_not_fill_wildcard_with_partial_wildcard_and_wildcard() {
check_assist(
check_assist_not_applicable(
add_missing_match_arms,
r#"
//- /main.rs crate:main deps:e
Expand All @@ -1789,14 +1822,6 @@ fn foo(t: ::e::E, b: bool) {
}
//- /e.rs crate:e
pub enum E { #[doc(hidden)] A, }"#,
r#"
fn foo(t: ::e::E, b: bool) {
match t {
_ if b => todo!(),
_ => todo!(),
}
}
"#,
);
}

Expand Down
Loading