Skip to content

[WIP] Add initial crate-specific flycheck support #12994

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[WIP] Add initial crate-specific flycheck support #12994

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 42 additions & 17 deletions crates/flycheck/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#![warn(rust_2018_idioms, unused_lifetimes, semicolon_in_expressions_from_macros)]

use std::{
ffi::OsString,
fmt, io,
process::{ChildStderr, ChildStdout, Command, Stdio},
time::Duration,
Expand Down Expand Up @@ -77,8 +78,8 @@ impl FlycheckHandle {
}

/// Schedule a re-start of the cargo check worker.
pub fn update(&self) {
self.sender.send(Restart).unwrap();
pub fn update(&self, path: Option<AbsPathBuf>) {
self.sender.send(Restart(path)).unwrap();
}

pub fn id(&self) -> usize {
Expand Down Expand Up @@ -122,7 +123,7 @@ pub enum Progress {
DidCancel,
}

struct Restart;
struct Restart(Option<AbsPathBuf>);

struct FlycheckActor {
id: usize,
Expand Down Expand Up @@ -164,25 +165,31 @@ impl FlycheckActor {
fn run(mut self, inbox: Receiver<Restart>) {
while let Some(event) = self.next_event(&inbox) {
match event {
Event::Restart(Restart) => {
Event::Restart(Restart(path)) => {
// Cancel the previously spawned process
self.cancel_check_process();
while let Ok(Restart) = inbox.recv_timeout(Duration::from_millis(50)) {}
let mut changed_path = path;
while let Ok(Restart(path)) = inbox.recv_timeout(Duration::from_millis(50)) {
// TODO: this usually means the user saved multiple files, so fall back to
// a full workspace check. maybe the threshold should be 3 saves instead of
// 2 if rustfmt on save causes the file to be written to disk twice?
changed_path = path
}

let command = self.check_command();
let command = self.check_command(&changed_path);
tracing::debug!(?command, "will restart flycheck");
match CargoHandle::spawn(command) {
Ok(cargo_handle) => {
tracing::debug!(
command = ?self.check_command(),
"did restart flycheck"
command = ?self.check_command(&changed_path),
"did restart flycheck"
);
self.cargo_handle = Some(cargo_handle);
self.progress(Progress::DidStart);
}
Err(error) => {
tracing::error!(
command = ?self.check_command(),
command = ?self.check_command(&changed_path),
%error, "failed to restart flycheck"
);
}
Expand All @@ -193,12 +200,13 @@ impl FlycheckActor {

// Watcher finished
let cargo_handle = self.cargo_handle.take().unwrap();
let err = format!(
"Flycheck failed to run the following command: {:?}",
cargo_handle.command
);
let res = cargo_handle.join();
if res.is_err() {
tracing::error!(
"Flycheck failed to run the following command: {:?}",
self.check_command()
);
tracing::error!(err);
}
self.progress(Progress::DidFinish(res));
}
Expand Down Expand Up @@ -228,7 +236,23 @@ impl FlycheckActor {
}
}

fn check_command(&self) -> Command {
fn substitute_args(&self, args: &[String], changed_path: &Option<AbsPathBuf>) -> Vec<OsString> {
let mut result = Vec::new();
for arg in args {
if arg == "$changed_file" {
if let Some(path) = changed_path {
result.push(path.as_os_str().to_os_string())
} else {
// TODO: remove substitutions entirely? or replace with "ALL" or something?
}
} else {
result.push(arg.into())
}
}
result
}

fn check_command(&self, changed_path: &Option<AbsPathBuf>) -> Command {
let mut cmd = match &self.config {
FlycheckConfig::CargoCommand {
command,
Expand Down Expand Up @@ -267,7 +291,7 @@ impl FlycheckActor {
}
FlycheckConfig::CustomCommand { command, args } => {
let mut cmd = Command::new(command);
cmd.args(args);
cmd.args(self.substitute_args(args, changed_path));
cmd
}
};
Expand All @@ -285,14 +309,15 @@ struct CargoHandle {
/// The handle to the actual cargo process. As we cannot cancel directly from with
/// a read syscall dropping and therefor terminating the process is our best option.
child: JodChild,
command: Command,
thread: jod_thread::JoinHandle<io::Result<(bool, String)>>,
receiver: Receiver<CargoMessage>,
}

impl CargoHandle {
fn spawn(mut command: Command) -> std::io::Result<CargoHandle> {
command.stdout(Stdio::piped()).stderr(Stdio::piped()).stdin(Stdio::null());
let mut child = JodChild::spawn(command)?;
let mut child = JodChild::spawn(&mut command)?;

let stdout = child.stdout.take().unwrap();
let stderr = child.stderr.take().unwrap();
Expand All @@ -303,7 +328,7 @@ impl CargoHandle {
.name("CargoHandle".to_owned())
.spawn(move || actor.run())
.expect("failed to spawn thread");
Ok(CargoHandle { child, thread, receiver })
Ok(CargoHandle { child, command, thread, receiver })
}

fn cancel(mut self) {
Expand Down
14 changes: 8 additions & 6 deletions crates/rust-analyzer/src/main_loop.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,9 @@ impl GlobalState {
&& !self.fetch_build_data_queue.op_requested()
{
for flycheck in &self.flycheck {
flycheck.update();
// TODO: why do we do workspace wide checks here? is this only triggered upon
// initial load?
flycheck.update(None);
}
if self.config.prefill_caches() {
self.prime_caches_queue.request_op("became quiescent".to_string());
Expand Down Expand Up @@ -794,7 +796,7 @@ impl GlobalState {
for (id, _) in workspace_ids.clone() {
if id == flycheck.id() {
updated = true;
flycheck.update();
flycheck.update(vfs_path.as_path().map(ToOwned::to_owned));
continue;
}
}
Expand All @@ -806,10 +808,10 @@ impl GlobalState {
.request_op(format!("DidSaveTextDocument {}", abs_path.display()));
}
}
}
if !updated {
for flycheck in &this.flycheck {
flycheck.update();
if !updated {
for flycheck in &this.flycheck {
flycheck.update(vfs_path.as_path().map(ToOwned::to_owned));
}
}
}
Ok(())
Expand Down
2 changes: 1 addition & 1 deletion crates/stdx/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ impl Drop for JodChild {
}

impl JodChild {
pub fn spawn(mut command: Command) -> sio::Result<Self> {
pub fn spawn(command: &mut Command) -> sio::Result<Self> {
command.spawn().map(Self)
}

Expand Down