From 3fea861c319225185524db9797e47dfada7bd80c Mon Sep 17 00:00:00 2001 From: discord9 Date: Thu, 15 May 2025 21:49:00 +0800 Subject: [PATCH 1/6] feat: add user_choice_group --- crates/ide-assists/src/assist_context.rs | 41 +++++++- crates/ide-db/src/assists.rs | 4 +- crates/ide-db/src/source_change.rs | 97 +++++++++++++++++++ .../trait_impl_redundant_assoc_item.rs | 1 + .../src/handlers/typed_hole.rs | 1 + .../src/handlers/unresolved_field.rs | 4 + .../src/handlers/unresolved_method.rs | 2 + .../src/handlers/unused_variables.rs | 1 + crates/ide-diagnostics/src/lib.rs | 1 + crates/ide/src/ssr.rs | 1 + 10 files changed, 151 insertions(+), 2 deletions(-) diff --git a/crates/ide-assists/src/assist_context.rs b/crates/ide-assists/src/assist_context.rs index 9eb9452a2b83..cf59f20a00c3 100644 --- a/crates/ide-assists/src/assist_context.rs +++ b/crates/ide-assists/src/assist_context.rs @@ -1,6 +1,7 @@ //! See [`AssistContext`]. use hir::{EditionedFileId, FileRange, Semantics}; +use ide_db::source_change::UserChoiceGroup; use ide_db::{FileId, RootDatabase, label::Label}; use syntax::Edition; use syntax::{ @@ -202,6 +203,36 @@ impl Assists { self.add_impl(Some(group), id, label.into(), target, &mut |it| f.take().unwrap()(it)) } + /// Give user multiple choices, user's choice will be passed to `f` as a list of indices. + /// The indices are the indices of the choices in the original list. + pub(crate) fn add_choices( + &mut self, + group: &Option, + id: AssistId, + label: impl Into, + target: TextRange, + choices: Vec>, + f: impl FnOnce(&mut SourceChangeBuilder, &[usize]) + Send + 'static, + ) -> Option<()> { + if !self.is_allowed(&id) { + return None; + } + let label = Label::new(label.into()); + let group = group.clone(); + + self.buf.push(Assist { + id, + label, + group, + target, + source_change: None, + command: None, + user_choice_group: Some(UserChoiceGroup::new(choices, f)), + }); + + Some(()) + } + fn add_impl( &mut self, group: Option<&GroupLabel>, @@ -226,7 +257,15 @@ impl Assists { let label = Label::new(label); let group = group.cloned(); - self.buf.push(Assist { id, label, group, target, source_change, command }); + self.buf.push(Assist { + id, + label, + group, + target, + source_change, + command, + user_choice_group: None, + }); Some(()) } diff --git a/crates/ide-db/src/assists.rs b/crates/ide-db/src/assists.rs index 384eb57c0fd5..27b9da0e532e 100644 --- a/crates/ide-db/src/assists.rs +++ b/crates/ide-db/src/assists.rs @@ -9,7 +9,7 @@ use std::str::FromStr; use syntax::TextRange; -use crate::{label::Label, source_change::SourceChange}; +use crate::{label::Label, source_change::{SourceChange, UserChoiceGroup}}; #[derive(Debug, Clone)] pub struct Assist { @@ -31,6 +31,8 @@ pub struct Assist { pub source_change: Option, /// The command to execute after the assist is applied. pub command: Option, + /// The group of choices to show to the user when applying the assist. + pub user_choice_group: Option } #[derive(Debug, Clone, Copy, PartialEq, Eq)] diff --git a/crates/ide-db/src/source_change.rs b/crates/ide-db/src/source_change.rs index b1b58d6568cb..da294faeed6e 100644 --- a/crates/ide-db/src/source_change.rs +++ b/crates/ide-db/src/source_change.rs @@ -3,8 +3,10 @@ //! //! It can be viewed as a dual for `Change`. +use std::sync::{Arc, Mutex}; use std::{collections::hash_map::Entry, fmt, iter, mem}; +use crate::source_change; use crate::text_edit::{TextEdit, TextEditBuilder}; use crate::{SnippetCap, assists::Command, syntax_helpers::tree_diff::diff}; use base_db::AnchoredPathBuf; @@ -557,3 +559,98 @@ impl PlaceSnippet { } } } + +/// a function that takes a `SourceChangeBuilder` and a slice of indices +/// which represent the indices of the choices made by the user +/// which is the choice being made, each one from corrseponding choice list in `Assists::add_choices` +pub type ChoiceCallback = dyn FnOnce(&mut SourceChangeBuilder, &[usize]) + Send + 'static; + +/// Represents a group of choices offered to the user, along with a callback +/// to be executed based on the user's selection. +/// +/// This is typically used in scenarios like "assists" or "quick fixes" where +/// the user needs to pick from several options to proceed with a source code change. +#[derive(Clone)] +pub struct UserChoiceGroup { + /// A list of choice groups. Each inner vector represents a set of options + /// from which the user can make one selection. + /// For example, `choices[0]` might be `["Option A", "Option B"]` and + /// `choices[1]` might be `["Setting X", "Setting Y"]`. + choice_options: Vec>, + /// The callback function to be invoked with the user's selections. + /// The `&[usize]` argument to the callback will contain the indices + /// of the choices made by the user, corresponding to each group in `choice_options`. + callback: Arc>>>, + /// The current choices made by the user, represented as a vector of indices. + cur_choices: Vec, +} + +impl std::fmt::Debug for UserChoiceGroup { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("UserChoiceGroup") + .field("choice_options", &self.choice_options) + .field("callback", &"") + .field("cur_choices", &self.cur_choices) + .finish() + } +} + +impl UserChoiceGroup { + /// Creates a new `UserChoiceGroup`. + /// + /// # Arguments + /// + /// * `choice_options`: A vector of vectors of strings, where each inner vector + /// represents a group of choices. + /// * `callback`: A function that will be called with the indices of the + /// user's selections after they make their choices. + /// + pub fn new( + choice_options: Vec>, + callback: impl FnOnce(&mut SourceChangeBuilder, &[usize]) + Send + 'static, + ) -> Self { + Self { + cur_choices: vec![], + choice_options, + callback: Arc::new(Mutex::new(Some(Box::new(callback)))), + } + } + + /// Returns next question(and it's indices) to be asked to the user. + /// + pub fn next_question(&self) -> Option<(usize, &Vec)> { + if self.cur_choices.len() < self.choice_options.len() { + let idx = self.cur_choices.len(); + let choices = &self.choice_options[idx]; + Some((idx, choices)) + } else { + None + } + } + + /// Make the idx-th choice in the group. + /// `choice` is the index of the choice in the group(0-based). + /// This function will be called when the user makes a choice. + pub fn make_choice(&mut self, idx: usize, choice: usize) -> Result<(), String> { + if idx < self.choice_options.len() && idx == self.cur_choices.len() { + self.cur_choices.push(choice); + } else { + return Err("Invalid index for choice group".to_string()); + } + + Ok(()) + } + + /// Finalizes the choices made by the user and invokes the callback. + /// This function should be called when the user has finished making their choices. + pub fn finish(self, builder: &mut SourceChangeBuilder) { + let mut callback = self.callback.lock().unwrap(); + let callback = callback.take().expect("Callback already"); + callback(builder, &self.cur_choices); + } + + // Potentially add other methods here, for example: + // - To get the number of choice groups. + // - To get the options for a specific group. + // - To validate selection indices. +} diff --git a/crates/ide-diagnostics/src/handlers/trait_impl_redundant_assoc_item.rs b/crates/ide-diagnostics/src/handlers/trait_impl_redundant_assoc_item.rs index 4327b12dce70..956d2779dc3d 100644 --- a/crates/ide-diagnostics/src/handlers/trait_impl_redundant_assoc_item.rs +++ b/crates/ide-diagnostics/src/handlers/trait_impl_redundant_assoc_item.rs @@ -106,6 +106,7 @@ fn quickfix_for_redundant_assoc_item( target: range, source_change: Some(source_change_builder.finish()), command: None, + user_choice_group: None, }]) } diff --git a/crates/ide-diagnostics/src/handlers/typed_hole.rs b/crates/ide-diagnostics/src/handlers/typed_hole.rs index 1915a88dd002..57a64b160ce2 100644 --- a/crates/ide-diagnostics/src/handlers/typed_hole.rs +++ b/crates/ide-diagnostics/src/handlers/typed_hole.rs @@ -94,6 +94,7 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::TypedHole) -> Option TextEdit::replace(original_range.range, code), )), command: None, + user_choice_group: None, }) .collect(); diff --git a/crates/ide-diagnostics/src/handlers/unresolved_field.rs b/crates/ide-diagnostics/src/handlers/unresolved_field.rs index 0649c97f8205..024f85b157ab 100644 --- a/crates/ide-diagnostics/src/handlers/unresolved_field.rs +++ b/crates/ide-diagnostics/src/handlers/unresolved_field.rs @@ -127,6 +127,7 @@ fn add_variant_to_union( target: error_range.range, source_change: Some(src_change_builder.finish()), command: None, + user_choice_group: None, }) } @@ -176,6 +177,7 @@ fn add_field_to_struct_fix( target: error_range.range, source_change: Some(src_change_builder.finish()), command: None, + user_choice_group: None, }) } None => { @@ -213,6 +215,7 @@ fn add_field_to_struct_fix( target: error_range.range, source_change: Some(src_change_builder.finish()), command: None, + user_choice_group: None, }) } Some(FieldList::TupleFieldList(_tuple)) => { @@ -275,6 +278,7 @@ fn method_fix( TextEdit::insert(range.end(), "()".to_owned()), )), command: None, + user_choice_group: None, }) } #[cfg(test)] diff --git a/crates/ide-diagnostics/src/handlers/unresolved_method.rs b/crates/ide-diagnostics/src/handlers/unresolved_method.rs index 00c2a8c4c468..59eb8eea4329 100644 --- a/crates/ide-diagnostics/src/handlers/unresolved_method.rs +++ b/crates/ide-diagnostics/src/handlers/unresolved_method.rs @@ -104,6 +104,7 @@ fn field_fix( (file_id.file_id(ctx.sema.db), TextEdit::insert(range.end(), ")".to_owned())), ])), command: None, + user_choice_group: None, }) } @@ -185,6 +186,7 @@ fn assoc_func_fix(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedMethodCall) - TextEdit::replace(range, assoc_func_call_expr_string), )), command: None, + user_choice_group: None, }) } else { None diff --git a/crates/ide-diagnostics/src/handlers/unused_variables.rs b/crates/ide-diagnostics/src/handlers/unused_variables.rs index e6bbff05f7e8..52ded9c6292b 100644 --- a/crates/ide-diagnostics/src/handlers/unused_variables.rs +++ b/crates/ide-diagnostics/src/handlers/unused_variables.rs @@ -80,6 +80,7 @@ fn fixes( TextEdit::replace(name_range, format!("_{}", var_name.display(db, edition))), )), command: None, + user_choice_group: None, }]) } diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs index 2af14ca949bf..946ab75d3578 100644 --- a/crates/ide-diagnostics/src/lib.rs +++ b/crates/ide-diagnostics/src/lib.rs @@ -983,6 +983,7 @@ fn unresolved_fix(id: &'static str, label: &str, target: TextRange) -> Assist { target, source_change: None, command: None, + user_choice_group: None, } } diff --git a/crates/ide/src/ssr.rs b/crates/ide/src/ssr.rs index 7df4499a0c2f..05e612944ce7 100644 --- a/crates/ide/src/ssr.rs +++ b/crates/ide/src/ssr.rs @@ -46,6 +46,7 @@ pub(crate) fn ssr_assists( target: comment_range, source_change, command: None, + user_choice_group: None, }; ssr_assists.push(assist); From 443b2926f354c138c8b13f1f6649363156979a22 Mon Sep 17 00:00:00 2001 From: discord9 Date: Thu, 15 May 2025 21:58:37 +0800 Subject: [PATCH 2/6] todo: add in handle_code_action to global snapshot --- crates/ide-assists/src/assist_context.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/ide-assists/src/assist_context.rs b/crates/ide-assists/src/assist_context.rs index cf59f20a00c3..cab8909e8f96 100644 --- a/crates/ide-assists/src/assist_context.rs +++ b/crates/ide-assists/src/assist_context.rs @@ -205,6 +205,8 @@ impl Assists { /// Give user multiple choices, user's choice will be passed to `f` as a list of indices. /// The indices are the indices of the choices in the original list. + /// TODO(discord9): remove allow(unused) once auto import all use this function + #[allow(unused)] pub(crate) fn add_choices( &mut self, group: &Option, From 3d5b46de233281c52ee3e640e576ee1f72aa153c Mon Sep 17 00:00:00 2001 From: discord9 Date: Mon, 19 May 2025 20:35:42 +0800 Subject: [PATCH 3/6] todo: build&apply source change --- crates/ide-assists/src/assist_context.rs | 13 ++- crates/ide-db/src/source_change.rs | 89 +++++++++++---- crates/rust-analyzer/src/global_state.rs | 10 +- crates/rust-analyzer/src/handlers/request.rs | 5 +- crates/rust-analyzer/src/main_loop.rs | 108 ++++++++++++++++++- 5 files changed, 200 insertions(+), 25 deletions(-) diff --git a/crates/ide-assists/src/assist_context.rs b/crates/ide-assists/src/assist_context.rs index cab8909e8f96..4db1fe2e3c9d 100644 --- a/crates/ide-assists/src/assist_context.rs +++ b/crates/ide-assists/src/assist_context.rs @@ -1,7 +1,7 @@ //! See [`AssistContext`]. use hir::{EditionedFileId, FileRange, Semantics}; -use ide_db::source_change::UserChoiceGroup; +use ide_db::source_change::{UserChoice, UserChoiceGroup}; use ide_db::{FileId, RootDatabase, label::Label}; use syntax::Edition; use syntax::{ @@ -213,7 +213,7 @@ impl Assists { id: AssistId, label: impl Into, target: TextRange, - choices: Vec>, + choices: Vec<(String, Vec)>, f: impl FnOnce(&mut SourceChangeBuilder, &[usize]) + Send + 'static, ) -> Option<()> { if !self.is_allowed(&id) { @@ -229,7 +229,14 @@ impl Assists { target, source_change: None, command: None, - user_choice_group: Some(UserChoiceGroup::new(choices, f)), + user_choice_group: Some(UserChoiceGroup::new( + choices + .into_iter() + .map(|(title, choices)| UserChoice::new(title, choices)) + .collect(), + f, + self.file, + )), }); Some(()) diff --git a/crates/ide-db/src/source_change.rs b/crates/ide-db/src/source_change.rs index da294faeed6e..77e51f8fd685 100644 --- a/crates/ide-db/src/source_change.rs +++ b/crates/ide-db/src/source_change.rs @@ -3,10 +3,10 @@ //! //! It can be viewed as a dual for `Change`. +use std::collections::VecDeque; use std::sync::{Arc, Mutex}; use std::{collections::hash_map::Entry, fmt, iter, mem}; -use crate::source_change; use crate::text_edit::{TextEdit, TextEditBuilder}; use crate::{SnippetCap, assists::Command, syntax_helpers::tree_diff::diff}; use base_db::AnchoredPathBuf; @@ -565,24 +565,39 @@ impl PlaceSnippet { /// which is the choice being made, each one from corrseponding choice list in `Assists::add_choices` pub type ChoiceCallback = dyn FnOnce(&mut SourceChangeBuilder, &[usize]) + Send + 'static; -/// Represents a group of choices offered to the user, along with a callback +/// Represents a group of choices offered to the user(Using ShowMessageRequest), along with a callback /// to be executed based on the user's selection. /// /// This is typically used in scenarios like "assists" or "quick fixes" where /// the user needs to pick from several options to proceed with a source code change. #[derive(Clone)] pub struct UserChoiceGroup { - /// A list of choice groups. Each inner vector represents a set of options + /// A list of choice groups. Each inner tuple's first string is title, second vector represents a set of options /// from which the user can make one selection. - /// For example, `choices[0]` might be `["Option A", "Option B"]` and - /// `choices[1]` might be `["Setting X", "Setting Y"]`. - choice_options: Vec>, + /// For example, `choice_options[0]` might be `["Question 1", ["Option A", "Option B"]]` and + /// `choices[1]` might be `["Question 2", ["Setting X", "Setting Y"]]`. + choice_options: Vec, /// The callback function to be invoked with the user's selections. /// The `&[usize]` argument to the callback will contain the indices /// of the choices made by the user, corresponding to each group in `choice_options`. callback: Arc>>>, /// The current choices made by the user, represented as a vector of indices. cur_choices: Vec, + /// The file ID associated with the choices. Used for construct SourceChangeBuilder. + /// This is typically the file where the changes will be applied. + file: FileId, +} + +#[derive(Debug, Clone)] +pub struct UserChoice { + pub title: String, + pub actions: Vec, +} + +impl UserChoice { + pub fn new(title: String, actions: Vec) -> Self { + Self { title, actions } + } } impl std::fmt::Debug for UserChoiceGroup { @@ -600,29 +615,30 @@ impl UserChoiceGroup { /// /// # Arguments /// - /// * `choice_options`: A vector of vectors of strings, where each inner vector - /// represents a group of choices. + /// * `choice_options`: A vector of `UserChoice` objects representing the choices /// * `callback`: A function that will be called with the indices of the /// user's selections after they make their choices. /// pub fn new( - choice_options: Vec>, + choice_options: Vec, callback: impl FnOnce(&mut SourceChangeBuilder, &[usize]) + Send + 'static, + file: FileId, ) -> Self { Self { cur_choices: vec![], choice_options, callback: Arc::new(Mutex::new(Some(Box::new(callback)))), + file, } } - /// Returns next question(and it's indices) to be asked to the user. + /// Returns (`idx`, `title`, `choices`) of the current question. /// - pub fn next_question(&self) -> Option<(usize, &Vec)> { + pub fn get_cur_question(&self) -> Option<(usize, &UserChoice)> { if self.cur_choices.len() < self.choice_options.len() { let idx = self.cur_choices.len(); - let choices = &self.choice_options[idx]; - Some((idx, choices)) + let user_choice = &self.choice_options[idx]; + Some((idx, user_choice)) } else { None } @@ -631,8 +647,8 @@ impl UserChoiceGroup { /// Make the idx-th choice in the group. /// `choice` is the index of the choice in the group(0-based). /// This function will be called when the user makes a choice. - pub fn make_choice(&mut self, idx: usize, choice: usize) -> Result<(), String> { - if idx < self.choice_options.len() && idx == self.cur_choices.len() { + pub fn make_choice(&mut self, question_idx: usize, choice: usize) -> Result<(), String> { + if question_idx < self.choice_options.len() && question_idx == self.cur_choices.len() { self.cur_choices.push(choice); } else { return Err("Invalid index for choice group".to_string()); @@ -648,9 +664,44 @@ impl UserChoiceGroup { let callback = callback.take().expect("Callback already"); callback(builder, &self.cur_choices); } +} + +/// A handler for managing user choices in a queue. +#[derive(Debug, Default)] +pub struct UserChoiceHandler { + /// If multiple choice group are made, we will queue them up and ask the user + /// one by one. + queue: VecDeque, + /// Indicates if the first choice group in the queue is being processed. Prevent send requests repeatedly. + is_awaiting: bool, +} + +impl UserChoiceHandler { + /// Creates a new `UserChoiceHandler`. + pub fn new() -> Self { + Self::default() + } + + /// Adds a new `UserChoiceGroup` to the queue. + pub fn add_choice_group(&mut self, group: UserChoiceGroup) { + self.queue.push_back(group); + } + + pub fn first_mut_choice_group(&mut self) -> Option<&mut UserChoiceGroup> { + self.queue.front_mut() + } + + pub fn pop_choice_group(&mut self) -> Option { + self.set_awaiting(false); + self.queue.pop_front() + } + + pub fn is_awaiting(&self) -> bool { + self.is_awaiting + } + + pub fn set_awaiting(&mut self, awaiting: bool) { + self.is_awaiting = awaiting; + } - // Potentially add other methods here, for example: - // - To get the number of choice groups. - // - To get the options for a specific group. - // - To validate selection indices. } diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index 3b3b9c879754..8578e13bd81c 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -8,7 +8,7 @@ use std::{ops::Not as _, time::Instant}; use crossbeam_channel::{Receiver, Sender, unbounded}; use hir::ChangeWithProcMacros; use ide::{Analysis, AnalysisHost, Cancellable, FileId, SourceRootId}; -use ide_db::base_db::{Crate, ProcMacroPaths, SourceDatabase}; +use ide_db::{base_db::{Crate, ProcMacroPaths, SourceDatabase}, source_change::UserChoiceHandler}; use itertools::Itertools; use load_cargo::SourceRootConfig; use lsp_types::{SemanticTokens, Url}; @@ -171,6 +171,9 @@ pub(crate) struct GlobalState { /// this queue should run only *after* [`GlobalState::process_changes`] has /// been called. pub(crate) deferred_task_queue: TaskQueue, + + /// For handling user choice group using `ShowMessageRequest`. + pub(crate) user_choice_handler: Arc>, } /// An immutable snapshot of the world's state at a point in time. @@ -187,6 +190,8 @@ pub(crate) struct GlobalStateSnapshot { // FIXME: Can we derive this from somewhere else? pub(crate) proc_macros_loaded: bool, pub(crate) flycheck: Arc<[FlycheckHandle]>, + /// For handling user choice group using `ShowMessageRequest`. + pub(crate) user_choice_handler: Arc>, } impl std::panic::UnwindSafe for GlobalStateSnapshot {} @@ -282,6 +287,8 @@ impl GlobalState { discover_workspace_queue: OpQueue::default(), deferred_task_queue: task_queue, + + user_choice_handler: Arc::new(Mutex::new(UserChoiceHandler::default())), }; // Apply any required database inputs from the config. this.update_configuration(config); @@ -531,6 +538,7 @@ impl GlobalState { proc_macros_loaded: !self.config.expand_proc_macros() || self.fetch_proc_macros_queue.last_op_result().copied().unwrap_or(false), flycheck: self.flycheck.clone(), + user_choice_handler: self.user_choice_handler.clone() } } diff --git a/crates/rust-analyzer/src/handlers/request.rs b/crates/rust-analyzer/src/handlers/request.rs index 69983a676261..cf1990852284 100644 --- a/crates/rust-analyzer/src/handlers/request.rs +++ b/crates/rust-analyzer/src/handlers/request.rs @@ -1443,7 +1443,10 @@ pub(crate) fn handle_code_action( resolve, frange, )?; - for (index, assist) in assists.into_iter().enumerate() { + for (index, mut assist) in assists.into_iter().enumerate() { + if let Some(user_choice_group) = assist.user_choice_group.take() { + snap.user_choice_handler.lock().add_choice_group(user_choice_group); + } let resolve_data = if code_action_resolve_cap { Some((index, params.clone(), snap.file_version(file_id))) } else { diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index bd213ffa57a1..8c5c1c097e15 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -11,7 +11,10 @@ use std::{ use crossbeam_channel::{Receiver, select}; use ide_db::base_db::{SourceDatabase, VfsPath, salsa::Database as _}; use lsp_server::{Connection, Notification, Request}; -use lsp_types::{TextDocumentIdentifier, notification::Notification as _}; +use lsp_types::{ + ShowMessageRequestParams, TextDocumentIdentifier, notification::Notification as _, + request::Request as _, request::ShowMessageRequest, +}; use stdx::thread::ThreadIntent; use tracing::{Level, error, span}; use vfs::{AbsPathBuf, FileId, loader::LoadingProgress}; @@ -515,6 +518,7 @@ impl GlobalState { } self.update_status_or_notify(); + self.ask_for_choice(); let loop_duration = loop_start.elapsed(); if loop_duration > Duration::from_millis(100) && was_quiescent { @@ -713,6 +717,108 @@ impl GlobalState { } } + /// Ask for user choice by sending ShowMessageRequest + fn ask_for_choice(&mut self) { + let params = { + let mut handler = self.user_choice_handler.lock(); + if handler.is_awaiting() { + // already sent a request, do nothing + return; + } + let params = if let Some(choice_group) = handler.first_mut_choice_group() { + if let Some((_idx, choice)) = choice_group.get_cur_question() { + Some(ShowMessageRequestParams { + typ: lsp_types::MessageType::INFO, + message: choice.title.clone(), + actions: Some( + choice + .actions + .clone() + .into_iter() + .map(|action| lsp_types::MessageActionItem { + title: action, + properties: Default::default(), + }) + .collect(), + ), + }) + } else { + // TODO: handle finished choice + // spawn a new task to handle the finished choice, in case of panic + None + } + } else { + None + }; + if params.is_some() { + handler.set_awaiting(true); + } + params + }; + + // send ShowMessageRequest to the client, and handle the response + if let Some(params) = params { + self.send_request::(params, |state, response| { + let lsp_server::Response { error: None, result: Some(result), .. } = response + else { + return; + }; + let choice = match crate::from_json::< + ::Result, + >( + lsp_types::request::ShowMessageRequest::METHOD, &result + ) { + Ok(Some(item)) => Some(item.title.clone()), + Err(err) => { + tracing::error!("Failed to deserialize ShowMessageRequest result: {err}"); + None + } + // user made no choice + Ok(None) => None, + }; + let mut do_pop = false; + let mut handler = state.user_choice_handler.lock(); + match (handler.first_mut_choice_group(), choice) { + (Some(choice_group), Some(choice)) => { + let Some((question_idx, user_choices)) = choice_group.get_cur_question() + else { + tracing::error!("No question found for user choice"); + return; + }; + let choice_idx = user_choices + .actions + .iter() + .position(|it| *it == choice) + .unwrap_or(user_choices.actions.len()); + if let Err(err) = choice_group.make_choice(question_idx, choice_idx) { + tracing::error!("Failed to make choice: {err}"); + } + } + (None, Some(choice)) => { + tracing::error!("No ongoing choice group found for user choice: {choice}"); + } + (Some(_), None) => { + // user made no choice, pop&drop current choice group + do_pop = true; + } + _ => (), + } + + if do_pop { + let group = handler.pop_choice_group(); + tracing::error!( + "User made no choice, dropping current choice group: {group:?}" + ); + } + handler.set_awaiting(false); + drop(handler); + + // recursively call handle_choice to handle the next question + state.ask_for_choice(); + }); + } + } + fn handle_task(&mut self, prime_caches_progress: &mut Vec, task: Task) { match task { Task::Response(response) => self.respond(response), From 71c8a09f871322e461e2cb1f94b51f2f56765e47 Mon Sep 17 00:00:00 2001 From: discord9 Date: Tue, 20 May 2025 19:07:48 +0800 Subject: [PATCH 4/6] die horribly with null request id --- crates/ide-db/src/source_change.rs | 11 ++ crates/rust-analyzer/src/lsp/utils.rs | 174 +++++++++++++++++++++++++- crates/rust-analyzer/src/main_loop.rs | 107 +--------------- 3 files changed, 182 insertions(+), 110 deletions(-) diff --git a/crates/ide-db/src/source_change.rs b/crates/ide-db/src/source_change.rs index 77e51f8fd685..92f40fb0deaf 100644 --- a/crates/ide-db/src/source_change.rs +++ b/crates/ide-db/src/source_change.rs @@ -644,6 +644,11 @@ impl UserChoiceGroup { } } + /// Whether the user has finished making their choices. + pub fn is_done_asking(&self) -> bool { + self.cur_choices.len() == self.choice_options.len() + } + /// Make the idx-th choice in the group. /// `choice` is the index of the choice in the group(0-based). /// This function will be called when the user makes a choice. @@ -664,6 +669,10 @@ impl UserChoiceGroup { let callback = callback.take().expect("Callback already"); callback(builder, &self.cur_choices); } + + pub fn file_id(&self) -> FileId { + self.file + } } /// A handler for managing user choices in a queue. @@ -696,10 +705,12 @@ impl UserChoiceHandler { self.queue.pop_front() } + /// Whether awaiting for sent request's response. pub fn is_awaiting(&self) -> bool { self.is_awaiting } + /// Sets the awaiting state. pub fn set_awaiting(&mut self, awaiting: bool) { self.is_awaiting = awaiting; } diff --git a/crates/rust-analyzer/src/lsp/utils.rs b/crates/rust-analyzer/src/lsp/utils.rs index 673eaa5952f0..29fac47f2f16 100644 --- a/crates/rust-analyzer/src/lsp/utils.rs +++ b/crates/rust-analyzer/src/lsp/utils.rs @@ -1,15 +1,21 @@ //! Utilities for LSP-related boilerplate code. -use std::{mem, ops::Range}; +use std::{mem, ops::Range, panic}; -use lsp_server::Notification; -use lsp_types::request::Request; +use ide_db::{base_db::DbPanicContext, source_change::SourceChangeBuilder}; +use lsp_server::{Notification, RequestId, Response, ResponseError}; +use lsp_types::{ + ShowMessageRequestParams, + request::{Request, ShowMessageRequest}, +}; +use stdx::thread::ThreadIntent; use triomphe::Arc; use crate::{ global_state::GlobalState, line_index::{LineEndings, LineIndex, PositionEncoding}, - lsp::{LspError, from_proto}, + lsp::{LspError, from_proto, to_proto}, lsp_ext, + main_loop::Task, }; pub(crate) fn invalid_params_error(message: String) -> LspError { @@ -95,6 +101,166 @@ impl GlobalState { } } + /// Ask for user choice by sending ShowMessageRequest + pub(crate) fn ask_for_choice(&mut self) { + let params = { + let mut handler = self.user_choice_handler.lock(); + if handler.is_awaiting() { + // already sent a request, do nothing + return; + } + let mut is_done_asking = false; + let params = if let Some(choice_group) = handler.first_mut_choice_group() { + if let Some((_idx, choice)) = choice_group.get_cur_question() { + Some(ShowMessageRequestParams { + typ: lsp_types::MessageType::INFO, + message: choice.title.clone(), + actions: Some( + choice + .actions + .clone() + .into_iter() + .map(|action| lsp_types::MessageActionItem { + title: action, + properties: Default::default(), + }) + .collect(), + ), + }) + } else { + is_done_asking = choice_group.is_done_asking(); + None + } + } else { + None + }; + + if is_done_asking { + let Some(choice_group) = handler.pop_choice_group() else { + return; + }; + let snap = self.snapshot(); + // TODO: handle finished choice + // spawn a new task to handle the finished choice, in case of panic + self.task_pool.handle.spawn(ThreadIntent::Worker, move || { + let result = panic::catch_unwind(move || { + let _pctx = DbPanicContext::enter("ask_for_choice".to_string()); + let mut source_change_builder = + SourceChangeBuilder::new(choice_group.file_id()); + choice_group.finish(&mut source_change_builder); + let source_change = source_change_builder.finish(); + to_proto::workspace_edit(&snap, source_change) + }); + + // it's either this or die horribly + let empty_req_id = RequestId::from("".to_string()); + match result { + Ok(Ok(result)) => Task::Response(Response::new_ok(empty_req_id, result)), + Ok(Err(_cancelled)) => Task::Response(Response { + id: empty_req_id, + result: None, + error: Some(ResponseError { + code: lsp_server::ErrorCode::ContentModified as i32, + message: "content modified".to_owned(), + data: None, + }), + }), + Err(panic) => { + let panic_message = panic + .downcast_ref::() + .map(String::as_str) + .or_else(|| panic.downcast_ref::<&str>().copied()); + + let mut message = "request handler panicked".to_owned(); + if let Some(panic_message) = panic_message { + message.push_str(": "); + message.push_str(panic_message) + } else if let Ok(_cancelled) = + panic.downcast::() + { + tracing::error!( + "Cancellation propagated out of salsa! This is a bug" + ); + } + Task::Response(Response::new_err( + empty_req_id, + lsp_server::ErrorCode::InternalError as i32, + message, + )) + } + } + }); + } + + if params.is_some() { + handler.set_awaiting(true); + } + params + }; + + // send ShowMessageRequest to the client, and handle the response + if let Some(params) = params { + self.send_request::(params, |state, response| { + let lsp_server::Response { error: None, result: Some(result), .. } = response + else { + return; + }; + let choice = match crate::from_json::< + ::Result, + >( + lsp_types::request::ShowMessageRequest::METHOD, &result + ) { + Ok(Some(item)) => Some(item.title.clone()), + Err(err) => { + tracing::error!("Failed to deserialize ShowMessageRequest result: {err}"); + None + } + // user made no choice + Ok(None) => None, + }; + let mut do_pop = false; + let mut handler = state.user_choice_handler.lock(); + match (handler.first_mut_choice_group(), choice) { + (Some(choice_group), Some(choice)) => { + let Some((question_idx, user_choices)) = choice_group.get_cur_question() + else { + tracing::error!("No question found for user choice"); + return; + }; + let choice_idx = user_choices + .actions + .iter() + .position(|it| *it == choice) + .unwrap_or(user_choices.actions.len()); + if let Err(err) = choice_group.make_choice(question_idx, choice_idx) { + tracing::error!("Failed to make choice: {err}"); + } + } + (None, Some(choice)) => { + tracing::error!("No ongoing choice group found for user choice: {choice}"); + } + (Some(_), None) => { + // user made no choice, pop&drop current choice group + do_pop = true; + } + _ => (), + } + + if do_pop { + let group = handler.pop_choice_group(); + tracing::error!( + "User made no choice, dropping current choice group: {group:?}" + ); + } + handler.set_awaiting(false); + drop(handler); + + // recursively call handle_choice to handle the next question + state.ask_for_choice(); + }); + } + } + /// rust-analyzer is resilient -- if it fails, this doesn't usually affect /// the user experience. Part of that is that we deliberately hide panics /// from the user. diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index 8c5c1c097e15..05a0e8e5b8c5 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -11,10 +11,7 @@ use std::{ use crossbeam_channel::{Receiver, select}; use ide_db::base_db::{SourceDatabase, VfsPath, salsa::Database as _}; use lsp_server::{Connection, Notification, Request}; -use lsp_types::{ - ShowMessageRequestParams, TextDocumentIdentifier, notification::Notification as _, - request::Request as _, request::ShowMessageRequest, -}; +use lsp_types::{TextDocumentIdentifier, notification::Notification as _}; use stdx::thread::ThreadIntent; use tracing::{Level, error, span}; use vfs::{AbsPathBuf, FileId, loader::LoadingProgress}; @@ -717,108 +714,6 @@ impl GlobalState { } } - /// Ask for user choice by sending ShowMessageRequest - fn ask_for_choice(&mut self) { - let params = { - let mut handler = self.user_choice_handler.lock(); - if handler.is_awaiting() { - // already sent a request, do nothing - return; - } - let params = if let Some(choice_group) = handler.first_mut_choice_group() { - if let Some((_idx, choice)) = choice_group.get_cur_question() { - Some(ShowMessageRequestParams { - typ: lsp_types::MessageType::INFO, - message: choice.title.clone(), - actions: Some( - choice - .actions - .clone() - .into_iter() - .map(|action| lsp_types::MessageActionItem { - title: action, - properties: Default::default(), - }) - .collect(), - ), - }) - } else { - // TODO: handle finished choice - // spawn a new task to handle the finished choice, in case of panic - None - } - } else { - None - }; - if params.is_some() { - handler.set_awaiting(true); - } - params - }; - - // send ShowMessageRequest to the client, and handle the response - if let Some(params) = params { - self.send_request::(params, |state, response| { - let lsp_server::Response { error: None, result: Some(result), .. } = response - else { - return; - }; - let choice = match crate::from_json::< - ::Result, - >( - lsp_types::request::ShowMessageRequest::METHOD, &result - ) { - Ok(Some(item)) => Some(item.title.clone()), - Err(err) => { - tracing::error!("Failed to deserialize ShowMessageRequest result: {err}"); - None - } - // user made no choice - Ok(None) => None, - }; - let mut do_pop = false; - let mut handler = state.user_choice_handler.lock(); - match (handler.first_mut_choice_group(), choice) { - (Some(choice_group), Some(choice)) => { - let Some((question_idx, user_choices)) = choice_group.get_cur_question() - else { - tracing::error!("No question found for user choice"); - return; - }; - let choice_idx = user_choices - .actions - .iter() - .position(|it| *it == choice) - .unwrap_or(user_choices.actions.len()); - if let Err(err) = choice_group.make_choice(question_idx, choice_idx) { - tracing::error!("Failed to make choice: {err}"); - } - } - (None, Some(choice)) => { - tracing::error!("No ongoing choice group found for user choice: {choice}"); - } - (Some(_), None) => { - // user made no choice, pop&drop current choice group - do_pop = true; - } - _ => (), - } - - if do_pop { - let group = handler.pop_choice_group(); - tracing::error!( - "User made no choice, dropping current choice group: {group:?}" - ); - } - handler.set_awaiting(false); - drop(handler); - - // recursively call handle_choice to handle the next question - state.ask_for_choice(); - }); - } - } - fn handle_task(&mut self, prime_caches_progress: &mut Vec, task: Task) { match task { Task::Response(response) => self.respond(response), From d4cf3861b1b9d16a7bf40e57d7aa2c7987dff951 Mon Sep 17 00:00:00 2001 From: discord9 Date: Tue, 20 May 2025 19:54:10 +0800 Subject: [PATCH 5/6] chore: fix ci --- crates/ide-assists/src/tests.rs | 16 ++++++++++++++++ crates/ide-db/src/assists.rs | 7 +++++-- crates/ide-db/src/source_change.rs | 3 +-- crates/ide/src/ssr.rs | 4 ++++ crates/rust-analyzer/src/global_state.rs | 7 +++++-- crates/rust-analyzer/src/lsp/utils.rs | 1 - 6 files changed, 31 insertions(+), 7 deletions(-) diff --git a/crates/ide-assists/src/tests.rs b/crates/ide-assists/src/tests.rs index 5e6889792db6..dc02a321b919 100644 --- a/crates/ide-assists/src/tests.rs +++ b/crates/ide-assists/src/tests.rs @@ -548,6 +548,7 @@ pub fn test_some_range(a: int) -> bool { target: 59..60, source_change: None, command: None, + user_choice_group: None, } "#]] .assert_debug_eq(&extract_into_variable_assist); @@ -569,6 +570,7 @@ pub fn test_some_range(a: int) -> bool { target: 59..60, source_change: None, command: None, + user_choice_group: None, } "#]] .assert_debug_eq(&extract_into_constant_assist); @@ -590,6 +592,7 @@ pub fn test_some_range(a: int) -> bool { target: 59..60, source_change: None, command: None, + user_choice_group: None, } "#]] .assert_debug_eq(&extract_into_static_assist); @@ -611,6 +614,7 @@ pub fn test_some_range(a: int) -> bool { target: 59..60, source_change: None, command: None, + user_choice_group: None, } "#]] .assert_debug_eq(&extract_into_function_assist); @@ -647,6 +651,7 @@ pub fn test_some_range(a: int) -> bool { target: 59..60, source_change: None, command: None, + user_choice_group: None, } "#]] .assert_debug_eq(&extract_into_variable_assist); @@ -668,6 +673,7 @@ pub fn test_some_range(a: int) -> bool { target: 59..60, source_change: None, command: None, + user_choice_group: None, } "#]] .assert_debug_eq(&extract_into_constant_assist); @@ -689,6 +695,7 @@ pub fn test_some_range(a: int) -> bool { target: 59..60, source_change: None, command: None, + user_choice_group: None, } "#]] .assert_debug_eq(&extract_into_static_assist); @@ -710,6 +717,7 @@ pub fn test_some_range(a: int) -> bool { target: 59..60, source_change: None, command: None, + user_choice_group: None, } "#]] .assert_debug_eq(&extract_into_function_assist); @@ -792,6 +800,7 @@ pub fn test_some_range(a: int) -> bool { command: Some( Rename, ), + user_choice_group: None, } "#]] .assert_debug_eq(&extract_into_variable_assist); @@ -813,6 +822,7 @@ pub fn test_some_range(a: int) -> bool { target: 59..60, source_change: None, command: None, + user_choice_group: None, } "#]] .assert_debug_eq(&extract_into_constant_assist); @@ -834,6 +844,7 @@ pub fn test_some_range(a: int) -> bool { target: 59..60, source_change: None, command: None, + user_choice_group: None, } "#]] .assert_debug_eq(&extract_into_static_assist); @@ -855,6 +866,7 @@ pub fn test_some_range(a: int) -> bool { target: 59..60, source_change: None, command: None, + user_choice_group: None, } "#]] .assert_debug_eq(&extract_into_function_assist); @@ -933,6 +945,7 @@ pub fn test_some_range(a: int) -> bool { command: Some( Rename, ), + user_choice_group: None, } "#]] .assert_debug_eq(&extract_into_variable_assist); @@ -1004,6 +1017,7 @@ pub fn test_some_range(a: int) -> bool { command: Some( Rename, ), + user_choice_group: None, } "#]] .assert_debug_eq(&extract_into_constant_assist); @@ -1075,6 +1089,7 @@ pub fn test_some_range(a: int) -> bool { command: Some( Rename, ), + user_choice_group: None, } "#]] .assert_debug_eq(&extract_into_static_assist); @@ -1132,6 +1147,7 @@ pub fn test_some_range(a: int) -> bool { }, ), command: None, + user_choice_group: None, } "#]] .assert_debug_eq(&extract_into_function_assist); diff --git a/crates/ide-db/src/assists.rs b/crates/ide-db/src/assists.rs index 27b9da0e532e..c1be9aeee171 100644 --- a/crates/ide-db/src/assists.rs +++ b/crates/ide-db/src/assists.rs @@ -9,7 +9,10 @@ use std::str::FromStr; use syntax::TextRange; -use crate::{label::Label, source_change::{SourceChange, UserChoiceGroup}}; +use crate::{ + label::Label, + source_change::{SourceChange, UserChoiceGroup}, +}; #[derive(Debug, Clone)] pub struct Assist { @@ -32,7 +35,7 @@ pub struct Assist { /// The command to execute after the assist is applied. pub command: Option, /// The group of choices to show to the user when applying the assist. - pub user_choice_group: Option + pub user_choice_group: Option, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] diff --git a/crates/ide-db/src/source_change.rs b/crates/ide-db/src/source_change.rs index 92f40fb0deaf..5d7b1a68dc58 100644 --- a/crates/ide-db/src/source_change.rs +++ b/crates/ide-db/src/source_change.rs @@ -562,7 +562,7 @@ impl PlaceSnippet { /// a function that takes a `SourceChangeBuilder` and a slice of indices /// which represent the indices of the choices made by the user -/// which is the choice being made, each one from corrseponding choice list in `Assists::add_choices` +/// which is the choice being made, each one from corresponding choice list in `Assists::add_choices` pub type ChoiceCallback = dyn FnOnce(&mut SourceChangeBuilder, &[usize]) + Send + 'static; /// Represents a group of choices offered to the user(Using ShowMessageRequest), along with a callback @@ -714,5 +714,4 @@ impl UserChoiceHandler { pub fn set_awaiting(&mut self, awaiting: bool) { self.is_awaiting = awaiting; } - } diff --git a/crates/ide/src/ssr.rs b/crates/ide/src/ssr.rs index 05e612944ce7..1c2506c8a02e 100644 --- a/crates/ide/src/ssr.rs +++ b/crates/ide/src/ssr.rs @@ -155,6 +155,7 @@ mod tests { }, ), command: None, + user_choice_group: None, } "#]] .assert_debug_eq(&apply_in_file_assist); @@ -213,6 +214,7 @@ mod tests { }, ), command: None, + user_choice_group: None, } "#]] .assert_debug_eq(&apply_in_workspace_assist); @@ -254,6 +256,7 @@ mod tests { target: 10..21, source_change: None, command: None, + user_choice_group: None, } "#]] .assert_debug_eq(&apply_in_file_assist); @@ -275,6 +278,7 @@ mod tests { target: 10..21, source_change: None, command: None, + user_choice_group: None, } "#]] .assert_debug_eq(&apply_in_workspace_assist); diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index 8578e13bd81c..f641713e847e 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -8,7 +8,10 @@ use std::{ops::Not as _, time::Instant}; use crossbeam_channel::{Receiver, Sender, unbounded}; use hir::ChangeWithProcMacros; use ide::{Analysis, AnalysisHost, Cancellable, FileId, SourceRootId}; -use ide_db::{base_db::{Crate, ProcMacroPaths, SourceDatabase}, source_change::UserChoiceHandler}; +use ide_db::{ + base_db::{Crate, ProcMacroPaths, SourceDatabase}, + source_change::UserChoiceHandler, +}; use itertools::Itertools; use load_cargo::SourceRootConfig; use lsp_types::{SemanticTokens, Url}; @@ -538,7 +541,7 @@ impl GlobalState { proc_macros_loaded: !self.config.expand_proc_macros() || self.fetch_proc_macros_queue.last_op_result().copied().unwrap_or(false), flycheck: self.flycheck.clone(), - user_choice_handler: self.user_choice_handler.clone() + user_choice_handler: self.user_choice_handler.clone(), } } diff --git a/crates/rust-analyzer/src/lsp/utils.rs b/crates/rust-analyzer/src/lsp/utils.rs index 29fac47f2f16..5ac50ac5d744 100644 --- a/crates/rust-analyzer/src/lsp/utils.rs +++ b/crates/rust-analyzer/src/lsp/utils.rs @@ -140,7 +140,6 @@ impl GlobalState { return; }; let snap = self.snapshot(); - // TODO: handle finished choice // spawn a new task to handle the finished choice, in case of panic self.task_pool.handle.spawn(ThreadIntent::Worker, move || { let result = panic::catch_unwind(move || { From fc9f9bbb95b35f33e159f7e80dcc6fef713905df Mon Sep 17 00:00:00 2001 From: discord9 Date: Tue, 20 May 2025 20:01:45 +0800 Subject: [PATCH 6/6] chore: clippy --- crates/ide-db/src/source_change.rs | 2 +- crates/rust-analyzer/src/lsp/utils.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/ide-db/src/source_change.rs b/crates/ide-db/src/source_change.rs index 5d7b1a68dc58..bc7368721945 100644 --- a/crates/ide-db/src/source_change.rs +++ b/crates/ide-db/src/source_change.rs @@ -656,7 +656,7 @@ impl UserChoiceGroup { if question_idx < self.choice_options.len() && question_idx == self.cur_choices.len() { self.cur_choices.push(choice); } else { - return Err("Invalid index for choice group".to_string()); + return Err("Invalid index for choice group".to_owned()); } Ok(()) diff --git a/crates/rust-analyzer/src/lsp/utils.rs b/crates/rust-analyzer/src/lsp/utils.rs index 5ac50ac5d744..7a09d4f8d785 100644 --- a/crates/rust-analyzer/src/lsp/utils.rs +++ b/crates/rust-analyzer/src/lsp/utils.rs @@ -143,7 +143,7 @@ impl GlobalState { // spawn a new task to handle the finished choice, in case of panic self.task_pool.handle.spawn(ThreadIntent::Worker, move || { let result = panic::catch_unwind(move || { - let _pctx = DbPanicContext::enter("ask_for_choice".to_string()); + let _pctx = DbPanicContext::enter("ask_for_choice".to_owned()); let mut source_change_builder = SourceChangeBuilder::new(choice_group.file_id()); choice_group.finish(&mut source_change_builder); @@ -152,7 +152,7 @@ impl GlobalState { }); // it's either this or die horribly - let empty_req_id = RequestId::from("".to_string()); + let empty_req_id = RequestId::from("".to_owned()); match result { Ok(Ok(result)) => Task::Response(Response::new_ok(empty_req_id, result)), Ok(Err(_cancelled)) => Task::Response(Response {