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).
This commit is contained in:
bors 2024-08-20 11:10:55 +00:00
commit 0e8df6f873
9 changed files with 75 additions and 65 deletions

View file

@ -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);

View file

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

View file

@ -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();

View file

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

View file

@ -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;
}

View file

@ -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 {

View file

@ -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);
}

View file

@ -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,10 +226,15 @@ 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)
@ -239,7 +242,14 @@ impl Server {
.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 {

View file

@ -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.