Merge #8711

8711: Only resolve selected assist r=matklad a=SomeoneToIgnore

Part of https://github.com/rust-analyzer/rust-analyzer/issues/8700

Now resolves only the assist that was selected out of the list, while before the whole assist list was resolved despite a single popup selection.

Co-authored-by: Kirill Bulatov <[email protected]>

3 files changed, 326 insertions, 14 deletions

diff --git a/crates/ide_assists/src/assist_context.rs b/crates/ide_assists/src/assist_context.rs
index 4b0bba2ab..112939948 100644
--- a/crates/ide_assists/src/assist_context.rs
+++ b/crates/ide_assists/src/assist_context.rs
@@ -19,7 +19,9 @@ use syntax::{
 };
 use text_edit::{TextEdit, TextEditBuilder};
 
-use crate::{assist_config::AssistConfig, Assist, AssistId, AssistKind, GroupLabel};
+use crate::{
+    assist_config::AssistConfig, Assist, AssistId, AssistKind, AssistResolveStrategy, GroupLabel,
+};
 
 /// `AssistContext` allows to apply an assist or check if it could be applied.
 ///
@@ -105,14 +107,14 @@ impl<'a> AssistContext<'a> {
 }
 
 pub(crate) struct Assists {
-    resolve: bool,
     file: FileId,
+    resolve: AssistResolveStrategy,
     buf: Vec<Assist>,
     allowed: Option<Vec<AssistKind>>,
 }
 
 impl Assists {
-    pub(crate) fn new(ctx: &AssistContext, resolve: bool) -> Assists {
+    pub(crate) fn new(ctx: &AssistContext, resolve: AssistResolveStrategy) -> Assists {
         Assists {
             resolve,
             file: ctx.frange.file_id,
@@ -158,7 +160,7 @@ impl Assists {
     }
 
     fn add_impl(&mut self, mut assist: Assist, f: impl FnOnce(&mut AssistBuilder)) -> Option<()> {
-        let source_change = if self.resolve {
+        let source_change = if self.resolve.should_resolve(&assist.id) {
             let mut builder = AssistBuilder::new(self.file);
             f(&mut builder);
             Some(builder.finish())
diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs
index 88ae5c9a9..2e0c58504 100644
--- a/crates/ide_assists/src/lib.rs
+++ b/crates/ide_assists/src/lib.rs
@@ -17,6 +17,8 @@ mod tests;
 pub mod utils;
 pub mod ast_transform;
 
+use std::str::FromStr;
+
 use hir::Semantics;
 use ide_db::base_db::FileRange;
 use ide_db::{label::Label, source_change::SourceChange, RootDatabase};
@@ -56,6 +58,35 @@ impl AssistKind {
             _ => return false,
         }
     }
+
+    pub fn name(&self) -> &str {
+        match self {
+            AssistKind::None => "None",
+            AssistKind::QuickFix => "QuickFix",
+            AssistKind::Generate => "Generate",
+            AssistKind::Refactor => "Refactor",
+            AssistKind::RefactorExtract => "RefactorExtract",
+            AssistKind::RefactorInline => "RefactorInline",
+            AssistKind::RefactorRewrite => "RefactorRewrite",
+        }
+    }
+}
+
+impl FromStr for AssistKind {
+    type Err = String;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match s {
+            "None" => Ok(AssistKind::None),
+            "QuickFix" => Ok(AssistKind::QuickFix),
+            "Generate" => Ok(AssistKind::Generate),
+            "Refactor" => Ok(AssistKind::Refactor),
+            "RefactorExtract" => Ok(AssistKind::RefactorExtract),
+            "RefactorInline" => Ok(AssistKind::RefactorInline),
+            "RefactorRewrite" => Ok(AssistKind::RefactorRewrite),
+            unknown => Err(format!("Unknown AssistKind: '{}'", unknown)),
+        }
+    }
 }
 
 /// Unique identifier of the assist, should not be shown to the user
@@ -63,6 +94,41 @@ impl AssistKind {
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub struct AssistId(pub &'static str, pub AssistKind);
 
+/// A way to control how many asssist to resolve during the assist resolution.
+/// When an assist is resolved, its edits are calculated that might be costly to always do by default.
+#[derive(Debug)]
+pub enum AssistResolveStrategy {
+    /// No assists should be resolved.
+    None,
+    /// All assists should be resolved.
+    All,
+    /// Only a certain assist should be resolved.
+    Single(SingleResolve),
+}
+
+/// Hold the [`AssistId`] data of a certain assist to resolve.
+/// The original id object cannot be used due to a `'static` lifetime
+/// and the requirement to construct this struct dynamically during the resolve handling.
+#[derive(Debug)]
+pub struct SingleResolve {
+    /// The id of the assist.
+    pub assist_id: String,
+    // The kind of the assist.
+    pub assist_kind: AssistKind,
+}
+
+impl AssistResolveStrategy {
+    pub fn should_resolve(&self, id: &AssistId) -> bool {
+        match self {
+            AssistResolveStrategy::None => false,
+            AssistResolveStrategy::All => true,
+            AssistResolveStrategy::Single(single_resolve) => {
+                single_resolve.assist_id == id.0 && single_resolve.assist_kind == id.1
+            }
+        }
+    }
+}
+
 #[derive(Clone, Debug)]
 pub struct GroupLabel(pub String);
 
@@ -91,7 +157,7 @@ impl Assist {
     pub fn get(
         db: &RootDatabase,
         config: &AssistConfig,
-        resolve: bool,
+        resolve: AssistResolveStrategy,
         range: FileRange,
     ) -> Vec<Assist> {
         let sema = Semantics::new(db);
diff --git a/crates/ide_assists/src/tests.rs b/crates/ide_assists/src/tests.rs
index 6f4f97361..9c2847998 100644
--- a/crates/ide_assists/src/tests.rs
+++ b/crates/ide_assists/src/tests.rs
@@ -12,7 +12,10 @@ use stdx::{format_to, trim_indent};
 use syntax::TextRange;
 use test_utils::{assert_eq_text, extract_offset};
 
-use crate::{handlers::Handler, Assist, AssistConfig, AssistContext, AssistKind, Assists};
+use crate::{
+    handlers::Handler, Assist, AssistConfig, AssistContext, AssistKind, AssistResolveStrategy,
+    Assists, SingleResolve,
+};
 
 pub(crate) const TEST_CONFIG: AssistConfig = AssistConfig {
     snippet_cap: SnippetCap::new(true),
@@ -65,14 +68,14 @@ fn check_doc_test(assist_id: &str, before: &str, after: &str) {
     let before = db.file_text(file_id).to_string();
     let frange = FileRange { file_id, range: selection.into() };
 
-    let assist = Assist::get(&db, &TEST_CONFIG, true, frange)
+    let assist = Assist::get(&db, &TEST_CONFIG, AssistResolveStrategy::All, frange)
         .into_iter()
         .find(|assist| assist.id.0 == assist_id)
         .unwrap_or_else(|| {
             panic!(
                 "\n\nAssist is not applicable: {}\nAvailable assists: {}",
                 assist_id,
-                Assist::get(&db, &TEST_CONFIG, false, frange)
+                Assist::get(&db, &TEST_CONFIG, AssistResolveStrategy::None, frange)
                     .into_iter()
                     .map(|assist| assist.id.0)
                     .collect::<Vec<_>>()
@@ -108,7 +111,7 @@ fn check(handler: Handler, before: &str, expected: ExpectedResult, assist_label:
     let sema = Semantics::new(&db);
     let config = TEST_CONFIG;
     let ctx = AssistContext::new(sema, &config, frange);
-    let mut acc = Assists::new(&ctx, true);
+    let mut acc = Assists::new(&ctx, AssistResolveStrategy::All);
     handler(&mut acc, &ctx);
     let mut res = acc.finish();
 
@@ -186,7 +189,7 @@ fn assist_order_field_struct() {
     let (before_cursor_pos, before) = extract_offset(before);
     let (db, file_id) = with_single_file(&before);
     let frange = FileRange { file_id, range: TextRange::empty(before_cursor_pos) };
-    let assists = Assist::get(&db, &TEST_CONFIG, false, frange);
+    let assists = Assist::get(&db, &TEST_CONFIG, AssistResolveStrategy::None, frange);
     let mut assists = assists.iter();
 
     assert_eq!(assists.next().expect("expected assist").label, "Change visibility to pub(crate)");
@@ -211,7 +214,7 @@ pub fn test_some_range(a: int) -> bool {
 "#,
     );
 
-    let assists = Assist::get(&db, &TEST_CONFIG, false, frange);
+    let assists = Assist::get(&db, &TEST_CONFIG, AssistResolveStrategy::None, frange);
     let expected = labels(&assists);
 
     expect![[r#"
@@ -240,7 +243,7 @@ pub fn test_some_range(a: int) -> bool {
         let mut cfg = TEST_CONFIG;
         cfg.allowed = Some(vec![AssistKind::Refactor]);
 
-        let assists = Assist::get(&db, &cfg, false, frange);
+        let assists = Assist::get(&db, &cfg, AssistResolveStrategy::None, frange);
         let expected = labels(&assists);
 
         expect![[r#"
@@ -255,7 +258,7 @@ pub fn test_some_range(a: int) -> bool {
     {
         let mut cfg = TEST_CONFIG;
         cfg.allowed = Some(vec![AssistKind::RefactorExtract]);
-        let assists = Assist::get(&db, &cfg, false, frange);
+        let assists = Assist::get(&db, &cfg, AssistResolveStrategy::None, frange);
         let expected = labels(&assists);
 
         expect![[r#"
@@ -268,9 +271,250 @@ pub fn test_some_range(a: int) -> bool {
     {
         let mut cfg = TEST_CONFIG;
         cfg.allowed = Some(vec![AssistKind::QuickFix]);
-        let assists = Assist::get(&db, &cfg, false, frange);
+        let assists = Assist::get(&db, &cfg, AssistResolveStrategy::None, frange);
         let expected = labels(&assists);
 
         expect![[r#""#]].assert_eq(&expected);
     }
 }
+
+#[test]
+fn various_resolve_strategies() {
+    let (db, frange) = RootDatabase::with_range(
+        r#"
+pub fn test_some_range(a: int) -> bool {
+    if let 2..6 = $05$0 {
+        true
+    } else {
+        false
+    }
+}
+"#,
+    );
+
+    let mut cfg = TEST_CONFIG;
+    cfg.allowed = Some(vec![AssistKind::RefactorExtract]);
+
+    {
+        let assists = Assist::get(&db, &cfg, AssistResolveStrategy::None, frange);
+        assert_eq!(2, assists.len());
+        let mut assists = assists.into_iter();
+
+        let extract_into_variable_assist = assists.next().unwrap();
+        expect![[r#"
+            Assist {
+                id: AssistId(
+                    "extract_variable",
+                    RefactorExtract,
+                ),
+                label: "Extract into variable",
+                group: None,
+                target: 59..60,
+                source_change: None,
+            }
+        "#]]
+        .assert_debug_eq(&extract_into_variable_assist);
+
+        let extract_into_function_assist = assists.next().unwrap();
+        expect![[r#"
+            Assist {
+                id: AssistId(
+                    "extract_function",
+                    RefactorExtract,
+                ),
+                label: "Extract into function",
+                group: None,
+                target: 59..60,
+                source_change: None,
+            }
+        "#]]
+        .assert_debug_eq(&extract_into_function_assist);
+    }
+
+    {
+        let assists = Assist::get(
+            &db,
+            &cfg,
+            AssistResolveStrategy::Single(SingleResolve {
+                assist_id: "SOMETHING_MISMATCHING".to_string(),
+                assist_kind: AssistKind::RefactorExtract,
+            }),
+            frange,
+        );
+        assert_eq!(2, assists.len());
+        let mut assists = assists.into_iter();
+
+        let extract_into_variable_assist = assists.next().unwrap();
+        expect![[r#"
+            Assist {
+                id: AssistId(
+                    "extract_variable",
+                    RefactorExtract,
+                ),
+                label: "Extract into variable",
+                group: None,
+                target: 59..60,
+                source_change: None,
+            }
+        "#]]
+        .assert_debug_eq(&extract_into_variable_assist);
+
+        let extract_into_function_assist = assists.next().unwrap();
+        expect![[r#"
+            Assist {
+                id: AssistId(
+                    "extract_function",
+                    RefactorExtract,
+                ),
+                label: "Extract into function",
+                group: None,
+                target: 59..60,
+                source_change: None,
+            }
+        "#]]
+        .assert_debug_eq(&extract_into_function_assist);
+    }
+
+    {
+        let assists = Assist::get(
+            &db,
+            &cfg,
+            AssistResolveStrategy::Single(SingleResolve {
+                assist_id: "extract_variable".to_string(),
+                assist_kind: AssistKind::RefactorExtract,
+            }),
+            frange,
+        );
+        assert_eq!(2, assists.len());
+        let mut assists = assists.into_iter();
+
+        let extract_into_variable_assist = assists.next().unwrap();
+        expect![[r#"
+            Assist {
+                id: AssistId(
+                    "extract_variable",
+                    RefactorExtract,
+                ),
+                label: "Extract into variable",
+                group: None,
+                target: 59..60,
+                source_change: Some(
+                    SourceChange {
+                        source_file_edits: {
+                            FileId(
+                                0,
+                            ): TextEdit {
+                                indels: [
+                                    Indel {
+                                        insert: "let $0var_name = 5;\n    ",
+                                        delete: 45..45,
+                                    },
+                                    Indel {
+                                        insert: "var_name",
+                                        delete: 59..60,
+                                    },
+                                ],
+                            },
+                        },
+                        file_system_edits: [],
+                        is_snippet: true,
+                    },
+                ),
+            }
+        "#]]
+        .assert_debug_eq(&extract_into_variable_assist);
+
+        let extract_into_function_assist = assists.next().unwrap();
+        expect![[r#"
+            Assist {
+                id: AssistId(
+                    "extract_function",
+                    RefactorExtract,
+                ),
+                label: "Extract into function",
+                group: None,
+                target: 59..60,
+                source_change: None,
+            }
+        "#]]
+        .assert_debug_eq(&extract_into_function_assist);
+    }
+
+    {
+        let assists = Assist::get(&db, &cfg, AssistResolveStrategy::All, frange);
+        assert_eq!(2, assists.len());
+        let mut assists = assists.into_iter();
+
+        let extract_into_variable_assist = assists.next().unwrap();
+        expect![[r#"
+            Assist {
+                id: AssistId(
+                    "extract_variable",
+                    RefactorExtract,
+                ),
+                label: "Extract into variable",
+                group: None,
+                target: 59..60,
+                source_change: Some(
+                    SourceChange {
+                        source_file_edits: {
+                            FileId(
+                                0,
+                            ): TextEdit {
+                                indels: [
+                                    Indel {
+                                        insert: "let $0var_name = 5;\n    ",
+                                        delete: 45..45,
+                                    },
+                                    Indel {
+                                        insert: "var_name",
+                                        delete: 59..60,
+                                    },
+                                ],
+                            },
+                        },
+                        file_system_edits: [],
+                        is_snippet: true,
+                    },
+                ),
+            }
+        "#]]
+        .assert_debug_eq(&extract_into_variable_assist);
+
+        let extract_into_function_assist = assists.next().unwrap();
+        expect![[r#"
+            Assist {
+                id: AssistId(
+                    "extract_function",
+                    RefactorExtract,
+                ),
+                label: "Extract into function",
+                group: None,
+                target: 59..60,
+                source_change: Some(
+                    SourceChange {
+                        source_file_edits: {
+                            FileId(
+                                0,
+                            ): TextEdit {
+                                indels: [
+                                    Indel {
+                                        insert: "fun_name()",
+                                        delete: 59..60,
+                                    },
+                                    Indel {
+                                        insert: "\n\nfn $0fun_name() -> i32 {\n    5\n}",
+                                        delete: 110..110,
+                                    },
+                                ],
+                            },
+                        },
+                        file_system_edits: [],
+                        is_snippet: true,
+                    },
+                ),
+            }
+        "#]]
+        .assert_debug_eq(&extract_into_function_assist);
+    }
+}