Auto merge of #15260 - DropDemBits:structured-snippet-migrate-3, r=lowr

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

Continuing from #15231

Migrates the following assists:
- `add_missing_match_arms`
- `fix_visibility`
- `promote_local_to_const`

The `add_missing_match_arms` changes are best reviewed commit-by-commit since they're relatively big changes compared to the rest of the commits.

Author: bors

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

@@ -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
 //
@@ -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| {
@@ -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()));
@@ -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()
@@ -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());
@@ -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
@@ -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()
@@ -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(
@@ -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());
         },
     )
 }
@@ -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
@@ -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!(),
-    }
-}
 "#,
         );
     }
@@ -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
@@ -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!(),
-    }
-}
-"#,
         );
     }