Enforce a current directory being set for spawned commands

clippy.toml

@@ -3,3 +3,7 @@ disallowed-types = [
     { path = "std::collections::HashSet", reason = "use FxHashSet" },
     { path = "std::collections::hash_map::RandomState", reason = "use BuildHasherDefault<FxHasher>"}
 ]
+
+disallowed-methods = [
+    { path = "std::process::Command::new", reason = "use `toolchain::command` instead as it forces the choice of a working directory" },
+]

crates/ide/src/expand_macro.rs

@@ -253,6 +253,7 @@ fn _format(
     let &crate_id = db.relevant_crates(file_id).iter().next()?;
     let edition = db.crate_graph()[crate_id].edition;
 
+    #[allow(clippy::disallowed_methods)]
     let mut cmd = std::process::Command::new(toolchain::Tool::Rustfmt.path());
     cmd.arg("--edition");
     cmd.arg(edition.to_string());

crates/proc-macro-api/src/process.rs

@@ -202,6 +202,7 @@ fn mk_child(
     env: impl IntoIterator<Item = (impl AsRef<std::ffi::OsStr>, impl AsRef<std::ffi::OsStr>)>,
     null_stderr: bool,
 ) -> io::Result<Child> {
+    #[allow(clippy::disallowed_methods)]
     let mut cmd = Command::new(path);
     cmd.envs(env)
         .env("RUST_ANALYZER_INTERNALS_DO_NOT_USE", "this is unstable")

crates/proc-macro-srv/build.rs

@@ -7,6 +7,7 @@ fn main() {
     println!("cargo::rustc-check-cfg=cfg(rust_analyzer)");
 
     let rustc = env::var("RUSTC").expect("proc-macro-srv's build script expects RUSTC to be set");
+    #[allow(clippy::disallowed_methods)]
     let output = Command::new(rustc).arg("--version").output().expect("rustc --version must run");
     let version_string = std::str::from_utf8(&output.stdout[..])
         .expect("rustc --version output must be UTF-8")

crates/proc-macro-srv/proc-macro-test/build.rs

@@ -7,6 +7,8 @@
 //! a specific rustup toolchain: this allows testing against older ABIs (e.g.
 //! 1.58) and future ABIs (stage1, nightly)
 
+#![allow(clippy::disallowed_methods)]
+
 use std::{
     env,
     path::{Path, PathBuf},

crates/project-model/src/build_dependencies.rs

@@ -172,16 +172,15 @@ impl WorkspaceBuildScripts {
         }
         let res = (|| {
             let target_libdir = (|| {
-                let mut cargo_config = sysroot.tool(Tool::Cargo);
+                let mut cargo_config = sysroot.tool(Tool::Cargo, current_dir);
                 cargo_config.envs(extra_env);
                 cargo_config
-                    .current_dir(current_dir)
                     .args(["rustc", "-Z", "unstable-options", "--print", "target-libdir"])
                     .env("RUSTC_BOOTSTRAP", "1");
                 if let Ok(it) = utf8_stdout(&mut cargo_config) {
                     return Ok(it);
                 }
-                let mut cmd = sysroot.tool(Tool::Rustc);
+                let mut cmd = sysroot.tool(Tool::Rustc, current_dir);
                 cmd.envs(extra_env);
                 cmd.args(["--print", "target-libdir"]);
                 utf8_stdout(&mut cmd)
@@ -390,12 +389,12 @@ impl WorkspaceBuildScripts {
     ) -> io::Result<Command> {
         let mut cmd = match config.run_build_script_command.as_deref() {
             Some([program, args @ ..]) => {
-                let mut cmd = Command::new(program);
+                let mut cmd = toolchain::command(program, current_dir);
                 cmd.args(args);
                 cmd
             }
             _ => {
-                let mut cmd = sysroot.tool(Tool::Cargo);
+                let mut cmd = sysroot.tool(Tool::Cargo, current_dir);
 
                 cmd.args(["check", "--quiet", "--workspace", "--message-format=json"]);
                 cmd.args(&config.extra_args);
@@ -448,7 +447,6 @@ impl WorkspaceBuildScripts {
             }
         };
 
-        cmd.current_dir(current_dir);
         cmd.envs(&config.extra_env);
         if config.wrap_rustc_in_build_scripts {
             // Setup RUSTC_WRAPPER to point to `rust-analyzer` binary itself. We use

crates/project-model/src/cargo_workspace.rs

@@ -294,7 +294,7 @@ impl CargoWorkspace {
         no_deps: bool,
         progress: &dyn Fn(String),
     ) -> anyhow::Result<(cargo_metadata::Metadata, Option<anyhow::Error>)> {
-        let cargo = sysroot.tool(Tool::Cargo);
+        let cargo = sysroot.tool(Tool::Cargo, current_dir);
         let mut meta = MetadataCommand::new();
         meta.cargo_path(cargo.get_program());
         cargo.get_envs().for_each(|(var, val)| _ = meta.env(var, val.unwrap_or_default()));

crates/project-model/src/env.rs

@@ -74,10 +74,9 @@ pub(crate) fn cargo_config_env(
     extra_env: &FxHashMap<String, String>,
     sysroot: &Sysroot,
 ) -> FxHashMap<String, String> {
-    let mut cargo_config = sysroot.tool(Tool::Cargo);
+    let mut cargo_config = sysroot.tool(Tool::Cargo, manifest.parent());
     cargo_config.envs(extra_env);
     cargo_config
-        .current_dir(manifest.parent())
         .args(["-Z", "unstable-options", "config", "get", "env"])
         .env("RUSTC_BOOTSTRAP", "1");
     if manifest.is_rust_manifest() {

crates/project-model/src/sysroot.rs

@@ -4,7 +4,7 @@
 //! but we can't process `.rlib` and need source code instead. The source code
 //! is typically installed with `rustup component add rust-src` command.
 
-use std::{env, fs, ops, process::Command};
+use std::{env, fs, ops, path::Path, process::Command};
 
 use anyhow::{format_err, Result};
 use base_db::CrateName;
@@ -170,7 +170,7 @@ impl Sysroot {
     }
 
     /// Returns a command to run a tool preferring the cargo proxies if the sysroot exists.
-    pub fn tool(&self, tool: Tool) -> Command {
+    pub fn tool(&self, tool: Tool, current_dir: impl AsRef<Path>) -> Command {
         match self.root() {
             Some(root) => {
                 // special case rustc, we can look that up directly in the sysroot's bin folder
@@ -179,15 +179,15 @@ impl Sysroot {
                     if let Some(path) =
                         probe_for_binary(root.join("bin").join(Tool::Rustc.name()).into())
                     {
-                        return Command::new(path);
+                        return toolchain::command(path, current_dir);
                     }
                 }
 
-                let mut cmd = Command::new(tool.prefer_proxy());
+                let mut cmd = toolchain::command(tool.prefer_proxy(), current_dir);
                 cmd.env("RUSTUP_TOOLCHAIN", AsRef::<std::path::Path>::as_ref(root));
                 cmd
             }
-            _ => Command::new(tool.path()),
+            _ => toolchain::command(tool.path(), current_dir),
         }
     }
 
@@ -436,7 +436,7 @@ fn discover_sysroot_dir(
     current_dir: &AbsPath,
     extra_env: &FxHashMap<String, String>,
 ) -> Result<AbsPathBuf> {
-    let mut rustc = Command::new(Tool::Rustc.path());
+    let mut rustc = toolchain::command(Tool::Rustc.path(), current_dir);
     rustc.envs(extra_env);
     rustc.current_dir(current_dir).args(["--print", "sysroot"]);
     tracing::debug!("Discovering sysroot by {:?}", rustc);
@@ -468,9 +468,9 @@ fn discover_sysroot_src_dir_or_add_component(
 ) -> Result<AbsPathBuf> {
     discover_sysroot_src_dir(sysroot_path)
         .or_else(|| {
-            let mut rustup = Command::new(Tool::Rustup.prefer_proxy());
+            let mut rustup = toolchain::command(Tool::Rustup.prefer_proxy(), current_dir);
             rustup.envs(extra_env);
-            rustup.current_dir(current_dir).args(["component", "add", "rust-src"]);
+            rustup.args(["component", "add", "rust-src"]);
             tracing::info!("adding rust-src component by {:?}", rustup);
             utf8_stdout(&mut rustup).ok()?;
             get_rust_src(sysroot_path)

crates/project-model/src/toolchain_info/rustc_cfg.rs

@@ -45,9 +45,9 @@ fn rustc_print_cfg(
     const RUSTC_ARGS: [&str; 3] = ["--print", "cfg", "-O"];
     let sysroot = match config {
         QueryConfig::Cargo(sysroot, cargo_toml) => {
-            let mut cmd = sysroot.tool(Tool::Cargo);
+            let mut cmd = sysroot.tool(Tool::Cargo, cargo_toml.parent());
             cmd.envs(extra_env);
-            cmd.current_dir(cargo_toml.parent()).env("RUSTC_BOOTSTRAP", "1");
+            cmd.env("RUSTC_BOOTSTRAP", "1");
             cmd.args(["rustc", "-Z", "unstable-options"]).args(RUSTC_ARGS);
             if let Some(target) = target {
                 cmd.args(["--target", target]);
@@ -67,7 +67,7 @@ fn rustc_print_cfg(
         QueryConfig::Rustc(sysroot) => sysroot,
     };
 
-    let mut cmd = sysroot.tool(Tool::Rustc);
+    let mut cmd = sysroot.tool(Tool::Rustc, &std::env::current_dir()?);
     cmd.envs(extra_env);
     cmd.args(RUSTC_ARGS);
     if let Some(target) = target {

crates/project-model/src/toolchain_info/target_data_layout.rs

@@ -21,9 +21,9 @@ pub fn get(
     };
     let sysroot = match config {
         QueryConfig::Cargo(sysroot, cargo_toml) => {
-            let mut cmd = sysroot.tool(Tool::Cargo);
+            let mut cmd = sysroot.tool(Tool::Cargo, cargo_toml.parent());
             cmd.envs(extra_env);
-            cmd.current_dir(cargo_toml.parent()).env("RUSTC_BOOTSTRAP", "1");
+            cmd.env("RUSTC_BOOTSTRAP", "1");
             cmd.args(["rustc", "-Z", "unstable-options"]).args(RUSTC_ARGS).args([
                 "--",
                 "-Z",
@@ -43,7 +43,7 @@ pub fn get(
         QueryConfig::Rustc(sysroot) => sysroot,
     };
 
-    let mut cmd = Sysroot::tool(sysroot, Tool::Rustc);
+    let mut cmd = Sysroot::tool(sysroot, Tool::Rustc, &std::env::current_dir()?);
     cmd.envs(extra_env)
         .env("RUSTC_BOOTSTRAP", "1")
         .args(["-Z", "unstable-options"])

crates/project-model/src/toolchain_info/target_triple.rs

@@ -32,7 +32,7 @@ fn rustc_discover_host_triple(
     extra_env: &FxHashMap<String, String>,
     sysroot: &Sysroot,
 ) -> anyhow::Result<String> {
-    let mut cmd = sysroot.tool(Tool::Rustc);
+    let mut cmd = sysroot.tool(Tool::Rustc, &std::env::current_dir()?);
     cmd.envs(extra_env);
     cmd.arg("-vV");
     let stdout = utf8_stdout(&mut cmd)
@@ -52,7 +52,7 @@ fn cargo_config_build_target(
     extra_env: &FxHashMap<String, String>,
     sysroot: &Sysroot,
 ) -> Option<Vec<String>> {
-    let mut cmd = sysroot.tool(Tool::Cargo);
+    let mut cmd = sysroot.tool(Tool::Cargo, cargo_toml.parent());
     cmd.envs(extra_env);
     cmd.current_dir(cargo_toml.parent()).env("RUSTC_BOOTSTRAP", "1");
     cmd.args(["-Z", "unstable-options", "config", "get", "build.target"]);

crates/project-model/src/workspace.rs

@@ -176,9 +176,9 @@ fn get_toolchain_version(
     prefix: &str,
 ) -> Result<Option<Version>, anyhow::Error> {
     let cargo_version = utf8_stdout(&mut {
-        let mut cmd = Sysroot::tool(sysroot, tool);
+        let mut cmd = Sysroot::tool(sysroot, tool, current_dir);
         cmd.envs(extra_env);
-        cmd.arg("--version").current_dir(current_dir);
+        cmd.arg("--version");
         cmd
     })
     .with_context(|| format!("Failed to query rust toolchain version at {current_dir}, is your toolchain setup correctly?"))?;

crates/rust-analyzer/build.rs

@@ -32,6 +32,7 @@ fn set_rerun() {
 }
 
 fn set_commit_info() {
+    #[allow(clippy::disallowed_methods)]
     let output = match Command::new("git")
         .arg("log")
         .arg("-1")

crates/rust-analyzer/src/bin/rustc_wrapper.rs

@@ -46,6 +46,7 @@ fn run_rustc_skipping_cargo_checking(
 }
 
 fn run_rustc(rustc_executable: OsString, args: Vec<OsString>) -> io::Result<ExitCode> {
+    #[allow(clippy::disallowed_methods)]
     let mut child = Command::new(rustc_executable)
         .args(args)
         .stdin(Stdio::inherit())

crates/rust-analyzer/src/discover.rs

@@ -1,6 +1,6 @@
 //! Infrastructure for lazy project discovery. Currently only support rust-project.json discovery
 //! via a custom discover command.
-use std::{io, process::Command};
+use std::{io, path::Path};
 
 use crossbeam_channel::Sender;
 use paths::{AbsPathBuf, Utf8Path, Utf8PathBuf};
@@ -43,7 +43,11 @@ impl DiscoverCommand {
     }
 
     /// Spawn the command inside [Discover] and report progress, if any.
-    pub(crate) fn spawn(&self, discover_arg: DiscoverArgument) -> io::Result<DiscoverHandle> {
+    pub(crate) fn spawn(
+        &self,
+        discover_arg: DiscoverArgument,
+        current_dir: &Path,
+    ) -> io::Result<DiscoverHandle> {
         let command = &self.command[0];
         let args = &self.command[1..];
 
@@ -58,7 +62,7 @@ impl DiscoverCommand {
             })
             .collect();
 
-        let mut cmd = Command::new(command);
+        let mut cmd = toolchain::command(command, current_dir);
         cmd.args(args);
 
         Ok(DiscoverHandle {

crates/rust-analyzer/src/flycheck.rs

@@ -444,12 +444,11 @@ impl FlycheckActor {
     ) -> Option<Command> {
         match &self.config {
             FlycheckConfig::CargoCommand { command, options, ansi_color_output } => {
-                let mut cmd = Command::new(Tool::Cargo.path());
+                let mut cmd = toolchain::command(Tool::Cargo.path(), &*self.root);
                 if let Some(sysroot_root) = &self.sysroot_root {
                     cmd.env("RUSTUP_TOOLCHAIN", AsRef::<std::path::Path>::as_ref(sysroot_root));
                 }
                 cmd.arg(command);
-                cmd.current_dir(&*self.root);
 
                 match package {
                     Some(pkg) => cmd.arg("-p").arg(pkg),
@@ -486,18 +485,15 @@ impl FlycheckActor {
                 Some(cmd)
             }
             FlycheckConfig::CustomCommand { command, args, extra_env, invocation_strategy } => {
-                let mut cmd = Command::new(command);
-                cmd.envs(extra_env);
-
-                match invocation_strategy {
-                    InvocationStrategy::Once => {
-                        cmd.current_dir(&*self.root);
-                    }
+                let root = match invocation_strategy {
+                    InvocationStrategy::Once => &*self.root,
                     InvocationStrategy::PerWorkspace => {
-                        // FIXME: cmd.current_dir(&affected_workspace);
-                        cmd.current_dir(&*self.root);
+                        // FIXME: &affected_workspace
+                        &*self.root
                     }
-                }
+                };
+                let mut cmd = toolchain::command(command, root);
+                cmd.envs(extra_env);
 
                 // If the custom command has a $saved_file placeholder, and
                 // we're saving a file, replace the placeholder in the arguments.

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

@@ -1,12 +1,7 @@
 //! This module is responsible for implementing handlers for Language Server
 //! Protocol. This module specifically handles requests.
 
-use std::{
-    fs,
-    io::Write as _,
-    ops::Not,
-    process::{self, Stdio},
-};
+use std::{fs, io::Write as _, ops::Not, process::Stdio};
 
 use anyhow::Context;
 
@@ -2243,10 +2238,31 @@ fn run_rustfmt(
     let line_index = snap.file_line_index(file_id)?;
     let source_root_id = snap.analysis.source_root_id(file_id).ok();
 
+    // try to chdir to the file so we can respect `rustfmt.toml`
+    // FIXME: use `rustfmt --config-path` once
+    // https://github.com/rust-lang/rustfmt/issues/4660 gets fixed
+    let current_dir = match text_document.uri.to_file_path() {
+        Ok(mut path) => {
+            // pop off file name
+            if path.pop() && path.is_dir() {
+                path
+            } else {
+                std::env::current_dir()?
+            }
+        }
+        Err(_) => {
+            tracing::error!(
+                text_document = ?text_document.uri,
+                "Unable to get path, rustfmt.toml might be ignored"
+            );
+            std::env::current_dir()?
+        }
+    };
+
     let mut command = match snap.config.rustfmt(source_root_id) {
         RustfmtConfig::Rustfmt { extra_args, enable_range_formatting } => {
             // FIXME: Set RUSTUP_TOOLCHAIN
-            let mut cmd = process::Command::new(toolchain::Tool::Rustfmt.path());
+            let mut cmd = toolchain::command(toolchain::Tool::Rustfmt.path(), current_dir);
             cmd.envs(snap.config.extra_env(source_root_id));
             cmd.args(extra_args);
 
@@ -2300,9 +2316,9 @@ fn run_rustfmt(
                     } else {
                         cmd
                     };
-                    process::Command::new(cmd_path)
+                    toolchain::command(cmd_path, current_dir)
                 }
-                _ => process::Command::new(cmd),
+                _ => toolchain::command(cmd, current_dir),
             };
 
             cmd.envs(snap.config.extra_env(source_root_id));
@@ -2313,24 +2329,6 @@ fn run_rustfmt(
 
     tracing::debug!(?command, "created format command");
 
-    // try to chdir to the file so we can respect `rustfmt.toml`
-    // FIXME: use `rustfmt --config-path` once
-    // https://github.com/rust-lang/rustfmt/issues/4660 gets fixed
-    match text_document.uri.to_file_path() {
-        Ok(mut path) => {
-            // pop off file name
-            if path.pop() && path.is_dir() {
-                command.current_dir(path);
-            }
-        }
-        Err(_) => {
-            tracing::error!(
-                text_document = ?text_document.uri,
-                "Unable to get path, rustfmt.toml might be ignored"
-            );
-        }
-    }
-
     let mut rustfmt = command
         .stdin(Stdio::piped())
         .stdout(Stdio::piped())

crates/rust-analyzer/src/main_loop.rs

@@ -744,7 +744,8 @@ impl GlobalState {
                             DiscoverProjectParam::Path(it) => DiscoverArgument::Path(it),
                         };
 
-                        let handle = discover.spawn(arg).unwrap();
+                        let handle =
+                            discover.spawn(arg, &std::env::current_dir().unwrap()).unwrap();
                         self.discover_handle = Some(handle);
                     }
                 }

crates/rust-analyzer/src/test_runner.rs

@@ -1,8 +1,6 @@
 //! This module provides the functionality needed to run `cargo test` in a background
 //! thread and report the result of each test in a channel.
 
-use std::process::Command;
-
 use crossbeam_channel::Sender;
 use paths::AbsPath;
 use serde::Deserialize as _;
@@ -78,7 +76,7 @@ impl CargoTestHandle {
         test_target: TestTarget,
         sender: Sender<CargoTestMessage>,
     ) -> std::io::Result<Self> {
-        let mut cmd = Command::new(Tool::Cargo.path());
+        let mut cmd = toolchain::command(Tool::Cargo.path(), root);
         cmd.env("RUSTC_BOOTSTRAP", "1");
         cmd.arg("test");
 

crates/toolchain/src/lib.rs

@@ -1,6 +1,12 @@
 //! Discovery of `cargo` & `rustc` executables.
 
-use std::{env, iter, path::PathBuf};
+use std::{
+    env,
+    ffi::OsStr,
+    iter,
+    path::{Path, PathBuf},
+    process::Command,
+};
 
 use camino::{Utf8Path, Utf8PathBuf};
 
@@ -65,6 +71,14 @@ impl Tool {
     }
 }
 
+pub fn command(cmd: impl AsRef<OsStr>, working_directory: impl AsRef<Path>) -> Command {
+    // we are `toolchain::command``
+    #[allow(clippy::disallowed_methods)]
+    let mut cmd = Command::new(cmd);
+    cmd.current_dir(working_directory);
+    cmd
+}
+
 fn invoke(list: &[fn(&str) -> Option<Utf8PathBuf>], executable: &str) -> Utf8PathBuf {
     list.iter().find_map(|it| it(executable)).unwrap_or_else(|| executable.into())
 }

xtask/src/main.rs

@@ -9,7 +9,12 @@
 //! `.cargo/config`.
 
 #![warn(rust_2018_idioms, unused_lifetimes)]
-#![allow(clippy::print_stderr, clippy::print_stdout)]
+#![allow(
+    clippy::print_stderr,
+    clippy::print_stdout,
+    clippy::disallowed_methods,
+    clippy::disallowed_types
+)]
 
 mod flags;