From 66e8ef53a0ed018d03340577a0443030a193f773 Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Wed, 25 Dec 2019 12:21:38 +0100 Subject: [PATCH 01/20] Initial implementation of cargo check watching --- Cargo.lock | 1 + crates/ra_lsp_server/Cargo.toml | 1 + crates/ra_lsp_server/src/caps.rs | 4 +- crates/ra_lsp_server/src/cargo_check.rs | 533 ++++++++++++++++++ crates/ra_lsp_server/src/lib.rs | 1 + crates/ra_lsp_server/src/main_loop.rs | 27 +- .../ra_lsp_server/src/main_loop/handlers.rs | 28 +- crates/ra_lsp_server/src/world.rs | 8 + 8 files changed, 599 insertions(+), 4 deletions(-) create mode 100644 crates/ra_lsp_server/src/cargo_check.rs diff --git a/Cargo.lock b/Cargo.lock index 792e30494a..77c10a10e3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1051,6 +1051,7 @@ dependencies = [ name = "ra_lsp_server" version = "0.1.0" dependencies = [ + "cargo_metadata 0.9.1 (registry+https://github.com/rust-lang/crates.io-index)", "crossbeam-channel 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "jod-thread 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/crates/ra_lsp_server/Cargo.toml b/crates/ra_lsp_server/Cargo.toml index 030e9033ce..aa1acdc330 100644 --- a/crates/ra_lsp_server/Cargo.toml +++ b/crates/ra_lsp_server/Cargo.toml @@ -27,6 +27,7 @@ ra_project_model = { path = "../ra_project_model" } ra_prof = { path = "../ra_prof" } ra_vfs_glob = { path = "../ra_vfs_glob" } env_logger = { version = "0.7.1", default-features = false, features = ["humantime"] } +cargo_metadata = "0.9.1" [dev-dependencies] tempfile = "3" diff --git a/crates/ra_lsp_server/src/caps.rs b/crates/ra_lsp_server/src/caps.rs index ceb4c4259d..0f84e7a34e 100644 --- a/crates/ra_lsp_server/src/caps.rs +++ b/crates/ra_lsp_server/src/caps.rs @@ -6,7 +6,7 @@ use lsp_types::{ ImplementationProviderCapability, RenameOptions, RenameProviderCapability, SelectionRangeProviderCapability, ServerCapabilities, SignatureHelpOptions, TextDocumentSyncCapability, TextDocumentSyncKind, TextDocumentSyncOptions, - TypeDefinitionProviderCapability, WorkDoneProgressOptions, + TypeDefinitionProviderCapability, WorkDoneProgressOptions, SaveOptions }; pub fn server_capabilities() -> ServerCapabilities { @@ -16,7 +16,7 @@ pub fn server_capabilities() -> ServerCapabilities { change: Some(TextDocumentSyncKind::Full), will_save: None, will_save_wait_until: None, - save: None, + save: Some(SaveOptions::default()), })), hover_provider: Some(true), completion_provider: Some(CompletionOptions { diff --git a/crates/ra_lsp_server/src/cargo_check.rs b/crates/ra_lsp_server/src/cargo_check.rs new file mode 100644 index 0000000000..d5ff021541 --- /dev/null +++ b/crates/ra_lsp_server/src/cargo_check.rs @@ -0,0 +1,533 @@ +use cargo_metadata::{ + diagnostic::{ + Applicability, Diagnostic as RustDiagnostic, DiagnosticLevel, DiagnosticSpan, + DiagnosticSpanMacroExpansion, + }, + Message, +}; +use crossbeam_channel::{select, unbounded, Receiver, RecvError, Sender, TryRecvError}; +use lsp_types::{ + Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity, DiagnosticTag, Location, + NumberOrString, Position, Range, Url, +}; +use parking_lot::RwLock; +use std::{ + collections::HashMap, + fmt::Write, + path::PathBuf, + process::{Command, Stdio}, + sync::Arc, + thread::JoinHandle, + time::Instant, +}; + +#[derive(Debug)] +pub struct CheckWatcher { + pub task_recv: Receiver, + pub cmd_send: Sender, + pub shared: Arc>, + handle: JoinHandle<()>, +} + +impl CheckWatcher { + pub fn new(workspace_root: PathBuf) -> CheckWatcher { + let shared = Arc::new(RwLock::new(CheckWatcherSharedState::new())); + + let (task_send, task_recv) = unbounded::(); + let (cmd_send, cmd_recv) = unbounded::(); + let shared_ = shared.clone(); + let handle = std::thread::spawn(move || { + let mut check = CheckWatcherState::new(shared_, workspace_root); + check.run(&task_send, &cmd_recv); + }); + + CheckWatcher { task_recv, cmd_send, handle, shared } + } + + pub fn update(&self) { + self.cmd_send.send(CheckCommand::Update).unwrap(); + } +} + +pub struct CheckWatcherState { + workspace_root: PathBuf, + running: bool, + watcher: WatchThread, + last_update_req: Option, + shared: Arc>, +} + +#[derive(Debug)] +pub struct CheckWatcherSharedState { + diagnostic_collection: HashMap>, + suggested_fix_collection: HashMap>, +} + +impl CheckWatcherSharedState { + fn new() -> CheckWatcherSharedState { + CheckWatcherSharedState { + diagnostic_collection: HashMap::new(), + suggested_fix_collection: HashMap::new(), + } + } + + pub fn clear(&mut self, task_send: &Sender) { + let cleared_files: Vec = self.diagnostic_collection.keys().cloned().collect(); + + self.diagnostic_collection.clear(); + self.suggested_fix_collection.clear(); + + for uri in cleared_files { + task_send.send(CheckTask::Update(uri.clone())).unwrap(); + } + } + + 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()) + } + + 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 { + Update(Url), +} + +pub enum CheckCommand { + Update, +} + +impl CheckWatcherState { + pub fn new( + shared: Arc>, + workspace_root: PathBuf, + ) -> CheckWatcherState { + let watcher = WatchThread::new(&workspace_root); + CheckWatcherState { workspace_root, running: false, watcher, last_update_req: None, shared } + } + + pub fn run(&mut self, task_send: &Sender, cmd_recv: &Receiver) { + self.running = true; + while self.running { + select! { + recv(&cmd_recv) -> cmd => match cmd { + Ok(cmd) => self.handle_command(cmd), + Err(RecvError) => { + // Command channel has closed, so shut down + self.running = false; + }, + }, + recv(self.watcher.message_recv) -> msg => match msg { + Ok(msg) => self.handle_message(msg, task_send), + Err(RecvError) => {}, + } + }; + + if self.should_recheck() { + self.last_update_req.take(); + self.shared.write().clear(task_send); + + self.watcher.cancel(); + self.watcher = WatchThread::new(&self.workspace_root); + } + } + } + + fn should_recheck(&mut self) -> bool { + if let Some(_last_update_req) = &self.last_update_req { + // We currently only request an update on save, as we need up to + // date source on disk for cargo check to do it's magic, so we + // don't really need to debounce the requests at this point. + return true; + } + false + } + + fn handle_command(&mut self, cmd: CheckCommand) { + match cmd { + CheckCommand::Update => self.last_update_req = Some(Instant::now()), + } + } + + fn handle_message(&mut self, msg: cargo_metadata::Message, task_send: &Sender) { + match msg { + Message::CompilerArtifact(_msg) => { + // TODO: Status display + } + + Message::CompilerMessage(msg) => { + let map_result = + match map_rust_diagnostic_to_lsp(&msg.message, &self.workspace_root) { + Some(map_result) => map_result, + None => return, + }; + + let MappedRustDiagnostic { location, diagnostic, suggested_fixes } = map_result; + let file_uri = location.uri.clone(); + + if !suggested_fixes.is_empty() { + for suggested_fix in suggested_fixes { + self.shared + .write() + .add_suggested_fix_for_diagnostic(suggested_fix, &diagnostic); + } + } + self.shared.write().add_diagnostic(file_uri, diagnostic); + + task_send.send(CheckTask::Update(location.uri)).unwrap(); + } + + Message::BuildScriptExecuted(_msg) => {} + Message::Unknown => {} + } + } +} + +/// WatchThread exists to wrap around the communication needed to be able to +/// run `cargo check` without blocking. Currently the Rust standard library +/// doesn't provide a way to read sub-process output without blocking, so we +/// have to wrap sub-processes output handling in a thread and pass messages +/// back over a channel. +struct WatchThread { + message_recv: Receiver, + cancel_send: Sender<()>, +} + +impl WatchThread { + fn new(workspace_root: &PathBuf) -> WatchThread { + let manifest_path = format!("{}/Cargo.toml", workspace_root.to_string_lossy()); + let (message_send, message_recv) = unbounded(); + let (cancel_send, cancel_recv) = unbounded(); + std::thread::spawn(move || { + let mut command = Command::new("cargo") + .args(&["check", "--message-format=json", "--manifest-path", &manifest_path]) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .spawn() + .expect("couldn't launch cargo"); + + for message in cargo_metadata::parse_messages(command.stdout.take().unwrap()) { + match cancel_recv.try_recv() { + Ok(()) | Err(TryRecvError::Disconnected) => { + command.kill().expect("couldn't kill command"); + } + Err(TryRecvError::Empty) => (), + } + + message_send.send(message.unwrap()).unwrap(); + } + }); + WatchThread { message_recv, cancel_send } + } + + fn cancel(&self) { + let _ = self.cancel_send.send(()); + } +} + +/// Converts a Rust level string to a LSP severity +fn map_level_to_severity(val: DiagnosticLevel) -> Option { + match val { + DiagnosticLevel::Ice => Some(DiagnosticSeverity::Error), + DiagnosticLevel::Error => Some(DiagnosticSeverity::Error), + DiagnosticLevel::Warning => Some(DiagnosticSeverity::Warning), + DiagnosticLevel::Note => Some(DiagnosticSeverity::Information), + DiagnosticLevel::Help => Some(DiagnosticSeverity::Hint), + DiagnosticLevel::Unknown => None, + } +} + +/// Check whether a file name is from macro invocation +fn is_from_macro(file_name: &str) -> bool { + file_name.starts_with('<') && file_name.ends_with('>') +} + +/// Converts a Rust macro span to a LSP location recursively +fn map_macro_span_to_location( + span_macro: &DiagnosticSpanMacroExpansion, + workspace_root: &PathBuf, +) -> Option { + if !is_from_macro(&span_macro.span.file_name) { + return Some(map_span_to_location(&span_macro.span, workspace_root)); + } + + if let Some(expansion) = &span_macro.span.expansion { + return map_macro_span_to_location(&expansion, workspace_root); + } + + None +} + +/// Converts a Rust span to a LSP location +fn map_span_to_location(span: &DiagnosticSpan, workspace_root: &PathBuf) -> Location { + if is_from_macro(&span.file_name) && span.expansion.is_some() { + let expansion = span.expansion.as_ref().unwrap(); + if let Some(macro_range) = map_macro_span_to_location(&expansion, workspace_root) { + return macro_range; + } + } + + let mut file_name = workspace_root.clone(); + file_name.push(&span.file_name); + let uri = Url::from_file_path(file_name).unwrap(); + + let range = Range::new( + Position::new(span.line_start as u64 - 1, span.column_start as u64 - 1), + Position::new(span.line_end as u64 - 1, span.column_end as u64 - 1), + ); + + Location { uri, range } +} + +/// Converts a secondary Rust span to a LSP related information +/// +/// If the span is unlabelled this will return `None`. +fn map_secondary_span_to_related( + span: &DiagnosticSpan, + workspace_root: &PathBuf, +) -> Option { + if let Some(label) = &span.label { + let location = map_span_to_location(span, workspace_root); + Some(DiagnosticRelatedInformation { location, message: label.clone() }) + } else { + // Nothing to label this with + None + } +} + +/// Determines if diagnostic is related to unused code +fn is_unused_or_unnecessary(rd: &RustDiagnostic) -> bool { + if let Some(code) = &rd.code { + match code.code.as_str() { + "dead_code" | "unknown_lints" | "unreachable_code" | "unused_attributes" + | "unused_imports" | "unused_macros" | "unused_variables" => true, + _ => false, + } + } else { + false + } +} + +/// Determines if diagnostic is related to deprecated code +fn is_deprecated(rd: &RustDiagnostic) -> bool { + if let Some(code) = &rd.code { + match code.code.as_str() { + "deprecated" => true, + _ => false, + } + } else { + false + } +} + +#[derive(Debug)] +pub struct SuggestedFix { + pub title: String, + pub location: Location, + pub replacement: String, + pub applicability: Applicability, + pub diagnostics: Vec, +} + +impl std::cmp::PartialEq 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), + MessageLine(String), +} + +fn map_rust_child_diagnostic( + rd: &RustDiagnostic, + workspace_root: &PathBuf, +) -> MappedRustChildDiagnostic { + let span: &DiagnosticSpan = match rd.spans.iter().find(|s| s.is_primary) { + Some(span) => span, + None => { + // `rustc` uses these spanless children as a way to print multi-line + // messages + return MappedRustChildDiagnostic::MessageLine(rd.message.clone()); + } + }; + + // If we have a primary span use its location, otherwise use the parent + let location = map_span_to_location(&span, workspace_root); + + if let Some(suggested_replacement) = &span.suggested_replacement { + // Include our replacement in the title unless it's empty + let title = if !suggested_replacement.is_empty() { + format!("{}: '{}'", rd.message, suggested_replacement) + } else { + rd.message.clone() + }; + + MappedRustChildDiagnostic::SuggestedFix(SuggestedFix { + title, + location, + replacement: suggested_replacement.clone(), + applicability: span.suggestion_applicability.clone().unwrap_or(Applicability::Unknown), + diagnostics: vec![], + }) + } else { + MappedRustChildDiagnostic::Related(DiagnosticRelatedInformation { + location, + message: rd.message.clone(), + }) + } +} + +struct MappedRustDiagnostic { + location: Location, + diagnostic: Diagnostic, + suggested_fixes: Vec, +} + +/// Converts a Rust root diagnostic to LSP form +/// +/// This flattens the Rust diagnostic by: +/// +/// 1. Creating a LSP diagnostic with the root message and primary span. +/// 2. Adding any labelled secondary spans to `relatedInformation` +/// 3. Categorising child diagnostics as either `SuggestedFix`es, +/// `relatedInformation` or additional message lines. +/// +/// If the diagnostic has no primary span this will return `None` +fn map_rust_diagnostic_to_lsp( + rd: &RustDiagnostic, + workspace_root: &PathBuf, +) -> Option { + let primary_span = rd.spans.iter().find(|s| s.is_primary)?; + + let location = map_span_to_location(&primary_span, workspace_root); + + let severity = map_level_to_severity(rd.level); + let mut primary_span_label = primary_span.label.as_ref(); + + let mut source = String::from("rustc"); + let mut code = rd.code.as_ref().map(|c| c.code.clone()); + if let Some(code_val) = &code { + // See if this is an RFC #2103 scoped lint (e.g. from Clippy) + let scoped_code: Vec<&str> = code_val.split("::").collect(); + if scoped_code.len() == 2 { + source = String::from(scoped_code[0]); + code = Some(String::from(scoped_code[1])); + } + } + + let mut related_information = vec![]; + let mut tags = vec![]; + + for secondary_span in rd.spans.iter().filter(|s| !s.is_primary) { + let related = map_secondary_span_to_related(secondary_span, workspace_root); + if let Some(related) = related { + related_information.push(related); + } + } + + let mut suggested_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::MessageLine(message_line) => { + write!(&mut message, "\n{}", message_line).unwrap(); + + // These secondary messages usually duplicate the content of the + // primary span label. + primary_span_label = None; + } + } + } + + if let Some(primary_span_label) = primary_span_label { + write!(&mut message, "\n{}", primary_span_label).unwrap(); + } + + if is_unused_or_unnecessary(rd) { + tags.push(DiagnosticTag::Unnecessary); + } + + if is_deprecated(rd) { + tags.push(DiagnosticTag::Deprecated); + } + + let diagnostic = Diagnostic { + range: location.range, + severity, + code: code.map(NumberOrString::String), + source: Some(source), + message: rd.message.clone(), + related_information: if !related_information.is_empty() { + Some(related_information) + } else { + None + }, + tags: if !tags.is_empty() { Some(tags) } else { None }, + }; + + Some(MappedRustDiagnostic { location, diagnostic, suggested_fixes }) +} + +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 2ca149fd56..2811231fac 100644 --- a/crates/ra_lsp_server/src/lib.rs +++ b/crates/ra_lsp_server/src/lib.rs @@ -22,6 +22,7 @@ macro_rules! print { } mod caps; +mod cargo_check; mod cargo_target_spec; mod conv; mod main_loop; diff --git a/crates/ra_lsp_server/src/main_loop.rs b/crates/ra_lsp_server/src/main_loop.rs index dda318e43e..943d38943f 100644 --- a/crates/ra_lsp_server/src/main_loop.rs +++ b/crates/ra_lsp_server/src/main_loop.rs @@ -19,6 +19,7 @@ use serde::{de::DeserializeOwned, Serialize}; use threadpool::ThreadPool; use crate::{ + cargo_check::CheckTask, main_loop::{ pending_requests::{PendingRequest, PendingRequests}, subscriptions::Subscriptions, @@ -176,7 +177,8 @@ pub fn main_loop( Ok(task) => Event::Vfs(task), Err(RecvError) => Err("vfs died")?, }, - recv(libdata_receiver) -> data => Event::Lib(data.unwrap()) + recv(libdata_receiver) -> data => Event::Lib(data.unwrap()), + recv(world_state.check_watcher.task_recv) -> task => Event::CheckWatcher(task.unwrap()) }; if let Event::Msg(Message::Request(req)) = &event { if connection.handle_shutdown(&req)? { @@ -222,6 +224,7 @@ enum Event { Task(Task), Vfs(VfsTask), Lib(LibraryData), + CheckWatcher(CheckTask), } impl fmt::Debug for Event { @@ -259,6 +262,7 @@ impl fmt::Debug for Event { Event::Task(it) => fmt::Debug::fmt(it, f), Event::Vfs(it) => fmt::Debug::fmt(it, f), Event::Lib(it) => fmt::Debug::fmt(it, f), + Event::CheckWatcher(it) => fmt::Debug::fmt(it, f), } } } @@ -318,6 +322,20 @@ fn loop_turn( world_state.maybe_collect_garbage(); loop_state.in_flight_libraries -= 1; } + Event::CheckWatcher(task) => match task { + CheckTask::Update(uri) => { + // 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. + let path = uri.to_file_path().map_err(|()| format!("invalid uri: {}", uri))?; + if let Some(file_id) = world_state.vfs.read().path2file(&path) { + let params = + handlers::publish_diagnostics(&world_state.snapshot(), FileId(file_id.0))?; + let not = notification_new::(params); + task_sender.send(Task::Notify(not)).unwrap(); + } + } + }, Event::Msg(msg) => match msg { Message::Request(req) => on_request( world_state, @@ -517,6 +535,13 @@ fn on_notification( } Err(not) => not, }; + let not = match notification_cast::(not) { + Ok(_params) => { + state.check_watcher.update(); + return Ok(()); + } + Err(not) => not, + }; let not = match notification_cast::(not) { Ok(params) => { let uri = params.text_document.uri; diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs index 39eb3df3e5..331beab134 100644 --- a/crates/ra_lsp_server/src/main_loop/handlers.rs +++ b/crates/ra_lsp_server/src/main_loop/handlers.rs @@ -654,6 +654,29 @@ 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() + { + let fix_range = fix.location.range.conv_with(&line_index); + if fix_range.intersection(&range).is_none() { + continue; + } + + 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); + let edit = 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()); + } + for assist in assists { let title = assist.change.label.clone(); let edit = assist.change.try_conv_with(&world)?; @@ -820,7 +843,7 @@ pub fn publish_diagnostics( 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 diagnostics = world + let mut diagnostics: Vec = world .analysis() .diagnostics(file_id)? .into_iter() @@ -834,6 +857,9 @@ pub fn publish_diagnostics( tags: None, }) .collect(); + if let Some(check_diags) = world.check_watcher.read().diagnostics_for(&uri) { + diagnostics.extend(check_diags.iter().cloned()); + } Ok(req::PublishDiagnosticsParams { uri, diagnostics, version: None }) } diff --git a/crates/ra_lsp_server/src/world.rs b/crates/ra_lsp_server/src/world.rs index 79431e7e6f..8e9380ca07 100644 --- a/crates/ra_lsp_server/src/world.rs +++ b/crates/ra_lsp_server/src/world.rs @@ -24,6 +24,7 @@ use std::path::{Component, Prefix}; use crate::{ main_loop::pending_requests::{CompletedRequest, LatestRequests}, + cargo_check::{CheckWatcher, CheckWatcherSharedState}, LspError, Result, }; use std::str::FromStr; @@ -52,6 +53,7 @@ pub struct WorldState { pub vfs: Arc>, pub task_receiver: Receiver, pub latest_requests: Arc>, + pub check_watcher: CheckWatcher, } /// An immutable snapshot of the world's state at a point in time. @@ -61,6 +63,7 @@ pub struct WorldSnapshot { pub analysis: Analysis, pub vfs: Arc>, pub latest_requests: Arc>, + pub check_watcher: Arc>, } impl WorldState { @@ -127,6 +130,9 @@ impl WorldState { } change.set_crate_graph(crate_graph); + // FIXME: Figure out the multi-workspace situation + let check_watcher = CheckWatcher::new(folder_roots.first().cloned().unwrap()); + let mut analysis_host = AnalysisHost::new(lru_capacity, feature_flags); analysis_host.apply_change(change); WorldState { @@ -138,6 +144,7 @@ impl WorldState { vfs: Arc::new(RwLock::new(vfs)), task_receiver, latest_requests: Default::default(), + check_watcher, } } @@ -199,6 +206,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.shared.clone(), } } From 41a1ec723ce2ea3fa78ae468830f0a77e5658307 Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Wed, 25 Dec 2019 16:44:42 +0100 Subject: [PATCH 02/20] Remove cargo-watch from vscode extension. Still keeps tests around for reference when porting them to rust --- editors/code/package.json | 12 +- editors/code/src/commands/cargo_watch.ts | 264 --------------------- editors/code/src/commands/runnables.ts | 91 ------- editors/code/src/extension.ts | 25 -- editors/code/src/utils/processes.ts | 51 ---- editors/code/src/utils/terminateProcess.sh | 12 - 6 files changed, 1 insertion(+), 454 deletions(-) delete mode 100644 editors/code/src/commands/cargo_watch.ts delete mode 100644 editors/code/src/utils/processes.ts delete mode 100644 editors/code/src/utils/terminateProcess.sh diff --git a/editors/code/package.json b/editors/code/package.json index f75fafeb9e..6cb24a3cec 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -18,7 +18,7 @@ "scripts": { "vscode:prepublish": "npm run compile", "package": "vsce package", - "compile": "rollup -c && shx cp src/utils/terminateProcess.sh bundle/terminateProcess.sh", + "compile": "rollup -c", "watch": "tsc -watch -p ./", "fix": "prettier **/*.{json,ts} --write && tslint --project . --fix", "lint": "tslint --project .", @@ -133,16 +133,6 @@ "command": "rust-analyzer.reload", "title": "Restart server", "category": "Rust Analyzer" - }, - { - "command": "rust-analyzer.startCargoWatch", - "title": "Start Cargo Watch", - "category": "Rust Analyzer" - }, - { - "command": "rust-analyzer.stopCargoWatch", - "title": "Stop Cargo Watch", - "category": "Rust Analyzer" } ], "keybindings": [ diff --git a/editors/code/src/commands/cargo_watch.ts b/editors/code/src/commands/cargo_watch.ts deleted file mode 100644 index ac62bdd48d..0000000000 --- a/editors/code/src/commands/cargo_watch.ts +++ /dev/null @@ -1,264 +0,0 @@ -import * as child_process from 'child_process'; -import * as path from 'path'; -import * as vscode from 'vscode'; - -import { Server } from '../server'; -import { terminate } from '../utils/processes'; -import { LineBuffer } from './line_buffer'; -import { StatusDisplay } from './watch_status'; - -import { - mapRustDiagnosticToVsCode, - RustDiagnostic, -} from '../utils/diagnostics/rust'; -import SuggestedFixCollection from '../utils/diagnostics/SuggestedFixCollection'; -import { areDiagnosticsEqual } from '../utils/diagnostics/vscode'; - -export async function registerCargoWatchProvider( - subscriptions: vscode.Disposable[], -): Promise { - let cargoExists = false; - - // Check if the working directory is valid cargo root path - const cargoTomlPath = path.join(vscode.workspace.rootPath!, 'Cargo.toml'); - const cargoTomlUri = vscode.Uri.file(cargoTomlPath); - const cargoTomlFileInfo = await vscode.workspace.fs.stat(cargoTomlUri); - - if (cargoTomlFileInfo) { - cargoExists = true; - } - - if (!cargoExists) { - vscode.window.showErrorMessage( - `Couldn\'t find \'Cargo.toml\' at ${cargoTomlPath}`, - ); - return; - } - - const provider = new CargoWatchProvider(); - subscriptions.push(provider); - return provider; -} - -export class CargoWatchProvider implements vscode.Disposable { - private readonly diagnosticCollection: vscode.DiagnosticCollection; - private readonly statusDisplay: StatusDisplay; - private readonly outputChannel: vscode.OutputChannel; - - private suggestedFixCollection: SuggestedFixCollection; - private codeActionDispose: vscode.Disposable; - - private cargoProcess?: child_process.ChildProcess; - - constructor() { - this.diagnosticCollection = vscode.languages.createDiagnosticCollection( - 'rustc', - ); - this.statusDisplay = new StatusDisplay( - Server.config.cargoWatchOptions.command, - ); - this.outputChannel = vscode.window.createOutputChannel( - 'Cargo Watch Trace', - ); - - // Track `rustc`'s suggested fixes so we can convert them to code actions - this.suggestedFixCollection = new SuggestedFixCollection(); - this.codeActionDispose = vscode.languages.registerCodeActionsProvider( - [{ scheme: 'file', language: 'rust' }], - this.suggestedFixCollection, - { - providedCodeActionKinds: - SuggestedFixCollection.PROVIDED_CODE_ACTION_KINDS, - }, - ); - } - - public start() { - if (this.cargoProcess) { - vscode.window.showInformationMessage( - 'Cargo Watch is already running', - ); - return; - } - - let args = - Server.config.cargoWatchOptions.command + ' --message-format json'; - if (Server.config.cargoWatchOptions.allTargets) { - args += ' --all-targets'; - } - if (Server.config.cargoWatchOptions.command.length > 0) { - // Excape the double quote string: - args += ' ' + Server.config.cargoWatchOptions.arguments; - } - // Windows handles arguments differently than the unix-likes, so we need to wrap the args in double quotes - if (process.platform === 'win32') { - args = '"' + args + '"'; - } - - const ignoreFlags = Server.config.cargoWatchOptions.ignore.reduce( - (flags, pattern) => [...flags, '--ignore', pattern], - [] as string[], - ); - - // Start the cargo watch with json message - this.cargoProcess = child_process.spawn( - 'cargo', - ['watch', '-x', args, ...ignoreFlags], - { - stdio: ['ignore', 'pipe', 'pipe'], - cwd: vscode.workspace.rootPath, - windowsVerbatimArguments: true, - }, - ); - - if (!this.cargoProcess) { - vscode.window.showErrorMessage('Cargo Watch failed to start'); - return; - } - - const stdoutData = new LineBuffer(); - this.cargoProcess.stdout?.on('data', (s: string) => { - stdoutData.processOutput(s, line => { - this.logInfo(line); - try { - this.parseLine(line); - } catch (err) { - this.logError(`Failed to parse: ${err}, content : ${line}`); - } - }); - }); - - const stderrData = new LineBuffer(); - this.cargoProcess.stderr?.on('data', (s: string) => { - stderrData.processOutput(s, line => { - this.logError('Error on cargo-watch : {\n' + line + '}\n'); - }); - }); - - this.cargoProcess.on('error', (err: Error) => { - this.logError( - 'Error on cargo-watch process : {\n' + err.message + '}\n', - ); - }); - - this.logInfo('cargo-watch started.'); - } - - public stop() { - if (this.cargoProcess) { - this.cargoProcess.kill(); - terminate(this.cargoProcess); - this.cargoProcess = undefined; - } else { - vscode.window.showInformationMessage('Cargo Watch is not running'); - } - } - - public dispose(): void { - this.stop(); - - this.diagnosticCollection.clear(); - this.diagnosticCollection.dispose(); - this.outputChannel.dispose(); - this.statusDisplay.dispose(); - this.codeActionDispose.dispose(); - } - - private logInfo(line: string) { - if (Server.config.cargoWatchOptions.trace === 'verbose') { - this.outputChannel.append(line); - } - } - - private logError(line: string) { - if ( - Server.config.cargoWatchOptions.trace === 'error' || - Server.config.cargoWatchOptions.trace === 'verbose' - ) { - this.outputChannel.append(line); - } - } - - private parseLine(line: string) { - if (line.startsWith('[Running')) { - this.diagnosticCollection.clear(); - this.suggestedFixCollection.clear(); - this.statusDisplay.show(); - } - - if (line.startsWith('[Finished running')) { - this.statusDisplay.hide(); - } - - interface CargoArtifact { - reason: string; - package_id: string; - } - - // https://github.com/rust-lang/cargo/blob/master/src/cargo/util/machine_message.rs - interface CargoMessage { - reason: string; - package_id: string; - message: RustDiagnostic; - } - - // cargo-watch itself output non json format - // Ignore these lines - let data: CargoMessage; - try { - data = JSON.parse(line.trim()); - } catch (error) { - this.logError(`Fail to parse to json : { ${error} }`); - return; - } - - if (data.reason === 'compiler-artifact') { - const msg = data as CargoArtifact; - - // The format of the package_id is "{name} {version} ({source_id})", - // https://github.com/rust-lang/cargo/blob/37ad03f86e895bb80b474c1c088322634f4725f5/src/cargo/core/package_id.rs#L53 - this.statusDisplay.packageName = msg.package_id.split(' ')[0]; - } else if (data.reason === 'compiler-message') { - const msg = data.message as RustDiagnostic; - - const mapResult = mapRustDiagnosticToVsCode(msg); - if (!mapResult) { - return; - } - - const { location, diagnostic, suggestedFixes } = mapResult; - const fileUri = location.uri; - - const diagnostics: vscode.Diagnostic[] = [ - ...(this.diagnosticCollection!.get(fileUri) || []), - ]; - - // If we're building multiple targets it's possible we've already seen this diagnostic - const isDuplicate = diagnostics.some(d => - areDiagnosticsEqual(d, diagnostic), - ); - if (isDuplicate) { - return; - } - - diagnostics.push(diagnostic); - this.diagnosticCollection!.set(fileUri, diagnostics); - - if (suggestedFixes.length) { - for (const suggestedFix of suggestedFixes) { - this.suggestedFixCollection.addSuggestedFixForDiagnostic( - suggestedFix, - diagnostic, - ); - } - - // Have VsCode query us for the code actions - vscode.commands.executeCommand( - 'vscode.executeCodeActionProvider', - fileUri, - diagnostic.range, - ); - } - } - } -} diff --git a/editors/code/src/commands/runnables.ts b/editors/code/src/commands/runnables.ts index cf980e2578..7728541de6 100644 --- a/editors/code/src/commands/runnables.ts +++ b/editors/code/src/commands/runnables.ts @@ -1,11 +1,7 @@ -import * as child_process from 'child_process'; - -import * as util from 'util'; import * as vscode from 'vscode'; import * as lc from 'vscode-languageclient'; import { Server } from '../server'; -import { CargoWatchProvider, registerCargoWatchProvider } from './cargo_watch'; interface RunnablesParams { textDocument: lc.TextDocumentIdentifier; @@ -131,90 +127,3 @@ export async function handleSingle(runnable: Runnable) { return vscode.tasks.executeTask(task); } - -/** - * Interactively asks the user whether we should run `cargo check` in order to - * provide inline diagnostics; the user is met with a series of dialog boxes - * that, when accepted, allow us to `cargo install cargo-watch` and then run it. - */ -export async function interactivelyStartCargoWatch( - context: vscode.ExtensionContext, -): Promise { - if (Server.config.cargoWatchOptions.enableOnStartup === 'disabled') { - return; - } - - if (Server.config.cargoWatchOptions.enableOnStartup === 'ask') { - const watch = await vscode.window.showInformationMessage( - 'Start watching changes with cargo? (Executes `cargo watch`, provides inline diagnostics)', - 'yes', - 'no', - ); - if (watch !== 'yes') { - return; - } - } - - return startCargoWatch(context); -} - -export async function startCargoWatch( - context: vscode.ExtensionContext, -): Promise { - const execPromise = util.promisify(child_process.exec); - - const { stderr, code = 0 } = await execPromise( - 'cargo watch --version', - ).catch(e => e); - - if (stderr.includes('no such subcommand: `watch`')) { - const msg = - 'The `cargo-watch` subcommand is not installed. Install? (takes ~1-2 minutes)'; - const install = await vscode.window.showInformationMessage( - msg, - 'yes', - 'no', - ); - if (install !== 'yes') { - return; - } - - const label = 'install-cargo-watch'; - const taskFinished = new Promise((resolve, _reject) => { - const disposable = vscode.tasks.onDidEndTask(({ execution }) => { - if (execution.task.name === label) { - disposable.dispose(); - resolve(); - } - }); - }); - - vscode.tasks.executeTask( - createTask({ - label, - bin: 'cargo', - args: ['install', 'cargo-watch'], - env: {}, - }), - ); - await taskFinished; - const output = await execPromise('cargo watch --version').catch(e => e); - if (output.stderr !== '') { - vscode.window.showErrorMessage( - `Couldn't install \`cargo-\`watch: ${output.stderr}`, - ); - return; - } - } else if (code !== 0) { - vscode.window.showErrorMessage( - `\`cargo watch\` failed with ${code}: ${stderr}`, - ); - return; - } - - const provider = await registerCargoWatchProvider(context.subscriptions); - if (provider) { - provider.start(); - } - return provider; -} diff --git a/editors/code/src/extension.ts b/editors/code/src/extension.ts index 815f3692c0..72a4d4bf25 100644 --- a/editors/code/src/extension.ts +++ b/editors/code/src/extension.ts @@ -2,13 +2,8 @@ import * as vscode from 'vscode'; import * as lc from 'vscode-languageclient'; import * as commands from './commands'; -import { CargoWatchProvider } from './commands/cargo_watch'; import { ExpandMacroContentProvider } from './commands/expand_macro'; import { HintsUpdater } from './commands/inlay_hints'; -import { - interactivelyStartCargoWatch, - startCargoWatch, -} from './commands/runnables'; import { SyntaxTreeContentProvider } from './commands/syntaxTree'; import * as events from './events'; import * as notifications from './notifications'; @@ -139,26 +134,6 @@ export async function activate(context: vscode.ExtensionContext) { vscode.commands.registerCommand('rust-analyzer.reload', reloadCommand); - // Executing `cargo watch` provides us with inline diagnostics on save - let provider: CargoWatchProvider | undefined; - interactivelyStartCargoWatch(context).then(p => { - provider = p; - }); - registerCommand('rust-analyzer.startCargoWatch', () => { - if (provider) { - provider.start(); - } else { - startCargoWatch(context).then(p => { - provider = p; - }); - } - }); - registerCommand('rust-analyzer.stopCargoWatch', () => { - if (provider) { - provider.stop(); - } - }); - // Start the language server, finally! try { await startServer(); diff --git a/editors/code/src/utils/processes.ts b/editors/code/src/utils/processes.ts deleted file mode 100644 index a1d6b7eafb..0000000000 --- a/editors/code/src/utils/processes.ts +++ /dev/null @@ -1,51 +0,0 @@ -'use strict'; - -import * as cp from 'child_process'; -import ChildProcess = cp.ChildProcess; - -import { join } from 'path'; - -const isWindows = process.platform === 'win32'; -const isMacintosh = process.platform === 'darwin'; -const isLinux = process.platform === 'linux'; - -// this is very complex, but is basically copy-pased from VSCode implementation here: -// https://github.com/Microsoft/vscode-languageserver-node/blob/dbfd37e35953ad0ee14c4eeced8cfbc41697b47e/client/src/utils/processes.ts#L15 - -// And see discussion at -// https://github.com/rust-analyzer/rust-analyzer/pull/1079#issuecomment-478908109 - -export function terminate(process: ChildProcess, cwd?: string): boolean { - if (isWindows) { - try { - // This we run in Atom execFileSync is available. - // Ignore stderr since this is otherwise piped to parent.stderr - // which might be already closed. - const options: any = { - stdio: ['pipe', 'pipe', 'ignore'], - }; - if (cwd) { - options.cwd = cwd; - } - cp.execFileSync( - 'taskkill', - ['/T', '/F', '/PID', process.pid.toString()], - options, - ); - return true; - } catch (err) { - return false; - } - } else if (isLinux || isMacintosh) { - try { - const cmd = join(__dirname, 'terminateProcess.sh'); - const result = cp.spawnSync(cmd, [process.pid.toString()]); - return result.error ? false : true; - } catch (err) { - return false; - } - } else { - process.kill('SIGKILL'); - return true; - } -} diff --git a/editors/code/src/utils/terminateProcess.sh b/editors/code/src/utils/terminateProcess.sh deleted file mode 100644 index 2ec9e1c2ec..0000000000 --- a/editors/code/src/utils/terminateProcess.sh +++ /dev/null @@ -1,12 +0,0 @@ -#!/bin/bash - -terminateTree() { - for cpid in $(pgrep -P $1); do - terminateTree $cpid - done - kill -9 $1 > /dev/null 2>&1 -} - -for pid in $*; do - terminateTree $pid -done \ No newline at end of file From 6af4bf7a8d27e653d2e6316172fe140871054d27 Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Wed, 25 Dec 2019 16:50:38 +0100 Subject: [PATCH 03/20] Configuration plumbing for cargo watcher --- crates/ra_lsp_server/src/cargo_check.rs | 50 ++++++++++++++++---- crates/ra_lsp_server/src/config.rs | 7 +++ crates/ra_lsp_server/src/main_loop.rs | 3 ++ crates/ra_lsp_server/src/world.rs | 5 +- editors/code/package.json | 40 ++++------------ editors/code/src/config.ts | 63 +++++++------------------ editors/code/src/server.ts | 3 ++ 7 files changed, 85 insertions(+), 86 deletions(-) diff --git a/crates/ra_lsp_server/src/cargo_check.rs b/crates/ra_lsp_server/src/cargo_check.rs index d5ff021541..f98b4f69cd 100644 --- a/crates/ra_lsp_server/src/cargo_check.rs +++ b/crates/ra_lsp_server/src/cargo_check.rs @@ -1,3 +1,4 @@ +use crate::world::Options; use cargo_metadata::{ diagnostic::{ Applicability, Diagnostic as RustDiagnostic, DiagnosticLevel, DiagnosticSpan, @@ -30,14 +31,17 @@ pub struct CheckWatcher { } impl CheckWatcher { - pub fn new(workspace_root: PathBuf) -> CheckWatcher { + pub fn new(options: &Options, workspace_root: PathBuf) -> CheckWatcher { + let check_command = options.cargo_check_command.clone(); + let check_args = options.cargo_check_args.clone(); let shared = Arc::new(RwLock::new(CheckWatcherSharedState::new())); let (task_send, task_recv) = unbounded::(); let (cmd_send, cmd_recv) = unbounded::(); let shared_ = shared.clone(); let handle = std::thread::spawn(move || { - let mut check = CheckWatcherState::new(shared_, workspace_root); + let mut check = + CheckWatcherState::new(check_command, check_args, workspace_root, shared_); check.run(&task_send, &cmd_recv); }); @@ -50,6 +54,8 @@ impl CheckWatcher { } pub struct CheckWatcherState { + check_command: Option, + check_args: Vec, workspace_root: PathBuf, running: bool, watcher: WatchThread, @@ -134,11 +140,21 @@ pub enum CheckCommand { impl CheckWatcherState { pub fn new( - shared: Arc>, + check_command: Option, + check_args: Vec, workspace_root: PathBuf, + shared: Arc>, ) -> CheckWatcherState { - let watcher = WatchThread::new(&workspace_root); - CheckWatcherState { workspace_root, running: false, watcher, last_update_req: None, shared } + let watcher = WatchThread::new(check_command.as_ref(), &check_args, &workspace_root); + CheckWatcherState { + check_command, + check_args, + workspace_root, + running: false, + watcher, + last_update_req: None, + shared, + } } pub fn run(&mut self, task_send: &Sender, cmd_recv: &Receiver) { @@ -163,7 +179,11 @@ impl CheckWatcherState { self.shared.write().clear(task_send); self.watcher.cancel(); - self.watcher = WatchThread::new(&self.workspace_root); + self.watcher = WatchThread::new( + self.check_command.as_ref(), + &self.check_args, + &self.workspace_root, + ); } } } @@ -229,13 +249,25 @@ struct WatchThread { } impl WatchThread { - fn new(workspace_root: &PathBuf) -> WatchThread { - let manifest_path = format!("{}/Cargo.toml", workspace_root.to_string_lossy()); + fn new( + check_command: Option<&String>, + check_args: &[String], + workspace_root: &PathBuf, + ) -> WatchThread { + let check_command = check_command.cloned().unwrap_or("check".to_string()); + let mut args: Vec = vec![ + check_command, + "--message-format=json".to_string(), + "--manifest-path".to_string(), + format!("{}/Cargo.toml", workspace_root.to_string_lossy()), + ]; + args.extend(check_args.iter().cloned()); + let (message_send, message_recv) = unbounded(); let (cancel_send, cancel_recv) = unbounded(); std::thread::spawn(move || { let mut command = Command::new("cargo") - .args(&["check", "--message-format=json", "--manifest-path", &manifest_path]) + .args(&args) .stdout(Stdio::piped()) .stderr(Stdio::null()) .spawn() diff --git a/crates/ra_lsp_server/src/config.rs b/crates/ra_lsp_server/src/config.rs index 67942aa414..621f2238c5 100644 --- a/crates/ra_lsp_server/src/config.rs +++ b/crates/ra_lsp_server/src/config.rs @@ -32,6 +32,10 @@ pub struct ServerConfig { pub max_inlay_hint_length: Option, + pub cargo_check_enable: bool, + pub cargo_check_command: Option, + pub cargo_check_args: Vec, + /// For internal usage to make integrated tests faster. #[serde(deserialize_with = "nullable_bool_true")] pub with_sysroot: bool, @@ -51,6 +55,9 @@ impl Default for ServerConfig { use_client_watching: false, lru_capacity: None, max_inlay_hint_length: None, + cargo_check_enable: true, + cargo_check_command: None, + cargo_check_args: vec![], with_sysroot: true, feature_flags: FxHashMap::default(), cargo_features: Default::default(), diff --git a/crates/ra_lsp_server/src/main_loop.rs b/crates/ra_lsp_server/src/main_loop.rs index 943d38943f..1f6175699a 100644 --- a/crates/ra_lsp_server/src/main_loop.rs +++ b/crates/ra_lsp_server/src/main_loop.rs @@ -127,6 +127,9 @@ pub fn main_loop( .and_then(|it| it.line_folding_only) .unwrap_or(false), max_inlay_hint_length: config.max_inlay_hint_length, + cargo_check_enable: config.cargo_check_enable, + cargo_check_command: config.cargo_check_command, + cargo_check_args: config.cargo_check_args, } }; diff --git a/crates/ra_lsp_server/src/world.rs b/crates/ra_lsp_server/src/world.rs index 8e9380ca07..235eb199d0 100644 --- a/crates/ra_lsp_server/src/world.rs +++ b/crates/ra_lsp_server/src/world.rs @@ -35,6 +35,9 @@ pub struct Options { pub supports_location_link: bool, pub line_folding_only: bool, pub max_inlay_hint_length: Option, + pub cargo_check_enable: bool, + pub cargo_check_command: Option, + pub cargo_check_args: Vec, } /// `WorldState` is the primary mutable state of the language server @@ -131,7 +134,7 @@ impl WorldState { change.set_crate_graph(crate_graph); // FIXME: Figure out the multi-workspace situation - let check_watcher = CheckWatcher::new(folder_roots.first().cloned().unwrap()); + let check_watcher = CheckWatcher::new(&options, folder_roots.first().cloned().unwrap()); let mut analysis_host = AnalysisHost::new(lru_capacity, feature_flags); analysis_host.apply_change(change); diff --git a/editors/code/package.json b/editors/code/package.json index 6cb24a3cec..5f4123397b 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -188,20 +188,10 @@ "default": "ra_lsp_server", "description": "Path to ra_lsp_server executable" }, - "rust-analyzer.enableCargoWatchOnStartup": { - "type": "string", - "default": "ask", - "enum": [ - "ask", - "enabled", - "disabled" - ], - "enumDescriptions": [ - "Asks each time whether to run `cargo watch`", - "`cargo watch` is always started", - "Don't start `cargo watch`" - ], - "description": "Whether to run `cargo watch` on startup" + "rust-analyzer.enableCargoCheck": { + "type": "boolean", + "default": true, + "description": "Run `cargo check` for diagnostics on save" }, "rust-analyzer.excludeGlobs": { "type": "array", @@ -213,25 +203,15 @@ "default": true, "description": "client provided file watching instead of notify watching." }, - "rust-analyzer.cargo-watch.arguments": { - "type": "string", - "description": "`cargo-watch` arguments. (e.g: `--features=\"shumway,pdf\"` will run as `cargo watch -x \"check --features=\"shumway,pdf\"\"` )", - "default": "" - }, - "rust-analyzer.cargo-watch.command": { - "type": "string", - "description": "`cargo-watch` command. (e.g: `clippy` will run as `cargo watch -x clippy` )", - "default": "check" - }, - "rust-analyzer.cargo-watch.ignore": { + "rust-analyzer.cargo-check.arguments": { "type": "array", - "description": "A list of patterns for cargo-watch to ignore (will be passed as `--ignore`)", + "description": "`cargo-check` arguments. (e.g: `--features=\"shumway,pdf\"` will run as `cargo check --features=\"shumway,pdf\"` )", "default": [] }, - "rust-analyzer.cargo-watch.allTargets": { - "type": "boolean", - "description": "Check all targets and tests (will be passed as `--all-targets`)", - "default": true + "rust-analyzer.cargo-check.command": { + "type": "string", + "description": "`cargo-check` command. (e.g: `clippy` will run as `cargo clippy` )", + "default": "check" }, "rust-analyzer.trace.server": { "type": "string", diff --git a/editors/code/src/config.ts b/editors/code/src/config.ts index e131f09df6..96532e2c9d 100644 --- a/editors/code/src/config.ts +++ b/editors/code/src/config.ts @@ -4,16 +4,10 @@ import { Server } from './server'; const RA_LSP_DEBUG = process.env.__RA_LSP_SERVER_DEBUG; -export type CargoWatchStartupOptions = 'ask' | 'enabled' | 'disabled'; -export type CargoWatchTraceOptions = 'off' | 'error' | 'verbose'; - -export interface CargoWatchOptions { - enableOnStartup: CargoWatchStartupOptions; - arguments: string; - command: string; - trace: CargoWatchTraceOptions; - ignore: string[]; - allTargets: boolean; +export interface CargoCheckOptions { + enabled: boolean; + arguments: string[]; + command: null | string; } export interface CargoFeatures { @@ -35,13 +29,10 @@ export class Config { public featureFlags = {}; // for internal use public withSysroot: null | boolean = null; - public cargoWatchOptions: CargoWatchOptions = { - enableOnStartup: 'ask', - trace: 'off', - arguments: '', - command: '', - ignore: [], - allTargets: true, + public cargoCheckOptions: CargoCheckOptions = { + enabled: true, + arguments: [], + command: null, }; public cargoFeatures: CargoFeatures = { noDefaultFeatures: false, @@ -100,44 +91,24 @@ export class Config { RA_LSP_DEBUG || (config.get('raLspServerPath') as string); } - if (config.has('enableCargoWatchOnStartup')) { - this.cargoWatchOptions.enableOnStartup = config.get< - CargoWatchStartupOptions - >('enableCargoWatchOnStartup', 'ask'); - } - - if (config.has('trace.cargo-watch')) { - this.cargoWatchOptions.trace = config.get( - 'trace.cargo-watch', - 'off', + if (config.has('enableCargoCheck')) { + this.cargoCheckOptions.enabled = config.get( + 'enableCargoCheck', + true, ); } if (config.has('cargo-watch.arguments')) { - this.cargoWatchOptions.arguments = config.get( + this.cargoCheckOptions.arguments = config.get( 'cargo-watch.arguments', - '', - ); - } - - if (config.has('cargo-watch.command')) { - this.cargoWatchOptions.command = config.get( - 'cargo-watch.command', - '', - ); - } - - if (config.has('cargo-watch.ignore')) { - this.cargoWatchOptions.ignore = config.get( - 'cargo-watch.ignore', [], ); } - if (config.has('cargo-watch.allTargets')) { - this.cargoWatchOptions.allTargets = config.get( - 'cargo-watch.allTargets', - true, + if (config.has('cargo-watch.command')) { + this.cargoCheckOptions.command = config.get( + 'cargo-watch.command', + '', ); } diff --git a/editors/code/src/server.ts b/editors/code/src/server.ts index 5ace1d0fae..409d3b4b73 100644 --- a/editors/code/src/server.ts +++ b/editors/code/src/server.ts @@ -55,6 +55,9 @@ export class Server { publishDecorations: true, lruCapacity: Server.config.lruCapacity, maxInlayHintLength: Server.config.maxInlayHintLength, + cargoCheckEnable: Server.config.cargoCheckOptions.enabled, + cargoCheckCommand: Server.config.cargoCheckOptions.command, + cargoCheckArgs: Server.config.cargoCheckOptions.arguments, excludeGlobs: Server.config.excludeGlobs, useClientWatching: Server.config.useClientWatching, featureFlags: Server.config.featureFlags, From 17360b5d14ec98085169bfb344b220dcfc75b9f0 Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Wed, 25 Dec 2019 17:14:20 +0100 Subject: [PATCH 04/20] Fix use of wrong message in diagnostic --- crates/ra_lsp_server/src/cargo_check.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ra_lsp_server/src/cargo_check.rs b/crates/ra_lsp_server/src/cargo_check.rs index f98b4f69cd..30716fc436 100644 --- a/crates/ra_lsp_server/src/cargo_check.rs +++ b/crates/ra_lsp_server/src/cargo_check.rs @@ -545,7 +545,7 @@ fn map_rust_diagnostic_to_lsp( severity, code: code.map(NumberOrString::String), source: Some(source), - message: rd.message.clone(), + message, related_information: if !related_information.is_empty() { Some(related_information) } else { From c21fbc3e87d1e701f29fafcdad0a73e8d69a2f29 Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Wed, 25 Dec 2019 17:31:49 +0100 Subject: [PATCH 05/20] Migrate tests from extension to rust --- Cargo.lock | 1 + crates/ra_lsp_server/Cargo.toml | 1 + crates/ra_lsp_server/src/cargo_check.rs | 709 ++++++++++++++++++ .../test__snap_clippy_pass_by_ref.snap | 85 +++ .../test__snap_handles_macro_location.snap | 46 ++ ...nap_rustc_incompatible_type_for_trait.snap | 46 ++ .../test__snap_rustc_mismatched_type.snap | 46 ++ .../test__snap_rustc_unused_variable.snap | 70 ++ ...snap_rustc_wrong_number_of_parameters.snap | 65 ++ 9 files changed, 1069 insertions(+) create mode 100644 crates/ra_lsp_server/src/snapshots/test__snap_clippy_pass_by_ref.snap create mode 100644 crates/ra_lsp_server/src/snapshots/test__snap_handles_macro_location.snap create mode 100644 crates/ra_lsp_server/src/snapshots/test__snap_rustc_incompatible_type_for_trait.snap create mode 100644 crates/ra_lsp_server/src/snapshots/test__snap_rustc_mismatched_type.snap create mode 100644 crates/ra_lsp_server/src/snapshots/test__snap_rustc_unused_variable.snap create mode 100644 crates/ra_lsp_server/src/snapshots/test__snap_rustc_wrong_number_of_parameters.snap diff --git a/Cargo.lock b/Cargo.lock index 77c10a10e3..08e4b7106e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1054,6 +1054,7 @@ dependencies = [ "cargo_metadata 0.9.1 (registry+https://github.com/rust-lang/crates.io-index)", "crossbeam-channel 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", + "insta 0.12.0 (registry+https://github.com/rust-lang/crates.io-index)", "jod-thread 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", "lsp-server 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/crates/ra_lsp_server/Cargo.toml b/crates/ra_lsp_server/Cargo.toml index aa1acdc330..54a01d7a2c 100644 --- a/crates/ra_lsp_server/Cargo.toml +++ b/crates/ra_lsp_server/Cargo.toml @@ -30,6 +30,7 @@ env_logger = { version = "0.7.1", default-features = false, features = ["humanti cargo_metadata = "0.9.1" [dev-dependencies] +insta = "0.12.0" tempfile = "3" test_utils = { path = "../test_utils" } diff --git a/crates/ra_lsp_server/src/cargo_check.rs b/crates/ra_lsp_server/src/cargo_check.rs index 30716fc436..5a6a209eba 100644 --- a/crates/ra_lsp_server/src/cargo_check.rs +++ b/crates/ra_lsp_server/src/cargo_check.rs @@ -461,6 +461,7 @@ fn map_rust_child_diagnostic( } } +#[derive(Debug)] struct MappedRustDiagnostic { location: Location, diagnostic: Diagnostic, @@ -563,3 +564,711 @@ fn are_diagnostics_equal(left: &Diagnostic, right: &Diagnostic) -> bool { && left.range == right.range && left.message == right.message } + +#[cfg(test)] +mod test { + use super::*; + + fn parse_diagnostic(val: &str) -> cargo_metadata::diagnostic::Diagnostic { + serde_json::from_str::(val).unwrap() + } + + #[test] + fn snap_rustc_incompatible_type_for_trait() { + let diag = parse_diagnostic( + r##"{ + "message": "method `next` has an incompatible type for trait", + "code": { + "code": "E0053", + "explanation": "\nThe parameters of any trait method must match between a trait implementation\nand the trait definition.\n\nHere are a couple examples of this error:\n\n```compile_fail,E0053\ntrait Foo {\n fn foo(x: u16);\n fn bar(&self);\n}\n\nstruct Bar;\n\nimpl Foo for Bar {\n // error, expected u16, found i16\n fn foo(x: i16) { }\n\n // error, types differ in mutability\n fn bar(&mut self) { }\n}\n```\n" + }, + "level": "error", + "spans": [ + { + "file_name": "compiler/ty/list_iter.rs", + "byte_start": 1307, + "byte_end": 1350, + "line_start": 52, + "line_end": 52, + "column_start": 5, + "column_end": 48, + "is_primary": true, + "text": [ + { + "text": " fn next(&self) -> Option<&'list ty::Ref> {", + "highlight_start": 5, + "highlight_end": 48 + } + ], + "label": "types differ in mutability", + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + ], + "children": [ + { + "message": "expected type `fn(&mut ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&ty::Ref>`\n found type `fn(&ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&'list ty::Ref>`", + "code": null, + "level": "note", + "spans": [], + "children": [], + "rendered": null + } + ], + "rendered": "error[E0053]: method `next` has an incompatible type for trait\n --> compiler/ty/list_iter.rs:52:5\n |\n52 | fn next(&self) -> Option<&'list ty::Ref> {\n | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ types differ in mutability\n |\n = note: expected type `fn(&mut ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&ty::Ref>`\n found type `fn(&ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&'list ty::Ref>`\n\n" + } + "##, + ); + + let workspace_root = PathBuf::from("/test/"); + let diag = + map_rust_diagnostic_to_lsp(&diag, &workspace_root).expect("couldn't map diagnostic"); + insta::assert_debug_snapshot!(diag); + } + + #[test] + fn snap_rustc_unused_variable() { + let diag = parse_diagnostic( + r##"{ + "message": "unused variable: `foo`", + "code": { + "code": "unused_variables", + "explanation": null + }, + "level": "warning", + "spans": [ + { + "file_name": "driver/subcommand/repl.rs", + "byte_start": 9228, + "byte_end": 9231, + "line_start": 291, + "line_end": 291, + "column_start": 9, + "column_end": 12, + "is_primary": true, + "text": [ + { + "text": " let foo = 42;", + "highlight_start": 9, + "highlight_end": 12 + } + ], + "label": null, + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + ], + "children": [ + { + "message": "#[warn(unused_variables)] on by default", + "code": null, + "level": "note", + "spans": [], + "children": [], + "rendered": null + }, + { + "message": "consider prefixing with an underscore", + "code": null, + "level": "help", + "spans": [ + { + "file_name": "driver/subcommand/repl.rs", + "byte_start": 9228, + "byte_end": 9231, + "line_start": 291, + "line_end": 291, + "column_start": 9, + "column_end": 12, + "is_primary": true, + "text": [ + { + "text": " let foo = 42;", + "highlight_start": 9, + "highlight_end": 12 + } + ], + "label": null, + "suggested_replacement": "_foo", + "suggestion_applicability": "MachineApplicable", + "expansion": null + } + ], + "children": [], + "rendered": null + } + ], + "rendered": "warning: unused variable: `foo`\n --> driver/subcommand/repl.rs:291:9\n |\n291 | let foo = 42;\n | ^^^ help: consider prefixing with an underscore: `_foo`\n |\n = note: #[warn(unused_variables)] on by default\n\n" +}"##, + ); + + let workspace_root = PathBuf::from("/test/"); + let diag = + map_rust_diagnostic_to_lsp(&diag, &workspace_root).expect("couldn't map diagnostic"); + insta::assert_debug_snapshot!(diag); + } + + #[test] + fn snap_rustc_wrong_number_of_parameters() { + let diag = parse_diagnostic( + r##"{ + "message": "this function takes 2 parameters but 3 parameters were supplied", + "code": { + "code": "E0061", + "explanation": "\nThe number of arguments passed to a function must match the number of arguments\nspecified in the function signature.\n\nFor example, a function like:\n\n```\nfn f(a: u16, b: &str) {}\n```\n\nMust always be called with exactly two arguments, e.g., `f(2, \"test\")`.\n\nNote that Rust does not have a notion of optional function arguments or\nvariadic functions (except for its C-FFI).\n" + }, + "level": "error", + "spans": [ + { + "file_name": "compiler/ty/select.rs", + "byte_start": 8787, + "byte_end": 9241, + "line_start": 219, + "line_end": 231, + "column_start": 5, + "column_end": 6, + "is_primary": false, + "text": [ + { + "text": " pub fn add_evidence(", + "highlight_start": 5, + "highlight_end": 25 + }, + { + "text": " &mut self,", + "highlight_start": 1, + "highlight_end": 19 + }, + { + "text": " target_poly: &ty::Ref,", + "highlight_start": 1, + "highlight_end": 41 + }, + { + "text": " evidence_poly: &ty::Ref,", + "highlight_start": 1, + "highlight_end": 43 + }, + { + "text": " ) {", + "highlight_start": 1, + "highlight_end": 8 + }, + { + "text": " match target_poly {", + "highlight_start": 1, + "highlight_end": 28 + }, + { + "text": " ty::Ref::Var(tvar, _) => self.add_var_evidence(tvar, evidence_poly),", + "highlight_start": 1, + "highlight_end": 81 + }, + { + "text": " ty::Ref::Fixed(target_ty) => {", + "highlight_start": 1, + "highlight_end": 43 + }, + { + "text": " let evidence_ty = evidence_poly.resolve_to_ty();", + "highlight_start": 1, + "highlight_end": 65 + }, + { + "text": " self.add_evidence_ty(target_ty, evidence_poly, evidence_ty)", + "highlight_start": 1, + "highlight_end": 76 + }, + { + "text": " }", + "highlight_start": 1, + "highlight_end": 14 + }, + { + "text": " }", + "highlight_start": 1, + "highlight_end": 10 + }, + { + "text": " }", + "highlight_start": 1, + "highlight_end": 6 + } + ], + "label": "defined here", + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + }, + { + "file_name": "compiler/ty/select.rs", + "byte_start": 4045, + "byte_end": 4057, + "line_start": 104, + "line_end": 104, + "column_start": 18, + "column_end": 30, + "is_primary": true, + "text": [ + { + "text": " self.add_evidence(target_fixed, evidence_fixed, false);", + "highlight_start": 18, + "highlight_end": 30 + } + ], + "label": "expected 2 parameters", + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + ], + "children": [], + "rendered": "error[E0061]: this function takes 2 parameters but 3 parameters were supplied\n --> compiler/ty/select.rs:104:18\n |\n104 | self.add_evidence(target_fixed, evidence_fixed, false);\n | ^^^^^^^^^^^^ expected 2 parameters\n...\n219 | / pub fn add_evidence(\n220 | | &mut self,\n221 | | target_poly: &ty::Ref,\n222 | | evidence_poly: &ty::Ref,\n... |\n230 | | }\n231 | | }\n | |_____- defined here\n\n" +}"##, + ); + + let workspace_root = PathBuf::from("/test/"); + let diag = + map_rust_diagnostic_to_lsp(&diag, &workspace_root).expect("couldn't map diagnostic"); + insta::assert_debug_snapshot!(diag); + } + + #[test] + fn snap_clippy_pass_by_ref() { + let diag = parse_diagnostic( + r##"{ + "message": "this argument is passed by reference, but would be more efficient if passed by value", + "code": { + "code": "clippy::trivially_copy_pass_by_ref", + "explanation": null + }, + "level": "warning", + "spans": [ + { + "file_name": "compiler/mir/tagset.rs", + "byte_start": 941, + "byte_end": 946, + "line_start": 42, + "line_end": 42, + "column_start": 24, + "column_end": 29, + "is_primary": true, + "text": [ + { + "text": " pub fn is_disjoint(&self, other: Self) -> bool {", + "highlight_start": 24, + "highlight_end": 29 + } + ], + "label": null, + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + ], + "children": [ + { + "message": "lint level defined here", + "code": null, + "level": "note", + "spans": [ + { + "file_name": "compiler/lib.rs", + "byte_start": 8, + "byte_end": 19, + "line_start": 1, + "line_end": 1, + "column_start": 9, + "column_end": 20, + "is_primary": true, + "text": [ + { + "text": "#![warn(clippy::all)]", + "highlight_start": 9, + "highlight_end": 20 + } + ], + "label": null, + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + ], + "children": [], + "rendered": null + }, + { + "message": "#[warn(clippy::trivially_copy_pass_by_ref)] implied by #[warn(clippy::all)]", + "code": null, + "level": "note", + "spans": [], + "children": [], + "rendered": null + }, + { + "message": "for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref", + "code": null, + "level": "help", + "spans": [], + "children": [], + "rendered": null + }, + { + "message": "consider passing by value instead", + "code": null, + "level": "help", + "spans": [ + { + "file_name": "compiler/mir/tagset.rs", + "byte_start": 941, + "byte_end": 946, + "line_start": 42, + "line_end": 42, + "column_start": 24, + "column_end": 29, + "is_primary": true, + "text": [ + { + "text": " pub fn is_disjoint(&self, other: Self) -> bool {", + "highlight_start": 24, + "highlight_end": 29 + } + ], + "label": null, + "suggested_replacement": "self", + "suggestion_applicability": "Unspecified", + "expansion": null + } + ], + "children": [], + "rendered": null + } + ], + "rendered": "warning: this argument is passed by reference, but would be more efficient if passed by value\n --> compiler/mir/tagset.rs:42:24\n |\n42 | pub fn is_disjoint(&self, other: Self) -> bool {\n | ^^^^^ help: consider passing by value instead: `self`\n |\nnote: lint level defined here\n --> compiler/lib.rs:1:9\n |\n1 | #![warn(clippy::all)]\n | ^^^^^^^^^^^\n = note: #[warn(clippy::trivially_copy_pass_by_ref)] implied by #[warn(clippy::all)]\n = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref\n\n" +}"##, + ); + + let workspace_root = PathBuf::from("/test/"); + let diag = + map_rust_diagnostic_to_lsp(&diag, &workspace_root).expect("couldn't map diagnostic"); + insta::assert_debug_snapshot!(diag); + } + + #[test] + fn snap_rustc_mismatched_type() { + let diag = parse_diagnostic( + r##"{ + "message": "mismatched types", + "code": { + "code": "E0308", + "explanation": "\nThis error occurs when the compiler was unable to infer the concrete type of a\nvariable. It can occur for several cases, the most common of which is a\nmismatch in the expected type that the compiler inferred for a variable's\ninitializing expression, and the actual type explicitly assigned to the\nvariable.\n\nFor example:\n\n```compile_fail,E0308\nlet x: i32 = \"I am not a number!\";\n// ~~~ ~~~~~~~~~~~~~~~~~~~~\n// | |\n// | initializing expression;\n// | compiler infers type `&str`\n// |\n// type `i32` assigned to variable `x`\n```\n" + }, + "level": "error", + "spans": [ + { + "file_name": "runtime/compiler_support.rs", + "byte_start": 1589, + "byte_end": 1594, + "line_start": 48, + "line_end": 48, + "column_start": 65, + "column_end": 70, + "is_primary": true, + "text": [ + { + "text": " let layout = alloc::Layout::from_size_align_unchecked(size, align);", + "highlight_start": 65, + "highlight_end": 70 + } + ], + "label": "expected usize, found u32", + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + ], + "children": [], + "rendered": "error[E0308]: mismatched types\n --> runtime/compiler_support.rs:48:65\n |\n48 | let layout = alloc::Layout::from_size_align_unchecked(size, align);\n | ^^^^^ expected usize, found u32\n\n" +}"##, + ); + + let workspace_root = PathBuf::from("/test/"); + let diag = + map_rust_diagnostic_to_lsp(&diag, &workspace_root).expect("couldn't map diagnostic"); + insta::assert_debug_snapshot!(diag); + } + + #[test] + fn snap_handles_macro_location() { + let diag = parse_diagnostic( + r##"{ + "rendered": "error[E0277]: can't compare `{integer}` with `&str`\n --> src/main.rs:2:5\n |\n2 | assert_eq!(1, \"love\");\n | ^^^^^^^^^^^^^^^^^^^^^^ no implementation for `{integer} == &str`\n |\n = help: the trait `std::cmp::PartialEq<&str>` is not implemented for `{integer}`\n = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)\n\n", + "children": [ + { + "children": [], + "code": null, + "level": "help", + "message": "the trait `std::cmp::PartialEq<&str>` is not implemented for `{integer}`", + "rendered": null, + "spans": [] + } + ], + "code": { + "code": "E0277", + "explanation": "\nYou tried to use a type which doesn't implement some trait in a place which\nexpected that trait. Erroneous code example:\n\n```compile_fail,E0277\n// here we declare the Foo trait with a bar method\ntrait Foo {\n fn bar(&self);\n}\n\n// we now declare a function which takes an object implementing the Foo trait\nfn some_func(foo: T) {\n foo.bar();\n}\n\nfn main() {\n // we now call the method with the i32 type, which doesn't implement\n // the Foo trait\n some_func(5i32); // error: the trait bound `i32 : Foo` is not satisfied\n}\n```\n\nIn order to fix this error, verify that the type you're using does implement\nthe trait. Example:\n\n```\ntrait Foo {\n fn bar(&self);\n}\n\nfn some_func(foo: T) {\n foo.bar(); // we can now use this method since i32 implements the\n // Foo trait\n}\n\n// we implement the trait on the i32 type\nimpl Foo for i32 {\n fn bar(&self) {}\n}\n\nfn main() {\n some_func(5i32); // ok!\n}\n```\n\nOr in a generic context, an erroneous code example would look like:\n\n```compile_fail,E0277\nfn some_func(foo: T) {\n println!(\"{:?}\", foo); // error: the trait `core::fmt::Debug` is not\n // implemented for the type `T`\n}\n\nfn main() {\n // We now call the method with the i32 type,\n // which *does* implement the Debug trait.\n some_func(5i32);\n}\n```\n\nNote that the error here is in the definition of the generic function: Although\nwe only call it with a parameter that does implement `Debug`, the compiler\nstill rejects the function: It must work with all possible input types. In\norder to make this example compile, we need to restrict the generic type we're\naccepting:\n\n```\nuse std::fmt;\n\n// Restrict the input type to types that implement Debug.\nfn some_func(foo: T) {\n println!(\"{:?}\", foo);\n}\n\nfn main() {\n // Calling the method is still fine, as i32 implements Debug.\n some_func(5i32);\n\n // This would fail to compile now:\n // struct WithoutDebug;\n // some_func(WithoutDebug);\n}\n```\n\nRust only looks at the signature of the called function, as such it must\nalready specify all requirements that will be used for every type parameter.\n" + }, + "level": "error", + "message": "can't compare `{integer}` with `&str`", + "spans": [ + { + "byte_end": 155, + "byte_start": 153, + "column_end": 33, + "column_start": 31, + "expansion": { + "def_site_span": { + "byte_end": 940, + "byte_start": 0, + "column_end": 6, + "column_start": 1, + "expansion": null, + "file_name": "<::core::macros::assert_eq macros>", + "is_primary": false, + "label": null, + "line_end": 36, + "line_start": 1, + "suggested_replacement": null, + "suggestion_applicability": null, + "text": [ + { + "highlight_end": 35, + "highlight_start": 1, + "text": "($ left : expr, $ right : expr) =>" + }, + { + "highlight_end": 3, + "highlight_start": 1, + "text": "({" + }, + { + "highlight_end": 33, + "highlight_start": 1, + "text": " match (& $ left, & $ right)" + }, + { + "highlight_end": 7, + "highlight_start": 1, + "text": " {" + }, + { + "highlight_end": 34, + "highlight_start": 1, + "text": " (left_val, right_val) =>" + }, + { + "highlight_end": 11, + "highlight_start": 1, + "text": " {" + }, + { + "highlight_end": 46, + "highlight_start": 1, + "text": " if ! (* left_val == * right_val)" + }, + { + "highlight_end": 15, + "highlight_start": 1, + "text": " {" + }, + { + "highlight_end": 25, + "highlight_start": 1, + "text": " panic !" + }, + { + "highlight_end": 57, + "highlight_start": 1, + "text": " (r#\"assertion failed: `(left == right)`" + }, + { + "highlight_end": 16, + "highlight_start": 1, + "text": " left: `{:?}`," + }, + { + "highlight_end": 18, + "highlight_start": 1, + "text": " right: `{:?}`\"#," + }, + { + "highlight_end": 47, + "highlight_start": 1, + "text": " & * left_val, & * right_val)" + }, + { + "highlight_end": 15, + "highlight_start": 1, + "text": " }" + }, + { + "highlight_end": 11, + "highlight_start": 1, + "text": " }" + }, + { + "highlight_end": 7, + "highlight_start": 1, + "text": " }" + }, + { + "highlight_end": 42, + "highlight_start": 1, + "text": " }) ; ($ left : expr, $ right : expr,) =>" + }, + { + "highlight_end": 49, + "highlight_start": 1, + "text": "({ $ crate :: assert_eq ! ($ left, $ right) }) ;" + }, + { + "highlight_end": 53, + "highlight_start": 1, + "text": "($ left : expr, $ right : expr, $ ($ arg : tt) +) =>" + }, + { + "highlight_end": 3, + "highlight_start": 1, + "text": "({" + }, + { + "highlight_end": 37, + "highlight_start": 1, + "text": " match (& ($ left), & ($ right))" + }, + { + "highlight_end": 7, + "highlight_start": 1, + "text": " {" + }, + { + "highlight_end": 34, + "highlight_start": 1, + "text": " (left_val, right_val) =>" + }, + { + "highlight_end": 11, + "highlight_start": 1, + "text": " {" + }, + { + "highlight_end": 46, + "highlight_start": 1, + "text": " if ! (* left_val == * right_val)" + }, + { + "highlight_end": 15, + "highlight_start": 1, + "text": " {" + }, + { + "highlight_end": 25, + "highlight_start": 1, + "text": " panic !" + }, + { + "highlight_end": 57, + "highlight_start": 1, + "text": " (r#\"assertion failed: `(left == right)`" + }, + { + "highlight_end": 16, + "highlight_start": 1, + "text": " left: `{:?}`," + }, + { + "highlight_end": 22, + "highlight_start": 1, + "text": " right: `{:?}`: {}\"#," + }, + { + "highlight_end": 72, + "highlight_start": 1, + "text": " & * left_val, & * right_val, $ crate :: format_args !" + }, + { + "highlight_end": 33, + "highlight_start": 1, + "text": " ($ ($ arg) +))" + }, + { + "highlight_end": 15, + "highlight_start": 1, + "text": " }" + }, + { + "highlight_end": 11, + "highlight_start": 1, + "text": " }" + }, + { + "highlight_end": 7, + "highlight_start": 1, + "text": " }" + }, + { + "highlight_end": 6, + "highlight_start": 1, + "text": " }) ;" + } + ] + }, + "macro_decl_name": "assert_eq!", + "span": { + "byte_end": 38, + "byte_start": 16, + "column_end": 27, + "column_start": 5, + "expansion": null, + "file_name": "src/main.rs", + "is_primary": false, + "label": null, + "line_end": 2, + "line_start": 2, + "suggested_replacement": null, + "suggestion_applicability": null, + "text": [ + { + "highlight_end": 27, + "highlight_start": 5, + "text": " assert_eq!(1, \"love\");" + } + ] + } + }, + "file_name": "<::core::macros::assert_eq macros>", + "is_primary": true, + "label": "no implementation for `{integer} == &str`", + "line_end": 7, + "line_start": 7, + "suggested_replacement": null, + "suggestion_applicability": null, + "text": [ + { + "highlight_end": 33, + "highlight_start": 31, + "text": " if ! (* left_val == * right_val)" + } + ] + } + ] +}"##, + ); + + let workspace_root = PathBuf::from("/test/"); + let diag = + map_rust_diagnostic_to_lsp(&diag, &workspace_root).expect("couldn't map diagnostic"); + insta::assert_debug_snapshot!(diag); + } +} diff --git a/crates/ra_lsp_server/src/snapshots/test__snap_clippy_pass_by_ref.snap b/crates/ra_lsp_server/src/snapshots/test__snap_clippy_pass_by_ref.snap new file mode 100644 index 0000000000..a5ce291572 --- /dev/null +++ b/crates/ra_lsp_server/src/snapshots/test__snap_clippy_pass_by_ref.snap @@ -0,0 +1,85 @@ +--- +source: crates/ra_lsp_server/src/cargo_check.rs +expression: diag +--- +MappedRustDiagnostic { + location: Location { + uri: "file:///test/compiler/mir/tagset.rs", + range: Range { + start: Position { + line: 41, + character: 23, + }, + end: Position { + line: 41, + character: 28, + }, + }, + }, + diagnostic: Diagnostic { + range: Range { + start: Position { + line: 41, + character: 23, + }, + end: Position { + line: 41, + character: 28, + }, + }, + severity: Some( + Warning, + ), + code: Some( + String( + "trivially_copy_pass_by_ref", + ), + ), + source: Some( + "clippy", + ), + message: "this argument is passed by reference, but would be more efficient if passed by value\n#[warn(clippy::trivially_copy_pass_by_ref)] implied by #[warn(clippy::all)]\nfor further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref", + related_information: Some( + [ + DiagnosticRelatedInformation { + location: Location { + uri: "file:///test/compiler/lib.rs", + range: Range { + start: Position { + line: 0, + character: 8, + }, + end: Position { + line: 0, + character: 19, + }, + }, + }, + message: "lint level defined here", + }, + ], + ), + tags: None, + }, + suggested_fixes: [ + SuggestedFix { + 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, + }, + }, + }, + replacement: "self", + applicability: Unspecified, + diagnostics: [], + }, + ], +} diff --git a/crates/ra_lsp_server/src/snapshots/test__snap_handles_macro_location.snap b/crates/ra_lsp_server/src/snapshots/test__snap_handles_macro_location.snap new file mode 100644 index 0000000000..07e363ebf1 --- /dev/null +++ b/crates/ra_lsp_server/src/snapshots/test__snap_handles_macro_location.snap @@ -0,0 +1,46 @@ +--- +source: crates/ra_lsp_server/src/cargo_check.rs +expression: diag +--- +MappedRustDiagnostic { + location: Location { + uri: "file:///test/src/main.rs", + range: Range { + start: Position { + line: 1, + character: 4, + }, + end: Position { + line: 1, + character: 26, + }, + }, + }, + diagnostic: Diagnostic { + range: Range { + start: Position { + line: 1, + character: 4, + }, + end: Position { + line: 1, + character: 26, + }, + }, + severity: Some( + Error, + ), + code: Some( + String( + "E0277", + ), + ), + source: Some( + "rustc", + ), + message: "can\'t compare `{integer}` with `&str`\nthe trait `std::cmp::PartialEq<&str>` is not implemented for `{integer}`", + related_information: None, + tags: None, + }, + suggested_fixes: [], +} diff --git a/crates/ra_lsp_server/src/snapshots/test__snap_rustc_incompatible_type_for_trait.snap b/crates/ra_lsp_server/src/snapshots/test__snap_rustc_incompatible_type_for_trait.snap new file mode 100644 index 0000000000..85a87db0b8 --- /dev/null +++ b/crates/ra_lsp_server/src/snapshots/test__snap_rustc_incompatible_type_for_trait.snap @@ -0,0 +1,46 @@ +--- +source: crates/ra_lsp_server/src/cargo_check.rs +expression: diag +--- +MappedRustDiagnostic { + location: Location { + uri: "file:///test/compiler/ty/list_iter.rs", + range: Range { + start: Position { + line: 51, + character: 4, + }, + end: Position { + line: 51, + character: 47, + }, + }, + }, + diagnostic: Diagnostic { + range: Range { + start: Position { + line: 51, + character: 4, + }, + end: Position { + line: 51, + character: 47, + }, + }, + severity: Some( + Error, + ), + code: Some( + String( + "E0053", + ), + ), + source: Some( + "rustc", + ), + message: "method `next` has an incompatible type for trait\nexpected type `fn(&mut ty::list_iter::ListIterator<\'list, M>) -> std::option::Option<&ty::Ref>`\n found type `fn(&ty::list_iter::ListIterator<\'list, M>) -> std::option::Option<&\'list ty::Ref>`", + related_information: None, + tags: None, + }, + suggested_fixes: [], +} diff --git a/crates/ra_lsp_server/src/snapshots/test__snap_rustc_mismatched_type.snap b/crates/ra_lsp_server/src/snapshots/test__snap_rustc_mismatched_type.snap new file mode 100644 index 0000000000..69cb8badfc --- /dev/null +++ b/crates/ra_lsp_server/src/snapshots/test__snap_rustc_mismatched_type.snap @@ -0,0 +1,46 @@ +--- +source: crates/ra_lsp_server/src/cargo_check.rs +expression: diag +--- +MappedRustDiagnostic { + location: Location { + uri: "file:///test/runtime/compiler_support.rs", + range: Range { + start: Position { + line: 47, + character: 64, + }, + end: Position { + line: 47, + character: 69, + }, + }, + }, + diagnostic: Diagnostic { + range: Range { + start: Position { + line: 47, + character: 64, + }, + end: Position { + line: 47, + character: 69, + }, + }, + severity: Some( + Error, + ), + code: Some( + String( + "E0308", + ), + ), + source: Some( + "rustc", + ), + message: "mismatched types\nexpected usize, found u32", + related_information: None, + tags: None, + }, + suggested_fixes: [], +} diff --git a/crates/ra_lsp_server/src/snapshots/test__snap_rustc_unused_variable.snap b/crates/ra_lsp_server/src/snapshots/test__snap_rustc_unused_variable.snap new file mode 100644 index 0000000000..33a3e30348 --- /dev/null +++ b/crates/ra_lsp_server/src/snapshots/test__snap_rustc_unused_variable.snap @@ -0,0 +1,70 @@ +--- +source: crates/ra_lsp_server/src/cargo_check.rs +expression: diag +--- +MappedRustDiagnostic { + location: Location { + uri: "file:///test/driver/subcommand/repl.rs", + range: Range { + start: Position { + line: 290, + character: 8, + }, + end: Position { + line: 290, + character: 11, + }, + }, + }, + diagnostic: Diagnostic { + range: Range { + start: Position { + line: 290, + character: 8, + }, + end: Position { + line: 290, + character: 11, + }, + }, + severity: Some( + Warning, + ), + code: Some( + String( + "unused_variables", + ), + ), + source: Some( + "rustc", + ), + message: "unused variable: `foo`\n#[warn(unused_variables)] on by default", + related_information: None, + tags: Some( + [ + Unnecessary, + ], + ), + }, + suggested_fixes: [ + SuggestedFix { + 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, + }, + }, + }, + replacement: "_foo", + applicability: MachineApplicable, + diagnostics: [], + }, + ], +} diff --git a/crates/ra_lsp_server/src/snapshots/test__snap_rustc_wrong_number_of_parameters.snap b/crates/ra_lsp_server/src/snapshots/test__snap_rustc_wrong_number_of_parameters.snap new file mode 100644 index 0000000000..ced6fa4df1 --- /dev/null +++ b/crates/ra_lsp_server/src/snapshots/test__snap_rustc_wrong_number_of_parameters.snap @@ -0,0 +1,65 @@ +--- +source: crates/ra_lsp_server/src/cargo_check.rs +expression: diag +--- +MappedRustDiagnostic { + location: Location { + uri: "file:///test/compiler/ty/select.rs", + range: Range { + start: Position { + line: 103, + character: 17, + }, + end: Position { + line: 103, + character: 29, + }, + }, + }, + diagnostic: Diagnostic { + range: Range { + start: Position { + line: 103, + character: 17, + }, + end: Position { + line: 103, + character: 29, + }, + }, + severity: Some( + Error, + ), + code: Some( + String( + "E0061", + ), + ), + source: Some( + "rustc", + ), + message: "this function takes 2 parameters but 3 parameters were supplied\nexpected 2 parameters", + related_information: Some( + [ + DiagnosticRelatedInformation { + location: Location { + uri: "file:///test/compiler/ty/select.rs", + range: Range { + start: Position { + line: 218, + character: 4, + }, + end: Position { + line: 230, + character: 5, + }, + }, + }, + message: "defined here", + }, + ], + ), + tags: None, + }, + suggested_fixes: [], +} From 500fe46e6c0df7827d56c7cd07741533422e7743 Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Wed, 25 Dec 2019 17:34:51 +0100 Subject: [PATCH 06/20] Remove cargo watch supporting code and tests from vscode extension --- .../clippy/trivially_copy_pass_by_ref.json | 110 ------- .../rust-diagnostics/error/E0053.json | 42 --- .../rust-diagnostics/error/E0061.json | 114 ------- .../rust-diagnostics/error/E0277.json | 261 --------------- .../rust-diagnostics/error/E0308.json | 33 -- .../warning/unused_variables.json | 72 ----- .../utils/diagnotics/SuggestedFix.test.ts | 134 -------- .../diagnotics/SuggestedFixCollection.test.ts | 127 -------- .../src/test/utils/diagnotics/rust.test.ts | 236 -------------- .../src/test/utils/diagnotics/vscode.test.ts | 98 ------ .../src/utils/diagnostics/SuggestedFix.ts | 67 ---- .../diagnostics/SuggestedFixCollection.ts | 77 ----- editors/code/src/utils/diagnostics/rust.ts | 299 ------------------ editors/code/src/utils/diagnostics/vscode.ts | 14 - 14 files changed, 1684 deletions(-) delete mode 100644 editors/code/src/test/fixtures/rust-diagnostics/clippy/trivially_copy_pass_by_ref.json delete mode 100644 editors/code/src/test/fixtures/rust-diagnostics/error/E0053.json delete mode 100644 editors/code/src/test/fixtures/rust-diagnostics/error/E0061.json delete mode 100644 editors/code/src/test/fixtures/rust-diagnostics/error/E0277.json delete mode 100644 editors/code/src/test/fixtures/rust-diagnostics/error/E0308.json delete mode 100644 editors/code/src/test/fixtures/rust-diagnostics/warning/unused_variables.json delete mode 100644 editors/code/src/test/utils/diagnotics/SuggestedFix.test.ts delete mode 100644 editors/code/src/test/utils/diagnotics/SuggestedFixCollection.test.ts delete mode 100644 editors/code/src/test/utils/diagnotics/rust.test.ts delete mode 100644 editors/code/src/test/utils/diagnotics/vscode.test.ts delete mode 100644 editors/code/src/utils/diagnostics/SuggestedFix.ts delete mode 100644 editors/code/src/utils/diagnostics/SuggestedFixCollection.ts delete mode 100644 editors/code/src/utils/diagnostics/rust.ts delete mode 100644 editors/code/src/utils/diagnostics/vscode.ts diff --git a/editors/code/src/test/fixtures/rust-diagnostics/clippy/trivially_copy_pass_by_ref.json b/editors/code/src/test/fixtures/rust-diagnostics/clippy/trivially_copy_pass_by_ref.json deleted file mode 100644 index d874e99bc5..0000000000 --- a/editors/code/src/test/fixtures/rust-diagnostics/clippy/trivially_copy_pass_by_ref.json +++ /dev/null @@ -1,110 +0,0 @@ -{ - "message": "this argument is passed by reference, but would be more efficient if passed by value", - "code": { - "code": "clippy::trivially_copy_pass_by_ref", - "explanation": null - }, - "level": "warning", - "spans": [ - { - "file_name": "compiler/mir/tagset.rs", - "byte_start": 941, - "byte_end": 946, - "line_start": 42, - "line_end": 42, - "column_start": 24, - "column_end": 29, - "is_primary": true, - "text": [ - { - "text": " pub fn is_disjoint(&self, other: Self) -> bool {", - "highlight_start": 24, - "highlight_end": 29 - } - ], - "label": null, - "suggested_replacement": null, - "suggestion_applicability": null, - "expansion": null - } - ], - "children": [ - { - "message": "lint level defined here", - "code": null, - "level": "note", - "spans": [ - { - "file_name": "compiler/lib.rs", - "byte_start": 8, - "byte_end": 19, - "line_start": 1, - "line_end": 1, - "column_start": 9, - "column_end": 20, - "is_primary": true, - "text": [ - { - "text": "#![warn(clippy::all)]", - "highlight_start": 9, - "highlight_end": 20 - } - ], - "label": null, - "suggested_replacement": null, - "suggestion_applicability": null, - "expansion": null - } - ], - "children": [], - "rendered": null - }, - { - "message": "#[warn(clippy::trivially_copy_pass_by_ref)] implied by #[warn(clippy::all)]", - "code": null, - "level": "note", - "spans": [], - "children": [], - "rendered": null - }, - { - "message": "for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref", - "code": null, - "level": "help", - "spans": [], - "children": [], - "rendered": null - }, - { - "message": "consider passing by value instead", - "code": null, - "level": "help", - "spans": [ - { - "file_name": "compiler/mir/tagset.rs", - "byte_start": 941, - "byte_end": 946, - "line_start": 42, - "line_end": 42, - "column_start": 24, - "column_end": 29, - "is_primary": true, - "text": [ - { - "text": " pub fn is_disjoint(&self, other: Self) -> bool {", - "highlight_start": 24, - "highlight_end": 29 - } - ], - "label": null, - "suggested_replacement": "self", - "suggestion_applicability": "Unspecified", - "expansion": null - } - ], - "children": [], - "rendered": null - } - ], - "rendered": "warning: this argument is passed by reference, but would be more efficient if passed by value\n --> compiler/mir/tagset.rs:42:24\n |\n42 | pub fn is_disjoint(&self, other: Self) -> bool {\n | ^^^^^ help: consider passing by value instead: `self`\n |\nnote: lint level defined here\n --> compiler/lib.rs:1:9\n |\n1 | #![warn(clippy::all)]\n | ^^^^^^^^^^^\n = note: #[warn(clippy::trivially_copy_pass_by_ref)] implied by #[warn(clippy::all)]\n = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref\n\n" -} diff --git a/editors/code/src/test/fixtures/rust-diagnostics/error/E0053.json b/editors/code/src/test/fixtures/rust-diagnostics/error/E0053.json deleted file mode 100644 index ea5c976d1d..0000000000 --- a/editors/code/src/test/fixtures/rust-diagnostics/error/E0053.json +++ /dev/null @@ -1,42 +0,0 @@ -{ - "message": "method `next` has an incompatible type for trait", - "code": { - "code": "E0053", - "explanation": "\nThe parameters of any trait method must match between a trait implementation\nand the trait definition.\n\nHere are a couple examples of this error:\n\n```compile_fail,E0053\ntrait Foo {\n fn foo(x: u16);\n fn bar(&self);\n}\n\nstruct Bar;\n\nimpl Foo for Bar {\n // error, expected u16, found i16\n fn foo(x: i16) { }\n\n // error, types differ in mutability\n fn bar(&mut self) { }\n}\n```\n" - }, - "level": "error", - "spans": [ - { - "file_name": "compiler/ty/list_iter.rs", - "byte_start": 1307, - "byte_end": 1350, - "line_start": 52, - "line_end": 52, - "column_start": 5, - "column_end": 48, - "is_primary": true, - "text": [ - { - "text": " fn next(&self) -> Option<&'list ty::Ref> {", - "highlight_start": 5, - "highlight_end": 48 - } - ], - "label": "types differ in mutability", - "suggested_replacement": null, - "suggestion_applicability": null, - "expansion": null - } - ], - "children": [ - { - "message": "expected type `fn(&mut ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&ty::Ref>`\n found type `fn(&ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&'list ty::Ref>`", - "code": null, - "level": "note", - "spans": [], - "children": [], - "rendered": null - } - ], - "rendered": "error[E0053]: method `next` has an incompatible type for trait\n --> compiler/ty/list_iter.rs:52:5\n |\n52 | fn next(&self) -> Option<&'list ty::Ref> {\n | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ types differ in mutability\n |\n = note: expected type `fn(&mut ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&ty::Ref>`\n found type `fn(&ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&'list ty::Ref>`\n\n" -} diff --git a/editors/code/src/test/fixtures/rust-diagnostics/error/E0061.json b/editors/code/src/test/fixtures/rust-diagnostics/error/E0061.json deleted file mode 100644 index 3154d1098c..0000000000 --- a/editors/code/src/test/fixtures/rust-diagnostics/error/E0061.json +++ /dev/null @@ -1,114 +0,0 @@ -{ - "message": "this function takes 2 parameters but 3 parameters were supplied", - "code": { - "code": "E0061", - "explanation": "\nThe number of arguments passed to a function must match the number of arguments\nspecified in the function signature.\n\nFor example, a function like:\n\n```\nfn f(a: u16, b: &str) {}\n```\n\nMust always be called with exactly two arguments, e.g., `f(2, \"test\")`.\n\nNote that Rust does not have a notion of optional function arguments or\nvariadic functions (except for its C-FFI).\n" - }, - "level": "error", - "spans": [ - { - "file_name": "compiler/ty/select.rs", - "byte_start": 8787, - "byte_end": 9241, - "line_start": 219, - "line_end": 231, - "column_start": 5, - "column_end": 6, - "is_primary": false, - "text": [ - { - "text": " pub fn add_evidence(", - "highlight_start": 5, - "highlight_end": 25 - }, - { - "text": " &mut self,", - "highlight_start": 1, - "highlight_end": 19 - }, - { - "text": " target_poly: &ty::Ref,", - "highlight_start": 1, - "highlight_end": 41 - }, - { - "text": " evidence_poly: &ty::Ref,", - "highlight_start": 1, - "highlight_end": 43 - }, - { - "text": " ) {", - "highlight_start": 1, - "highlight_end": 8 - }, - { - "text": " match target_poly {", - "highlight_start": 1, - "highlight_end": 28 - }, - { - "text": " ty::Ref::Var(tvar, _) => self.add_var_evidence(tvar, evidence_poly),", - "highlight_start": 1, - "highlight_end": 81 - }, - { - "text": " ty::Ref::Fixed(target_ty) => {", - "highlight_start": 1, - "highlight_end": 43 - }, - { - "text": " let evidence_ty = evidence_poly.resolve_to_ty();", - "highlight_start": 1, - "highlight_end": 65 - }, - { - "text": " self.add_evidence_ty(target_ty, evidence_poly, evidence_ty)", - "highlight_start": 1, - "highlight_end": 76 - }, - { - "text": " }", - "highlight_start": 1, - "highlight_end": 14 - }, - { - "text": " }", - "highlight_start": 1, - "highlight_end": 10 - }, - { - "text": " }", - "highlight_start": 1, - "highlight_end": 6 - } - ], - "label": "defined here", - "suggested_replacement": null, - "suggestion_applicability": null, - "expansion": null - }, - { - "file_name": "compiler/ty/select.rs", - "byte_start": 4045, - "byte_end": 4057, - "line_start": 104, - "line_end": 104, - "column_start": 18, - "column_end": 30, - "is_primary": true, - "text": [ - { - "text": " self.add_evidence(target_fixed, evidence_fixed, false);", - "highlight_start": 18, - "highlight_end": 30 - } - ], - "label": "expected 2 parameters", - "suggested_replacement": null, - "suggestion_applicability": null, - "expansion": null - } - ], - "children": [], - "rendered": "error[E0061]: this function takes 2 parameters but 3 parameters were supplied\n --> compiler/ty/select.rs:104:18\n |\n104 | self.add_evidence(target_fixed, evidence_fixed, false);\n | ^^^^^^^^^^^^ expected 2 parameters\n...\n219 | / pub fn add_evidence(\n220 | | &mut self,\n221 | | target_poly: &ty::Ref,\n222 | | evidence_poly: &ty::Ref,\n... |\n230 | | }\n231 | | }\n | |_____- defined here\n\n" -} diff --git a/editors/code/src/test/fixtures/rust-diagnostics/error/E0277.json b/editors/code/src/test/fixtures/rust-diagnostics/error/E0277.json deleted file mode 100644 index bfef33c7de..0000000000 --- a/editors/code/src/test/fixtures/rust-diagnostics/error/E0277.json +++ /dev/null @@ -1,261 +0,0 @@ -{ - "rendered": "error[E0277]: can't compare `{integer}` with `&str`\n --> src/main.rs:2:5\n |\n2 | assert_eq!(1, \"love\");\n | ^^^^^^^^^^^^^^^^^^^^^^ no implementation for `{integer} == &str`\n |\n = help: the trait `std::cmp::PartialEq<&str>` is not implemented for `{integer}`\n = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)\n\n", - "children": [ - { - "children": [], - "code": null, - "level": "help", - "message": "the trait `std::cmp::PartialEq<&str>` is not implemented for `{integer}`", - "rendered": null, - "spans": [] - } - ], - "code": { - "code": "E0277", - "explanation": "\nYou tried to use a type which doesn't implement some trait in a place which\nexpected that trait. Erroneous code example:\n\n```compile_fail,E0277\n// here we declare the Foo trait with a bar method\ntrait Foo {\n fn bar(&self);\n}\n\n// we now declare a function which takes an object implementing the Foo trait\nfn some_func(foo: T) {\n foo.bar();\n}\n\nfn main() {\n // we now call the method with the i32 type, which doesn't implement\n // the Foo trait\n some_func(5i32); // error: the trait bound `i32 : Foo` is not satisfied\n}\n```\n\nIn order to fix this error, verify that the type you're using does implement\nthe trait. Example:\n\n```\ntrait Foo {\n fn bar(&self);\n}\n\nfn some_func(foo: T) {\n foo.bar(); // we can now use this method since i32 implements the\n // Foo trait\n}\n\n// we implement the trait on the i32 type\nimpl Foo for i32 {\n fn bar(&self) {}\n}\n\nfn main() {\n some_func(5i32); // ok!\n}\n```\n\nOr in a generic context, an erroneous code example would look like:\n\n```compile_fail,E0277\nfn some_func(foo: T) {\n println!(\"{:?}\", foo); // error: the trait `core::fmt::Debug` is not\n // implemented for the type `T`\n}\n\nfn main() {\n // We now call the method with the i32 type,\n // which *does* implement the Debug trait.\n some_func(5i32);\n}\n```\n\nNote that the error here is in the definition of the generic function: Although\nwe only call it with a parameter that does implement `Debug`, the compiler\nstill rejects the function: It must work with all possible input types. In\norder to make this example compile, we need to restrict the generic type we're\naccepting:\n\n```\nuse std::fmt;\n\n// Restrict the input type to types that implement Debug.\nfn some_func(foo: T) {\n println!(\"{:?}\", foo);\n}\n\nfn main() {\n // Calling the method is still fine, as i32 implements Debug.\n some_func(5i32);\n\n // This would fail to compile now:\n // struct WithoutDebug;\n // some_func(WithoutDebug);\n}\n```\n\nRust only looks at the signature of the called function, as such it must\nalready specify all requirements that will be used for every type parameter.\n" - }, - "level": "error", - "message": "can't compare `{integer}` with `&str`", - "spans": [ - { - "byte_end": 155, - "byte_start": 153, - "column_end": 33, - "column_start": 31, - "expansion": { - "def_site_span": { - "byte_end": 940, - "byte_start": 0, - "column_end": 6, - "column_start": 1, - "expansion": null, - "file_name": "<::core::macros::assert_eq macros>", - "is_primary": false, - "label": null, - "line_end": 36, - "line_start": 1, - "suggested_replacement": null, - "suggestion_applicability": null, - "text": [ - { - "highlight_end": 35, - "highlight_start": 1, - "text": "($ left : expr, $ right : expr) =>" - }, - { - "highlight_end": 3, - "highlight_start": 1, - "text": "({" - }, - { - "highlight_end": 33, - "highlight_start": 1, - "text": " match (& $ left, & $ right)" - }, - { - "highlight_end": 7, - "highlight_start": 1, - "text": " {" - }, - { - "highlight_end": 34, - "highlight_start": 1, - "text": " (left_val, right_val) =>" - }, - { - "highlight_end": 11, - "highlight_start": 1, - "text": " {" - }, - { - "highlight_end": 46, - "highlight_start": 1, - "text": " if ! (* left_val == * right_val)" - }, - { - "highlight_end": 15, - "highlight_start": 1, - "text": " {" - }, - { - "highlight_end": 25, - "highlight_start": 1, - "text": " panic !" - }, - { - "highlight_end": 57, - "highlight_start": 1, - "text": " (r#\"assertion failed: `(left == right)`" - }, - { - "highlight_end": 16, - "highlight_start": 1, - "text": " left: `{:?}`," - }, - { - "highlight_end": 18, - "highlight_start": 1, - "text": " right: `{:?}`\"#," - }, - { - "highlight_end": 47, - "highlight_start": 1, - "text": " & * left_val, & * right_val)" - }, - { - "highlight_end": 15, - "highlight_start": 1, - "text": " }" - }, - { - "highlight_end": 11, - "highlight_start": 1, - "text": " }" - }, - { - "highlight_end": 7, - "highlight_start": 1, - "text": " }" - }, - { - "highlight_end": 42, - "highlight_start": 1, - "text": " }) ; ($ left : expr, $ right : expr,) =>" - }, - { - "highlight_end": 49, - "highlight_start": 1, - "text": "({ $ crate :: assert_eq ! ($ left, $ right) }) ;" - }, - { - "highlight_end": 53, - "highlight_start": 1, - "text": "($ left : expr, $ right : expr, $ ($ arg : tt) +) =>" - }, - { - "highlight_end": 3, - "highlight_start": 1, - "text": "({" - }, - { - "highlight_end": 37, - "highlight_start": 1, - "text": " match (& ($ left), & ($ right))" - }, - { - "highlight_end": 7, - "highlight_start": 1, - "text": " {" - }, - { - "highlight_end": 34, - "highlight_start": 1, - "text": " (left_val, right_val) =>" - }, - { - "highlight_end": 11, - "highlight_start": 1, - "text": " {" - }, - { - "highlight_end": 46, - "highlight_start": 1, - "text": " if ! (* left_val == * right_val)" - }, - { - "highlight_end": 15, - "highlight_start": 1, - "text": " {" - }, - { - "highlight_end": 25, - "highlight_start": 1, - "text": " panic !" - }, - { - "highlight_end": 57, - "highlight_start": 1, - "text": " (r#\"assertion failed: `(left == right)`" - }, - { - "highlight_end": 16, - "highlight_start": 1, - "text": " left: `{:?}`," - }, - { - "highlight_end": 22, - "highlight_start": 1, - "text": " right: `{:?}`: {}\"#," - }, - { - "highlight_end": 72, - "highlight_start": 1, - "text": " & * left_val, & * right_val, $ crate :: format_args !" - }, - { - "highlight_end": 33, - "highlight_start": 1, - "text": " ($ ($ arg) +))" - }, - { - "highlight_end": 15, - "highlight_start": 1, - "text": " }" - }, - { - "highlight_end": 11, - "highlight_start": 1, - "text": " }" - }, - { - "highlight_end": 7, - "highlight_start": 1, - "text": " }" - }, - { - "highlight_end": 6, - "highlight_start": 1, - "text": " }) ;" - } - ] - }, - "macro_decl_name": "assert_eq!", - "span": { - "byte_end": 38, - "byte_start": 16, - "column_end": 27, - "column_start": 5, - "expansion": null, - "file_name": "src/main.rs", - "is_primary": false, - "label": null, - "line_end": 2, - "line_start": 2, - "suggested_replacement": null, - "suggestion_applicability": null, - "text": [ - { - "highlight_end": 27, - "highlight_start": 5, - "text": " assert_eq!(1, \"love\");" - } - ] - } - }, - "file_name": "<::core::macros::assert_eq macros>", - "is_primary": true, - "label": "no implementation for `{integer} == &str`", - "line_end": 7, - "line_start": 7, - "suggested_replacement": null, - "suggestion_applicability": null, - "text": [ - { - "highlight_end": 33, - "highlight_start": 31, - "text": " if ! (* left_val == * right_val)" - } - ] - } - ] -} diff --git a/editors/code/src/test/fixtures/rust-diagnostics/error/E0308.json b/editors/code/src/test/fixtures/rust-diagnostics/error/E0308.json deleted file mode 100644 index fb23824a3c..0000000000 --- a/editors/code/src/test/fixtures/rust-diagnostics/error/E0308.json +++ /dev/null @@ -1,33 +0,0 @@ -{ - "message": "mismatched types", - "code": { - "code": "E0308", - "explanation": "\nThis error occurs when the compiler was unable to infer the concrete type of a\nvariable. It can occur for several cases, the most common of which is a\nmismatch in the expected type that the compiler inferred for a variable's\ninitializing expression, and the actual type explicitly assigned to the\nvariable.\n\nFor example:\n\n```compile_fail,E0308\nlet x: i32 = \"I am not a number!\";\n// ~~~ ~~~~~~~~~~~~~~~~~~~~\n// | |\n// | initializing expression;\n// | compiler infers type `&str`\n// |\n// type `i32` assigned to variable `x`\n```\n" - }, - "level": "error", - "spans": [ - { - "file_name": "runtime/compiler_support.rs", - "byte_start": 1589, - "byte_end": 1594, - "line_start": 48, - "line_end": 48, - "column_start": 65, - "column_end": 70, - "is_primary": true, - "text": [ - { - "text": " let layout = alloc::Layout::from_size_align_unchecked(size, align);", - "highlight_start": 65, - "highlight_end": 70 - } - ], - "label": "expected usize, found u32", - "suggested_replacement": null, - "suggestion_applicability": null, - "expansion": null - } - ], - "children": [], - "rendered": "error[E0308]: mismatched types\n --> runtime/compiler_support.rs:48:65\n |\n48 | let layout = alloc::Layout::from_size_align_unchecked(size, align);\n | ^^^^^ expected usize, found u32\n\n" -} diff --git a/editors/code/src/test/fixtures/rust-diagnostics/warning/unused_variables.json b/editors/code/src/test/fixtures/rust-diagnostics/warning/unused_variables.json deleted file mode 100644 index d1e2be7221..0000000000 --- a/editors/code/src/test/fixtures/rust-diagnostics/warning/unused_variables.json +++ /dev/null @@ -1,72 +0,0 @@ -{ - "message": "unused variable: `foo`", - "code": { - "code": "unused_variables", - "explanation": null - }, - "level": "warning", - "spans": [ - { - "file_name": "driver/subcommand/repl.rs", - "byte_start": 9228, - "byte_end": 9231, - "line_start": 291, - "line_end": 291, - "column_start": 9, - "column_end": 12, - "is_primary": true, - "text": [ - { - "text": " let foo = 42;", - "highlight_start": 9, - "highlight_end": 12 - } - ], - "label": null, - "suggested_replacement": null, - "suggestion_applicability": null, - "expansion": null - } - ], - "children": [ - { - "message": "#[warn(unused_variables)] on by default", - "code": null, - "level": "note", - "spans": [], - "children": [], - "rendered": null - }, - { - "message": "consider prefixing with an underscore", - "code": null, - "level": "help", - "spans": [ - { - "file_name": "driver/subcommand/repl.rs", - "byte_start": 9228, - "byte_end": 9231, - "line_start": 291, - "line_end": 291, - "column_start": 9, - "column_end": 12, - "is_primary": true, - "text": [ - { - "text": " let foo = 42;", - "highlight_start": 9, - "highlight_end": 12 - } - ], - "label": null, - "suggested_replacement": "_foo", - "suggestion_applicability": "MachineApplicable", - "expansion": null - } - ], - "children": [], - "rendered": null - } - ], - "rendered": "warning: unused variable: `foo`\n --> driver/subcommand/repl.rs:291:9\n |\n291 | let foo = 42;\n | ^^^ help: consider prefixing with an underscore: `_foo`\n |\n = note: #[warn(unused_variables)] on by default\n\n" -} diff --git a/editors/code/src/test/utils/diagnotics/SuggestedFix.test.ts b/editors/code/src/test/utils/diagnotics/SuggestedFix.test.ts deleted file mode 100644 index 2b25eb705d..0000000000 --- a/editors/code/src/test/utils/diagnotics/SuggestedFix.test.ts +++ /dev/null @@ -1,134 +0,0 @@ -import * as assert from 'assert'; -import * as vscode from 'vscode'; - -import { SuggestionApplicability } from '../../../utils/diagnostics/rust'; -import SuggestedFix from '../../../utils/diagnostics/SuggestedFix'; - -const location1 = new vscode.Location( - vscode.Uri.file('/file/1'), - new vscode.Range(new vscode.Position(1, 2), new vscode.Position(3, 4)), -); - -const location2 = new vscode.Location( - vscode.Uri.file('/file/2'), - new vscode.Range(new vscode.Position(5, 6), new vscode.Position(7, 8)), -); - -describe('SuggestedFix', () => { - describe('isEqual', () => { - it('should treat identical instances as equal', () => { - const suggestion1 = new SuggestedFix( - 'Replace me!', - location1, - 'With this!', - ); - - const suggestion2 = new SuggestedFix( - 'Replace me!', - location1, - 'With this!', - ); - - assert(suggestion1.isEqual(suggestion2)); - }); - - it('should treat instances with different titles as inequal', () => { - const suggestion1 = new SuggestedFix( - 'Replace me!', - location1, - 'With this!', - ); - - const suggestion2 = new SuggestedFix( - 'Not the same title!', - location1, - 'With this!', - ); - - assert(!suggestion1.isEqual(suggestion2)); - }); - - it('should treat instances with different replacements as inequal', () => { - const suggestion1 = new SuggestedFix( - 'Replace me!', - location1, - 'With this!', - ); - - const suggestion2 = new SuggestedFix( - 'Replace me!', - location1, - 'With something else!', - ); - - assert(!suggestion1.isEqual(suggestion2)); - }); - - it('should treat instances with different locations as inequal', () => { - const suggestion1 = new SuggestedFix( - 'Replace me!', - location1, - 'With this!', - ); - - const suggestion2 = new SuggestedFix( - 'Replace me!', - location2, - 'With this!', - ); - - assert(!suggestion1.isEqual(suggestion2)); - }); - - it('should treat instances with different applicability as inequal', () => { - const suggestion1 = new SuggestedFix( - 'Replace me!', - location1, - 'With this!', - SuggestionApplicability.MachineApplicable, - ); - - const suggestion2 = new SuggestedFix( - 'Replace me!', - location2, - 'With this!', - SuggestionApplicability.HasPlaceholders, - ); - - assert(!suggestion1.isEqual(suggestion2)); - }); - }); - - describe('toCodeAction', () => { - it('should map a simple suggestion', () => { - const suggestion = new SuggestedFix( - 'Replace me!', - location1, - 'With this!', - ); - - const codeAction = suggestion.toCodeAction(); - assert.strictEqual(codeAction.kind, vscode.CodeActionKind.QuickFix); - assert.strictEqual(codeAction.title, 'Replace me!'); - assert.strictEqual(codeAction.isPreferred, false); - - const edit = codeAction.edit; - if (!edit) { - assert.fail('Code Action edit unexpectedly missing'); - return; - } - - const editEntries = edit.entries(); - assert.strictEqual(editEntries.length, 1); - - const [[editUri, textEdits]] = editEntries; - assert.strictEqual(editUri.toString(), location1.uri.toString()); - - assert.strictEqual(textEdits.length, 1); - const [textEdit] = textEdits; - - assert(textEdit.range.isEqual(location1.range)); - assert.strictEqual(textEdit.newText, 'With this!'); - }); - }); -}); diff --git a/editors/code/src/test/utils/diagnotics/SuggestedFixCollection.test.ts b/editors/code/src/test/utils/diagnotics/SuggestedFixCollection.test.ts deleted file mode 100644 index ef09013f41..0000000000 --- a/editors/code/src/test/utils/diagnotics/SuggestedFixCollection.test.ts +++ /dev/null @@ -1,127 +0,0 @@ -import * as assert from 'assert'; -import * as vscode from 'vscode'; - -import SuggestedFix from '../../../utils/diagnostics/SuggestedFix'; -import SuggestedFixCollection from '../../../utils/diagnostics/SuggestedFixCollection'; - -const uri1 = vscode.Uri.file('/file/1'); -const uri2 = vscode.Uri.file('/file/2'); - -const mockDocument1 = ({ - uri: uri1, -} as unknown) as vscode.TextDocument; - -const mockDocument2 = ({ - uri: uri2, -} as unknown) as vscode.TextDocument; - -const range1 = new vscode.Range( - new vscode.Position(1, 2), - new vscode.Position(3, 4), -); -const range2 = new vscode.Range( - new vscode.Position(5, 6), - new vscode.Position(7, 8), -); - -const diagnostic1 = new vscode.Diagnostic(range1, 'First diagnostic'); -const diagnostic2 = new vscode.Diagnostic(range2, 'Second diagnostic'); - -// This is a mutable object so return a fresh instance every time -function suggestion1(): SuggestedFix { - return new SuggestedFix( - 'Replace me!', - new vscode.Location(uri1, range1), - 'With this!', - ); -} - -describe('SuggestedFixCollection', () => { - it('should add a suggestion then return it as a code action', () => { - const suggestedFixes = new SuggestedFixCollection(); - suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic1); - - // Specify the document and range that exactly matches - const codeActions = suggestedFixes.provideCodeActions( - mockDocument1, - range1, - ); - - assert.strictEqual(codeActions.length, 1); - const [codeAction] = codeActions; - assert.strictEqual(codeAction.title, suggestion1().title); - - const { diagnostics } = codeAction; - if (!diagnostics) { - assert.fail('Diagnostics unexpectedly missing'); - return; - } - - assert.strictEqual(diagnostics.length, 1); - assert.strictEqual(diagnostics[0], diagnostic1); - }); - - it('should not return code actions for different ranges', () => { - const suggestedFixes = new SuggestedFixCollection(); - suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic1); - - const codeActions = suggestedFixes.provideCodeActions( - mockDocument1, - range2, - ); - - assert(!codeActions || codeActions.length === 0); - }); - - it('should not return code actions for different documents', () => { - const suggestedFixes = new SuggestedFixCollection(); - suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic1); - - const codeActions = suggestedFixes.provideCodeActions( - mockDocument2, - range1, - ); - - assert(!codeActions || codeActions.length === 0); - }); - - it('should not return code actions that have been cleared', () => { - const suggestedFixes = new SuggestedFixCollection(); - suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic1); - suggestedFixes.clear(); - - const codeActions = suggestedFixes.provideCodeActions( - mockDocument1, - range1, - ); - - assert(!codeActions || codeActions.length === 0); - }); - - it('should merge identical suggestions together', () => { - const suggestedFixes = new SuggestedFixCollection(); - - // Add the same suggestion for two diagnostics - suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic1); - suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic2); - - const codeActions = suggestedFixes.provideCodeActions( - mockDocument1, - range1, - ); - - assert.strictEqual(codeActions.length, 1); - const [codeAction] = codeActions; - const { diagnostics } = codeAction; - - if (!diagnostics) { - assert.fail('Diagnostics unexpectedly missing'); - return; - } - - // We should be associated with both diagnostics - assert.strictEqual(diagnostics.length, 2); - assert.strictEqual(diagnostics[0], diagnostic1); - assert.strictEqual(diagnostics[1], diagnostic2); - }); -}); diff --git a/editors/code/src/test/utils/diagnotics/rust.test.ts b/editors/code/src/test/utils/diagnotics/rust.test.ts deleted file mode 100644 index 358325cc8d..0000000000 --- a/editors/code/src/test/utils/diagnotics/rust.test.ts +++ /dev/null @@ -1,236 +0,0 @@ -import * as assert from 'assert'; -import * as fs from 'fs'; -import * as vscode from 'vscode'; - -import { - MappedRustDiagnostic, - mapRustDiagnosticToVsCode, - RustDiagnostic, - SuggestionApplicability, -} from '../../../utils/diagnostics/rust'; - -function loadDiagnosticFixture(name: string): RustDiagnostic { - const jsonText = fs - .readFileSync( - // We're actually in our JavaScript output directory, climb out - `${__dirname}/../../../../src/test/fixtures/rust-diagnostics/${name}.json`, - ) - .toString(); - - return JSON.parse(jsonText); -} - -function mapFixtureToVsCode(name: string): MappedRustDiagnostic { - const rd = loadDiagnosticFixture(name); - const mapResult = mapRustDiagnosticToVsCode(rd); - - if (!mapResult) { - return assert.fail('Mapping unexpectedly failed'); - } - return mapResult; -} - -describe('mapRustDiagnosticToVsCode', () => { - it('should map an incompatible type for trait error', () => { - const { diagnostic, suggestedFixes } = mapFixtureToVsCode( - 'error/E0053', - ); - - assert.strictEqual( - diagnostic.severity, - vscode.DiagnosticSeverity.Error, - ); - assert.strictEqual(diagnostic.source, 'rustc'); - assert.strictEqual( - diagnostic.message, - [ - `method \`next\` has an incompatible type for trait`, - `expected type \`fn(&mut ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&ty::Ref>\``, - ` found type \`fn(&ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&'list ty::Ref>\``, - ].join('\n'), - ); - assert.strictEqual(diagnostic.code, 'E0053'); - assert.deepStrictEqual(diagnostic.tags, []); - - // No related information - assert.deepStrictEqual(diagnostic.relatedInformation, []); - - // There are no suggested fixes - assert.strictEqual(suggestedFixes.length, 0); - }); - - it('should map an unused variable warning', () => { - const { diagnostic, suggestedFixes } = mapFixtureToVsCode( - 'warning/unused_variables', - ); - - assert.strictEqual( - diagnostic.severity, - vscode.DiagnosticSeverity.Warning, - ); - assert.strictEqual( - diagnostic.message, - [ - 'unused variable: `foo`', - '#[warn(unused_variables)] on by default', - ].join('\n'), - ); - assert.strictEqual(diagnostic.code, 'unused_variables'); - assert.strictEqual(diagnostic.source, 'rustc'); - assert.deepStrictEqual(diagnostic.tags, [ - vscode.DiagnosticTag.Unnecessary, - ]); - - // No related information - assert.deepStrictEqual(diagnostic.relatedInformation, []); - - // One suggested fix available to prefix the variable - assert.strictEqual(suggestedFixes.length, 1); - const [suggestedFix] = suggestedFixes; - assert.strictEqual( - suggestedFix.title, - 'consider prefixing with an underscore: `_foo`', - ); - assert.strictEqual( - suggestedFix.applicability, - SuggestionApplicability.MachineApplicable, - ); - }); - - it('should map a wrong number of parameters error', () => { - const { diagnostic, suggestedFixes } = mapFixtureToVsCode( - 'error/E0061', - ); - - assert.strictEqual( - diagnostic.severity, - vscode.DiagnosticSeverity.Error, - ); - assert.strictEqual( - diagnostic.message, - [ - 'this function takes 2 parameters but 3 parameters were supplied', - 'expected 2 parameters', - ].join('\n'), - ); - assert.strictEqual(diagnostic.code, 'E0061'); - assert.strictEqual(diagnostic.source, 'rustc'); - assert.deepStrictEqual(diagnostic.tags, []); - - // One related information for the original definition - const relatedInformation = diagnostic.relatedInformation; - if (!relatedInformation) { - assert.fail('Related information unexpectedly undefined'); - return; - } - assert.strictEqual(relatedInformation.length, 1); - const [related] = relatedInformation; - assert.strictEqual(related.message, 'defined here'); - - // There are no suggested fixes - assert.strictEqual(suggestedFixes.length, 0); - }); - - it('should map a Clippy copy pass by ref warning', () => { - const { diagnostic, suggestedFixes } = mapFixtureToVsCode( - 'clippy/trivially_copy_pass_by_ref', - ); - - assert.strictEqual( - diagnostic.severity, - vscode.DiagnosticSeverity.Warning, - ); - assert.strictEqual(diagnostic.source, 'clippy'); - assert.strictEqual( - diagnostic.message, - [ - 'this argument is passed by reference, but would be more efficient if passed by value', - '#[warn(clippy::trivially_copy_pass_by_ref)] implied by #[warn(clippy::all)]', - 'for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref', - ].join('\n'), - ); - assert.strictEqual(diagnostic.code, 'trivially_copy_pass_by_ref'); - assert.deepStrictEqual(diagnostic.tags, []); - - // One related information for the lint definition - const relatedInformation = diagnostic.relatedInformation; - if (!relatedInformation) { - assert.fail('Related information unexpectedly undefined'); - return; - } - assert.strictEqual(relatedInformation.length, 1); - const [related] = relatedInformation; - assert.strictEqual(related.message, 'lint level defined here'); - - // One suggested fix to pass by value - assert.strictEqual(suggestedFixes.length, 1); - const [suggestedFix] = suggestedFixes; - assert.strictEqual( - suggestedFix.title, - 'consider passing by value instead: `self`', - ); - // Clippy does not mark this with any applicability - assert.strictEqual( - suggestedFix.applicability, - SuggestionApplicability.Unspecified, - ); - }); - - it('should map a mismatched type error', () => { - const { diagnostic, suggestedFixes } = mapFixtureToVsCode( - 'error/E0308', - ); - - assert.strictEqual( - diagnostic.severity, - vscode.DiagnosticSeverity.Error, - ); - assert.strictEqual( - diagnostic.message, - ['mismatched types', 'expected usize, found u32'].join('\n'), - ); - assert.strictEqual(diagnostic.code, 'E0308'); - assert.strictEqual(diagnostic.source, 'rustc'); - assert.deepStrictEqual(diagnostic.tags, []); - - // No related information - assert.deepStrictEqual(diagnostic.relatedInformation, []); - - // There are no suggested fixes - assert.strictEqual(suggestedFixes.length, 0); - }); - - it('should map a macro invocation location to normal file path', () => { - const { location, diagnostic, suggestedFixes } = mapFixtureToVsCode( - 'error/E0277', - ); - - assert.strictEqual( - diagnostic.severity, - vscode.DiagnosticSeverity.Error, - ); - assert.strictEqual( - diagnostic.message, - [ - "can't compare `{integer}` with `&str`", - 'the trait `std::cmp::PartialEq<&str>` is not implemented for `{integer}`', - ].join('\n'), - ); - assert.strictEqual(diagnostic.code, 'E0277'); - assert.strictEqual(diagnostic.source, 'rustc'); - assert.deepStrictEqual(diagnostic.tags, []); - - // No related information - assert.deepStrictEqual(diagnostic.relatedInformation, []); - - // There are no suggested fixes - assert.strictEqual(suggestedFixes.length, 0); - - // The file url should be normal file - // Ignore the first part because it depends on vs workspace location - assert.strictEqual( - location.uri.path.substr(-'src/main.rs'.length), - 'src/main.rs', - ); - }); -}); diff --git a/editors/code/src/test/utils/diagnotics/vscode.test.ts b/editors/code/src/test/utils/diagnotics/vscode.test.ts deleted file mode 100644 index 4944dd0328..0000000000 --- a/editors/code/src/test/utils/diagnotics/vscode.test.ts +++ /dev/null @@ -1,98 +0,0 @@ -import * as assert from 'assert'; -import * as vscode from 'vscode'; - -import { areDiagnosticsEqual } from '../../../utils/diagnostics/vscode'; - -const range1 = new vscode.Range( - new vscode.Position(1, 2), - new vscode.Position(3, 4), -); - -const range2 = new vscode.Range( - new vscode.Position(5, 6), - new vscode.Position(7, 8), -); - -describe('areDiagnosticsEqual', () => { - it('should treat identical diagnostics as equal', () => { - const diagnostic1 = new vscode.Diagnostic( - range1, - 'Hello, world!', - vscode.DiagnosticSeverity.Error, - ); - - const diagnostic2 = new vscode.Diagnostic( - range1, - 'Hello, world!', - vscode.DiagnosticSeverity.Error, - ); - - assert(areDiagnosticsEqual(diagnostic1, diagnostic2)); - }); - - it('should treat diagnostics with different sources as inequal', () => { - const diagnostic1 = new vscode.Diagnostic( - range1, - 'Hello, world!', - vscode.DiagnosticSeverity.Error, - ); - diagnostic1.source = 'rustc'; - - const diagnostic2 = new vscode.Diagnostic( - range1, - 'Hello, world!', - vscode.DiagnosticSeverity.Error, - ); - diagnostic2.source = 'clippy'; - - assert(!areDiagnosticsEqual(diagnostic1, diagnostic2)); - }); - - it('should treat diagnostics with different ranges as inequal', () => { - const diagnostic1 = new vscode.Diagnostic( - range1, - 'Hello, world!', - vscode.DiagnosticSeverity.Error, - ); - - const diagnostic2 = new vscode.Diagnostic( - range2, - 'Hello, world!', - vscode.DiagnosticSeverity.Error, - ); - - assert(!areDiagnosticsEqual(diagnostic1, diagnostic2)); - }); - - it('should treat diagnostics with different messages as inequal', () => { - const diagnostic1 = new vscode.Diagnostic( - range1, - 'Hello, world!', - vscode.DiagnosticSeverity.Error, - ); - - const diagnostic2 = new vscode.Diagnostic( - range1, - 'Goodbye!, world!', - vscode.DiagnosticSeverity.Error, - ); - - assert(!areDiagnosticsEqual(diagnostic1, diagnostic2)); - }); - - it('should treat diagnostics with different severities as inequal', () => { - const diagnostic1 = new vscode.Diagnostic( - range1, - 'Hello, world!', - vscode.DiagnosticSeverity.Warning, - ); - - const diagnostic2 = new vscode.Diagnostic( - range1, - 'Hello, world!', - vscode.DiagnosticSeverity.Error, - ); - - assert(!areDiagnosticsEqual(diagnostic1, diagnostic2)); - }); -}); diff --git a/editors/code/src/utils/diagnostics/SuggestedFix.ts b/editors/code/src/utils/diagnostics/SuggestedFix.ts deleted file mode 100644 index 6e660bb61d..0000000000 --- a/editors/code/src/utils/diagnostics/SuggestedFix.ts +++ /dev/null @@ -1,67 +0,0 @@ -import * as vscode from 'vscode'; - -import { SuggestionApplicability } from './rust'; - -/** - * Model object for text replacements suggested by the Rust compiler - * - * This is an intermediate form between the raw `rustc` JSON and a - * `vscode.CodeAction`. It's optimised for the use-cases of - * `SuggestedFixCollection`. - */ -export default class SuggestedFix { - public readonly title: string; - public readonly location: vscode.Location; - public readonly replacement: string; - public readonly applicability: SuggestionApplicability; - - /** - * Diagnostics this suggested fix could resolve - */ - public diagnostics: vscode.Diagnostic[]; - - constructor( - title: string, - location: vscode.Location, - replacement: string, - applicability: SuggestionApplicability = SuggestionApplicability.Unspecified, - ) { - this.title = title; - this.location = location; - this.replacement = replacement; - this.applicability = applicability; - this.diagnostics = []; - } - - /** - * Determines if this suggested fix is equivalent to another instance - */ - public isEqual(other: SuggestedFix): boolean { - return ( - this.title === other.title && - this.location.range.isEqual(other.location.range) && - this.replacement === other.replacement && - this.applicability === other.applicability - ); - } - - /** - * Converts this suggested fix to a VS Code Quick Fix code action - */ - public toCodeAction(): vscode.CodeAction { - const codeAction = new vscode.CodeAction( - this.title, - vscode.CodeActionKind.QuickFix, - ); - - const edit = new vscode.WorkspaceEdit(); - edit.replace(this.location.uri, this.location.range, this.replacement); - codeAction.edit = edit; - - codeAction.isPreferred = - this.applicability === SuggestionApplicability.MachineApplicable; - - codeAction.diagnostics = [...this.diagnostics]; - return codeAction; - } -} diff --git a/editors/code/src/utils/diagnostics/SuggestedFixCollection.ts b/editors/code/src/utils/diagnostics/SuggestedFixCollection.ts deleted file mode 100644 index 57c9856cfa..0000000000 --- a/editors/code/src/utils/diagnostics/SuggestedFixCollection.ts +++ /dev/null @@ -1,77 +0,0 @@ -import * as vscode from 'vscode'; -import SuggestedFix from './SuggestedFix'; - -/** - * Collection of suggested fixes across multiple documents - * - * This stores `SuggestedFix` model objects and returns them via the - * `vscode.CodeActionProvider` interface. - */ -export default class SuggestedFixCollection - implements vscode.CodeActionProvider { - public static PROVIDED_CODE_ACTION_KINDS = [vscode.CodeActionKind.QuickFix]; - - /** - * Map of document URI strings to suggested fixes - */ - private suggestedFixes: Map; - - constructor() { - this.suggestedFixes = new Map(); - } - - /** - * Clears all suggested fixes across all documents - */ - public clear(): void { - this.suggestedFixes = new Map(); - } - - /** - * Adds a suggested fix for the given diagnostic - * - * Some suggested fixes will appear in multiple diagnostics. For example, - * forgetting a `mut` on a variable will suggest changing the delaration on - * every mutable usage site. If the suggested fix has already been added - * this method will instead associate the existing fix with the new - * diagnostic. - */ - public addSuggestedFixForDiagnostic( - suggestedFix: SuggestedFix, - diagnostic: vscode.Diagnostic, - ): void { - const fileUriString = suggestedFix.location.uri.toString(); - const fileSuggestions = this.suggestedFixes.get(fileUriString) || []; - - const existingSuggestion = fileSuggestions.find(s => - s.isEqual(suggestedFix), - ); - - if (existingSuggestion) { - // The existing suggestion also applies to this new diagnostic - existingSuggestion.diagnostics.push(diagnostic); - } else { - // We haven't seen this suggestion before - suggestedFix.diagnostics.push(diagnostic); - fileSuggestions.push(suggestedFix); - } - - this.suggestedFixes.set(fileUriString, fileSuggestions); - } - - /** - * Filters suggested fixes by their document and range and converts them to - * code actions - */ - public provideCodeActions( - document: vscode.TextDocument, - range: vscode.Range, - ): vscode.CodeAction[] { - const documentUriString = document.uri.toString(); - - const suggestedFixes = this.suggestedFixes.get(documentUriString); - return (suggestedFixes || []) - .filter(({ location }) => location.range.intersection(range)) - .map(suggestedEdit => suggestedEdit.toCodeAction()); - } -} diff --git a/editors/code/src/utils/diagnostics/rust.ts b/editors/code/src/utils/diagnostics/rust.ts deleted file mode 100644 index 1f0c0d3e40..0000000000 --- a/editors/code/src/utils/diagnostics/rust.ts +++ /dev/null @@ -1,299 +0,0 @@ -import * as path from 'path'; -import * as vscode from 'vscode'; - -import SuggestedFix from './SuggestedFix'; - -export enum SuggestionApplicability { - MachineApplicable = 'MachineApplicable', - HasPlaceholders = 'HasPlaceholders', - MaybeIncorrect = 'MaybeIncorrect', - Unspecified = 'Unspecified', -} - -export interface RustDiagnosticSpanMacroExpansion { - span: RustDiagnosticSpan; - macro_decl_name: string; - def_site_span?: RustDiagnosticSpan; -} - -// Reference: -// https://github.com/rust-lang/rust/blob/master/src/libsyntax/json.rs -export interface RustDiagnosticSpan { - line_start: number; - line_end: number; - column_start: number; - column_end: number; - is_primary: boolean; - file_name: string; - label?: string; - expansion?: RustDiagnosticSpanMacroExpansion; - suggested_replacement?: string; - suggestion_applicability?: SuggestionApplicability; -} - -export interface RustDiagnostic { - spans: RustDiagnosticSpan[]; - rendered: string; - message: string; - level: string; - code?: { - code: string; - }; - children: RustDiagnostic[]; -} - -export interface MappedRustDiagnostic { - location: vscode.Location; - diagnostic: vscode.Diagnostic; - suggestedFixes: SuggestedFix[]; -} - -interface MappedRustChildDiagnostic { - related?: vscode.DiagnosticRelatedInformation; - suggestedFix?: SuggestedFix; - messageLine?: string; -} - -/** - * Converts a Rust level string to a VsCode severity - */ -function mapLevelToSeverity(s: string): vscode.DiagnosticSeverity { - if (s === 'error') { - return vscode.DiagnosticSeverity.Error; - } - if (s.startsWith('warn')) { - return vscode.DiagnosticSeverity.Warning; - } - return vscode.DiagnosticSeverity.Information; -} - -/** - * Check whether a file name is from macro invocation - */ -function isFromMacro(fileName: string): boolean { - return fileName.startsWith('<') && fileName.endsWith('>'); -} - -/** - * Converts a Rust macro span to a VsCode location recursively - */ -function mapMacroSpanToLocation( - spanMacro: RustDiagnosticSpanMacroExpansion, -): vscode.Location | undefined { - if (!isFromMacro(spanMacro.span.file_name)) { - return mapSpanToLocation(spanMacro.span); - } - - if (spanMacro.span.expansion) { - return mapMacroSpanToLocation(spanMacro.span.expansion); - } - - return; -} - -/** - * Converts a Rust span to a VsCode location - */ -function mapSpanToLocation(span: RustDiagnosticSpan): vscode.Location { - if (isFromMacro(span.file_name) && span.expansion) { - const macroLoc = mapMacroSpanToLocation(span.expansion); - if (macroLoc) { - return macroLoc; - } - } - - const fileName = path.join(vscode.workspace.rootPath || '', span.file_name); - const fileUri = vscode.Uri.file(fileName); - - const range = new vscode.Range( - new vscode.Position(span.line_start - 1, span.column_start - 1), - new vscode.Position(span.line_end - 1, span.column_end - 1), - ); - - return new vscode.Location(fileUri, range); -} - -/** - * Converts a secondary Rust span to a VsCode related information - * - * If the span is unlabelled this will return `undefined`. - */ -function mapSecondarySpanToRelated( - span: RustDiagnosticSpan, -): vscode.DiagnosticRelatedInformation | undefined { - if (!span.label) { - // Nothing to label this with - return; - } - - const location = mapSpanToLocation(span); - return new vscode.DiagnosticRelatedInformation(location, span.label); -} - -/** - * Determines if diagnostic is related to unused code - */ -function isUnusedOrUnnecessary(rd: RustDiagnostic): boolean { - if (!rd.code) { - return false; - } - - return [ - 'dead_code', - 'unknown_lints', - 'unreachable_code', - 'unused_attributes', - 'unused_imports', - 'unused_macros', - 'unused_variables', - ].includes(rd.code.code); -} - -/** - * Determines if diagnostic is related to deprecated code - */ -function isDeprecated(rd: RustDiagnostic): boolean { - if (!rd.code) { - return false; - } - - return ['deprecated'].includes(rd.code.code); -} - -/** - * Converts a Rust child diagnostic to a VsCode related information - * - * This can have three outcomes: - * - * 1. If this is no primary span this will return a `noteLine` - * 2. If there is a primary span with a suggested replacement it will return a - * `codeAction`. - * 3. If there is a primary span without a suggested replacement it will return - * a `related`. - */ -function mapRustChildDiagnostic(rd: RustDiagnostic): MappedRustChildDiagnostic { - const span = rd.spans.find(s => s.is_primary); - - if (!span) { - // `rustc` uses these spanless children as a way to print multi-line - // messages - return { messageLine: rd.message }; - } - - // If we have a primary span use its location, otherwise use the parent - const location = mapSpanToLocation(span); - - // We need to distinguish `null` from an empty string - if (span && typeof span.suggested_replacement === 'string') { - // Include our replacement in the title unless it's empty - const title = span.suggested_replacement - ? `${rd.message}: \`${span.suggested_replacement}\`` - : rd.message; - - return { - suggestedFix: new SuggestedFix( - title, - location, - span.suggested_replacement, - span.suggestion_applicability, - ), - }; - } else { - const related = new vscode.DiagnosticRelatedInformation( - location, - rd.message, - ); - - return { related }; - } -} - -/** - * Converts a Rust root diagnostic to VsCode form - * - * This flattens the Rust diagnostic by: - * - * 1. Creating a `vscode.Diagnostic` with the root message and primary span. - * 2. Adding any labelled secondary spans to `relatedInformation` - * 3. Categorising child diagnostics as either `SuggestedFix`es, - * `relatedInformation` or additional message lines. - * - * If the diagnostic has no primary span this will return `undefined` - */ -export function mapRustDiagnosticToVsCode( - rd: RustDiagnostic, -): MappedRustDiagnostic | undefined { - const primarySpan = rd.spans.find(s => s.is_primary); - if (!primarySpan) { - return; - } - - const location = mapSpanToLocation(primarySpan); - const secondarySpans = rd.spans.filter(s => !s.is_primary); - - const severity = mapLevelToSeverity(rd.level); - let primarySpanLabel = primarySpan.label; - - const vd = new vscode.Diagnostic(location.range, rd.message, severity); - - let source = 'rustc'; - let code = rd.code && rd.code.code; - if (code) { - // See if this is an RFC #2103 scoped lint (e.g. from Clippy) - const scopedCode = code.split('::'); - if (scopedCode.length === 2) { - [source, code] = scopedCode; - } - } - - vd.source = source; - vd.code = code; - vd.relatedInformation = []; - vd.tags = []; - - for (const secondarySpan of secondarySpans) { - const related = mapSecondarySpanToRelated(secondarySpan); - if (related) { - vd.relatedInformation.push(related); - } - } - - const suggestedFixes = []; - for (const child of rd.children) { - const { related, suggestedFix, messageLine } = mapRustChildDiagnostic( - child, - ); - - if (related) { - vd.relatedInformation.push(related); - } - if (suggestedFix) { - suggestedFixes.push(suggestedFix); - } - if (messageLine) { - vd.message += `\n${messageLine}`; - - // These secondary messages usually duplicate the content of the - // primary span label. - primarySpanLabel = undefined; - } - } - - if (primarySpanLabel) { - vd.message += `\n${primarySpanLabel}`; - } - - if (isUnusedOrUnnecessary(rd)) { - vd.tags.push(vscode.DiagnosticTag.Unnecessary); - } - - if (isDeprecated(rd)) { - vd.tags.push(vscode.DiagnosticTag.Deprecated); - } - - return { - location, - diagnostic: vd, - suggestedFixes, - }; -} diff --git a/editors/code/src/utils/diagnostics/vscode.ts b/editors/code/src/utils/diagnostics/vscode.ts deleted file mode 100644 index f4a5450e2b..0000000000 --- a/editors/code/src/utils/diagnostics/vscode.ts +++ /dev/null @@ -1,14 +0,0 @@ -import * as vscode from 'vscode'; - -/** Compares two `vscode.Diagnostic`s for equality */ -export function areDiagnosticsEqual( - left: vscode.Diagnostic, - right: vscode.Diagnostic, -): boolean { - return ( - left.source === right.source && - left.severity === right.severity && - left.range.isEqual(right.range) && - left.message === right.message - ); -} From 178c23f50549298aad0dc0f098f8ed510a57f9d6 Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Wed, 25 Dec 2019 19:08:44 +0100 Subject: [PATCH 07/20] Re-implement status display using LSP 3.15 progress event --- crates/ra_lsp_server/src/cargo_check.rs | 53 +++++++++++++++++++---- crates/ra_lsp_server/src/main_loop.rs | 8 ++++ editors/code/src/commands/watch_status.ts | 43 ++++++++++++++++++ editors/code/src/extension.ts | 8 ++++ 4 files changed, 103 insertions(+), 9 deletions(-) diff --git a/crates/ra_lsp_server/src/cargo_check.rs b/crates/ra_lsp_server/src/cargo_check.rs index 5a6a209eba..dd8c5d407a 100644 --- a/crates/ra_lsp_server/src/cargo_check.rs +++ b/crates/ra_lsp_server/src/cargo_check.rs @@ -9,7 +9,8 @@ use cargo_metadata::{ use crossbeam_channel::{select, unbounded, Receiver, RecvError, Sender, TryRecvError}; use lsp_types::{ Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity, DiagnosticTag, Location, - NumberOrString, Position, Range, Url, + NumberOrString, Position, Range, Url, WorkDoneProgress, WorkDoneProgressBegin, + WorkDoneProgressEnd, WorkDoneProgressReport, }; use parking_lot::RwLock; use std::{ @@ -132,6 +133,7 @@ impl CheckWatcherSharedState { #[derive(Debug)] pub enum CheckTask { Update(Url), + Status(WorkDoneProgress), } pub enum CheckCommand { @@ -204,13 +206,38 @@ impl CheckWatcherState { } } - fn handle_message(&mut self, msg: cargo_metadata::Message, task_send: &Sender) { + fn handle_message(&mut self, msg: CheckEvent, task_send: &Sender) { match msg { - Message::CompilerArtifact(_msg) => { - // TODO: Status display + CheckEvent::Begin => { + task_send + .send(CheckTask::Status(WorkDoneProgress::Begin(WorkDoneProgressBegin { + title: "Running 'cargo check'".to_string(), + cancellable: Some(false), + message: None, + percentage: None, + }))) + .unwrap(); } - Message::CompilerMessage(msg) => { + CheckEvent::End => { + task_send + .send(CheckTask::Status(WorkDoneProgress::End(WorkDoneProgressEnd { + message: None, + }))) + .unwrap(); + } + + CheckEvent::Msg(Message::CompilerArtifact(msg)) => { + task_send + .send(CheckTask::Status(WorkDoneProgress::Report(WorkDoneProgressReport { + cancellable: Some(false), + message: Some(msg.target.name), + percentage: None, + }))) + .unwrap(); + } + + CheckEvent::Msg(Message::CompilerMessage(msg)) => { let map_result = match map_rust_diagnostic_to_lsp(&msg.message, &self.workspace_root) { Some(map_result) => map_result, @@ -232,8 +259,8 @@ impl CheckWatcherState { task_send.send(CheckTask::Update(location.uri)).unwrap(); } - Message::BuildScriptExecuted(_msg) => {} - Message::Unknown => {} + CheckEvent::Msg(Message::BuildScriptExecuted(_msg)) => {} + CheckEvent::Msg(Message::Unknown) => {} } } } @@ -244,10 +271,16 @@ impl CheckWatcherState { /// have to wrap sub-processes output handling in a thread and pass messages /// back over a channel. struct WatchThread { - message_recv: Receiver, + message_recv: Receiver, cancel_send: Sender<()>, } +enum CheckEvent { + Begin, + Msg(cargo_metadata::Message), + End, +} + impl WatchThread { fn new( check_command: Option<&String>, @@ -273,6 +306,7 @@ impl WatchThread { .spawn() .expect("couldn't launch cargo"); + message_send.send(CheckEvent::Begin).unwrap(); for message in cargo_metadata::parse_messages(command.stdout.take().unwrap()) { match cancel_recv.try_recv() { Ok(()) | Err(TryRecvError::Disconnected) => { @@ -281,8 +315,9 @@ impl WatchThread { Err(TryRecvError::Empty) => (), } - message_send.send(message.unwrap()).unwrap(); + message_send.send(CheckEvent::Msg(message.unwrap())).unwrap(); } + message_send.send(CheckEvent::End).unwrap(); }); WatchThread { message_recv, cancel_send } } diff --git a/crates/ra_lsp_server/src/main_loop.rs b/crates/ra_lsp_server/src/main_loop.rs index 1f6175699a..045e4660dd 100644 --- a/crates/ra_lsp_server/src/main_loop.rs +++ b/crates/ra_lsp_server/src/main_loop.rs @@ -338,6 +338,14 @@ fn loop_turn( task_sender.send(Task::Notify(not)).unwrap(); } } + CheckTask::Status(progress) => { + let params = req::ProgressParams { + token: req::ProgressToken::String("rustAnalyzer/cargoWatcher".to_string()), + value: req::ProgressParamsValue::WorkDone(progress), + }; + let not = notification_new::(params); + task_sender.send(Task::Notify(not)).unwrap(); + } }, Event::Msg(msg) => match msg { Message::Request(req) => on_request( diff --git a/editors/code/src/commands/watch_status.ts b/editors/code/src/commands/watch_status.ts index 8d64394c7b..2404c3f164 100644 --- a/editors/code/src/commands/watch_status.ts +++ b/editors/code/src/commands/watch_status.ts @@ -57,7 +57,50 @@ export class StatusDisplay implements vscode.Disposable { this.statusBarItem.dispose(); } + public handleProgressNotification(params: ProgressParams) { + const { token, value } = params; + if (token !== "rustAnalyzer/cargoWatcher") { + return; + } + + console.log("Got progress notification", token, value) + switch (value.kind) { + case "begin": + this.show(); + break; + + case "report": + if (value.message) { + this.packageName = value.message; + } + break; + + case "end": + this.hide(); + break; + } + } + private frame() { return spinnerFrames[(this.i = ++this.i % spinnerFrames.length)]; } } + +// FIXME: Replace this once vscode-languageclient is updated to LSP 3.15 +interface ProgressParams { + token: string + value: WorkDoneProgress +} + +enum WorkDoneProgressKind { + Begin = "begin", + Report = "report", + End = "end" +} + +interface WorkDoneProgress { + kind: WorkDoneProgressKind, + message?: string + cancelable?: boolean + percentage?: string +} \ No newline at end of file diff --git a/editors/code/src/extension.ts b/editors/code/src/extension.ts index 72a4d4bf25..1507cb26ea 100644 --- a/editors/code/src/extension.ts +++ b/editors/code/src/extension.ts @@ -8,6 +8,7 @@ import { SyntaxTreeContentProvider } from './commands/syntaxTree'; import * as events from './events'; import * as notifications from './notifications'; import { Server } from './server'; +import { StatusDisplay } from './commands/watch_status'; export async function activate(context: vscode.ExtensionContext) { function disposeOnDeactivation(disposable: vscode.Disposable) { @@ -83,6 +84,9 @@ export async function activate(context: vscode.ExtensionContext) { overrideCommand('type', commands.onEnter.handle); } + const watchStatus = new StatusDisplay(Server.config.cargoCheckOptions.command || 'check'); + disposeOnDeactivation(watchStatus); + // Notifications are events triggered by the language server const allNotifications: Iterable<[ string, @@ -92,6 +96,10 @@ export async function activate(context: vscode.ExtensionContext) { 'rust-analyzer/publishDecorations', notifications.publishDecorations.handle, ], + [ + '$/progress', + (params) => watchStatus.handleProgressNotification(params), + ] ]; const syntaxTreeContentProvider = new SyntaxTreeContentProvider(); const expandMacroContentProvider = new ExpandMacroContentProvider(); From b9c10ed97f22d665b75895a387c633dfa412ed2b Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Wed, 25 Dec 2019 19:10:30 +0100 Subject: [PATCH 08/20] Re-format VSCode extension changes --- editors/code/src/commands/watch_status.ts | 29 +++++++++++------------ editors/code/src/extension.ts | 10 ++++---- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/editors/code/src/commands/watch_status.ts b/editors/code/src/commands/watch_status.ts index 2404c3f164..10787b5101 100644 --- a/editors/code/src/commands/watch_status.ts +++ b/editors/code/src/commands/watch_status.ts @@ -59,23 +59,22 @@ export class StatusDisplay implements vscode.Disposable { public handleProgressNotification(params: ProgressParams) { const { token, value } = params; - if (token !== "rustAnalyzer/cargoWatcher") { + if (token !== 'rustAnalyzer/cargoWatcher') { return; } - console.log("Got progress notification", token, value) switch (value.kind) { - case "begin": + case 'begin': this.show(); break; - case "report": + case 'report': if (value.message) { this.packageName = value.message; } break; - case "end": + case 'end': this.hide(); break; } @@ -88,19 +87,19 @@ export class StatusDisplay implements vscode.Disposable { // FIXME: Replace this once vscode-languageclient is updated to LSP 3.15 interface ProgressParams { - token: string - value: WorkDoneProgress + token: string; + value: WorkDoneProgress; } enum WorkDoneProgressKind { - Begin = "begin", - Report = "report", - End = "end" + Begin = 'begin', + Report = 'report', + End = 'end', } interface WorkDoneProgress { - kind: WorkDoneProgressKind, - message?: string - cancelable?: boolean - percentage?: string -} \ No newline at end of file + kind: WorkDoneProgressKind; + message?: string; + cancelable?: boolean; + percentage?: string; +} diff --git a/editors/code/src/extension.ts b/editors/code/src/extension.ts index 1507cb26ea..36163b6bb6 100644 --- a/editors/code/src/extension.ts +++ b/editors/code/src/extension.ts @@ -5,10 +5,10 @@ import * as commands from './commands'; import { ExpandMacroContentProvider } from './commands/expand_macro'; import { HintsUpdater } from './commands/inlay_hints'; import { SyntaxTreeContentProvider } from './commands/syntaxTree'; +import { StatusDisplay } from './commands/watch_status'; import * as events from './events'; import * as notifications from './notifications'; import { Server } from './server'; -import { StatusDisplay } from './commands/watch_status'; export async function activate(context: vscode.ExtensionContext) { function disposeOnDeactivation(disposable: vscode.Disposable) { @@ -84,7 +84,9 @@ export async function activate(context: vscode.ExtensionContext) { overrideCommand('type', commands.onEnter.handle); } - const watchStatus = new StatusDisplay(Server.config.cargoCheckOptions.command || 'check'); + const watchStatus = new StatusDisplay( + Server.config.cargoCheckOptions.command || 'check', + ); disposeOnDeactivation(watchStatus); // Notifications are events triggered by the language server @@ -98,8 +100,8 @@ export async function activate(context: vscode.ExtensionContext) { ], [ '$/progress', - (params) => watchStatus.handleProgressNotification(params), - ] + params => watchStatus.handleProgressNotification(params), + ], ]; const syntaxTreeContentProvider = new SyntaxTreeContentProvider(); const expandMacroContentProvider = new ExpandMacroContentProvider(); From f7d04d05756e7074de2b73b35718b5b4ae670d1b Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Wed, 25 Dec 2019 19:10:45 +0100 Subject: [PATCH 09/20] Re-format ra_lsp_server changes --- crates/ra_lsp_server/src/caps.rs | 4 ++-- crates/ra_lsp_server/src/req.rs | 8 ++++---- crates/ra_lsp_server/src/world.rs | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/ra_lsp_server/src/caps.rs b/crates/ra_lsp_server/src/caps.rs index 0f84e7a34e..0dee1f6fea 100644 --- a/crates/ra_lsp_server/src/caps.rs +++ b/crates/ra_lsp_server/src/caps.rs @@ -3,10 +3,10 @@ use lsp_types::{ CodeActionProviderCapability, CodeLensOptions, CompletionOptions, DocumentOnTypeFormattingOptions, FoldingRangeProviderCapability, - ImplementationProviderCapability, RenameOptions, RenameProviderCapability, + ImplementationProviderCapability, RenameOptions, RenameProviderCapability, SaveOptions, SelectionRangeProviderCapability, ServerCapabilities, SignatureHelpOptions, TextDocumentSyncCapability, TextDocumentSyncKind, TextDocumentSyncOptions, - TypeDefinitionProviderCapability, WorkDoneProgressOptions, SaveOptions + TypeDefinitionProviderCapability, WorkDoneProgressOptions, }; pub fn server_capabilities() -> ServerCapabilities { diff --git a/crates/ra_lsp_server/src/req.rs b/crates/ra_lsp_server/src/req.rs index b34e6f9b89..40edaf6770 100644 --- a/crates/ra_lsp_server/src/req.rs +++ b/crates/ra_lsp_server/src/req.rs @@ -9,10 +9,10 @@ pub use lsp_types::{ CodeLensParams, CompletionParams, CompletionResponse, DidChangeConfigurationParams, DidChangeWatchedFilesParams, DidChangeWatchedFilesRegistrationOptions, DocumentOnTypeFormattingParams, DocumentSymbolParams, DocumentSymbolResponse, - FileSystemWatcher, Hover, InitializeResult, MessageType, PublishDiagnosticsParams, - ReferenceParams, Registration, RegistrationParams, SelectionRange, SelectionRangeParams, - ShowMessageParams, SignatureHelp, TextDocumentEdit, TextDocumentPositionParams, TextEdit, - WorkspaceEdit, WorkspaceSymbolParams, + FileSystemWatcher, Hover, InitializeResult, MessageType, ProgressParams, ProgressParamsValue, + ProgressToken, PublishDiagnosticsParams, ReferenceParams, Registration, RegistrationParams, + SelectionRange, SelectionRangeParams, ShowMessageParams, SignatureHelp, TextDocumentEdit, + TextDocumentPositionParams, TextEdit, WorkspaceEdit, WorkspaceSymbolParams, }; pub enum AnalyzerStatus {} diff --git a/crates/ra_lsp_server/src/world.rs b/crates/ra_lsp_server/src/world.rs index 235eb199d0..47c3823fb1 100644 --- a/crates/ra_lsp_server/src/world.rs +++ b/crates/ra_lsp_server/src/world.rs @@ -23,8 +23,8 @@ use relative_path::RelativePathBuf; use std::path::{Component, Prefix}; use crate::{ - main_loop::pending_requests::{CompletedRequest, LatestRequests}, cargo_check::{CheckWatcher, CheckWatcherSharedState}, + main_loop::pending_requests::{CompletedRequest, LatestRequests}, LspError, Result, }; use std::str::FromStr; From 069c1655369aa33223788b4f1b8407b6d6b63193 Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Wed, 25 Dec 2019 19:17:54 +0100 Subject: [PATCH 10/20] Actually respect disabling cargo check functionality --- crates/ra_lsp_server/src/cargo_check.rs | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/crates/ra_lsp_server/src/cargo_check.rs b/crates/ra_lsp_server/src/cargo_check.rs index dd8c5d407a..42e55565f2 100644 --- a/crates/ra_lsp_server/src/cargo_check.rs +++ b/crates/ra_lsp_server/src/cargo_check.rs @@ -33,6 +33,7 @@ pub struct CheckWatcher { impl CheckWatcher { pub fn new(options: &Options, workspace_root: PathBuf) -> CheckWatcher { + let check_enabled = options.cargo_check_enable; let check_command = options.cargo_check_command.clone(); let check_args = options.cargo_check_args.clone(); let shared = Arc::new(RwLock::new(CheckWatcherSharedState::new())); @@ -41,8 +42,13 @@ impl CheckWatcher { let (cmd_send, cmd_recv) = unbounded::(); let shared_ = shared.clone(); let handle = std::thread::spawn(move || { - let mut check = - CheckWatcherState::new(check_command, check_args, workspace_root, shared_); + let mut check = CheckWatcherState::new( + check_enabled, + check_command, + check_args, + workspace_root, + shared_, + ); check.run(&task_send, &cmd_recv); }); @@ -55,6 +61,7 @@ impl CheckWatcher { } pub struct CheckWatcherState { + check_enabled: bool, check_command: Option, check_args: Vec, workspace_root: PathBuf, @@ -142,13 +149,16 @@ pub enum CheckCommand { impl CheckWatcherState { pub fn new( + check_enabled: bool, check_command: Option, check_args: Vec, workspace_root: PathBuf, shared: Arc>, ) -> CheckWatcherState { - let watcher = WatchThread::new(check_command.as_ref(), &check_args, &workspace_root); + let watcher = + WatchThread::new(check_enabled, check_command.as_ref(), &check_args, &workspace_root); CheckWatcherState { + check_enabled, check_command, check_args, workspace_root, @@ -182,6 +192,7 @@ impl CheckWatcherState { self.watcher.cancel(); self.watcher = WatchThread::new( + self.check_enabled, self.check_command.as_ref(), &self.check_args, &self.workspace_root, @@ -283,6 +294,7 @@ enum CheckEvent { impl WatchThread { fn new( + check_enabled: bool, check_command: Option<&String>, check_args: &[String], workspace_root: &PathBuf, @@ -299,6 +311,10 @@ impl WatchThread { let (message_send, message_recv) = unbounded(); let (cancel_send, cancel_recv) = unbounded(); std::thread::spawn(move || { + if !check_enabled { + return; + } + let mut command = Command::new("cargo") .args(&args) .stdout(Stdio::piped()) From 71d2d81dcc879bbb7898df11ac00578e93b27ab5 Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Wed, 25 Dec 2019 19:56:07 +0100 Subject: [PATCH 11/20] Some documentatioN --- crates/ra_lsp_server/src/cargo_check.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/crates/ra_lsp_server/src/cargo_check.rs b/crates/ra_lsp_server/src/cargo_check.rs index 42e55565f2..fa0409ee04 100644 --- a/crates/ra_lsp_server/src/cargo_check.rs +++ b/crates/ra_lsp_server/src/cargo_check.rs @@ -1,3 +1,6 @@ +//! cargo_check provides the functionality needed to run `cargo check` or +//! another compatible command (f.x. clippy) in a background thread and provide +//! LSP diagnostics based on the output of the command. use crate::world::Options; use cargo_metadata::{ diagnostic::{ @@ -23,6 +26,9 @@ use std::{ time::Instant, }; +/// CheckWatcher wraps the shared state and communication machinery used for +/// running `cargo check` (or other compatible command) and providing +/// diagnostics based on the output. #[derive(Debug)] pub struct CheckWatcher { pub task_recv: Receiver, @@ -55,6 +61,7 @@ impl CheckWatcher { CheckWatcher { task_recv, cmd_send, handle, shared } } + /// Schedule a re-start of the cargo check worker. pub fn update(&self) { self.cmd_send.send(CheckCommand::Update).unwrap(); } @@ -85,6 +92,8 @@ impl CheckWatcherSharedState { } } + /// Clear the cached diagnostics, and schedule updating diagnostics by the + /// server, to clear stale results. pub fn clear(&mut self, task_send: &Sender) { let cleared_files: Vec = self.diagnostic_collection.keys().cloned().collect(); @@ -139,11 +148,15 @@ impl CheckWatcherSharedState { #[derive(Debug)] pub enum CheckTask { + /// Request a update of the given files diagnostics Update(Url), + + /// Request check progress notification to client Status(WorkDoneProgress), } pub enum CheckCommand { + /// Request re-start of check thread Update, } From 0cdbd0814958e174c5481d6bf16bd2a7e53ec981 Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Wed, 25 Dec 2019 20:23:44 +0100 Subject: [PATCH 12/20] Keep VSCode config mostly backwards compatible --- crates/ra_lsp_server/src/cargo_check.rs | 52 +++++++------------------ crates/ra_lsp_server/src/config.rs | 14 ++++--- crates/ra_lsp_server/src/main_loop.rs | 7 ++-- crates/ra_lsp_server/src/world.rs | 7 ++-- editors/code/package.json | 34 +++++++--------- editors/code/src/config.ts | 31 +++++++++------ editors/code/src/extension.ts | 2 +- editors/code/src/server.ts | 8 ++-- 8 files changed, 71 insertions(+), 84 deletions(-) diff --git a/crates/ra_lsp_server/src/cargo_check.rs b/crates/ra_lsp_server/src/cargo_check.rs index fa0409ee04..70c723b199 100644 --- a/crates/ra_lsp_server/src/cargo_check.rs +++ b/crates/ra_lsp_server/src/cargo_check.rs @@ -39,22 +39,14 @@ pub struct CheckWatcher { impl CheckWatcher { pub fn new(options: &Options, workspace_root: PathBuf) -> CheckWatcher { - let check_enabled = options.cargo_check_enable; - let check_command = options.cargo_check_command.clone(); - let check_args = options.cargo_check_args.clone(); + let options = options.clone(); let shared = Arc::new(RwLock::new(CheckWatcherSharedState::new())); let (task_send, task_recv) = unbounded::(); let (cmd_send, cmd_recv) = unbounded::(); let shared_ = shared.clone(); let handle = std::thread::spawn(move || { - let mut check = CheckWatcherState::new( - check_enabled, - check_command, - check_args, - workspace_root, - shared_, - ); + let mut check = CheckWatcherState::new(options, workspace_root, shared_); check.run(&task_send, &cmd_recv); }); @@ -68,9 +60,7 @@ impl CheckWatcher { } pub struct CheckWatcherState { - check_enabled: bool, - check_command: Option, - check_args: Vec, + options: Options, workspace_root: PathBuf, running: bool, watcher: WatchThread, @@ -162,18 +152,13 @@ pub enum CheckCommand { impl CheckWatcherState { pub fn new( - check_enabled: bool, - check_command: Option, - check_args: Vec, + options: Options, workspace_root: PathBuf, shared: Arc>, ) -> CheckWatcherState { - let watcher = - WatchThread::new(check_enabled, check_command.as_ref(), &check_args, &workspace_root); + let watcher = WatchThread::new(&options, &workspace_root); CheckWatcherState { - check_enabled, - check_command, - check_args, + options, workspace_root, running: false, watcher, @@ -204,12 +189,7 @@ impl CheckWatcherState { self.shared.write().clear(task_send); self.watcher.cancel(); - self.watcher = WatchThread::new( - self.check_enabled, - self.check_command.as_ref(), - &self.check_args, - &self.workspace_root, - ); + self.watcher = WatchThread::new(&self.options, &self.workspace_root); } } } @@ -306,25 +286,23 @@ enum CheckEvent { } impl WatchThread { - fn new( - check_enabled: bool, - check_command: Option<&String>, - check_args: &[String], - workspace_root: &PathBuf, - ) -> WatchThread { - let check_command = check_command.cloned().unwrap_or("check".to_string()); + fn new(options: &Options, workspace_root: &PathBuf) -> WatchThread { let mut args: Vec = vec![ - check_command, + options.cargo_watch_command.clone(), "--message-format=json".to_string(), "--manifest-path".to_string(), format!("{}/Cargo.toml", workspace_root.to_string_lossy()), ]; - args.extend(check_args.iter().cloned()); + if options.cargo_watch_all_targets { + args.push("--all-targets".to_string()); + } + args.extend(options.cargo_watch_args.iter().cloned()); let (message_send, message_recv) = unbounded(); let (cancel_send, cancel_recv) = unbounded(); + let enabled = options.cargo_watch_enable; std::thread::spawn(move || { - if !check_enabled { + if !enabled { return; } diff --git a/crates/ra_lsp_server/src/config.rs b/crates/ra_lsp_server/src/config.rs index 621f2238c5..2d7948d74a 100644 --- a/crates/ra_lsp_server/src/config.rs +++ b/crates/ra_lsp_server/src/config.rs @@ -32,9 +32,10 @@ pub struct ServerConfig { pub max_inlay_hint_length: Option, - pub cargo_check_enable: bool, - pub cargo_check_command: Option, - pub cargo_check_args: Vec, + pub cargo_watch_enable: bool, + pub cargo_watch_args: Vec, + pub cargo_watch_command: String, + pub cargo_watch_all_targets: bool, /// For internal usage to make integrated tests faster. #[serde(deserialize_with = "nullable_bool_true")] @@ -55,9 +56,10 @@ impl Default for ServerConfig { use_client_watching: false, lru_capacity: None, max_inlay_hint_length: None, - cargo_check_enable: true, - cargo_check_command: None, - cargo_check_args: vec![], + cargo_watch_enable: true, + cargo_watch_args: Vec::new(), + cargo_watch_command: "check".to_string(), + cargo_watch_all_targets: true, with_sysroot: true, feature_flags: FxHashMap::default(), cargo_features: Default::default(), diff --git a/crates/ra_lsp_server/src/main_loop.rs b/crates/ra_lsp_server/src/main_loop.rs index 045e4660dd..c58af7e473 100644 --- a/crates/ra_lsp_server/src/main_loop.rs +++ b/crates/ra_lsp_server/src/main_loop.rs @@ -127,9 +127,10 @@ pub fn main_loop( .and_then(|it| it.line_folding_only) .unwrap_or(false), max_inlay_hint_length: config.max_inlay_hint_length, - cargo_check_enable: config.cargo_check_enable, - cargo_check_command: config.cargo_check_command, - cargo_check_args: config.cargo_check_args, + cargo_watch_enable: config.cargo_watch_enable, + cargo_watch_args: config.cargo_watch_args, + cargo_watch_command: config.cargo_watch_command, + cargo_watch_all_targets: config.cargo_watch_all_targets, } }; diff --git a/crates/ra_lsp_server/src/world.rs b/crates/ra_lsp_server/src/world.rs index 47c3823fb1..39a07c01ac 100644 --- a/crates/ra_lsp_server/src/world.rs +++ b/crates/ra_lsp_server/src/world.rs @@ -35,9 +35,10 @@ pub struct Options { pub supports_location_link: bool, pub line_folding_only: bool, pub max_inlay_hint_length: Option, - pub cargo_check_enable: bool, - pub cargo_check_command: Option, - pub cargo_check_args: Vec, + pub cargo_watch_enable: bool, + pub cargo_watch_args: Vec, + pub cargo_watch_command: String, + pub cargo_watch_all_targets: bool, } /// `WorldState` is the primary mutable state of the language server diff --git a/editors/code/package.json b/editors/code/package.json index 5f4123397b..69298e9178 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -188,11 +188,6 @@ "default": "ra_lsp_server", "description": "Path to ra_lsp_server executable" }, - "rust-analyzer.enableCargoCheck": { - "type": "boolean", - "default": true, - "description": "Run `cargo check` for diagnostics on save" - }, "rust-analyzer.excludeGlobs": { "type": "array", "default": [], @@ -203,16 +198,26 @@ "default": true, "description": "client provided file watching instead of notify watching." }, - "rust-analyzer.cargo-check.arguments": { + "rust-analyzer.cargo-watch.enable": { + "type": "boolean", + "default": true, + "description": "Run `cargo check` for diagnostics on save" + }, + "rust-analyzer.cargo-watch.arguments": { "type": "array", - "description": "`cargo-check` arguments. (e.g: `--features=\"shumway,pdf\"` will run as `cargo check --features=\"shumway,pdf\"` )", + "description": "`cargo-watch` arguments. (e.g: `--features=\"shumway,pdf\"` will run as `cargo watch -x \"check --features=\"shumway,pdf\"\"` )", "default": [] }, - "rust-analyzer.cargo-check.command": { + "rust-analyzer.cargo-watch.command": { "type": "string", - "description": "`cargo-check` command. (e.g: `clippy` will run as `cargo clippy` )", + "description": "`cargo-watch` command. (e.g: `clippy` will run as `cargo watch -x clippy` )", "default": "check" }, + "rust-analyzer.cargo-watch.allTargets": { + "type": "boolean", + "description": "Check all targets and tests (will be passed as `--all-targets`)", + "default": true + }, "rust-analyzer.trace.server": { "type": "string", "scope": "window", @@ -229,17 +234,6 @@ "default": "off", "description": "Trace requests to the ra_lsp_server" }, - "rust-analyzer.trace.cargo-watch": { - "type": "string", - "scope": "window", - "enum": [ - "off", - "error", - "verbose" - ], - "default": "off", - "description": "Trace output of cargo-watch" - }, "rust-analyzer.lruCapacity": { "type": "number", "default": null, diff --git a/editors/code/src/config.ts b/editors/code/src/config.ts index 96532e2c9d..4b388b80c5 100644 --- a/editors/code/src/config.ts +++ b/editors/code/src/config.ts @@ -4,10 +4,11 @@ import { Server } from './server'; const RA_LSP_DEBUG = process.env.__RA_LSP_SERVER_DEBUG; -export interface CargoCheckOptions { - enabled: boolean; +export interface CargoWatchOptions { + enable: boolean; arguments: string[]; - command: null | string; + command: string; + allTargets: boolean; } export interface CargoFeatures { @@ -29,10 +30,11 @@ export class Config { public featureFlags = {}; // for internal use public withSysroot: null | boolean = null; - public cargoCheckOptions: CargoCheckOptions = { - enabled: true, + public cargoWatchOptions: CargoWatchOptions = { + enable: true, arguments: [], - command: null, + command: '', + allTargets: true, }; public cargoFeatures: CargoFeatures = { noDefaultFeatures: false, @@ -91,27 +93,34 @@ export class Config { RA_LSP_DEBUG || (config.get('raLspServerPath') as string); } - if (config.has('enableCargoCheck')) { - this.cargoCheckOptions.enabled = config.get( - 'enableCargoCheck', + if (config.has('cargo-watch.enable')) { + this.cargoWatchOptions.enable = config.get( + 'cargo-watch.enable', true, ); } if (config.has('cargo-watch.arguments')) { - this.cargoCheckOptions.arguments = config.get( + this.cargoWatchOptions.arguments = config.get( 'cargo-watch.arguments', [], ); } if (config.has('cargo-watch.command')) { - this.cargoCheckOptions.command = config.get( + this.cargoWatchOptions.command = config.get( 'cargo-watch.command', '', ); } + if (config.has('cargo-watch.allTargets')) { + this.cargoWatchOptions.allTargets = config.get( + 'cargo-watch.allTargets', + true, + ); + } + if (config.has('lruCapacity')) { this.lruCapacity = config.get('lruCapacity') as number; } diff --git a/editors/code/src/extension.ts b/editors/code/src/extension.ts index 36163b6bb6..1da10ebd06 100644 --- a/editors/code/src/extension.ts +++ b/editors/code/src/extension.ts @@ -85,7 +85,7 @@ export async function activate(context: vscode.ExtensionContext) { } const watchStatus = new StatusDisplay( - Server.config.cargoCheckOptions.command || 'check', + Server.config.cargoWatchOptions.command, ); disposeOnDeactivation(watchStatus); diff --git a/editors/code/src/server.ts b/editors/code/src/server.ts index 409d3b4b73..ae81af8483 100644 --- a/editors/code/src/server.ts +++ b/editors/code/src/server.ts @@ -55,9 +55,11 @@ export class Server { publishDecorations: true, lruCapacity: Server.config.lruCapacity, maxInlayHintLength: Server.config.maxInlayHintLength, - cargoCheckEnable: Server.config.cargoCheckOptions.enabled, - cargoCheckCommand: Server.config.cargoCheckOptions.command, - cargoCheckArgs: Server.config.cargoCheckOptions.arguments, + cargoWatchEnable: Server.config.cargoWatchOptions.enable, + cargoWatchArgumets: Server.config.cargoWatchOptions.arguments, + cargoWatchCommand: Server.config.cargoWatchOptions.command, + cargoWatchAllTargets: + Server.config.cargoWatchOptions.allTargets, excludeGlobs: Server.config.excludeGlobs, useClientWatching: Server.config.useClientWatching, featureFlags: Server.config.featureFlags, From 428a6ff5b8bad2c80a3522599195bf2a393f744e Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Fri, 27 Dec 2019 11:10:07 +0100 Subject: [PATCH 13/20] Move cargo watch functionality to separate crate --- Cargo.lock | 17 +- crates/ra_cargo_watch/Cargo.toml | 17 + crates/ra_cargo_watch/src/conv.rs | 280 ++++ .../test__snap_clippy_pass_by_ref.snap | 2 +- .../test__snap_handles_macro_location.snap | 2 +- ...nap_rustc_incompatible_type_for_trait.snap | 2 +- .../test__snap_rustc_mismatched_type.snap | 2 +- .../test__snap_rustc_unused_variable.snap | 2 +- ...snap_rustc_wrong_number_of_parameters.snap | 2 +- crates/ra_cargo_watch/src/conv/test.rs | 698 +++++++++ crates/ra_cargo_watch/src/lib.rs | 345 +++++ crates/ra_lsp_server/Cargo.toml | 3 +- crates/ra_lsp_server/src/cargo_check.rs | 1316 ----------------- crates/ra_lsp_server/src/lib.rs | 1 - crates/ra_lsp_server/src/main_loop.rs | 12 +- crates/ra_lsp_server/src/world.rs | 10 +- 16 files changed, 1373 insertions(+), 1338 deletions(-) create mode 100644 crates/ra_cargo_watch/Cargo.toml create mode 100644 crates/ra_cargo_watch/src/conv.rs rename crates/{ra_lsp_server/src => ra_cargo_watch/src/conv}/snapshots/test__snap_clippy_pass_by_ref.snap (98%) rename crates/{ra_lsp_server/src => ra_cargo_watch/src/conv}/snapshots/test__snap_handles_macro_location.snap (95%) rename crates/{ra_lsp_server/src => ra_cargo_watch/src/conv}/snapshots/test__snap_rustc_incompatible_type_for_trait.snap (96%) rename crates/{ra_lsp_server/src => ra_cargo_watch/src/conv}/snapshots/test__snap_rustc_mismatched_type.snap (95%) rename crates/{ra_lsp_server/src => ra_cargo_watch/src/conv}/snapshots/test__snap_rustc_unused_variable.snap (97%) rename crates/{ra_lsp_server/src => ra_cargo_watch/src/conv}/snapshots/test__snap_rustc_wrong_number_of_parameters.snap (97%) create mode 100644 crates/ra_cargo_watch/src/conv/test.rs create mode 100644 crates/ra_cargo_watch/src/lib.rs delete mode 100644 crates/ra_lsp_server/src/cargo_check.rs diff --git a/Cargo.lock b/Cargo.lock index 08e4b7106e..7d29110763 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -897,6 +897,20 @@ dependencies = [ "rustc-hash 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "ra_cargo_watch" +version = "0.1.0" +dependencies = [ + "cargo_metadata 0.9.1 (registry+https://github.com/rust-lang/crates.io-index)", + "crossbeam-channel 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "insta 0.12.0 (registry+https://github.com/rust-lang/crates.io-index)", + "jod-thread 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", + "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", + "lsp-types 0.67.1 (registry+https://github.com/rust-lang/crates.io-index)", + "parking_lot 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)", + "serde_json 1.0.44 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "ra_cfg" version = "0.1.0" @@ -1051,15 +1065,14 @@ dependencies = [ name = "ra_lsp_server" version = "0.1.0" dependencies = [ - "cargo_metadata 0.9.1 (registry+https://github.com/rust-lang/crates.io-index)", "crossbeam-channel 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", - "insta 0.12.0 (registry+https://github.com/rust-lang/crates.io-index)", "jod-thread 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", "lsp-server 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "lsp-types 0.67.1 (registry+https://github.com/rust-lang/crates.io-index)", "parking_lot 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)", + "ra_cargo_watch 0.1.0", "ra_ide 0.1.0", "ra_prof 0.1.0", "ra_project_model 0.1.0", diff --git a/crates/ra_cargo_watch/Cargo.toml b/crates/ra_cargo_watch/Cargo.toml new file mode 100644 index 0000000000..bcc4648ff4 --- /dev/null +++ b/crates/ra_cargo_watch/Cargo.toml @@ -0,0 +1,17 @@ +[package] +edition = "2018" +name = "ra_cargo_watch" +version = "0.1.0" +authors = ["rust-analyzer developers"] + +[dependencies] +crossbeam-channel = "0.4" +lsp-types = { version = "0.67.0", features = ["proposed"] } +log = "0.4.3" +cargo_metadata = "0.9.1" +jod-thread = "0.1.0" +parking_lot = "0.10.0" + +[dev-dependencies] +insta = "0.12.0" +serde_json = "1.0" \ No newline at end of file diff --git a/crates/ra_cargo_watch/src/conv.rs b/crates/ra_cargo_watch/src/conv.rs new file mode 100644 index 0000000000..3bd4bf7a5f --- /dev/null +++ b/crates/ra_cargo_watch/src/conv.rs @@ -0,0 +1,280 @@ +//! 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, +}; +use lsp_types::{ + Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity, DiagnosticTag, Location, + NumberOrString, Position, Range, Url, +}; +use std::{fmt::Write, path::PathBuf}; + +#[cfg(test)] +mod test; + +/// Converts a Rust level string to a LSP severity +fn map_level_to_severity(val: DiagnosticLevel) -> Option { + match val { + DiagnosticLevel::Ice => Some(DiagnosticSeverity::Error), + DiagnosticLevel::Error => Some(DiagnosticSeverity::Error), + DiagnosticLevel::Warning => Some(DiagnosticSeverity::Warning), + DiagnosticLevel::Note => Some(DiagnosticSeverity::Information), + DiagnosticLevel::Help => Some(DiagnosticSeverity::Hint), + DiagnosticLevel::Unknown => None, + } +} + +/// Check whether a file name is from macro invocation +fn is_from_macro(file_name: &str) -> bool { + file_name.starts_with('<') && file_name.ends_with('>') +} + +/// Converts a Rust macro span to a LSP location recursively +fn map_macro_span_to_location( + span_macro: &DiagnosticSpanMacroExpansion, + workspace_root: &PathBuf, +) -> Option { + if !is_from_macro(&span_macro.span.file_name) { + return Some(map_span_to_location(&span_macro.span, workspace_root)); + } + + if let Some(expansion) = &span_macro.span.expansion { + return map_macro_span_to_location(&expansion, workspace_root); + } + + None +} + +/// Converts a Rust span to a LSP location +fn map_span_to_location(span: &DiagnosticSpan, workspace_root: &PathBuf) -> Location { + if is_from_macro(&span.file_name) && span.expansion.is_some() { + let expansion = span.expansion.as_ref().unwrap(); + if let Some(macro_range) = map_macro_span_to_location(&expansion, workspace_root) { + return macro_range; + } + } + + let mut file_name = workspace_root.clone(); + file_name.push(&span.file_name); + let uri = Url::from_file_path(file_name).unwrap(); + + let range = Range::new( + Position::new(span.line_start as u64 - 1, span.column_start as u64 - 1), + Position::new(span.line_end as u64 - 1, span.column_end as u64 - 1), + ); + + Location { uri, range } +} + +/// Converts a secondary Rust span to a LSP related information +/// +/// If the span is unlabelled this will return `None`. +fn map_secondary_span_to_related( + span: &DiagnosticSpan, + workspace_root: &PathBuf, +) -> Option { + if let Some(label) = &span.label { + let location = map_span_to_location(span, workspace_root); + Some(DiagnosticRelatedInformation { location, message: label.clone() }) + } else { + // Nothing to label this with + None + } +} + +/// Determines if diagnostic is related to unused code +fn is_unused_or_unnecessary(rd: &RustDiagnostic) -> bool { + if let Some(code) = &rd.code { + match code.code.as_str() { + "dead_code" | "unknown_lints" | "unreachable_code" | "unused_attributes" + | "unused_imports" | "unused_macros" | "unused_variables" => true, + _ => false, + } + } else { + false + } +} + +/// Determines if diagnostic is related to deprecated code +fn is_deprecated(rd: &RustDiagnostic) -> bool { + if let Some(code) = &rd.code { + match code.code.as_str() { + "deprecated" => true, + _ => false, + } + } else { + false + } +} + +#[derive(Debug)] +pub struct SuggestedFix { + pub title: String, + pub location: Location, + pub replacement: String, + pub applicability: Applicability, + pub diagnostics: Vec, +} + +impl std::cmp::PartialEq 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), + MessageLine(String), +} + +fn map_rust_child_diagnostic( + rd: &RustDiagnostic, + workspace_root: &PathBuf, +) -> MappedRustChildDiagnostic { + let span: &DiagnosticSpan = match rd.spans.iter().find(|s| s.is_primary) { + Some(span) => span, + None => { + // `rustc` uses these spanless children as a way to print multi-line + // messages + return MappedRustChildDiagnostic::MessageLine(rd.message.clone()); + } + }; + + // If we have a primary span use its location, otherwise use the parent + let location = map_span_to_location(&span, workspace_root); + + if let Some(suggested_replacement) = &span.suggested_replacement { + // Include our replacement in the title unless it's empty + let title = if !suggested_replacement.is_empty() { + format!("{}: '{}'", rd.message, suggested_replacement) + } else { + rd.message.clone() + }; + + MappedRustChildDiagnostic::SuggestedFix(SuggestedFix { + title, + location, + replacement: suggested_replacement.clone(), + applicability: span.suggestion_applicability.clone().unwrap_or(Applicability::Unknown), + diagnostics: vec![], + }) + } else { + MappedRustChildDiagnostic::Related(DiagnosticRelatedInformation { + location, + message: rd.message.clone(), + }) + } +} + +#[derive(Debug)] +pub(crate) struct MappedRustDiagnostic { + pub location: Location, + pub diagnostic: Diagnostic, + pub suggested_fixes: Vec, +} + +/// Converts a Rust root diagnostic to LSP form +/// +/// This flattens the Rust diagnostic by: +/// +/// 1. Creating a LSP diagnostic with the root message and primary span. +/// 2. Adding any labelled secondary spans to `relatedInformation` +/// 3. Categorising child diagnostics as either `SuggestedFix`es, +/// `relatedInformation` or additional message lines. +/// +/// If the diagnostic has no primary span this will return `None` +pub(crate) fn map_rust_diagnostic_to_lsp( + rd: &RustDiagnostic, + workspace_root: &PathBuf, +) -> Option { + let primary_span = rd.spans.iter().find(|s| s.is_primary)?; + + let location = map_span_to_location(&primary_span, workspace_root); + + let severity = map_level_to_severity(rd.level); + let mut primary_span_label = primary_span.label.as_ref(); + + let mut source = String::from("rustc"); + let mut code = rd.code.as_ref().map(|c| c.code.clone()); + if let Some(code_val) = &code { + // See if this is an RFC #2103 scoped lint (e.g. from Clippy) + let scoped_code: Vec<&str> = code_val.split("::").collect(); + if scoped_code.len() == 2 { + source = String::from(scoped_code[0]); + code = Some(String::from(scoped_code[1])); + } + } + + let mut related_information = vec![]; + let mut tags = vec![]; + + for secondary_span in rd.spans.iter().filter(|s| !s.is_primary) { + let related = map_secondary_span_to_related(secondary_span, workspace_root); + if let Some(related) = related { + related_information.push(related); + } + } + + let mut suggested_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::MessageLine(message_line) => { + write!(&mut message, "\n{}", message_line).unwrap(); + + // These secondary messages usually duplicate the content of the + // primary span label. + primary_span_label = None; + } + } + } + + if let Some(primary_span_label) = primary_span_label { + write!(&mut message, "\n{}", primary_span_label).unwrap(); + } + + if is_unused_or_unnecessary(rd) { + tags.push(DiagnosticTag::Unnecessary); + } + + if is_deprecated(rd) { + tags.push(DiagnosticTag::Deprecated); + } + + let diagnostic = Diagnostic { + range: location.range, + severity, + code: code.map(NumberOrString::String), + source: Some(source), + message, + related_information: if !related_information.is_empty() { + Some(related_information) + } else { + None + }, + tags: if !tags.is_empty() { Some(tags) } else { None }, + }; + + Some(MappedRustDiagnostic { location, diagnostic, suggested_fixes }) +} diff --git a/crates/ra_lsp_server/src/snapshots/test__snap_clippy_pass_by_ref.snap b/crates/ra_cargo_watch/src/conv/snapshots/test__snap_clippy_pass_by_ref.snap similarity index 98% rename from crates/ra_lsp_server/src/snapshots/test__snap_clippy_pass_by_ref.snap rename to crates/ra_cargo_watch/src/conv/snapshots/test__snap_clippy_pass_by_ref.snap index a5ce291572..cb0920914b 100644 --- a/crates/ra_lsp_server/src/snapshots/test__snap_clippy_pass_by_ref.snap +++ b/crates/ra_cargo_watch/src/conv/snapshots/test__snap_clippy_pass_by_ref.snap @@ -1,5 +1,5 @@ --- -source: crates/ra_lsp_server/src/cargo_check.rs +source: crates/ra_cargo_watch/src/conv/test.rs expression: diag --- MappedRustDiagnostic { diff --git a/crates/ra_lsp_server/src/snapshots/test__snap_handles_macro_location.snap b/crates/ra_cargo_watch/src/conv/snapshots/test__snap_handles_macro_location.snap similarity index 95% rename from crates/ra_lsp_server/src/snapshots/test__snap_handles_macro_location.snap rename to crates/ra_cargo_watch/src/conv/snapshots/test__snap_handles_macro_location.snap index 07e363ebf1..19510ecc1e 100644 --- a/crates/ra_lsp_server/src/snapshots/test__snap_handles_macro_location.snap +++ b/crates/ra_cargo_watch/src/conv/snapshots/test__snap_handles_macro_location.snap @@ -1,5 +1,5 @@ --- -source: crates/ra_lsp_server/src/cargo_check.rs +source: crates/ra_cargo_watch/src/conv/test.rs expression: diag --- MappedRustDiagnostic { diff --git a/crates/ra_lsp_server/src/snapshots/test__snap_rustc_incompatible_type_for_trait.snap b/crates/ra_cargo_watch/src/conv/snapshots/test__snap_rustc_incompatible_type_for_trait.snap similarity index 96% rename from crates/ra_lsp_server/src/snapshots/test__snap_rustc_incompatible_type_for_trait.snap rename to crates/ra_cargo_watch/src/conv/snapshots/test__snap_rustc_incompatible_type_for_trait.snap index 85a87db0b8..cf683e4b6f 100644 --- a/crates/ra_lsp_server/src/snapshots/test__snap_rustc_incompatible_type_for_trait.snap +++ b/crates/ra_cargo_watch/src/conv/snapshots/test__snap_rustc_incompatible_type_for_trait.snap @@ -1,5 +1,5 @@ --- -source: crates/ra_lsp_server/src/cargo_check.rs +source: crates/ra_cargo_watch/src/conv/test.rs expression: diag --- MappedRustDiagnostic { diff --git a/crates/ra_lsp_server/src/snapshots/test__snap_rustc_mismatched_type.snap b/crates/ra_cargo_watch/src/conv/snapshots/test__snap_rustc_mismatched_type.snap similarity index 95% rename from crates/ra_lsp_server/src/snapshots/test__snap_rustc_mismatched_type.snap rename to crates/ra_cargo_watch/src/conv/snapshots/test__snap_rustc_mismatched_type.snap index 69cb8badfc..8c1483c74b 100644 --- a/crates/ra_lsp_server/src/snapshots/test__snap_rustc_mismatched_type.snap +++ b/crates/ra_cargo_watch/src/conv/snapshots/test__snap_rustc_mismatched_type.snap @@ -1,5 +1,5 @@ --- -source: crates/ra_lsp_server/src/cargo_check.rs +source: crates/ra_cargo_watch/src/conv/test.rs expression: diag --- MappedRustDiagnostic { diff --git a/crates/ra_lsp_server/src/snapshots/test__snap_rustc_unused_variable.snap b/crates/ra_cargo_watch/src/conv/snapshots/test__snap_rustc_unused_variable.snap similarity index 97% rename from crates/ra_lsp_server/src/snapshots/test__snap_rustc_unused_variable.snap rename to crates/ra_cargo_watch/src/conv/snapshots/test__snap_rustc_unused_variable.snap index 33a3e30348..eb5a2247be 100644 --- a/crates/ra_lsp_server/src/snapshots/test__snap_rustc_unused_variable.snap +++ b/crates/ra_cargo_watch/src/conv/snapshots/test__snap_rustc_unused_variable.snap @@ -1,5 +1,5 @@ --- -source: crates/ra_lsp_server/src/cargo_check.rs +source: crates/ra_cargo_watch/src/conv/test.rs expression: diag --- MappedRustDiagnostic { diff --git a/crates/ra_lsp_server/src/snapshots/test__snap_rustc_wrong_number_of_parameters.snap b/crates/ra_cargo_watch/src/conv/snapshots/test__snap_rustc_wrong_number_of_parameters.snap similarity index 97% rename from crates/ra_lsp_server/src/snapshots/test__snap_rustc_wrong_number_of_parameters.snap rename to crates/ra_cargo_watch/src/conv/snapshots/test__snap_rustc_wrong_number_of_parameters.snap index ced6fa4df1..2f4518931e 100644 --- a/crates/ra_lsp_server/src/snapshots/test__snap_rustc_wrong_number_of_parameters.snap +++ b/crates/ra_cargo_watch/src/conv/snapshots/test__snap_rustc_wrong_number_of_parameters.snap @@ -1,5 +1,5 @@ --- -source: crates/ra_lsp_server/src/cargo_check.rs +source: crates/ra_cargo_watch/src/conv/test.rs expression: diag --- MappedRustDiagnostic { diff --git a/crates/ra_cargo_watch/src/conv/test.rs b/crates/ra_cargo_watch/src/conv/test.rs new file mode 100644 index 0000000000..69a07be11c --- /dev/null +++ b/crates/ra_cargo_watch/src/conv/test.rs @@ -0,0 +1,698 @@ +use crate::*; + +fn parse_diagnostic(val: &str) -> cargo_metadata::diagnostic::Diagnostic { + serde_json::from_str::(val).unwrap() +} + +#[test] +fn snap_rustc_incompatible_type_for_trait() { + let diag = parse_diagnostic( + r##"{ + "message": "method `next` has an incompatible type for trait", + "code": { + "code": "E0053", + "explanation": "\nThe parameters of any trait method must match between a trait implementation\nand the trait definition.\n\nHere are a couple examples of this error:\n\n```compile_fail,E0053\ntrait Foo {\n fn foo(x: u16);\n fn bar(&self);\n}\n\nstruct Bar;\n\nimpl Foo for Bar {\n // error, expected u16, found i16\n fn foo(x: i16) { }\n\n // error, types differ in mutability\n fn bar(&mut self) { }\n}\n```\n" + }, + "level": "error", + "spans": [ + { + "file_name": "compiler/ty/list_iter.rs", + "byte_start": 1307, + "byte_end": 1350, + "line_start": 52, + "line_end": 52, + "column_start": 5, + "column_end": 48, + "is_primary": true, + "text": [ + { + "text": " fn next(&self) -> Option<&'list ty::Ref> {", + "highlight_start": 5, + "highlight_end": 48 + } + ], + "label": "types differ in mutability", + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + ], + "children": [ + { + "message": "expected type `fn(&mut ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&ty::Ref>`\n found type `fn(&ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&'list ty::Ref>`", + "code": null, + "level": "note", + "spans": [], + "children": [], + "rendered": null + } + ], + "rendered": "error[E0053]: method `next` has an incompatible type for trait\n --> compiler/ty/list_iter.rs:52:5\n |\n52 | fn next(&self) -> Option<&'list ty::Ref> {\n | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ types differ in mutability\n |\n = note: expected type `fn(&mut ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&ty::Ref>`\n found type `fn(&ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&'list ty::Ref>`\n\n" + } + "##, + ); + + let workspace_root = PathBuf::from("/test/"); + let diag = map_rust_diagnostic_to_lsp(&diag, &workspace_root).expect("couldn't map diagnostic"); + insta::assert_debug_snapshot!(diag); +} + +#[test] +fn snap_rustc_unused_variable() { + let diag = parse_diagnostic( + r##"{ +"message": "unused variable: `foo`", +"code": { + "code": "unused_variables", + "explanation": null +}, +"level": "warning", +"spans": [ + { + "file_name": "driver/subcommand/repl.rs", + "byte_start": 9228, + "byte_end": 9231, + "line_start": 291, + "line_end": 291, + "column_start": 9, + "column_end": 12, + "is_primary": true, + "text": [ + { + "text": " let foo = 42;", + "highlight_start": 9, + "highlight_end": 12 + } + ], + "label": null, + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } +], +"children": [ + { + "message": "#[warn(unused_variables)] on by default", + "code": null, + "level": "note", + "spans": [], + "children": [], + "rendered": null + }, + { + "message": "consider prefixing with an underscore", + "code": null, + "level": "help", + "spans": [ + { + "file_name": "driver/subcommand/repl.rs", + "byte_start": 9228, + "byte_end": 9231, + "line_start": 291, + "line_end": 291, + "column_start": 9, + "column_end": 12, + "is_primary": true, + "text": [ + { + "text": " let foo = 42;", + "highlight_start": 9, + "highlight_end": 12 + } + ], + "label": null, + "suggested_replacement": "_foo", + "suggestion_applicability": "MachineApplicable", + "expansion": null + } + ], + "children": [], + "rendered": null + } +], +"rendered": "warning: unused variable: `foo`\n --> driver/subcommand/repl.rs:291:9\n |\n291 | let foo = 42;\n | ^^^ help: consider prefixing with an underscore: `_foo`\n |\n = note: #[warn(unused_variables)] on by default\n\n" +}"##, + ); + + let workspace_root = PathBuf::from("/test/"); + let diag = map_rust_diagnostic_to_lsp(&diag, &workspace_root).expect("couldn't map diagnostic"); + insta::assert_debug_snapshot!(diag); +} + +#[test] +fn snap_rustc_wrong_number_of_parameters() { + let diag = parse_diagnostic( + r##"{ +"message": "this function takes 2 parameters but 3 parameters were supplied", +"code": { + "code": "E0061", + "explanation": "\nThe number of arguments passed to a function must match the number of arguments\nspecified in the function signature.\n\nFor example, a function like:\n\n```\nfn f(a: u16, b: &str) {}\n```\n\nMust always be called with exactly two arguments, e.g., `f(2, \"test\")`.\n\nNote that Rust does not have a notion of optional function arguments or\nvariadic functions (except for its C-FFI).\n" +}, +"level": "error", +"spans": [ + { + "file_name": "compiler/ty/select.rs", + "byte_start": 8787, + "byte_end": 9241, + "line_start": 219, + "line_end": 231, + "column_start": 5, + "column_end": 6, + "is_primary": false, + "text": [ + { + "text": " pub fn add_evidence(", + "highlight_start": 5, + "highlight_end": 25 + }, + { + "text": " &mut self,", + "highlight_start": 1, + "highlight_end": 19 + }, + { + "text": " target_poly: &ty::Ref,", + "highlight_start": 1, + "highlight_end": 41 + }, + { + "text": " evidence_poly: &ty::Ref,", + "highlight_start": 1, + "highlight_end": 43 + }, + { + "text": " ) {", + "highlight_start": 1, + "highlight_end": 8 + }, + { + "text": " match target_poly {", + "highlight_start": 1, + "highlight_end": 28 + }, + { + "text": " ty::Ref::Var(tvar, _) => self.add_var_evidence(tvar, evidence_poly),", + "highlight_start": 1, + "highlight_end": 81 + }, + { + "text": " ty::Ref::Fixed(target_ty) => {", + "highlight_start": 1, + "highlight_end": 43 + }, + { + "text": " let evidence_ty = evidence_poly.resolve_to_ty();", + "highlight_start": 1, + "highlight_end": 65 + }, + { + "text": " self.add_evidence_ty(target_ty, evidence_poly, evidence_ty)", + "highlight_start": 1, + "highlight_end": 76 + }, + { + "text": " }", + "highlight_start": 1, + "highlight_end": 14 + }, + { + "text": " }", + "highlight_start": 1, + "highlight_end": 10 + }, + { + "text": " }", + "highlight_start": 1, + "highlight_end": 6 + } + ], + "label": "defined here", + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + }, + { + "file_name": "compiler/ty/select.rs", + "byte_start": 4045, + "byte_end": 4057, + "line_start": 104, + "line_end": 104, + "column_start": 18, + "column_end": 30, + "is_primary": true, + "text": [ + { + "text": " self.add_evidence(target_fixed, evidence_fixed, false);", + "highlight_start": 18, + "highlight_end": 30 + } + ], + "label": "expected 2 parameters", + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } +], +"children": [], +"rendered": "error[E0061]: this function takes 2 parameters but 3 parameters were supplied\n --> compiler/ty/select.rs:104:18\n |\n104 | self.add_evidence(target_fixed, evidence_fixed, false);\n | ^^^^^^^^^^^^ expected 2 parameters\n...\n219 | / pub fn add_evidence(\n220 | | &mut self,\n221 | | target_poly: &ty::Ref,\n222 | | evidence_poly: &ty::Ref,\n... |\n230 | | }\n231 | | }\n | |_____- defined here\n\n" +}"##, + ); + + let workspace_root = PathBuf::from("/test/"); + let diag = map_rust_diagnostic_to_lsp(&diag, &workspace_root).expect("couldn't map diagnostic"); + insta::assert_debug_snapshot!(diag); +} + +#[test] +fn snap_clippy_pass_by_ref() { + let diag = parse_diagnostic( + r##"{ +"message": "this argument is passed by reference, but would be more efficient if passed by value", +"code": { + "code": "clippy::trivially_copy_pass_by_ref", + "explanation": null +}, +"level": "warning", +"spans": [ + { + "file_name": "compiler/mir/tagset.rs", + "byte_start": 941, + "byte_end": 946, + "line_start": 42, + "line_end": 42, + "column_start": 24, + "column_end": 29, + "is_primary": true, + "text": [ + { + "text": " pub fn is_disjoint(&self, other: Self) -> bool {", + "highlight_start": 24, + "highlight_end": 29 + } + ], + "label": null, + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } +], +"children": [ + { + "message": "lint level defined here", + "code": null, + "level": "note", + "spans": [ + { + "file_name": "compiler/lib.rs", + "byte_start": 8, + "byte_end": 19, + "line_start": 1, + "line_end": 1, + "column_start": 9, + "column_end": 20, + "is_primary": true, + "text": [ + { + "text": "#![warn(clippy::all)]", + "highlight_start": 9, + "highlight_end": 20 + } + ], + "label": null, + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + ], + "children": [], + "rendered": null + }, + { + "message": "#[warn(clippy::trivially_copy_pass_by_ref)] implied by #[warn(clippy::all)]", + "code": null, + "level": "note", + "spans": [], + "children": [], + "rendered": null + }, + { + "message": "for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref", + "code": null, + "level": "help", + "spans": [], + "children": [], + "rendered": null + }, + { + "message": "consider passing by value instead", + "code": null, + "level": "help", + "spans": [ + { + "file_name": "compiler/mir/tagset.rs", + "byte_start": 941, + "byte_end": 946, + "line_start": 42, + "line_end": 42, + "column_start": 24, + "column_end": 29, + "is_primary": true, + "text": [ + { + "text": " pub fn is_disjoint(&self, other: Self) -> bool {", + "highlight_start": 24, + "highlight_end": 29 + } + ], + "label": null, + "suggested_replacement": "self", + "suggestion_applicability": "Unspecified", + "expansion": null + } + ], + "children": [], + "rendered": null + } +], +"rendered": "warning: this argument is passed by reference, but would be more efficient if passed by value\n --> compiler/mir/tagset.rs:42:24\n |\n42 | pub fn is_disjoint(&self, other: Self) -> bool {\n | ^^^^^ help: consider passing by value instead: `self`\n |\nnote: lint level defined here\n --> compiler/lib.rs:1:9\n |\n1 | #![warn(clippy::all)]\n | ^^^^^^^^^^^\n = note: #[warn(clippy::trivially_copy_pass_by_ref)] implied by #[warn(clippy::all)]\n = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref\n\n" +}"##, + ); + + let workspace_root = PathBuf::from("/test/"); + let diag = map_rust_diagnostic_to_lsp(&diag, &workspace_root).expect("couldn't map diagnostic"); + insta::assert_debug_snapshot!(diag); +} + +#[test] +fn snap_rustc_mismatched_type() { + let diag = parse_diagnostic( + r##"{ +"message": "mismatched types", +"code": { + "code": "E0308", + "explanation": "\nThis error occurs when the compiler was unable to infer the concrete type of a\nvariable. It can occur for several cases, the most common of which is a\nmismatch in the expected type that the compiler inferred for a variable's\ninitializing expression, and the actual type explicitly assigned to the\nvariable.\n\nFor example:\n\n```compile_fail,E0308\nlet x: i32 = \"I am not a number!\";\n// ~~~ ~~~~~~~~~~~~~~~~~~~~\n// | |\n// | initializing expression;\n// | compiler infers type `&str`\n// |\n// type `i32` assigned to variable `x`\n```\n" +}, +"level": "error", +"spans": [ + { + "file_name": "runtime/compiler_support.rs", + "byte_start": 1589, + "byte_end": 1594, + "line_start": 48, + "line_end": 48, + "column_start": 65, + "column_end": 70, + "is_primary": true, + "text": [ + { + "text": " let layout = alloc::Layout::from_size_align_unchecked(size, align);", + "highlight_start": 65, + "highlight_end": 70 + } + ], + "label": "expected usize, found u32", + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } +], +"children": [], +"rendered": "error[E0308]: mismatched types\n --> runtime/compiler_support.rs:48:65\n |\n48 | let layout = alloc::Layout::from_size_align_unchecked(size, align);\n | ^^^^^ expected usize, found u32\n\n" +}"##, + ); + + let workspace_root = PathBuf::from("/test/"); + let diag = map_rust_diagnostic_to_lsp(&diag, &workspace_root).expect("couldn't map diagnostic"); + insta::assert_debug_snapshot!(diag); +} + +#[test] +fn snap_handles_macro_location() { + let diag = parse_diagnostic( + r##"{ +"rendered": "error[E0277]: can't compare `{integer}` with `&str`\n --> src/main.rs:2:5\n |\n2 | assert_eq!(1, \"love\");\n | ^^^^^^^^^^^^^^^^^^^^^^ no implementation for `{integer} == &str`\n |\n = help: the trait `std::cmp::PartialEq<&str>` is not implemented for `{integer}`\n = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)\n\n", +"children": [ + { + "children": [], + "code": null, + "level": "help", + "message": "the trait `std::cmp::PartialEq<&str>` is not implemented for `{integer}`", + "rendered": null, + "spans": [] + } +], +"code": { + "code": "E0277", + "explanation": "\nYou tried to use a type which doesn't implement some trait in a place which\nexpected that trait. Erroneous code example:\n\n```compile_fail,E0277\n// here we declare the Foo trait with a bar method\ntrait Foo {\n fn bar(&self);\n}\n\n// we now declare a function which takes an object implementing the Foo trait\nfn some_func(foo: T) {\n foo.bar();\n}\n\nfn main() {\n // we now call the method with the i32 type, which doesn't implement\n // the Foo trait\n some_func(5i32); // error: the trait bound `i32 : Foo` is not satisfied\n}\n```\n\nIn order to fix this error, verify that the type you're using does implement\nthe trait. Example:\n\n```\ntrait Foo {\n fn bar(&self);\n}\n\nfn some_func(foo: T) {\n foo.bar(); // we can now use this method since i32 implements the\n // Foo trait\n}\n\n// we implement the trait on the i32 type\nimpl Foo for i32 {\n fn bar(&self) {}\n}\n\nfn main() {\n some_func(5i32); // ok!\n}\n```\n\nOr in a generic context, an erroneous code example would look like:\n\n```compile_fail,E0277\nfn some_func(foo: T) {\n println!(\"{:?}\", foo); // error: the trait `core::fmt::Debug` is not\n // implemented for the type `T`\n}\n\nfn main() {\n // We now call the method with the i32 type,\n // which *does* implement the Debug trait.\n some_func(5i32);\n}\n```\n\nNote that the error here is in the definition of the generic function: Although\nwe only call it with a parameter that does implement `Debug`, the compiler\nstill rejects the function: It must work with all possible input types. In\norder to make this example compile, we need to restrict the generic type we're\naccepting:\n\n```\nuse std::fmt;\n\n// Restrict the input type to types that implement Debug.\nfn some_func(foo: T) {\n println!(\"{:?}\", foo);\n}\n\nfn main() {\n // Calling the method is still fine, as i32 implements Debug.\n some_func(5i32);\n\n // This would fail to compile now:\n // struct WithoutDebug;\n // some_func(WithoutDebug);\n}\n```\n\nRust only looks at the signature of the called function, as such it must\nalready specify all requirements that will be used for every type parameter.\n" +}, +"level": "error", +"message": "can't compare `{integer}` with `&str`", +"spans": [ + { + "byte_end": 155, + "byte_start": 153, + "column_end": 33, + "column_start": 31, + "expansion": { + "def_site_span": { + "byte_end": 940, + "byte_start": 0, + "column_end": 6, + "column_start": 1, + "expansion": null, + "file_name": "<::core::macros::assert_eq macros>", + "is_primary": false, + "label": null, + "line_end": 36, + "line_start": 1, + "suggested_replacement": null, + "suggestion_applicability": null, + "text": [ + { + "highlight_end": 35, + "highlight_start": 1, + "text": "($ left : expr, $ right : expr) =>" + }, + { + "highlight_end": 3, + "highlight_start": 1, + "text": "({" + }, + { + "highlight_end": 33, + "highlight_start": 1, + "text": " match (& $ left, & $ right)" + }, + { + "highlight_end": 7, + "highlight_start": 1, + "text": " {" + }, + { + "highlight_end": 34, + "highlight_start": 1, + "text": " (left_val, right_val) =>" + }, + { + "highlight_end": 11, + "highlight_start": 1, + "text": " {" + }, + { + "highlight_end": 46, + "highlight_start": 1, + "text": " if ! (* left_val == * right_val)" + }, + { + "highlight_end": 15, + "highlight_start": 1, + "text": " {" + }, + { + "highlight_end": 25, + "highlight_start": 1, + "text": " panic !" + }, + { + "highlight_end": 57, + "highlight_start": 1, + "text": " (r#\"assertion failed: `(left == right)`" + }, + { + "highlight_end": 16, + "highlight_start": 1, + "text": " left: `{:?}`," + }, + { + "highlight_end": 18, + "highlight_start": 1, + "text": " right: `{:?}`\"#," + }, + { + "highlight_end": 47, + "highlight_start": 1, + "text": " & * left_val, & * right_val)" + }, + { + "highlight_end": 15, + "highlight_start": 1, + "text": " }" + }, + { + "highlight_end": 11, + "highlight_start": 1, + "text": " }" + }, + { + "highlight_end": 7, + "highlight_start": 1, + "text": " }" + }, + { + "highlight_end": 42, + "highlight_start": 1, + "text": " }) ; ($ left : expr, $ right : expr,) =>" + }, + { + "highlight_end": 49, + "highlight_start": 1, + "text": "({ $ crate :: assert_eq ! ($ left, $ right) }) ;" + }, + { + "highlight_end": 53, + "highlight_start": 1, + "text": "($ left : expr, $ right : expr, $ ($ arg : tt) +) =>" + }, + { + "highlight_end": 3, + "highlight_start": 1, + "text": "({" + }, + { + "highlight_end": 37, + "highlight_start": 1, + "text": " match (& ($ left), & ($ right))" + }, + { + "highlight_end": 7, + "highlight_start": 1, + "text": " {" + }, + { + "highlight_end": 34, + "highlight_start": 1, + "text": " (left_val, right_val) =>" + }, + { + "highlight_end": 11, + "highlight_start": 1, + "text": " {" + }, + { + "highlight_end": 46, + "highlight_start": 1, + "text": " if ! (* left_val == * right_val)" + }, + { + "highlight_end": 15, + "highlight_start": 1, + "text": " {" + }, + { + "highlight_end": 25, + "highlight_start": 1, + "text": " panic !" + }, + { + "highlight_end": 57, + "highlight_start": 1, + "text": " (r#\"assertion failed: `(left == right)`" + }, + { + "highlight_end": 16, + "highlight_start": 1, + "text": " left: `{:?}`," + }, + { + "highlight_end": 22, + "highlight_start": 1, + "text": " right: `{:?}`: {}\"#," + }, + { + "highlight_end": 72, + "highlight_start": 1, + "text": " & * left_val, & * right_val, $ crate :: format_args !" + }, + { + "highlight_end": 33, + "highlight_start": 1, + "text": " ($ ($ arg) +))" + }, + { + "highlight_end": 15, + "highlight_start": 1, + "text": " }" + }, + { + "highlight_end": 11, + "highlight_start": 1, + "text": " }" + }, + { + "highlight_end": 7, + "highlight_start": 1, + "text": " }" + }, + { + "highlight_end": 6, + "highlight_start": 1, + "text": " }) ;" + } + ] + }, + "macro_decl_name": "assert_eq!", + "span": { + "byte_end": 38, + "byte_start": 16, + "column_end": 27, + "column_start": 5, + "expansion": null, + "file_name": "src/main.rs", + "is_primary": false, + "label": null, + "line_end": 2, + "line_start": 2, + "suggested_replacement": null, + "suggestion_applicability": null, + "text": [ + { + "highlight_end": 27, + "highlight_start": 5, + "text": " assert_eq!(1, \"love\");" + } + ] + } + }, + "file_name": "<::core::macros::assert_eq macros>", + "is_primary": true, + "label": "no implementation for `{integer} == &str`", + "line_end": 7, + "line_start": 7, + "suggested_replacement": null, + "suggestion_applicability": null, + "text": [ + { + "highlight_end": 33, + "highlight_start": 31, + "text": " if ! (* left_val == * right_val)" + } + ] + } +] +}"##, + ); + + let workspace_root = PathBuf::from("/test/"); + let diag = map_rust_diagnostic_to_lsp(&diag, &workspace_root).expect("couldn't map diagnostic"); + insta::assert_debug_snapshot!(diag); +} diff --git a/crates/ra_cargo_watch/src/lib.rs b/crates/ra_cargo_watch/src/lib.rs new file mode 100644 index 0000000000..c863866108 --- /dev/null +++ b/crates/ra_cargo_watch/src/lib.rs @@ -0,0 +1,345 @@ +//! cargo_check provides the functionality needed to run `cargo check` or +//! another compatible command (f.x. clippy) in a background thread and provide +//! LSP diagnostics based on the output of the command. +use cargo_metadata::Message; +use crossbeam_channel::{select, unbounded, Receiver, RecvError, Sender, TryRecvError}; +use lsp_types::{ + Diagnostic, Url, WorkDoneProgress, WorkDoneProgressBegin, WorkDoneProgressEnd, + WorkDoneProgressReport, +}; +use parking_lot::RwLock; +use std::{ + collections::HashMap, + path::PathBuf, + process::{Command, Stdio}, + sync::Arc, + thread::JoinHandle, + time::Instant, +}; + +mod conv; + +use crate::conv::{map_rust_diagnostic_to_lsp, MappedRustDiagnostic, SuggestedFix}; + +#[derive(Clone, Debug)] +pub struct CheckOptions { + pub enable: bool, + pub args: Vec, + pub command: String, + pub all_targets: bool, +} + +/// CheckWatcher wraps the shared state and communication machinery used for +/// running `cargo check` (or other compatible command) and providing +/// diagnostics based on the output. +#[derive(Debug)] +pub struct CheckWatcher { + pub task_recv: Receiver, + pub cmd_send: Sender, + pub shared: Arc>, + handle: JoinHandle<()>, +} + +impl CheckWatcher { + pub fn new(options: &CheckOptions, workspace_root: PathBuf) -> CheckWatcher { + let options = options.clone(); + let shared = Arc::new(RwLock::new(CheckWatcherSharedState::new())); + + let (task_send, task_recv) = unbounded::(); + let (cmd_send, cmd_recv) = unbounded::(); + let shared_ = shared.clone(); + let handle = std::thread::spawn(move || { + let mut check = CheckWatcherState::new(options, workspace_root, shared_); + check.run(&task_send, &cmd_recv); + }); + + CheckWatcher { task_recv, cmd_send, handle, shared } + } + + /// Schedule a re-start of the cargo check worker. + pub fn update(&self) { + self.cmd_send.send(CheckCommand::Update).unwrap(); + } +} + +pub struct CheckWatcherState { + options: CheckOptions, + workspace_root: PathBuf, + running: bool, + watcher: WatchThread, + last_update_req: Option, + shared: Arc>, +} + +#[derive(Debug)] +pub struct CheckWatcherSharedState { + diagnostic_collection: HashMap>, + suggested_fix_collection: HashMap>, +} + +impl CheckWatcherSharedState { + fn new() -> CheckWatcherSharedState { + CheckWatcherSharedState { + 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, task_send: &Sender) { + let cleared_files: Vec = self.diagnostic_collection.keys().cloned().collect(); + + self.diagnostic_collection.clear(); + self.suggested_fix_collection.clear(); + + for uri in cleared_files { + task_send.send(CheckTask::Update(uri.clone())).unwrap(); + } + } + + 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()) + } + + 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 update of the given files diagnostics + Update(Url), + + /// Request check progress notification to client + Status(WorkDoneProgress), +} + +pub enum CheckCommand { + /// Request re-start of check thread + Update, +} + +impl CheckWatcherState { + pub fn new( + options: CheckOptions, + workspace_root: PathBuf, + shared: Arc>, + ) -> CheckWatcherState { + let watcher = WatchThread::new(&options, &workspace_root); + CheckWatcherState { + options, + workspace_root, + running: false, + watcher, + last_update_req: None, + shared, + } + } + + pub fn run(&mut self, task_send: &Sender, cmd_recv: &Receiver) { + self.running = true; + while self.running { + select! { + recv(&cmd_recv) -> cmd => match cmd { + Ok(cmd) => self.handle_command(cmd), + Err(RecvError) => { + // Command channel has closed, so shut down + self.running = false; + }, + }, + recv(self.watcher.message_recv) -> msg => match msg { + Ok(msg) => self.handle_message(msg, task_send), + Err(RecvError) => {}, + } + }; + + if self.should_recheck() { + self.last_update_req.take(); + self.shared.write().clear(task_send); + + self.watcher.cancel(); + self.watcher = WatchThread::new(&self.options, &self.workspace_root); + } + } + } + + fn should_recheck(&mut self) -> bool { + if let Some(_last_update_req) = &self.last_update_req { + // We currently only request an update on save, as we need up to + // date source on disk for cargo check to do it's magic, so we + // don't really need to debounce the requests at this point. + return true; + } + false + } + + fn handle_command(&mut self, cmd: CheckCommand) { + match cmd { + CheckCommand::Update => self.last_update_req = Some(Instant::now()), + } + } + + fn handle_message(&mut self, msg: CheckEvent, task_send: &Sender) { + match msg { + CheckEvent::Begin => { + task_send + .send(CheckTask::Status(WorkDoneProgress::Begin(WorkDoneProgressBegin { + title: "Running 'cargo check'".to_string(), + cancellable: Some(false), + message: None, + percentage: None, + }))) + .unwrap(); + } + + CheckEvent::End => { + task_send + .send(CheckTask::Status(WorkDoneProgress::End(WorkDoneProgressEnd { + message: None, + }))) + .unwrap(); + } + + CheckEvent::Msg(Message::CompilerArtifact(msg)) => { + task_send + .send(CheckTask::Status(WorkDoneProgress::Report(WorkDoneProgressReport { + cancellable: Some(false), + message: Some(msg.target.name), + percentage: None, + }))) + .unwrap(); + } + + CheckEvent::Msg(Message::CompilerMessage(msg)) => { + let map_result = + match map_rust_diagnostic_to_lsp(&msg.message, &self.workspace_root) { + Some(map_result) => map_result, + None => return, + }; + + let MappedRustDiagnostic { location, diagnostic, suggested_fixes } = map_result; + let file_uri = location.uri.clone(); + + if !suggested_fixes.is_empty() { + for suggested_fix in suggested_fixes { + self.shared + .write() + .add_suggested_fix_for_diagnostic(suggested_fix, &diagnostic); + } + } + self.shared.write().add_diagnostic(file_uri, diagnostic); + + task_send.send(CheckTask::Update(location.uri)).unwrap(); + } + + CheckEvent::Msg(Message::BuildScriptExecuted(_msg)) => {} + CheckEvent::Msg(Message::Unknown) => {} + } + } +} + +/// WatchThread exists to wrap around the communication needed to be able to +/// run `cargo check` without blocking. Currently the Rust standard library +/// doesn't provide a way to read sub-process output without blocking, so we +/// have to wrap sub-processes output handling in a thread and pass messages +/// back over a channel. +struct WatchThread { + message_recv: Receiver, + cancel_send: Sender<()>, +} + +enum CheckEvent { + Begin, + Msg(cargo_metadata::Message), + End, +} + +impl WatchThread { + fn new(options: &CheckOptions, workspace_root: &PathBuf) -> WatchThread { + let mut args: Vec = vec![ + options.command.clone(), + "--message-format=json".to_string(), + "--manifest-path".to_string(), + format!("{}/Cargo.toml", workspace_root.to_string_lossy()), + ]; + if options.all_targets { + args.push("--all-targets".to_string()); + } + args.extend(options.args.iter().cloned()); + + let (message_send, message_recv) = unbounded(); + let (cancel_send, cancel_recv) = unbounded(); + let enabled = options.enable; + std::thread::spawn(move || { + if !enabled { + return; + } + + let mut command = Command::new("cargo") + .args(&args) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .spawn() + .expect("couldn't launch cargo"); + + message_send.send(CheckEvent::Begin).unwrap(); + for message in cargo_metadata::parse_messages(command.stdout.take().unwrap()) { + match cancel_recv.try_recv() { + Ok(()) | Err(TryRecvError::Disconnected) => { + command.kill().expect("couldn't kill command"); + } + Err(TryRecvError::Empty) => (), + } + + message_send.send(CheckEvent::Msg(message.unwrap())).unwrap(); + } + message_send.send(CheckEvent::End).unwrap(); + }); + WatchThread { message_recv, cancel_send } + } + + fn cancel(&self) { + let _ = self.cancel_send.send(()); + } +} + +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/Cargo.toml b/crates/ra_lsp_server/Cargo.toml index 54a01d7a2c..9b7dcb6e9a 100644 --- a/crates/ra_lsp_server/Cargo.toml +++ b/crates/ra_lsp_server/Cargo.toml @@ -27,10 +27,9 @@ ra_project_model = { path = "../ra_project_model" } ra_prof = { path = "../ra_prof" } ra_vfs_glob = { path = "../ra_vfs_glob" } env_logger = { version = "0.7.1", default-features = false, features = ["humantime"] } -cargo_metadata = "0.9.1" +ra_cargo_watch = { path = "../ra_cargo_watch" } [dev-dependencies] -insta = "0.12.0" tempfile = "3" test_utils = { path = "../test_utils" } diff --git a/crates/ra_lsp_server/src/cargo_check.rs b/crates/ra_lsp_server/src/cargo_check.rs deleted file mode 100644 index 70c723b199..0000000000 --- a/crates/ra_lsp_server/src/cargo_check.rs +++ /dev/null @@ -1,1316 +0,0 @@ -//! cargo_check provides the functionality needed to run `cargo check` or -//! another compatible command (f.x. clippy) in a background thread and provide -//! LSP diagnostics based on the output of the command. -use crate::world::Options; -use cargo_metadata::{ - diagnostic::{ - Applicability, Diagnostic as RustDiagnostic, DiagnosticLevel, DiagnosticSpan, - DiagnosticSpanMacroExpansion, - }, - Message, -}; -use crossbeam_channel::{select, unbounded, Receiver, RecvError, Sender, TryRecvError}; -use lsp_types::{ - Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity, DiagnosticTag, Location, - NumberOrString, Position, Range, Url, WorkDoneProgress, WorkDoneProgressBegin, - WorkDoneProgressEnd, WorkDoneProgressReport, -}; -use parking_lot::RwLock; -use std::{ - collections::HashMap, - fmt::Write, - path::PathBuf, - process::{Command, Stdio}, - sync::Arc, - thread::JoinHandle, - time::Instant, -}; - -/// CheckWatcher wraps the shared state and communication machinery used for -/// running `cargo check` (or other compatible command) and providing -/// diagnostics based on the output. -#[derive(Debug)] -pub struct CheckWatcher { - pub task_recv: Receiver, - pub cmd_send: Sender, - pub shared: Arc>, - handle: JoinHandle<()>, -} - -impl CheckWatcher { - pub fn new(options: &Options, workspace_root: PathBuf) -> CheckWatcher { - let options = options.clone(); - let shared = Arc::new(RwLock::new(CheckWatcherSharedState::new())); - - let (task_send, task_recv) = unbounded::(); - let (cmd_send, cmd_recv) = unbounded::(); - let shared_ = shared.clone(); - let handle = std::thread::spawn(move || { - let mut check = CheckWatcherState::new(options, workspace_root, shared_); - check.run(&task_send, &cmd_recv); - }); - - CheckWatcher { task_recv, cmd_send, handle, shared } - } - - /// Schedule a re-start of the cargo check worker. - pub fn update(&self) { - self.cmd_send.send(CheckCommand::Update).unwrap(); - } -} - -pub struct CheckWatcherState { - options: Options, - workspace_root: PathBuf, - running: bool, - watcher: WatchThread, - last_update_req: Option, - shared: Arc>, -} - -#[derive(Debug)] -pub struct CheckWatcherSharedState { - diagnostic_collection: HashMap>, - suggested_fix_collection: HashMap>, -} - -impl CheckWatcherSharedState { - fn new() -> CheckWatcherSharedState { - CheckWatcherSharedState { - 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, task_send: &Sender) { - let cleared_files: Vec = self.diagnostic_collection.keys().cloned().collect(); - - self.diagnostic_collection.clear(); - self.suggested_fix_collection.clear(); - - for uri in cleared_files { - task_send.send(CheckTask::Update(uri.clone())).unwrap(); - } - } - - 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()) - } - - 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 update of the given files diagnostics - Update(Url), - - /// Request check progress notification to client - Status(WorkDoneProgress), -} - -pub enum CheckCommand { - /// Request re-start of check thread - Update, -} - -impl CheckWatcherState { - pub fn new( - options: Options, - workspace_root: PathBuf, - shared: Arc>, - ) -> CheckWatcherState { - let watcher = WatchThread::new(&options, &workspace_root); - CheckWatcherState { - options, - workspace_root, - running: false, - watcher, - last_update_req: None, - shared, - } - } - - pub fn run(&mut self, task_send: &Sender, cmd_recv: &Receiver) { - self.running = true; - while self.running { - select! { - recv(&cmd_recv) -> cmd => match cmd { - Ok(cmd) => self.handle_command(cmd), - Err(RecvError) => { - // Command channel has closed, so shut down - self.running = false; - }, - }, - recv(self.watcher.message_recv) -> msg => match msg { - Ok(msg) => self.handle_message(msg, task_send), - Err(RecvError) => {}, - } - }; - - if self.should_recheck() { - self.last_update_req.take(); - self.shared.write().clear(task_send); - - self.watcher.cancel(); - self.watcher = WatchThread::new(&self.options, &self.workspace_root); - } - } - } - - fn should_recheck(&mut self) -> bool { - if let Some(_last_update_req) = &self.last_update_req { - // We currently only request an update on save, as we need up to - // date source on disk for cargo check to do it's magic, so we - // don't really need to debounce the requests at this point. - return true; - } - false - } - - fn handle_command(&mut self, cmd: CheckCommand) { - match cmd { - CheckCommand::Update => self.last_update_req = Some(Instant::now()), - } - } - - fn handle_message(&mut self, msg: CheckEvent, task_send: &Sender) { - match msg { - CheckEvent::Begin => { - task_send - .send(CheckTask::Status(WorkDoneProgress::Begin(WorkDoneProgressBegin { - title: "Running 'cargo check'".to_string(), - cancellable: Some(false), - message: None, - percentage: None, - }))) - .unwrap(); - } - - CheckEvent::End => { - task_send - .send(CheckTask::Status(WorkDoneProgress::End(WorkDoneProgressEnd { - message: None, - }))) - .unwrap(); - } - - CheckEvent::Msg(Message::CompilerArtifact(msg)) => { - task_send - .send(CheckTask::Status(WorkDoneProgress::Report(WorkDoneProgressReport { - cancellable: Some(false), - message: Some(msg.target.name), - percentage: None, - }))) - .unwrap(); - } - - CheckEvent::Msg(Message::CompilerMessage(msg)) => { - let map_result = - match map_rust_diagnostic_to_lsp(&msg.message, &self.workspace_root) { - Some(map_result) => map_result, - None => return, - }; - - let MappedRustDiagnostic { location, diagnostic, suggested_fixes } = map_result; - let file_uri = location.uri.clone(); - - if !suggested_fixes.is_empty() { - for suggested_fix in suggested_fixes { - self.shared - .write() - .add_suggested_fix_for_diagnostic(suggested_fix, &diagnostic); - } - } - self.shared.write().add_diagnostic(file_uri, diagnostic); - - task_send.send(CheckTask::Update(location.uri)).unwrap(); - } - - CheckEvent::Msg(Message::BuildScriptExecuted(_msg)) => {} - CheckEvent::Msg(Message::Unknown) => {} - } - } -} - -/// WatchThread exists to wrap around the communication needed to be able to -/// run `cargo check` without blocking. Currently the Rust standard library -/// doesn't provide a way to read sub-process output without blocking, so we -/// have to wrap sub-processes output handling in a thread and pass messages -/// back over a channel. -struct WatchThread { - message_recv: Receiver, - cancel_send: Sender<()>, -} - -enum CheckEvent { - Begin, - Msg(cargo_metadata::Message), - End, -} - -impl WatchThread { - fn new(options: &Options, workspace_root: &PathBuf) -> WatchThread { - let mut args: Vec = vec![ - options.cargo_watch_command.clone(), - "--message-format=json".to_string(), - "--manifest-path".to_string(), - format!("{}/Cargo.toml", workspace_root.to_string_lossy()), - ]; - if options.cargo_watch_all_targets { - args.push("--all-targets".to_string()); - } - args.extend(options.cargo_watch_args.iter().cloned()); - - let (message_send, message_recv) = unbounded(); - let (cancel_send, cancel_recv) = unbounded(); - let enabled = options.cargo_watch_enable; - std::thread::spawn(move || { - if !enabled { - return; - } - - let mut command = Command::new("cargo") - .args(&args) - .stdout(Stdio::piped()) - .stderr(Stdio::null()) - .spawn() - .expect("couldn't launch cargo"); - - message_send.send(CheckEvent::Begin).unwrap(); - for message in cargo_metadata::parse_messages(command.stdout.take().unwrap()) { - match cancel_recv.try_recv() { - Ok(()) | Err(TryRecvError::Disconnected) => { - command.kill().expect("couldn't kill command"); - } - Err(TryRecvError::Empty) => (), - } - - message_send.send(CheckEvent::Msg(message.unwrap())).unwrap(); - } - message_send.send(CheckEvent::End).unwrap(); - }); - WatchThread { message_recv, cancel_send } - } - - fn cancel(&self) { - let _ = self.cancel_send.send(()); - } -} - -/// Converts a Rust level string to a LSP severity -fn map_level_to_severity(val: DiagnosticLevel) -> Option { - match val { - DiagnosticLevel::Ice => Some(DiagnosticSeverity::Error), - DiagnosticLevel::Error => Some(DiagnosticSeverity::Error), - DiagnosticLevel::Warning => Some(DiagnosticSeverity::Warning), - DiagnosticLevel::Note => Some(DiagnosticSeverity::Information), - DiagnosticLevel::Help => Some(DiagnosticSeverity::Hint), - DiagnosticLevel::Unknown => None, - } -} - -/// Check whether a file name is from macro invocation -fn is_from_macro(file_name: &str) -> bool { - file_name.starts_with('<') && file_name.ends_with('>') -} - -/// Converts a Rust macro span to a LSP location recursively -fn map_macro_span_to_location( - span_macro: &DiagnosticSpanMacroExpansion, - workspace_root: &PathBuf, -) -> Option { - if !is_from_macro(&span_macro.span.file_name) { - return Some(map_span_to_location(&span_macro.span, workspace_root)); - } - - if let Some(expansion) = &span_macro.span.expansion { - return map_macro_span_to_location(&expansion, workspace_root); - } - - None -} - -/// Converts a Rust span to a LSP location -fn map_span_to_location(span: &DiagnosticSpan, workspace_root: &PathBuf) -> Location { - if is_from_macro(&span.file_name) && span.expansion.is_some() { - let expansion = span.expansion.as_ref().unwrap(); - if let Some(macro_range) = map_macro_span_to_location(&expansion, workspace_root) { - return macro_range; - } - } - - let mut file_name = workspace_root.clone(); - file_name.push(&span.file_name); - let uri = Url::from_file_path(file_name).unwrap(); - - let range = Range::new( - Position::new(span.line_start as u64 - 1, span.column_start as u64 - 1), - Position::new(span.line_end as u64 - 1, span.column_end as u64 - 1), - ); - - Location { uri, range } -} - -/// Converts a secondary Rust span to a LSP related information -/// -/// If the span is unlabelled this will return `None`. -fn map_secondary_span_to_related( - span: &DiagnosticSpan, - workspace_root: &PathBuf, -) -> Option { - if let Some(label) = &span.label { - let location = map_span_to_location(span, workspace_root); - Some(DiagnosticRelatedInformation { location, message: label.clone() }) - } else { - // Nothing to label this with - None - } -} - -/// Determines if diagnostic is related to unused code -fn is_unused_or_unnecessary(rd: &RustDiagnostic) -> bool { - if let Some(code) = &rd.code { - match code.code.as_str() { - "dead_code" | "unknown_lints" | "unreachable_code" | "unused_attributes" - | "unused_imports" | "unused_macros" | "unused_variables" => true, - _ => false, - } - } else { - false - } -} - -/// Determines if diagnostic is related to deprecated code -fn is_deprecated(rd: &RustDiagnostic) -> bool { - if let Some(code) = &rd.code { - match code.code.as_str() { - "deprecated" => true, - _ => false, - } - } else { - false - } -} - -#[derive(Debug)] -pub struct SuggestedFix { - pub title: String, - pub location: Location, - pub replacement: String, - pub applicability: Applicability, - pub diagnostics: Vec, -} - -impl std::cmp::PartialEq 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), - MessageLine(String), -} - -fn map_rust_child_diagnostic( - rd: &RustDiagnostic, - workspace_root: &PathBuf, -) -> MappedRustChildDiagnostic { - let span: &DiagnosticSpan = match rd.spans.iter().find(|s| s.is_primary) { - Some(span) => span, - None => { - // `rustc` uses these spanless children as a way to print multi-line - // messages - return MappedRustChildDiagnostic::MessageLine(rd.message.clone()); - } - }; - - // If we have a primary span use its location, otherwise use the parent - let location = map_span_to_location(&span, workspace_root); - - if let Some(suggested_replacement) = &span.suggested_replacement { - // Include our replacement in the title unless it's empty - let title = if !suggested_replacement.is_empty() { - format!("{}: '{}'", rd.message, suggested_replacement) - } else { - rd.message.clone() - }; - - MappedRustChildDiagnostic::SuggestedFix(SuggestedFix { - title, - location, - replacement: suggested_replacement.clone(), - applicability: span.suggestion_applicability.clone().unwrap_or(Applicability::Unknown), - diagnostics: vec![], - }) - } else { - MappedRustChildDiagnostic::Related(DiagnosticRelatedInformation { - location, - message: rd.message.clone(), - }) - } -} - -#[derive(Debug)] -struct MappedRustDiagnostic { - location: Location, - diagnostic: Diagnostic, - suggested_fixes: Vec, -} - -/// Converts a Rust root diagnostic to LSP form -/// -/// This flattens the Rust diagnostic by: -/// -/// 1. Creating a LSP diagnostic with the root message and primary span. -/// 2. Adding any labelled secondary spans to `relatedInformation` -/// 3. Categorising child diagnostics as either `SuggestedFix`es, -/// `relatedInformation` or additional message lines. -/// -/// If the diagnostic has no primary span this will return `None` -fn map_rust_diagnostic_to_lsp( - rd: &RustDiagnostic, - workspace_root: &PathBuf, -) -> Option { - let primary_span = rd.spans.iter().find(|s| s.is_primary)?; - - let location = map_span_to_location(&primary_span, workspace_root); - - let severity = map_level_to_severity(rd.level); - let mut primary_span_label = primary_span.label.as_ref(); - - let mut source = String::from("rustc"); - let mut code = rd.code.as_ref().map(|c| c.code.clone()); - if let Some(code_val) = &code { - // See if this is an RFC #2103 scoped lint (e.g. from Clippy) - let scoped_code: Vec<&str> = code_val.split("::").collect(); - if scoped_code.len() == 2 { - source = String::from(scoped_code[0]); - code = Some(String::from(scoped_code[1])); - } - } - - let mut related_information = vec![]; - let mut tags = vec![]; - - for secondary_span in rd.spans.iter().filter(|s| !s.is_primary) { - let related = map_secondary_span_to_related(secondary_span, workspace_root); - if let Some(related) = related { - related_information.push(related); - } - } - - let mut suggested_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::MessageLine(message_line) => { - write!(&mut message, "\n{}", message_line).unwrap(); - - // These secondary messages usually duplicate the content of the - // primary span label. - primary_span_label = None; - } - } - } - - if let Some(primary_span_label) = primary_span_label { - write!(&mut message, "\n{}", primary_span_label).unwrap(); - } - - if is_unused_or_unnecessary(rd) { - tags.push(DiagnosticTag::Unnecessary); - } - - if is_deprecated(rd) { - tags.push(DiagnosticTag::Deprecated); - } - - let diagnostic = Diagnostic { - range: location.range, - severity, - code: code.map(NumberOrString::String), - source: Some(source), - message, - related_information: if !related_information.is_empty() { - Some(related_information) - } else { - None - }, - tags: if !tags.is_empty() { Some(tags) } else { None }, - }; - - Some(MappedRustDiagnostic { location, diagnostic, suggested_fixes }) -} - -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 -} - -#[cfg(test)] -mod test { - use super::*; - - fn parse_diagnostic(val: &str) -> cargo_metadata::diagnostic::Diagnostic { - serde_json::from_str::(val).unwrap() - } - - #[test] - fn snap_rustc_incompatible_type_for_trait() { - let diag = parse_diagnostic( - r##"{ - "message": "method `next` has an incompatible type for trait", - "code": { - "code": "E0053", - "explanation": "\nThe parameters of any trait method must match between a trait implementation\nand the trait definition.\n\nHere are a couple examples of this error:\n\n```compile_fail,E0053\ntrait Foo {\n fn foo(x: u16);\n fn bar(&self);\n}\n\nstruct Bar;\n\nimpl Foo for Bar {\n // error, expected u16, found i16\n fn foo(x: i16) { }\n\n // error, types differ in mutability\n fn bar(&mut self) { }\n}\n```\n" - }, - "level": "error", - "spans": [ - { - "file_name": "compiler/ty/list_iter.rs", - "byte_start": 1307, - "byte_end": 1350, - "line_start": 52, - "line_end": 52, - "column_start": 5, - "column_end": 48, - "is_primary": true, - "text": [ - { - "text": " fn next(&self) -> Option<&'list ty::Ref> {", - "highlight_start": 5, - "highlight_end": 48 - } - ], - "label": "types differ in mutability", - "suggested_replacement": null, - "suggestion_applicability": null, - "expansion": null - } - ], - "children": [ - { - "message": "expected type `fn(&mut ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&ty::Ref>`\n found type `fn(&ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&'list ty::Ref>`", - "code": null, - "level": "note", - "spans": [], - "children": [], - "rendered": null - } - ], - "rendered": "error[E0053]: method `next` has an incompatible type for trait\n --> compiler/ty/list_iter.rs:52:5\n |\n52 | fn next(&self) -> Option<&'list ty::Ref> {\n | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ types differ in mutability\n |\n = note: expected type `fn(&mut ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&ty::Ref>`\n found type `fn(&ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&'list ty::Ref>`\n\n" - } - "##, - ); - - let workspace_root = PathBuf::from("/test/"); - let diag = - map_rust_diagnostic_to_lsp(&diag, &workspace_root).expect("couldn't map diagnostic"); - insta::assert_debug_snapshot!(diag); - } - - #[test] - fn snap_rustc_unused_variable() { - let diag = parse_diagnostic( - r##"{ - "message": "unused variable: `foo`", - "code": { - "code": "unused_variables", - "explanation": null - }, - "level": "warning", - "spans": [ - { - "file_name": "driver/subcommand/repl.rs", - "byte_start": 9228, - "byte_end": 9231, - "line_start": 291, - "line_end": 291, - "column_start": 9, - "column_end": 12, - "is_primary": true, - "text": [ - { - "text": " let foo = 42;", - "highlight_start": 9, - "highlight_end": 12 - } - ], - "label": null, - "suggested_replacement": null, - "suggestion_applicability": null, - "expansion": null - } - ], - "children": [ - { - "message": "#[warn(unused_variables)] on by default", - "code": null, - "level": "note", - "spans": [], - "children": [], - "rendered": null - }, - { - "message": "consider prefixing with an underscore", - "code": null, - "level": "help", - "spans": [ - { - "file_name": "driver/subcommand/repl.rs", - "byte_start": 9228, - "byte_end": 9231, - "line_start": 291, - "line_end": 291, - "column_start": 9, - "column_end": 12, - "is_primary": true, - "text": [ - { - "text": " let foo = 42;", - "highlight_start": 9, - "highlight_end": 12 - } - ], - "label": null, - "suggested_replacement": "_foo", - "suggestion_applicability": "MachineApplicable", - "expansion": null - } - ], - "children": [], - "rendered": null - } - ], - "rendered": "warning: unused variable: `foo`\n --> driver/subcommand/repl.rs:291:9\n |\n291 | let foo = 42;\n | ^^^ help: consider prefixing with an underscore: `_foo`\n |\n = note: #[warn(unused_variables)] on by default\n\n" -}"##, - ); - - let workspace_root = PathBuf::from("/test/"); - let diag = - map_rust_diagnostic_to_lsp(&diag, &workspace_root).expect("couldn't map diagnostic"); - insta::assert_debug_snapshot!(diag); - } - - #[test] - fn snap_rustc_wrong_number_of_parameters() { - let diag = parse_diagnostic( - r##"{ - "message": "this function takes 2 parameters but 3 parameters were supplied", - "code": { - "code": "E0061", - "explanation": "\nThe number of arguments passed to a function must match the number of arguments\nspecified in the function signature.\n\nFor example, a function like:\n\n```\nfn f(a: u16, b: &str) {}\n```\n\nMust always be called with exactly two arguments, e.g., `f(2, \"test\")`.\n\nNote that Rust does not have a notion of optional function arguments or\nvariadic functions (except for its C-FFI).\n" - }, - "level": "error", - "spans": [ - { - "file_name": "compiler/ty/select.rs", - "byte_start": 8787, - "byte_end": 9241, - "line_start": 219, - "line_end": 231, - "column_start": 5, - "column_end": 6, - "is_primary": false, - "text": [ - { - "text": " pub fn add_evidence(", - "highlight_start": 5, - "highlight_end": 25 - }, - { - "text": " &mut self,", - "highlight_start": 1, - "highlight_end": 19 - }, - { - "text": " target_poly: &ty::Ref,", - "highlight_start": 1, - "highlight_end": 41 - }, - { - "text": " evidence_poly: &ty::Ref,", - "highlight_start": 1, - "highlight_end": 43 - }, - { - "text": " ) {", - "highlight_start": 1, - "highlight_end": 8 - }, - { - "text": " match target_poly {", - "highlight_start": 1, - "highlight_end": 28 - }, - { - "text": " ty::Ref::Var(tvar, _) => self.add_var_evidence(tvar, evidence_poly),", - "highlight_start": 1, - "highlight_end": 81 - }, - { - "text": " ty::Ref::Fixed(target_ty) => {", - "highlight_start": 1, - "highlight_end": 43 - }, - { - "text": " let evidence_ty = evidence_poly.resolve_to_ty();", - "highlight_start": 1, - "highlight_end": 65 - }, - { - "text": " self.add_evidence_ty(target_ty, evidence_poly, evidence_ty)", - "highlight_start": 1, - "highlight_end": 76 - }, - { - "text": " }", - "highlight_start": 1, - "highlight_end": 14 - }, - { - "text": " }", - "highlight_start": 1, - "highlight_end": 10 - }, - { - "text": " }", - "highlight_start": 1, - "highlight_end": 6 - } - ], - "label": "defined here", - "suggested_replacement": null, - "suggestion_applicability": null, - "expansion": null - }, - { - "file_name": "compiler/ty/select.rs", - "byte_start": 4045, - "byte_end": 4057, - "line_start": 104, - "line_end": 104, - "column_start": 18, - "column_end": 30, - "is_primary": true, - "text": [ - { - "text": " self.add_evidence(target_fixed, evidence_fixed, false);", - "highlight_start": 18, - "highlight_end": 30 - } - ], - "label": "expected 2 parameters", - "suggested_replacement": null, - "suggestion_applicability": null, - "expansion": null - } - ], - "children": [], - "rendered": "error[E0061]: this function takes 2 parameters but 3 parameters were supplied\n --> compiler/ty/select.rs:104:18\n |\n104 | self.add_evidence(target_fixed, evidence_fixed, false);\n | ^^^^^^^^^^^^ expected 2 parameters\n...\n219 | / pub fn add_evidence(\n220 | | &mut self,\n221 | | target_poly: &ty::Ref,\n222 | | evidence_poly: &ty::Ref,\n... |\n230 | | }\n231 | | }\n | |_____- defined here\n\n" -}"##, - ); - - let workspace_root = PathBuf::from("/test/"); - let diag = - map_rust_diagnostic_to_lsp(&diag, &workspace_root).expect("couldn't map diagnostic"); - insta::assert_debug_snapshot!(diag); - } - - #[test] - fn snap_clippy_pass_by_ref() { - let diag = parse_diagnostic( - r##"{ - "message": "this argument is passed by reference, but would be more efficient if passed by value", - "code": { - "code": "clippy::trivially_copy_pass_by_ref", - "explanation": null - }, - "level": "warning", - "spans": [ - { - "file_name": "compiler/mir/tagset.rs", - "byte_start": 941, - "byte_end": 946, - "line_start": 42, - "line_end": 42, - "column_start": 24, - "column_end": 29, - "is_primary": true, - "text": [ - { - "text": " pub fn is_disjoint(&self, other: Self) -> bool {", - "highlight_start": 24, - "highlight_end": 29 - } - ], - "label": null, - "suggested_replacement": null, - "suggestion_applicability": null, - "expansion": null - } - ], - "children": [ - { - "message": "lint level defined here", - "code": null, - "level": "note", - "spans": [ - { - "file_name": "compiler/lib.rs", - "byte_start": 8, - "byte_end": 19, - "line_start": 1, - "line_end": 1, - "column_start": 9, - "column_end": 20, - "is_primary": true, - "text": [ - { - "text": "#![warn(clippy::all)]", - "highlight_start": 9, - "highlight_end": 20 - } - ], - "label": null, - "suggested_replacement": null, - "suggestion_applicability": null, - "expansion": null - } - ], - "children": [], - "rendered": null - }, - { - "message": "#[warn(clippy::trivially_copy_pass_by_ref)] implied by #[warn(clippy::all)]", - "code": null, - "level": "note", - "spans": [], - "children": [], - "rendered": null - }, - { - "message": "for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref", - "code": null, - "level": "help", - "spans": [], - "children": [], - "rendered": null - }, - { - "message": "consider passing by value instead", - "code": null, - "level": "help", - "spans": [ - { - "file_name": "compiler/mir/tagset.rs", - "byte_start": 941, - "byte_end": 946, - "line_start": 42, - "line_end": 42, - "column_start": 24, - "column_end": 29, - "is_primary": true, - "text": [ - { - "text": " pub fn is_disjoint(&self, other: Self) -> bool {", - "highlight_start": 24, - "highlight_end": 29 - } - ], - "label": null, - "suggested_replacement": "self", - "suggestion_applicability": "Unspecified", - "expansion": null - } - ], - "children": [], - "rendered": null - } - ], - "rendered": "warning: this argument is passed by reference, but would be more efficient if passed by value\n --> compiler/mir/tagset.rs:42:24\n |\n42 | pub fn is_disjoint(&self, other: Self) -> bool {\n | ^^^^^ help: consider passing by value instead: `self`\n |\nnote: lint level defined here\n --> compiler/lib.rs:1:9\n |\n1 | #![warn(clippy::all)]\n | ^^^^^^^^^^^\n = note: #[warn(clippy::trivially_copy_pass_by_ref)] implied by #[warn(clippy::all)]\n = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref\n\n" -}"##, - ); - - let workspace_root = PathBuf::from("/test/"); - let diag = - map_rust_diagnostic_to_lsp(&diag, &workspace_root).expect("couldn't map diagnostic"); - insta::assert_debug_snapshot!(diag); - } - - #[test] - fn snap_rustc_mismatched_type() { - let diag = parse_diagnostic( - r##"{ - "message": "mismatched types", - "code": { - "code": "E0308", - "explanation": "\nThis error occurs when the compiler was unable to infer the concrete type of a\nvariable. It can occur for several cases, the most common of which is a\nmismatch in the expected type that the compiler inferred for a variable's\ninitializing expression, and the actual type explicitly assigned to the\nvariable.\n\nFor example:\n\n```compile_fail,E0308\nlet x: i32 = \"I am not a number!\";\n// ~~~ ~~~~~~~~~~~~~~~~~~~~\n// | |\n// | initializing expression;\n// | compiler infers type `&str`\n// |\n// type `i32` assigned to variable `x`\n```\n" - }, - "level": "error", - "spans": [ - { - "file_name": "runtime/compiler_support.rs", - "byte_start": 1589, - "byte_end": 1594, - "line_start": 48, - "line_end": 48, - "column_start": 65, - "column_end": 70, - "is_primary": true, - "text": [ - { - "text": " let layout = alloc::Layout::from_size_align_unchecked(size, align);", - "highlight_start": 65, - "highlight_end": 70 - } - ], - "label": "expected usize, found u32", - "suggested_replacement": null, - "suggestion_applicability": null, - "expansion": null - } - ], - "children": [], - "rendered": "error[E0308]: mismatched types\n --> runtime/compiler_support.rs:48:65\n |\n48 | let layout = alloc::Layout::from_size_align_unchecked(size, align);\n | ^^^^^ expected usize, found u32\n\n" -}"##, - ); - - let workspace_root = PathBuf::from("/test/"); - let diag = - map_rust_diagnostic_to_lsp(&diag, &workspace_root).expect("couldn't map diagnostic"); - insta::assert_debug_snapshot!(diag); - } - - #[test] - fn snap_handles_macro_location() { - let diag = parse_diagnostic( - r##"{ - "rendered": "error[E0277]: can't compare `{integer}` with `&str`\n --> src/main.rs:2:5\n |\n2 | assert_eq!(1, \"love\");\n | ^^^^^^^^^^^^^^^^^^^^^^ no implementation for `{integer} == &str`\n |\n = help: the trait `std::cmp::PartialEq<&str>` is not implemented for `{integer}`\n = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)\n\n", - "children": [ - { - "children": [], - "code": null, - "level": "help", - "message": "the trait `std::cmp::PartialEq<&str>` is not implemented for `{integer}`", - "rendered": null, - "spans": [] - } - ], - "code": { - "code": "E0277", - "explanation": "\nYou tried to use a type which doesn't implement some trait in a place which\nexpected that trait. Erroneous code example:\n\n```compile_fail,E0277\n// here we declare the Foo trait with a bar method\ntrait Foo {\n fn bar(&self);\n}\n\n// we now declare a function which takes an object implementing the Foo trait\nfn some_func(foo: T) {\n foo.bar();\n}\n\nfn main() {\n // we now call the method with the i32 type, which doesn't implement\n // the Foo trait\n some_func(5i32); // error: the trait bound `i32 : Foo` is not satisfied\n}\n```\n\nIn order to fix this error, verify that the type you're using does implement\nthe trait. Example:\n\n```\ntrait Foo {\n fn bar(&self);\n}\n\nfn some_func(foo: T) {\n foo.bar(); // we can now use this method since i32 implements the\n // Foo trait\n}\n\n// we implement the trait on the i32 type\nimpl Foo for i32 {\n fn bar(&self) {}\n}\n\nfn main() {\n some_func(5i32); // ok!\n}\n```\n\nOr in a generic context, an erroneous code example would look like:\n\n```compile_fail,E0277\nfn some_func(foo: T) {\n println!(\"{:?}\", foo); // error: the trait `core::fmt::Debug` is not\n // implemented for the type `T`\n}\n\nfn main() {\n // We now call the method with the i32 type,\n // which *does* implement the Debug trait.\n some_func(5i32);\n}\n```\n\nNote that the error here is in the definition of the generic function: Although\nwe only call it with a parameter that does implement `Debug`, the compiler\nstill rejects the function: It must work with all possible input types. In\norder to make this example compile, we need to restrict the generic type we're\naccepting:\n\n```\nuse std::fmt;\n\n// Restrict the input type to types that implement Debug.\nfn some_func(foo: T) {\n println!(\"{:?}\", foo);\n}\n\nfn main() {\n // Calling the method is still fine, as i32 implements Debug.\n some_func(5i32);\n\n // This would fail to compile now:\n // struct WithoutDebug;\n // some_func(WithoutDebug);\n}\n```\n\nRust only looks at the signature of the called function, as such it must\nalready specify all requirements that will be used for every type parameter.\n" - }, - "level": "error", - "message": "can't compare `{integer}` with `&str`", - "spans": [ - { - "byte_end": 155, - "byte_start": 153, - "column_end": 33, - "column_start": 31, - "expansion": { - "def_site_span": { - "byte_end": 940, - "byte_start": 0, - "column_end": 6, - "column_start": 1, - "expansion": null, - "file_name": "<::core::macros::assert_eq macros>", - "is_primary": false, - "label": null, - "line_end": 36, - "line_start": 1, - "suggested_replacement": null, - "suggestion_applicability": null, - "text": [ - { - "highlight_end": 35, - "highlight_start": 1, - "text": "($ left : expr, $ right : expr) =>" - }, - { - "highlight_end": 3, - "highlight_start": 1, - "text": "({" - }, - { - "highlight_end": 33, - "highlight_start": 1, - "text": " match (& $ left, & $ right)" - }, - { - "highlight_end": 7, - "highlight_start": 1, - "text": " {" - }, - { - "highlight_end": 34, - "highlight_start": 1, - "text": " (left_val, right_val) =>" - }, - { - "highlight_end": 11, - "highlight_start": 1, - "text": " {" - }, - { - "highlight_end": 46, - "highlight_start": 1, - "text": " if ! (* left_val == * right_val)" - }, - { - "highlight_end": 15, - "highlight_start": 1, - "text": " {" - }, - { - "highlight_end": 25, - "highlight_start": 1, - "text": " panic !" - }, - { - "highlight_end": 57, - "highlight_start": 1, - "text": " (r#\"assertion failed: `(left == right)`" - }, - { - "highlight_end": 16, - "highlight_start": 1, - "text": " left: `{:?}`," - }, - { - "highlight_end": 18, - "highlight_start": 1, - "text": " right: `{:?}`\"#," - }, - { - "highlight_end": 47, - "highlight_start": 1, - "text": " & * left_val, & * right_val)" - }, - { - "highlight_end": 15, - "highlight_start": 1, - "text": " }" - }, - { - "highlight_end": 11, - "highlight_start": 1, - "text": " }" - }, - { - "highlight_end": 7, - "highlight_start": 1, - "text": " }" - }, - { - "highlight_end": 42, - "highlight_start": 1, - "text": " }) ; ($ left : expr, $ right : expr,) =>" - }, - { - "highlight_end": 49, - "highlight_start": 1, - "text": "({ $ crate :: assert_eq ! ($ left, $ right) }) ;" - }, - { - "highlight_end": 53, - "highlight_start": 1, - "text": "($ left : expr, $ right : expr, $ ($ arg : tt) +) =>" - }, - { - "highlight_end": 3, - "highlight_start": 1, - "text": "({" - }, - { - "highlight_end": 37, - "highlight_start": 1, - "text": " match (& ($ left), & ($ right))" - }, - { - "highlight_end": 7, - "highlight_start": 1, - "text": " {" - }, - { - "highlight_end": 34, - "highlight_start": 1, - "text": " (left_val, right_val) =>" - }, - { - "highlight_end": 11, - "highlight_start": 1, - "text": " {" - }, - { - "highlight_end": 46, - "highlight_start": 1, - "text": " if ! (* left_val == * right_val)" - }, - { - "highlight_end": 15, - "highlight_start": 1, - "text": " {" - }, - { - "highlight_end": 25, - "highlight_start": 1, - "text": " panic !" - }, - { - "highlight_end": 57, - "highlight_start": 1, - "text": " (r#\"assertion failed: `(left == right)`" - }, - { - "highlight_end": 16, - "highlight_start": 1, - "text": " left: `{:?}`," - }, - { - "highlight_end": 22, - "highlight_start": 1, - "text": " right: `{:?}`: {}\"#," - }, - { - "highlight_end": 72, - "highlight_start": 1, - "text": " & * left_val, & * right_val, $ crate :: format_args !" - }, - { - "highlight_end": 33, - "highlight_start": 1, - "text": " ($ ($ arg) +))" - }, - { - "highlight_end": 15, - "highlight_start": 1, - "text": " }" - }, - { - "highlight_end": 11, - "highlight_start": 1, - "text": " }" - }, - { - "highlight_end": 7, - "highlight_start": 1, - "text": " }" - }, - { - "highlight_end": 6, - "highlight_start": 1, - "text": " }) ;" - } - ] - }, - "macro_decl_name": "assert_eq!", - "span": { - "byte_end": 38, - "byte_start": 16, - "column_end": 27, - "column_start": 5, - "expansion": null, - "file_name": "src/main.rs", - "is_primary": false, - "label": null, - "line_end": 2, - "line_start": 2, - "suggested_replacement": null, - "suggestion_applicability": null, - "text": [ - { - "highlight_end": 27, - "highlight_start": 5, - "text": " assert_eq!(1, \"love\");" - } - ] - } - }, - "file_name": "<::core::macros::assert_eq macros>", - "is_primary": true, - "label": "no implementation for `{integer} == &str`", - "line_end": 7, - "line_start": 7, - "suggested_replacement": null, - "suggestion_applicability": null, - "text": [ - { - "highlight_end": 33, - "highlight_start": 31, - "text": " if ! (* left_val == * right_val)" - } - ] - } - ] -}"##, - ); - - let workspace_root = PathBuf::from("/test/"); - let diag = - map_rust_diagnostic_to_lsp(&diag, &workspace_root).expect("couldn't map diagnostic"); - insta::assert_debug_snapshot!(diag); - } -} diff --git a/crates/ra_lsp_server/src/lib.rs b/crates/ra_lsp_server/src/lib.rs index 2811231fac..2ca149fd56 100644 --- a/crates/ra_lsp_server/src/lib.rs +++ b/crates/ra_lsp_server/src/lib.rs @@ -22,7 +22,6 @@ macro_rules! print { } mod caps; -mod cargo_check; mod cargo_target_spec; mod conv; mod main_loop; diff --git a/crates/ra_lsp_server/src/main_loop.rs b/crates/ra_lsp_server/src/main_loop.rs index c58af7e473..e66b8f9eb2 100644 --- a/crates/ra_lsp_server/src/main_loop.rs +++ b/crates/ra_lsp_server/src/main_loop.rs @@ -10,6 +10,7 @@ use std::{error::Error, fmt, panic, path::PathBuf, sync::Arc, time::Instant}; 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_ide::{Canceled, FeatureFlags, FileId, LibraryData, SourceRootId}; use ra_prof::profile; use ra_vfs::{VfsTask, Watch}; @@ -19,7 +20,6 @@ use serde::{de::DeserializeOwned, Serialize}; use threadpool::ThreadPool; use crate::{ - cargo_check::CheckTask, main_loop::{ pending_requests::{PendingRequest, PendingRequests}, subscriptions::Subscriptions, @@ -127,10 +127,12 @@ pub fn main_loop( .and_then(|it| it.line_folding_only) .unwrap_or(false), max_inlay_hint_length: config.max_inlay_hint_length, - cargo_watch_enable: config.cargo_watch_enable, - cargo_watch_args: config.cargo_watch_args, - cargo_watch_command: config.cargo_watch_command, - cargo_watch_all_targets: config.cargo_watch_all_targets, + cargo_watch: CheckOptions { + enable: config.cargo_watch_enable, + args: config.cargo_watch_args, + command: config.cargo_watch_command, + all_targets: config.cargo_watch_all_targets, + }, } }; diff --git a/crates/ra_lsp_server/src/world.rs b/crates/ra_lsp_server/src/world.rs index 39a07c01ac..4b3959e38a 100644 --- a/crates/ra_lsp_server/src/world.rs +++ b/crates/ra_lsp_server/src/world.rs @@ -12,6 +12,7 @@ use crossbeam_channel::{unbounded, Receiver}; use lsp_server::ErrorCode; use lsp_types::Url; use parking_lot::RwLock; +use ra_cargo_watch::{CheckOptions, CheckWatcher, CheckWatcherSharedState}; use ra_ide::{ Analysis, AnalysisChange, AnalysisHost, CrateGraph, FeatureFlags, FileId, LibraryData, SourceRootId, @@ -23,7 +24,6 @@ use relative_path::RelativePathBuf; use std::path::{Component, Prefix}; use crate::{ - cargo_check::{CheckWatcher, CheckWatcherSharedState}, main_loop::pending_requests::{CompletedRequest, LatestRequests}, LspError, Result, }; @@ -35,10 +35,7 @@ pub struct Options { pub supports_location_link: bool, pub line_folding_only: bool, pub max_inlay_hint_length: Option, - pub cargo_watch_enable: bool, - pub cargo_watch_args: Vec, - pub cargo_watch_command: String, - pub cargo_watch_all_targets: bool, + pub cargo_watch: CheckOptions, } /// `WorldState` is the primary mutable state of the language server @@ -135,7 +132,8 @@ impl WorldState { change.set_crate_graph(crate_graph); // FIXME: Figure out the multi-workspace situation - let check_watcher = CheckWatcher::new(&options, folder_roots.first().cloned().unwrap()); + let check_watcher = + CheckWatcher::new(&options.cargo_watch, folder_roots.first().cloned().unwrap()); let mut analysis_host = AnalysisHost::new(lru_capacity, feature_flags); analysis_host.apply_change(change); From a2d10694ccfcd12dad8796fc86966ea10ca3fc01 Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Fri, 27 Dec 2019 11:31:25 +0100 Subject: [PATCH 14/20] Consistent, hopefully robust, shutdown/cancelation story for cargo check subprocess --- crates/ra_cargo_watch/src/lib.rs | 70 ++++++++++++++++++++++++-------- 1 file changed, 52 insertions(+), 18 deletions(-) diff --git a/crates/ra_cargo_watch/src/lib.rs b/crates/ra_cargo_watch/src/lib.rs index c863866108..70afd7f8ac 100644 --- a/crates/ra_cargo_watch/src/lib.rs +++ b/crates/ra_cargo_watch/src/lib.rs @@ -2,7 +2,7 @@ //! another compatible command (f.x. clippy) in a background thread and provide //! LSP diagnostics based on the output of the command. use cargo_metadata::Message; -use crossbeam_channel::{select, unbounded, Receiver, RecvError, Sender, TryRecvError}; +use crossbeam_channel::{select, unbounded, Receiver, RecvError, Sender}; use lsp_types::{ Diagnostic, Url, WorkDoneProgress, WorkDoneProgressBegin, WorkDoneProgressEnd, WorkDoneProgressReport, @@ -191,7 +191,8 @@ impl CheckWatcherState { self.last_update_req.take(); self.shared.write().clear(task_send); - self.watcher.cancel(); + // By replacing the watcher, we drop the previous one which + // causes it to shut down automatically. self.watcher = WatchThread::new(&self.options, &self.workspace_root); } } @@ -277,9 +278,11 @@ impl CheckWatcherState { /// doesn't provide a way to read sub-process output without blocking, so we /// have to wrap sub-processes output handling in a thread and pass messages /// back over a channel. +/// The correct way to dispose of the thread is to drop it, on which the +/// sub-process will be killed, and the thread will be joined. struct WatchThread { + handle: Option>, message_recv: Receiver, - cancel_send: Sender<()>, } enum CheckEvent { @@ -302,9 +305,8 @@ impl WatchThread { args.extend(options.args.iter().cloned()); let (message_send, message_recv) = unbounded(); - let (cancel_send, cancel_recv) = unbounded(); let enabled = options.enable; - std::thread::spawn(move || { + let handle = std::thread::spawn(move || { if !enabled { return; } @@ -316,24 +318,56 @@ impl WatchThread { .spawn() .expect("couldn't launch cargo"); - message_send.send(CheckEvent::Begin).unwrap(); + // If we trigger an error here, we will do so in the loop instead, + // which will break out of the loop, and continue the shutdown + let _ = message_send.send(CheckEvent::Begin); + for message in cargo_metadata::parse_messages(command.stdout.take().unwrap()) { - match cancel_recv.try_recv() { - Ok(()) | Err(TryRecvError::Disconnected) => { - command.kill().expect("couldn't kill command"); + let message = match message { + Ok(message) => message, + Err(err) => { + log::error!("Invalid json from cargo check, ignoring: {}", err); + continue; + } + }; + + match message_send.send(CheckEvent::Msg(message)) { + Ok(()) => {} + Err(_err) => { + // The send channel was closed, so we want to shutdown + break; } - Err(TryRecvError::Empty) => (), } - - message_send.send(CheckEvent::Msg(message.unwrap())).unwrap(); } - message_send.send(CheckEvent::End).unwrap(); - }); - WatchThread { message_recv, cancel_send } - } - fn cancel(&self) { - let _ = self.cancel_send.send(()); + // We can ignore any error here, as we are already in the progress + // of shutting down. + let _ = message_send.send(CheckEvent::End); + + // It is okay to ignore the result, as it only errors if the process is already dead + let _ = command.kill(); + + // Again, we don't care about the exit status so just ignore the result + let _ = command.wait(); + }); + WatchThread { handle: Some(handle), message_recv } + } +} + +impl std::ops::Drop for WatchThread { + fn drop(&mut self) { + if let Some(handle) = self.handle.take() { + // Replace our reciever with dummy one, so we can drop and close the + // one actually communicating with the thread + let recv = std::mem::replace(&mut self.message_recv, crossbeam_channel::never()); + + // Dropping the original reciever initiates thread sub-process shutdown + drop(recv); + + // Join the thread, it should finish shortly. We don't really care + // whether it panicked, so it is safe to ignore the result + let _ = handle.join(); + } } } From 02ce5bbf6b46a67c3fc12b964de204b8130f1b10 Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Fri, 27 Dec 2019 11:43:05 +0100 Subject: [PATCH 15/20] Shutdown/cancelation story for main cargo watch thread --- crates/ra_cargo_watch/src/lib.rs | 47 ++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/crates/ra_cargo_watch/src/lib.rs b/crates/ra_cargo_watch/src/lib.rs index 70afd7f8ac..4af26ff8c6 100644 --- a/crates/ra_cargo_watch/src/lib.rs +++ b/crates/ra_cargo_watch/src/lib.rs @@ -32,12 +32,13 @@ pub struct CheckOptions { /// CheckWatcher wraps the shared state and communication machinery used for /// running `cargo check` (or other compatible command) and providing /// diagnostics based on the output. +/// The spawned thread is shut down when this struct is dropped. #[derive(Debug)] pub struct CheckWatcher { pub task_recv: Receiver, pub cmd_send: Sender, pub shared: Arc>, - handle: JoinHandle<()>, + handle: Option>, } impl CheckWatcher { @@ -52,8 +53,7 @@ impl CheckWatcher { let mut check = CheckWatcherState::new(options, workspace_root, shared_); check.run(&task_send, &cmd_recv); }); - - CheckWatcher { task_recv, cmd_send, handle, shared } + CheckWatcher { task_recv, cmd_send, handle: Some(handle), shared } } /// Schedule a re-start of the cargo check worker. @@ -62,13 +62,21 @@ impl CheckWatcher { } } -pub struct CheckWatcherState { - options: CheckOptions, - workspace_root: PathBuf, - running: bool, - watcher: WatchThread, - last_update_req: Option, - shared: Arc>, +impl std::ops::Drop for CheckWatcher { + fn drop(&mut self) { + if let Some(handle) = self.handle.take() { + // Replace our reciever with dummy one, so we can drop and close the + // one actually communicating with the thread + let recv = std::mem::replace(&mut self.task_recv, crossbeam_channel::never()); + + // Dropping the original reciever finishes the thread loop + drop(recv); + + // Join the thread, it should finish shortly. We don't really care + // whether it panicked, so it is safe to ignore the result + let _ = handle.join(); + } + } } #[derive(Debug)] @@ -153,6 +161,14 @@ pub enum CheckCommand { Update, } +struct CheckWatcherState { + options: CheckOptions, + workspace_root: PathBuf, + watcher: WatchThread, + last_update_req: Option, + shared: Arc>, +} + impl CheckWatcherState { pub fn new( options: CheckOptions, @@ -163,7 +179,6 @@ impl CheckWatcherState { CheckWatcherState { options, workspace_root, - running: false, watcher, last_update_req: None, shared, @@ -171,19 +186,21 @@ impl CheckWatcherState { } pub fn run(&mut self, task_send: &Sender, cmd_recv: &Receiver) { - self.running = true; - while self.running { + loop { select! { recv(&cmd_recv) -> cmd => match cmd { Ok(cmd) => self.handle_command(cmd), Err(RecvError) => { // Command channel has closed, so shut down - self.running = false; + break; }, }, recv(self.watcher.message_recv) -> msg => match msg { Ok(msg) => self.handle_message(msg, task_send), - Err(RecvError) => {}, + Err(RecvError) => { + // Task channel has closed, so shut down + break; + }, } }; From 4d33835a34b4ccdfc4ea34b8f3f8ff61d092a543 Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Fri, 27 Dec 2019 11:47:09 +0100 Subject: [PATCH 16/20] Cargo fmt run --- crates/ra_cargo_watch/src/lib.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/crates/ra_cargo_watch/src/lib.rs b/crates/ra_cargo_watch/src/lib.rs index 4af26ff8c6..d17edd7750 100644 --- a/crates/ra_cargo_watch/src/lib.rs +++ b/crates/ra_cargo_watch/src/lib.rs @@ -176,13 +176,7 @@ impl CheckWatcherState { shared: Arc>, ) -> CheckWatcherState { let watcher = WatchThread::new(&options, &workspace_root); - CheckWatcherState { - options, - workspace_root, - watcher, - last_update_req: None, - shared, - } + CheckWatcherState { options, workspace_root, watcher, last_update_req: None, shared } } pub fn run(&mut self, task_send: &Sender, cmd_recv: &Receiver) { From 59837c75f44f4b1b6a09e8db7e59a7bbd78d074a Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Fri, 27 Dec 2019 11:57:00 +0100 Subject: [PATCH 17/20] Add doc comment to module --- crates/ra_cargo_watch/src/conv/test.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/ra_cargo_watch/src/conv/test.rs b/crates/ra_cargo_watch/src/conv/test.rs index 69a07be11c..6817245c2b 100644 --- a/crates/ra_cargo_watch/src/conv/test.rs +++ b/crates/ra_cargo_watch/src/conv/test.rs @@ -1,3 +1,5 @@ +//! This module contains the large and verbose snapshot tests for the +//! conversions between `cargo check` json and LSP diagnostics. use crate::*; fn parse_diagnostic(val: &str) -> cargo_metadata::diagnostic::Diagnostic { From ed84c85aef859e97ab355e78bf77f435689f25b7 Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Fri, 27 Dec 2019 12:42:18 +0100 Subject: [PATCH 18/20] Fix shutdown behavoir of main cargo-watch thread. Even though this didn't error, it became clear to me that it was closing the wrong channel, resulting in the child thread never finishing. --- crates/ra_cargo_watch/src/lib.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/crates/ra_cargo_watch/src/lib.rs b/crates/ra_cargo_watch/src/lib.rs index d17edd7750..11b624abfe 100644 --- a/crates/ra_cargo_watch/src/lib.rs +++ b/crates/ra_cargo_watch/src/lib.rs @@ -36,7 +36,7 @@ pub struct CheckOptions { #[derive(Debug)] pub struct CheckWatcher { pub task_recv: Receiver, - pub cmd_send: Sender, + pub cmd_send: Option>, pub shared: Arc>, handle: Option>, } @@ -53,23 +53,24 @@ impl CheckWatcher { let mut check = CheckWatcherState::new(options, workspace_root, shared_); check.run(&task_send, &cmd_recv); }); - CheckWatcher { task_recv, cmd_send, handle: Some(handle), shared } + CheckWatcher { task_recv, cmd_send: Some(cmd_send), handle: Some(handle), shared } } /// Schedule a re-start of the cargo check worker. pub fn update(&self) { - self.cmd_send.send(CheckCommand::Update).unwrap(); + if let Some(cmd_send) = &self.cmd_send { + cmd_send.send(CheckCommand::Update).unwrap(); + } } } impl std::ops::Drop for CheckWatcher { fn drop(&mut self) { if let Some(handle) = self.handle.take() { - // Replace our reciever with dummy one, so we can drop and close the - // one actually communicating with the thread - let recv = std::mem::replace(&mut self.task_recv, crossbeam_channel::never()); + // Take the sender out of the option + let recv = self.cmd_send.take(); - // Dropping the original reciever finishes the thread loop + // Dropping the sender finishes the thread loop drop(recv); // Join the thread, it should finish shortly. We don't really care From c732f215cb31e9f022090b8d0212f6ea9c134c11 Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Fri, 27 Dec 2019 12:43:14 +0100 Subject: [PATCH 19/20] Don't finish main cargo watch thread when subprocess finishes. --- crates/ra_cargo_watch/src/lib.rs | 3 +-- crates/ra_lsp_server/src/main_loop.rs | 5 ++++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/ra_cargo_watch/src/lib.rs b/crates/ra_cargo_watch/src/lib.rs index 11b624abfe..d683d43d20 100644 --- a/crates/ra_cargo_watch/src/lib.rs +++ b/crates/ra_cargo_watch/src/lib.rs @@ -193,8 +193,7 @@ impl CheckWatcherState { recv(self.watcher.message_recv) -> msg => match msg { Ok(msg) => self.handle_message(msg, task_send), Err(RecvError) => { - // Task channel has closed, so shut down - break; + // Watcher finished, do nothing. }, } }; diff --git a/crates/ra_lsp_server/src/main_loop.rs b/crates/ra_lsp_server/src/main_loop.rs index e66b8f9eb2..af1a487ded 100644 --- a/crates/ra_lsp_server/src/main_loop.rs +++ b/crates/ra_lsp_server/src/main_loop.rs @@ -184,7 +184,10 @@ pub fn main_loop( Err(RecvError) => Err("vfs died")?, }, recv(libdata_receiver) -> data => Event::Lib(data.unwrap()), - recv(world_state.check_watcher.task_recv) -> task => Event::CheckWatcher(task.unwrap()) + recv(world_state.check_watcher.task_recv) -> task => match task { + Ok(task) => Event::CheckWatcher(task), + Err(RecvError) => Err("check watcher died")?, + } }; if let Event::Msg(Message::Request(req)) = &event { if connection.handle_shutdown(&req)? { From 899dbebd02b41b12d89c9f485e85208b39b81932 Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Fri, 27 Dec 2019 17:29:02 +0100 Subject: [PATCH 20/20] Fix busy-waiting issue in main cargo watch thread --- crates/ra_cargo_watch/src/lib.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/ra_cargo_watch/src/lib.rs b/crates/ra_cargo_watch/src/lib.rs index d683d43d20..78250f9107 100644 --- a/crates/ra_cargo_watch/src/lib.rs +++ b/crates/ra_cargo_watch/src/lib.rs @@ -2,7 +2,7 @@ //! another compatible command (f.x. clippy) in a background thread and provide //! LSP diagnostics based on the output of the command. use cargo_metadata::Message; -use crossbeam_channel::{select, unbounded, Receiver, RecvError, Sender}; +use crossbeam_channel::{never, select, unbounded, Receiver, RecvError, Sender}; use lsp_types::{ Diagnostic, Url, WorkDoneProgress, WorkDoneProgressBegin, WorkDoneProgressEnd, WorkDoneProgressReport, @@ -193,7 +193,9 @@ impl CheckWatcherState { recv(self.watcher.message_recv) -> msg => match msg { Ok(msg) => self.handle_message(msg, task_send), Err(RecvError) => { - // Watcher finished, do nothing. + // Watcher finished, replace it with a never channel to + // avoid busy-waiting. + std::mem::replace(&mut self.watcher.message_recv, never()); }, } }; @@ -370,7 +372,7 @@ impl std::ops::Drop for WatchThread { if let Some(handle) = self.handle.take() { // Replace our reciever with dummy one, so we can drop and close the // one actually communicating with the thread - let recv = std::mem::replace(&mut self.message_recv, crossbeam_channel::never()); + let recv = std::mem::replace(&mut self.message_recv, never()); // Dropping the original reciever initiates thread sub-process shutdown drop(recv);