For toolchain binaries ue the full path found in $PATH

7 files changed, 63 insertions, 67 deletions

diff --git a/Cargo.lock b/Cargo.lock
index 3667dec23a6..c6df491596a 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1336,7 +1336,6 @@ name = "proc-macro-test"
 version = "0.0.0"
 dependencies = [
  "cargo_metadata",
- "toolchain",
 ]
 
 [[package]]
diff --git a/crates/ide/src/expand_macro.rs b/crates/ide/src/expand_macro.rs
index 9eb5a9db625..4b0961cbbeb 100644
--- a/crates/ide/src/expand_macro.rs
+++ b/crates/ide/src/expand_macro.rs
@@ -189,7 +189,7 @@ fn _format(
     let &crate_id = db.relevant_crates(file_id).iter().next()?;
     let edition = db.crate_graph()[crate_id].edition;
 
-    let mut cmd = std::process::Command::new(toolchain::rustfmt());
+    let mut cmd = std::process::Command::new(toolchain::Tool::Rustfmt.path());
     cmd.arg("--edition");
     cmd.arg(edition.to_string());
 
diff --git a/crates/proc-macro-srv/proc-macro-test/Cargo.toml b/crates/proc-macro-srv/proc-macro-test/Cargo.toml
index 7977afb1cbd..7c6a1ba46b5 100644
--- a/crates/proc-macro-srv/proc-macro-test/Cargo.toml
+++ b/crates/proc-macro-srv/proc-macro-test/Cargo.toml
@@ -11,6 +11,3 @@ doctest = false
 
 [build-dependencies]
 cargo_metadata = "0.18.1"
-
-# local deps
-toolchain.workspace = true
diff --git a/crates/proc-macro-srv/proc-macro-test/build.rs b/crates/proc-macro-srv/proc-macro-test/build.rs
index ff62980e4ff..c76c201d69e 100644
--- a/crates/proc-macro-srv/proc-macro-test/build.rs
+++ b/crates/proc-macro-srv/proc-macro-test/build.rs
@@ -18,12 +18,12 @@ use cargo_metadata::Message;
 fn main() {
     println!("cargo:rerun-if-changed=imp");
 
+    let cargo = env::var_os("CARGO").unwrap_or_else(|| "cargo".into());
+
     let has_features = env::var_os("RUSTC_BOOTSTRAP").is_some()
-        || String::from_utf8(
-            Command::new(toolchain::cargo()).arg("--version").output().unwrap().stdout,
-        )
-        .unwrap()
-        .contains("nightly");
+        || String::from_utf8(Command::new(&cargo).arg("--version").output().unwrap().stdout)
+            .unwrap()
+            .contains("nightly");
 
     let out_dir = env::var_os("OUT_DIR").unwrap();
     let out_dir = Path::new(&out_dir);
@@ -66,7 +66,7 @@ fn main() {
 
     let target_dir = out_dir.join("target");
 
-    let mut cmd = Command::new(toolchain::cargo());
+    let mut cmd = Command::new(&cargo);
     cmd.current_dir(&staging_dir)
         .args(["build", "-p", "proc-macro-test-impl", "--message-format", "json"])
         // Explicit override the target directory to avoid using the same one which the parent
@@ -96,7 +96,7 @@ fn main() {
     let repr = format!("{name} {version}");
     // New Package Id Spec since rust-lang/cargo#13311
     let pkgid = String::from_utf8(
-        Command::new(toolchain::cargo())
+        Command::new(cargo)
             .current_dir(&staging_dir)
             .args(["pkgid", name])
             .output()
diff --git a/crates/project-model/src/sysroot.rs b/crates/project-model/src/sysroot.rs
index ea24393ed8a..a486a38510a 100644
--- a/crates/project-model/src/sysroot.rs
+++ b/crates/project-model/src/sysroot.rs
@@ -12,7 +12,7 @@ use itertools::Itertools;
 use la_arena::{Arena, Idx};
 use paths::{AbsPath, AbsPathBuf};
 use rustc_hash::FxHashMap;
-use toolchain::probe_for_binary;
+use toolchain::{probe_for_binary, Tool};
 
 use crate::{utf8_stdout, CargoConfig, CargoWorkspace, ManifestPath};
 
@@ -411,7 +411,7 @@ fn discover_sysroot_dir(
     current_dir: &AbsPath,
     extra_env: &FxHashMap<String, String>,
 ) -> Result<AbsPathBuf> {
-    let mut rustc = Command::new(toolchain::rustc());
+    let mut rustc = Command::new(Tool::Rustc.path());
     rustc.envs(extra_env);
     rustc.current_dir(current_dir).args(["--print", "sysroot"]);
     tracing::debug!("Discovering sysroot by {:?}", rustc);
@@ -443,7 +443,7 @@ fn discover_sysroot_src_dir_or_add_component(
 ) -> Result<AbsPathBuf> {
     discover_sysroot_src_dir(sysroot_path)
         .or_else(|| {
-            let mut rustup = Command::new(toolchain::rustup());
+            let mut rustup = Command::new(Tool::Rustup.prefer_proxy());
             rustup.envs(extra_env);
             rustup.current_dir(current_dir).args(["component", "add", "rust-src"]);
             tracing::info!("adding rust-src component by {:?}", rustup);
diff --git a/crates/rust-analyzer/src/handlers/request.rs b/crates/rust-analyzer/src/handlers/request.rs
index 0e005975fba..091f485db00 100644
--- a/crates/rust-analyzer/src/handlers/request.rs
+++ b/crates/rust-analyzer/src/handlers/request.rs
@@ -1937,7 +1937,7 @@ fn run_rustfmt(
     let mut command = match snap.config.rustfmt() {
         RustfmtConfig::Rustfmt { extra_args, enable_range_formatting } => {
             // FIXME: Set RUSTUP_TOOLCHAIN
-            let mut cmd = process::Command::new(toolchain::rustfmt());
+            let mut cmd = process::Command::new(toolchain::Tool::Rustfmt.path());
             cmd.envs(snap.config.extra_env());
             cmd.args(extra_args);
 
diff --git a/crates/toolchain/src/lib.rs b/crates/toolchain/src/lib.rs
index ae71b6700c0..e65a060672e 100644
--- a/crates/toolchain/src/lib.rs
+++ b/crates/toolchain/src/lib.rs
@@ -16,8 +16,42 @@ pub enum Tool {
 }
 
 impl Tool {
+    pub fn proxy(self) -> Option<PathBuf> {
+        cargo_proxy(self.name())
+    }
+
+    /// Return a `PathBuf` to use for the given executable.
+    ///
+    /// The current implementation checks three places for an executable to use:
+    /// 1) `$CARGO_HOME/bin/<executable_name>`
+    ///      where $CARGO_HOME defaults to ~/.cargo (see https://doc.rust-lang.org/cargo/guide/cargo-home.html)
+    ///      example: for cargo, this tries $CARGO_HOME/bin/cargo, or ~/.cargo/bin/cargo if $CARGO_HOME is unset.
+    ///      It seems that this is a reasonable place to try for cargo, rustc, and rustup
+    /// 2) Appropriate environment variable (erroring if this is set but not a usable executable)
+    ///      example: for cargo, this checks $CARGO environment variable; for rustc, $RUSTC; etc
+    /// 3) $PATH/`<executable_name>`
+    ///      example: for cargo, this tries all paths in $PATH with appended `cargo`, returning the
+    ///      first that exists
+    /// 4) If all else fails, we just try to use the executable name directly
+    pub fn prefer_proxy(self) -> PathBuf {
+        invoke(&[cargo_proxy, lookup_as_env_var, lookup_in_path], self.name())
+    }
+
+    /// Return a `PathBuf` to use for the given executable.
+    ///
+    /// The current implementation checks three places for an executable to use:
+    /// 1) Appropriate environment variable (erroring if this is set but not a usable executable)
+    ///      example: for cargo, this checks $CARGO environment variable; for rustc, $RUSTC; etc
+    /// 2) $PATH/`<executable_name>`
+    ///      example: for cargo, this tries all paths in $PATH with appended `cargo`, returning the
+    ///      first that exists
+    /// 3) `$CARGO_HOME/bin/<executable_name>`
+    ///      where $CARGO_HOME defaults to ~/.cargo (see https://doc.rust-lang.org/cargo/guide/cargo-home.html)
+    ///      example: for cargo, this tries $CARGO_HOME/bin/cargo, or ~/.cargo/bin/cargo if $CARGO_HOME is unset.
+    ///      It seems that this is a reasonable place to try for cargo, rustc, and rustup
+    /// 4) If all else fails, we just try to use the executable name directly
     pub fn path(self) -> PathBuf {
-        get_path_for_executable(self.name())
+        invoke(&[lookup_as_env_var, lookup_in_path, cargo_proxy], self.name())
     }
 
     pub fn path_in(self, path: &Path) -> Option<PathBuf> {
@@ -38,60 +72,21 @@ impl Tool {
     }
 }
 
-pub fn cargo() -> PathBuf {
-    get_path_for_executable("cargo")
-}
-
-pub fn rustc() -> PathBuf {
-    get_path_for_executable("rustc")
+fn invoke(list: &[fn(&str) -> Option<PathBuf>], executable: &str) -> PathBuf {
+    list.iter().find_map(|it| it(executable)).unwrap_or_else(|| executable.into())
 }
 
-pub fn rustup() -> PathBuf {
-    get_path_for_executable("rustup")
+/// Looks up the binary as its SCREAMING upper case in the env variables.
+fn lookup_as_env_var(executable_name: &str) -> Option<PathBuf> {
+    env::var_os(executable_name.to_ascii_uppercase()).map(Into::into)
 }
 
-pub fn rustfmt() -> PathBuf {
-    get_path_for_executable("rustfmt")
-}
-
-/// Return a `PathBuf` to use for the given executable.
-///
-/// E.g., `get_path_for_executable("cargo")` may return just `cargo` if that
-/// gives a valid Cargo executable; or it may return a full path to a valid
-/// Cargo.
-fn get_path_for_executable(executable_name: &'static str) -> PathBuf {
-    // The current implementation checks three places for an executable to use:
-    // 1) Appropriate environment variable (erroring if this is set but not a usable executable)
-    //      example: for cargo, this checks $CARGO environment variable; for rustc, $RUSTC; etc
-    // 2) `<executable_name>`
-    //      example: for cargo, this tries just `cargo`, which will succeed if `cargo` is on the $PATH
-    // 3) `$CARGO_HOME/bin/<executable_name>`
-    //      where $CARGO_HOME defaults to ~/.cargo (see https://doc.rust-lang.org/cargo/guide/cargo-home.html)
-    //      example: for cargo, this tries $CARGO_HOME/bin/cargo, or ~/.cargo/bin/cargo if $CARGO_HOME is unset.
-    //      It seems that this is a reasonable place to try for cargo, rustc, and rustup
-    let env_var = executable_name.to_ascii_uppercase();
-    if let Some(path) = env::var_os(env_var) {
-        return path.into();
-    }
-
-    if lookup_in_path(executable_name) {
-        return executable_name.into();
-    }
-
-    if let Some(mut path) = get_cargo_home() {
-        path.push("bin");
-        path.push(executable_name);
-        if let Some(path) = probe_for_binary(path) {
-            return path;
-        }
-    }
-
-    executable_name.into()
-}
-
-fn lookup_in_path(exec: &str) -> bool {
-    let paths = env::var_os("PATH").unwrap_or_default();
-    env::split_paths(&paths).map(|path| path.join(exec)).find_map(probe_for_binary).is_some()
+/// Looks up the binary in the cargo home directory if it exists.
+fn cargo_proxy(executable_name: &str) -> Option<PathBuf> {
+    let mut path = get_cargo_home()?;
+    path.push("bin");
+    path.push(executable_name);
+    probe_for_binary(path)
 }
 
 fn get_cargo_home() -> Option<PathBuf> {
@@ -107,6 +102,11 @@ fn get_cargo_home() -> Option<PathBuf> {
     None
 }
 
+fn lookup_in_path(exec: &str) -> Option<PathBuf> {
+    let paths = env::var_os("PATH").unwrap_or_default();
+    env::split_paths(&paths).map(|path| path.join(exec)).find_map(probe_for_binary)
+}
+
 pub fn probe_for_binary(path: PathBuf) -> Option<PathBuf> {
     let with_extension = match env::consts::EXE_EXTENSION {
         "" => None,