1 files changed, 117 insertions, 151 deletions

diff --git a/crates/ra_flycheck/src/lib.rs b/crates/ra_flycheck/src/lib.rs
index 38940a77b..13494a731 100644
--- a/crates/ra_flycheck/src/lib.rs
+++ b/crates/ra_flycheck/src/lib.rs
@@ -1,55 +1,53 @@
 //! 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.
+mod conv;
+
+use std::{
+    env,
+    io::{self, BufRead, BufReader},
+    path::PathBuf,
+    process::{Command, Stdio},
+    time::Instant,
+};
+
 use cargo_metadata::Message;
 use crossbeam_channel::{never, select, unbounded, Receiver, RecvError, Sender};
 use lsp_types::{
     CodeAction, CodeActionOrCommand, Diagnostic, Url, WorkDoneProgress, WorkDoneProgressBegin,
     WorkDoneProgressEnd, WorkDoneProgressReport,
 };
-use std::{
-    error, fmt,
-    io::{BufRead, BufReader},
-    path::{Path, PathBuf},
-    process::{Command, Stdio},
-    time::Instant,
-};
-
-mod conv;
 
 use crate::conv::{map_rust_diagnostic_to_lsp, MappedRustDiagnostic};
 
 pub use crate::conv::url_from_path_with_drive_lowercasing;
 
 #[derive(Clone, Debug)]
-pub struct CheckConfig {
-    pub enable: bool,
-    pub args: Vec<String>,
-    pub command: String,
-    pub all_targets: bool,
+pub enum FlycheckConfig {
+    CargoCommand { command: String, all_targets: bool, extra_args: Vec<String> },
+    CustomCommand { command: String, args: Vec<String> },
 }
 
-/// CheckWatcher wraps the shared state and communication machinery used for
+/// Flycheck 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 struct Flycheck {
     // XXX: drop order is significant
     cmd_send: Sender<CheckCommand>,
-    handle: Option<jod_thread::JoinHandle<()>>,
+    handle: jod_thread::JoinHandle<()>,
     pub task_recv: Receiver<CheckTask>,
 }
 
-impl CheckWatcher {
-    pub fn new(config: CheckConfig, workspace_root: PathBuf) -> CheckWatcher {
+impl Flycheck {
+    pub fn new(config: FlycheckConfig, workspace_root: PathBuf) -> Flycheck {
         let (task_send, task_recv) = unbounded::<CheckTask>();
         let (cmd_send, cmd_recv) = unbounded::<CheckCommand>();
         let handle = jod_thread::spawn(move || {
-            let mut check = CheckWatcherThread::new(config, workspace_root);
-            check.run(&task_send, &cmd_recv);
+            FlycheckThread::new(config, workspace_root).run(&task_send, &cmd_recv);
         });
-        CheckWatcher { task_recv, cmd_send, handle: Some(handle) }
+        Flycheck { task_recv, cmd_send, handle }
     }
 
     /// Schedule a re-start of the cargo check worker.
@@ -75,20 +73,28 @@ pub enum CheckCommand {
     Update,
 }
 
-struct CheckWatcherThread {
-    options: CheckConfig,
+struct FlycheckThread {
+    config: FlycheckConfig,
     workspace_root: PathBuf,
-    watcher: WatchThread,
     last_update_req: Option<Instant>,
+    // XXX: drop order is significant
+    message_recv: Receiver<CheckEvent>,
+    /// 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.
+    check_process: Option<jod_thread::JoinHandle<()>>,
 }
 
-impl CheckWatcherThread {
-    fn new(options: CheckConfig, workspace_root: PathBuf) -> CheckWatcherThread {
-        CheckWatcherThread {
-            options,
+impl FlycheckThread {
+    fn new(config: FlycheckConfig, workspace_root: PathBuf) -> FlycheckThread {
+        FlycheckThread {
+            config,
             workspace_root,
-            watcher: WatchThread::dummy(),
             last_update_req: None,
+            message_recv: never(),
+            check_process: None,
         }
     }
 
@@ -105,25 +111,21 @@ impl CheckWatcherThread {
                         break;
                     },
                 },
-                recv(self.watcher.message_recv) -> msg => match msg {
+                recv(self.message_recv) -> msg => match msg {
                     Ok(msg) => self.handle_message(msg, task_send),
                     Err(RecvError) => {
                         // Watcher finished, replace it with a never channel to
                         // avoid busy-waiting.
-                        std::mem::replace(&mut self.watcher.message_recv, never());
+                        self.message_recv = never();
+                        self.check_process = None;
                     },
                 }
             };
 
             if self.should_recheck() {
-                self.last_update_req.take();
+                self.last_update_req = None;
                 task_send.send(CheckTask::ClearDiagnostics).unwrap();
-
-                // Replace with a dummy watcher first so we drop the original and wait for completion
-                std::mem::replace(&mut self.watcher, WatchThread::dummy());
-
-                // Then create the actual new watcher
-                self.watcher = WatchThread::new(&self.options, &self.workspace_root);
+                self.restart_check_process();
             }
         }
     }
@@ -206,6 +208,63 @@ impl CheckWatcherThread {
             CheckEvent::Msg(Message::Unknown) => {}
         }
     }
+
+    fn restart_check_process(&mut self) {
+        // First, clear and cancel the old thread
+        self.message_recv = never();
+        self.check_process = None;
+
+        let mut cmd = match &self.config {
+            FlycheckConfig::CargoCommand { command, all_targets, extra_args } => {
+                let mut cmd = Command::new(cargo_binary());
+                cmd.arg(command);
+                cmd.args(&["--workspace", "--message-format=json", "--manifest-path"]);
+                cmd.arg(self.workspace_root.join("Cargo.toml"));
+                if *all_targets {
+                    cmd.arg("--all-targets");
+                }
+                cmd.args(extra_args);
+                cmd
+            }
+            FlycheckConfig::CustomCommand { command, args } => {
+                let mut cmd = Command::new(command);
+                cmd.args(args);
+                cmd
+            }
+        };
+        cmd.current_dir(&self.workspace_root);
+
+        let (message_send, message_recv) = unbounded();
+        self.message_recv = message_recv;
+        self.check_process = Some(jod_thread::spawn(move || {
+            // 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);
+
+            let res = run_cargo(cmd, &mut |message| {
+                // Skip certain kinds of messages to only spend time on what's useful
+                match &message {
+                    Message::CompilerArtifact(artifact) if artifact.fresh => return true,
+                    Message::BuildScriptExecuted(_) => return true,
+                    Message::Unknown => return true,
+                    _ => {}
+                }
+
+                // if the send channel was closed, we want to shutdown
+                message_send.send(CheckEvent::Msg(message)).is_ok()
+            });
+
+            if let Err(err) = res {
+                // FIXME: make the `message_send` to be `Sender<Result<CheckEvent, CargoError>>`
+                // to display user-caused misconfiguration errors instead of just logging them here
+                log::error!("Cargo watcher failed {:?}", err);
+            }
+
+            // We can ignore any error here, as we are already in the progress
+            // of shutting down.
+            let _ = message_send.send(CheckEvent::End);
+        }))
+    }
 }
 
 #[derive(Debug)]
@@ -214,52 +273,18 @@ pub struct DiagnosticWithFixes {
     fixes: Vec<CodeAction>,
 }
 
-/// 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.
-/// 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 {
-    // XXX: drop order is significant
-    message_recv: Receiver<CheckEvent>,
-    _handle: Option<jod_thread::JoinHandle<()>>,
-}
-
 enum CheckEvent {
     Begin,
     Msg(cargo_metadata::Message),
     End,
 }
 
-#[derive(Debug)]
-pub struct CargoError(String);
-
-impl fmt::Display for CargoError {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "Cargo failed: {}", self.0)
-    }
-}
-impl error::Error for CargoError {}
-
 fn run_cargo(
-    args: &[String],
-    current_dir: Option<&Path>,
+    mut command: Command,
     on_message: &mut dyn FnMut(cargo_metadata::Message) -> bool,
-) -> Result<(), CargoError> {
-    let mut command = Command::new("cargo");
-    if let Some(current_dir) = current_dir {
-        command.current_dir(current_dir);
-    }
-
-    let mut child = command
-        .args(args)
-        .stdout(Stdio::piped())
-        .stderr(Stdio::null())
-        .stdin(Stdio::null())
-        .spawn()
-        .expect("couldn't launch cargo");
+) -> io::Result<()> {
+    let mut child =
+        command.stdout(Stdio::piped()).stderr(Stdio::null()).stdin(Stdio::null()).spawn()?;
 
     // We manually read a line at a time, instead of using serde's
     // stream deserializers, because the deserializer cannot recover
@@ -273,13 +298,7 @@ fn run_cargo(
     let mut read_at_least_one_message = false;
 
     for line in stdout.lines() {
-        let line = match line {
-            Ok(line) => line,
-            Err(err) => {
-                log::error!("Couldn't read line from cargo: {}", err);
-                continue;
-            }
-        };
+        let line = line?;
 
         let message = serde_json::from_str::<cargo_metadata::Message>(&line);
         let message = match message {
@@ -300,75 +319,22 @@ fn run_cargo(
     // It is okay to ignore the result, as it only errors if the process is already dead
     let _ = child.kill();
 
-    let err_msg = match child.wait() {
-        Ok(exit_code) if !exit_code.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
+    let exit_status = child.wait()?;
+    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!(
-                "the command produced no valid metadata (exit code: {:?}): cargo {}",
-                exit_code,
-                args.join(" ")
-            )
-        }
-        Err(err) => format!("io error: {:?}", err),
-        Ok(_) => return Ok(()),
-    };
-
-    Err(CargoError(err_msg))
-}
-
-impl WatchThread {
-    fn dummy() -> WatchThread {
-        WatchThread { message_recv: never(), _handle: None }
+                "the command produced no valid metadata (exit code: {:?}): {:?}",
+                exit_status, command
+            ),
+        ));
     }
 
-    fn new(options: &CheckConfig, workspace_root: &Path) -> WatchThread {
-        let mut args: Vec<String> = vec![
-            options.command.clone(),
-            "--workspace".to_string(),
-            "--message-format=json".to_string(),
-            "--manifest-path".to_string(),
-            format!("{}/Cargo.toml", workspace_root.display()),
-        ];
-        if options.all_targets {
-            args.push("--all-targets".to_string());
-        }
-        args.extend(options.args.iter().cloned());
-
-        let (message_send, message_recv) = unbounded();
-        let workspace_root = workspace_root.to_owned();
-        let handle = if options.enable {
-            Some(jod_thread::spawn(move || {
-                // 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);
-
-                let res = run_cargo(&args, Some(&workspace_root), &mut |message| {
-                    // Skip certain kinds of messages to only spend time on what's useful
-                    match &message {
-                        Message::CompilerArtifact(artifact) if artifact.fresh => return true,
-                        Message::BuildScriptExecuted(_) => return true,
-                        Message::Unknown => return true,
-                        _ => {}
-                    }
-
-                    // if the send channel was closed, we want to shutdown
-                    message_send.send(CheckEvent::Msg(message)).is_ok()
-                });
-
-                if let Err(err) = res {
-                    // FIXME: make the `message_send` to be `Sender<Result<CheckEvent, CargoError>>`
-                    // to display user-caused misconfiguration errors instead of just logging them here
-                    log::error!("Cargo watcher failed {:?}", err);
-                }
+    Ok(())
+}
 
-                // We can ignore any error here, as we are already in the progress
-                // of shutting down.
-                let _ = message_send.send(CheckEvent::End);
-            }))
-        } else {
-            None
-        };
-        WatchThread { message_recv, _handle: handle }
-    }
+fn cargo_binary() -> String {
+    env::var("CARGO").unwrap_or_else(|_| "cargo".to_string())
 }