Auto merge of rust-lang#15476 - Wilfred:implement-saved-file3, r=Veykril

Substitute $saved_file in custom check commands

If the custom command has a $saved_file placeholder, and we know the file being saved, replace the placeholder and run a check command.

If there's a placeholder and we don't know the saved file, do nothing.

This is a simplified version of rust-lang#15381, which I hope is easier to review.

Author: bors

crates/flycheck/src/lib.rs

@@ -14,7 +14,7 @@ use std::{
 
 use command_group::{CommandGroup, GroupChild};
 use crossbeam_channel::{never, select, unbounded, Receiver, Sender};
-use paths::AbsPathBuf;
+use paths::{AbsPath, AbsPathBuf};
 use rustc_hash::FxHashMap;
 use serde::Deserialize;
 use stdx::process::streaming_output;
@@ -102,13 +102,15 @@ impl FlycheckHandle {
     }
 
     /// Schedule a re-start of the cargo check worker to do a workspace wide check.
-    pub fn restart_workspace(&self) {
-        self.sender.send(StateChange::Restart(None)).unwrap();
+    pub fn restart_workspace(&self, saved_file: Option<AbsPathBuf>) {
+        self.sender.send(StateChange::Restart { package: None, saved_file }).unwrap();
     }
 
     /// Schedule a re-start of the cargo check worker to do a package wide check.
     pub fn restart_for_package(&self, package: String) {
-        self.sender.send(StateChange::Restart(Some(package))).unwrap();
+        self.sender
+            .send(StateChange::Restart { package: Some(package), saved_file: None })
+            .unwrap();
     }
 
     /// Stop this cargo check worker.
@@ -159,7 +161,7 @@ pub enum Progress {
 }
 
 enum StateChange {
-    Restart(Option<String>),
+    Restart { package: Option<String>, saved_file: Option<AbsPathBuf> },
     Cancel,
 }
 
@@ -186,6 +188,8 @@ enum Event {
     CheckEvent(Option<CargoMessage>),
 }
 
+const SAVED_FILE_PLACEHOLDER: &str = "$saved_file";
+
 impl FlycheckActor {
     fn new(
         id: usize,
@@ -221,7 +225,7 @@ impl FlycheckActor {
                     tracing::debug!(flycheck_id = self.id, "flycheck cancelled");
                     self.cancel_check_process();
                 }
-                Event::RequestStateChange(StateChange::Restart(package)) => {
+                Event::RequestStateChange(StateChange::Restart { package, saved_file }) => {
                     // Cancel the previously spawned process
                     self.cancel_check_process();
                     while let Ok(restart) = inbox.recv_timeout(Duration::from_millis(50)) {
@@ -231,7 +235,11 @@ impl FlycheckActor {
                         }
                     }
 
-                    let command = self.check_command(package.as_deref());
+                    let command =
+                        match self.check_command(package.as_deref(), saved_file.as_deref()) {
+                            Some(c) => c,
+                            None => continue,
+                        };
                     let formatted_command = format!("{:?}", command);
 
                     tracing::debug!(?command, "will restart flycheck");
@@ -305,7 +313,14 @@ impl FlycheckActor {
         }
     }
 
-    fn check_command(&self, package: Option<&str>) -> Command {
+    /// Construct a `Command` object for checking the user's code. If the user
+    /// has specified a custom command with placeholders that we cannot fill,
+    /// return None.
+    fn check_command(
+        &self,
+        package: Option<&str>,
+        saved_file: Option<&AbsPath>,
+    ) -> Option<Command> {
         let (mut cmd, args) = match &self.config {
             FlycheckConfig::CargoCommand {
                 command,
@@ -358,7 +373,7 @@ impl FlycheckActor {
                     cmd.arg("--target-dir").arg(target_dir);
                 }
                 cmd.envs(extra_env);
-                (cmd, extra_args)
+                (cmd, extra_args.clone())
             }
             FlycheckConfig::CustomCommand {
                 command,
@@ -387,12 +402,34 @@ impl FlycheckActor {
                     }
                 }
 
-                (cmd, args)
+                if args.contains(&SAVED_FILE_PLACEHOLDER.to_owned()) {
+                    // If the custom command has a $saved_file placeholder, and
+                    // we're saving a file, replace the placeholder in the arguments.
+                    if let Some(saved_file) = saved_file {
+                        let args = args
+                            .iter()
+                            .map(|arg| {
+                                if arg == SAVED_FILE_PLACEHOLDER {
+                                    saved_file.to_string()
+                                } else {
+                                    arg.clone()
+                                }
+                            })
+                            .collect();
+                        (cmd, args)
+                    } else {
+                        // The custom command has a $saved_file placeholder,
+                        // but we had an IDE event that wasn't a file save. Do nothing.
+                        return None;
+                    }
+                } else {
+                    (cmd, args.clone())
+                }
             }
         };
 
         cmd.args(args);
-        cmd
+        Some(cmd)
     }
 
     fn send(&self, check_task: Message) {

crates/rust-analyzer/src/config.rs

@@ -209,6 +209,11 @@ config_data! {
         /// by changing `#rust-analyzer.check.invocationStrategy#` and
         /// `#rust-analyzer.check.invocationLocation#`.
         ///
+        /// If `$saved_file` is part of the command, rust-analyzer will pass
+        /// the absolute path of the saved file to the provided command. This is
+        /// intended to be used with non-Cargo build systems.
+        /// Note that `$saved_file` is experimental and may be removed in the futureg.
+        ///
         /// An example command would be:
         ///
         /// ```bash

crates/rust-analyzer/src/handlers/notification.rs

@@ -168,7 +168,7 @@ pub(crate) fn handle_did_save_text_document(
     } else if state.config.check_on_save() {
         // No specific flycheck was triggered, so let's trigger all of them.
         for flycheck in state.flycheck.iter() {
-            flycheck.restart_workspace();
+            flycheck.restart_workspace(None);
         }
     }
     Ok(())
@@ -314,14 +314,16 @@ fn run_flycheck(state: &mut GlobalState, vfs_path: VfsPath) -> bool {
                 Some((idx, package))
             });
 
+            let saved_file = vfs_path.as_path().map(|p| p.to_owned());
+
             // Find and trigger corresponding flychecks
             for flycheck in world.flycheck.iter() {
                 for (id, package) in workspace_ids.clone() {
                     if id == flycheck.id() {
                         updated = true;
                         match package.filter(|_| !world.config.flycheck_workspace()) {
                             Some(package) => flycheck.restart_for_package(package),
-                            None => flycheck.restart_workspace(),
+                            None => flycheck.restart_workspace(saved_file.clone()),
                         }
                         continue;
                     }
@@ -330,7 +332,7 @@ fn run_flycheck(state: &mut GlobalState, vfs_path: VfsPath) -> bool {
             // No specific flycheck was triggered, so let's trigger all of them.
             if !updated {
                 for flycheck in world.flycheck.iter() {
-                    flycheck.restart_workspace();
+                    flycheck.restart_workspace(saved_file.clone());
                 }
             }
             Ok(())
@@ -372,7 +374,7 @@ pub(crate) fn handle_run_flycheck(
     }
     // No specific flycheck was triggered, so let's trigger all of them.
     for flycheck in state.flycheck.iter() {
-        flycheck.restart_workspace();
+        flycheck.restart_workspace(None);
     }
     Ok(())
 }

crates/rust-analyzer/src/main_loop.rs

@@ -8,7 +8,6 @@ use std::{
 
 use always_assert::always;
 use crossbeam_channel::{select, Receiver};
-use flycheck::FlycheckHandle;
 use ide_db::base_db::{SourceDatabaseExt, VfsPath};
 use lsp_server::{Connection, Notification, Request};
 use lsp_types::notification::Notification as _;
@@ -337,7 +336,7 @@ impl GlobalState {
             if became_quiescent {
                 if self.config.check_on_save() {
                     // Project has loaded properly, kick off initial flycheck
-                    self.flycheck.iter().for_each(FlycheckHandle::restart_workspace);
+                    self.flycheck.iter().for_each(|flycheck| flycheck.restart_workspace(None));
                 }
                 if self.config.prefill_caches() {
                     self.prime_caches_queue.request_op("became quiescent".to_owned(), ());

docs/user/generated_config.adoc

@@ -234,6 +234,11 @@ each of them, with the working directory being the workspace root
 by changing `#rust-analyzer.check.invocationStrategy#` and
 `#rust-analyzer.check.invocationLocation#`.
 
+If `$saved_file` is part of the command, rust-analyzer will pass
+the absolute path of the saved file to the provided command. This is
+intended to be used with non-Cargo build systems.
+Note that `$saved_file` is experimental and may be removed in the futureg.
+
 An example command would be:
 
 ```bash

editors/code/package.json

@@ -775,7 +775,7 @@
                     ]
                 },
                 "rust-analyzer.check.overrideCommand": {
-                    "markdownDescription": "Override the command rust-analyzer uses instead of `cargo check` for\ndiagnostics on save. The command is required to output json and\nshould therefore include `--message-format=json` or a similar option\n(if your client supports the `colorDiagnosticOutput` experimental\ncapability, you can use `--message-format=json-diagnostic-rendered-ansi`).\n\nIf you're changing this because you're using some tool wrapping\nCargo, you might also want to change\n`#rust-analyzer.cargo.buildScripts.overrideCommand#`.\n\nIf there are multiple linked projects/workspaces, this command is invoked for\neach of them, with the working directory being the workspace root\n(i.e., the folder containing the `Cargo.toml`). This can be overwritten\nby changing `#rust-analyzer.check.invocationStrategy#` and\n`#rust-analyzer.check.invocationLocation#`.\n\nAn example command would be:\n\n```bash\ncargo check --workspace --message-format=json --all-targets\n```\n.",
+                    "markdownDescription": "Override the command rust-analyzer uses instead of `cargo check` for\ndiagnostics on save. The command is required to output json and\nshould therefore include `--message-format=json` or a similar option\n(if your client supports the `colorDiagnosticOutput` experimental\ncapability, you can use `--message-format=json-diagnostic-rendered-ansi`).\n\nIf you're changing this because you're using some tool wrapping\nCargo, you might also want to change\n`#rust-analyzer.cargo.buildScripts.overrideCommand#`.\n\nIf there are multiple linked projects/workspaces, this command is invoked for\neach of them, with the working directory being the workspace root\n(i.e., the folder containing the `Cargo.toml`). This can be overwritten\nby changing `#rust-analyzer.check.invocationStrategy#` and\n`#rust-analyzer.check.invocationLocation#`.\n\nIf `$saved_file` is part of the command, rust-analyzer will pass\nthe absolute path of the saved file to the provided command. This is\nintended to be used with non-Cargo build systems.\nNote that `$saved_file` is experimental and may be removed in the futureg.\n\nAn example command would be:\n\n```bash\ncargo check --workspace --message-format=json --all-targets\n```\n.",
                     "default": null,
                     "type": [
                         "null",