Repository: rust-lang/rust-analyzer (rust)

Issue:
VSCode should honor toolchain overrides that contain a rust-analyzer component for server spawning
We tend to drop support for stable releases somewhat frequently as its difficult to really support more than a handful of rust versions with a rolling release model. This means it becomes the job of the user to pin their rust-analyzer somehow to some version that fits their current roolchain install, that's not ideal. We should offer a setting that when enabled checks for a toolchain override in the workspace and attempts to use its rust-analyzer component if installed when spawning the server.

Addtionally we should probably attempt to let the server know its MSRV and inform the user if their toolchain looks too old.

Pull request:
feat: Use oldest rustup rust-analyzer when toolchain override is present
Selects a rust-toolchain declared RA based on its date. The earliest (oldest) RA wins and becomes the one that the workspace uses as a whole.

In terms of precedence:

nightly > stable-with-version > stable

With stable-with-version, we invoke the RA with a `--version` arg and attempt to extract a date. Given the same date as a nightly, the nightly RA will win.

Fixes #17663

Patch:
diff --git a/crates/hir-expand/src/builtin_fn_macro.rs b/crates/hir-expand/src/builtin_fn_macro.rs
index 2725bdb768..6272e1df7d 100644
--- a/crates/hir-expand/src/builtin_fn_macro.rs
+++ b/crates/hir-expand/src/builtin_fn_macro.rs
@@ -4,7 +4,7 @@ use base_db::AnchoredPath;
 use cfg::CfgExpr;
 use either::Either;
 use intern::{sym, Symbol};
-use mbe::{parse_exprs_with_sep, parse_to_token_tree, DelimiterKind};
+use mbe::{parse_exprs_with_sep, parse_to_token_tree};
 use span::{Edition, EditionedFileId, Span, SpanAnchor, SyntaxContextId, ROOT_ERASED_FILE_AST_ID};
 use stdx::format_to;
 use syntax::{
@@ -34,7 +34,7 @@ macro_rules! register_builtin {
         }
 
         impl BuiltinFnLikeExpander {
-            fn expander(&self) -> fn (&dyn ExpandDatabase, MacroCallId, &tt::Subtree, Span) -> ExpandResult<tt::Subtree>  {
+            pub fn expander(&self) -> fn (&dyn ExpandDatabase, MacroCallId, &tt::Subtree, Span) -> ExpandResult<tt::Subtree>  {
                 match *self {
                     $( BuiltinFnLikeExpander::$kind => $expand, )*
                 }
@@ -42,7 +42,7 @@ macro_rules! register_builtin {
         }
 
         impl EagerExpander {
-            fn expander(&self) -> fn (&dyn ExpandDatabase, MacroCallId, &tt::Subtree, Span) -> ExpandResult<tt::Subtree>  {
+            pub fn expander(&self) -> fn (&dyn ExpandDatabase, MacroCallId, &tt::Subtree, Span) -> ExpandResult<tt::Subtree>  {
                 match *self {
                     $( EagerExpander::$e_kind => $e_expand, )*
                 }
@@ -711,20 +711,6 @@ fn parse_string(tt: &tt::Subtree) -> Result<(Symbol, Span), ExpandError> {
                 kind: tt::LitKind::Str,
                 suffix: _,
             })) => Some((unescape_str(text), *span)),
-            // FIXME: We wrap expression fragments in parentheses which can break this expectation
-            // here
-            // Remove this once we handle none delims correctly
-            tt::TokenTree::Subtree(t) if t.delimiter.kind == DelimiterKind::Parenthesis => {
-                t.token_trees.first().and_then(|tt| match tt {
-                    tt::TokenTree::Leaf(tt::Leaf::Literal(tt::Literal {
-                        symbol: text,
-                        span,
-                        kind: tt::LitKind::Str,
-                        suffix: _,
-                    })) => Some((unescape_str(text), *span)),
-                    _ => None,
-                })
-            }
             _ => None,
         })
         .ok_or(mbe::ExpandError::ConversionError.into())
diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs
index c262fcae47..67b7adaf90 100644
--- a/crates/hir-expand/src/lib.rs
+++ b/crates/hir-expand/src/lib.rs
@@ -24,7 +24,6 @@ pub mod span_map;
 
 mod cfg_process;
 mod fixup;
-
 use attrs::collect_attrs;
 use rustc_hash::FxHashMap;
 use triomphe::Arc;
diff --git a/crates/hir-ty/src/method_resolution.rs b/crates/hir-ty/src/method_resolution.rs
index 8ba8071d36..c07bd08be4 100644
--- a/crates/hir-ty/src/method_resolution.rs
+++ b/crates/hir-ty/src/method_resolution.rs
@@ -30,8 +30,8 @@ use crate::{
     utils::all_super_traits,
     AdtId, Canonical, CanonicalVarKinds, DebruijnIndex, DynTyExt, ForeignDefId, GenericArgData,
     Goal, Guidance, InEnvironment, Interner, Mutability, Scalar, Solution, Substitution,
-    TraitEnvironment, TraitRef, TraitRefExt, Ty, TyBuilder, TyExt, TyKind, TyVariableKind,
-    VariableKind, WhereClause,
+    TraitEnvironment, TraitRef, TraitRefExt, Ty, TyBuilder, TyExt, TyKind, VariableKind,
+    WhereClause,
 };
 
 /// This is used as a key for indexing impls.
@@ -1083,11 +1083,6 @@ fn iterate_method_candidates_by_receiver(
     table.run_in_snapshot(|table| {
         let mut autoderef = autoderef::Autoderef::new(table, receiver_ty.clone(), true);
         while let Some((self_ty, _)) = autoderef.next() {
-            if matches!(self_ty.kind(Interner), TyKind::InferenceVar(_, TyVariableKind::General)) {
-                // don't try to resolve methods on unknown types
-                return ControlFlow::Continue(());
-            }
-
             iterate_trait_method_candidates(
                 &self_ty,
                 autoderef.table,
diff --git a/crates/ide-diagnostics/src/handlers/unresolved_field.rs b/crates/ide-diagnostics/src/handlers/unresolved_field.rs
index eb8eea69f6..467d4edbf2 100644
--- a/crates/ide-diagnostics/src/handlers/unresolved_field.rs
+++ b/crates/ide-diagnostics/src/handlers/unresolved_field.rs
@@ -152,12 +152,7 @@ fn add_field_to_struct_fix(
             } else {
                 Some(make::visibility_pub_crate())
             };
-
-            let field_name = match field_name.chars().next() {
-                Some(ch) if ch.is_numeric() => return None,
-                Some(_) => make::name(field_name),
-                None => return None,
-            };
+            let field_name = make::name(field_name);
 
             let (offset, record_field) = record_field_layout(
                 visibility,
@@ -183,12 +178,7 @@ fn add_field_to_struct_fix(
         None => {
             // Add a field list to the Unit Struct
             let mut src_change_builder = SourceChangeBuilder::new(struct_range.file_id);
-            let field_name = match field_name.chars().next() {
-                // FIXME : See match arm below regarding tuple structs.
-                Some(ch) if ch.is_numeric() => return None,
-                Some(_) => make::name(field_name),
-                None => return None,
-            };
+            let field_name = make::name(field_name);
             let visibility = if error_range.file_id == struct_range.file_id {
                 None
             } else {
@@ -284,7 +274,7 @@ mod tests {
     use crate::{
         tests::{
             check_diagnostics, check_diagnostics_with_config, check_diagnostics_with_disabled,
-            check_fix, check_no_fix,
+            check_fix,
         },
         DiagnosticsConfig,
     };
@@ -469,36 +459,4 @@ fn foo() {
             "#,
         );
     }
-
-    #[test]
-    fn no_fix_when_indexed() {
-        check_no_fix(
-            r#"
-            struct Kek {}
-impl Kek {
-    pub fn foo(self) {
-        self.$00
-    }
-}
-
-fn main() {}
-            "#,
-        )
-    }
-
-    #[test]
-    fn no_fix_when_without_field() {
-        check_no_fix(
-            r#"
-            struct Kek {}
-impl Kek {
-    pub fn foo(self) {
-        self.$0
-    }
-}
-
-fn main() {}
-            "#,
-        )
-    }
 }
diff --git a/docs/dev/architecture.md b/docs/dev/architecture.md
index 4f8723a936..f4e7263868 100644
--- a/docs/dev/architecture.md
+++ b/docs/dev/architecture.md
@@ -408,7 +408,7 @@ It has a much richer vocabulary of types than `ide`, but the basic testing setup
 For comparisons, we use the `expect` crate for snapshot testing.
 
 To test various analysis corner cases and avoid forgetting about old tests, we use so-called marks.
-See the [cov_mark](https://docs.rs/cov-mark/latest/cov_mark/) crate documentation for more.
+See the `marks` module in the `test_utils` crate for more.
 
 **Architecture Invariant:** rust-analyzer tests do not use libcore or libstd.
 All required library code must be a part of the tests.
diff --git a/editors/code/src/bootstrap.ts b/editors/code/src/bootstrap.ts
index 527edf19eb..2bcd9ca0a4 100644
--- a/editors/code/src/bootstrap.ts
+++ b/editors/code/src/bootstrap.ts
@@ -4,6 +4,7 @@ import type { Config } from "./config";
 import { type Env, log } from "./util";
 import type { PersistentState } from "./persistent_state";
 import { exec, spawnSync } from "child_process";
+import { TextDecoder } from "node:util";
 
 export async function bootstrap(
     context: vscode.ExtensionContext,
@@ -50,26 +51,35 @@ async function getServer(
         }
         return explicitPath;
     }
-    if (packageJson.releaseTag === null) return "rust-analyzer";
 
-    if (vscode.workspace.workspaceFolders?.length === 1) {
-        // otherwise check if there is a toolchain override for the current vscode workspace
-        // and if the toolchain of this override has a rust-analyzer component
-        // if so, use the rust-analyzer component
-        const toolchainTomlExists = await fileExists(
-            vscode.Uri.joinPath(vscode.workspace.workspaceFolders[0]!.uri, "rust-toolchain.toml"),
-        );
-        if (toolchainTomlExists) {
-            const res = spawnSync("rustup", ["which", "rust-analyzer"], {
-                encoding: "utf8",
-                env: { ...process.env },
-                cwd: vscode.workspace.workspaceFolders[0]!.uri.fsPath,
-            });
-            if (!res.error && res.status === 0) {
-                return res.stdout.trim();
+    let toolchainServerPath = undefined;
+    if (vscode.workspace.workspaceFolders) {
+        for (const workspaceFolder of vscode.workspace.workspaceFolders) {
+            // otherwise check if there is a toolchain override for the current vscode workspace
+            // and if the toolchain of this override has a rust-analyzer component
+            // if so, use the rust-analyzer component
+            const toolchainUri = vscode.Uri.joinPath(workspaceFolder.uri, "rust-toolchain.toml");
+            if (await hasToolchainFileWithRaDeclared(toolchainUri)) {
+                const res = spawnSync("rustup", ["which", "rust-analyzer"], {
+                    encoding: "utf8",
+                    env: { ...process.env },
+                    cwd: workspaceFolder.uri.fsPath,
+                });
+                if (!res.error && res.status === 0) {
+                    toolchainServerPath = earliestToolchainPath(
+                        toolchainServerPath,
+                        res.stdout.trim(),
+                        raVersionResolver,
+                    );
+                }
             }
         }
     }
+    if (toolchainServerPath) {
+        return toolchainServerPath;
+    }
+
+    if (packageJson.releaseTag === null) return "rust-analyzer";
 
     // finally, use the bundled one
     const ext = process.platform === "win32" ? ".exe" : "";
@@ -102,6 +112,57 @@ async function getServer(
     return undefined;
 }
 
+// Given a path to a rust-analyzer executable, resolve its version and return it.
+function raVersionResolver(path: string): string | undefined {
+    const res = spawnSync(path, ["--version"], {
+        encoding: "utf8",
+    });
+    if (!res.error && res.status === 0) {
+        return res.stdout;
+    } else {
+        return undefined;
+    }
+}
+
+// Given a path to two rust-analyzer executables, return the earliest one by date.
+function earliestToolchainPath(
+    path0: string | undefined,
+    path1: string,
+    raVersionResolver: (path: string) => string | undefined,
+): string {
+    if (path0) {
+        if (orderFromPath(path0, raVersionResolver) < orderFromPath(path1, raVersionResolver)) {
+            return path0;
+        } else {
+            return path1;
+        }
+    } else {
+        return path1;
+    }
+}
+
+// Further to extracting a date for comparison, determine the order of a toolchain as follows:
+//  Highest - nightly
+//  Medium  - versioned
+//  Lowest  - stable
+// Example paths:
+//  nightly   - /Users/myuser/.rustup/toolchains/nightly-2022-11-22-aarch64-apple-darwin/bin/rust-analyzer
+//  versioned - /Users/myuser/.rustup/toolchains/1.72.1-aarch64-apple-darwin/bin/rust-analyzer
+//  stable    - /Users/myuser/.rustup/toolchains/stable-aarch64-apple-darwin/bin/rust-analyzer
+function orderFromPath(
+    path: string,
+    raVersionResolver: (path: string) => string | undefined,
+): string {
+    const raVersion = raVersionResolver(path);
+    const raDate = raVersion?.match(/^rust-analyzer .*\(.* (\d{4}-\d{2}-\d{2})\)$/);
+    if (raDate?.length === 2) {
+        const precedence = path.includes("nightly-") ? "0" : "1";
+        return "0-" + raDate[1] + "/" + precedence;
+    } else {
+        return "2";
+    }
+}
+
 async function fileExists(uri: vscode.Uri) {
     return await vscode.workspace.fs.stat(uri).then(
         () => true,
@@ -109,6 +170,19 @@ async function fileExists(uri: vscode.Uri) {
     );
 }
 
+async function hasToolchainFileWithRaDeclared(uri: vscode.Uri): Promise<boolean> {
+    try {
+        const toolchainFileContents = new TextDecoder().decode(
+            await vscode.workspace.fs.readFile(uri),
+        );
+        return (
+            toolchainFileContents.match(/components\s*=\s*\[.*\"rust-analyzer\".*\]/g)?.length === 1
+        );
+    } catch (e) {
+        return false;
+    }
+}
+
 export function isValidExecutable(path: string, extraEnv: Env): boolean {
     log.debug("Checking availability of a binary at", path);
 
@@ -205,3 +279,8 @@ async function patchelf(dest: vscode.Uri): Promise<void> {
         },
     );
 }
+
+export const _private = {
+    earliestToolchainPath,
+    orderFromPath,
+};
diff --git a/editors/code/src/client.ts b/editors/code/src/client.ts
index bf58112916..357f131b93 100644
--- a/editors/code/src/client.ts
+++ b/editors/code/src/client.ts
@@ -392,18 +392,14 @@ function isCodeActionWithoutEditsAndCommands(value: any): boolean {
 // to proxy around that. We store the last hover's reference command link
 // here, as only one hover can be active at a time, and we don't need to
 // keep a history of these.
-export let HOVER_REFERENCE_COMMAND: ra.CommandLink[] = [];
+export let HOVER_REFERENCE_COMMAND: ra.CommandLink | undefined = undefined;
 
 function renderCommand(cmd: ra.CommandLink): string {
-    HOVER_REFERENCE_COMMAND.push(cmd);
-    return `[${cmd.title}](command:rust-analyzer.hoverRefCommandProxy?${
-        HOVER_REFERENCE_COMMAND.length - 1
-    } '${cmd.tooltip}')`;
+    HOVER_REFERENCE_COMMAND = cmd;
+    return `[${cmd.title}](command:rust-analyzer.hoverRefCommandProxy '${cmd.tooltip}')`;
 }
 
 function renderHoverActions(actions: ra.CommandLinkGroup[]): vscode.MarkdownString {
-    // clean up the previous hover ref command
-    HOVER_REFERENCE_COMMAND = [];
     const text = actions
         .map(
             (group) =>
diff --git a/editors/code/src/commands.ts b/editors/code/src/commands.ts
index 773a460601..621b969578 100644
--- a/editors/code/src/commands.ts
+++ b/editors/code/src/commands.ts
@@ -1203,10 +1203,9 @@ export function newDebugConfig(ctx: CtxInit): Cmd {
 }
 
 export function hoverRefCommandProxy(_: Ctx): Cmd {
-    return async (index: number) => {
-        const link = HOVER_REFERENCE_COMMAND[index];
-        if (link) {
-            const { command, arguments: args = [] } = link;
+    return async () => {
+        if (HOVER_REFERENCE_COMMAND) {
+            const { command, arguments: args = [] } = HOVER_REFERENCE_COMMAND;
             await vscode.commands.executeCommand(command, ...args);
         }
     };


Test patch:
diff --git a/crates/hir-ty/src/tests/method_resolution.rs b/crates/hir-ty/src/tests/method_resolution.rs
index 14e2e74653..360a129283 100644
--- a/crates/hir-ty/src/tests/method_resolution.rs
+++ b/crates/hir-ty/src/tests/method_resolution.rs
@@ -2099,42 +2099,3 @@ fn test() {
 "#,
     );
 }
-
-#[test]
-fn mismatched_args_due_to_supertraits_with_deref() {
-    check_no_mismatches(
-        r#"
-//- minicore: deref
-use core::ops::Deref;
-
-trait Trait1 {
-    type Assoc: Deref<Target = String>;
-}
-
-trait Trait2: Trait1 {
-}
-
-trait Trait3 {
-    type T1: Trait1;
-    type T2: Trait2;
-    fn bar(&self, x: bool, y: bool);
-}
-
-struct Foo;
-
-impl Foo {
-    fn bar(&mut self, _: &'static str) {}
-}
-
-impl Deref for Foo {
-    type Target = u32;
-    fn deref(&self) -> &Self::Target { &0 }
-}
-
-fn problem_method<T: Trait3>() {
-    let mut foo = Foo;
-    foo.bar("hello"); // Rustc ok, RA errors (mismatched args)
-}
-"#,
-    );
-}
diff --git a/crates/ide/src/syntax_highlighting/tests.rs b/crates/ide/src/syntax_highlighting/tests.rs
index 2070022d41..5f711600a2 100644
--- a/crates/ide/src/syntax_highlighting/tests.rs
+++ b/crates/ide/src/syntax_highlighting/tests.rs
@@ -1238,7 +1238,7 @@ fn benchmark_syntax_highlighting_parser() {
             })
             .count()
     };
-    assert_eq!(hash, 1167);
+    assert_eq!(hash, 1169);
 }
 
 #[test]
diff --git a/editors/code/tests/unit/bootstrap.test.ts b/editors/code/tests/unit/bootstrap.test.ts
new file mode 100644
index 0000000000..8aeb72180a
--- /dev/null
+++ b/editors/code/tests/unit/bootstrap.test.ts
@@ -0,0 +1,96 @@
+import * as assert from "assert";
+import { _private } from "../../src/bootstrap";
+import type { Context } from ".";
+
+export async function getTests(ctx: Context) {
+    await ctx.suite("Bootstrap/Select toolchain RA", (suite) => {
+        suite.addTest("Order of nightly RA", async () => {
+            assert.deepStrictEqual(
+                _private.orderFromPath(
+                    "/Users/myuser/.rustup/toolchains/nightly-2022-11-22-aarch64-apple-darwin/bin/rust-analyzer",
+                    function (path: string) {
+                        assert.deepStrictEqual(
+                            path,
+                            "/Users/myuser/.rustup/toolchains/nightly-2022-11-22-aarch64-apple-darwin/bin/rust-analyzer",
+                        );
+                        return "rust-analyzer 1.67.0-nightly (b7bc90fe 2022-11-21)";
+                    },
+                ),
+                "0-2022-11-21/0",
+            );
+        });
+
+        suite.addTest("Order of versioned RA", async () => {
+            assert.deepStrictEqual(
+                _private.orderFromPath(
+                    "/Users/myuser/.rustup/toolchains/1.72.1-aarch64-apple-darwin/bin/rust-analyzer",
+                    function (path: string) {
+                        assert.deepStrictEqual(
+                            path,
+                            "/Users/myuser/.rustup/toolchains/1.72.1-aarch64-apple-darwin/bin/rust-analyzer",
+                        );
+                        return "rust-analyzer 1.72.1 (d5c2e9c3 2023-09-13)";
+                    },
+                ),
+                "0-2023-09-13/1",
+            );
+        });
+
+        suite.addTest("Order of versioned RA when unable to obtain version date", async () => {
+            assert.deepStrictEqual(
+                _private.orderFromPath(
+                    "/Users/myuser/.rustup/toolchains/1.72.1-aarch64-apple-darwin/bin/rust-analyzer",
+                    function () {
+                        return "rust-analyzer 1.72.1";
+                    },
+                ),
+                "2",
+            );
+        });
+
+        suite.addTest("Order of stable RA", async () => {
+            assert.deepStrictEqual(
+                _private.orderFromPath(
+                    "/Users/myuser/.rustup/toolchains/stable-aarch64-apple-darwin/bin/rust-analyzer",
+                    function (path: string) {
+                        assert.deepStrictEqual(
+                            path,
+                            "/Users/myuser/.rustup/toolchains/stable-aarch64-apple-darwin/bin/rust-analyzer",
+                        );
+                        return "rust-analyzer 1.79.0 (129f3b99 2024-06-10)";
+                    },
+                ),
+                "0-2024-06-10/1",
+            );
+        });
+
+        suite.addTest("Order with invalid path to RA", async () => {
+            assert.deepStrictEqual(
+                _private.orderFromPath("some-weird-path", function () {
+                    return undefined;
+                }),
+                "2",
+            );
+        });
+
+        suite.addTest("Earliest RA between nightly and stable", async () => {
+            assert.deepStrictEqual(
+                _private.earliestToolchainPath(
+                    "/Users/myuser/.rustup/toolchains/stable-aarch64-apple-darwin/bin/rust-analyzer",
+                    "/Users/myuser/.rustup/toolchains/nightly-2022-11-22-aarch64-apple-darwin/bin/rust-analyzer",
+                    function (path: string) {
+                        if (
+                            path ===
+                            "/Users/myuser/.rustup/toolchains/nightly-2022-11-22-aarch64-apple-darwin/bin/rust-analyzer"
+                        ) {
+                            return "rust-analyzer 1.67.0-nightly (b7bc90fe 2022-11-21)";
+                        } else {
+                            return "rust-analyzer 1.79.0 (129f3b99 2024-06-10)";
+                        }
+                    },
+                ),
+                "/Users/myuser/.rustup/toolchains/nightly-2022-11-22-aarch64-apple-darwin/bin/rust-analyzer",
+            );
+        });
+    });
+}