From d14c9be321a5a1bb27aeaff1e4bd8528dca114db Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 11 Oct 2021 14:09:20 +0200 Subject: [PATCH] Show cargo check failures to the user --- crates/flycheck/src/lib.rs | 126 ++++++++++++-------------- crates/rust-analyzer/src/main_loop.rs | 5 +- 2 files changed, 61 insertions(+), 70 deletions(-) diff --git a/crates/flycheck/src/lib.rs b/crates/flycheck/src/lib.rs index 0661d776f1..7256f07688 100644 --- a/crates/flycheck/src/lib.rs +++ b/crates/flycheck/src/lib.rs @@ -2,17 +2,12 @@ //! another compatible command (f.x. clippy) in a background thread and provide //! LSP diagnostics based on the output of the command. -use std::{ - fmt, - io::{self, BufRead, BufReader}, - process::{self, Command, Stdio}, - time::Duration, -}; +use std::{fmt, io, process::Command, time::Duration}; use crossbeam_channel::{never, select, unbounded, Receiver, Sender}; use paths::AbsPathBuf; use serde::Deserialize; -use stdx::JodChild; +use stdx::process::streaming_output; pub use cargo_metadata::diagnostic::{ Applicability, Diagnostic, DiagnosticCode, DiagnosticLevel, DiagnosticSpan, @@ -162,13 +157,10 @@ impl FlycheckActor { self.cancel_check_process(); - let mut command = self.check_command(); + let command = self.check_command(); tracing::info!("restart flycheck {:?}", command); - command.stdout(Stdio::piped()).stderr(Stdio::null()).stdin(Stdio::null()); - if let Ok(child) = command.spawn().map(JodChild) { - self.cargo_handle = Some(CargoHandle::spawn(child)); - self.progress(Progress::DidStart); - } + self.cargo_handle = Some(CargoHandle::spawn(command)); + self.progress(Progress::DidStart); } Event::CheckEvent(None) => { // Watcher finished, replace it with a never channel to @@ -258,53 +250,37 @@ impl FlycheckActor { } struct CargoHandle { - child: JodChild, #[allow(unused)] - thread: jod_thread::JoinHandle, + thread: jod_thread::JoinHandle>, receiver: Receiver, } impl CargoHandle { - fn spawn(mut child: JodChild) -> CargoHandle { - let child_stdout = child.stdout.take().unwrap(); + fn spawn(command: Command) -> CargoHandle { let (sender, receiver) = unbounded(); - let actor = CargoActor::new(child_stdout, sender); + let actor = CargoActor::new(sender); let thread = jod_thread::Builder::new() .name("CargoHandle".to_owned()) - .spawn(move || actor.run()) + .spawn(move || actor.run(command)) .expect("failed to spawn thread"); - CargoHandle { child, thread, receiver } + CargoHandle { thread, receiver } } - fn join(mut self) -> io::Result<()> { - // It is okay to ignore the result, as it only errors if the process is already dead - let _ = self.child.kill(); - let exit_status = self.child.wait()?; - let read_at_least_one_message = self.thread.join(); - if !exit_status.success() && !read_at_least_one_message { - // FIXME: Read the stderr to display the reason, see `read2()` reference in PR comment: - // https://github.com/rust-analyzer/rust-analyzer/pull/3632#discussion_r395605298 - return Err(io::Error::new( - io::ErrorKind::Other, - format!( - "Cargo watcher failed, the command produced no valid metadata (exit code: {:?})", - exit_status - ), - )); - } - Ok(()) + + fn join(self) -> io::Result<()> { + self.thread.join() } } struct CargoActor { - child_stdout: process::ChildStdout, sender: Sender, } impl CargoActor { - fn new(child_stdout: process::ChildStdout, sender: Sender) -> CargoActor { - CargoActor { child_stdout, sender } + fn new(sender: Sender) -> CargoActor { + CargoActor { sender } } - fn run(self) -> bool { + + fn run(self, command: Command) -> io::Result<()> { // We manually read a line at a time, instead of using serde's // stream deserializers, because the deserializer cannot recover // from an error, resulting in it getting stuck, because we try to @@ -313,41 +289,53 @@ impl CargoActor { // Because cargo only outputs one JSON object per line, we can // simply skip a line if it doesn't parse, which just ignores any // erroneus output. - let stdout = BufReader::new(self.child_stdout); + + let mut error = String::new(); let mut read_at_least_one_message = false; - for message in stdout.lines() { - let message = match message { - Ok(message) => message, - Err(err) => { - tracing::error!("Invalid json from cargo check, ignoring ({})", err); - continue; - } - }; + let output = streaming_output( + command, + &mut |line| { + read_at_least_one_message = true; - read_at_least_one_message = true; - - // Try to deserialize a message from Cargo or Rustc. - let mut deserializer = serde_json::Deserializer::from_str(&message); - deserializer.disable_recursion_limit(); - if let Ok(message) = JsonMessage::deserialize(&mut deserializer) { - match message { - // Skip certain kinds of messages to only spend time on what's useful - JsonMessage::Cargo(message) => match message { - cargo_metadata::Message::CompilerArtifact(artifact) if !artifact.fresh => { - self.sender.send(CargoMessage::CompilerArtifact(artifact)).unwrap(); + // Try to deserialize a message from Cargo or Rustc. + let mut deserializer = serde_json::Deserializer::from_str(&line); + deserializer.disable_recursion_limit(); + if let Ok(message) = JsonMessage::deserialize(&mut deserializer) { + match message { + // Skip certain kinds of messages to only spend time on what's useful + JsonMessage::Cargo(message) => match message { + cargo_metadata::Message::CompilerArtifact(artifact) + if !artifact.fresh => + { + self.sender.send(CargoMessage::CompilerArtifact(artifact)).unwrap(); + } + cargo_metadata::Message::CompilerMessage(msg) => { + self.sender.send(CargoMessage::Diagnostic(msg.message)).unwrap(); + } + _ => (), + }, + JsonMessage::Rustc(message) => { + self.sender.send(CargoMessage::Diagnostic(message)).unwrap(); } - cargo_metadata::Message::CompilerMessage(msg) => { - self.sender.send(CargoMessage::Diagnostic(msg.message)).unwrap(); - } - _ => (), - }, - JsonMessage::Rustc(message) => { - self.sender.send(CargoMessage::Diagnostic(message)).unwrap(); } } + }, + &mut |line| { + error.push_str(line); + error.push('\n'); + }, + ); + match output { + Ok(_) if read_at_least_one_message => Ok(()), + Ok(output) if output.status.success() => { + Err(io::Error::new(io::ErrorKind::Other, format!( + "Cargo watcher failed, the command produced no valid metadata (exit code: {:?})", + output.status + ))) } + Ok(_) => Err(io::Error::new(io::ErrorKind::Other, error)), + Err(e) => Err(e), } - read_at_least_one_message } } diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index 0c48b22bdb..9e1bac854d 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -394,7 +394,10 @@ impl GlobalState { flycheck::Progress::DidCancel => (Progress::End, None), flycheck::Progress::DidFinish(result) => { if let Err(err) = result { - tracing::error!("cargo check failed: {}", err) + self.show_message( + lsp_types::MessageType::Error, + format!("cargo check failed: {}", err), + ); } (Progress::End, None) }