{manifest-path} interpolation

Veykril · Veykril · commit 7db50294a3e4 · 2022-10-19T23:21:34.000+02:00
diff --git a/crates/flycheck/src/lib.rs b/crates/flycheck/src/lib.rs
@@ -6,7 +6,6 @@
 
 use std::{
     fmt, io,
-    path::Path,
     process::{ChildStderr, ChildStdout, Command, Stdio},
     time::Duration,
 };
@@ -25,7 +24,6 @@ pub use cargo_metadata::diagnostic::{
 #[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
 pub enum InvocationStrategy {
     OnceInRoot,
-    PerWorkspaceWithManifestPath,
     #[default]
     PerWorkspace,
 }
@@ -153,7 +151,9 @@ struct FlycheckActor {
     id: usize,
     sender: Box<dyn Fn(Message) + Send>,
     config: FlycheckConfig,
-    workspace_root: AbsPathBuf,
+    /// Either the workspace root of the workspace we are flychecking,
+    /// or the project root of the project.
+    root: AbsPathBuf,
     /// CargoHandle 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
@@ -175,7 +175,7 @@ impl FlycheckActor {
         workspace_root: AbsPathBuf,
     ) -> FlycheckActor {
         tracing::info!(%id, ?workspace_root, "Spawning flycheck");
-        FlycheckActor { id, sender, config, workspace_root, cargo_handle: None }
+        FlycheckActor { id, sender, config, root: workspace_root, cargo_handle: None }
     }
 
     fn report_progress(&self, progress: Progress) {
@@ -210,20 +210,7 @@ impl FlycheckActor {
                         }
                     }
 
-                    let mut command = self.check_command();
-                    let invocation_strategy = self.invocation_strategy();
-                    match invocation_strategy {
-                        InvocationStrategy::OnceInRoot => (),
-                        InvocationStrategy::PerWorkspaceWithManifestPath => {
-                            command.arg("--manifest-path");
-                            command.arg(<_ as AsRef<Path>>::as_ref(
-                                &self.workspace_root.join("Cargo.toml"),
-                            ));
-                        }
-                        InvocationStrategy::PerWorkspace => {
-                            command.current_dir(&self.workspace_root);
-                        }
-                    }
+                    let command = self.check_command();
                     tracing::debug!(?command, "will restart flycheck");
                     match CargoHandle::spawn(command) {
                         Ok(cargo_handle) => {
@@ -265,7 +252,7 @@ impl FlycheckActor {
                     CargoMessage::Diagnostic(msg) => {
                         self.send(Message::AddDiagnostic {
                             id: self.id,
-                            workspace_root: self.workspace_root.clone(),
+                            workspace_root: self.root.clone(),
                             diagnostic: msg,
                         });
                     }
@@ -287,15 +274,8 @@ impl FlycheckActor {
         }
     }
 
-    fn invocation_strategy(&self) -> InvocationStrategy {
-        match self.config {
-            FlycheckConfig::CargoCommand { invocation_strategy, .. }
-            | FlycheckConfig::CustomCommand { invocation_strategy, .. } => invocation_strategy,
-        }
-    }
-
     fn check_command(&self) -> Command {
-        let mut cmd = match &self.config {
+        let (mut cmd, args, invocation_strategy) = match &self.config {
             FlycheckConfig::CargoCommand {
                 command,
                 target_triple,
@@ -305,13 +285,11 @@ impl FlycheckActor {
                 extra_args,
                 features,
                 extra_env,
-                invocation_strategy: _,
+                invocation_strategy,
             } => {
                 let mut cmd = Command::new(toolchain::cargo());
                 cmd.arg(command);
-                cmd.current_dir(&self.workspace_root);
-                cmd.args(&["--workspace", "--message-format=json", "--manifest-path"])
-                    .arg(self.workspace_root.join("Cargo.toml").as_os_str());
+                cmd.args(&["--workspace", "--message-format=json"]);
 
                 if let Some(target) = target_triple {
                     cmd.args(&["--target", target.as_str()]);
@@ -330,18 +308,35 @@ impl FlycheckActor {
                         cmd.arg(features.join(" "));
                     }
                 }
-                cmd.args(extra_args);
                 cmd.envs(extra_env);
-                cmd
+                (cmd, extra_args, invocation_strategy)
             }
-            FlycheckConfig::CustomCommand { command, args, extra_env, invocation_strategy: _ } => {
+            FlycheckConfig::CustomCommand { command, args, extra_env, invocation_strategy } => {
                 let mut cmd = Command::new(command);
-                cmd.args(args);
                 cmd.envs(extra_env);
-                cmd
+                (cmd, args, invocation_strategy)
             }
         };
-        cmd.current_dir(&self.workspace_root);
+        if let InvocationStrategy::PerWorkspace = invocation_strategy {
+            let mut with_manifest_path = false;
+            for arg in args {
+                if let Some(_) = arg.find("$manifest_path") {
+                    with_manifest_path = true;
+                    cmd.arg(arg.replace(
+                        "$manifest_path",
+                        &self.root.join("Cargo.toml").display().to_string(),
+                    ));
+                } else {
+                    cmd.arg(arg);
+                }
+            }
+
+            if !with_manifest_path {
+                cmd.current_dir(&self.root);
+            }
+        } else {
+            cmd.args(args);
+        }
         cmd
     }
 
diff --git a/crates/project-model/src/build_scripts.rs b/crates/project-model/src/build_scripts.rs
@@ -55,11 +55,37 @@ impl BuildScriptOutput {
 }
 
 impl WorkspaceBuildScripts {
-    fn build_command(config: &CargoConfig) -> io::Result<Command> {
+    fn build_command(
+        config: &CargoConfig,
+        workspace_root: Option<&path::Path>,
+    ) -> io::Result<Command> {
         let mut cmd = match config.run_build_script_command.as_deref() {
             Some([program, args @ ..]) => {
                 let mut cmd = Command::new(program);
-                cmd.args(args);
+
+                // FIXME: strategy and workspace root are coupled, express that in code
+                if let (InvocationStrategy::PerWorkspace, Some(workspace_root)) =
+                    (config.invocation_strategy, workspace_root)
+                {
+                    let mut with_manifest_path = false;
+                    for arg in args {
+                        if let Some(_) = arg.find("$manifest_path") {
+                            with_manifest_path = true;
+                            cmd.arg(arg.replace(
+                                "$manifest_path",
+                                &workspace_root.join("Cargo.toml").display().to_string(),
+                            ));
+                        } else {
+                            cmd.arg(arg);
+                        }
+                    }
+
+                    if !with_manifest_path {
+                        cmd.current_dir(workspace_root);
+                    }
+                } else {
+                    cmd.args(args);
+                }
                 cmd
             }
             _ => {
@@ -90,9 +116,15 @@ impl WorkspaceBuildScripts {
                         }
                     }
                 }
+
+                if let Some(workspace_root) = workspace_root {
+                    cmd.current_dir(workspace_root);
+                }
+
                 cmd
             }
         };
+
         cmd.envs(&config.extra_env);
         if config.wrap_rustc_in_build_scripts {
             // Setup RUSTC_WRAPPER to point to `rust-analyzer` binary itself. We use
@@ -115,15 +147,21 @@ impl WorkspaceBuildScripts {
     ) -> io::Result<WorkspaceBuildScripts> {
         const RUST_1_62: Version = Version::new(1, 62, 0);
 
-        match Self::run_per_ws(Self::build_command(config)?, config, workspace, progress) {
+        let workspace_root: &path::Path = &workspace.workspace_root().as_ref();
+
+        match Self::run_per_ws(
+            Self::build_command(config, Some(workspace_root))?,
+            workspace,
+            progress,
+        ) {
             Ok(WorkspaceBuildScripts { error: Some(error), .. })
                 if toolchain.as_ref().map_or(false, |it| *it >= RUST_1_62) =>
             {
                 // building build scripts failed, attempt to build with --keep-going so
                 // that we potentially get more build data
-                let mut cmd = Self::build_command(config)?;
+                let mut cmd = Self::build_command(config, Some(workspace_root))?;
                 cmd.args(&["-Z", "unstable-options", "--keep-going"]).env("RUSTC_BOOTSTRAP", "1");
-                let mut res = Self::run_per_ws(cmd, config, workspace, progress)?;
+                let mut res = Self::run_per_ws(cmd, workspace, progress)?;
                 res.error = Some(error);
                 Ok(res)
             }
@@ -139,7 +177,7 @@ impl WorkspaceBuildScripts {
         progress: &dyn Fn(String),
     ) -> io::Result<Vec<WorkspaceBuildScripts>> {
         assert_eq!(config.invocation_strategy, InvocationStrategy::OnceInRoot);
-        let cmd = Self::build_command(config)?;
+        let cmd = Self::build_command(config, None)?;
         // NB: Cargo.toml could have been modified between `cargo metadata` and
         // `cargo check`. We shouldn't assume that package ids we see here are
         // exactly those from `config`.
@@ -187,24 +225,10 @@ impl WorkspaceBuildScripts {
     }
 
     fn run_per_ws(
-        mut cmd: Command,
-        config: &CargoConfig,
+        cmd: Command,
         workspace: &CargoWorkspace,
         progress: &dyn Fn(String),
     ) -> io::Result<WorkspaceBuildScripts> {
-        let workspace_root: &path::Path = &workspace.workspace_root().as_ref();
-
-        match config.invocation_strategy {
-            InvocationStrategy::OnceInRoot => (),
-            InvocationStrategy::PerWorkspaceWithManifestPath => {
-                cmd.arg("--manifest-path");
-                cmd.arg(workspace_root.join("Cargo.toml"));
-            }
-            InvocationStrategy::PerWorkspace => {
-                cmd.current_dir(workspace_root);
-            }
-        }
-
         let mut res = WorkspaceBuildScripts::default();
         let outputs = &mut res.outputs;
         // NB: Cargo.toml could have been modified between `cargo metadata` and
diff --git a/crates/project-model/src/lib.rs b/crates/project-model/src/lib.rs
@@ -158,10 +158,9 @@ fn utf8_stdout(mut cmd: Command) -> Result<String> {
     Ok(stdout.trim().to_string())
 }
 
-#[derive(Clone, Debug, Default, PartialEq, Eq)]
+#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
 pub enum InvocationStrategy {
     OnceInRoot,
-    PerWorkspaceWithManifestPath,
     #[default]
     PerWorkspace,
 }
diff --git a/crates/project-model/src/workspace.rs b/crates/project-model/src/workspace.rs
@@ -323,9 +323,7 @@ impl ProjectWorkspace {
         config: &CargoConfig,
         progress: &dyn Fn(String),
     ) -> Vec<Result<WorkspaceBuildScripts>> {
-        if let InvocationStrategy::PerWorkspaceWithManifestPath | InvocationStrategy::PerWorkspace =
-            config.invocation_strategy
-        {
+        if let InvocationStrategy::PerWorkspace = config.invocation_strategy {
             return workspaces.iter().map(|it| it.run_build_scripts(config, progress)).collect();
         }
 
diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs
@@ -70,12 +70,15 @@ config_data! {
         /// Run build scripts (`build.rs`) for more precise code analysis.
         cargo_buildScripts_enable: bool  = "true",
         /// Specifies the invocation strategy to use when running the build scripts command.
-        /// If `per_workspace_with_manifest_path` is set, the command will be executed for each
-        /// workspace, `--manifest-path {workspace-dir}` will be passed to the invoked command and
-        /// the command will be executed from the project root.
-        /// If `per_workspace` is set, the command will be executed for each workspace and the
-        /// command will be executed from the corresponding workspace root.
+        /// If `per_workspace` is set, the command will be executed for each workspace and all
+        /// occurrences of `$manifest_path` in the command will be replaced by the corresponding
+        /// manifest path of the workspace that the command is being invoked for. If interpolation
+        /// for the manifest path happens at least once, the commands will be executed from the
+        /// project root, otherwise the commands will be executed from the corresponding workspace
+        /// root.
         /// If `once_in_root` is set, the command will be executed once in the project root.
+        /// This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`
+        /// is set.
         cargo_buildScripts_invocationStrategy: InvocationStrategy = "\"per_workspace\"",
         /// Override the command rust-analyzer uses to run build scripts and
         /// build procedural macros. The command is required to output json
@@ -131,12 +134,15 @@ config_data! {
         /// Set to `"all"` to pass `--all-features` to Cargo.
         checkOnSave_features: Option<CargoFeaturesDef>      = "null",
         /// Specifies the invocation strategy to use when running the checkOnSave command.
-        /// If `per_workspace_with_manifest_path` is set, the command will be executed for each
-        /// workspace, `--manifest-path {workspace-dir}` will be passed to the invoked command and
-        /// the command will be executed from the project root.
-        /// If `per_workspace` is set, the command will be executed for each workspace and the
-        /// command will be executed from the corresponding workspace root.
+        /// If `per_workspace` is set, the command will be executed for each workspace and all
+        /// occurrences of `$manifest_path` in the command will be replaced by the corresponding
+        /// manifest path of the workspace that the command is being invoked for. If interpolation
+        /// for the manifest path happens at least once, the commands will be executed from the
+        /// project root, otherwise the commands will be executed from the corresponding workspace
+        /// root.
         /// If `once_in_root` is set, the command will be executed once in the project root.
+        /// This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`
+        /// is set.
         checkOnSave_invocationStrategy: InvocationStrategy = "\"per_workspace\"",
         /// Whether to pass `--no-default-features` to Cargo. Defaults to
         /// `#rust-analyzer.cargo.noDefaultFeatures#`.
@@ -1074,9 +1080,6 @@ impl Config {
             wrap_rustc_in_build_scripts: self.data.cargo_buildScripts_useRustcWrapper,
             invocation_strategy: match self.data.cargo_buildScripts_invocationStrategy {
                 InvocationStrategy::OnceInRoot => project_model::InvocationStrategy::OnceInRoot,
-                InvocationStrategy::PerWorkspaceWithManifestPath => {
-                    project_model::InvocationStrategy::PerWorkspaceWithManifestPath
-                }
                 InvocationStrategy::PerWorkspace => project_model::InvocationStrategy::PerWorkspace,
             },
             run_build_script_command: self.data.cargo_buildScripts_overrideCommand.clone(),
@@ -1104,9 +1107,6 @@ impl Config {
         }
         let invocation_strategy = match self.data.checkOnSave_invocationStrategy {
             InvocationStrategy::OnceInRoot => flycheck::InvocationStrategy::OnceInRoot,
-            InvocationStrategy::PerWorkspaceWithManifestPath => {
-                flycheck::InvocationStrategy::PerWorkspaceWithManifestPath
-            }
             InvocationStrategy::PerWorkspace => flycheck::InvocationStrategy::PerWorkspace,
         };
         let flycheck_config = match &self.data.checkOnSave_overrideCommand {
@@ -1623,7 +1623,6 @@ enum CargoFeaturesDef {
 #[serde(rename_all = "snake_case")]
 enum InvocationStrategy {
     OnceInRoot,
-    PerWorkspaceWithManifestPath,
     PerWorkspace,
 }
 
@@ -2043,10 +2042,9 @@ fn field_props(field: &str, ty: &str, doc: &[&str], default: &str) -> serde_json
         },
         "InvocationStrategy" => set! {
             "type": "string",
-            "enum": ["per_workspace", "per_workspace_with_manifest_path", "once_in_root"],
+            "enum": ["per_workspace", "once_in_root"],
             "enumDescriptions": [
-                "The command will be executed for each workspace, `--manifest-path {workspace-dir}` will be passed to the invoked command and the command will be executed from the project root.",
-                "The command will be executed for each workspace and the command will be executed from the corresponding workspace root.",
+                "The command will be executed for each workspace and `{manifest-path}` usages will be interpolated with the corresponding workspace manifests. If `{manifest-path}` is used, the commands will be executed in the project root, otherwise in the corresponding workspace roots.",
                 "The command will be executed once in the project root."
             ],
         },
diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs
@@ -483,8 +483,7 @@ impl GlobalState {
                 config.clone(),
                 self.config.root_path().clone(),
             )],
-            flycheck::InvocationStrategy::PerWorkspaceWithManifestPath
-            | flycheck::InvocationStrategy::PerWorkspace => {
+            flycheck::InvocationStrategy::PerWorkspace => {
                 self.workspaces
                     .iter()
                     .enumerate()
diff --git a/docs/user/generated_config.adoc b/docs/user/generated_config.adoc
diff --git a/editors/code/package.json b/editors/code/package.json

Original file line number	Diff line number	Diff line change
`@@ -158,10 +158,9 @@ fn utf8_stdout(mut cmd: Command) -> Result<String> {`
`158`	`158`	`Ok(stdout.trim().to_string())`
`159`	`159`	`}`
`160`	`160`
`161`		`-#[derive(Clone, Debug, Default, PartialEq, Eq)]`
	`161`	`+#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]`
`162`	`162`	`pub enum InvocationStrategy {`
`163`	`163`	`OnceInRoot,`
`164`		`- PerWorkspaceWithManifestPath,`
`165`	`164`	`#[default]`
`166`	`165`	`PerWorkspace,`
`167`	`166`	`}`