From 05aa5b854b230c2f6ba182c0f2c94f3de9a47204 Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Thu, 23 Jan 2020 09:26:08 +0100 Subject: Remove RWLock from check watcher. @matklad mentioned this might be a good idea. So the general idea is that we don't really need the lock, as we can just clone the check watcher state when creating a snapshot. We can then use `Arc::get_mut` to get mutable access to the state from `WorldState` when needed. Running with this it seems to improve responsiveness a bit while cargo is running, but I have no hard numbers to prove it. In any case, a serialization point less is always better when we're trying to be responsive. --- crates/ra_lsp_server/src/main_loop/handlers.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'crates/ra_lsp_server/src/main_loop/handlers.rs') diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs index 8e43f0575..666f2ee29 100644 --- a/crates/ra_lsp_server/src/main_loop/handlers.rs +++ b/crates/ra_lsp_server/src/main_loop/handlers.rs @@ -674,8 +674,7 @@ pub fn handle_code_action( res.push(action.into()); } - for fix in world.check_watcher.read().fixes_for(¶ms.text_document.uri).into_iter().flatten() - { + for fix in world.check_watcher.fixes_for(¶ms.text_document.uri).into_iter().flatten() { let fix_range = fix.location.range.conv_with(&line_index); if fix_range.intersection(&range).is_none() { continue; @@ -895,7 +894,7 @@ pub fn publish_diagnostics( tags: None, }) .collect(); - if let Some(check_diags) = world.check_watcher.read().diagnostics_for(&uri) { + if let Some(check_diags) = world.check_watcher.diagnostics_for(&uri) { diagnostics.extend(check_diags.iter().cloned()); } Ok(req::PublishDiagnosticsParams { uri, diagnostics, version: None }) -- cgit v1.2.3 From 9d5a5211a4ab83fb891c6eb32f6ed6d63b6b027f Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Fri, 31 Jan 2020 13:34:44 +0100 Subject: Small cleanup --- crates/ra_lsp_server/src/main_loop/handlers.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'crates/ra_lsp_server/src/main_loop/handlers.rs') diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs index 666f2ee29..9aa1e7eea 100644 --- a/crates/ra_lsp_server/src/main_loop/handlers.rs +++ b/crates/ra_lsp_server/src/main_loop/handlers.rs @@ -1,7 +1,11 @@ //! This module is responsible for implementing handlers for Lanuage Server Protocol. //! The majority of requests are fulfilled by calling into the `ra_ide` crate. -use std::{fmt::Write as _, io::Write as _}; +use std::{ + fmt::Write as _, + io::Write as _, + process::{self, Stdio}, +}; use either::Either; use lsp_server::ErrorCode; @@ -582,21 +586,19 @@ pub fn handle_formatting( let file_line_index = world.analysis().file_line_index(file_id)?; let end_position = TextUnit::of_str(&file).conv_with(&file_line_index); - use std::process; let mut rustfmt = process::Command::new("rustfmt"); if let Some(&crate_id) = crate_ids.first() { // Assume all crates are in the same edition let edition = world.analysis().crate_edition(crate_id)?; rustfmt.args(&["--edition", &edition.to_string()]); } - rustfmt.stdin(process::Stdio::piped()).stdout(process::Stdio::piped()); if let Ok(path) = params.text_document.uri.to_file_path() { if let Some(parent) = path.parent() { rustfmt.current_dir(parent); } } - let mut rustfmt = rustfmt.spawn()?; + let mut rustfmt = rustfmt.stdin(Stdio::piped()).stdout(Stdio::piped()).spawn()?; rustfmt.stdin.as_mut().unwrap().write_all(file.as_bytes())?; -- cgit v1.2.3 From 790788d5f4013d8d92f110bc12a581d18cf4b6ae Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Fri, 31 Jan 2020 19:23:25 +0100 Subject: Rework how we send diagnostics to client. The previous way of sending from the thread pool suffered from stale diagnostics due to being canceled before we could clear the old ones. The key change is moving to sending diagnostics from the main loop thread, but doing all the hard work in the thread pool. This should provide the best of both worlds, with little to no of the downsides. This should hopefully fix a lot of issues, but we'll need testing in each individual issue to be sure. --- crates/ra_lsp_server/src/main_loop/handlers.rs | 37 ++++++-------------------- 1 file changed, 8 insertions(+), 29 deletions(-) (limited to 'crates/ra_lsp_server/src/main_loop/handlers.rs') diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs index 9aa1e7eea..839bca342 100644 --- a/crates/ra_lsp_server/src/main_loop/handlers.rs +++ b/crates/ra_lsp_server/src/main_loop/handlers.rs @@ -33,6 +33,7 @@ use crate::{ to_call_hierarchy_item, to_location, Conv, ConvWith, FoldConvCtx, MapConvWith, TryConvWith, TryConvWithToVec, }, + diagnostics::DiagnosticTask, req::{self, Decoration, InlayHint, InlayHintsParams, InlayKind}, world::WorldSnapshot, LspError, Result, @@ -656,6 +657,7 @@ pub fn handle_code_action( .filter(|(diag_range, _fix)| diag_range.intersection(&range).is_some()) .map(|(_range, fix)| fix); + // TODO: When done, we won't need this, only the one pulling from world.diagnostics for source_edit in fixes_from_diagnostics { let title = source_edit.label.clone(); let edit = source_edit.try_conv_with(&world)?; @@ -676,28 +678,12 @@ pub fn handle_code_action( res.push(action.into()); } - for fix in world.check_watcher.fixes_for(¶ms.text_document.uri).into_iter().flatten() { - let fix_range = fix.location.range.conv_with(&line_index); + for fix in world.check_fixes.get(&file_id).into_iter().flatten() { + let fix_range = fix.range.conv_with(&line_index); if fix_range.intersection(&range).is_none() { continue; } - - let edit = { - let edits = vec![TextEdit::new(fix.location.range, fix.replacement.clone())]; - let mut edit_map = std::collections::HashMap::new(); - edit_map.insert(fix.location.uri.clone(), edits); - WorkspaceEdit::new(edit_map) - }; - - let action = CodeAction { - title: fix.title.clone(), - kind: Some("quickfix".to_string()), - diagnostics: Some(fix.diagnostics.clone()), - edit: Some(edit), - command: None, - is_preferred: None, - }; - res.push(action.into()); + res.push(fix.action.clone()); } for assist in world.analysis().assists(FileRange { file_id, range })?.into_iter() { @@ -875,14 +861,10 @@ pub fn handle_document_highlight( )) } -pub fn publish_diagnostics( - world: &WorldSnapshot, - file_id: FileId, -) -> Result { +pub fn publish_diagnostics(world: &WorldSnapshot, file_id: FileId) -> Result { let _p = profile("publish_diagnostics"); - let uri = world.file_id_to_uri(file_id)?; let line_index = world.analysis().file_line_index(file_id)?; - let mut diagnostics: Vec = world + let diagnostics: Vec = world .analysis() .diagnostics(file_id)? .into_iter() @@ -896,10 +878,7 @@ pub fn publish_diagnostics( tags: None, }) .collect(); - if let Some(check_diags) = world.check_watcher.diagnostics_for(&uri) { - diagnostics.extend(check_diags.iter().cloned()); - } - Ok(req::PublishDiagnosticsParams { uri, diagnostics, version: None }) + Ok(DiagnosticTask::SetNative(file_id, diagnostics)) } pub fn publish_decorations( -- cgit v1.2.3 From cde20bf8f0a354dd1471bbb8c6e5d09cc79675d5 Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Mon, 3 Feb 2020 12:18:06 +0100 Subject: Remove stray todo --- crates/ra_lsp_server/src/main_loop/handlers.rs | 1 - 1 file changed, 1 deletion(-) (limited to 'crates/ra_lsp_server/src/main_loop/handlers.rs') diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs index 839bca342..282f6e8fc 100644 --- a/crates/ra_lsp_server/src/main_loop/handlers.rs +++ b/crates/ra_lsp_server/src/main_loop/handlers.rs @@ -657,7 +657,6 @@ pub fn handle_code_action( .filter(|(diag_range, _fix)| diag_range.intersection(&range).is_some()) .map(|(_range, fix)| fix); - // TODO: When done, we won't need this, only the one pulling from world.diagnostics for source_edit in fixes_from_diagnostics { let title = source_edit.label.clone(); let edit = source_edit.try_conv_with(&world)?; -- cgit v1.2.3 From 9769c5140c9c406a4cc880e698593a6c4bcc6826 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 9 Feb 2020 15:32:53 +0100 Subject: Simplify Assists interface Instead of building a physical tree structure, just "tag" related assists with the same group --- crates/ra_lsp_server/src/main_loop/handlers.rs | 76 ++++++++++++++++---------- 1 file changed, 48 insertions(+), 28 deletions(-) (limited to 'crates/ra_lsp_server/src/main_loop/handlers.rs') diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs index 282f6e8fc..65e8bc856 100644 --- a/crates/ra_lsp_server/src/main_loop/handlers.rs +++ b/crates/ra_lsp_server/src/main_loop/handlers.rs @@ -2,20 +2,21 @@ //! The majority of requests are fulfilled by calling into the `ra_ide` crate. use std::{ + collections::hash_map::Entry, fmt::Write as _, io::Write as _, process::{self, Stdio}, }; -use either::Either; use lsp_server::ErrorCode; use lsp_types::{ CallHierarchyIncomingCall, CallHierarchyIncomingCallsParams, CallHierarchyItem, CallHierarchyOutgoingCall, CallHierarchyOutgoingCallsParams, CallHierarchyPrepareParams, - CodeAction, CodeActionResponse, CodeLens, Command, CompletionItem, Diagnostic, - DocumentFormattingParams, DocumentHighlight, DocumentSymbol, FoldingRange, FoldingRangeParams, - Hover, HoverContents, Location, MarkupContent, MarkupKind, Position, PrepareRenameResponse, - Range, RenameParams, SymbolInformation, TextDocumentIdentifier, TextEdit, WorkspaceEdit, + CodeAction, CodeActionOrCommand, CodeActionResponse, CodeLens, Command, CompletionItem, + Diagnostic, DocumentFormattingParams, DocumentHighlight, DocumentSymbol, FoldingRange, + FoldingRangeParams, Hover, HoverContents, Location, MarkupContent, MarkupKind, Position, + PrepareRenameResponse, Range, RenameParams, SymbolInformation, TextDocumentIdentifier, + TextEdit, WorkspaceEdit, }; use ra_ide::{ AssistId, FileId, FilePosition, FileRange, Query, RangeInfo, Runnable, RunnableKind, @@ -685,34 +686,53 @@ pub fn handle_code_action( res.push(fix.action.clone()); } + let mut groups = FxHashMap::default(); for assist in world.analysis().assists(FileRange { file_id, range })?.into_iter() { - let title = assist.label.clone(); + let arg = to_value(assist.source_change.try_conv_with(&world)?)?; + + let (command, title, arg) = match assist.group_label { + None => ("rust-analyzer.applySourceChange", assist.label.clone(), arg), + + // Group all assists with the same `group_label` into a single CodeAction. + Some(group_label) => { + match groups.entry(group_label.clone()) { + Entry::Occupied(entry) => { + let idx: usize = *entry.get(); + match &mut res[idx] { + CodeActionOrCommand::CodeAction(CodeAction { + command: Some(Command { arguments: Some(arguments), .. }), + .. + }) => match arguments.as_mut_slice() { + [serde_json::Value::Array(arguments)] => arguments.push(arg), + _ => panic!("invalid group"), + }, + _ => panic!("invalid group"), + } + continue; + } + Entry::Vacant(entry) => { + entry.insert(res.len()); + } + } + ("rust-analyzer.selectAndApplySourceChange", group_label, to_value(vec![arg])?) + } + }; - let command = match assist.change_data { - Either::Left(change) => Command { - title, - command: "rust-analyzer.applySourceChange".to_string(), - arguments: Some(vec![to_value(change.try_conv_with(&world)?)?]), - }, - Either::Right(changes) => Command { - title, - command: "rust-analyzer.selectAndApplySourceChange".to_string(), - arguments: Some(vec![to_value( - changes - .into_iter() - .map(|change| change.try_conv_with(&world)) - .collect::>>()?, - )?]), - }, + let command = Command { + title: assist.label.clone(), + command: command.to_string(), + arguments: Some(vec![arg]), + }; + + let kind = match assist.id { + AssistId("introduce_variable") => Some("refactor.extract.variable".to_string()), + AssistId("add_custom_impl") => Some("refactor.rewrite.add_custom_impl".to_string()), + _ => None, }; let action = CodeAction { - title: command.title.clone(), - kind: match assist.id { - AssistId("introduce_variable") => Some("refactor.extract.variable".to_string()), - AssistId("add_custom_impl") => Some("refactor.rewrite.add_custom_impl".to_string()), - _ => None, - }, + title, + kind, diagnostics: None, edit: None, command: Some(command), -- cgit v1.2.3