Auto merge of #17930 - Veykril:config-user-config, r=alibektas

Remove the ability to configure the user config path

Being able to do this makes little sense as this is effectively a cyclic dependency (and we do not want to fixpoint this really).

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

@@ -227,7 +227,7 @@ fn run_server() -> anyhow::Result<()> {
         .filter(|workspaces| !workspaces.is_empty())
         .unwrap_or_else(|| vec![root_path.clone()]);
     let mut config =
-        Config::new(root_path, capabilities, workspace_roots, visual_studio_code_version, None);
+        Config::new(root_path, capabilities, workspace_roots, visual_studio_code_version);
     if let Some(json) = initialization_options {
         let mut change = ConfigChange::default();
         change.change_client_config(json);

crates/rust-analyzer/src/cli/scip.rs

@@ -37,7 +37,6 @@ impl flags::Scip {
             lsp_types::ClientCapabilities::default(),
             vec![],
             None,
-            None,
         );
 
         if let Some(p) = self.config_path {

crates/rust-analyzer/src/config.rs

@@ -3,10 +3,13 @@
 //! Of particular interest is the `feature_flags` hash map: while other fields
 //! configure the server itself, feature flags are passed into analysis, and
 //! tweak things like automatic insertion of `()` in completions.
-use std::{fmt, iter, ops::Not, sync::OnceLock};
+use std::{
+    env, fmt, iter,
+    ops::Not,
+    sync::{LazyLock, OnceLock},
+};
 
 use cfg::{CfgAtom, CfgDiff};
-use dirs::config_dir;
 use hir::Symbol;
 use ide::{
     AssistConfig, CallableSnippets, CompletionConfig, DiagnosticsConfig, ExprFillDefaultMode,
@@ -735,7 +738,6 @@ pub enum RatomlFileKind {
 }
 
 #[derive(Debug, Clone)]
-// FIXME @alibektas : Seems like a clippy warning of this sort should tell that combining different ConfigInputs into one enum was not a good idea.
 #[allow(clippy::large_enum_variant)]
 enum RatomlFile {
     Workspace(GlobalLocalConfigInput),
@@ -757,16 +759,6 @@ pub struct Config {
     /// by receiving a `lsp_types::notification::DidChangeConfiguration`.
     client_config: (FullConfigInput, ConfigErrors),
 
-    /// Path to the root configuration file. This can be seen as a generic way to define what would be `$XDG_CONFIG_HOME/rust-analyzer/rust-analyzer.toml` in Linux.
-    /// If not specified by init of a `Config` object this value defaults to :
-    ///
-    /// |Platform | Value                                 | Example                                  |
-    /// | ------- | ------------------------------------- | ---------------------------------------- |
-    /// | Linux   | `$XDG_CONFIG_HOME` or `$HOME`/.config | /home/alice/.config                      |
-    /// | macOS   | `$HOME`/Library/Application Support   | /Users/Alice/Library/Application Support |
-    /// | Windows | `{FOLDERID_RoamingAppData}`           | C:\Users\Alice\AppData\Roaming           |
-    user_config_path: VfsPath,
-
     /// Config node whose values apply to **every** Rust project.
     user_config: Option<(GlobalLocalConfigInput, ConfigErrors)>,
 
@@ -794,8 +786,25 @@ impl std::ops::Deref for Config {
 }
 
 impl Config {
-    pub fn user_config_path(&self) -> &VfsPath {
-        &self.user_config_path
+    /// Path to the root configuration file. This can be seen as a generic way to define what would be `$XDG_CONFIG_HOME/rust-analyzer/rust-analyzer.toml` in Linux.
+    /// This path is equal to:
+    ///
+    /// |Platform | Value                                 | Example                                  |
+    /// | ------- | ------------------------------------- | ---------------------------------------- |
+    /// | Linux   | `$XDG_CONFIG_HOME` or `$HOME`/.config | /home/alice/.config                      |
+    /// | macOS   | `$HOME`/Library/Application Support   | /Users/Alice/Library/Application Support |
+    /// | Windows | `{FOLDERID_RoamingAppData}`           | C:\Users\Alice\AppData\Roaming           |
+    pub fn user_config_path() -> Option<&'static AbsPath> {
+        static USER_CONFIG_PATH: LazyLock<Option<AbsPathBuf>> = LazyLock::new(|| {
+            let user_config_path = if let Some(path) = env::var_os("__TEST_RA_USER_CONFIG_DIR") {
+                std::path::PathBuf::from(path)
+            } else {
+                dirs::config_dir()?.join("rust-analyzer")
+            }
+            .join("rust-analyzer.toml");
+            Some(AbsPathBuf::assert_utf8(user_config_path))
+        });
+        USER_CONFIG_PATH.as_deref()
     }
 
     pub fn same_source_root_parent_map(
@@ -1315,24 +1324,8 @@ impl Config {
         caps: lsp_types::ClientCapabilities,
         workspace_roots: Vec<AbsPathBuf>,
         visual_studio_code_version: Option<Version>,
-        user_config_path: Option<Utf8PathBuf>,
     ) -> Self {
         static DEFAULT_CONFIG_DATA: OnceLock<&'static DefaultConfigData> = OnceLock::new();
-        let user_config_path = if let Some(user_config_path) = user_config_path {
-            user_config_path.join("rust-analyzer").join("rust-analyzer.toml")
-        } else {
-            let p = config_dir()
-                .expect("A config dir is expected to existed on all platforms ra supports.")
-                .join("rust-analyzer")
-                .join("rust-analyzer.toml");
-            Utf8PathBuf::from_path_buf(p).expect("Config dir expected to be abs.")
-        };
-
-        // A user config cannot be a virtual path as rust-analyzer cannot support watching changes in virtual paths.
-        // See `GlobalState::process_changes` to get more info.
-        // FIXME @alibektas : Temporary solution. I don't think this is right as at some point we may allow users to specify
-        // custom USER_CONFIG_PATHs which may also be relative.
-        let user_config_path = VfsPath::from(AbsPathBuf::assert(user_config_path));
 
         Config {
             caps: ClientCapabilities::new(caps),
@@ -1345,7 +1338,6 @@ impl Config {
             default_config: DEFAULT_CONFIG_DATA.get_or_init(|| Box::leak(Box::default())),
             source_root_parent_map: Arc::new(FxHashMap::default()),
             user_config: None,
-            user_config_path,
             detached_files: Default::default(),
             validation_errors: Default::default(),
             ratoml_file: Default::default(),
@@ -3417,7 +3409,7 @@ mod tests {
     #[test]
     fn proc_macro_srv_null() {
         let mut config =
-            Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
+            Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None);
 
         let mut change = ConfigChange::default();
         change.change_client_config(serde_json::json!({
@@ -3432,7 +3424,7 @@ mod tests {
     #[test]
     fn proc_macro_srv_abs() {
         let mut config =
-            Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
+            Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None);
         let mut change = ConfigChange::default();
         change.change_client_config(serde_json::json!({
         "procMacro" : {
@@ -3446,7 +3438,7 @@ mod tests {
     #[test]
     fn proc_macro_srv_rel() {
         let mut config =
-            Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
+            Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None);
 
         let mut change = ConfigChange::default();
 
@@ -3466,7 +3458,7 @@ mod tests {
     #[test]
     fn cargo_target_dir_unset() {
         let mut config =
-            Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
+            Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None);
 
         let mut change = ConfigChange::default();
 
@@ -3484,7 +3476,7 @@ mod tests {
     #[test]
     fn cargo_target_dir_subdir() {
         let mut config =
-            Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
+            Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None);
 
         let mut change = ConfigChange::default();
         change.change_client_config(serde_json::json!({
@@ -3502,7 +3494,7 @@ mod tests {
     #[test]
     fn cargo_target_dir_relative_dir() {
         let mut config =
-            Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
+            Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None);
 
         let mut change = ConfigChange::default();
         change.change_client_config(serde_json::json!({
@@ -3523,7 +3515,7 @@ mod tests {
     #[test]
     fn toml_unknown_key() {
         let config =
-            Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
+            Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None);
 
         let mut change = ConfigChange::default();
 

crates/rust-analyzer/src/diagnostics/to_proto.rs

@@ -548,7 +548,6 @@ mod tests {
                 ClientCapabilities::default(),
                 Vec::new(),
                 None,
-                None,
             ),
         );
         let snap = state.snapshot();

crates/rust-analyzer/src/global_state.rs

@@ -380,7 +380,7 @@ impl GlobalState {
             || !self.config.same_source_root_parent_map(&self.local_roots_parent_map)
         {
             let config_change = {
-                let user_config_path = self.config.user_config_path();
+                let user_config_path = Config::user_config_path();
                 let mut change = ConfigChange::default();
                 let db = self.analysis_host.raw_database();
 
@@ -399,7 +399,7 @@ impl GlobalState {
                     .collect_vec();
 
                 for (file_id, (_change_kind, vfs_path)) in modified_ratoml_files {
-                    if vfs_path == *user_config_path {
+                    if vfs_path.as_path() == user_config_path {
                         change.change_user_config(Some(db.file_text(file_id)));
                         continue;
                     }

crates/rust-analyzer/src/reload.rs

@@ -565,7 +565,7 @@ impl GlobalState {
                 };
 
             watchers.extend(
-                iter::once(self.config.user_config_path().as_path())
+                iter::once(Config::user_config_path())
                     .chain(self.workspaces.iter().map(|ws| ws.manifest().map(ManifestPath::as_ref)))
                     .flatten()
                     .map(|glob_pattern| lsp_types::FileSystemWatcher {

crates/rust-analyzer/tests/slow-tests/ratoml.rs

@@ -8,6 +8,7 @@ use lsp_types::{
 };
 use paths::Utf8PathBuf;
 
+use rust_analyzer::config::Config;
 use rust_analyzer::lsp::ext::{InternalTestingFetchConfig, InternalTestingFetchConfigParams};
 use serde_json::json;
 use test_utils::skip_slow_tests;
@@ -24,7 +25,6 @@ struct RatomlTest {
     urls: Vec<Url>,
     server: Server,
     tmp_path: Utf8PathBuf,
-    user_config_dir: Utf8PathBuf,
 }
 
 impl RatomlTest {
@@ -41,11 +41,7 @@ impl RatomlTest {
 
         let full_fixture = fixtures.join("\n");
 
-        let user_cnf_dir = TestDir::new();
-        let user_config_dir = user_cnf_dir.path().to_owned();
-
-        let mut project =
-            Project::with_fixture(&full_fixture).tmp_dir(tmp_dir).user_config_dir(user_cnf_dir);
+        let mut project = Project::with_fixture(&full_fixture).tmp_dir(tmp_dir);
 
         for root in roots {
             project = project.root(root);
@@ -57,7 +53,7 @@ impl RatomlTest {
 
         let server = project.server().wait_until_workspace_is_loaded();
 
-        let mut case = Self { urls: vec![], server, tmp_path, user_config_dir };
+        let mut case = Self { urls: vec![], server, tmp_path };
         let urls = fixtures.iter().map(|fixture| case.fixture_path(fixture)).collect::<Vec<_>>();
         case.urls = urls;
         case
@@ -81,7 +77,7 @@ impl RatomlTest {
         let mut spl = spl.into_iter();
         if let Some(first) = spl.next() {
             if first == "$$CONFIG_DIR$$" {
-                path = self.user_config_dir.clone();
+                path = Config::user_config_path().unwrap().to_path_buf().into();
             } else {
                 path = path.join(first);
             }

crates/rust-analyzer/tests/slow-tests/support.rs

@@ -8,6 +8,7 @@ use std::{
 use crossbeam_channel::{after, select, Receiver};
 use lsp_server::{Connection, Message, Notification, Request};
 use lsp_types::{notification::Exit, request::Shutdown, TextDocumentIdentifier, Url};
+use parking_lot::{Mutex, MutexGuard};
 use paths::{Utf8Path, Utf8PathBuf};
 use rust_analyzer::{
     config::{Config, ConfigChange, ConfigErrors},
@@ -27,7 +28,6 @@ pub(crate) struct Project<'a> {
     roots: Vec<Utf8PathBuf>,
     config: serde_json::Value,
     root_dir_contains_symlink: bool,
-    user_config_path: Option<Utf8PathBuf>,
 }
 
 impl Project<'_> {
@@ -51,15 +51,9 @@ impl Project<'_> {
                 }
             }),
             root_dir_contains_symlink: false,
-            user_config_path: None,
         }
     }
 
-    pub(crate) fn user_config_dir(mut self, config_path_dir: TestDir) -> Self {
-        self.user_config_path = Some(config_path_dir.path().to_owned());
-        self
-    }
-
     pub(crate) fn tmp_dir(mut self, tmp_dir: TestDir) -> Self {
         self.tmp_dir = Some(tmp_dir);
         self
@@ -91,6 +85,7 @@ impl Project<'_> {
     }
 
     pub(crate) fn server(self) -> Server {
+        static CONFIG_DIR_LOCK: Mutex<()> = Mutex::new(());
         let tmp_dir = self.tmp_dir.unwrap_or_else(|| {
             if self.root_dir_contains_symlink {
                 TestDir::new_symlink()
@@ -122,9 +117,13 @@ impl Project<'_> {
         assert!(mini_core.is_none());
         assert!(toolchain.is_none());
 
+        let mut config_dir_guard = None;
         for entry in fixture {
             if let Some(pth) = entry.path.strip_prefix("/$$CONFIG_DIR$$") {
-                let path = self.user_config_path.clone().unwrap().join(&pth['/'.len_utf8()..]);
+                if config_dir_guard.is_none() {
+                    config_dir_guard = Some(CONFIG_DIR_LOCK.lock());
+                }
+                let path = Config::user_config_path().unwrap().join(&pth['/'.len_utf8()..]);
                 fs::create_dir_all(path.parent().unwrap()).unwrap();
                 fs::write(path.as_path(), entry.text.as_bytes()).unwrap();
             } else {
@@ -201,7 +200,6 @@ impl Project<'_> {
             },
             roots,
             None,
-            self.user_config_path,
         );
         let mut change = ConfigChange::default();
 
@@ -213,7 +211,7 @@ impl Project<'_> {
 
         config.rediscover_workspaces();
 
-        Server::new(tmp_dir.keep(), config)
+        Server::new(config_dir_guard, tmp_dir.keep(), config)
     }
 }
 
@@ -228,18 +226,30 @@ pub(crate) struct Server {
     client: Connection,
     /// XXX: remove the tempdir last
     dir: TestDir,
+    _config_dir_guard: Option<MutexGuard<'static, ()>>,
 }
 
 impl Server {
-    fn new(dir: TestDir, config: Config) -> Server {
+    fn new(
+        config_dir_guard: Option<MutexGuard<'static, ()>>,
+        dir: TestDir,
+        config: Config,
+    ) -> Server {
         let (connection, client) = Connection::memory();
 
         let _thread = stdx::thread::Builder::new(stdx::thread::ThreadIntent::Worker)
             .name("test server".to_owned())
             .spawn(move || main_loop(config, connection).unwrap())
             .expect("failed to spawn a thread");
 
-        Server { req_id: Cell::new(1), dir, messages: Default::default(), client, _thread }
+        Server {
+            req_id: Cell::new(1),
+            dir,
+            messages: Default::default(),
+            client,
+            _thread,
+            _config_dir_guard: config_dir_guard,
+        }
     }
 
     pub(crate) fn doc_id(&self, rel_path: &str) -> TextDocumentIdentifier {

crates/vfs/src/vfs_path.rs

@@ -313,6 +313,20 @@ impl fmt::Debug for VfsPathRepr {
     }
 }
 
+impl PartialEq<AbsPath> for VfsPath {
+    fn eq(&self, other: &AbsPath) -> bool {
+        match &self.0 {
+            VfsPathRepr::PathBuf(lhs) => lhs == other,
+            VfsPathRepr::VirtualPath(_) => false,
+        }
+    }
+}
+impl PartialEq<VfsPath> for AbsPath {
+    fn eq(&self, other: &VfsPath) -> bool {
+        other == self
+    }
+}
+
 /// `/`-separated virtual path.
 ///
 /// This is used to describe files that do not reside on the file system.