Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the ability to configure the user config path #17930

Merged
merged 2 commits into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/rust-analyzer/src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion crates/rust-analyzer/src/cli/scip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ impl flags::Scip {
lsp_types::ClientCapabilities::default(),
vec![],
None,
None,
);

if let Some(p) = self.config_path {
Expand Down
70 changes: 31 additions & 39 deletions crates/rust-analyzer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand All @@ -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)>,

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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),
Expand All @@ -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(),
Expand Down Expand Up @@ -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!({
Expand All @@ -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" : {
Expand All @@ -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();

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

Expand All @@ -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!({
Expand All @@ -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!({
Expand All @@ -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();

Expand Down
1 change: 0 additions & 1 deletion crates/rust-analyzer/src/diagnostics/to_proto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,6 @@ mod tests {
ClientCapabilities::default(),
Vec::new(),
None,
None,
),
);
let snap = state.snapshot();
Expand Down
4 changes: 2 additions & 2 deletions crates/rust-analyzer/src/global_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion crates/rust-analyzer/src/reload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,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 {
Expand Down
12 changes: 4 additions & 8 deletions crates/rust-analyzer/tests/slow-tests/ratoml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -24,7 +25,6 @@ struct RatomlTest {
urls: Vec<Url>,
server: Server,
tmp_path: Utf8PathBuf,
user_config_dir: Utf8PathBuf,
}

impl RatomlTest {
Expand All @@ -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);
Expand All @@ -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
Expand All @@ -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);
}
Expand Down
34 changes: 22 additions & 12 deletions crates/rust-analyzer/tests/slow-tests/support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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<'_> {
Expand All @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -201,7 +200,6 @@ impl Project<'_> {
},
roots,
None,
self.user_config_path,
);
let mut change = ConfigChange::default();

Expand All @@ -213,7 +211,7 @@ impl Project<'_> {

config.rediscover_workspaces();

Server::new(tmp_dir.keep(), config)
Server::new(config_dir_guard, tmp_dir.keep(), config)
}
}

Expand All @@ -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 {
Expand Down
14 changes: 14 additions & 0 deletions crates/vfs/src/vfs_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down