diff --git a/crates/ra_cargo_watch/src/conv.rs b/crates/ra_cargo_watch/src/conv.rs index 8fba400aee1..506370535f7 100644 --- a/crates/ra_cargo_watch/src/conv.rs +++ b/crates/ra_cargo_watch/src/conv.rs @@ -1,12 +1,11 @@ //! This module provides the functionality needed to convert diagnostics from //! `cargo check` json format to the LSP diagnostic format. use cargo_metadata::diagnostic::{ - Applicability, Diagnostic as RustDiagnostic, DiagnosticLevel, DiagnosticSpan, - DiagnosticSpanMacroExpansion, + Diagnostic as RustDiagnostic, DiagnosticLevel, DiagnosticSpan, DiagnosticSpanMacroExpansion, }; use lsp_types::{ - Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity, DiagnosticTag, Location, - NumberOrString, Position, Range, Url, + CodeAction, Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity, DiagnosticTag, + Location, NumberOrString, Position, Range, TextEdit, Url, WorkspaceEdit, }; use std::{ fmt::Write, @@ -117,38 +116,9 @@ fn is_deprecated(rd: &RustDiagnostic) -> bool { } } -#[derive(Clone, Debug)] -pub struct SuggestedFix { - pub title: String, - pub location: Location, - pub replacement: String, - pub applicability: Applicability, - pub diagnostics: Vec<Diagnostic>, -} - -impl std::cmp::PartialEq<SuggestedFix> for SuggestedFix { - fn eq(&self, other: &SuggestedFix) -> bool { - if self.title == other.title - && self.location == other.location - && self.replacement == other.replacement - { - // Applicability doesn't impl PartialEq... - match (&self.applicability, &other.applicability) { - (Applicability::MachineApplicable, Applicability::MachineApplicable) => true, - (Applicability::HasPlaceholders, Applicability::HasPlaceholders) => true, - (Applicability::MaybeIncorrect, Applicability::MaybeIncorrect) => true, - (Applicability::Unspecified, Applicability::Unspecified) => true, - _ => false, - } - } else { - false - } - } -} - enum MappedRustChildDiagnostic { Related(DiagnosticRelatedInformation), - SuggestedFix(SuggestedFix), + SuggestedFix(CodeAction), MessageLine(String), } @@ -176,12 +146,20 @@ fn map_rust_child_diagnostic( rd.message.clone() }; - MappedRustChildDiagnostic::SuggestedFix(SuggestedFix { + let edit = { + let edits = vec![TextEdit::new(location.range, suggested_replacement.clone())]; + let mut edit_map = std::collections::HashMap::new(); + edit_map.insert(location.uri, edits); + WorkspaceEdit::new(edit_map) + }; + + MappedRustChildDiagnostic::SuggestedFix(CodeAction { title, - location, - replacement: suggested_replacement.clone(), - applicability: span.suggestion_applicability.clone().unwrap_or(Applicability::Unknown), - diagnostics: vec![], + kind: Some("quickfix".to_string()), + diagnostics: None, + edit: Some(edit), + command: None, + is_preferred: None, }) } else { MappedRustChildDiagnostic::Related(DiagnosticRelatedInformation { @@ -195,7 +173,7 @@ fn map_rust_child_diagnostic( pub(crate) struct MappedRustDiagnostic { pub location: Location, pub diagnostic: Diagnostic, - pub suggested_fixes: Vec<SuggestedFix>, + pub fixes: Vec<CodeAction>, } /// Converts a Rust root diagnostic to LSP form @@ -250,15 +228,13 @@ pub(crate) fn map_rust_diagnostic_to_lsp( } } - let mut suggested_fixes = vec![]; + let mut fixes = vec![]; let mut message = rd.message.clone(); for child in &rd.children { let child = map_rust_child_diagnostic(&child, workspace_root); match child { MappedRustChildDiagnostic::Related(related) => related_information.push(related), - MappedRustChildDiagnostic::SuggestedFix(suggested_fix) => { - suggested_fixes.push(suggested_fix) - } + MappedRustChildDiagnostic::SuggestedFix(code_action) => fixes.push(code_action.into()), MappedRustChildDiagnostic::MessageLine(message_line) => { write!(&mut message, "\n{}", message_line).unwrap(); @@ -295,7 +271,7 @@ pub(crate) fn map_rust_diagnostic_to_lsp( tags: if !tags.is_empty() { Some(tags) } else { None }, }; - Some(MappedRustDiagnostic { location, diagnostic, suggested_fixes }) + Some(MappedRustDiagnostic { location, diagnostic, fixes }) } /// Returns a `Url` object from a given path, will lowercase drive letters if present. diff --git a/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_clippy_pass_by_ref.snap b/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_clippy_pass_by_ref.snap index cb0920914b6..95ca163dcee 100644 --- a/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_clippy_pass_by_ref.snap +++ b/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_clippy_pass_by_ref.snap @@ -61,25 +61,39 @@ MappedRustDiagnostic { ), tags: None, }, - suggested_fixes: [ - SuggestedFix { + fixes: [ + CodeAction { title: "consider passing by value instead: \'self\'", - location: Location { - uri: "file:///test/compiler/mir/tagset.rs", - range: Range { - start: Position { - line: 41, - character: 23, - }, - end: Position { - line: 41, - character: 28, - }, + kind: Some( + "quickfix", + ), + diagnostics: None, + edit: Some( + WorkspaceEdit { + changes: Some( + { + "file:///test/compiler/mir/tagset.rs": [ + TextEdit { + range: Range { + start: Position { + line: 41, + character: 23, + }, + end: Position { + line: 41, + character: 28, + }, + }, + new_text: "self", + }, + ], + }, + ), + document_changes: None, }, - }, - replacement: "self", - applicability: Unspecified, - diagnostics: [], + ), + command: None, + is_preferred: None, }, ], } diff --git a/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_handles_macro_location.snap b/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_handles_macro_location.snap index 19510ecc1ee..12eb32df48b 100644 --- a/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_handles_macro_location.snap +++ b/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_handles_macro_location.snap @@ -42,5 +42,5 @@ MappedRustDiagnostic { related_information: None, tags: None, }, - suggested_fixes: [], + fixes: [], } diff --git a/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_macro_compiler_error.snap b/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_macro_compiler_error.snap index 92f7eec0570..7b83a7cd04a 100644 --- a/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_macro_compiler_error.snap +++ b/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_macro_compiler_error.snap @@ -57,5 +57,5 @@ MappedRustDiagnostic { ), tags: None, }, - suggested_fixes: [], + fixes: [], } diff --git a/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_rustc_incompatible_type_for_trait.snap b/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_rustc_incompatible_type_for_trait.snap index cf683e4b6f8..54679c5db49 100644 --- a/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_rustc_incompatible_type_for_trait.snap +++ b/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_rustc_incompatible_type_for_trait.snap @@ -42,5 +42,5 @@ MappedRustDiagnostic { related_information: None, tags: None, }, - suggested_fixes: [], + fixes: [], } diff --git a/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_rustc_mismatched_type.snap b/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_rustc_mismatched_type.snap index 8c1483c74b1..57df4ceaf7b 100644 --- a/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_rustc_mismatched_type.snap +++ b/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_rustc_mismatched_type.snap @@ -42,5 +42,5 @@ MappedRustDiagnostic { related_information: None, tags: None, }, - suggested_fixes: [], + fixes: [], } diff --git a/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_rustc_unused_variable.snap b/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_rustc_unused_variable.snap index eb5a2247be5..3e1fe736c05 100644 --- a/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_rustc_unused_variable.snap +++ b/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_rustc_unused_variable.snap @@ -46,25 +46,39 @@ MappedRustDiagnostic { ], ), }, - suggested_fixes: [ - SuggestedFix { + fixes: [ + CodeAction { title: "consider prefixing with an underscore: \'_foo\'", - location: Location { - uri: "file:///test/driver/subcommand/repl.rs", - range: Range { - start: Position { - line: 290, - character: 8, - }, - end: Position { - line: 290, - character: 11, - }, + kind: Some( + "quickfix", + ), + diagnostics: None, + edit: Some( + WorkspaceEdit { + changes: Some( + { + "file:///test/driver/subcommand/repl.rs": [ + TextEdit { + range: Range { + start: Position { + line: 290, + character: 8, + }, + end: Position { + line: 290, + character: 11, + }, + }, + new_text: "_foo", + }, + ], + }, + ), + document_changes: None, }, - }, - replacement: "_foo", - applicability: MachineApplicable, - diagnostics: [], + ), + command: None, + is_preferred: None, }, ], } diff --git a/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_rustc_wrong_number_of_parameters.snap b/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_rustc_wrong_number_of_parameters.snap index 2f4518931ec..69301078d99 100644 --- a/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_rustc_wrong_number_of_parameters.snap +++ b/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_rustc_wrong_number_of_parameters.snap @@ -61,5 +61,5 @@ MappedRustDiagnostic { ), tags: None, }, - suggested_fixes: [], + fixes: [], } diff --git a/crates/ra_cargo_watch/src/lib.rs b/crates/ra_cargo_watch/src/lib.rs index a718a5e520f..f07c3454996 100644 --- a/crates/ra_cargo_watch/src/lib.rs +++ b/crates/ra_cargo_watch/src/lib.rs @@ -4,22 +4,20 @@ use cargo_metadata::Message; use crossbeam_channel::{never, select, unbounded, Receiver, RecvError, Sender}; use lsp_types::{ - Diagnostic, Url, WorkDoneProgress, WorkDoneProgressBegin, WorkDoneProgressEnd, - WorkDoneProgressReport, + CodeAction, CodeActionOrCommand, Diagnostic, Url, WorkDoneProgress, WorkDoneProgressBegin, + WorkDoneProgressEnd, WorkDoneProgressReport, }; use std::{ - collections::HashMap, io::{BufRead, BufReader}, path::PathBuf, process::{Command, Stdio}, - sync::Arc, thread::JoinHandle, time::Instant, }; mod conv; -use crate::conv::{map_rust_diagnostic_to_lsp, MappedRustDiagnostic, SuggestedFix}; +use crate::conv::{map_rust_diagnostic_to_lsp, MappedRustDiagnostic}; pub use crate::conv::url_from_path_with_drive_lowercasing; @@ -38,7 +36,6 @@ pub struct CheckOptions { #[derive(Debug)] pub struct CheckWatcher { pub task_recv: Receiver<CheckTask>, - pub state: Arc<CheckState>, cmd_send: Option<Sender<CheckCommand>>, handle: Option<JoinHandle<()>>, } @@ -46,7 +43,6 @@ pub struct CheckWatcher { impl CheckWatcher { pub fn new(options: &CheckOptions, workspace_root: PathBuf) -> CheckWatcher { let options = options.clone(); - let state = Arc::new(CheckState::new()); let (task_send, task_recv) = unbounded::<CheckTask>(); let (cmd_send, cmd_recv) = unbounded::<CheckCommand>(); @@ -54,13 +50,12 @@ impl CheckWatcher { let mut check = CheckWatcherThread::new(options, workspace_root); check.run(&task_send, &cmd_recv); }); - CheckWatcher { task_recv, cmd_send: Some(cmd_send), handle: Some(handle), state } + CheckWatcher { task_recv, cmd_send: Some(cmd_send), handle: Some(handle) } } /// Returns a CheckWatcher that doesn't actually do anything pub fn dummy() -> CheckWatcher { - let state = Arc::new(CheckState::new()); - CheckWatcher { task_recv: never(), cmd_send: None, handle: None, state } + CheckWatcher { task_recv: never(), cmd_send: None, handle: None } } /// Schedule a re-start of the cargo check worker. @@ -87,84 +82,13 @@ impl std::ops::Drop for CheckWatcher { } } -#[derive(Clone, Debug)] -pub struct CheckState { - diagnostic_collection: HashMap<Url, Vec<Diagnostic>>, - suggested_fix_collection: HashMap<Url, Vec<SuggestedFix>>, -} - -impl CheckState { - fn new() -> CheckState { - CheckState { - diagnostic_collection: HashMap::new(), - suggested_fix_collection: HashMap::new(), - } - } - - /// Clear the cached diagnostics, and schedule updating diagnostics by the - /// server, to clear stale results. - pub fn clear(&mut self) -> Vec<Url> { - let cleared_files: Vec<Url> = self.diagnostic_collection.keys().cloned().collect(); - self.diagnostic_collection.clear(); - self.suggested_fix_collection.clear(); - cleared_files - } - - pub fn diagnostics_for(&self, uri: &Url) -> Option<&[Diagnostic]> { - self.diagnostic_collection.get(uri).map(|d| d.as_slice()) - } - - pub fn fixes_for(&self, uri: &Url) -> Option<&[SuggestedFix]> { - self.suggested_fix_collection.get(uri).map(|d| d.as_slice()) - } - - pub fn add_diagnostic_with_fixes(&mut self, file_uri: Url, diagnostic: DiagnosticWithFixes) { - for fix in diagnostic.suggested_fixes { - self.add_suggested_fix_for_diagnostic(fix, &diagnostic.diagnostic); - } - self.add_diagnostic(file_uri, diagnostic.diagnostic); - } - - fn add_diagnostic(&mut self, file_uri: Url, diagnostic: Diagnostic) { - let diagnostics = self.diagnostic_collection.entry(file_uri).or_default(); - - // If we're building multiple targets it's possible we've already seen this diagnostic - let is_duplicate = diagnostics.iter().any(|d| are_diagnostics_equal(d, &diagnostic)); - if is_duplicate { - return; - } - - diagnostics.push(diagnostic); - } - - fn add_suggested_fix_for_diagnostic( - &mut self, - mut suggested_fix: SuggestedFix, - diagnostic: &Diagnostic, - ) { - let file_uri = suggested_fix.location.uri.clone(); - let file_suggestions = self.suggested_fix_collection.entry(file_uri).or_default(); - - let existing_suggestion: Option<&mut SuggestedFix> = - file_suggestions.iter_mut().find(|s| s == &&suggested_fix); - if let Some(existing_suggestion) = existing_suggestion { - // The existing suggestion also applies to this new diagnostic - existing_suggestion.diagnostics.push(diagnostic.clone()); - } else { - // We haven't seen this suggestion before - suggested_fix.diagnostics.push(diagnostic.clone()); - file_suggestions.push(suggested_fix); - } - } -} - #[derive(Debug)] pub enum CheckTask { /// Request a clearing of all cached diagnostics from the check watcher ClearDiagnostics, /// Request adding a diagnostic with fixes included to a file - AddDiagnostic(Url, DiagnosticWithFixes), + AddDiagnostic { url: Url, diagnostic: Diagnostic, fixes: Vec<CodeActionOrCommand> }, /// Request check progress notification to client Status(WorkDoneProgress), @@ -279,10 +203,17 @@ impl CheckWatcherThread { None => return, }; - let MappedRustDiagnostic { location, diagnostic, suggested_fixes } = map_result; + let MappedRustDiagnostic { location, diagnostic, fixes } = map_result; + let fixes = fixes + .into_iter() + .map(|fix| { + CodeAction { diagnostics: Some(vec![diagnostic.clone()]), ..fix }.into() + }) + .collect(); - let diagnostic = DiagnosticWithFixes { diagnostic, suggested_fixes }; - task_send.send(CheckTask::AddDiagnostic(location.uri, diagnostic)).unwrap(); + task_send + .send(CheckTask::AddDiagnostic { url: location.uri, diagnostic, fixes }) + .unwrap(); } CheckEvent::Msg(Message::BuildScriptExecuted(_msg)) => {} @@ -294,7 +225,7 @@ impl CheckWatcherThread { #[derive(Debug)] pub struct DiagnosticWithFixes { diagnostic: Diagnostic, - suggested_fixes: Vec<SuggestedFix>, + fixes: Vec<CodeAction>, } /// WatchThread exists to wrap around the communication needed to be able to @@ -429,10 +360,3 @@ impl std::ops::Drop for WatchThread { } } } - -fn are_diagnostics_equal(left: &Diagnostic, right: &Diagnostic) -> bool { - left.source == right.source - && left.severity == right.severity - && left.range == right.range - && left.message == right.message -} diff --git a/crates/ra_lsp_server/src/diagnostics.rs b/crates/ra_lsp_server/src/diagnostics.rs new file mode 100644 index 00000000000..ea08bce24bc --- /dev/null +++ b/crates/ra_lsp_server/src/diagnostics.rs @@ -0,0 +1,85 @@ +//! Book keeping for keeping diagnostics easily in sync with the client. +use lsp_types::{CodeActionOrCommand, Diagnostic, Range}; +use ra_ide::FileId; +use std::{collections::HashMap, sync::Arc}; + +pub type CheckFixes = Arc<HashMap<FileId, Vec<Fix>>>; + +#[derive(Debug, Default, Clone)] +pub struct DiagnosticCollection { + pub native: HashMap<FileId, Vec<Diagnostic>>, + pub check: HashMap<FileId, Vec<Diagnostic>>, + pub check_fixes: CheckFixes, +} + +#[derive(Debug, Clone)] +pub struct Fix { + pub range: Range, + pub action: CodeActionOrCommand, +} + +#[derive(Debug)] +pub enum DiagnosticTask { + ClearCheck, + AddCheck(FileId, Diagnostic, Vec<CodeActionOrCommand>), + SetNative(FileId, Vec<Diagnostic>), +} + +impl DiagnosticCollection { + pub fn clear_check(&mut self) -> Vec<FileId> { + Arc::make_mut(&mut self.check_fixes).clear(); + self.check.drain().map(|(key, _value)| key).collect() + } + + pub fn add_check_diagnostic( + &mut self, + file_id: FileId, + diagnostic: Diagnostic, + fixes: Vec<CodeActionOrCommand>, + ) { + let diagnostics = self.check.entry(file_id).or_default(); + for existing_diagnostic in diagnostics.iter() { + if are_diagnostics_equal(&existing_diagnostic, &diagnostic) { + return; + } + } + + let check_fixes = Arc::make_mut(&mut self.check_fixes); + check_fixes + .entry(file_id) + .or_default() + .extend(fixes.into_iter().map(|action| Fix { range: diagnostic.range, action })); + diagnostics.push(diagnostic); + } + + pub fn set_native_diagnostics(&mut self, file_id: FileId, diagnostics: Vec<Diagnostic>) { + self.native.insert(file_id, diagnostics); + } + + pub fn diagnostics_for(&self, file_id: FileId) -> impl Iterator<Item = &Diagnostic> { + let native = self.native.get(&file_id).into_iter().flatten(); + let check = self.check.get(&file_id).into_iter().flatten(); + native.chain(check) + } + + pub fn handle_task(&mut self, task: DiagnosticTask) -> Vec<FileId> { + match task { + DiagnosticTask::ClearCheck => self.clear_check(), + DiagnosticTask::AddCheck(file_id, diagnostic, fixes) => { + self.add_check_diagnostic(file_id, diagnostic, fixes); + vec![file_id] + } + DiagnosticTask::SetNative(file_id, diagnostics) => { + self.set_native_diagnostics(file_id, diagnostics); + vec![file_id] + } + } + } +} + +fn are_diagnostics_equal(left: &Diagnostic, right: &Diagnostic) -> bool { + left.source == right.source + && left.severity == right.severity + && left.range == right.range + && left.message == right.message +} diff --git a/crates/ra_lsp_server/src/lib.rs b/crates/ra_lsp_server/src/lib.rs index 2ca149fd56b..1208c13435b 100644 --- a/crates/ra_lsp_server/src/lib.rs +++ b/crates/ra_lsp_server/src/lib.rs @@ -29,6 +29,7 @@ mod markdown; pub mod req; mod config; mod world; +mod diagnostics; pub type Result<T> = std::result::Result<T, Box<dyn std::error::Error + Send + Sync>>; pub use crate::{ diff --git a/crates/ra_lsp_server/src/main_loop.rs b/crates/ra_lsp_server/src/main_loop.rs index 508fe08c034..12961ba377f 100644 --- a/crates/ra_lsp_server/src/main_loop.rs +++ b/crates/ra_lsp_server/src/main_loop.rs @@ -17,16 +17,17 @@ use std::{ use crossbeam_channel::{select, unbounded, RecvError, Sender}; use lsp_server::{Connection, ErrorCode, Message, Notification, Request, RequestId, Response}; use lsp_types::{ClientCapabilities, NumberOrString}; -use ra_cargo_watch::{CheckOptions, CheckTask}; +use ra_cargo_watch::{url_from_path_with_drive_lowercasing, CheckOptions, CheckTask}; use ra_ide::{Canceled, FeatureFlags, FileId, LibraryData, SourceRootId}; use ra_prof::profile; -use ra_vfs::{VfsTask, Watch}; +use ra_vfs::{VfsFile, VfsTask, Watch}; use relative_path::RelativePathBuf; use rustc_hash::FxHashSet; use serde::{de::DeserializeOwned, Serialize}; use threadpool::ThreadPool; use crate::{ + diagnostics::DiagnosticTask, main_loop::{ pending_requests::{PendingRequest, PendingRequests}, subscriptions::Subscriptions, @@ -254,6 +255,7 @@ pub fn main_loop( enum Task { Respond(Response), Notify(Notification), + Diagnostic(DiagnosticTask), } enum Event { @@ -359,7 +361,7 @@ fn loop_turn( world_state.maybe_collect_garbage(); loop_state.in_flight_libraries -= 1; } - Event::CheckWatcher(task) => on_check_task(pool, task, world_state, task_sender)?, + Event::CheckWatcher(task) => on_check_task(task, world_state, task_sender)?, Event::Msg(msg) => match msg { Message::Request(req) => on_request( world_state, @@ -464,6 +466,7 @@ fn on_task( Task::Notify(n) => { msg_sender.send(n.into()).unwrap(); } + Task::Diagnostic(task) => on_diagnostic_task(task, msg_sender, state), } } @@ -621,23 +624,26 @@ fn on_notification( } fn on_check_task( - pool: &ThreadPool, task: CheckTask, world_state: &mut WorldState, task_sender: &Sender<Task>, ) -> Result<()> { - let urls = match task { + match task { CheckTask::ClearDiagnostics => { - let state = Arc::get_mut(&mut world_state.check_watcher.state) - .expect("couldn't get check watcher state as mutable"); - state.clear() + task_sender.send(Task::Diagnostic(DiagnosticTask::ClearCheck))?; } - CheckTask::AddDiagnostic(url, diagnostic) => { - let state = Arc::get_mut(&mut world_state.check_watcher.state) - .expect("couldn't get check watcher state as mutable"); - state.add_diagnostic_with_fixes(url.clone(), diagnostic); - vec![url] + CheckTask::AddDiagnostic { url, diagnostic, fixes } => { + let path = url.to_file_path().map_err(|()| format!("invalid uri: {}", url))?; + let file_id = world_state + .vfs + .read() + .path2file(&path) + .map(|it| FileId(it.0)) + .ok_or_else(|| format!("unknown file: {}", path.to_string_lossy()))?; + + task_sender + .send(Task::Diagnostic(DiagnosticTask::AddCheck(file_id, diagnostic, fixes)))?; } CheckTask::Status(progress) => { @@ -647,33 +653,32 @@ fn on_check_task( }; let not = notification_new::<req::Progress>(params); task_sender.send(Task::Notify(not)).unwrap(); - Vec::new() } }; - let subscriptions = urls - .into_iter() - .map(|url| { - let path = url.to_file_path().map_err(|()| format!("invalid uri: {}", url))?; - Ok(world_state.vfs.read().path2file(&path).map(|it| FileId(it.0))) - }) - .filter_map(|res| res.transpose()) - .collect::<Result<Vec<_>>>()?; - - // We manually send a diagnostic update when the watcher asks - // us to, to avoid the issue of having to change the file to - // receive updated diagnostics. - update_file_notifications_on_threadpool( - pool, - world_state.snapshot(), - false, - task_sender.clone(), - subscriptions, - ); - Ok(()) } +fn on_diagnostic_task(task: DiagnosticTask, msg_sender: &Sender<Message>, state: &mut WorldState) { + let subscriptions = state.diagnostics.handle_task(task); + + for file_id in subscriptions { + let path = state.vfs.read().file2path(VfsFile(file_id.0)); + let uri = match url_from_path_with_drive_lowercasing(&path) { + Ok(uri) => uri, + Err(err) => { + log::error!("Couldn't convert path to url ({}): {:?}", err, path.to_string_lossy()); + continue; + } + }; + + let diagnostics = state.diagnostics.diagnostics_for(file_id).cloned().collect(); + let params = req::PublishDiagnosticsParams { uri, diagnostics, version: None }; + let not = notification_new::<req::PublishDiagnostics>(params); + msg_sender.send(not.into()).unwrap(); + } +} + struct PoolDispatcher<'a> { req: Option<Request>, pool: &'a ThreadPool, @@ -819,9 +824,8 @@ fn update_file_notifications_on_threadpool( log::error!("failed to compute diagnostics: {:?}", e); } } - Ok(params) => { - let not = notification_new::<req::PublishDiagnostics>(params); - task_sender.send(Task::Notify(not)).unwrap(); + Ok(task) => { + task_sender.send(Task::Diagnostic(task)).unwrap(); } } } diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs index 9aa1e7eea27..282f6e8fcd0 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, @@ -676,28 +677,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 +860,10 @@ pub fn handle_document_highlight( )) } -pub fn publish_diagnostics( - world: &WorldSnapshot, - file_id: FileId, -) -> Result<req::PublishDiagnosticsParams> { +pub fn publish_diagnostics(world: &WorldSnapshot, file_id: FileId) -> Result<DiagnosticTask> { 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<Diagnostic> = world + let diagnostics: Vec<Diagnostic> = world .analysis() .diagnostics(file_id)? .into_iter() @@ -896,10 +877,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( diff --git a/crates/ra_lsp_server/src/world.rs b/crates/ra_lsp_server/src/world.rs index 3059ef9ec62..1ee02b47c93 100644 --- a/crates/ra_lsp_server/src/world.rs +++ b/crates/ra_lsp_server/src/world.rs @@ -12,9 +12,7 @@ use crossbeam_channel::{unbounded, Receiver}; use lsp_server::ErrorCode; use lsp_types::Url; use parking_lot::RwLock; -use ra_cargo_watch::{ - url_from_path_with_drive_lowercasing, CheckOptions, CheckState, CheckWatcher, -}; +use ra_cargo_watch::{url_from_path_with_drive_lowercasing, CheckOptions, CheckWatcher}; use ra_ide::{ Analysis, AnalysisChange, AnalysisHost, CrateGraph, FeatureFlags, FileId, LibraryData, SourceRootId, @@ -25,6 +23,7 @@ use ra_vfs_glob::{Glob, RustPackageFilterBuilder}; use relative_path::RelativePathBuf; use crate::{ + diagnostics::{CheckFixes, DiagnosticCollection}, main_loop::pending_requests::{CompletedRequest, LatestRequests}, LspError, Result, }; @@ -55,6 +54,7 @@ pub struct WorldState { pub task_receiver: Receiver<VfsTask>, pub latest_requests: Arc<RwLock<LatestRequests>>, pub check_watcher: CheckWatcher, + pub diagnostics: DiagnosticCollection, } /// An immutable snapshot of the world's state at a point in time. @@ -63,7 +63,7 @@ pub struct WorldSnapshot { pub workspaces: Arc<Vec<ProjectWorkspace>>, pub analysis: Analysis, pub latest_requests: Arc<RwLock<LatestRequests>>, - pub check_watcher: CheckState, + pub check_fixes: CheckFixes, vfs: Arc<RwLock<Vfs>>, } @@ -159,6 +159,7 @@ impl WorldState { task_receiver, latest_requests: Default::default(), check_watcher, + diagnostics: Default::default(), } } @@ -220,7 +221,7 @@ impl WorldState { analysis: self.analysis_host.analysis(), vfs: Arc::clone(&self.vfs), latest_requests: Arc::clone(&self.latest_requests), - check_watcher: (*self.check_watcher.state).clone(), + check_fixes: Arc::clone(&self.diagnostics.check_fixes), } }