From 23a5f31ff49c930d83fba5899f44a376748de504 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Thu, 18 Apr 2024 16:28:32 +0200 Subject: [PATCH 1/8] Apply suggested changes --- Cargo.lock | 10 + crates/ide/src/lib.rs | 9 +- crates/paths/src/lib.rs | 18 + crates/rust-analyzer/Cargo.toml | 1 + crates/rust-analyzer/src/bin/main.rs | 16 +- crates/rust-analyzer/src/cli/scip.rs | 10 +- crates/rust-analyzer/src/config.rs | 760 ++++++++---- crates/rust-analyzer/src/diagnostics.rs | 2 +- .../rust-analyzer/src/diagnostics/to_proto.rs | 1 + crates/rust-analyzer/src/global_state.rs | 108 +- .../src/handlers/notification.rs | 10 +- crates/rust-analyzer/src/handlers/request.rs | 26 +- crates/rust-analyzer/src/lib.rs | 2 +- crates/rust-analyzer/src/main_loop.rs | 5 + crates/rust-analyzer/src/reload.rs | 117 +- crates/rust-analyzer/src/tracing/config.rs | 1 + crates/rust-analyzer/tests/slow-tests/main.rs | 1048 ++++++++++++++++- .../rust-analyzer/tests/slow-tests/support.rs | 33 +- 18 files changed, 1878 insertions(+), 299 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3558c39bb32c..d150c31c48bf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -328,6 +328,15 @@ dependencies = [ "dirs-sys", ] +[[package]] +name = "dirs" +version = "5.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "44c45a9d03d6676652bcb5e724c7e988de1acad23a711b5217ab9cbecbec2225" +dependencies = [ + "dirs-sys", +] + [[package]] name = "dirs-sys" version = "0.4.1" @@ -1665,6 +1674,7 @@ dependencies = [ "anyhow", "cfg", "crossbeam-channel", + "dirs", "dissimilar", "expect-test", "flycheck", diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index 431aa30e56f2..e9408bf89766 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -273,10 +273,17 @@ impl Analysis { self.with_db(|db| status::status(db, file_id)) } - pub fn source_root(&self, file_id: FileId) -> Cancellable { + pub fn source_root_id(&self, file_id: FileId) -> Cancellable { self.with_db(|db| db.file_source_root(file_id)) } + pub fn is_local_source_root(&self, source_root_id: SourceRootId) -> Cancellable { + self.with_db(|db| { + let sr = db.source_root(source_root_id); + !sr.is_library + }) + } + pub fn parallel_prime_caches(&self, num_worker_threads: u8, cb: F) -> Cancellable<()> where F: Fn(ParallelPrimeCachesProgress) + Sync + std::panic::UnwindSafe, diff --git a/crates/paths/src/lib.rs b/crates/paths/src/lib.rs index 1dda02e3f10f..7d7cf0220e6d 100644 --- a/crates/paths/src/lib.rs +++ b/crates/paths/src/lib.rs @@ -135,6 +135,24 @@ impl AbsPathBuf { pub fn pop(&mut self) -> bool { self.0.pop() } + + /// Equivalent of [`PathBuf::push`] for `AbsPathBuf`. + /// + /// Extends `self` with `path`. + /// + /// If `path` is absolute, it replaces the current path. + /// + /// On Windows: + /// + /// * if `path` has a root but no prefix (e.g., `\windows`), it + /// replaces everything except for the prefix (if any) of `self`. + /// * if `path` has a prefix but no root, it replaces `self`. + /// * if `self` has a verbatim prefix (e.g. `\\?\C:\windows`) + /// and `path` is not empty, the new path is normalized: all references + /// to `.` and `..` are removed. + pub fn push(&mut self, suffix: &str) { + self.0.push(suffix) + } } impl fmt::Display for AbsPathBuf { diff --git a/crates/rust-analyzer/Cargo.toml b/crates/rust-analyzer/Cargo.toml index 34b3e4931403..8ff7235b8fa5 100644 --- a/crates/rust-analyzer/Cargo.toml +++ b/crates/rust-analyzer/Cargo.toml @@ -22,6 +22,7 @@ path = "src/bin/main.rs" [dependencies] anyhow.workspace = true crossbeam-channel = "0.5.5" +dirs = "5.0.1" dissimilar.workspace = true itertools.workspace = true scip = "0.3.3" diff --git a/crates/rust-analyzer/src/bin/main.rs b/crates/rust-analyzer/src/bin/main.rs index 9daae914d79c..7e58cd70c494 100644 --- a/crates/rust-analyzer/src/bin/main.rs +++ b/crates/rust-analyzer/src/bin/main.rs @@ -15,7 +15,11 @@ use std::{env, fs, path::PathBuf, process::ExitCode, sync::Arc}; use anyhow::Context; use lsp_server::Connection; -use rust_analyzer::{cli::flags, config::Config, from_json}; +use rust_analyzer::{ + cli::flags, + config::{Config, ConfigChange, ConfigError}, + from_json, +}; use semver::Version; use tracing_subscriber::fmt::writer::BoxMakeWriter; use vfs::AbsPathBuf; @@ -220,16 +224,20 @@ 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); + Config::new(root_path, capabilities, workspace_roots, visual_studio_code_version, None); if let Some(json) = initialization_options { - if let Err(e) = config.update(json) { + let mut change = ConfigChange::default(); + change.change_client_config(json); + let mut error_sink = ConfigError::default(); + config = config.apply_change(change, &mut error_sink); + if !error_sink.is_empty() { use lsp_types::{ notification::{Notification, ShowMessage}, MessageType, ShowMessageParams, }; let not = lsp_server::Notification::new( ShowMessage::METHOD.to_owned(), - ShowMessageParams { typ: MessageType::WARNING, message: e.to_string() }, + ShowMessageParams { typ: MessageType::WARNING, message: error_sink.to_string() }, ); connection.sender.send(lsp_server::Message::Notification(not)).unwrap(); } diff --git a/crates/rust-analyzer/src/cli/scip.rs b/crates/rust-analyzer/src/cli/scip.rs index aef2c1be2249..b2d70562890f 100644 --- a/crates/rust-analyzer/src/cli/scip.rs +++ b/crates/rust-analyzer/src/cli/scip.rs @@ -10,9 +10,11 @@ use ide_db::LineIndexDatabase; use load_cargo::{load_workspace_at, LoadCargoConfig, ProcMacroServerChoice}; use rustc_hash::{FxHashMap, FxHashSet}; use scip::types as scip_types; +use tracing::error; use crate::{ cli::flags, + config::{ConfigChange, ConfigError}, line_index::{LineEndings, LineIndex, PositionEncoding}, }; @@ -35,12 +37,18 @@ impl flags::Scip { lsp_types::ClientCapabilities::default(), vec![], None, + None, ); if let Some(p) = self.config_path { let mut file = std::io::BufReader::new(std::fs::File::open(p)?); let json = serde_json::from_reader(&mut file)?; - config.update(json)?; + let mut change = ConfigChange::default(); + change.change_client_config(json); + let mut error_sink = ConfigError::default(); + config = config.apply_change(change, &mut error_sink); + // FIXME @alibektas : What happens to errors without logging? + error!(?error_sink, "Config Error(s)"); } let cargo_config = config.cargo(); let (db, vfs, _) = load_workspace_at( diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index a8d1e72aed9f..51664dd799eb 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -1,14 +1,12 @@ //! Config used by the language server. //! -//! We currently get this config from `initialize` LSP request, which is not the -//! best way to do it, but was the simplest thing we could implement. -//! //! 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}; use cfg::{CfgAtom, CfgDiff}; +use dirs::config_dir; use flycheck::{CargoOptions, FlycheckConfig}; use ide::{ AssistConfig, CallableSnippets, CompletionConfig, DiagnosticsConfig, ExprFillDefaultMode, @@ -29,9 +27,14 @@ use project_model::{ }; use rustc_hash::{FxHashMap, FxHashSet}; use semver::Version; -use serde::{de::DeserializeOwned, Deserialize, Serialize}; +use serde::{ + de::{DeserializeOwned, Error}, + ser::SerializeStruct, + Deserialize, Serialize, +}; use stdx::format_to_acc; -use vfs::{AbsPath, AbsPathBuf}; +use triomphe::Arc; +use vfs::{AbsPath, AbsPathBuf, FileId, VfsPath}; use crate::{ caps::completion_item_edit_resolve, @@ -67,12 +70,6 @@ config_data! { /// /// A config is searched for by traversing a "config tree" in a bottom up fashion. It is chosen by the nearest first principle. global: struct GlobalDefaultConfigData <- GlobalConfigInput -> { - /// Whether to insert #[must_use] when generating `as_` methods - /// for enum variants. - assist_emitMustUse: bool = false, - /// Placeholder expression to use for missing expressions in assists. - assist_expressionFillDefault: ExprFillDefaultDef = ExprFillDefaultDef::Todo, - /// Warm up caches on project load. cachePriming_enable: bool = true, /// How many worker threads to handle priming caches. The default `0` means to pick automatically. @@ -250,6 +247,71 @@ config_data! { /// If false, `-p ` will be passed instead. check_workspace: bool = true, + + /// Toggles the additional completions that automatically add imports when completed. + /// Note that your client must specify the `additionalTextEdits` LSP client capability to truly have this feature enabled. + completion_autoimport_enable: bool = true, + /// Toggles the additional completions that automatically show method calls and field accesses + /// with `self` prefixed to them when inside a method. + completion_autoself_enable: bool = true, + /// Whether to add parenthesis and argument snippets when completing function. + completion_callable_snippets: CallableCompletionDef = CallableCompletionDef::FillArguments, + /// Whether to show full function/method signatures in completion docs. + completion_fullFunctionSignatures_enable: bool = false, + /// Maximum number of completions to return. If `None`, the limit is infinite. + completion_limit: Option = None, + /// Whether to show postfix snippets like `dbg`, `if`, `not`, etc. + completion_postfix_enable: bool = true, + /// Enables completions of private items and fields that are defined in the current workspace even if they are not visible at the current position. + completion_privateEditable_enable: bool = false, + /// Custom completion snippets. + // NOTE: we use IndexMap for deterministic serialization ordering + completion_snippets_custom: IndexMap = serde_json::from_str(r#"{ + "Arc::new": { + "postfix": "arc", + "body": "Arc::new(${receiver})", + "requires": "std::sync::Arc", + "description": "Put the expression into an `Arc`", + "scope": "expr" + }, + "Rc::new": { + "postfix": "rc", + "body": "Rc::new(${receiver})", + "requires": "std::rc::Rc", + "description": "Put the expression into an `Rc`", + "scope": "expr" + }, + "Box::pin": { + "postfix": "pinbox", + "body": "Box::pin(${receiver})", + "requires": "std::boxed::Box", + "description": "Put the expression into a pinned `Box`", + "scope": "expr" + }, + "Ok": { + "postfix": "ok", + "body": "Ok(${receiver})", + "description": "Wrap the expression in a `Result::Ok`", + "scope": "expr" + }, + "Err": { + "postfix": "err", + "body": "Err(${receiver})", + "description": "Wrap the expression in a `Result::Err`", + "scope": "expr" + }, + "Some": { + "postfix": "some", + "body": "Some(${receiver})", + "description": "Wrap the expression in an `Option::Some`", + "scope": "expr" + } + }"#).unwrap(), + /// Whether to enable term search based snippets like `Some(foo.bar().baz())`. + completion_termSearch_enable: bool = false, + /// Term search fuel in "units of work" for autocompletion (Defaults to 200). + completion_termSearch_fuel: usize = 200, + /// List of rust-analyzer diagnostics to disable. diagnostics_disabled: FxHashSet = FxHashSet::default(), /// Whether to show native rust-analyzer diagnostics. @@ -451,76 +513,16 @@ config_data! { } config_data! { - /// Local configurations can be overridden for every crate by placing a `rust-analyzer.toml` on crate root. - /// A config is searched for by traversing a "config tree" in a bottom up fashion. It is chosen by the nearest first principle. + /// Local configurations can be defined per `SourceRoot`. This almost always corresponds to a `Crate`. local: struct LocalDefaultConfigData <- LocalConfigInput -> { + /// Whether to insert #[must_use] when generating `as_` methods + /// for enum variants. + assist_emitMustUse: bool = false, + /// Placeholder expression to use for missing expressions in assists. + assist_expressionFillDefault: ExprFillDefaultDef = ExprFillDefaultDef::Todo, /// Term search fuel in "units of work" for assists (Defaults to 400). assist_termSearch_fuel: usize = 400, - /// Toggles the additional completions that automatically add imports when completed. - /// Note that your client must specify the `additionalTextEdits` LSP client capability to truly have this feature enabled. - completion_autoimport_enable: bool = true, - /// Toggles the additional completions that automatically show method calls and field accesses - /// with `self` prefixed to them when inside a method. - completion_autoself_enable: bool = true, - /// Whether to add parenthesis and argument snippets when completing function. - completion_callable_snippets: CallableCompletionDef = CallableCompletionDef::FillArguments, - /// Whether to show full function/method signatures in completion docs. - completion_fullFunctionSignatures_enable: bool = false, - /// Maximum number of completions to return. If `None`, the limit is infinite. - completion_limit: Option = None, - /// Whether to show postfix snippets like `dbg`, `if`, `not`, etc. - completion_postfix_enable: bool = true, - /// Enables completions of private items and fields that are defined in the current workspace even if they are not visible at the current position. - completion_privateEditable_enable: bool = false, - /// Custom completion snippets. - // NOTE: we use IndexMap for deterministic serialization ordering - completion_snippets_custom: IndexMap = serde_json::from_str(r#"{ - "Arc::new": { - "postfix": "arc", - "body": "Arc::new(${receiver})", - "requires": "std::sync::Arc", - "description": "Put the expression into an `Arc`", - "scope": "expr" - }, - "Rc::new": { - "postfix": "rc", - "body": "Rc::new(${receiver})", - "requires": "std::rc::Rc", - "description": "Put the expression into an `Rc`", - "scope": "expr" - }, - "Box::pin": { - "postfix": "pinbox", - "body": "Box::pin(${receiver})", - "requires": "std::boxed::Box", - "description": "Put the expression into a pinned `Box`", - "scope": "expr" - }, - "Ok": { - "postfix": "ok", - "body": "Ok(${receiver})", - "description": "Wrap the expression in a `Result::Ok`", - "scope": "expr" - }, - "Err": { - "postfix": "err", - "body": "Err(${receiver})", - "description": "Wrap the expression in a `Result::Err`", - "scope": "expr" - }, - "Some": { - "postfix": "some", - "body": "Some(${receiver})", - "description": "Wrap the expression in an `Option::Some`", - "scope": "expr" - } - }"#).unwrap(), - /// Whether to enable term search based snippets like `Some(foo.bar().baz())`. - completion_termSearch_enable: bool = false, - /// Term search fuel in "units of work" for autocompletion (Defaults to 200). - completion_termSearch_fuel: usize = 200, - /// Enables highlighting of related references while the cursor is on `break`, `loop`, `while`, or `for` keywords. highlightRelated_breakPoints_enable: bool = true, /// Enables highlighting of all captures of a closure while the cursor is on the `|` or move keyword of a closure. @@ -659,23 +661,304 @@ pub struct Config { workspace_roots: Vec, caps: lsp_types::ClientCapabilities, root_path: AbsPathBuf, - detached_files: Vec, snippets: Vec, visual_studio_code_version: Option, default_config: DefaultConfigData, - client_config: FullConfigInput, - user_config: GlobalLocalConfigInput, - #[allow(dead_code)] + /// Config node that obtains its initial value during the server initialization and + /// by receiving a `lsp_types::notification::DidChangeConfiguration`. + client_config: ClientConfig, + + /// 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, + + /// FIXME @alibektas : Change this to sth better. + /// Config node whose values apply to **every** Rust project. + user_config: Option, + + /// A special file for this session whose path is set to `self.root_path.join("rust-analyzer.toml")` + root_ratoml_path: VfsPath, + + /// This file can be used to make global changes while having only a workspace-wide scope. + root_ratoml: Option, + + /// For every `SourceRoot` there can be at most one RATOML file. ratoml_files: FxHashMap, + + /// Clone of the value that is stored inside a `GlobalState`. + source_root_parent_map: Arc>, + + /// Changes made to client and global configurations will partially not be reflected even after `.apply_change()` was called. + /// This field signals that the `GlobalState` should call its `update_configuration()` method. + should_update: bool, } #[derive(Clone, Debug)] struct RatomlNode { - #[allow(dead_code)] node: GlobalLocalConfigInput, + file_id: FileId, +} + +#[derive(Debug, Clone, Default)] +struct ClientConfig { + node: FullConfigInput, + detached_files: Vec, +} + +impl Serialize for RatomlNode { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + let mut s = serializer.serialize_struct("RatomlNode", 2)?; + s.serialize_field("file_id", &self.file_id.index())?; + s.serialize_field("config", &self.node)?; + s.end() + } +} + +#[derive(Debug, Hash, Eq, PartialEq)] +pub(crate) enum ConfigNodeKey { + Ratoml(SourceRootId), + Client, + User, +} + +impl Serialize for ConfigNodeKey { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + match self { + ConfigNodeKey::Ratoml(source_root_id) => serializer.serialize_u32(source_root_id.0), + ConfigNodeKey::Client => serializer.serialize_str("client"), + ConfigNodeKey::User => serializer.serialize_str("user"), + } + } +} + +#[derive(Debug, Serialize)] +enum ConfigNodeValue<'a> { + /// `rust-analyzer::config` module works by setting + /// a mapping between `SourceRootId` and `ConfigInput`. + /// Storing a `FileId` is mostly for debugging purposes. + Ratoml(&'a RatomlNode), + Client(&'a FullConfigInput), +} + +impl Config { + /// FIXME @alibektas : Before integration tests, I thought I would + /// get the debug output of the config tree and do assertions based on it. + /// The reason why I didn't delete this is that we may want to have a lsp_ext + /// like "DebugConfigTree" so that it is easier for users to get a snapshot of + /// the config state for us to debug. #[allow(dead_code)] - parent: Option, + /// Walk towards the root starting from a specified `ConfigNode` + fn traverse( + &self, + start: ConfigNodeKey, + ) -> impl Iterator)> { + let mut v = vec![]; + + if let ConfigNodeKey::Ratoml(start) = start { + let mut par: Option = Some(start); + while let Some(source_root_id) = par { + par = self.source_root_parent_map.get(&start).copied(); + if let Some(config) = self.ratoml_files.get(&source_root_id) { + v.push(( + ConfigNodeKey::Ratoml(source_root_id), + ConfigNodeValue::Ratoml(config), + )); + } + } + } + + v.push((ConfigNodeKey::Client, ConfigNodeValue::Client(&self.client_config.node))); + + if let Some(user_config) = self.user_config.as_ref() { + v.push((ConfigNodeKey::User, ConfigNodeValue::Ratoml(user_config))); + } + + v.into_iter() + } + + pub fn user_config_path(&self) -> &VfsPath { + &self.user_config_path + } + + pub fn should_update(&self) -> bool { + self.should_update + } + + // FIXME @alibektas : Server's health uses error sink but in other places it is not used atm. + pub fn apply_change(&self, change: ConfigChange, error_sink: &mut ConfigError) -> Config { + let mut config = self.clone(); + let mut toml_errors = vec![]; + let mut json_errors = vec![]; + + config.should_update = false; + + if let Some((file_id, change)) = change.user_config_change { + config.user_config = Some(RatomlNode { + file_id, + node: GlobalLocalConfigInput::from_toml( + toml::from_str(change.to_string().as_str()).unwrap(), + &mut toml_errors, + ), + }); + config.should_update = true; + } + + if let Some(mut json) = change.client_config_change { + tracing::info!("updating config from JSON: {:#}", json); + if !(json.is_null() || json.as_object().map_or(false, |it| it.is_empty())) { + let detached_files = get_field::>( + &mut json, + &mut json_errors, + "detachedFiles", + None, + ) + .unwrap_or_default() + .into_iter() + .map(AbsPathBuf::assert) + .collect(); + + patch_old_style::patch_json_for_outdated_configs(&mut json); + + config.client_config = ClientConfig { + node: FullConfigInput::from_json(json, &mut json_errors), + detached_files, + } + } + config.should_update = true; + } + + if let Some((file_id, change)) = change.root_ratoml_change { + config.root_ratoml = Some(RatomlNode { + file_id, + node: GlobalLocalConfigInput::from_toml( + toml::from_str(change.to_string().as_str()).unwrap(), + &mut toml_errors, + ), + }); + config.should_update = true; + } + + if let Some(change) = change.ratoml_file_change { + for (source_root_id, (file_id, _, text)) in change { + if let Some(text) = text { + config.ratoml_files.insert( + source_root_id, + RatomlNode { + file_id, + node: GlobalLocalConfigInput::from_toml( + toml::from_str(&text).unwrap(), + &mut toml_errors, + ), + }, + ); + } + } + } + + if let Some(source_root_map) = change.source_map_change { + config.source_root_parent_map = source_root_map; + } + + let snips = self.completion_snippets_custom().to_owned(); + + for (name, def) in snips.iter() { + if def.prefix.is_empty() && def.postfix.is_empty() { + continue; + } + let scope = match def.scope { + SnippetScopeDef::Expr => SnippetScope::Expr, + SnippetScopeDef::Type => SnippetScope::Type, + SnippetScopeDef::Item => SnippetScope::Item, + }; + match Snippet::new( + &def.prefix, + &def.postfix, + &def.body, + def.description.as_ref().unwrap_or(name), + &def.requires, + scope, + ) { + Some(snippet) => config.snippets.push(snippet), + None => error_sink.0.push(ConfigErrorInner::JsonError( + format!("snippet {name} is invalid"), + ::custom( + "snippet path is invalid or triggers are missing", + ), + )), + } + } + + if config.check_command().is_empty() { + error_sink.0.push(ConfigErrorInner::JsonError( + "/check/command".to_owned(), + serde_json::Error::custom("expected a non-empty string"), + )); + } + config + } +} + +#[derive(Default, Debug)] +pub struct ConfigChange { + user_config_change: Option<(FileId, String)>, + root_ratoml_change: Option<(FileId, String)>, + client_config_change: Option, + ratoml_file_change: Option)>>, + source_map_change: Option>>, +} + +impl ConfigChange { + pub fn change_ratoml( + &mut self, + source_root: SourceRootId, + file_id: FileId, + vfs_path: VfsPath, + content: Option, + ) -> Option<(FileId, VfsPath, Option)> { + if let Some(changes) = self.ratoml_file_change.as_mut() { + changes.insert(source_root, (file_id, vfs_path, content)) + } else { + let mut map = FxHashMap::default(); + map.insert(source_root, (file_id, vfs_path, content)); + self.ratoml_file_change = Some(map); + None + } + } + + pub fn change_user_config(&mut self, content: Option<(FileId, String)>) { + assert!(self.user_config_change.is_none()); // Otherwise it is a double write. + self.user_config_change = content; + } + + pub fn change_root_ratoml(&mut self, content: Option<(FileId, String)>) { + assert!(self.user_config_change.is_none()); // Otherwise it is a double write. + self.root_ratoml_change = content; + } + + pub fn change_client_config(&mut self, change: serde_json::Value) { + self.client_config_change = Some(change); + } + + pub fn change_source_root_parent_map( + &mut self, + source_root_map: Arc>, + ) { + assert!(self.source_map_change.is_none()); + self.source_map_change = Some(source_root_map.clone()); + } } macro_rules! try_ { @@ -866,23 +1149,37 @@ pub struct ClientCommandsConfig { } #[derive(Debug)] -pub struct ConfigError { - errors: Vec<(String, serde_json::Error)>, +pub enum ConfigErrorInner { + JsonError(String, serde_json::Error), + Toml(String, toml::de::Error), } +#[derive(Debug, Default)] +pub struct ConfigError(Vec); + +impl ConfigError { + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } +} + +impl ConfigError {} + impl fmt::Display for ConfigError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let errors = self.errors.iter().format_with("\n", |(key, e), f| { - f(key)?; - f(&": ")?; - f(e) + let errors = self.0.iter().format_with("\n", |inner, f| match inner { + ConfigErrorInner::JsonError(key, e) => { + f(key)?; + f(&": ")?; + f(e) + } + ConfigErrorInner::Toml(key, e) => { + f(key)?; + f(&": ")?; + f(e) + } }); - write!( - f, - "invalid config value{}:\n{}", - if self.errors.len() == 1 { "" } else { "s" }, - errors - ) + write!(f, "invalid config value{}:\n{}", if self.0.len() == 1 { "" } else { "s" }, errors) } } @@ -894,19 +1191,45 @@ impl Config { caps: ClientCapabilities, workspace_roots: Vec, visual_studio_code_version: Option, + user_config_path: Option, ) -> Self { + 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)); + let root_ratoml_path = { + let mut p = root_path.clone(); + p.push("rust-analyzer.toml"); + VfsPath::new_real_path(p.to_string()) + }; + Config { caps, - detached_files: Vec::new(), discovered_projects: Vec::new(), root_path, snippets: Default::default(), workspace_roots, visual_studio_code_version, - client_config: FullConfigInput::default(), - user_config: GlobalLocalConfigInput::default(), + client_config: ClientConfig::default(), + user_config: None, ratoml_files: FxHashMap::default(), default_config: DefaultConfigData::default(), + source_root_parent_map: Arc::new(FxHashMap::default()), + user_config_path, + root_ratoml: None, + root_ratoml_path, + should_update: false, } } @@ -929,71 +1252,6 @@ impl Config { self.workspace_roots.extend(paths); } - pub fn update(&mut self, mut json: serde_json::Value) -> Result<(), ConfigError> { - tracing::info!("updating config from JSON: {:#}", json); - if json.is_null() || json.as_object().map_or(false, |it| it.is_empty()) { - return Ok(()); - } - let mut errors = Vec::new(); - self.detached_files = - get_field::>(&mut json, &mut errors, "detachedFiles", None) - .unwrap_or_default() - .into_iter() - .map(AbsPathBuf::assert) - .collect(); - patch_old_style::patch_json_for_outdated_configs(&mut json); - self.client_config = FullConfigInput::from_json(json, &mut errors); - tracing::debug!(?self.client_config, "deserialized config data"); - self.snippets.clear(); - - let snips = self.completion_snippets_custom(None).to_owned(); - - for (name, def) in snips.iter() { - if def.prefix.is_empty() && def.postfix.is_empty() { - continue; - } - let scope = match def.scope { - SnippetScopeDef::Expr => SnippetScope::Expr, - SnippetScopeDef::Type => SnippetScope::Type, - SnippetScopeDef::Item => SnippetScope::Item, - }; - match Snippet::new( - &def.prefix, - &def.postfix, - &def.body, - def.description.as_ref().unwrap_or(name), - &def.requires, - scope, - ) { - Some(snippet) => self.snippets.push(snippet), - None => errors.push(( - format!("snippet {name} is invalid"), - ::custom( - "snippet path is invalid or triggers are missing", - ), - )), - } - } - - self.validate(&mut errors); - - if errors.is_empty() { - Ok(()) - } else { - Err(ConfigError { errors }) - } - } - - fn validate(&self, error_sink: &mut Vec<(String, serde_json::Error)>) { - use serde::de::Error; - if self.check_command().is_empty() { - error_sink.push(( - "/check/command".to_owned(), - serde_json::Error::custom("expected a non-empty string"), - )); - } - } - pub fn json_schema() -> serde_json::Value { FullConfigInput::json_schema() } @@ -1002,12 +1260,12 @@ impl Config { &self.root_path } - pub fn caps(&self) -> &lsp_types::ClientCapabilities { - &self.caps + pub fn root_ratoml_path(&self) -> &VfsPath { + &self.root_ratoml_path } - pub fn detached_files(&self) -> &[AbsPathBuf] { - &self.detached_files + pub fn caps(&self) -> &lsp_types::ClientCapabilities { + &self.caps } } @@ -1018,7 +1276,7 @@ impl Config { allowed: None, insert_use: self.insert_use_config(source_root), prefer_no_std: self.imports_preferNoStd(source_root).to_owned(), - assist_emit_must_use: self.assist_emitMustUse().to_owned(), + assist_emit_must_use: self.assist_emitMustUse(source_root).to_owned(), prefer_prelude: self.imports_preferPrelude(source_root).to_owned(), term_search_fuel: self.assist_termSearch_fuel(source_root).to_owned() as u64, } @@ -1026,17 +1284,13 @@ impl Config { pub fn completion(&self, source_root: Option) -> CompletionConfig { CompletionConfig { - enable_postfix_completions: self.completion_postfix_enable(source_root).to_owned(), - enable_imports_on_the_fly: self.completion_autoimport_enable(source_root).to_owned() + enable_postfix_completions: self.completion_postfix_enable().to_owned(), + enable_imports_on_the_fly: self.completion_autoimport_enable().to_owned() && completion_item_edit_resolve(&self.caps), - enable_self_on_the_fly: self.completion_autoself_enable(source_root).to_owned(), - enable_private_editable: self.completion_privateEditable_enable(source_root).to_owned(), - enable_term_search: self.completion_termSearch_enable(source_root).to_owned(), - term_search_fuel: self.completion_termSearch_fuel(source_root).to_owned() as u64, - full_function_signatures: self - .completion_fullFunctionSignatures_enable(source_root) - .to_owned(), - callable: match self.completion_callable_snippets(source_root) { + enable_self_on_the_fly: self.completion_autoself_enable().to_owned(), + enable_private_editable: self.completion_privateEditable_enable().to_owned(), + full_function_signatures: self.completion_fullFunctionSignatures_enable().to_owned(), + callable: match self.completion_callable_snippets() { CallableCompletionDef::FillArguments => Some(CallableSnippets::FillArguments), CallableCompletionDef::AddParentheses => Some(CallableSnippets::AddParentheses), CallableCompletionDef::None => None, @@ -1055,10 +1309,18 @@ impl Config { prefer_no_std: self.imports_preferNoStd(source_root).to_owned(), prefer_prelude: self.imports_preferPrelude(source_root).to_owned(), snippets: self.snippets.clone().to_vec(), - limit: self.completion_limit(source_root).to_owned(), + limit: self.completion_limit().to_owned(), + enable_term_search: self.completion_termSearch_enable().to_owned(), + term_search_fuel: self.completion_termSearch_fuel().to_owned() as u64, } } + pub fn detached_files(&self) -> &Vec { + // FIXME @alibektas : This is the only config that is confusing. If it's a proper configuration + // why is it not among the others? If it's client only which I doubt it is current state should be alright + &self.client_config.detached_files + } + pub fn diagnostics(&self, source_root: Option) -> DiagnosticsConfig { DiagnosticsConfig { enabled: *self.diagnostics_enable(), @@ -1066,7 +1328,7 @@ impl Config { proc_macros_enabled: *self.procMacro_enable(), disable_experimental: !self.diagnostics_experimental_enable(), disabled: self.diagnostics_disabled().clone(), - expr_fill_default: match self.assist_expressionFillDefault() { + expr_fill_default: match self.assist_expressionFillDefault(source_root) { ExprFillDefaultDef::Todo => ExprFillDefaultMode::Todo, ExprFillDefaultDef::Default => ExprFillDefaultMode::Default, }, @@ -2016,7 +2278,7 @@ enum SnippetScopeDef { #[derive(Serialize, Deserialize, Debug, Clone, Default)] #[serde(default)] -struct SnippetDef { +pub(crate) struct SnippetDef { #[serde(with = "single_or_array")] #[serde(skip_serializing_if = "Vec::is_empty")] prefix: Vec, @@ -2111,7 +2373,7 @@ enum ImportGranularityDef { #[derive(Serialize, Deserialize, Debug, Copy, Clone)] #[serde(rename_all = "snake_case")] -enum CallableCompletionDef { +pub(crate) enum CallableCompletionDef { FillArguments, AddParentheses, None, @@ -2318,15 +2580,30 @@ macro_rules! _impl_for_config_data { $( $($doc)* #[allow(non_snake_case)] - $vis fn $field(&self, _source_root: Option) -> &$ty { - if let Some(v) = self.client_config.local.$field.as_ref() { - return &v; + $vis fn $field(&self, source_root: Option) -> &$ty { + + if source_root.is_some() { + let mut par: Option = source_root; + while let Some(source_root_id) = par { + par = self.source_root_parent_map.get(&source_root_id).copied(); + if let Some(config) = self.ratoml_files.get(&source_root_id) { + if let Some(value) = config.node.local.$field.as_ref() { + return value; + } + } + } } - if let Some(v) = self.user_config.local.$field.as_ref() { + if let Some(v) = self.client_config.node.local.$field.as_ref() { return &v; } + if let Some(user_config) = self.user_config.as_ref() { + if let Some(v) = user_config.node.local.$field.as_ref() { + return &v; + } + } + &self.default_config.local.$field } )* @@ -2342,14 +2619,23 @@ macro_rules! _impl_for_config_data { $($doc)* #[allow(non_snake_case)] $vis fn $field(&self) -> &$ty { - if let Some(v) = self.client_config.global.$field.as_ref() { - return &v; + + if let Some(root_path_ratoml) = self.root_ratoml.as_ref() { + if let Some(v) = root_path_ratoml.node.global.$field.as_ref() { + return &v; + } } - if let Some(v) = self.user_config.global.$field.as_ref() { + if let Some(v) = self.client_config.node.global.$field.as_ref() { return &v; } + if let Some(user_config) = self.user_config.as_ref() { + if let Some(v) = user_config.node.global.$field.as_ref() { + return &v; + } + } + &self.default_config.global.$field } )* @@ -2502,11 +2788,10 @@ struct DefaultConfigData { /// All of the config levels, all fields `Option`, to describe fields that are actually set by /// some rust-analyzer.toml file or JSON blob. An empty rust-analyzer.toml corresponds to /// all fields being None. -#[derive(Debug, Clone, Default)] +#[derive(Debug, Clone, Default, Serialize)] struct FullConfigInput { global: GlobalConfigInput, local: LocalConfigInput, - #[allow(dead_code)] client: ClientConfigInput, } @@ -2545,7 +2830,7 @@ impl FullConfigInput { /// All of the config levels, all fields `Option`, to describe fields that are actually set by /// some rust-analyzer.toml file or JSON blob. An empty rust-analyzer.toml corresponds to /// all fields being None. -#[derive(Debug, Clone, Default)] +#[derive(Debug, Clone, Default, Serialize)] struct GlobalLocalConfigInput { global: GlobalConfigInput, local: LocalConfigInput, @@ -3104,12 +3389,17 @@ mod tests { Default::default(), vec![], None, + None, ); - config - .update(serde_json::json!({ - "procMacro_server": null, - })) - .unwrap(); + + let mut change = ConfigChange::default(); + change.change_client_config(serde_json::json!({ + "procMacro" : { + "server": null, + }})); + + let mut error_sink = ConfigError::default(); + config = config.apply_change(change, &mut error_sink); assert_eq!(config.proc_macro_srv(), None); } @@ -3120,12 +3410,16 @@ mod tests { Default::default(), vec![], None, + None, ); - config - .update(serde_json::json!({ - "procMacro": {"server": project_root().display().to_string()} - })) - .unwrap(); + let mut change = ConfigChange::default(); + change.change_client_config(serde_json::json!({ + "procMacro" : { + "server": project_root().display().to_string(), + }})); + + let mut error_sink = ConfigError::default(); + config = config.apply_change(change, &mut error_sink); assert_eq!(config.proc_macro_srv(), Some(AbsPathBuf::try_from(project_root()).unwrap())); } @@ -3136,12 +3430,19 @@ mod tests { Default::default(), vec![], None, + None, ); - config - .update(serde_json::json!({ - "procMacro": {"server": "./server"} - })) - .unwrap(); + + let mut change = ConfigChange::default(); + + change.change_client_config(serde_json::json!({ + "procMacro" : { + "server": "./server" + }})); + + let mut error_sink = ConfigError::default(); + config = config.apply_change(change, &mut error_sink); + assert_eq!( config.proc_macro_srv(), Some(AbsPathBuf::try_from(project_root().join("./server")).unwrap()) @@ -3155,12 +3456,17 @@ mod tests { Default::default(), vec![], None, + None, ); - config - .update(serde_json::json!({ - "rust": { "analyzerTargetDir": null } - })) - .unwrap(); + + let mut change = ConfigChange::default(); + + change.change_client_config(serde_json::json!({ + "rust" : { "analyzerTargetDir" : null } + })); + + let mut error_sink = ConfigError::default(); + config = config.apply_change(change, &mut error_sink); assert_eq!(config.cargo_targetDir(), &None); assert!( matches!(config.flycheck(), FlycheckConfig::CargoCommand { options, .. } if options.target_dir.is_none()) @@ -3174,12 +3480,17 @@ mod tests { Default::default(), vec![], None, + None, ); - config - .update(serde_json::json!({ - "rust": { "analyzerTargetDir": true } - })) - .unwrap(); + + let mut change = ConfigChange::default(); + change.change_client_config(serde_json::json!({ + "rust" : { "analyzerTargetDir" : true } + })); + + let mut error_sink = ConfigError::default(); + config = config.apply_change(change, &mut error_sink); + assert_eq!(config.cargo_targetDir(), &Some(TargetDirectory::UseSubdirectory(true))); assert!( matches!(config.flycheck(), FlycheckConfig::CargoCommand { options, .. } if options.target_dir == Some(Utf8PathBuf::from("target/rust-analyzer"))) @@ -3193,12 +3504,17 @@ mod tests { Default::default(), vec![], None, + None, ); - config - .update(serde_json::json!({ - "rust": { "analyzerTargetDir": "other_folder" } - })) - .unwrap(); + + let mut change = ConfigChange::default(); + change.change_client_config(serde_json::json!({ + "rust" : { "analyzerTargetDir" : "other_folder" } + })); + + let mut error_sink = ConfigError::default(); + config = config.apply_change(change, &mut error_sink); + assert_eq!( config.cargo_targetDir(), &Some(TargetDirectory::Directory(Utf8PathBuf::from("other_folder"))) diff --git a/crates/rust-analyzer/src/diagnostics.rs b/crates/rust-analyzer/src/diagnostics.rs index 65a9a4914934..6dc608c77762 100644 --- a/crates/rust-analyzer/src/diagnostics.rs +++ b/crates/rust-analyzer/src/diagnostics.rs @@ -154,7 +154,7 @@ pub(crate) fn fetch_native_diagnostics( .copied() .filter_map(|file_id| { let line_index = snapshot.file_line_index(file_id).ok()?; - let source_root = snapshot.analysis.source_root(file_id).ok()?; + let source_root = snapshot.analysis.source_root_id(file_id).ok()?; let diagnostics = snapshot .analysis diff --git a/crates/rust-analyzer/src/diagnostics/to_proto.rs b/crates/rust-analyzer/src/diagnostics/to_proto.rs index 3d3f94401991..4832e8cab43f 100644 --- a/crates/rust-analyzer/src/diagnostics/to_proto.rs +++ b/crates/rust-analyzer/src/diagnostics/to_proto.rs @@ -547,6 +547,7 @@ mod tests { ClientCapabilities::default(), Vec::new(), None, + None, ), ); let snap = state.snapshot(); diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index f64e66183d1b..2210dab0f575 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -25,13 +25,16 @@ use project_model::{ use rustc_hash::{FxHashMap, FxHashSet}; use tracing::{span, Level}; use triomphe::Arc; -use vfs::{AnchoredPathBuf, Vfs}; +use vfs::{AnchoredPathBuf, Vfs, VfsPath}; use crate::{ - config::{Config, ConfigError}, + config::{Config, ConfigChange, ConfigError}, diagnostics::{CheckFixes, DiagnosticCollection}, line_index::{LineEndings, LineIndex}, - lsp::{from_proto, to_proto::url_from_abs_path}, + lsp::{ + from_proto::{self}, + to_proto::url_from_abs_path, + }, lsp_ext, main_loop::Task, mem_docs::MemDocs, @@ -71,7 +74,7 @@ pub(crate) struct GlobalState { pub(crate) mem_docs: MemDocs, pub(crate) source_root_config: SourceRootConfig, /// A mapping that maps a local source root's `SourceRootId` to it parent's `SourceRootId`, if it has one. - pub(crate) local_roots_parent_map: FxHashMap, + pub(crate) local_roots_parent_map: Arc>, pub(crate) semantic_tokens_cache: Arc>>, // status @@ -213,7 +216,7 @@ impl GlobalState { shutdown_requested: false, last_reported_status: None, source_root_config: SourceRootConfig::default(), - local_roots_parent_map: FxHashMap::default(), + local_roots_parent_map: Arc::new(FxHashMap::default()), config_errors: Default::default(), proc_macro_clients: Arc::from_iter([]), @@ -254,6 +257,24 @@ impl GlobalState { pub(crate) fn process_changes(&mut self) -> bool { let _p = span!(Level::INFO, "GlobalState::process_changes").entered(); + + // We cannot directly resolve a change in a ratoml file to a format + // that can be used by the config module because config talks + // in `SourceRootId`s instead of `FileId`s and `FileId` -> `SourceRootId` + // mapping is not ready until `AnalysisHost::apply_changes` has been called. + let mut modified_ratoml_files: FxHashMap = FxHashMap::default(); + let mut ratoml_text_map: FxHashMap)> = + FxHashMap::default(); + + let mut user_config_file: Option<(FileId, Option)> = None; + let mut root_path_ratoml: Option<(FileId, Option)> = None; + + let root_vfs_path = { + let mut root_vfs_path = self.config.root_path().to_path_buf(); + root_vfs_path.push("rust-analyzer.toml"); + VfsPath::new_real_path(root_vfs_path.to_string()) + }; + let (change, modified_rust_files, workspace_structure_change) = { let mut change = ChangeWithProcMacros::new(); let mut guard = self.vfs.write(); @@ -273,6 +294,11 @@ impl GlobalState { let mut modified_rust_files = vec![]; for file in changed_files.into_values() { let vfs_path = vfs.file_path(file.file_id); + if let Some(("rust-analyzer", Some("toml"))) = vfs_path.name_and_extension() { + // Remember ids to use them after `apply_changes` + modified_ratoml_files.insert(file.file_id, vfs_path.clone()); + } + if let Some(path) = vfs_path.as_path() { has_structure_changes |= file.is_created_or_deleted(); @@ -311,10 +337,30 @@ impl GlobalState { } let (vfs, line_endings_map) = &mut *RwLockUpgradableReadGuard::upgrade(guard); bytes.into_iter().for_each(|(file_id, text)| match text { - None => change.change_file(file_id, None), + None => { + change.change_file(file_id, None); + if let Some(vfs_path) = modified_ratoml_files.get(&file_id) { + if vfs_path == self.config.user_config_path() { + user_config_file = Some((file_id, None)); + } else if vfs_path == &root_vfs_path { + root_path_ratoml = Some((file_id, None)); + } else { + ratoml_text_map.insert(file_id, (vfs_path.clone(), None)); + } + } + } Some((text, line_endings)) => { line_endings_map.insert(file_id, line_endings); - change.change_file(file_id, Some(text)); + change.change_file(file_id, Some(text.clone())); + if let Some(vfs_path) = modified_ratoml_files.get(&file_id) { + if vfs_path == self.config.user_config_path() { + user_config_file = Some((file_id, Some(text.clone()))); + } else if vfs_path == &root_vfs_path { + root_path_ratoml = Some((file_id, Some(text.clone()))); + } else { + ratoml_text_map.insert(file_id, (vfs_path.clone(), Some(text.clone()))); + } + } } }); if has_structure_changes { @@ -327,6 +373,54 @@ impl GlobalState { let _p = span!(Level::INFO, "GlobalState::process_changes/apply_change").entered(); self.analysis_host.apply_change(change); + let config_change = { + let mut change = ConfigChange::default(); + let snap = self.analysis_host.analysis(); + + for (file_id, (vfs_path, text)) in ratoml_text_map { + // If change has been made to a ratoml file that + // belongs to a non-local source root, we will ignore it. + // As it doesn't make sense a users to use external config files. + if let Ok(source_root) = snap.source_root_id(file_id) { + if let Ok(true) = snap.is_local_source_root(source_root) { + if let Some((old_file, old_path, old_text)) = + change.change_ratoml(source_root, file_id, vfs_path.clone(), text) + { + // SourceRoot has more than 1 RATOML files. In this case lexicographically smaller wins. + if old_path < vfs_path { + span!(Level::ERROR, "Two `rust-analyzer.toml` files were found inside the same crate. {vfs_path} has no effect."); + // Put the old one back in. + change.change_ratoml(source_root, old_file, old_path, old_text); + } + } + } + } else { + // Mapping to a SourceRoot should always end up in `Ok` + span!(Level::ERROR, "Mapping to SourceRootId failed."); + } + } + + if let Some((file_id, Some(txt))) = user_config_file { + change.change_user_config(Some((file_id, txt))); + } + + if let Some((file_id, Some(txt))) = root_path_ratoml { + change.change_root_ratoml(Some((file_id, txt))); + } + + change + }; + + let mut error_sink = ConfigError::default(); + let config = self.config.apply_change(config_change, &mut error_sink); + + if config.should_update() { + self.update_configuration(config); + } else { + // No global or client level config was changed. So we can just naively replace config. + self.config = Arc::new(config); + } + { if !matches!(&workspace_structure_change, Some((.., true))) { _ = self diff --git a/crates/rust-analyzer/src/handlers/notification.rs b/crates/rust-analyzer/src/handlers/notification.rs index 9d30063ccc8a..123a9a06a3b8 100644 --- a/crates/rust-analyzer/src/handlers/notification.rs +++ b/crates/rust-analyzer/src/handlers/notification.rs @@ -13,7 +13,7 @@ use triomphe::Arc; use vfs::{AbsPathBuf, ChangeKind, VfsPath}; use crate::{ - config::Config, + config::{Config, ConfigChange, ConfigError}, global_state::GlobalState, lsp::{from_proto, utils::apply_document_changes}, lsp_ext::{self, RunFlycheckParams}, @@ -71,6 +71,7 @@ pub(crate) fn handle_did_open_text_document( tracing::error!("duplicate DidOpenTextDocument: {}", path); } + tracing::info!("New file content set {:?}", params.text_document.text); state.vfs.write().0.set_file_contents(path, Some(params.text_document.text.into_bytes())); if state.config.notifications().unindexed_project { tracing::debug!("queuing task"); @@ -196,10 +197,11 @@ pub(crate) fn handle_did_change_configuration( } (None, Some(mut configs)) => { if let Some(json) = configs.get_mut(0) { - // Note that json can be null according to the spec if the client can't - // provide a configuration. This is handled in Config::update below. let mut config = Config::clone(&*this.config); - this.config_errors = config.update(json.take()).err(); + let mut change = ConfigChange::default(); + change.change_client_config(json.take()); + let mut error_sink = ConfigError::default(); + config = config.apply_change(change, &mut error_sink); this.update_configuration(config); } } diff --git a/crates/rust-analyzer/src/handlers/request.rs b/crates/rust-analyzer/src/handlers/request.rs index 1e24bf3aae3b..ac80e7871cf4 100644 --- a/crates/rust-analyzer/src/handlers/request.rs +++ b/crates/rust-analyzer/src/handlers/request.rs @@ -367,7 +367,7 @@ pub(crate) fn handle_join_lines( let _p = tracing::span!(tracing::Level::INFO, "handle_join_lines").entered(); let file_id = from_proto::file_id(&snap, ¶ms.text_document.uri)?; - let source_root = snap.analysis.source_root(file_id)?; + let source_root = snap.analysis.source_root_id(file_id)?; let config = snap.config.join_lines(Some(source_root)); let line_index = snap.file_line_index(file_id)?; @@ -949,7 +949,7 @@ pub(crate) fn handle_completion( let completion_trigger_character = context.and_then(|ctx| ctx.trigger_character).and_then(|s| s.chars().next()); - let source_root = snap.analysis.source_root(position.file_id)?; + let source_root = snap.analysis.source_root_id(position.file_id)?; let completion_config = &snap.config.completion(Some(source_root)); // FIXME: We should fix up the position when retrying the cancelled request instead position.offset = position.offset.min(line_index.index.len()); @@ -997,7 +997,7 @@ pub(crate) fn handle_completion_resolve( let Ok(offset) = from_proto::offset(&line_index, resolve_data.position.position) else { return Ok(original_completion); }; - let source_root = snap.analysis.source_root(file_id)?; + let source_root = snap.analysis.source_root_id(file_id)?; let additional_edits = snap .analysis @@ -1229,7 +1229,7 @@ pub(crate) fn handle_code_action( let file_id = from_proto::file_id(&snap, ¶ms.text_document.uri)?; let line_index = snap.file_line_index(file_id)?; let frange = from_proto::file_range(&snap, ¶ms.text_document, params.range)?; - let source_root = snap.analysis.source_root(file_id)?; + let source_root = snap.analysis.source_root_id(file_id)?; let mut assists_config = snap.config.assist(Some(source_root)); assists_config.allowed = params @@ -1307,7 +1307,7 @@ pub(crate) fn handle_code_action_resolve( let line_index = snap.file_line_index(file_id)?; let range = from_proto::text_range(&line_index, params.code_action_params.range)?; let frange = FileRange { file_id, range }; - let source_root = snap.analysis.source_root(file_id)?; + let source_root = snap.analysis.source_root_id(file_id)?; let mut assists_config = snap.config.assist(Some(source_root)); assists_config.allowed = params @@ -1460,7 +1460,7 @@ pub(crate) fn handle_document_highlight( let _p = tracing::span!(tracing::Level::INFO, "handle_document_highlight").entered(); let position = from_proto::file_position(&snap, params.text_document_position_params)?; let line_index = snap.file_line_index(position.file_id)?; - let source_root = snap.analysis.source_root(position.file_id)?; + let source_root = snap.analysis.source_root_id(position.file_id)?; let refs = match snap .analysis @@ -1511,7 +1511,7 @@ pub(crate) fn handle_inlay_hints( params.range, )?; let line_index = snap.file_line_index(file_id)?; - let source_root = snap.analysis.source_root(file_id)?; + let source_root = snap.analysis.source_root_id(file_id)?; let range = TextRange::new( range.start().min(line_index.index.len()), range.end().min(line_index.index.len()), @@ -1553,7 +1553,7 @@ pub(crate) fn handle_inlay_hints_resolve( let line_index = snap.file_line_index(file_id)?; let hint_position = from_proto::offset(&line_index, original_hint.position)?; - let source_root = snap.analysis.source_root(file_id)?; + let source_root = snap.analysis.source_root_id(file_id)?; let mut forced_resolve_inlay_hints_config = snap.config.inlay_hints(Some(source_root)); forced_resolve_inlay_hints_config.fields_to_resolve = InlayFieldsToResolve::empty(); @@ -1687,7 +1687,7 @@ pub(crate) fn handle_semantic_tokens_full( let file_id = from_proto::file_id(&snap, ¶ms.text_document.uri)?; let text = snap.analysis.file_text(file_id)?; let line_index = snap.file_line_index(file_id)?; - let source_root = snap.analysis.source_root(file_id)?; + let source_root = snap.analysis.source_root_id(file_id)?; let mut highlight_config = snap.config.highlighting_config(Some(source_root)); // Avoid flashing a bunch of unresolved references when the proc-macro servers haven't been spawned yet. @@ -1718,7 +1718,7 @@ pub(crate) fn handle_semantic_tokens_full_delta( let file_id = from_proto::file_id(&snap, ¶ms.text_document.uri)?; let text = snap.analysis.file_text(file_id)?; let line_index = snap.file_line_index(file_id)?; - let source_root = snap.analysis.source_root(file_id)?; + let source_root = snap.analysis.source_root_id(file_id)?; let mut highlight_config = snap.config.highlighting_config(Some(source_root)); // Avoid flashing a bunch of unresolved references when the proc-macro servers haven't been spawned yet. @@ -1762,7 +1762,7 @@ pub(crate) fn handle_semantic_tokens_range( let frange = from_proto::file_range(&snap, ¶ms.text_document, params.range)?; let text = snap.analysis.file_text(frange.file_id)?; let line_index = snap.file_line_index(frange.file_id)?; - let source_root = snap.analysis.source_root(frange.file_id)?; + let source_root = snap.analysis.source_root_id(frange.file_id)?; let mut highlight_config = snap.config.highlighting_config(Some(source_root)); // Avoid flashing a bunch of unresolved references when the proc-macro servers haven't been spawned yet. @@ -1991,8 +1991,8 @@ fn goto_type_action_links( snap: &GlobalStateSnapshot, nav_targets: &[HoverGotoTypeData], ) -> Option { - if nav_targets.is_empty() - || !snap.config.hover_actions().goto_type_def + if !snap.config.hover_actions().goto_type_def + || nav_targets.is_empty() || !snap.config.client_commands().goto_location { return None; diff --git a/crates/rust-analyzer/src/lib.rs b/crates/rust-analyzer/src/lib.rs index 175ffa622ff7..d9b31550c562 100644 --- a/crates/rust-analyzer/src/lib.rs +++ b/crates/rust-analyzer/src/lib.rs @@ -18,7 +18,6 @@ mod cargo_target_spec; mod diagnostics; mod diff; mod dispatch; -mod global_state; mod hack_recover_crate_name; mod line_index; mod main_loop; @@ -40,6 +39,7 @@ pub mod tracing { } pub mod config; +pub mod global_state; pub mod lsp; use self::lsp::ext as lsp_ext; diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index 193b3fdd4a8e..dd2104c9c371 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -186,6 +186,11 @@ impl GlobalState { scheme: None, pattern: Some("**/Cargo.lock".into()), }, + lsp_types::DocumentFilter { + language: None, + scheme: None, + pattern: Some("**/rust-analyzer.toml".into()), + }, ]), }, }; diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index 627be7e951ad..e804ba3db955 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -24,14 +24,16 @@ use ide_db::{ }; use itertools::Itertools; use load_cargo::{load_proc_macro, ProjectFolders}; +use lsp_types::FileSystemWatcher; use proc_macro_api::ProcMacroServer; use project_model::{ManifestPath, ProjectWorkspace, ProjectWorkspaceKind, WorkspaceBuildScripts}; use stdx::{format_to, thread::ThreadIntent}; +use tracing::error; use triomphe::Arc; use vfs::{AbsPath, AbsPathBuf, ChangeKind}; use crate::{ - config::{Config, FilesWatcher, LinkedProject}, + config::{Config, ConfigChange, ConfigError, FilesWatcher, LinkedProject}, global_state::GlobalState, lsp_ext, main_loop::Task, @@ -443,40 +445,61 @@ impl GlobalState { let filter = self.workspaces.iter().flat_map(|ws| ws.to_roots()).filter(|it| it.is_local); - let watchers = if self.config.did_change_watched_files_relative_pattern_support() { - // When relative patterns are supported by the client, prefer using them - filter - .flat_map(|root| { - root.include.into_iter().flat_map(|base| { - [(base.clone(), "**/*.rs"), (base, "**/Cargo.{lock,toml}")] + let mut watchers: Vec = + if self.config.did_change_watched_files_relative_pattern_support() { + // When relative patterns are supported by the client, prefer using them + filter + .flat_map(|root| { + root.include.into_iter().flat_map(|base| { + [ + (base.clone(), "**/*.rs"), + (base.clone(), "**/Cargo.{lock,toml}"), + (base, "**/rust-analyzer.toml"), + ] + }) }) - }) - .map(|(base, pat)| lsp_types::FileSystemWatcher { - glob_pattern: lsp_types::GlobPattern::Relative( - lsp_types::RelativePattern { - base_uri: lsp_types::OneOf::Right( - lsp_types::Url::from_file_path(base).unwrap(), - ), - pattern: pat.to_owned(), - }, - ), - kind: None, - }) - .collect() - } else { - // When they're not, integrate the base to make them into absolute patterns - filter - .flat_map(|root| { - root.include.into_iter().flat_map(|base| { - [format!("{base}/**/*.rs"), format!("{base}/**/Cargo.{{lock,toml}}")] + .map(|(base, pat)| lsp_types::FileSystemWatcher { + glob_pattern: lsp_types::GlobPattern::Relative( + lsp_types::RelativePattern { + base_uri: lsp_types::OneOf::Right( + lsp_types::Url::from_file_path(base).unwrap(), + ), + pattern: pat.to_owned(), + }, + ), + kind: None, }) - }) + .collect() + } else { + // When they're not, integrate the base to make them into absolute patterns + filter + .flat_map(|root| { + root.include.into_iter().flat_map(|it| { + [ + format!("{it}/**/*.rs"), + // FIXME @alibektas : Following dbarsky's recomm I merged toml and lock patterns into one. + // Is this correct? + format!("{it}/**/Cargo.{{toml,lock}}"), + format!("{it}/**/rust-analyzer.toml"), + ] + }) + }) + .map(|glob_pattern| lsp_types::FileSystemWatcher { + glob_pattern: lsp_types::GlobPattern::String(glob_pattern), + kind: None, + }) + .collect() + }; + + watchers.extend( + iter::once(self.config.user_config_path().to_string()) + .chain(iter::once(self.config.root_ratoml_path().to_string())) .map(|glob_pattern| lsp_types::FileSystemWatcher { glob_pattern: lsp_types::GlobPattern::String(glob_pattern), kind: None, }) - .collect() - }; + .collect::>(), + ); let registration_options = lsp_types::DidChangeWatchedFilesRegistrationOptions { watchers }; @@ -548,7 +571,41 @@ impl GlobalState { version: self.vfs_config_version, }); self.source_root_config = project_folders.source_root_config; - self.local_roots_parent_map = self.source_root_config.source_root_parent_map(); + self.local_roots_parent_map = Arc::new(self.source_root_config.source_root_parent_map()); + + let user_config_path = self.config.user_config_path(); + let root_ratoml_path = self.config.root_ratoml_path(); + + { + let vfs = &mut self.vfs.write().0; + let loader = &mut self.loader; + + if vfs.file_id(user_config_path).is_none() { + if let Some(user_cfg_abs) = user_config_path.as_path() { + let contents = loader.handle.load_sync(user_cfg_abs); + vfs.set_file_contents(user_config_path.clone(), contents); + } else { + error!("Non-abs virtual path for user config."); + } + } + + if vfs.file_id(root_ratoml_path).is_none() { + // FIXME @alibektas : Sometimes root_path_ratoml collide with a regular ratoml. + // Although this shouldn't be a problem because everything is mapped to a `FileId`. + // We may want to further think about this. + if let Some(root_ratoml_abs) = root_ratoml_path.as_path() { + let contents = loader.handle.load_sync(root_ratoml_abs); + vfs.set_file_contents(root_ratoml_path.clone(), contents); + } else { + error!("Non-abs virtual path for user config."); + } + } + } + + let mut config_change = ConfigChange::default(); + config_change.change_source_root_parent_map(self.local_roots_parent_map.clone()); + let mut error_sink = ConfigError::default(); + self.config = Arc::new(self.config.apply_change(config_change, &mut error_sink)); self.recreate_crate_graph(cause); diff --git a/crates/rust-analyzer/src/tracing/config.rs b/crates/rust-analyzer/src/tracing/config.rs index f77d98933044..fcdbf6c69497 100644 --- a/crates/rust-analyzer/src/tracing/config.rs +++ b/crates/rust-analyzer/src/tracing/config.rs @@ -13,6 +13,7 @@ use tracing_tree::HierarchicalLayer; use crate::tracing::hprof; +#[derive(Debug)] pub struct Config { pub writer: T, pub filter: String, diff --git a/crates/rust-analyzer/tests/slow-tests/main.rs b/crates/rust-analyzer/tests/slow-tests/main.rs index 43a830501058..fd6f79abc1cc 100644 --- a/crates/rust-analyzer/tests/slow-tests/main.rs +++ b/crates/rust-analyzer/tests/slow-tests/main.rs @@ -17,28 +17,32 @@ mod support; mod testdir; mod tidy; -use std::{collections::HashMap, path::PathBuf, time::Instant}; +use std::{collections::HashMap, path::PathBuf, sync::Once, time::Instant}; use lsp_types::{ - notification::DidOpenTextDocument, + notification::{DidChangeTextDocument, DidOpenTextDocument, DidSaveTextDocument}, request::{ CodeActionRequest, Completion, Formatting, GotoTypeDefinition, HoverRequest, InlayHintRequest, InlayHintResolveRequest, WillRenameFiles, WorkspaceSymbolRequest, }, - CodeActionContext, CodeActionParams, CompletionParams, DidOpenTextDocumentParams, - DocumentFormattingParams, FileRename, FormattingOptions, GotoDefinitionParams, HoverParams, - InlayHint, InlayHintLabel, InlayHintParams, PartialResultParams, Position, Range, - RenameFilesParams, TextDocumentItem, TextDocumentPositionParams, WorkDoneProgressParams, + CodeAction, CodeActionContext, CodeActionOrCommand, CodeActionParams, CodeActionResponse, + CompletionParams, DidChangeTextDocumentParams, DidOpenTextDocumentParams, + DidSaveTextDocumentParams, DocumentFormattingParams, FileRename, FormattingOptions, + GotoDefinitionParams, Hover, HoverParams, InlayHint, InlayHintLabel, InlayHintParams, + PartialResultParams, Position, Range, RenameFilesParams, TextDocumentContentChangeEvent, + TextDocumentIdentifier, TextDocumentItem, TextDocumentPositionParams, Url, + VersionedTextDocumentIdentifier, WorkDoneProgressParams, }; +use paths::Utf8PathBuf; use rust_analyzer::lsp::ext::{OnEnter, Runnables, RunnablesParams, UnindexedProject}; use serde_json::json; use stdx::format_to_acc; +use support::Server; use test_utils::skip_slow_tests; +use testdir::TestDir; +use tracing_subscriber::fmt::TestWriter; -use crate::{ - support::{project, Project}, - testdir::TestDir, -}; +use crate::support::{project, Project}; #[test] fn completes_items_from_standard_library() { @@ -1467,3 +1471,1027 @@ version = "0.0.0" server.request::(Default::default(), json!([])); } + +enum QueryType { + AssistEmitMustUse, + /// A query whose config key is a part of the global configs, so that + /// testing for changes to this config means testing if global changes + /// take affect. + GlobalHover, +} + +struct RatomlTest { + urls: Vec, + server: Server, + tmp_path: Utf8PathBuf, + user_config_dir: Utf8PathBuf, +} + +impl RatomlTest { + const EMIT_MUST_USE: &'static str = r#"assist.emitMustUse = true"#; + const EMIT_MUST_NOT_USE: &'static str = r#"assist.emitMustUse = false"#; + const EMIT_MUST_USE_SNIPPET: &'static str = r#" + +impl Value { + #[must_use] + fn as_text(&self) -> Option<&String> { + if let Self::Text(v) = self { + Some(v) + } else { + None + } + } +}"#; + + const GLOBAL_TRAIT_ASSOC_ITEMS_ZERO: &'static str = r#"hover.show.traitAssocItems = 0"#; + const GLOBAL_TRAIT_ASSOC_ITEMS_SNIPPET: &'static str = r#" +```rust +p1 +``` + +```rust +trait RandomTrait { + type B; + fn abc() -> i32; + fn def() -> i64; +} +```"#; + + fn new( + fixtures: Vec<&str>, + roots: Vec<&str>, + client_config: Option, + ) -> Self { + // setup(); + let tmp_dir = TestDir::new(); + let tmp_path = tmp_dir.path().to_owned(); + + 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); + + for root in roots { + project = project.root(root); + } + + if let Some(client_config) = client_config { + project = project.with_config(client_config); + } + + let server = project.server().wait_until_workspace_is_loaded(); + + let mut case = Self { urls: vec![], server, tmp_path, user_config_dir }; + let urls = fixtures.iter().map(|fixture| case.fixture_path(fixture)).collect::>(); + case.urls = urls; + case + } + + fn fixture_path(&self, fixture: &str) -> Url { + let mut lines = fixture.trim().split('\n'); + + let mut path = + lines.next().expect("All files in a fixture are expected to have at least one line."); + + if path.starts_with("//- minicore") { + path = lines.next().expect("A minicore line must be followed by a path.") + } + + path = path.strip_prefix("//- ").expect("Path must be preceded by a //- prefix "); + + let spl = path[1..].split('/'); + let mut path = self.tmp_path.clone(); + + let mut spl = spl.into_iter(); + if let Some(first) = spl.next() { + if first == "$$CONFIG_DIR$$" { + path = self.user_config_dir.clone(); + } else { + path = path.join(first); + } + } + for piece in spl { + path = path.join(piece); + } + + Url::parse( + format!( + "file://{}", + path.into_string().to_owned().replace("C:\\", "/c:/").replace('\\', "/") + ) + .as_str(), + ) + .unwrap() + } + + fn create(&mut self, fixture_path: &str, text: String) { + let url = self.fixture_path(fixture_path); + + self.server.notification::(DidOpenTextDocumentParams { + text_document: TextDocumentItem { + uri: url.clone(), + language_id: "rust".to_owned(), + version: 0, + text: String::new(), + }, + }); + + self.server.notification::(DidChangeTextDocumentParams { + text_document: VersionedTextDocumentIdentifier { uri: url, version: 0 }, + content_changes: vec![TextDocumentContentChangeEvent { + range: None, + range_length: None, + text, + }], + }); + } + + fn delete(&mut self, file_idx: usize) { + self.server.notification::(DidOpenTextDocumentParams { + text_document: TextDocumentItem { + uri: self.urls[file_idx].clone(), + language_id: "rust".to_owned(), + version: 0, + text: "".to_owned(), + }, + }); + + // See if deleting ratoml file will make the config of interest to return to its default value. + self.server.notification::(DidSaveTextDocumentParams { + text_document: TextDocumentIdentifier { uri: self.urls[file_idx].clone() }, + text: Some("".to_owned()), + }); + } + + fn edit(&mut self, file_idx: usize, text: String) { + self.server.notification::(DidOpenTextDocumentParams { + text_document: TextDocumentItem { + uri: self.urls[file_idx].clone(), + language_id: "rust".to_owned(), + version: 0, + text: String::new(), + }, + }); + + self.server.notification::(DidChangeTextDocumentParams { + text_document: VersionedTextDocumentIdentifier { + uri: self.urls[file_idx].clone(), + version: 0, + }, + content_changes: vec![TextDocumentContentChangeEvent { + range: None, + range_length: None, + text, + }], + }); + } + + fn query(&self, query: QueryType, source_file_idx: usize) -> bool { + match query { + QueryType::AssistEmitMustUse => { + let res = self.server.send_request::(CodeActionParams { + text_document: TextDocumentIdentifier { + uri: self.urls[source_file_idx].clone(), + }, + range: lsp_types::Range { + start: Position::new(2, 13), + end: Position::new(2, 15), + }, + context: CodeActionContext { + diagnostics: vec![], + only: None, + trigger_kind: None, + }, + work_done_progress_params: WorkDoneProgressParams { work_done_token: None }, + partial_result_params: lsp_types::PartialResultParams { + partial_result_token: None, + }, + }); + + let res = serde_json::de::from_str::(res.to_string().as_str()) + .unwrap(); + + // The difference setting the new config key will cause can be seen in the lower layers of this nested response + // so here are some ugly unwraps and other obscure stuff. + let ca: CodeAction = res + .into_iter() + .find_map(|it| { + if let CodeActionOrCommand::CodeAction(ca) = it { + if ca.title.as_str() == "Generate an `as_` method for this enum variant" + { + return Some(ca); + } + } + + None + }) + .unwrap(); + + if let lsp_types::DocumentChanges::Edits(edits) = + ca.edit.unwrap().document_changes.unwrap() + { + if let lsp_types::OneOf::Left(l) = &edits[0].edits[0] { + return l.new_text.as_str() == RatomlTest::EMIT_MUST_USE_SNIPPET; + } + } + } + QueryType::GlobalHover => { + let res = self.server.send_request::(HoverParams { + work_done_progress_params: WorkDoneProgressParams { work_done_token: None }, + text_document_position_params: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { + uri: self.urls[source_file_idx].clone(), + }, + position: Position::new(7, 18), + }, + }); + let res = serde_json::de::from_str::(res.to_string().as_str()).unwrap(); + assert!(matches!(res.contents, lsp_types::HoverContents::Markup(_))); + if let lsp_types::HoverContents::Markup(m) = res.contents { + return m.value == RatomlTest::GLOBAL_TRAIT_ASSOC_ITEMS_SNIPPET; + } + } + } + + panic!() + } +} + +static INIT: Once = Once::new(); + +fn setup() { + INIT.call_once(|| { + let trc = rust_analyzer::tracing::Config { + writer: TestWriter::default(), + // Deliberately enable all `error` logs if the user has not set RA_LOG, as there is usually + // useful information in there for debugging. + filter: std::env::var("RA_LOG").ok().unwrap_or_else(|| "error".to_owned()), + chalk_filter: std::env::var("CHALK_DEBUG").ok(), + profile_filter: std::env::var("RA_PROFILE").ok(), + }; + + trc.init().unwrap(); + }); +} + +// /// Check if we are listening for changes in user's config file ( e.g on Linux `~/.config/rust-analyzer/.rust-analyzer.toml`) +// #[test] +// #[cfg(target_os = "windows")] +// fn listen_to_user_config_scenario_windows() { +// todo!() +// } + +// #[test] +// #[cfg(target_os = "linux")] +// fn listen_to_user_config_scenario_linux() { +// todo!() +// } + +// #[test] +// #[cfg(target_os = "macos")] +// fn listen_to_user_config_scenario_macos() { +// todo!() +// } + +/// Check if made changes have had any effect on +/// the client config. +#[test] +fn ratoml_client_config_basic() { + let server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +[package] +name = "p1" +version = "0.1.0" +edition = "2021" +"#, + r#"//- /p1/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + ], + vec!["p1"], + Some(json!({ + "assist" : { + "emitMustUse" : true + } + })), + ); + + assert!(server.query(QueryType::AssistEmitMustUse, 1)); +} + +/// Checks if client config can be modified. +/// FIXME @alibektas : This test is atm not valid. +/// Asking for client config from the client is a 2 way communication +/// which we cannot imitate with the current slow-tests infrastructure. +/// See rust-analyzer::handlers::notifications#197 +// #[test] +// fn client_config_update() { +// setup(); + +// let server = RatomlTest::new( +// vec![ +// r#" +// //- /p1/Cargo.toml +// [package] +// name = "p1" +// version = "0.1.0" +// edition = "2021" +// "#, +// r#" +// //- /p1/src/lib.rs +// enum Value { +// Number(i32), +// Text(String), +// }"#, +// ], +// vec!["p1"], +// None, +// ); + +// assert!(!server.query(QueryType::AssistEmitMustUse, 1)); + +// // a.notification::(DidChangeConfigurationParams { +// // settings: json!({ +// // "assists" : { +// // "emitMustUse" : true +// // } +// // }), +// // }); + +// assert!(server.query(QueryType::AssistEmitMustUse, 1)); +// } + +// #[test] +// fn ratoml_create_ratoml_basic() { +// let server = RatomlTest::new( +// vec![ +// r#" +// //- /p1/Cargo.toml +// [package] +// name = "p1" +// version = "0.1.0" +// edition = "2021" +// "#, +// r#" +// //- /p1/rust-analyzer.toml +// assist.emitMustUse = true +// "#, +// r#" +// //- /p1/src/lib.rs +// enum Value { +// Number(i32), +// Text(String), +// } +// "#, +// ], +// vec!["p1"], +// None, +// ); + +// assert!(server.query(QueryType::AssistEmitMustUse, 2)); +// } + +#[test] +fn ratoml_user_config_detected() { + let server = RatomlTest::new( + vec![ + r#" +//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml +assist.emitMustUse = true +"#, + r#" +//- /p1/Cargo.toml +[package] +name = "p1" +version = "0.1.0" +edition = "2021" +"#, + r#"//- /p1/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + ], + vec!["p1"], + None, + ); + + assert!(server.query(QueryType::AssistEmitMustUse, 2)); +} + +#[test] +fn ratoml_create_user_config() { + setup(); + let mut server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +[package] +name = "p1" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + ], + vec!["p1"], + None, + ); + + assert!(!server.query(QueryType::AssistEmitMustUse, 1)); + server.create( + "//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml", + RatomlTest::EMIT_MUST_USE.to_owned(), + ); + assert!(server.query(QueryType::AssistEmitMustUse, 1)); +} + +#[test] +fn ratoml_modify_user_config() { + let mut server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +[package] +name = "p1" +version = "0.1.0" +edition = "2021""#, + r#" +//- /p1/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + r#" +//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml +assist.emitMustUse = true"#, + ], + vec!["p1"], + None, + ); + + assert!(server.query(QueryType::AssistEmitMustUse, 1)); + server.edit(2, String::new()); + assert!(!server.query(QueryType::AssistEmitMustUse, 1)); +} + +#[test] +fn ratoml_delete_user_config() { + let mut server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +[package] +name = "p1" +version = "0.1.0" +edition = "2021""#, + r#" +//- /p1/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + r#" +//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml +assist.emitMustUse = true"#, + ], + vec!["p1"], + None, + ); + + assert!(server.query(QueryType::AssistEmitMustUse, 1)); + server.delete(2); + assert!(!server.query(QueryType::AssistEmitMustUse, 1)); +} +// #[test] +// fn delete_user_config() { +// todo!() +// } + +// #[test] +// fn modify_client_config() { +// todo!() +// } + +#[test] +fn ratoml_inherit_config_from_ws_root() { + let server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +workspace = { members = ["p2"] } +[package] +name = "p1" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/rust-analyzer.toml +assist.emitMustUse = true +"#, + r#" +//- /p1/p2/Cargo.toml +[package] +name = "p2" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/p2/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + r#" +//- /p1/src/lib.rs +pub fn add(left: usize, right: usize) -> usize { + left + right +} +"#, + ], + vec!["p1"], + None, + ); + + assert!(server.query(QueryType::AssistEmitMustUse, 3)); +} + +#[test] +fn ratoml_modify_ratoml_at_ws_root() { + let mut server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +workspace = { members = ["p2"] } +[package] +name = "p1" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/rust-analyzer.toml +assist.emitMustUse = false +"#, + r#" +//- /p1/p2/Cargo.toml +[package] +name = "p2" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/p2/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + r#" +//- /p1/src/lib.rs +pub fn add(left: usize, right: usize) -> usize { + left + right +} +"#, + ], + vec!["p1"], + None, + ); + + assert!(!server.query(QueryType::AssistEmitMustUse, 3)); + server.edit(1, "assist.emitMustUse = true".to_owned()); + assert!(server.query(QueryType::AssistEmitMustUse, 3)); +} + +#[test] +fn ratoml_delete_ratoml_at_ws_root() { + let mut server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +workspace = { members = ["p2"] } +[package] +name = "p1" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/rust-analyzer.toml +assist.emitMustUse = true +"#, + r#" +//- /p1/p2/Cargo.toml +[package] +name = "p2" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/p2/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + r#" +//- /p1/src/lib.rs +pub fn add(left: usize, right: usize) -> usize { + left + right +} +"#, + ], + vec!["p1"], + None, + ); + + assert!(server.query(QueryType::AssistEmitMustUse, 3)); + server.delete(1); + assert!(!server.query(QueryType::AssistEmitMustUse, 3)); +} + +#[test] +fn ratoml_add_immediate_child_to_ws_root() { + let mut server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +workspace = { members = ["p2"] } +[package] +name = "p1" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/rust-analyzer.toml +assist.emitMustUse = true +"#, + r#" +//- /p1/p2/Cargo.toml +[package] +name = "p2" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/p2/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + r#" +//- /p1/src/lib.rs +pub fn add(left: usize, right: usize) -> usize { + left + right +} +"#, + ], + vec!["p1"], + None, + ); + + assert!(server.query(QueryType::AssistEmitMustUse, 3)); + server.create("//- /p1/p2/rust-analyzer.toml", RatomlTest::EMIT_MUST_NOT_USE.to_owned()); + assert!(!server.query(QueryType::AssistEmitMustUse, 3)); +} + +#[test] +fn ratoml_rm_ws_root_ratoml_child_has_client_as_parent_now() { + let mut server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +workspace = { members = ["p2"] } +[package] +name = "p1" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/rust-analyzer.toml +assist.emitMustUse = true +"#, + r#" +//- /p1/p2/Cargo.toml +[package] +name = "p2" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/p2/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + r#" +//- /p1/src/lib.rs +pub fn add(left: usize, right: usize) -> usize { + left + right +} +"#, + ], + vec!["p1"], + None, + ); + + assert!(server.query(QueryType::AssistEmitMustUse, 3)); + server.delete(1); + assert!(!server.query(QueryType::AssistEmitMustUse, 3)); +} + +#[test] +fn ratoml_crates_both_roots() { + let server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +workspace = { members = ["p2"] } +[package] +name = "p1" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/rust-analyzer.toml +assist.emitMustUse = true +"#, + r#" +//- /p1/p2/Cargo.toml +[package] +name = "p2" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/p2/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + r#" +//- /p1/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + ], + vec!["p1", "p2"], + None, + ); + + assert!(server.query(QueryType::AssistEmitMustUse, 3)); + assert!(server.query(QueryType::AssistEmitMustUse, 4)); +} + +#[test] +fn ratoml_multiple_ratoml_in_single_source_root() { + let server = RatomlTest::new( + vec![ + r#" + //- /p1/Cargo.toml + [package] + name = "p1" + version = "0.1.0" + edition = "2021" + "#, + r#" + //- /p1/rust-analyzer.toml + assist.emitMustUse = true + "#, + r#" + //- /p1/src/rust-analyzer.toml + assist.emitMustUse = false + "#, + r#" + //- /p1/src/lib.rs + enum Value { + Number(i32), + Text(String), + } + "#, + ], + vec!["p1"], + None, + ); + + assert!(server.query(QueryType::AssistEmitMustUse, 3)); + + let server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +[package] +name = "p1" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/src/rust-analyzer.toml +assist.emitMustUse = false +"#, + r#" +//- /p1/rust-analyzer.toml +assist.emitMustUse = true +"#, + r#" +//- /p1/src/lib.rs +enum Value { + Number(i32), + Text(String), +} +"#, + ], + vec!["p1"], + None, + ); + + assert!(server.query(QueryType::AssistEmitMustUse, 3)); +} + +/// If a root is non-local, so we cannot find what its parent is +/// in our `config.local_root_parent_map`. So if any config should +/// apply, it must be looked for starting from the client level. +/// FIXME @alibektas : "locality" is according to ra that, which is simply in the file system. +/// This doesn't really help us with what we want to achieve here. +// #[test] +// fn ratoml_non_local_crates_start_inheriting_from_client() { +// let server = RatomlTest::new( +// vec![ +// r#" +// //- /p1/Cargo.toml +// [package] +// name = "p1" +// version = "0.1.0" +// edition = "2021" + +// [dependencies] +// p2 = { path = "../p2" } +// #, +// r#" +// //- /p1/src/lib.rs +// enum Value { +// Number(i32), +// Text(String), +// } + +// use p2; + +// pub fn add(left: usize, right: usize) -> usize { +// p2::add(left, right) +// } + +// #[cfg(test)] +// mod tests { +// use super::*; + +// #[test] +// fn it_works() { +// let result = add(2, 2); +// assert_eq!(result, 4); +// } +// }"#, +// r#" +// //- /p2/Cargo.toml +// [package] +// name = "p2" +// version = "0.1.0" +// edition = "2021" + +// # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +// [dependencies] +// "#, +// r#" +// //- /p2/rust-analyzer.toml +// # DEF +// assist.emitMustUse = true +// "#, +// r#" +// //- /p2/src/lib.rs +// enum Value { +// Number(i32), +// Text(String), +// }"#, +// ], +// vec!["p1", "p2"], +// None, +// ); + +// assert!(!server.query(QueryType::AssistEmitMustUse, 5)); +// } + +/// Having a ratoml file at the root of a project enables +/// configuring global level configurations as well. +#[test] +fn ratoml_in_root_is_global() { + let server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +[package] +name = "p1" +version = "0.1.0" +edition = "2021" + "#, + r#" +//- /rust-analyzer.toml +hover.show.traitAssocItems = 4 + "#, + r#" +//- /p1/src/lib.rs +trait RandomTrait { + type B; + fn abc() -> i32; + fn def() -> i64; +} + +fn main() { + let a = RandomTrait; +}"#, + ], + vec![], + None, + ); + + server.query(QueryType::GlobalHover, 2); +} + +#[test] +fn ratoml_root_is_updateable() { + let mut server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +[package] +name = "p1" +version = "0.1.0" +edition = "2021" + "#, + r#" +//- /rust-analyzer.toml +hover.show.traitAssocItems = 4 + "#, + r#" +//- /p1/src/lib.rs +trait RandomTrait { + type B; + fn abc() -> i32; + fn def() -> i64; +} + +fn main() { + let a = RandomTrait; +}"#, + ], + vec![], + None, + ); + + assert!(server.query(QueryType::GlobalHover, 2)); + server.edit(1, RatomlTest::GLOBAL_TRAIT_ASSOC_ITEMS_ZERO.to_owned()); + assert!(!server.query(QueryType::GlobalHover, 2)); +} + +#[test] +fn ratoml_root_is_deletable() { + let mut server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +[package] +name = "p1" +version = "0.1.0" +edition = "2021" + "#, + r#" +//- /rust-analyzer.toml +hover.show.traitAssocItems = 4 + "#, + r#" +//- /p1/src/lib.rs +trait RandomTrait { + type B; + fn abc() -> i32; + fn def() -> i64; +} + +fn main() { + let a = RandomTrait; +}"#, + ], + vec![], + None, + ); + + assert!(server.query(QueryType::GlobalHover, 2)); + server.delete(1); + assert!(!server.query(QueryType::GlobalHover, 2)); +} diff --git a/crates/rust-analyzer/tests/slow-tests/support.rs b/crates/rust-analyzer/tests/slow-tests/support.rs index cf27cc7eeff6..17485ee3ae80 100644 --- a/crates/rust-analyzer/tests/slow-tests/support.rs +++ b/crates/rust-analyzer/tests/slow-tests/support.rs @@ -9,7 +9,10 @@ use crossbeam_channel::{after, select, Receiver}; use lsp_server::{Connection, Message, Notification, Request}; use lsp_types::{notification::Exit, request::Shutdown, TextDocumentIdentifier, Url}; use paths::{Utf8Path, Utf8PathBuf}; -use rust_analyzer::{config::Config, lsp, main_loop}; +use rust_analyzer::{ + config::{Config, ConfigChange, ConfigError}, + lsp, main_loop, +}; use serde::Serialize; use serde_json::{json, to_string_pretty, Value}; use test_utils::FixtureWithProjectMeta; @@ -24,6 +27,7 @@ pub(crate) struct Project<'a> { roots: Vec, config: serde_json::Value, root_dir_contains_symlink: bool, + user_config_path: Option, } impl Project<'_> { @@ -47,9 +51,15 @@ 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 @@ -111,10 +121,17 @@ impl Project<'_> { assert!(proc_macro_names.is_empty()); assert!(mini_core.is_none()); assert!(toolchain.is_none()); + for entry in fixture { - let path = tmp_dir.path().join(&entry.path['/'.len_utf8()..]); - fs::create_dir_all(path.parent().unwrap()).unwrap(); - fs::write(path.as_path(), entry.text.as_bytes()).unwrap(); + if let Some(pth) = entry.path.strip_prefix("/$$CONFIG_DIR$$") { + let path = self.user_config_path.clone().unwrap().join(&pth['/'.len_utf8()..]); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path.as_path(), entry.text.as_bytes()).unwrap(); + } else { + let path = tmp_dir.path().join(&entry.path['/'.len_utf8()..]); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path.as_path(), entry.text.as_bytes()).unwrap(); + } } let tmp_dir_path = AbsPathBuf::assert(tmp_dir.path().to_path_buf()); @@ -184,8 +201,14 @@ impl Project<'_> { }, roots, None, + self.user_config_path, ); - config.update(self.config).expect("invalid config"); + let mut change = ConfigChange::default(); + + change.change_client_config(self.config); + let mut error_sink = ConfigError::default(); + assert!(error_sink.is_empty()); + config = config.apply_change(change, &mut error_sink); config.rediscover_workspaces(); Server::new(tmp_dir.keep(), config) From cf97aac9948a02c3722a95af1e7912e587a721a0 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Sat, 27 Apr 2024 22:50:31 +0200 Subject: [PATCH 2/8] Apply requested changes round 2 --- crates/paths/src/lib.rs | 2 +- crates/rust-analyzer/src/bin/main.rs | 2 +- crates/rust-analyzer/src/cli/scip.rs | 2 +- crates/rust-analyzer/src/config.rs | 232 ++-- crates/rust-analyzer/src/global_state.rs | 114 +- .../src/handlers/notification.rs | 3 +- crates/rust-analyzer/src/lib.rs | 2 +- crates/rust-analyzer/src/reload.rs | 3 +- crates/rust-analyzer/tests/slow-tests/main.rs | 1045 +---------------- .../rust-analyzer/tests/slow-tests/ratoml.rs | 1019 ++++++++++++++++ .../rust-analyzer/tests/slow-tests/support.rs | 4 +- 11 files changed, 1159 insertions(+), 1269 deletions(-) create mode 100644 crates/rust-analyzer/tests/slow-tests/ratoml.rs diff --git a/crates/paths/src/lib.rs b/crates/paths/src/lib.rs index 7d7cf0220e6d..33c3f83db50f 100644 --- a/crates/paths/src/lib.rs +++ b/crates/paths/src/lib.rs @@ -150,7 +150,7 @@ impl AbsPathBuf { /// * if `self` has a verbatim prefix (e.g. `\\?\C:\windows`) /// and `path` is not empty, the new path is normalized: all references /// to `.` and `..` are removed. - pub fn push(&mut self, suffix: &str) { + pub fn push>(&mut self, suffix: P) { self.0.push(suffix) } } diff --git a/crates/rust-analyzer/src/bin/main.rs b/crates/rust-analyzer/src/bin/main.rs index 7e58cd70c494..316384f96331 100644 --- a/crates/rust-analyzer/src/bin/main.rs +++ b/crates/rust-analyzer/src/bin/main.rs @@ -229,7 +229,7 @@ fn run_server() -> anyhow::Result<()> { let mut change = ConfigChange::default(); change.change_client_config(json); let mut error_sink = ConfigError::default(); - config = config.apply_change(change, &mut error_sink); + (config, _) = config.apply_change(change, &mut error_sink); if !error_sink.is_empty() { use lsp_types::{ notification::{Notification, ShowMessage}, diff --git a/crates/rust-analyzer/src/cli/scip.rs b/crates/rust-analyzer/src/cli/scip.rs index b2d70562890f..d73e4028e518 100644 --- a/crates/rust-analyzer/src/cli/scip.rs +++ b/crates/rust-analyzer/src/cli/scip.rs @@ -46,7 +46,7 @@ impl flags::Scip { let mut change = ConfigChange::default(); change.change_client_config(json); let mut error_sink = ConfigError::default(); - config = config.apply_change(change, &mut error_sink); + (config, _) = config.apply_change(change, &mut error_sink); // FIXME @alibektas : What happens to errors without logging? error!(?error_sink, "Config Error(s)"); } diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 51664dd799eb..646bae4467a0 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -29,12 +29,11 @@ use rustc_hash::{FxHashMap, FxHashSet}; use semver::Version; use serde::{ de::{DeserializeOwned, Error}, - ser::SerializeStruct, Deserialize, Serialize, }; use stdx::format_to_acc; use triomphe::Arc; -use vfs::{AbsPath, AbsPathBuf, FileId, VfsPath}; +use vfs::{AbsPath, AbsPathBuf, VfsPath}; use crate::{ caps::completion_item_edit_resolve, @@ -667,7 +666,7 @@ pub struct Config { default_config: DefaultConfigData, /// Config node that obtains its initial value during the server initialization and /// by receiving a `lsp_types::notification::DidChangeConfiguration`. - client_config: ClientConfig, + client_config: FullConfigInput, /// 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 : @@ -681,139 +680,48 @@ pub struct Config { /// FIXME @alibektas : Change this to sth better. /// Config node whose values apply to **every** Rust project. - user_config: Option, + user_config: Option, /// A special file for this session whose path is set to `self.root_path.join("rust-analyzer.toml")` root_ratoml_path: VfsPath, /// This file can be used to make global changes while having only a workspace-wide scope. - root_ratoml: Option, + root_ratoml: Option, /// For every `SourceRoot` there can be at most one RATOML file. - ratoml_files: FxHashMap, + ratoml_files: FxHashMap, /// Clone of the value that is stored inside a `GlobalState`. source_root_parent_map: Arc>, - /// Changes made to client and global configurations will partially not be reflected even after `.apply_change()` was called. - /// This field signals that the `GlobalState` should call its `update_configuration()` method. - should_update: bool, -} - -#[derive(Clone, Debug)] -struct RatomlNode { - node: GlobalLocalConfigInput, - file_id: FileId, -} - -#[derive(Debug, Clone, Default)] -struct ClientConfig { - node: FullConfigInput, detached_files: Vec, } -impl Serialize for RatomlNode { - fn serialize(&self, serializer: S) -> Result - where - S: serde::Serializer, - { - let mut s = serializer.serialize_struct("RatomlNode", 2)?; - s.serialize_field("file_id", &self.file_id.index())?; - s.serialize_field("config", &self.node)?; - s.end() - } -} - -#[derive(Debug, Hash, Eq, PartialEq)] -pub(crate) enum ConfigNodeKey { - Ratoml(SourceRootId), - Client, - User, -} - -impl Serialize for ConfigNodeKey { - fn serialize(&self, serializer: S) -> Result - where - S: serde::Serializer, - { - match self { - ConfigNodeKey::Ratoml(source_root_id) => serializer.serialize_u32(source_root_id.0), - ConfigNodeKey::Client => serializer.serialize_str("client"), - ConfigNodeKey::User => serializer.serialize_str("user"), - } - } -} - -#[derive(Debug, Serialize)] -enum ConfigNodeValue<'a> { - /// `rust-analyzer::config` module works by setting - /// a mapping between `SourceRootId` and `ConfigInput`. - /// Storing a `FileId` is mostly for debugging purposes. - Ratoml(&'a RatomlNode), - Client(&'a FullConfigInput), -} - impl Config { - /// FIXME @alibektas : Before integration tests, I thought I would - /// get the debug output of the config tree and do assertions based on it. - /// The reason why I didn't delete this is that we may want to have a lsp_ext - /// like "DebugConfigTree" so that it is easier for users to get a snapshot of - /// the config state for us to debug. - #[allow(dead_code)] - /// Walk towards the root starting from a specified `ConfigNode` - fn traverse( - &self, - start: ConfigNodeKey, - ) -> impl Iterator)> { - let mut v = vec![]; - - if let ConfigNodeKey::Ratoml(start) = start { - let mut par: Option = Some(start); - while let Some(source_root_id) = par { - par = self.source_root_parent_map.get(&start).copied(); - if let Some(config) = self.ratoml_files.get(&source_root_id) { - v.push(( - ConfigNodeKey::Ratoml(source_root_id), - ConfigNodeValue::Ratoml(config), - )); - } - } - } - - v.push((ConfigNodeKey::Client, ConfigNodeValue::Client(&self.client_config.node))); - - if let Some(user_config) = self.user_config.as_ref() { - v.push((ConfigNodeKey::User, ConfigNodeValue::Ratoml(user_config))); - } - - v.into_iter() - } - pub fn user_config_path(&self) -> &VfsPath { &self.user_config_path } - pub fn should_update(&self) -> bool { - self.should_update - } - // FIXME @alibektas : Server's health uses error sink but in other places it is not used atm. - pub fn apply_change(&self, change: ConfigChange, error_sink: &mut ConfigError) -> Config { + /// Changes made to client and global configurations will partially not be reflected even after `.apply_change()` was called. + /// The return tuple's bool component signals whether the `GlobalState` should call its `update_configuration()` method. + pub fn apply_change( + &self, + change: ConfigChange, + error_sink: &mut ConfigError, + ) -> (Config, bool) { let mut config = self.clone(); let mut toml_errors = vec![]; let mut json_errors = vec![]; - config.should_update = false; - - if let Some((file_id, change)) = change.user_config_change { - config.user_config = Some(RatomlNode { - file_id, - node: GlobalLocalConfigInput::from_toml( - toml::from_str(change.to_string().as_str()).unwrap(), - &mut toml_errors, - ), - }); - config.should_update = true; + let mut should_update = false; + + if let Some(change) = change.user_config_change { + if let Ok(change) = toml::from_str(&change) { + config.user_config = + Some(GlobalLocalConfigInput::from_toml(change, &mut toml_errors)); + should_update = true; + } } if let Some(mut json) = change.client_config_change { @@ -832,38 +740,29 @@ impl Config { patch_old_style::patch_json_for_outdated_configs(&mut json); - config.client_config = ClientConfig { - node: FullConfigInput::from_json(json, &mut json_errors), - detached_files, - } + config.client_config = FullConfigInput::from_json(json, &mut json_errors); + config.detached_files = detached_files; } - config.should_update = true; + should_update = true; } - if let Some((file_id, change)) = change.root_ratoml_change { - config.root_ratoml = Some(RatomlNode { - file_id, - node: GlobalLocalConfigInput::from_toml( - toml::from_str(change.to_string().as_str()).unwrap(), - &mut toml_errors, - ), - }); - config.should_update = true; + if let Some(change) = change.root_ratoml_change { + if let Ok(change) = toml::from_str(&change) { + config.root_ratoml = + Some(GlobalLocalConfigInput::from_toml(change, &mut toml_errors)); + should_update = true; + } } if let Some(change) = change.ratoml_file_change { - for (source_root_id, (file_id, _, text)) in change { + for (source_root_id, (_, text)) in change { if let Some(text) = text { - config.ratoml_files.insert( - source_root_id, - RatomlNode { - file_id, - node: GlobalLocalConfigInput::from_toml( - toml::from_str(&text).unwrap(), - &mut toml_errors, - ), - }, - ); + if let Ok(change) = toml::from_str(&text) { + config.ratoml_files.insert( + source_root_id, + GlobalLocalConfigInput::from_toml(change, &mut toml_errors), + ); + } } } } @@ -907,16 +806,22 @@ impl Config { serde_json::Error::custom("expected a non-empty string"), )); } - config + (config, should_update) + } + + pub fn apply_change_whatever(&self, change: ConfigChange) -> (Config, ConfigError) { + let mut e = ConfigError(vec![]); + let (config, _) = self.apply_change(change, &mut e); + (config, e) } } #[derive(Default, Debug)] pub struct ConfigChange { - user_config_change: Option<(FileId, String)>, - root_ratoml_change: Option<(FileId, String)>, + user_config_change: Option, + root_ratoml_change: Option, client_config_change: Option, - ratoml_file_change: Option)>>, + ratoml_file_change: Option)>>, source_map_change: Option>>, } @@ -924,26 +829,25 @@ impl ConfigChange { pub fn change_ratoml( &mut self, source_root: SourceRootId, - file_id: FileId, vfs_path: VfsPath, content: Option, - ) -> Option<(FileId, VfsPath, Option)> { + ) -> Option<(VfsPath, Option)> { if let Some(changes) = self.ratoml_file_change.as_mut() { - changes.insert(source_root, (file_id, vfs_path, content)) + changes.insert(source_root, (vfs_path, content)) } else { let mut map = FxHashMap::default(); - map.insert(source_root, (file_id, vfs_path, content)); + map.insert(source_root, (vfs_path, content)); self.ratoml_file_change = Some(map); None } } - pub fn change_user_config(&mut self, content: Option<(FileId, String)>) { + pub fn change_user_config(&mut self, content: Option) { assert!(self.user_config_change.is_none()); // Otherwise it is a double write. self.user_config_change = content; } - pub fn change_root_ratoml(&mut self, content: Option<(FileId, String)>) { + pub fn change_root_ratoml(&mut self, content: Option) { assert!(self.user_config_change.is_none()); // Otherwise it is a double write. self.root_ratoml_change = content; } @@ -1163,8 +1067,6 @@ impl ConfigError { } } -impl ConfigError {} - impl fmt::Display for ConfigError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let errors = self.0.iter().format_with("\n", |inner, f| match inner { @@ -1221,7 +1123,7 @@ impl Config { snippets: Default::default(), workspace_roots, visual_studio_code_version, - client_config: ClientConfig::default(), + client_config: FullConfigInput::default(), user_config: None, ratoml_files: FxHashMap::default(), default_config: DefaultConfigData::default(), @@ -1229,7 +1131,7 @@ impl Config { user_config_path, root_ratoml: None, root_ratoml_path, - should_update: false, + detached_files: Default::default(), } } @@ -1318,7 +1220,7 @@ impl Config { pub fn detached_files(&self) -> &Vec { // FIXME @alibektas : This is the only config that is confusing. If it's a proper configuration // why is it not among the others? If it's client only which I doubt it is current state should be alright - &self.client_config.detached_files + &self.detached_files } pub fn diagnostics(&self, source_root: Option) -> DiagnosticsConfig { @@ -2587,19 +2489,19 @@ macro_rules! _impl_for_config_data { while let Some(source_root_id) = par { par = self.source_root_parent_map.get(&source_root_id).copied(); if let Some(config) = self.ratoml_files.get(&source_root_id) { - if let Some(value) = config.node.local.$field.as_ref() { + if let Some(value) = config.local.$field.as_ref() { return value; } } } } - if let Some(v) = self.client_config.node.local.$field.as_ref() { + if let Some(v) = self.client_config.local.$field.as_ref() { return &v; } if let Some(user_config) = self.user_config.as_ref() { - if let Some(v) = user_config.node.local.$field.as_ref() { + if let Some(v) = user_config.local.$field.as_ref() { return &v; } } @@ -2621,17 +2523,17 @@ macro_rules! _impl_for_config_data { $vis fn $field(&self) -> &$ty { if let Some(root_path_ratoml) = self.root_ratoml.as_ref() { - if let Some(v) = root_path_ratoml.node.global.$field.as_ref() { + if let Some(v) = root_path_ratoml.global.$field.as_ref() { return &v; } } - if let Some(v) = self.client_config.node.global.$field.as_ref() { + if let Some(v) = self.client_config.global.$field.as_ref() { return &v; } if let Some(user_config) = self.user_config.as_ref() { - if let Some(v) = user_config.node.global.$field.as_ref() { + if let Some(v) = user_config.global.$field.as_ref() { return &v; } } @@ -2673,7 +2575,7 @@ macro_rules! _config_data { }) => { /// Default config values for this grouping. #[allow(non_snake_case)] - #[derive(Debug, Clone, Serialize)] + #[derive(Debug, Clone )] struct $name { $($field: $ty,)* } impl_for_config_data!{ @@ -3399,7 +3301,7 @@ mod tests { }})); let mut error_sink = ConfigError::default(); - config = config.apply_change(change, &mut error_sink); + (config, _) = config.apply_change(change, &mut error_sink); assert_eq!(config.proc_macro_srv(), None); } @@ -3419,7 +3321,7 @@ mod tests { }})); let mut error_sink = ConfigError::default(); - config = config.apply_change(change, &mut error_sink); + (config, _) = config.apply_change(change, &mut error_sink); assert_eq!(config.proc_macro_srv(), Some(AbsPathBuf::try_from(project_root()).unwrap())); } @@ -3441,7 +3343,7 @@ mod tests { }})); let mut error_sink = ConfigError::default(); - config = config.apply_change(change, &mut error_sink); + (config, _) = config.apply_change(change, &mut error_sink); assert_eq!( config.proc_macro_srv(), @@ -3466,7 +3368,7 @@ mod tests { })); let mut error_sink = ConfigError::default(); - config = config.apply_change(change, &mut error_sink); + (config, _) = config.apply_change(change, &mut error_sink); assert_eq!(config.cargo_targetDir(), &None); assert!( matches!(config.flycheck(), FlycheckConfig::CargoCommand { options, .. } if options.target_dir.is_none()) @@ -3489,7 +3391,7 @@ mod tests { })); let mut error_sink = ConfigError::default(); - config = config.apply_change(change, &mut error_sink); + (config, _) = config.apply_change(change, &mut error_sink); assert_eq!(config.cargo_targetDir(), &Some(TargetDirectory::UseSubdirectory(true))); assert!( @@ -3513,7 +3415,7 @@ mod tests { })); let mut error_sink = ConfigError::default(); - config = config.apply_change(change, &mut error_sink); + (config, _) = config.apply_change(change, &mut error_sink); assert_eq!( config.cargo_targetDir(), diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index 2210dab0f575..c289a07978d2 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -9,7 +9,7 @@ use crossbeam_channel::{unbounded, Receiver, Sender}; use flycheck::FlycheckHandle; use hir::ChangeWithProcMacros; use ide::{Analysis, AnalysisHost, Cancellable, FileId, SourceRootId}; -use ide_db::base_db::{CrateId, ProcMacroPaths}; +use ide_db::base_db::{CrateId, ProcMacroPaths, SourceDatabaseExt}; use load_cargo::SourceRootConfig; use lsp_types::{SemanticTokens, Url}; use nohash_hasher::IntMap; @@ -266,8 +266,8 @@ impl GlobalState { let mut ratoml_text_map: FxHashMap)> = FxHashMap::default(); - let mut user_config_file: Option<(FileId, Option)> = None; - let mut root_path_ratoml: Option<(FileId, Option)> = None; + let mut user_config_file: Option> = None; + let mut root_path_ratoml: Option> = None; let root_vfs_path = { let mut root_vfs_path = self.config.root_path().to_path_buf(); @@ -336,30 +336,23 @@ impl GlobalState { bytes.push((file.file_id, text)); } let (vfs, line_endings_map) = &mut *RwLockUpgradableReadGuard::upgrade(guard); - bytes.into_iter().for_each(|(file_id, text)| match text { - None => { - change.change_file(file_id, None); - if let Some(vfs_path) = modified_ratoml_files.get(&file_id) { - if vfs_path == self.config.user_config_path() { - user_config_file = Some((file_id, None)); - } else if vfs_path == &root_vfs_path { - root_path_ratoml = Some((file_id, None)); - } else { - ratoml_text_map.insert(file_id, (vfs_path.clone(), None)); - } + bytes.into_iter().for_each(|(file_id, text)| { + let text = match text { + None => None, + Some((text, line_endings)) => { + line_endings_map.insert(file_id, line_endings); + Some(text) } - } - Some((text, line_endings)) => { - line_endings_map.insert(file_id, line_endings); - change.change_file(file_id, Some(text.clone())); - if let Some(vfs_path) = modified_ratoml_files.get(&file_id) { - if vfs_path == self.config.user_config_path() { - user_config_file = Some((file_id, Some(text.clone()))); - } else if vfs_path == &root_vfs_path { - root_path_ratoml = Some((file_id, Some(text.clone()))); - } else { - ratoml_text_map.insert(file_id, (vfs_path.clone(), Some(text.clone()))); - } + }; + + change.change_file(file_id, text.clone()); + if let Some(vfs_path) = modified_ratoml_files.get(&file_id) { + if vfs_path == self.config.user_config_path() { + user_config_file = Some(text); + } else if vfs_path == &root_vfs_path { + root_path_ratoml = Some(text); + } else { + ratoml_text_map.insert(file_id, (vfs_path.clone(), text)); } } }); @@ -372,53 +365,54 @@ impl GlobalState { let _p = span!(Level::INFO, "GlobalState::process_changes/apply_change").entered(); self.analysis_host.apply_change(change); - - let config_change = { - let mut change = ConfigChange::default(); - let snap = self.analysis_host.analysis(); - - for (file_id, (vfs_path, text)) in ratoml_text_map { - // If change has been made to a ratoml file that - // belongs to a non-local source root, we will ignore it. - // As it doesn't make sense a users to use external config files. - if let Ok(source_root) = snap.source_root_id(file_id) { - if let Ok(true) = snap.is_local_source_root(source_root) { - if let Some((old_file, old_path, old_text)) = - change.change_ratoml(source_root, file_id, vfs_path.clone(), text) + if !(ratoml_text_map.is_empty() && user_config_file.is_none() && root_path_ratoml.is_none()) + { + let config_change = { + let mut change = ConfigChange::default(); + let db = self.analysis_host.raw_database(); + + for (file_id, (vfs_path, text)) in ratoml_text_map { + // If change has been made to a ratoml file that + // belongs to a non-local source root, we will ignore it. + // As it doesn't make sense a users to use external config files. + let sr_id = db.file_source_root(file_id); + let sr = db.source_root(sr_id); + if !sr.is_library { + if let Some((old_path, old_text)) = + change.change_ratoml(sr_id, vfs_path.clone(), text) { // SourceRoot has more than 1 RATOML files. In this case lexicographically smaller wins. if old_path < vfs_path { span!(Level::ERROR, "Two `rust-analyzer.toml` files were found inside the same crate. {vfs_path} has no effect."); // Put the old one back in. - change.change_ratoml(source_root, old_file, old_path, old_text); + change.change_ratoml(sr_id, old_path, old_text); } } + } else { + // Mapping to a SourceRoot should always end up in `Ok` + span!(Level::ERROR, "Mapping to SourceRootId failed."); } - } else { - // Mapping to a SourceRoot should always end up in `Ok` - span!(Level::ERROR, "Mapping to SourceRootId failed."); } - } - if let Some((file_id, Some(txt))) = user_config_file { - change.change_user_config(Some((file_id, txt))); - } - - if let Some((file_id, Some(txt))) = root_path_ratoml { - change.change_root_ratoml(Some((file_id, txt))); - } + if let Some(Some(txt)) = user_config_file { + change.change_user_config(Some(txt)); + } - change - }; + if let Some(Some(txt)) = root_path_ratoml { + change.change_root_ratoml(Some(txt)); + } - let mut error_sink = ConfigError::default(); - let config = self.config.apply_change(config_change, &mut error_sink); + change + }; + let mut error_sink = ConfigError::default(); + let (config, should_update) = self.config.apply_change(config_change, &mut error_sink); - if config.should_update() { - self.update_configuration(config); - } else { - // No global or client level config was changed. So we can just naively replace config. - self.config = Arc::new(config); + if should_update { + self.update_configuration(config); + } else { + // No global or client level config was changed. So we can just naively replace config. + self.config = Arc::new(config); + } } { diff --git a/crates/rust-analyzer/src/handlers/notification.rs b/crates/rust-analyzer/src/handlers/notification.rs index 123a9a06a3b8..bdb27043ebe7 100644 --- a/crates/rust-analyzer/src/handlers/notification.rs +++ b/crates/rust-analyzer/src/handlers/notification.rs @@ -201,7 +201,8 @@ pub(crate) fn handle_did_change_configuration( let mut change = ConfigChange::default(); change.change_client_config(json.take()); let mut error_sink = ConfigError::default(); - config = config.apply_change(change, &mut error_sink); + (config, _) = config.apply_change(change, &mut error_sink); + // Client config changes neccesitates .update_config method to be called. this.update_configuration(config); } } diff --git a/crates/rust-analyzer/src/lib.rs b/crates/rust-analyzer/src/lib.rs index d9b31550c562..b3c11d0156ed 100644 --- a/crates/rust-analyzer/src/lib.rs +++ b/crates/rust-analyzer/src/lib.rs @@ -39,7 +39,7 @@ pub mod tracing { } pub mod config; -pub mod global_state; +mod global_state; pub mod lsp; use self::lsp::ext as lsp_ext; diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index e804ba3db955..364a0083e591 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -605,7 +605,8 @@ impl GlobalState { let mut config_change = ConfigChange::default(); config_change.change_source_root_parent_map(self.local_roots_parent_map.clone()); let mut error_sink = ConfigError::default(); - self.config = Arc::new(self.config.apply_change(config_change, &mut error_sink)); + let (config, _) = self.config.apply_change(config_change, &mut error_sink); + self.config = Arc::new(config); self.recreate_crate_graph(cause); diff --git a/crates/rust-analyzer/tests/slow-tests/main.rs b/crates/rust-analyzer/tests/slow-tests/main.rs index fd6f79abc1cc..f886df60e681 100644 --- a/crates/rust-analyzer/tests/slow-tests/main.rs +++ b/crates/rust-analyzer/tests/slow-tests/main.rs @@ -11,36 +11,33 @@ #![warn(rust_2018_idioms, unused_lifetimes)] #![allow(clippy::disallowed_types)] +mod ratoml; #[cfg(not(feature = "in-rust-tree"))] mod sourcegen; mod support; mod testdir; mod tidy; -use std::{collections::HashMap, path::PathBuf, sync::Once, time::Instant}; +use std::{collections::HashMap, path::PathBuf, time::Instant}; use lsp_types::{ - notification::{DidChangeTextDocument, DidOpenTextDocument, DidSaveTextDocument}, + notification::DidOpenTextDocument, request::{ CodeActionRequest, Completion, Formatting, GotoTypeDefinition, HoverRequest, InlayHintRequest, InlayHintResolveRequest, WillRenameFiles, WorkspaceSymbolRequest, }, - CodeAction, CodeActionContext, CodeActionOrCommand, CodeActionParams, CodeActionResponse, - CompletionParams, DidChangeTextDocumentParams, DidOpenTextDocumentParams, - DidSaveTextDocumentParams, DocumentFormattingParams, FileRename, FormattingOptions, - GotoDefinitionParams, Hover, HoverParams, InlayHint, InlayHintLabel, InlayHintParams, - PartialResultParams, Position, Range, RenameFilesParams, TextDocumentContentChangeEvent, - TextDocumentIdentifier, TextDocumentItem, TextDocumentPositionParams, Url, - VersionedTextDocumentIdentifier, WorkDoneProgressParams, + CodeActionContext, CodeActionParams, CompletionParams, DidOpenTextDocumentParams, + DocumentFormattingParams, FileRename, FormattingOptions, GotoDefinitionParams, HoverParams, + InlayHint, InlayHintLabel, InlayHintParams, PartialResultParams, Position, Range, + RenameFilesParams, TextDocumentItem, TextDocumentPositionParams, WorkDoneProgressParams, }; -use paths::Utf8PathBuf; + use rust_analyzer::lsp::ext::{OnEnter, Runnables, RunnablesParams, UnindexedProject}; use serde_json::json; use stdx::format_to_acc; -use support::Server; + use test_utils::skip_slow_tests; use testdir::TestDir; -use tracing_subscriber::fmt::TestWriter; use crate::support::{project, Project}; @@ -1471,1027 +1468,3 @@ version = "0.0.0" server.request::(Default::default(), json!([])); } - -enum QueryType { - AssistEmitMustUse, - /// A query whose config key is a part of the global configs, so that - /// testing for changes to this config means testing if global changes - /// take affect. - GlobalHover, -} - -struct RatomlTest { - urls: Vec, - server: Server, - tmp_path: Utf8PathBuf, - user_config_dir: Utf8PathBuf, -} - -impl RatomlTest { - const EMIT_MUST_USE: &'static str = r#"assist.emitMustUse = true"#; - const EMIT_MUST_NOT_USE: &'static str = r#"assist.emitMustUse = false"#; - const EMIT_MUST_USE_SNIPPET: &'static str = r#" - -impl Value { - #[must_use] - fn as_text(&self) -> Option<&String> { - if let Self::Text(v) = self { - Some(v) - } else { - None - } - } -}"#; - - const GLOBAL_TRAIT_ASSOC_ITEMS_ZERO: &'static str = r#"hover.show.traitAssocItems = 0"#; - const GLOBAL_TRAIT_ASSOC_ITEMS_SNIPPET: &'static str = r#" -```rust -p1 -``` - -```rust -trait RandomTrait { - type B; - fn abc() -> i32; - fn def() -> i64; -} -```"#; - - fn new( - fixtures: Vec<&str>, - roots: Vec<&str>, - client_config: Option, - ) -> Self { - // setup(); - let tmp_dir = TestDir::new(); - let tmp_path = tmp_dir.path().to_owned(); - - 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); - - for root in roots { - project = project.root(root); - } - - if let Some(client_config) = client_config { - project = project.with_config(client_config); - } - - let server = project.server().wait_until_workspace_is_loaded(); - - let mut case = Self { urls: vec![], server, tmp_path, user_config_dir }; - let urls = fixtures.iter().map(|fixture| case.fixture_path(fixture)).collect::>(); - case.urls = urls; - case - } - - fn fixture_path(&self, fixture: &str) -> Url { - let mut lines = fixture.trim().split('\n'); - - let mut path = - lines.next().expect("All files in a fixture are expected to have at least one line."); - - if path.starts_with("//- minicore") { - path = lines.next().expect("A minicore line must be followed by a path.") - } - - path = path.strip_prefix("//- ").expect("Path must be preceded by a //- prefix "); - - let spl = path[1..].split('/'); - let mut path = self.tmp_path.clone(); - - let mut spl = spl.into_iter(); - if let Some(first) = spl.next() { - if first == "$$CONFIG_DIR$$" { - path = self.user_config_dir.clone(); - } else { - path = path.join(first); - } - } - for piece in spl { - path = path.join(piece); - } - - Url::parse( - format!( - "file://{}", - path.into_string().to_owned().replace("C:\\", "/c:/").replace('\\', "/") - ) - .as_str(), - ) - .unwrap() - } - - fn create(&mut self, fixture_path: &str, text: String) { - let url = self.fixture_path(fixture_path); - - self.server.notification::(DidOpenTextDocumentParams { - text_document: TextDocumentItem { - uri: url.clone(), - language_id: "rust".to_owned(), - version: 0, - text: String::new(), - }, - }); - - self.server.notification::(DidChangeTextDocumentParams { - text_document: VersionedTextDocumentIdentifier { uri: url, version: 0 }, - content_changes: vec![TextDocumentContentChangeEvent { - range: None, - range_length: None, - text, - }], - }); - } - - fn delete(&mut self, file_idx: usize) { - self.server.notification::(DidOpenTextDocumentParams { - text_document: TextDocumentItem { - uri: self.urls[file_idx].clone(), - language_id: "rust".to_owned(), - version: 0, - text: "".to_owned(), - }, - }); - - // See if deleting ratoml file will make the config of interest to return to its default value. - self.server.notification::(DidSaveTextDocumentParams { - text_document: TextDocumentIdentifier { uri: self.urls[file_idx].clone() }, - text: Some("".to_owned()), - }); - } - - fn edit(&mut self, file_idx: usize, text: String) { - self.server.notification::(DidOpenTextDocumentParams { - text_document: TextDocumentItem { - uri: self.urls[file_idx].clone(), - language_id: "rust".to_owned(), - version: 0, - text: String::new(), - }, - }); - - self.server.notification::(DidChangeTextDocumentParams { - text_document: VersionedTextDocumentIdentifier { - uri: self.urls[file_idx].clone(), - version: 0, - }, - content_changes: vec![TextDocumentContentChangeEvent { - range: None, - range_length: None, - text, - }], - }); - } - - fn query(&self, query: QueryType, source_file_idx: usize) -> bool { - match query { - QueryType::AssistEmitMustUse => { - let res = self.server.send_request::(CodeActionParams { - text_document: TextDocumentIdentifier { - uri: self.urls[source_file_idx].clone(), - }, - range: lsp_types::Range { - start: Position::new(2, 13), - end: Position::new(2, 15), - }, - context: CodeActionContext { - diagnostics: vec![], - only: None, - trigger_kind: None, - }, - work_done_progress_params: WorkDoneProgressParams { work_done_token: None }, - partial_result_params: lsp_types::PartialResultParams { - partial_result_token: None, - }, - }); - - let res = serde_json::de::from_str::(res.to_string().as_str()) - .unwrap(); - - // The difference setting the new config key will cause can be seen in the lower layers of this nested response - // so here are some ugly unwraps and other obscure stuff. - let ca: CodeAction = res - .into_iter() - .find_map(|it| { - if let CodeActionOrCommand::CodeAction(ca) = it { - if ca.title.as_str() == "Generate an `as_` method for this enum variant" - { - return Some(ca); - } - } - - None - }) - .unwrap(); - - if let lsp_types::DocumentChanges::Edits(edits) = - ca.edit.unwrap().document_changes.unwrap() - { - if let lsp_types::OneOf::Left(l) = &edits[0].edits[0] { - return l.new_text.as_str() == RatomlTest::EMIT_MUST_USE_SNIPPET; - } - } - } - QueryType::GlobalHover => { - let res = self.server.send_request::(HoverParams { - work_done_progress_params: WorkDoneProgressParams { work_done_token: None }, - text_document_position_params: TextDocumentPositionParams { - text_document: TextDocumentIdentifier { - uri: self.urls[source_file_idx].clone(), - }, - position: Position::new(7, 18), - }, - }); - let res = serde_json::de::from_str::(res.to_string().as_str()).unwrap(); - assert!(matches!(res.contents, lsp_types::HoverContents::Markup(_))); - if let lsp_types::HoverContents::Markup(m) = res.contents { - return m.value == RatomlTest::GLOBAL_TRAIT_ASSOC_ITEMS_SNIPPET; - } - } - } - - panic!() - } -} - -static INIT: Once = Once::new(); - -fn setup() { - INIT.call_once(|| { - let trc = rust_analyzer::tracing::Config { - writer: TestWriter::default(), - // Deliberately enable all `error` logs if the user has not set RA_LOG, as there is usually - // useful information in there for debugging. - filter: std::env::var("RA_LOG").ok().unwrap_or_else(|| "error".to_owned()), - chalk_filter: std::env::var("CHALK_DEBUG").ok(), - profile_filter: std::env::var("RA_PROFILE").ok(), - }; - - trc.init().unwrap(); - }); -} - -// /// Check if we are listening for changes in user's config file ( e.g on Linux `~/.config/rust-analyzer/.rust-analyzer.toml`) -// #[test] -// #[cfg(target_os = "windows")] -// fn listen_to_user_config_scenario_windows() { -// todo!() -// } - -// #[test] -// #[cfg(target_os = "linux")] -// fn listen_to_user_config_scenario_linux() { -// todo!() -// } - -// #[test] -// #[cfg(target_os = "macos")] -// fn listen_to_user_config_scenario_macos() { -// todo!() -// } - -/// Check if made changes have had any effect on -/// the client config. -#[test] -fn ratoml_client_config_basic() { - let server = RatomlTest::new( - vec![ - r#" -//- /p1/Cargo.toml -[package] -name = "p1" -version = "0.1.0" -edition = "2021" -"#, - r#"//- /p1/src/lib.rs -enum Value { - Number(i32), - Text(String), -}"#, - ], - vec!["p1"], - Some(json!({ - "assist" : { - "emitMustUse" : true - } - })), - ); - - assert!(server.query(QueryType::AssistEmitMustUse, 1)); -} - -/// Checks if client config can be modified. -/// FIXME @alibektas : This test is atm not valid. -/// Asking for client config from the client is a 2 way communication -/// which we cannot imitate with the current slow-tests infrastructure. -/// See rust-analyzer::handlers::notifications#197 -// #[test] -// fn client_config_update() { -// setup(); - -// let server = RatomlTest::new( -// vec![ -// r#" -// //- /p1/Cargo.toml -// [package] -// name = "p1" -// version = "0.1.0" -// edition = "2021" -// "#, -// r#" -// //- /p1/src/lib.rs -// enum Value { -// Number(i32), -// Text(String), -// }"#, -// ], -// vec!["p1"], -// None, -// ); - -// assert!(!server.query(QueryType::AssistEmitMustUse, 1)); - -// // a.notification::(DidChangeConfigurationParams { -// // settings: json!({ -// // "assists" : { -// // "emitMustUse" : true -// // } -// // }), -// // }); - -// assert!(server.query(QueryType::AssistEmitMustUse, 1)); -// } - -// #[test] -// fn ratoml_create_ratoml_basic() { -// let server = RatomlTest::new( -// vec![ -// r#" -// //- /p1/Cargo.toml -// [package] -// name = "p1" -// version = "0.1.0" -// edition = "2021" -// "#, -// r#" -// //- /p1/rust-analyzer.toml -// assist.emitMustUse = true -// "#, -// r#" -// //- /p1/src/lib.rs -// enum Value { -// Number(i32), -// Text(String), -// } -// "#, -// ], -// vec!["p1"], -// None, -// ); - -// assert!(server.query(QueryType::AssistEmitMustUse, 2)); -// } - -#[test] -fn ratoml_user_config_detected() { - let server = RatomlTest::new( - vec![ - r#" -//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml -assist.emitMustUse = true -"#, - r#" -//- /p1/Cargo.toml -[package] -name = "p1" -version = "0.1.0" -edition = "2021" -"#, - r#"//- /p1/src/lib.rs -enum Value { - Number(i32), - Text(String), -}"#, - ], - vec!["p1"], - None, - ); - - assert!(server.query(QueryType::AssistEmitMustUse, 2)); -} - -#[test] -fn ratoml_create_user_config() { - setup(); - let mut server = RatomlTest::new( - vec![ - r#" -//- /p1/Cargo.toml -[package] -name = "p1" -version = "0.1.0" -edition = "2021" -"#, - r#" -//- /p1/src/lib.rs -enum Value { - Number(i32), - Text(String), -}"#, - ], - vec!["p1"], - None, - ); - - assert!(!server.query(QueryType::AssistEmitMustUse, 1)); - server.create( - "//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml", - RatomlTest::EMIT_MUST_USE.to_owned(), - ); - assert!(server.query(QueryType::AssistEmitMustUse, 1)); -} - -#[test] -fn ratoml_modify_user_config() { - let mut server = RatomlTest::new( - vec![ - r#" -//- /p1/Cargo.toml -[package] -name = "p1" -version = "0.1.0" -edition = "2021""#, - r#" -//- /p1/src/lib.rs -enum Value { - Number(i32), - Text(String), -}"#, - r#" -//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml -assist.emitMustUse = true"#, - ], - vec!["p1"], - None, - ); - - assert!(server.query(QueryType::AssistEmitMustUse, 1)); - server.edit(2, String::new()); - assert!(!server.query(QueryType::AssistEmitMustUse, 1)); -} - -#[test] -fn ratoml_delete_user_config() { - let mut server = RatomlTest::new( - vec![ - r#" -//- /p1/Cargo.toml -[package] -name = "p1" -version = "0.1.0" -edition = "2021""#, - r#" -//- /p1/src/lib.rs -enum Value { - Number(i32), - Text(String), -}"#, - r#" -//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml -assist.emitMustUse = true"#, - ], - vec!["p1"], - None, - ); - - assert!(server.query(QueryType::AssistEmitMustUse, 1)); - server.delete(2); - assert!(!server.query(QueryType::AssistEmitMustUse, 1)); -} -// #[test] -// fn delete_user_config() { -// todo!() -// } - -// #[test] -// fn modify_client_config() { -// todo!() -// } - -#[test] -fn ratoml_inherit_config_from_ws_root() { - let server = RatomlTest::new( - vec![ - r#" -//- /p1/Cargo.toml -workspace = { members = ["p2"] } -[package] -name = "p1" -version = "0.1.0" -edition = "2021" -"#, - r#" -//- /p1/rust-analyzer.toml -assist.emitMustUse = true -"#, - r#" -//- /p1/p2/Cargo.toml -[package] -name = "p2" -version = "0.1.0" -edition = "2021" -"#, - r#" -//- /p1/p2/src/lib.rs -enum Value { - Number(i32), - Text(String), -}"#, - r#" -//- /p1/src/lib.rs -pub fn add(left: usize, right: usize) -> usize { - left + right -} -"#, - ], - vec!["p1"], - None, - ); - - assert!(server.query(QueryType::AssistEmitMustUse, 3)); -} - -#[test] -fn ratoml_modify_ratoml_at_ws_root() { - let mut server = RatomlTest::new( - vec![ - r#" -//- /p1/Cargo.toml -workspace = { members = ["p2"] } -[package] -name = "p1" -version = "0.1.0" -edition = "2021" -"#, - r#" -//- /p1/rust-analyzer.toml -assist.emitMustUse = false -"#, - r#" -//- /p1/p2/Cargo.toml -[package] -name = "p2" -version = "0.1.0" -edition = "2021" -"#, - r#" -//- /p1/p2/src/lib.rs -enum Value { - Number(i32), - Text(String), -}"#, - r#" -//- /p1/src/lib.rs -pub fn add(left: usize, right: usize) -> usize { - left + right -} -"#, - ], - vec!["p1"], - None, - ); - - assert!(!server.query(QueryType::AssistEmitMustUse, 3)); - server.edit(1, "assist.emitMustUse = true".to_owned()); - assert!(server.query(QueryType::AssistEmitMustUse, 3)); -} - -#[test] -fn ratoml_delete_ratoml_at_ws_root() { - let mut server = RatomlTest::new( - vec![ - r#" -//- /p1/Cargo.toml -workspace = { members = ["p2"] } -[package] -name = "p1" -version = "0.1.0" -edition = "2021" -"#, - r#" -//- /p1/rust-analyzer.toml -assist.emitMustUse = true -"#, - r#" -//- /p1/p2/Cargo.toml -[package] -name = "p2" -version = "0.1.0" -edition = "2021" -"#, - r#" -//- /p1/p2/src/lib.rs -enum Value { - Number(i32), - Text(String), -}"#, - r#" -//- /p1/src/lib.rs -pub fn add(left: usize, right: usize) -> usize { - left + right -} -"#, - ], - vec!["p1"], - None, - ); - - assert!(server.query(QueryType::AssistEmitMustUse, 3)); - server.delete(1); - assert!(!server.query(QueryType::AssistEmitMustUse, 3)); -} - -#[test] -fn ratoml_add_immediate_child_to_ws_root() { - let mut server = RatomlTest::new( - vec![ - r#" -//- /p1/Cargo.toml -workspace = { members = ["p2"] } -[package] -name = "p1" -version = "0.1.0" -edition = "2021" -"#, - r#" -//- /p1/rust-analyzer.toml -assist.emitMustUse = true -"#, - r#" -//- /p1/p2/Cargo.toml -[package] -name = "p2" -version = "0.1.0" -edition = "2021" -"#, - r#" -//- /p1/p2/src/lib.rs -enum Value { - Number(i32), - Text(String), -}"#, - r#" -//- /p1/src/lib.rs -pub fn add(left: usize, right: usize) -> usize { - left + right -} -"#, - ], - vec!["p1"], - None, - ); - - assert!(server.query(QueryType::AssistEmitMustUse, 3)); - server.create("//- /p1/p2/rust-analyzer.toml", RatomlTest::EMIT_MUST_NOT_USE.to_owned()); - assert!(!server.query(QueryType::AssistEmitMustUse, 3)); -} - -#[test] -fn ratoml_rm_ws_root_ratoml_child_has_client_as_parent_now() { - let mut server = RatomlTest::new( - vec![ - r#" -//- /p1/Cargo.toml -workspace = { members = ["p2"] } -[package] -name = "p1" -version = "0.1.0" -edition = "2021" -"#, - r#" -//- /p1/rust-analyzer.toml -assist.emitMustUse = true -"#, - r#" -//- /p1/p2/Cargo.toml -[package] -name = "p2" -version = "0.1.0" -edition = "2021" -"#, - r#" -//- /p1/p2/src/lib.rs -enum Value { - Number(i32), - Text(String), -}"#, - r#" -//- /p1/src/lib.rs -pub fn add(left: usize, right: usize) -> usize { - left + right -} -"#, - ], - vec!["p1"], - None, - ); - - assert!(server.query(QueryType::AssistEmitMustUse, 3)); - server.delete(1); - assert!(!server.query(QueryType::AssistEmitMustUse, 3)); -} - -#[test] -fn ratoml_crates_both_roots() { - let server = RatomlTest::new( - vec![ - r#" -//- /p1/Cargo.toml -workspace = { members = ["p2"] } -[package] -name = "p1" -version = "0.1.0" -edition = "2021" -"#, - r#" -//- /p1/rust-analyzer.toml -assist.emitMustUse = true -"#, - r#" -//- /p1/p2/Cargo.toml -[package] -name = "p2" -version = "0.1.0" -edition = "2021" -"#, - r#" -//- /p1/p2/src/lib.rs -enum Value { - Number(i32), - Text(String), -}"#, - r#" -//- /p1/src/lib.rs -enum Value { - Number(i32), - Text(String), -}"#, - ], - vec!["p1", "p2"], - None, - ); - - assert!(server.query(QueryType::AssistEmitMustUse, 3)); - assert!(server.query(QueryType::AssistEmitMustUse, 4)); -} - -#[test] -fn ratoml_multiple_ratoml_in_single_source_root() { - let server = RatomlTest::new( - vec![ - r#" - //- /p1/Cargo.toml - [package] - name = "p1" - version = "0.1.0" - edition = "2021" - "#, - r#" - //- /p1/rust-analyzer.toml - assist.emitMustUse = true - "#, - r#" - //- /p1/src/rust-analyzer.toml - assist.emitMustUse = false - "#, - r#" - //- /p1/src/lib.rs - enum Value { - Number(i32), - Text(String), - } - "#, - ], - vec!["p1"], - None, - ); - - assert!(server.query(QueryType::AssistEmitMustUse, 3)); - - let server = RatomlTest::new( - vec![ - r#" -//- /p1/Cargo.toml -[package] -name = "p1" -version = "0.1.0" -edition = "2021" -"#, - r#" -//- /p1/src/rust-analyzer.toml -assist.emitMustUse = false -"#, - r#" -//- /p1/rust-analyzer.toml -assist.emitMustUse = true -"#, - r#" -//- /p1/src/lib.rs -enum Value { - Number(i32), - Text(String), -} -"#, - ], - vec!["p1"], - None, - ); - - assert!(server.query(QueryType::AssistEmitMustUse, 3)); -} - -/// If a root is non-local, so we cannot find what its parent is -/// in our `config.local_root_parent_map`. So if any config should -/// apply, it must be looked for starting from the client level. -/// FIXME @alibektas : "locality" is according to ra that, which is simply in the file system. -/// This doesn't really help us with what we want to achieve here. -// #[test] -// fn ratoml_non_local_crates_start_inheriting_from_client() { -// let server = RatomlTest::new( -// vec![ -// r#" -// //- /p1/Cargo.toml -// [package] -// name = "p1" -// version = "0.1.0" -// edition = "2021" - -// [dependencies] -// p2 = { path = "../p2" } -// #, -// r#" -// //- /p1/src/lib.rs -// enum Value { -// Number(i32), -// Text(String), -// } - -// use p2; - -// pub fn add(left: usize, right: usize) -> usize { -// p2::add(left, right) -// } - -// #[cfg(test)] -// mod tests { -// use super::*; - -// #[test] -// fn it_works() { -// let result = add(2, 2); -// assert_eq!(result, 4); -// } -// }"#, -// r#" -// //- /p2/Cargo.toml -// [package] -// name = "p2" -// version = "0.1.0" -// edition = "2021" - -// # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html - -// [dependencies] -// "#, -// r#" -// //- /p2/rust-analyzer.toml -// # DEF -// assist.emitMustUse = true -// "#, -// r#" -// //- /p2/src/lib.rs -// enum Value { -// Number(i32), -// Text(String), -// }"#, -// ], -// vec!["p1", "p2"], -// None, -// ); - -// assert!(!server.query(QueryType::AssistEmitMustUse, 5)); -// } - -/// Having a ratoml file at the root of a project enables -/// configuring global level configurations as well. -#[test] -fn ratoml_in_root_is_global() { - let server = RatomlTest::new( - vec![ - r#" -//- /p1/Cargo.toml -[package] -name = "p1" -version = "0.1.0" -edition = "2021" - "#, - r#" -//- /rust-analyzer.toml -hover.show.traitAssocItems = 4 - "#, - r#" -//- /p1/src/lib.rs -trait RandomTrait { - type B; - fn abc() -> i32; - fn def() -> i64; -} - -fn main() { - let a = RandomTrait; -}"#, - ], - vec![], - None, - ); - - server.query(QueryType::GlobalHover, 2); -} - -#[test] -fn ratoml_root_is_updateable() { - let mut server = RatomlTest::new( - vec![ - r#" -//- /p1/Cargo.toml -[package] -name = "p1" -version = "0.1.0" -edition = "2021" - "#, - r#" -//- /rust-analyzer.toml -hover.show.traitAssocItems = 4 - "#, - r#" -//- /p1/src/lib.rs -trait RandomTrait { - type B; - fn abc() -> i32; - fn def() -> i64; -} - -fn main() { - let a = RandomTrait; -}"#, - ], - vec![], - None, - ); - - assert!(server.query(QueryType::GlobalHover, 2)); - server.edit(1, RatomlTest::GLOBAL_TRAIT_ASSOC_ITEMS_ZERO.to_owned()); - assert!(!server.query(QueryType::GlobalHover, 2)); -} - -#[test] -fn ratoml_root_is_deletable() { - let mut server = RatomlTest::new( - vec![ - r#" -//- /p1/Cargo.toml -[package] -name = "p1" -version = "0.1.0" -edition = "2021" - "#, - r#" -//- /rust-analyzer.toml -hover.show.traitAssocItems = 4 - "#, - r#" -//- /p1/src/lib.rs -trait RandomTrait { - type B; - fn abc() -> i32; - fn def() -> i64; -} - -fn main() { - let a = RandomTrait; -}"#, - ], - vec![], - None, - ); - - assert!(server.query(QueryType::GlobalHover, 2)); - server.delete(1); - assert!(!server.query(QueryType::GlobalHover, 2)); -} diff --git a/crates/rust-analyzer/tests/slow-tests/ratoml.rs b/crates/rust-analyzer/tests/slow-tests/ratoml.rs new file mode 100644 index 000000000000..739da9c998d5 --- /dev/null +++ b/crates/rust-analyzer/tests/slow-tests/ratoml.rs @@ -0,0 +1,1019 @@ +use crate::support::{Project, Server}; +use crate::testdir::TestDir; +use lsp_types::{ + notification::{DidChangeTextDocument, DidOpenTextDocument, DidSaveTextDocument}, + request::{CodeActionRequest, HoverRequest}, + CodeAction, CodeActionContext, CodeActionOrCommand, CodeActionParams, CodeActionResponse, + DidChangeTextDocumentParams, DidOpenTextDocumentParams, DidSaveTextDocumentParams, Hover, + HoverParams, Position, TextDocumentContentChangeEvent, TextDocumentIdentifier, + TextDocumentItem, TextDocumentPositionParams, Url, VersionedTextDocumentIdentifier, + WorkDoneProgressParams, +}; +use paths::Utf8PathBuf; + +use serde_json::json; + +enum QueryType { + AssistEmitMustUse, + /// A query whose config key is a part of the global configs, so that + /// testing for changes to this config means testing if global changes + /// take affect. + GlobalHover, +} + +struct RatomlTest { + urls: Vec, + server: Server, + tmp_path: Utf8PathBuf, + user_config_dir: Utf8PathBuf, +} + +impl RatomlTest { + const EMIT_MUST_USE: &'static str = r#"assist.emitMustUse = true"#; + const EMIT_MUST_NOT_USE: &'static str = r#"assist.emitMustUse = false"#; + const EMIT_MUST_USE_SNIPPET: &'static str = r#" + +impl Value { + #[must_use] + fn as_text(&self) -> Option<&String> { + if let Self::Text(v) = self { + Some(v) + } else { + None + } + } +}"#; + + const GLOBAL_TRAIT_ASSOC_ITEMS_ZERO: &'static str = r#"hover.show.traitAssocItems = 0"#; + const GLOBAL_TRAIT_ASSOC_ITEMS_SNIPPET: &'static str = r#" +```rust +p1 +``` + +```rust +trait RandomTrait { + type B; + fn abc() -> i32; + fn def() -> i64; +} +```"#; + + fn new( + fixtures: Vec<&str>, + roots: Vec<&str>, + client_config: Option, + ) -> Self { + let tmp_dir = TestDir::new(); + let tmp_path = tmp_dir.path().to_owned(); + + 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); + + for root in roots { + project = project.root(root); + } + + if let Some(client_config) = client_config { + project = project.with_config(client_config); + } + + let server = project.server().wait_until_workspace_is_loaded(); + + let mut case = Self { urls: vec![], server, tmp_path, user_config_dir }; + let urls = fixtures.iter().map(|fixture| case.fixture_path(fixture)).collect::>(); + case.urls = urls; + case + } + + fn fixture_path(&self, fixture: &str) -> Url { + let mut lines = fixture.trim().split('\n'); + + let mut path = + lines.next().expect("All files in a fixture are expected to have at least one line."); + + if path.starts_with("//- minicore") { + path = lines.next().expect("A minicore line must be followed by a path.") + } + + path = path.strip_prefix("//- ").expect("Path must be preceded by a //- prefix "); + + let spl = path[1..].split('/'); + let mut path = self.tmp_path.clone(); + + let mut spl = spl.into_iter(); + if let Some(first) = spl.next() { + if first == "$$CONFIG_DIR$$" { + path = self.user_config_dir.clone(); + } else { + path = path.join(first); + } + } + for piece in spl { + path = path.join(piece); + } + + Url::parse( + format!( + "file://{}", + path.into_string().to_owned().replace("C:\\", "/c:/").replace('\\', "/") + ) + .as_str(), + ) + .unwrap() + } + + fn create(&mut self, fixture_path: &str, text: String) { + let url = self.fixture_path(fixture_path); + + self.server.notification::(DidOpenTextDocumentParams { + text_document: TextDocumentItem { + uri: url.clone(), + language_id: "rust".to_owned(), + version: 0, + text: String::new(), + }, + }); + + self.server.notification::(DidChangeTextDocumentParams { + text_document: VersionedTextDocumentIdentifier { uri: url, version: 0 }, + content_changes: vec![TextDocumentContentChangeEvent { + range: None, + range_length: None, + text, + }], + }); + } + + fn delete(&mut self, file_idx: usize) { + self.server.notification::(DidOpenTextDocumentParams { + text_document: TextDocumentItem { + uri: self.urls[file_idx].clone(), + language_id: "rust".to_owned(), + version: 0, + text: "".to_owned(), + }, + }); + + // See if deleting ratoml file will make the config of interest to return to its default value. + self.server.notification::(DidSaveTextDocumentParams { + text_document: TextDocumentIdentifier { uri: self.urls[file_idx].clone() }, + text: Some("".to_owned()), + }); + } + + fn edit(&mut self, file_idx: usize, text: String) { + self.server.notification::(DidOpenTextDocumentParams { + text_document: TextDocumentItem { + uri: self.urls[file_idx].clone(), + language_id: "rust".to_owned(), + version: 0, + text: String::new(), + }, + }); + + self.server.notification::(DidChangeTextDocumentParams { + text_document: VersionedTextDocumentIdentifier { + uri: self.urls[file_idx].clone(), + version: 0, + }, + content_changes: vec![TextDocumentContentChangeEvent { + range: None, + range_length: None, + text, + }], + }); + } + + fn query(&self, query: QueryType, source_file_idx: usize) -> bool { + match query { + QueryType::AssistEmitMustUse => { + let res = self.server.send_request::(CodeActionParams { + text_document: TextDocumentIdentifier { + uri: self.urls[source_file_idx].clone(), + }, + range: lsp_types::Range { + start: Position::new(2, 13), + end: Position::new(2, 15), + }, + context: CodeActionContext { + diagnostics: vec![], + only: None, + trigger_kind: None, + }, + work_done_progress_params: WorkDoneProgressParams { work_done_token: None }, + partial_result_params: lsp_types::PartialResultParams { + partial_result_token: None, + }, + }); + + let res = serde_json::de::from_str::(res.to_string().as_str()) + .unwrap(); + + // The difference setting the new config key will cause can be seen in the lower layers of this nested response + // so here are some ugly unwraps and other obscure stuff. + let ca: CodeAction = res + .into_iter() + .find_map(|it| { + if let CodeActionOrCommand::CodeAction(ca) = it { + if ca.title.as_str() == "Generate an `as_` method for this enum variant" + { + return Some(ca); + } + } + + None + }) + .unwrap(); + + if let lsp_types::DocumentChanges::Edits(edits) = + ca.edit.unwrap().document_changes.unwrap() + { + if let lsp_types::OneOf::Left(l) = &edits[0].edits[0] { + return l.new_text.as_str() == RatomlTest::EMIT_MUST_USE_SNIPPET; + } + } + } + QueryType::GlobalHover => { + let res = self.server.send_request::(HoverParams { + work_done_progress_params: WorkDoneProgressParams { work_done_token: None }, + text_document_position_params: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { + uri: self.urls[source_file_idx].clone(), + }, + position: Position::new(7, 18), + }, + }); + let res = serde_json::de::from_str::(res.to_string().as_str()).unwrap(); + assert!(matches!(res.contents, lsp_types::HoverContents::Markup(_))); + if let lsp_types::HoverContents::Markup(m) = res.contents { + return m.value == RatomlTest::GLOBAL_TRAIT_ASSOC_ITEMS_SNIPPET; + } + } + } + + panic!() + } +} + +// /// Check if we are listening for changes in user's config file ( e.g on Linux `~/.config/rust-analyzer/.rust-analyzer.toml`) +// #[test] +// #[cfg(target_os = "windows")] +// fn listen_to_user_config_scenario_windows() { +// todo!() +// } + +// #[test] +// #[cfg(target_os = "linux")] +// fn listen_to_user_config_scenario_linux() { +// todo!() +// } + +// #[test] +// #[cfg(target_os = "macos")] +// fn listen_to_user_config_scenario_macos() { +// todo!() +// } + +/// Check if made changes have had any effect on +/// the client config. +#[test] +fn ratoml_client_config_basic() { + let server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +[package] +name = "p1" +version = "0.1.0" +edition = "2021" +"#, + r#"//- /p1/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + ], + vec!["p1"], + Some(json!({ + "assist" : { + "emitMustUse" : true + } + })), + ); + + assert!(server.query(QueryType::AssistEmitMustUse, 1)); +} + +/// Checks if client config can be modified. +/// FIXME @alibektas : This test is atm not valid. +/// Asking for client config from the client is a 2 way communication +/// which we cannot imitate with the current slow-tests infrastructure. +/// See rust-analyzer::handlers::notifications#197 +// #[test] +// fn client_config_update() { +// setup(); + +// let server = RatomlTest::new( +// vec![ +// r#" +// //- /p1/Cargo.toml +// [package] +// name = "p1" +// version = "0.1.0" +// edition = "2021" +// "#, +// r#" +// //- /p1/src/lib.rs +// enum Value { +// Number(i32), +// Text(String), +// }"#, +// ], +// vec!["p1"], +// None, +// ); + +// assert!(!server.query(QueryType::AssistEmitMustUse, 1)); + +// // a.notification::(DidChangeConfigurationParams { +// // settings: json!({ +// // "assists" : { +// // "emitMustUse" : true +// // } +// // }), +// // }); + +// assert!(server.query(QueryType::AssistEmitMustUse, 1)); +// } + +// #[test] +// fn ratoml_create_ratoml_basic() { +// let server = RatomlTest::new( +// vec![ +// r#" +// //- /p1/Cargo.toml +// [package] +// name = "p1" +// version = "0.1.0" +// edition = "2021" +// "#, +// r#" +// //- /p1/rust-analyzer.toml +// assist.emitMustUse = true +// "#, +// r#" +// //- /p1/src/lib.rs +// enum Value { +// Number(i32), +// Text(String), +// } +// "#, +// ], +// vec!["p1"], +// None, +// ); + +// assert!(server.query(QueryType::AssistEmitMustUse, 2)); +// } + +#[test] +fn ratoml_user_config_detected() { + let server = RatomlTest::new( + vec![ + r#" +//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml +assist.emitMustUse = true +"#, + r#" +//- /p1/Cargo.toml +[package] +name = "p1" +version = "0.1.0" +edition = "2021" +"#, + r#"//- /p1/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + ], + vec!["p1"], + None, + ); + + assert!(server.query(QueryType::AssistEmitMustUse, 2)); +} + +#[test] +fn ratoml_create_user_config() { + let mut server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +[package] +name = "p1" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + ], + vec!["p1"], + None, + ); + + assert!(!server.query(QueryType::AssistEmitMustUse, 1)); + server.create( + "//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml", + RatomlTest::EMIT_MUST_USE.to_owned(), + ); + assert!(server.query(QueryType::AssistEmitMustUse, 1)); +} + +#[test] +fn ratoml_modify_user_config() { + let mut server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +[package] +name = "p1" +version = "0.1.0" +edition = "2021""#, + r#" +//- /p1/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + r#" +//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml +assist.emitMustUse = true"#, + ], + vec!["p1"], + None, + ); + + assert!(server.query(QueryType::AssistEmitMustUse, 1)); + server.edit(2, String::new()); + assert!(!server.query(QueryType::AssistEmitMustUse, 1)); +} + +#[test] +fn ratoml_delete_user_config() { + let mut server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +[package] +name = "p1" +version = "0.1.0" +edition = "2021""#, + r#" +//- /p1/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + r#" +//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml +assist.emitMustUse = true"#, + ], + vec!["p1"], + None, + ); + + assert!(server.query(QueryType::AssistEmitMustUse, 1)); + server.delete(2); + assert!(!server.query(QueryType::AssistEmitMustUse, 1)); +} +// #[test] +// fn delete_user_config() { +// todo!() +// } + +// #[test] +// fn modify_client_config() { +// todo!() +// } + +#[test] +fn ratoml_inherit_config_from_ws_root() { + let server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +workspace = { members = ["p2"] } +[package] +name = "p1" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/rust-analyzer.toml +assist.emitMustUse = true +"#, + r#" +//- /p1/p2/Cargo.toml +[package] +name = "p2" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/p2/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + r#" +//- /p1/src/lib.rs +pub fn add(left: usize, right: usize) -> usize { + left + right +} +"#, + ], + vec!["p1"], + None, + ); + + assert!(server.query(QueryType::AssistEmitMustUse, 3)); +} + +#[test] +fn ratoml_modify_ratoml_at_ws_root() { + let mut server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +workspace = { members = ["p2"] } +[package] +name = "p1" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/rust-analyzer.toml +assist.emitMustUse = false +"#, + r#" +//- /p1/p2/Cargo.toml +[package] +name = "p2" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/p2/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + r#" +//- /p1/src/lib.rs +pub fn add(left: usize, right: usize) -> usize { + left + right +} +"#, + ], + vec!["p1"], + None, + ); + + assert!(!server.query(QueryType::AssistEmitMustUse, 3)); + server.edit(1, "assist.emitMustUse = true".to_owned()); + assert!(server.query(QueryType::AssistEmitMustUse, 3)); +} + +#[test] +fn ratoml_delete_ratoml_at_ws_root() { + let mut server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +workspace = { members = ["p2"] } +[package] +name = "p1" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/rust-analyzer.toml +assist.emitMustUse = true +"#, + r#" +//- /p1/p2/Cargo.toml +[package] +name = "p2" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/p2/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + r#" +//- /p1/src/lib.rs +pub fn add(left: usize, right: usize) -> usize { + left + right +} +"#, + ], + vec!["p1"], + None, + ); + + assert!(server.query(QueryType::AssistEmitMustUse, 3)); + server.delete(1); + assert!(!server.query(QueryType::AssistEmitMustUse, 3)); +} + +#[test] +fn ratoml_add_immediate_child_to_ws_root() { + let mut server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +workspace = { members = ["p2"] } +[package] +name = "p1" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/rust-analyzer.toml +assist.emitMustUse = true +"#, + r#" +//- /p1/p2/Cargo.toml +[package] +name = "p2" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/p2/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + r#" +//- /p1/src/lib.rs +pub fn add(left: usize, right: usize) -> usize { + left + right +} +"#, + ], + vec!["p1"], + None, + ); + + assert!(server.query(QueryType::AssistEmitMustUse, 3)); + server.create("//- /p1/p2/rust-analyzer.toml", RatomlTest::EMIT_MUST_NOT_USE.to_owned()); + assert!(!server.query(QueryType::AssistEmitMustUse, 3)); +} + +#[test] +fn ratoml_rm_ws_root_ratoml_child_has_client_as_parent_now() { + let mut server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +workspace = { members = ["p2"] } +[package] +name = "p1" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/rust-analyzer.toml +assist.emitMustUse = true +"#, + r#" +//- /p1/p2/Cargo.toml +[package] +name = "p2" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/p2/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + r#" +//- /p1/src/lib.rs +pub fn add(left: usize, right: usize) -> usize { + left + right +} +"#, + ], + vec!["p1"], + None, + ); + + assert!(server.query(QueryType::AssistEmitMustUse, 3)); + server.delete(1); + assert!(!server.query(QueryType::AssistEmitMustUse, 3)); +} + +#[test] +fn ratoml_crates_both_roots() { + let server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +workspace = { members = ["p2"] } +[package] +name = "p1" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/rust-analyzer.toml +assist.emitMustUse = true +"#, + r#" +//- /p1/p2/Cargo.toml +[package] +name = "p2" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/p2/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + r#" +//- /p1/src/lib.rs +enum Value { + Number(i32), + Text(String), +}"#, + ], + vec!["p1", "p2"], + None, + ); + + assert!(server.query(QueryType::AssistEmitMustUse, 3)); + assert!(server.query(QueryType::AssistEmitMustUse, 4)); +} + +#[test] +fn ratoml_multiple_ratoml_in_single_source_root() { + let server = RatomlTest::new( + vec![ + r#" + //- /p1/Cargo.toml + [package] + name = "p1" + version = "0.1.0" + edition = "2021" + "#, + r#" + //- /p1/rust-analyzer.toml + assist.emitMustUse = true + "#, + r#" + //- /p1/src/rust-analyzer.toml + assist.emitMustUse = false + "#, + r#" + //- /p1/src/lib.rs + enum Value { + Number(i32), + Text(String), + } + "#, + ], + vec!["p1"], + None, + ); + + assert!(server.query(QueryType::AssistEmitMustUse, 3)); + + let server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +[package] +name = "p1" +version = "0.1.0" +edition = "2021" +"#, + r#" +//- /p1/src/rust-analyzer.toml +assist.emitMustUse = false +"#, + r#" +//- /p1/rust-analyzer.toml +assist.emitMustUse = true +"#, + r#" +//- /p1/src/lib.rs +enum Value { + Number(i32), + Text(String), +} +"#, + ], + vec!["p1"], + None, + ); + + assert!(server.query(QueryType::AssistEmitMustUse, 3)); +} + +/// If a root is non-local, so we cannot find what its parent is +/// in our `config.local_root_parent_map`. So if any config should +/// apply, it must be looked for starting from the client level. +/// FIXME @alibektas : "locality" is according to ra that, which is simply in the file system. +/// This doesn't really help us with what we want to achieve here. +// #[test] +// fn ratoml_non_local_crates_start_inheriting_from_client() { +// let server = RatomlTest::new( +// vec![ +// r#" +// //- /p1/Cargo.toml +// [package] +// name = "p1" +// version = "0.1.0" +// edition = "2021" + +// [dependencies] +// p2 = { path = "../p2" } +// #, +// r#" +// //- /p1/src/lib.rs +// enum Value { +// Number(i32), +// Text(String), +// } + +// use p2; + +// pub fn add(left: usize, right: usize) -> usize { +// p2::add(left, right) +// } + +// #[cfg(test)] +// mod tests { +// use super::*; + +// #[test] +// fn it_works() { +// let result = add(2, 2); +// assert_eq!(result, 4); +// } +// }"#, +// r#" +// //- /p2/Cargo.toml +// [package] +// name = "p2" +// version = "0.1.0" +// edition = "2021" + +// # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +// [dependencies] +// "#, +// r#" +// //- /p2/rust-analyzer.toml +// # DEF +// assist.emitMustUse = true +// "#, +// r#" +// //- /p2/src/lib.rs +// enum Value { +// Number(i32), +// Text(String), +// }"#, +// ], +// vec!["p1", "p2"], +// None, +// ); + +// assert!(!server.query(QueryType::AssistEmitMustUse, 5)); +// } + +/// Having a ratoml file at the root of a project enables +/// configuring global level configurations as well. +#[test] +fn ratoml_in_root_is_global() { + let server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +[package] +name = "p1" +version = "0.1.0" +edition = "2021" + "#, + r#" +//- /rust-analyzer.toml +hover.show.traitAssocItems = 4 + "#, + r#" +//- /p1/src/lib.rs +trait RandomTrait { + type B; + fn abc() -> i32; + fn def() -> i64; +} + +fn main() { + let a = RandomTrait; +}"#, + ], + vec![], + None, + ); + + server.query(QueryType::GlobalHover, 2); +} + +#[test] +fn ratoml_root_is_updateable() { + let mut server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +[package] +name = "p1" +version = "0.1.0" +edition = "2021" + "#, + r#" +//- /rust-analyzer.toml +hover.show.traitAssocItems = 4 + "#, + r#" +//- /p1/src/lib.rs +trait RandomTrait { + type B; + fn abc() -> i32; + fn def() -> i64; +} + +fn main() { + let a = RandomTrait; +}"#, + ], + vec![], + None, + ); + + assert!(server.query(QueryType::GlobalHover, 2)); + server.edit(1, RatomlTest::GLOBAL_TRAIT_ASSOC_ITEMS_ZERO.to_owned()); + assert!(!server.query(QueryType::GlobalHover, 2)); +} + +#[test] +fn ratoml_root_is_deletable() { + let mut server = RatomlTest::new( + vec![ + r#" +//- /p1/Cargo.toml +[package] +name = "p1" +version = "0.1.0" +edition = "2021" + "#, + r#" +//- /rust-analyzer.toml +hover.show.traitAssocItems = 4 + "#, + r#" +//- /p1/src/lib.rs +trait RandomTrait { + type B; + fn abc() -> i32; + fn def() -> i64; +} + +fn main() { + let a = RandomTrait; +}"#, + ], + vec![], + None, + ); + + assert!(server.query(QueryType::GlobalHover, 2)); + server.delete(1); + assert!(!server.query(QueryType::GlobalHover, 2)); +} diff --git a/crates/rust-analyzer/tests/slow-tests/support.rs b/crates/rust-analyzer/tests/slow-tests/support.rs index 17485ee3ae80..d12d0be53926 100644 --- a/crates/rust-analyzer/tests/slow-tests/support.rs +++ b/crates/rust-analyzer/tests/slow-tests/support.rs @@ -207,8 +207,8 @@ impl Project<'_> { change.change_client_config(self.config); let mut error_sink = ConfigError::default(); - assert!(error_sink.is_empty()); - config = config.apply_change(change, &mut error_sink); + assert!(error_sink.is_empty(), "{error_sink:?}"); + (config, _) = config.apply_change(change, &mut error_sink); config.rediscover_workspaces(); Server::new(tmp_dir.keep(), config) From 75409f79fd95a525c68db62f7545abfea7282ab9 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Mon, 29 Apr 2024 22:34:57 +0200 Subject: [PATCH 3/8] Apply requested changes round 3 --- crates/rust-analyzer/src/bin/main.rs | 6 ++-- crates/rust-analyzer/src/cli/scip.rs | 8 +++-- crates/rust-analyzer/src/config.rs | 29 +++++++++---------- crates/rust-analyzer/src/global_state.rs | 4 +-- .../src/handlers/notification.rs | 7 +++-- crates/rust-analyzer/src/reload.rs | 7 +++-- .../rust-analyzer/tests/slow-tests/support.rs | 6 ++-- 7 files changed, 36 insertions(+), 31 deletions(-) diff --git a/crates/rust-analyzer/src/bin/main.rs b/crates/rust-analyzer/src/bin/main.rs index 316384f96331..545ed6d18653 100644 --- a/crates/rust-analyzer/src/bin/main.rs +++ b/crates/rust-analyzer/src/bin/main.rs @@ -228,8 +228,10 @@ fn run_server() -> anyhow::Result<()> { if let Some(json) = initialization_options { let mut change = ConfigChange::default(); change.change_client_config(json); - let mut error_sink = ConfigError::default(); - (config, _) = config.apply_change(change, &mut error_sink); + + let error_sink: ConfigError; + (config, error_sink, _) = config.apply_change(change); + if !error_sink.is_empty() { use lsp_types::{ notification::{Notification, ShowMessage}, diff --git a/crates/rust-analyzer/src/cli/scip.rs b/crates/rust-analyzer/src/cli/scip.rs index d73e4028e518..8f60b17b5943 100644 --- a/crates/rust-analyzer/src/cli/scip.rs +++ b/crates/rust-analyzer/src/cli/scip.rs @@ -14,7 +14,7 @@ use tracing::error; use crate::{ cli::flags, - config::{ConfigChange, ConfigError}, + config::ConfigChange, line_index::{LineEndings, LineIndex, PositionEncoding}, }; @@ -45,8 +45,10 @@ impl flags::Scip { let json = serde_json::from_reader(&mut file)?; let mut change = ConfigChange::default(); change.change_client_config(json); - let mut error_sink = ConfigError::default(); - (config, _) = config.apply_change(change, &mut error_sink); + + let error_sink; + (config, error_sink, _) = config.apply_change(change); + // FIXME @alibektas : What happens to errors without logging? error!(?error_sink, "Config Error(s)"); } diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 646bae4467a0..8a3a9c7efb01 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -705,7 +705,7 @@ impl Config { // FIXME @alibektas : Server's health uses error sink but in other places it is not used atm. /// Changes made to client and global configurations will partially not be reflected even after `.apply_change()` was called. /// The return tuple's bool component signals whether the `GlobalState` should call its `update_configuration()` method. - pub fn apply_change( + fn apply_change_with_sink( &self, change: ConfigChange, error_sink: &mut ConfigError, @@ -809,10 +809,13 @@ impl Config { (config, should_update) } - pub fn apply_change_whatever(&self, change: ConfigChange) -> (Config, ConfigError) { + /// Given `change` this generates a new `Config`, thereby collecting errors of type `ConfigError`. + /// If there are changes that have global/client level effect, the last component of the return type + /// will be set to `true`, which should be used by the `GlobalState` to update itself. + pub fn apply_change(&self, change: ConfigChange) -> (Config, ConfigError, bool) { let mut e = ConfigError(vec![]); - let (config, _) = self.apply_change(change, &mut e); - (config, e) + let (config, should_update) = self.apply_change_with_sink(change, &mut e); + (config, e, should_update) } } @@ -3300,8 +3303,7 @@ mod tests { "server": null, }})); - let mut error_sink = ConfigError::default(); - (config, _) = config.apply_change(change, &mut error_sink); + (config, _, _) = config.apply_change(change); assert_eq!(config.proc_macro_srv(), None); } @@ -3320,8 +3322,7 @@ mod tests { "server": project_root().display().to_string(), }})); - let mut error_sink = ConfigError::default(); - (config, _) = config.apply_change(change, &mut error_sink); + (config, _, _) = config.apply_change(change); assert_eq!(config.proc_macro_srv(), Some(AbsPathBuf::try_from(project_root()).unwrap())); } @@ -3342,8 +3343,7 @@ mod tests { "server": "./server" }})); - let mut error_sink = ConfigError::default(); - (config, _) = config.apply_change(change, &mut error_sink); + (config, _, _) = config.apply_change(change); assert_eq!( config.proc_macro_srv(), @@ -3367,8 +3367,7 @@ mod tests { "rust" : { "analyzerTargetDir" : null } })); - let mut error_sink = ConfigError::default(); - (config, _) = config.apply_change(change, &mut error_sink); + (config, _, _) = config.apply_change(change); assert_eq!(config.cargo_targetDir(), &None); assert!( matches!(config.flycheck(), FlycheckConfig::CargoCommand { options, .. } if options.target_dir.is_none()) @@ -3390,8 +3389,7 @@ mod tests { "rust" : { "analyzerTargetDir" : true } })); - let mut error_sink = ConfigError::default(); - (config, _) = config.apply_change(change, &mut error_sink); + (config, _, _) = config.apply_change(change); assert_eq!(config.cargo_targetDir(), &Some(TargetDirectory::UseSubdirectory(true))); assert!( @@ -3414,8 +3412,7 @@ mod tests { "rust" : { "analyzerTargetDir" : "other_folder" } })); - let mut error_sink = ConfigError::default(); - (config, _) = config.apply_change(change, &mut error_sink); + (config, _, _) = config.apply_change(change); assert_eq!( config.cargo_targetDir(), diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index c289a07978d2..5e7e8061efdc 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -404,8 +404,8 @@ impl GlobalState { change }; - let mut error_sink = ConfigError::default(); - let (config, should_update) = self.config.apply_change(config_change, &mut error_sink); + + let (config, _, should_update) = self.config.apply_change(config_change); if should_update { self.update_configuration(config); diff --git a/crates/rust-analyzer/src/handlers/notification.rs b/crates/rust-analyzer/src/handlers/notification.rs index bdb27043ebe7..50489044309b 100644 --- a/crates/rust-analyzer/src/handlers/notification.rs +++ b/crates/rust-analyzer/src/handlers/notification.rs @@ -13,7 +13,7 @@ use triomphe::Arc; use vfs::{AbsPathBuf, ChangeKind, VfsPath}; use crate::{ - config::{Config, ConfigChange, ConfigError}, + config::{Config, ConfigChange}, global_state::GlobalState, lsp::{from_proto, utils::apply_document_changes}, lsp_ext::{self, RunFlycheckParams}, @@ -200,8 +200,9 @@ pub(crate) fn handle_did_change_configuration( let mut config = Config::clone(&*this.config); let mut change = ConfigChange::default(); change.change_client_config(json.take()); - let mut error_sink = ConfigError::default(); - (config, _) = config.apply_change(change, &mut error_sink); + + (config, _, _) = config.apply_change(change); + // Client config changes neccesitates .update_config method to be called. this.update_configuration(config); } diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index 364a0083e591..6061ccbfe868 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -33,7 +33,7 @@ use triomphe::Arc; use vfs::{AbsPath, AbsPathBuf, ChangeKind}; use crate::{ - config::{Config, ConfigChange, ConfigError, FilesWatcher, LinkedProject}, + config::{Config, ConfigChange, FilesWatcher, LinkedProject}, global_state::GlobalState, lsp_ext, main_loop::Task, @@ -604,8 +604,9 @@ impl GlobalState { let mut config_change = ConfigChange::default(); config_change.change_source_root_parent_map(self.local_roots_parent_map.clone()); - let mut error_sink = ConfigError::default(); - let (config, _) = self.config.apply_change(config_change, &mut error_sink); + + let (config, _, _) = self.config.apply_change(config_change); + self.config = Arc::new(config); self.recreate_crate_graph(cause); diff --git a/crates/rust-analyzer/tests/slow-tests/support.rs b/crates/rust-analyzer/tests/slow-tests/support.rs index d12d0be53926..2d9b3d560b7a 100644 --- a/crates/rust-analyzer/tests/slow-tests/support.rs +++ b/crates/rust-analyzer/tests/slow-tests/support.rs @@ -206,9 +206,11 @@ impl Project<'_> { let mut change = ConfigChange::default(); change.change_client_config(self.config); - let mut error_sink = ConfigError::default(); + + let error_sink: ConfigError; + (config, error_sink, _) = config.apply_change(change); assert!(error_sink.is_empty(), "{error_sink:?}"); - (config, _) = config.apply_change(change, &mut error_sink); + config.rediscover_workspaces(); Server::new(tmp_dir.keep(), config) From fb8a2c334fd60644cc832d154dd81d3cd92aee59 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 3 May 2024 09:43:22 +0200 Subject: [PATCH 4/8] Shuffle around some of the configs between the levels --- crates/rust-analyzer/src/config.rs | 557 ++++++++---------- crates/rust-analyzer/src/handlers/request.rs | 24 +- .../rust-analyzer/tests/slow-tests/ratoml.rs | 12 +- 3 files changed, 278 insertions(+), 315 deletions(-) diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 8a3a9c7efb01..199e1de25c16 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -62,6 +62,7 @@ mod patch_old_style; // parsing the old name. config_data! { /// Configs that apply on a workspace-wide scope. There are 3 levels on which a global configuration can be configured + // FIXME: 1. and 3. should be split, some configs do not make sense per project /// /// 1. `rust-analyzer.toml` file under user's config directory (e.g ~/.config/rust-analyzer.toml) /// 2. Client's own configurations (e.g `settings.json` on VS Code) @@ -246,71 +247,6 @@ config_data! { /// If false, `-p ` will be passed instead. check_workspace: bool = true, - - /// Toggles the additional completions that automatically add imports when completed. - /// Note that your client must specify the `additionalTextEdits` LSP client capability to truly have this feature enabled. - completion_autoimport_enable: bool = true, - /// Toggles the additional completions that automatically show method calls and field accesses - /// with `self` prefixed to them when inside a method. - completion_autoself_enable: bool = true, - /// Whether to add parenthesis and argument snippets when completing function. - completion_callable_snippets: CallableCompletionDef = CallableCompletionDef::FillArguments, - /// Whether to show full function/method signatures in completion docs. - completion_fullFunctionSignatures_enable: bool = false, - /// Maximum number of completions to return. If `None`, the limit is infinite. - completion_limit: Option = None, - /// Whether to show postfix snippets like `dbg`, `if`, `not`, etc. - completion_postfix_enable: bool = true, - /// Enables completions of private items and fields that are defined in the current workspace even if they are not visible at the current position. - completion_privateEditable_enable: bool = false, - /// Custom completion snippets. - // NOTE: we use IndexMap for deterministic serialization ordering - completion_snippets_custom: IndexMap = serde_json::from_str(r#"{ - "Arc::new": { - "postfix": "arc", - "body": "Arc::new(${receiver})", - "requires": "std::sync::Arc", - "description": "Put the expression into an `Arc`", - "scope": "expr" - }, - "Rc::new": { - "postfix": "rc", - "body": "Rc::new(${receiver})", - "requires": "std::rc::Rc", - "description": "Put the expression into an `Rc`", - "scope": "expr" - }, - "Box::pin": { - "postfix": "pinbox", - "body": "Box::pin(${receiver})", - "requires": "std::boxed::Box", - "description": "Put the expression into a pinned `Box`", - "scope": "expr" - }, - "Ok": { - "postfix": "ok", - "body": "Ok(${receiver})", - "description": "Wrap the expression in a `Result::Ok`", - "scope": "expr" - }, - "Err": { - "postfix": "err", - "body": "Err(${receiver})", - "description": "Wrap the expression in a `Result::Err`", - "scope": "expr" - }, - "Some": { - "postfix": "some", - "body": "Some(${receiver})", - "description": "Wrap the expression in an `Option::Some`", - "scope": "expr" - } - }"#).unwrap(), - /// Whether to enable term search based snippets like `Some(foo.bar().baz())`. - completion_termSearch_enable: bool = false, - /// Term search fuel in "units of work" for autocompletion (Defaults to 200). - completion_termSearch_fuel: usize = 200, - /// List of rust-analyzer diagnostics to disable. diagnostics_disabled: FxHashSet = FxHashSet::default(), /// Whether to show native rust-analyzer diagnostics. @@ -333,87 +269,12 @@ config_data! { /// The warnings will be indicated by a blue squiggly underline in code /// and a blue icon in the `Problems Panel`. diagnostics_warningsAsInfo: Vec = vec![], + /// These directories will be ignored by rust-analyzer. They are /// relative to the workspace root, and globs are not supported. You may /// also need to add the folders to Code's `files.watcherExclude`. files_excludeDirs: Vec = vec![], - /// Controls file watching implementation. - files_watcher: FilesWatcherDef = FilesWatcherDef::Client, - - /// Whether to show `Debug` action. Only applies when - /// `#rust-analyzer.hover.actions.enable#` is set. - hover_actions_debug_enable: bool = true, - /// Whether to show HoverActions in Rust files. - hover_actions_enable: bool = true, - /// Whether to show `Go to Type Definition` action. Only applies when - /// `#rust-analyzer.hover.actions.enable#` is set. - hover_actions_gotoTypeDef_enable: bool = true, - /// Whether to show `Implementations` action. Only applies when - /// `#rust-analyzer.hover.actions.enable#` is set. - hover_actions_implementations_enable: bool = true, - /// Whether to show `References` action. Only applies when - /// `#rust-analyzer.hover.actions.enable#` is set. - hover_actions_references_enable: bool = false, - /// Whether to show `Run` action. Only applies when - /// `#rust-analyzer.hover.actions.enable#` is set. - hover_actions_run_enable: bool = true, - /// Whether to show documentation on hover. - hover_documentation_enable: bool = true, - /// Whether to show keyword hover popups. Only applies when - /// `#rust-analyzer.hover.documentation.enable#` is set. - hover_documentation_keywords_enable: bool = true, - /// Use markdown syntax for links on hover. - hover_links_enable: bool = true, - /// How to render the align information in a memory layout hover. - hover_memoryLayout_alignment: Option = Some(MemoryLayoutHoverRenderKindDef::Hexadecimal), - /// Whether to show memory layout data on hover. - hover_memoryLayout_enable: bool = true, - /// How to render the niche information in a memory layout hover. - hover_memoryLayout_niches: Option = Some(false), - /// How to render the offset information in a memory layout hover. - hover_memoryLayout_offset: Option = Some(MemoryLayoutHoverRenderKindDef::Hexadecimal), - /// How to render the size information in a memory layout hover. - hover_memoryLayout_size: Option = Some(MemoryLayoutHoverRenderKindDef::Both), - - /// How many variants of an enum to display when hovering on. Show none if empty. - hover_show_enumVariants: Option = Some(5), - /// How many fields of a struct, variant or union to display when hovering on. Show none if empty. - hover_show_fields: Option = Some(5), - /// How many associated items of a trait to display when hovering a trait. - hover_show_traitAssocItems: Option = None, - - /// Enables the experimental support for interpreting tests. - interpret_tests: bool = false, - - /// Whether to show `Debug` lens. Only applies when - /// `#rust-analyzer.lens.enable#` is set. - lens_debug_enable: bool = true, - /// Whether to show CodeLens in Rust files. - lens_enable: bool = true, - /// Internal config: use custom client-side commands even when the - /// client doesn't set the corresponding capability. - lens_forceCustomCommands: bool = true, - /// Whether to show `Implementations` lens. Only applies when - /// `#rust-analyzer.lens.enable#` is set. - lens_implementations_enable: bool = true, - /// Where to render annotations. - lens_location: AnnotationLocation = AnnotationLocation::AboveName, - /// Whether to show `References` lens for Struct, Enum, and Union. - /// Only applies when `#rust-analyzer.lens.enable#` is set. - lens_references_adt_enable: bool = false, - /// Whether to show `References` lens for Enum Variants. - /// Only applies when `#rust-analyzer.lens.enable#` is set. - lens_references_enumVariant_enable: bool = false, - /// Whether to show `Method References` lens. Only applies when - /// `#rust-analyzer.lens.enable#` is set. - lens_references_method_enable: bool = false, - /// Whether to show `References` lens for Trait. - /// Only applies when `#rust-analyzer.lens.enable#` is set. - lens_references_trait_enable: bool = false, - /// Whether to show `Run` lens. Only applies when - /// `#rust-analyzer.lens.enable#` is set. - lens_run_enable: bool = true, /// Disable project auto-discovery in favor of explicitly specified set /// of projects. @@ -428,31 +289,10 @@ config_data! { /// Sets the LRU capacity of the specified queries. lru_query_capacities: FxHashMap, usize> = FxHashMap::default(), - /// Whether to show `can't find Cargo.toml` error message. - notifications_cargoTomlNotFound: bool = true, - - /// Whether to send an UnindexedProject notification to the client. - notifications_unindexedProject: bool = false, - - /// How many worker threads in the main loop. The default `null` means to pick automatically. - numThreads: Option = None, - - /// Expand attribute macros. Requires `#rust-analyzer.procMacro.enable#` to be set. - procMacro_attributes_enable: bool = true, - /// Enable support for procedural macros, implies `#rust-analyzer.cargo.buildScripts.enable#`. - procMacro_enable: bool = true, /// These proc-macros will be ignored when trying to expand them. /// /// This config takes a map of crate names with the exported proc-macro names to ignore as values. procMacro_ignored: FxHashMap, Box<[Box]>> = FxHashMap::default(), - /// Internal config, path to proc-macro server executable. - procMacro_server: Option = None, - - /// Exclude imports from find-all-references. - references_excludeImports: bool = false, - - /// Exclude tests from find-all-references. - references_excludeTests: bool = false, /// Command to be executed instead of 'cargo' for runnables. runnables_command: Option = None, @@ -490,24 +330,6 @@ config_data! { /// `textDocument/rangeFormatting` request. The rustfmt option is unstable and only /// available on a nightly build. rustfmt_rangeFormatting_enable: bool = false, - - - /// Show full signature of the callable. Only shows parameters if disabled. - signatureInfo_detail: SignatureDetail = SignatureDetail::Full, - /// Show documentation. - signatureInfo_documentation_enable: bool = true, - - /// Whether to insert closing angle brackets when typing an opening angle bracket of a generic argument list. - typing_autoClosingAngleBrackets_enable: bool = false, - - /// Workspace symbol search kind. - workspace_symbol_search_kind: WorkspaceSymbolSearchKindDef = WorkspaceSymbolSearchKindDef::OnlyTypes, - /// Limits the number of items returned from a workspace symbol search (Defaults to 128). - /// Some clients like vs-code issue new searches on result filtering and don't require all results to be returned in the initial search. - /// Other clients requires all results upfront and might require a higher limit. - workspace_symbol_search_limit: usize = 128, - /// Workspace symbol search scope. - workspace_symbol_search_scope: WorkspaceSymbolSearchScopeDef = WorkspaceSymbolSearchScopeDef::Workspace, } } @@ -522,17 +344,6 @@ config_data! { /// Term search fuel in "units of work" for assists (Defaults to 400). assist_termSearch_fuel: usize = 400, - /// Enables highlighting of related references while the cursor is on `break`, `loop`, `while`, or `for` keywords. - highlightRelated_breakPoints_enable: bool = true, - /// Enables highlighting of all captures of a closure while the cursor is on the `|` or move keyword of a closure. - highlightRelated_closureCaptures_enable: bool = true, - /// Enables highlighting of all exit points while the cursor is on any `return`, `?`, `fn`, or return type arrow (`->`). - highlightRelated_exitPoints_enable: bool = true, - /// Enables highlighting of related references while the cursor is on any identifier. - highlightRelated_references_enable: bool = true, - /// Enables highlighting of all break points for a loop or block context while the cursor is on any `async` or `await` keywords. - highlightRelated_yieldPoints_enable: bool = true, - /// Whether to enforce the import granularity setting for all files. If set to false rust-analyzer will try to keep import styles consistent per file. imports_granularity_enforce: bool = false, /// How imports should be grouped into use statements. @@ -547,7 +358,133 @@ config_data! { imports_preferPrelude: bool = false, /// The path structure for newly inserted paths to use. imports_prefix: ImportPrefixDef = ImportPrefixDef::Plain, + } +} + +config_data! { + /// Configs that only make sense when they are set by a client. As such they can only be defined + /// by setting them using client's settings (e.g `settings.json` on VS Code). + client: struct ClientDefaultConfigData <- ClientConfigInput -> { + /// Toggles the additional completions that automatically add imports when completed. + /// Note that your client must specify the `additionalTextEdits` LSP client capability to truly have this feature enabled. + completion_autoimport_enable: bool = true, + /// Toggles the additional completions that automatically show method calls and field accesses + /// with `self` prefixed to them when inside a method. + completion_autoself_enable: bool = true, + /// Whether to add parenthesis and argument snippets when completing function. + completion_callable_snippets: CallableCompletionDef = CallableCompletionDef::FillArguments, + /// Whether to show full function/method signatures in completion docs. + completion_fullFunctionSignatures_enable: bool = false, + /// Maximum number of completions to return. If `None`, the limit is infinite. + completion_limit: Option = None, + /// Whether to show postfix snippets like `dbg`, `if`, `not`, etc. + completion_postfix_enable: bool = true, + /// Enables completions of private items and fields that are defined in the current workspace even if they are not visible at the current position. + completion_privateEditable_enable: bool = false, + /// Custom completion snippets. + // NOTE: we use IndexMap for deterministic serialization ordering + completion_snippets_custom: IndexMap = serde_json::from_str(r#"{ + "Arc::new": { + "postfix": "arc", + "body": "Arc::new(${receiver})", + "requires": "std::sync::Arc", + "description": "Put the expression into an `Arc`", + "scope": "expr" + }, + "Rc::new": { + "postfix": "rc", + "body": "Rc::new(${receiver})", + "requires": "std::rc::Rc", + "description": "Put the expression into an `Rc`", + "scope": "expr" + }, + "Box::pin": { + "postfix": "pinbox", + "body": "Box::pin(${receiver})", + "requires": "std::boxed::Box", + "description": "Put the expression into a pinned `Box`", + "scope": "expr" + }, + "Ok": { + "postfix": "ok", + "body": "Ok(${receiver})", + "description": "Wrap the expression in a `Result::Ok`", + "scope": "expr" + }, + "Err": { + "postfix": "err", + "body": "Err(${receiver})", + "description": "Wrap the expression in a `Result::Err`", + "scope": "expr" + }, + "Some": { + "postfix": "some", + "body": "Some(${receiver})", + "description": "Wrap the expression in an `Option::Some`", + "scope": "expr" + } + }"#).unwrap(), + /// Whether to enable term search based snippets like `Some(foo.bar().baz())`. + completion_termSearch_enable: bool = false, + /// Term search fuel in "units of work" for autocompletion (Defaults to 200). + completion_termSearch_fuel: usize = 200, + /// Controls file watching implementation. + files_watcher: FilesWatcherDef = FilesWatcherDef::Client, + + /// Enables highlighting of related references while the cursor is on `break`, `loop`, `while`, or `for` keywords. + highlightRelated_breakPoints_enable: bool = true, + /// Enables highlighting of all captures of a closure while the cursor is on the `|` or move keyword of a closure. + highlightRelated_closureCaptures_enable: bool = true, + /// Enables highlighting of all exit points while the cursor is on any `return`, `?`, `fn`, or return type arrow (`->`). + highlightRelated_exitPoints_enable: bool = true, + /// Enables highlighting of related references while the cursor is on any identifier. + highlightRelated_references_enable: bool = true, + /// Enables highlighting of all break points for a loop or block context while the cursor is on any `async` or `await` keywords. + highlightRelated_yieldPoints_enable: bool = true, + + /// Whether to show `Debug` action. Only applies when + /// `#rust-analyzer.hover.actions.enable#` is set. + hover_actions_debug_enable: bool = true, + /// Whether to show HoverActions in Rust files. + hover_actions_enable: bool = true, + /// Whether to show `Go to Type Definition` action. Only applies when + /// `#rust-analyzer.hover.actions.enable#` is set. + hover_actions_gotoTypeDef_enable: bool = true, + /// Whether to show `Implementations` action. Only applies when + /// `#rust-analyzer.hover.actions.enable#` is set. + hover_actions_implementations_enable: bool = true, + /// Whether to show `References` action. Only applies when + /// `#rust-analyzer.hover.actions.enable#` is set. + hover_actions_references_enable: bool = false, + /// Whether to show `Run` action. Only applies when + /// `#rust-analyzer.hover.actions.enable#` is set. + hover_actions_run_enable: bool = true, + + /// Whether to show documentation on hover. + hover_documentation_enable: bool = true, + /// Whether to show keyword hover popups. Only applies when + /// `#rust-analyzer.hover.documentation.enable#` is set. + hover_documentation_keywords_enable: bool = true, + /// Use markdown syntax for links on hover. + hover_links_enable: bool = true, + /// How to render the align information in a memory layout hover. + hover_memoryLayout_alignment: Option = Some(MemoryLayoutHoverRenderKindDef::Hexadecimal), + /// Whether to show memory layout data on hover. + hover_memoryLayout_enable: bool = true, + /// How to render the niche information in a memory layout hover. + hover_memoryLayout_niches: Option = Some(false), + /// How to render the offset information in a memory layout hover. + hover_memoryLayout_offset: Option = Some(MemoryLayoutHoverRenderKindDef::Hexadecimal), + /// How to render the size information in a memory layout hover. + hover_memoryLayout_size: Option = Some(MemoryLayoutHoverRenderKindDef::Both), + + /// How many variants of an enum to display when hovering on. Show none if empty. + hover_show_enumVariants: Option = Some(5), + /// How many fields of a struct, variant or union to display when hovering on. Show none if empty. + hover_show_fields: Option = Some(5), + /// How many associated items of a trait to display when hovering a trait. + hover_show_traitAssocItems: Option = None, /// Whether to show inlay type hints for binding modes. inlayHints_bindingModeHints_enable: bool = false, @@ -598,6 +535,8 @@ config_data! { /// Whether to hide inlay type hints for constructors. inlayHints_typeHints_hideNamedConstructor: bool = false, + /// Enables the experimental support for interpreting tests. + interpret_tests: bool = false, /// Join lines merges consecutive declaration and initialization of an assignment. joinLines_joinAssignments: bool = true, @@ -608,6 +547,57 @@ config_data! { /// Join lines unwraps trivial blocks. joinLines_unwrapTrivialBlock: bool = true, + /// Whether to show `Debug` lens. Only applies when + /// `#rust-analyzer.lens.enable#` is set. + lens_debug_enable: bool = true, + /// Whether to show CodeLens in Rust files. + lens_enable: bool = true, + /// Internal config: use custom client-side commands even when the + /// client doesn't set the corresponding capability. + lens_forceCustomCommands: bool = true, + /// Whether to show `Implementations` lens. Only applies when + /// `#rust-analyzer.lens.enable#` is set. + lens_implementations_enable: bool = true, + /// Where to render annotations. + lens_location: AnnotationLocation = AnnotationLocation::AboveName, + /// Whether to show `References` lens for Struct, Enum, and Union. + /// Only applies when `#rust-analyzer.lens.enable#` is set. + lens_references_adt_enable: bool = false, + /// Whether to show `References` lens for Enum Variants. + /// Only applies when `#rust-analyzer.lens.enable#` is set. + lens_references_enumVariant_enable: bool = false, + /// Whether to show `Method References` lens. Only applies when + /// `#rust-analyzer.lens.enable#` is set. + lens_references_method_enable: bool = false, + /// Whether to show `References` lens for Trait. + /// Only applies when `#rust-analyzer.lens.enable#` is set. + lens_references_trait_enable: bool = false, + /// Whether to show `Run` lens. Only applies when + /// `#rust-analyzer.lens.enable#` is set. + lens_run_enable: bool = true, + + /// Whether to show `can't find Cargo.toml` error message. + notifications_cargoTomlNotFound: bool = true, + + /// Whether to send an UnindexedProject notification to the client. + notifications_unindexedProject: bool = false, + + /// How many worker threads in the main loop. The default `null` means to pick automatically. + numThreads: Option = None, + + /// Expand attribute macros. Requires `#rust-analyzer.procMacro.enable#` to be set. + procMacro_attributes_enable: bool = true, + /// Enable support for procedural macros, implies `#rust-analyzer.cargo.buildScripts.enable#`. + procMacro_enable: bool = true, + /// Internal config, path to proc-macro server executable. + procMacro_server: Option = None, + + /// Exclude imports from find-all-references. + references_excludeImports: bool = false, + + /// Exclude tests from find-all-references. + references_excludeTests: bool = false, + /// Inject additional highlighting into doc comments. /// /// When enabled, rust-analyzer will highlight rust source in doc comments as well as intra @@ -644,13 +634,24 @@ config_data! { /// By disabling semantic tokens for strings, other grammars can be used to highlight /// their contents. semanticHighlighting_strings_enable: bool = true, - } -} -config_data! { - /// Configs that only make sense when they are set by a client. As such they can only be defined - /// by setting them using client's settings (e.g `settings.json` on VS Code). - client: struct ClientDefaultConfigData <- ClientConfigInput -> {} + /// Show full signature of the callable. Only shows parameters if disabled. + signatureInfo_detail: SignatureDetail = SignatureDetail::Full, + /// Show documentation. + signatureInfo_documentation_enable: bool = true, + + /// Whether to insert closing angle brackets when typing an opening angle bracket of a generic argument list. + typing_autoClosingAngleBrackets_enable: bool = false, + + /// Workspace symbol search kind. + workspace_symbol_search_kind: WorkspaceSymbolSearchKindDef = WorkspaceSymbolSearchKindDef::OnlyTypes, + /// Limits the number of items returned from a workspace symbol search (Defaults to 128). + /// Some clients like vs-code issue new searches on result filtering and don't require all results to be returned in the initial search. + /// Other clients requires all results upfront and might require a higher limit. + workspace_symbol_search_limit: usize = 128, + /// Workspace symbol search scope. + workspace_symbol_search_scope: WorkspaceSymbolSearchScopeDef = WorkspaceSymbolSearchScopeDef::Workspace, + } } #[derive(Debug, Clone)] @@ -1248,13 +1249,13 @@ impl Config { self.procMacro_enable().to_owned() && self.procMacro_attributes_enable().to_owned() } - pub fn highlight_related(&self, source_root: Option) -> HighlightRelatedConfig { + pub fn highlight_related(&self, _source_root: Option) -> HighlightRelatedConfig { HighlightRelatedConfig { - references: self.highlightRelated_references_enable(source_root).to_owned(), - break_points: self.highlightRelated_breakPoints_enable(source_root).to_owned(), - exit_points: self.highlightRelated_exitPoints_enable(source_root).to_owned(), - yield_points: self.highlightRelated_yieldPoints_enable(source_root).to_owned(), - closure_captures: self.highlightRelated_closureCaptures_enable(source_root).to_owned(), + references: self.highlightRelated_references_enable().to_owned(), + break_points: self.highlightRelated_breakPoints_enable().to_owned(), + exit_points: self.highlightRelated_exitPoints_enable().to_owned(), + yield_points: self.highlightRelated_yieldPoints_enable().to_owned(), + closure_captures: self.highlightRelated_closureCaptures_enable().to_owned(), } } @@ -1308,7 +1309,7 @@ impl Config { } } - pub fn inlay_hints(&self, source_root: Option) -> InlayHintsConfig { + pub fn inlay_hints(&self) -> InlayHintsConfig { let client_capability_fields = self .caps .text_document @@ -1322,74 +1323,65 @@ impl Config { .collect::>(); InlayHintsConfig { - render_colons: self.inlayHints_renderColons(source_root).to_owned(), - type_hints: self.inlayHints_typeHints_enable(source_root).to_owned(), - parameter_hints: self.inlayHints_parameterHints_enable(source_root).to_owned(), - chaining_hints: self.inlayHints_chainingHints_enable(source_root).to_owned(), - discriminant_hints: match self.inlayHints_discriminantHints_enable(source_root) { + render_colons: self.inlayHints_renderColons().to_owned(), + type_hints: self.inlayHints_typeHints_enable().to_owned(), + parameter_hints: self.inlayHints_parameterHints_enable().to_owned(), + chaining_hints: self.inlayHints_chainingHints_enable().to_owned(), + discriminant_hints: match self.inlayHints_discriminantHints_enable() { DiscriminantHintsDef::Always => ide::DiscriminantHints::Always, DiscriminantHintsDef::Never => ide::DiscriminantHints::Never, DiscriminantHintsDef::Fieldless => ide::DiscriminantHints::Fieldless, }, - closure_return_type_hints: match self - .inlayHints_closureReturnTypeHints_enable(source_root) - { + closure_return_type_hints: match self.inlayHints_closureReturnTypeHints_enable() { ClosureReturnTypeHintsDef::Always => ide::ClosureReturnTypeHints::Always, ClosureReturnTypeHintsDef::Never => ide::ClosureReturnTypeHints::Never, ClosureReturnTypeHintsDef::WithBlock => ide::ClosureReturnTypeHints::WithBlock, }, - lifetime_elision_hints: match self.inlayHints_lifetimeElisionHints_enable(source_root) { + lifetime_elision_hints: match self.inlayHints_lifetimeElisionHints_enable() { LifetimeElisionDef::Always => ide::LifetimeElisionHints::Always, LifetimeElisionDef::Never => ide::LifetimeElisionHints::Never, LifetimeElisionDef::SkipTrivial => ide::LifetimeElisionHints::SkipTrivial, }, hide_named_constructor_hints: self - .inlayHints_typeHints_hideNamedConstructor(source_root) + .inlayHints_typeHints_hideNamedConstructor() .to_owned(), hide_closure_initialization_hints: self - .inlayHints_typeHints_hideClosureInitialization(source_root) + .inlayHints_typeHints_hideClosureInitialization() .to_owned(), - closure_style: match self.inlayHints_closureStyle(source_root) { + closure_style: match self.inlayHints_closureStyle() { ClosureStyle::ImplFn => hir::ClosureStyle::ImplFn, ClosureStyle::RustAnalyzer => hir::ClosureStyle::RANotation, ClosureStyle::WithId => hir::ClosureStyle::ClosureWithId, ClosureStyle::Hide => hir::ClosureStyle::Hide, }, - closure_capture_hints: self - .inlayHints_closureCaptureHints_enable(source_root) - .to_owned(), - adjustment_hints: match self.inlayHints_expressionAdjustmentHints_enable(source_root) { + closure_capture_hints: self.inlayHints_closureCaptureHints_enable().to_owned(), + adjustment_hints: match self.inlayHints_expressionAdjustmentHints_enable() { AdjustmentHintsDef::Always => ide::AdjustmentHints::Always, - AdjustmentHintsDef::Never => { - match self.inlayHints_reborrowHints_enable(source_root) { - ReborrowHintsDef::Always | ReborrowHintsDef::Mutable => { - ide::AdjustmentHints::ReborrowOnly - } - ReborrowHintsDef::Never => ide::AdjustmentHints::Never, + AdjustmentHintsDef::Never => match self.inlayHints_reborrowHints_enable() { + ReborrowHintsDef::Always | ReborrowHintsDef::Mutable => { + ide::AdjustmentHints::ReborrowOnly } - } + ReborrowHintsDef::Never => ide::AdjustmentHints::Never, + }, AdjustmentHintsDef::Reborrow => ide::AdjustmentHints::ReborrowOnly, }, - adjustment_hints_mode: match self.inlayHints_expressionAdjustmentHints_mode(source_root) - { + adjustment_hints_mode: match self.inlayHints_expressionAdjustmentHints_mode() { AdjustmentHintsModeDef::Prefix => ide::AdjustmentHintsMode::Prefix, AdjustmentHintsModeDef::Postfix => ide::AdjustmentHintsMode::Postfix, AdjustmentHintsModeDef::PreferPrefix => ide::AdjustmentHintsMode::PreferPrefix, AdjustmentHintsModeDef::PreferPostfix => ide::AdjustmentHintsMode::PreferPostfix, }, adjustment_hints_hide_outside_unsafe: self - .inlayHints_expressionAdjustmentHints_hideOutsideUnsafe(source_root) + .inlayHints_expressionAdjustmentHints_hideOutsideUnsafe() .to_owned(), - binding_mode_hints: self.inlayHints_bindingModeHints_enable(source_root).to_owned(), + binding_mode_hints: self.inlayHints_bindingModeHints_enable().to_owned(), param_names_for_lifetime_elision_hints: self - .inlayHints_lifetimeElisionHints_useParameterNames(source_root) + .inlayHints_lifetimeElisionHints_useParameterNames() .to_owned(), - max_length: self.inlayHints_maxLength(source_root).to_owned(), - closing_brace_hints_min_lines: if self - .inlayHints_closingBraceHints_enable(source_root) - .to_owned() + max_length: self.inlayHints_maxLength().to_owned(), + closing_brace_hints_min_lines: if self.inlayHints_closingBraceHints_enable().to_owned() { - Some(self.inlayHints_closingBraceHints_minLines(source_root).to_owned()) + Some(self.inlayHints_closingBraceHints_minLines().to_owned()) } else { None }, @@ -1400,10 +1392,8 @@ impl Config { resolve_label_location: client_capability_fields.contains("label.location"), resolve_label_command: client_capability_fields.contains("label.command"), }, - implicit_drop_hints: self.inlayHints_implicitDrops_enable(source_root).to_owned(), - range_exclusive_hints: self - .inlayHints_rangeExclusiveHints_enable(source_root) - .to_owned(), + implicit_drop_hints: self.inlayHints_implicitDrops_enable().to_owned(), + range_exclusive_hints: self.inlayHints_rangeExclusiveHints_enable().to_owned(), } } @@ -1427,36 +1417,32 @@ impl Config { } } - pub fn join_lines(&self, source_root: Option) -> JoinLinesConfig { + pub fn join_lines(&self) -> JoinLinesConfig { JoinLinesConfig { - join_else_if: self.joinLines_joinElseIf(source_root).to_owned(), - remove_trailing_comma: self.joinLines_removeTrailingComma(source_root).to_owned(), - unwrap_trivial_blocks: self.joinLines_unwrapTrivialBlock(source_root).to_owned(), - join_assignments: self.joinLines_joinAssignments(source_root).to_owned(), + join_else_if: self.joinLines_joinElseIf().to_owned(), + remove_trailing_comma: self.joinLines_removeTrailingComma().to_owned(), + unwrap_trivial_blocks: self.joinLines_unwrapTrivialBlock().to_owned(), + join_assignments: self.joinLines_joinAssignments().to_owned(), } } - pub fn highlighting_non_standard_tokens(&self, source_root: Option) -> bool { - self.semanticHighlighting_nonStandardTokens(source_root).to_owned() + pub fn highlighting_non_standard_tokens(&self) -> bool { + self.semanticHighlighting_nonStandardTokens().to_owned() } - pub fn highlighting_config(&self, source_root: Option) -> HighlightConfig { + pub fn highlighting_config(&self) -> HighlightConfig { HighlightConfig { - strings: self.semanticHighlighting_strings_enable(source_root).to_owned(), - punctuation: self.semanticHighlighting_punctuation_enable(source_root).to_owned(), + strings: self.semanticHighlighting_strings_enable().to_owned(), + punctuation: self.semanticHighlighting_punctuation_enable().to_owned(), specialize_punctuation: self - .semanticHighlighting_punctuation_specialization_enable(source_root) - .to_owned(), - macro_bang: self - .semanticHighlighting_punctuation_separate_macro_bang(source_root) + .semanticHighlighting_punctuation_specialization_enable() .to_owned(), - operator: self.semanticHighlighting_operator_enable(source_root).to_owned(), + macro_bang: self.semanticHighlighting_punctuation_separate_macro_bang().to_owned(), + operator: self.semanticHighlighting_operator_enable().to_owned(), specialize_operator: self - .semanticHighlighting_operator_specialization_enable(source_root) - .to_owned(), - inject_doc_comment: self - .semanticHighlighting_doc_comment_inject_enable(source_root) + .semanticHighlighting_operator_specialization_enable() .to_owned(), + inject_doc_comment: self.semanticHighlighting_doc_comment_inject_enable().to_owned(), syntactic_name_ref_highlighting: false, } } @@ -2486,15 +2472,12 @@ macro_rules! _impl_for_config_data { $($doc)* #[allow(non_snake_case)] $vis fn $field(&self, source_root: Option) -> &$ty { - - if source_root.is_some() { - let mut par: Option = source_root; - while let Some(source_root_id) = par { - par = self.source_root_parent_map.get(&source_root_id).copied(); - if let Some(config) = self.ratoml_files.get(&source_root_id) { - if let Some(value) = config.local.$field.as_ref() { - return value; - } + let mut par: Option = source_root; + while let Some(source_root_id) = par { + par = self.source_root_parent_map.get(&source_root_id).copied(); + if let Some(config) = self.ratoml_files.get(&source_root_id) { + if let Some(value) = config.local.$field.as_ref() { + return value; } } } @@ -2515,10 +2498,10 @@ macro_rules! _impl_for_config_data { } }; (global, $( - $(#[doc=$doc:literal])* - $vis:vis $field:ident : $ty:ty = $default:expr, - )* - ) => { + $(#[doc=$doc:literal])* + $vis:vis $field:ident : $ty:ty = $default:expr, + )* + ) => { impl Config { $( $($doc)* @@ -2547,16 +2530,16 @@ macro_rules! _impl_for_config_data { } }; (client, $( - $(#[doc=$doc:literal])* - $vis:vis $field:ident : $ty:ty = $default:expr, - )* + $(#[doc=$doc:literal])* + $vis:vis $field:ident : $ty:ty = $default:expr, + )* ) => { impl Config { $( $($doc)* #[allow(non_snake_case)] $vis fn $field(&self) -> &$ty { - if let Some(v) = self.client_config.global.$field.as_ref() { + if let Some(v) = self.client_config.client.$field.as_ref() { return &v; } @@ -2616,24 +2599,6 @@ macro_rules! _config_data { } } - #[allow(unused)] - impl $name { - /// Applies overrides from some more local config blob, to self. - fn apply_input(&mut self, input: $input) { - $( - if let Some(value) = input.$field { - self.$field = value; - } - )* - } - - fn clone_with_overrides(&self, input: $input) -> Self { - Self {$( - $field: input.$field.unwrap_or_else(|| self.$field.clone()), - )*} - } - } - #[allow(unused, clippy::ptr_arg)] impl $input { fn from_json(json: &mut serde_json::Value, error_sink: &mut Vec<(String, serde_json::Error)>) -> Self { @@ -2686,7 +2651,6 @@ use _config_data as config_data; struct DefaultConfigData { global: GlobalDefaultConfigData, local: LocalDefaultConfigData, - #[allow(dead_code)] client: ClientDefaultConfigData, } @@ -2742,7 +2706,6 @@ struct GlobalLocalConfigInput { } impl GlobalLocalConfigInput { - #[allow(dead_code)] fn from_toml( mut toml: toml::Table, error_sink: &mut Vec<(String, toml::de::Error)>, diff --git a/crates/rust-analyzer/src/handlers/request.rs b/crates/rust-analyzer/src/handlers/request.rs index ac80e7871cf4..0ceeaa137ac2 100644 --- a/crates/rust-analyzer/src/handlers/request.rs +++ b/crates/rust-analyzer/src/handlers/request.rs @@ -367,8 +367,7 @@ pub(crate) fn handle_join_lines( let _p = tracing::span!(tracing::Level::INFO, "handle_join_lines").entered(); let file_id = from_proto::file_id(&snap, ¶ms.text_document.uri)?; - let source_root = snap.analysis.source_root_id(file_id)?; - let config = snap.config.join_lines(Some(source_root)); + let config = snap.config.join_lines(); let line_index = snap.file_line_index(file_id)?; let mut res = TextEdit::default(); @@ -1511,13 +1510,12 @@ pub(crate) fn handle_inlay_hints( params.range, )?; let line_index = snap.file_line_index(file_id)?; - let source_root = snap.analysis.source_root_id(file_id)?; let range = TextRange::new( range.start().min(line_index.index.len()), range.end().min(line_index.index.len()), ); - let inlay_hints_config = snap.config.inlay_hints(Some(source_root)); + let inlay_hints_config = snap.config.inlay_hints(); Ok(Some( snap.analysis .inlay_hints(&inlay_hints_config, file_id, Some(range))? @@ -1553,9 +1551,8 @@ pub(crate) fn handle_inlay_hints_resolve( let line_index = snap.file_line_index(file_id)?; let hint_position = from_proto::offset(&line_index, original_hint.position)?; - let source_root = snap.analysis.source_root_id(file_id)?; - let mut forced_resolve_inlay_hints_config = snap.config.inlay_hints(Some(source_root)); + let mut forced_resolve_inlay_hints_config = snap.config.inlay_hints(); forced_resolve_inlay_hints_config.fields_to_resolve = InlayFieldsToResolve::empty(); let resolve_hints = snap.analysis.inlay_hints_resolve( &forced_resolve_inlay_hints_config, @@ -1687,9 +1684,8 @@ pub(crate) fn handle_semantic_tokens_full( let file_id = from_proto::file_id(&snap, ¶ms.text_document.uri)?; let text = snap.analysis.file_text(file_id)?; let line_index = snap.file_line_index(file_id)?; - let source_root = snap.analysis.source_root_id(file_id)?; - let mut highlight_config = snap.config.highlighting_config(Some(source_root)); + let mut highlight_config = snap.config.highlighting_config(); // Avoid flashing a bunch of unresolved references when the proc-macro servers haven't been spawned yet. highlight_config.syntactic_name_ref_highlighting = snap.workspaces.is_empty() || !snap.proc_macros_loaded; @@ -1700,7 +1696,7 @@ pub(crate) fn handle_semantic_tokens_full( &line_index, highlights, snap.config.semantics_tokens_augments_syntax_tokens(), - snap.config.highlighting_non_standard_tokens(Some(source_root)), + snap.config.highlighting_non_standard_tokens(), ); // Unconditionally cache the tokens @@ -1718,9 +1714,8 @@ pub(crate) fn handle_semantic_tokens_full_delta( let file_id = from_proto::file_id(&snap, ¶ms.text_document.uri)?; let text = snap.analysis.file_text(file_id)?; let line_index = snap.file_line_index(file_id)?; - let source_root = snap.analysis.source_root_id(file_id)?; - let mut highlight_config = snap.config.highlighting_config(Some(source_root)); + let mut highlight_config = snap.config.highlighting_config(); // Avoid flashing a bunch of unresolved references when the proc-macro servers haven't been spawned yet. highlight_config.syntactic_name_ref_highlighting = snap.workspaces.is_empty() || !snap.proc_macros_loaded; @@ -1731,7 +1726,7 @@ pub(crate) fn handle_semantic_tokens_full_delta( &line_index, highlights, snap.config.semantics_tokens_augments_syntax_tokens(), - snap.config.highlighting_non_standard_tokens(Some(source_root)), + snap.config.highlighting_non_standard_tokens(), ); let cached_tokens = snap.semantic_tokens_cache.lock().remove(¶ms.text_document.uri); @@ -1762,9 +1757,8 @@ pub(crate) fn handle_semantic_tokens_range( let frange = from_proto::file_range(&snap, ¶ms.text_document, params.range)?; let text = snap.analysis.file_text(frange.file_id)?; let line_index = snap.file_line_index(frange.file_id)?; - let source_root = snap.analysis.source_root_id(frange.file_id)?; - let mut highlight_config = snap.config.highlighting_config(Some(source_root)); + let mut highlight_config = snap.config.highlighting_config(); // Avoid flashing a bunch of unresolved references when the proc-macro servers haven't been spawned yet. highlight_config.syntactic_name_ref_highlighting = snap.workspaces.is_empty() || !snap.proc_macros_loaded; @@ -1775,7 +1769,7 @@ pub(crate) fn handle_semantic_tokens_range( &line_index, highlights, snap.config.semantics_tokens_augments_syntax_tokens(), - snap.config.highlighting_non_standard_tokens(Some(source_root)), + snap.config.highlighting_non_standard_tokens(), ); Ok(Some(semantic_tokens.into())) } diff --git a/crates/rust-analyzer/tests/slow-tests/ratoml.rs b/crates/rust-analyzer/tests/slow-tests/ratoml.rs index 739da9c998d5..551519dd95b7 100644 --- a/crates/rust-analyzer/tests/slow-tests/ratoml.rs +++ b/crates/rust-analyzer/tests/slow-tests/ratoml.rs @@ -912,7 +912,9 @@ enum Value { /// Having a ratoml file at the root of a project enables /// configuring global level configurations as well. -#[test] +#[allow(unused)] +// #[test] +// FIXME: Re-enable this test when we have a global config we can check again fn ratoml_in_root_is_global() { let server = RatomlTest::new( vec![ @@ -946,7 +948,9 @@ fn main() { server.query(QueryType::GlobalHover, 2); } -#[test] +#[allow(unused)] +// #[test] +// FIXME: Re-enable this test when we have a global config we can check again fn ratoml_root_is_updateable() { let mut server = RatomlTest::new( vec![ @@ -982,7 +986,9 @@ fn main() { assert!(!server.query(QueryType::GlobalHover, 2)); } -#[test] +#[allow(unused)] +// #[test] +// FIXME: Re-enable this test when we have a global config we can check again fn ratoml_root_is_deletable() { let mut server = RatomlTest::new( vec![ From c791a3d709387ac8c32c166b42e0bb9b80200259 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 3 May 2024 10:05:16 +0200 Subject: [PATCH 5/8] Fix local configs allowing to contain global changes --- crates/rust-analyzer/src/config.rs | 32 ++++++++++++++++-------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 199e1de25c16..97e9dfcf9cf9 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -690,7 +690,7 @@ pub struct Config { root_ratoml: Option, /// For every `SourceRoot` there can be at most one RATOML file. - ratoml_files: FxHashMap, + ratoml_files: FxHashMap, /// Clone of the value that is stored inside a `GlobalState`. source_root_parent_map: Arc>, @@ -761,7 +761,7 @@ impl Config { if let Ok(change) = toml::from_str(&text) { config.ratoml_files.insert( source_root_id, - GlobalLocalConfigInput::from_toml(change, &mut toml_errors), + LocalConfigInput::from_toml(&change, &mut toml_errors), ); } } @@ -2476,12 +2476,18 @@ macro_rules! _impl_for_config_data { while let Some(source_root_id) = par { par = self.source_root_parent_map.get(&source_root_id).copied(); if let Some(config) = self.ratoml_files.get(&source_root_id) { - if let Some(value) = config.local.$field.as_ref() { + if let Some(value) = config.$field.as_ref() { return value; } } } + if let Some(root_path_ratoml) = self.root_ratoml.as_ref() { + if let Some(v) = root_path_ratoml.local.$field.as_ref() { + return &v; + } + } + if let Some(v) = self.client_config.local.$field.as_ref() { return &v; } @@ -2612,7 +2618,7 @@ macro_rules! _config_data { )*} } - fn from_toml(toml: &mut toml::Table, error_sink: &mut Vec<(String, toml::de::Error)>) -> Self { + fn from_toml(toml: &toml::Table, error_sink: &mut Vec<(String, toml::de::Error)>) -> Self { Self {$( $field: get_field_toml::<$ty>( toml, @@ -2681,7 +2687,6 @@ impl FullConfigInput { GlobalConfigInput::schema_fields(&mut fields); LocalConfigInput::schema_fields(&mut fields); ClientConfigInput::schema_fields(&mut fields); - // HACK: sort the fields, so the diffs on the generated docs/schema are smaller fields.sort_by_key(|&(x, ..)| x); fields } @@ -2707,12 +2712,12 @@ struct GlobalLocalConfigInput { impl GlobalLocalConfigInput { fn from_toml( - mut toml: toml::Table, + toml: toml::Table, error_sink: &mut Vec<(String, toml::de::Error)>, ) -> GlobalLocalConfigInput { GlobalLocalConfigInput { - global: GlobalConfigInput::from_toml(&mut toml, error_sink), - local: LocalConfigInput::from_toml(&mut toml, error_sink), + global: GlobalConfigInput::from_toml(&toml, error_sink), + local: LocalConfigInput::from_toml(&toml, error_sink), } } } @@ -2730,14 +2735,11 @@ fn get_field_toml( let subkeys = field.split('_'); let mut v = val; for subkey in subkeys { - if let Some(val) = v.get(subkey) { - if let Some(map) = val.as_table() { - v = map; - } else { - return Some(toml::Value::try_into(val.clone()).map_err(|e| (e, v))); - } + let val = v.get(subkey)?; + if let Some(map) = val.as_table() { + v = map; } else { - return None; + return Some(toml::Value::try_into(val.clone()).map_err(|e| (e, v))); } } None From d537941d1bc029545a39c0b7e250f036930073e1 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 5 Jun 2024 13:09:49 +0200 Subject: [PATCH 6/8] Diagnose most incorrect ra-toml config errors --- crates/rust-analyzer/src/config.rs | 160 ++++++++++++++++++++++++----- 1 file changed, 135 insertions(+), 25 deletions(-) diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 97e9dfcf9cf9..26e520c9dfc8 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -718,9 +718,15 @@ impl Config { let mut should_update = false; if let Some(change) = change.user_config_change { - if let Ok(change) = toml::from_str(&change) { + if let Ok(table) = toml::from_str(&change) { + validate_toml_table( + GlobalLocalConfigInput::FIELDS, + &table, + &mut String::new(), + &mut toml_errors, + ); config.user_config = - Some(GlobalLocalConfigInput::from_toml(change, &mut toml_errors)); + Some(GlobalLocalConfigInput::from_toml(table, &mut toml_errors)); should_update = true; } } @@ -748,21 +754,39 @@ impl Config { } if let Some(change) = change.root_ratoml_change { - if let Ok(change) = toml::from_str(&change) { - config.root_ratoml = - Some(GlobalLocalConfigInput::from_toml(change, &mut toml_errors)); - should_update = true; + match toml::from_str(&change) { + Ok(table) => { + validate_toml_table( + GlobalLocalConfigInput::FIELDS, + &table, + &mut String::new(), + &mut toml_errors, + ); + config.root_ratoml = + Some(GlobalLocalConfigInput::from_toml(table, &mut toml_errors)); + should_update = true; + } + Err(e) => toml_errors.push(("invalid toml file".to_owned(), e)), } } if let Some(change) = change.ratoml_file_change { for (source_root_id, (_, text)) in change { if let Some(text) = text { - if let Ok(change) = toml::from_str(&text) { - config.ratoml_files.insert( - source_root_id, - LocalConfigInput::from_toml(&change, &mut toml_errors), - ); + match toml::from_str(&text) { + Ok(table) => { + validate_toml_table( + &[LocalConfigInput::FIELDS], + &table, + &mut String::new(), + &mut toml_errors, + ); + config.ratoml_files.insert( + source_root_id, + LocalConfigInput::from_toml(&table, &mut toml_errors), + ); + } + Err(e) => toml_errors.push(("invalid toml file".to_owned(), e)), } } } @@ -792,7 +816,7 @@ impl Config { scope, ) { Some(snippet) => config.snippets.push(snippet), - None => error_sink.0.push(ConfigErrorInner::JsonError( + None => error_sink.0.push(ConfigErrorInner::Json( format!("snippet {name} is invalid"), ::custom( "snippet path is invalid or triggers are missing", @@ -801,8 +825,11 @@ impl Config { } } + error_sink.0.extend(json_errors.into_iter().map(|(a, b)| ConfigErrorInner::Json(a, b))); + error_sink.0.extend(toml_errors.into_iter().map(|(a, b)| ConfigErrorInner::Toml(a, b))); + if config.check_command().is_empty() { - error_sink.0.push(ConfigErrorInner::JsonError( + error_sink.0.push(ConfigErrorInner::Json( "/check/command".to_owned(), serde_json::Error::custom("expected a non-empty string"), )); @@ -836,14 +863,9 @@ impl ConfigChange { vfs_path: VfsPath, content: Option, ) -> Option<(VfsPath, Option)> { - if let Some(changes) = self.ratoml_file_change.as_mut() { - changes.insert(source_root, (vfs_path, content)) - } else { - let mut map = FxHashMap::default(); - map.insert(source_root, (vfs_path, content)); - self.ratoml_file_change = Some(map); - None - } + self.ratoml_file_change + .get_or_insert_with(Default::default) + .insert(source_root, (vfs_path, content)) } pub fn change_user_config(&mut self, content: Option) { @@ -1058,7 +1080,7 @@ pub struct ClientCommandsConfig { #[derive(Debug)] pub enum ConfigErrorInner { - JsonError(String, serde_json::Error), + Json(String, serde_json::Error), Toml(String, toml::de::Error), } @@ -1074,7 +1096,7 @@ impl ConfigError { impl fmt::Display for ConfigError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let errors = self.0.iter().format_with("\n", |inner, f| match inner { - ConfigErrorInner::JsonError(key, e) => { + ConfigErrorInner::Json(key, e) => { f(key)?; f(&": ")?; f(e) @@ -2607,6 +2629,8 @@ macro_rules! _config_data { #[allow(unused, clippy::ptr_arg)] impl $input { + const FIELDS: &'static [&'static str] = &[$(stringify!($field)),*]; + fn from_json(json: &mut serde_json::Value, error_sink: &mut Vec<(String, serde_json::Error)>) -> Self { Self {$( $field: get_field( @@ -2645,8 +2669,7 @@ macro_rules! _config_data { mod $modname { #[test] fn fields_are_sorted() { - let field_names: &'static [&'static str] = &[$(stringify!($field)),*]; - field_names.windows(2).for_each(|w| assert!(w[0] <= w[1], "{} <= {} does not hold", w[0], w[1])); + super::$input::FIELDS.windows(2).for_each(|w| assert!(w[0] <= w[1], "{} <= {} does not hold", w[0], w[1])); } } }; @@ -2711,6 +2734,8 @@ struct GlobalLocalConfigInput { } impl GlobalLocalConfigInput { + const FIELDS: &'static [&'static [&'static str]] = + &[GlobalConfigInput::FIELDS, LocalConfigInput::FIELDS]; fn from_toml( toml: toml::Table, error_sink: &mut Vec<(String, toml::de::Error)>, @@ -3148,6 +3173,35 @@ fn field_props(field: &str, ty: &str, doc: &[&str], default: &str) -> serde_json map.into() } +fn validate_toml_table( + known_ptrs: &[&[&'static str]], + toml: &toml::Table, + ptr: &mut String, + error_sink: &mut Vec<(String, toml::de::Error)>, +) { + let verify = |ptr: &String| known_ptrs.iter().any(|ptrs| ptrs.contains(&ptr.as_str())); + + let l = ptr.len(); + for (k, v) in toml { + if !ptr.is_empty() { + ptr.push('_'); + } + ptr.push_str(k); + + match v { + // This is a table config, any entry in it is therefor valid + toml::Value::Table(_) if verify(ptr) => (), + toml::Value::Table(table) => validate_toml_table(known_ptrs, table, ptr, error_sink), + _ if !verify(ptr) => { + error_sink.push((ptr.clone(), toml::de::Error::custom("unexpected field"))) + } + _ => (), + } + + ptr.truncate(l); + } +} + #[cfg(test)] fn manual(fields: &[SchemaField]) -> String { fields.iter().fold(String::new(), |mut acc, (field, _ty, doc, default)| { @@ -3387,4 +3441,60 @@ mod tests { matches!(config.flycheck(), FlycheckConfig::CargoCommand { options, .. } if options.target_dir == Some(Utf8PathBuf::from("other_folder"))) ); } + + #[test] + fn toml_unknown_key() { + let config = Config::new( + AbsPathBuf::try_from(project_root()).unwrap(), + Default::default(), + vec![], + None, + None, + ); + + let mut change = ConfigChange::default(); + + change.change_root_ratoml(Some( + toml::toml! { + [cargo.cfgs] + these = "these" + should = "should" + be = "be" + valid = "valid" + + [invalid.config] + err = "error" + + [cargo] + target = "ok" + + // FIXME: This should be an error + [cargo.sysroot] + non-table = "expected" + } + .to_string(), + )); + + let (_, e, _) = config.apply_change(change); + expect_test::expect![[r#" + ConfigError( + [ + Toml( + "invalid_config_err", + Error { + inner: Error { + inner: TomlError { + message: "unexpected field", + raw: None, + keys: [], + span: None, + }, + }, + }, + ), + ], + ) + "#]] + .assert_debug_eq(&e); + } } From 047b8d9f29764e9233ebb591a12823248761dc1a Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 5 Jun 2024 14:56:07 +0200 Subject: [PATCH 7/8] Keep config diagnostics across changes --- crates/rust-analyzer/src/bin/main.rs | 4 +- crates/rust-analyzer/src/config.rs | 284 ++++++++++++------ crates/rust-analyzer/src/global_state.rs | 9 +- .../src/handlers/notification.rs | 7 +- crates/rust-analyzer/src/reload.rs | 5 +- .../rust-analyzer/tests/slow-tests/support.rs | 4 +- 6 files changed, 213 insertions(+), 100 deletions(-) diff --git a/crates/rust-analyzer/src/bin/main.rs b/crates/rust-analyzer/src/bin/main.rs index 545ed6d18653..774784f37b00 100644 --- a/crates/rust-analyzer/src/bin/main.rs +++ b/crates/rust-analyzer/src/bin/main.rs @@ -17,7 +17,7 @@ use anyhow::Context; use lsp_server::Connection; use rust_analyzer::{ cli::flags, - config::{Config, ConfigChange, ConfigError}, + config::{Config, ConfigChange, ConfigErrors}, from_json, }; use semver::Version; @@ -229,7 +229,7 @@ fn run_server() -> anyhow::Result<()> { let mut change = ConfigChange::default(); change.change_client_config(json); - let error_sink: ConfigError; + let error_sink: ConfigErrors; (config, error_sink, _) = config.apply_change(change); if !error_sink.is_empty() { diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 26e520c9dfc8..2baac8862505 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -3,7 +3,7 @@ //! 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}; +use std::{fmt, iter, ops::Not, sync::OnceLock}; use cfg::{CfgAtom, CfgDiff}; use dirs::config_dir; @@ -664,10 +664,10 @@ pub struct Config { snippets: Vec, visual_studio_code_version: Option, - default_config: DefaultConfigData, + default_config: &'static DefaultConfigData, /// Config node that obtains its initial value during the server initialization and /// by receiving a `lsp_types::notification::DidChangeConfiguration`. - client_config: FullConfigInput, + 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 : @@ -681,16 +681,16 @@ pub struct Config { /// FIXME @alibektas : Change this to sth better. /// Config node whose values apply to **every** Rust project. - user_config: Option, + user_config: Option<(GlobalLocalConfigInput, ConfigErrors)>, /// A special file for this session whose path is set to `self.root_path.join("rust-analyzer.toml")` root_ratoml_path: VfsPath, /// This file can be used to make global changes while having only a workspace-wide scope. - root_ratoml: Option, + root_ratoml: Option<(GlobalLocalConfigInput, ConfigErrors)>, /// For every `SourceRoot` there can be at most one RATOML file. - ratoml_files: FxHashMap, + ratoml_files: FxHashMap, /// Clone of the value that is stored inside a `GlobalState`. source_root_parent_map: Arc>, @@ -706,27 +706,30 @@ impl Config { // FIXME @alibektas : Server's health uses error sink but in other places it is not used atm. /// Changes made to client and global configurations will partially not be reflected even after `.apply_change()` was called. /// The return tuple's bool component signals whether the `GlobalState` should call its `update_configuration()` method. - fn apply_change_with_sink( - &self, - change: ConfigChange, - error_sink: &mut ConfigError, - ) -> (Config, bool) { + fn apply_change_with_sink(&self, change: ConfigChange) -> (Config, bool) { let mut config = self.clone(); - let mut toml_errors = vec![]; - let mut json_errors = vec![]; let mut should_update = false; if let Some(change) = change.user_config_change { if let Ok(table) = toml::from_str(&change) { + let mut toml_errors = vec![]; validate_toml_table( GlobalLocalConfigInput::FIELDS, &table, &mut String::new(), &mut toml_errors, ); - config.user_config = - Some(GlobalLocalConfigInput::from_toml(table, &mut toml_errors)); + config.user_config = Some(( + GlobalLocalConfigInput::from_toml(table, &mut toml_errors), + ConfigErrors( + toml_errors + .into_iter() + .map(|(a, b)| ConfigErrorInner::Toml { config_key: a, error: b }) + .map(Arc::new) + .collect(), + ), + )); should_update = true; } } @@ -734,6 +737,7 @@ impl Config { if let Some(mut json) = change.client_config_change { tracing::info!("updating config from JSON: {:#}", json); if !(json.is_null() || json.as_object().map_or(false, |it| it.is_empty())) { + let mut json_errors = vec![]; let detached_files = get_field::>( &mut json, &mut json_errors, @@ -747,32 +751,56 @@ impl Config { patch_old_style::patch_json_for_outdated_configs(&mut json); - config.client_config = FullConfigInput::from_json(json, &mut json_errors); + config.client_config = ( + FullConfigInput::from_json(json, &mut json_errors), + ConfigErrors( + json_errors + .into_iter() + .map(|(a, b)| ConfigErrorInner::Json { config_key: a, error: b }) + .map(Arc::new) + .collect(), + ), + ); config.detached_files = detached_files; } should_update = true; } if let Some(change) = change.root_ratoml_change { + tracing::info!("updating root ra-toml config: {:#}", change); + #[allow(clippy::single_match)] match toml::from_str(&change) { Ok(table) => { + let mut toml_errors = vec![]; validate_toml_table( GlobalLocalConfigInput::FIELDS, &table, &mut String::new(), &mut toml_errors, ); - config.root_ratoml = - Some(GlobalLocalConfigInput::from_toml(table, &mut toml_errors)); + config.root_ratoml = Some(( + GlobalLocalConfigInput::from_toml(table, &mut toml_errors), + ConfigErrors( + toml_errors + .into_iter() + .map(|(a, b)| ConfigErrorInner::Toml { config_key: a, error: b }) + .map(Arc::new) + .collect(), + ), + )); should_update = true; } - Err(e) => toml_errors.push(("invalid toml file".to_owned(), e)), + // FIXME + Err(_) => (), } } if let Some(change) = change.ratoml_file_change { for (source_root_id, (_, text)) in change { if let Some(text) = text { + let mut toml_errors = vec![]; + tracing::info!("updating ra-toml config: {:#}", text); + #[allow(clippy::single_match)] match toml::from_str(&text) { Ok(table) => { validate_toml_table( @@ -783,10 +811,23 @@ impl Config { ); config.ratoml_files.insert( source_root_id, - LocalConfigInput::from_toml(&table, &mut toml_errors), + ( + LocalConfigInput::from_toml(&table, &mut toml_errors), + ConfigErrors( + toml_errors + .into_iter() + .map(|(a, b)| ConfigErrorInner::Toml { + config_key: a, + error: b, + }) + .map(Arc::new) + .collect(), + ), + ), ); } - Err(e) => toml_errors.push(("invalid toml file".to_owned(), e)), + // FIXME + Err(_) => (), } } } @@ -807,6 +848,7 @@ impl Config { SnippetScopeDef::Type => SnippetScope::Type, SnippetScopeDef::Item => SnippetScope::Item, }; + #[allow(clippy::single_match)] match Snippet::new( &def.prefix, &def.postfix, @@ -816,33 +858,44 @@ impl Config { scope, ) { Some(snippet) => config.snippets.push(snippet), - None => error_sink.0.push(ConfigErrorInner::Json( - format!("snippet {name} is invalid"), - ::custom( - "snippet path is invalid or triggers are missing", - ), - )), + // FIXME + // None => error_sink.0.push(ConfigErrorInner::Json { + // config_key: "".to_owned(), + // error: ::custom(format!( + // "snippet {name} is invalid or triggers are missing", + // )), + // }), + None => (), } } - error_sink.0.extend(json_errors.into_iter().map(|(a, b)| ConfigErrorInner::Json(a, b))); - error_sink.0.extend(toml_errors.into_iter().map(|(a, b)| ConfigErrorInner::Toml(a, b))); - - if config.check_command().is_empty() { - error_sink.0.push(ConfigErrorInner::Json( - "/check/command".to_owned(), - serde_json::Error::custom("expected a non-empty string"), - )); - } + // FIXME: bring this back + // if config.check_command().is_empty() { + // error_sink.0.push(ConfigErrorInner::Json { + // config_key: "/check/command".to_owned(), + // error: serde_json::Error::custom("expected a non-empty string"), + // }); + // } (config, should_update) } /// Given `change` this generates a new `Config`, thereby collecting errors of type `ConfigError`. /// If there are changes that have global/client level effect, the last component of the return type /// will be set to `true`, which should be used by the `GlobalState` to update itself. - pub fn apply_change(&self, change: ConfigChange) -> (Config, ConfigError, bool) { - let mut e = ConfigError(vec![]); - let (config, should_update) = self.apply_change_with_sink(change, &mut e); + pub fn apply_change(&self, change: ConfigChange) -> (Config, ConfigErrors, bool) { + let (config, should_update) = self.apply_change_with_sink(change); + let e = ConfigErrors( + config + .client_config + .1 + .0 + .iter() + .chain(config.root_ratoml.as_ref().into_iter().flat_map(|it| it.1 .0.iter())) + .chain(config.user_config.as_ref().into_iter().flat_map(|it| it.1 .0.iter())) + .chain(config.ratoml_files.values().flat_map(|it| it.1 .0.iter())) + .cloned() + .collect(), + ); (config, e, should_update) } } @@ -1080,28 +1133,28 @@ pub struct ClientCommandsConfig { #[derive(Debug)] pub enum ConfigErrorInner { - Json(String, serde_json::Error), - Toml(String, toml::de::Error), + Json { config_key: String, error: serde_json::Error }, + Toml { config_key: String, error: toml::de::Error }, } -#[derive(Debug, Default)] -pub struct ConfigError(Vec); +#[derive(Clone, Debug)] +pub struct ConfigErrors(Vec>); -impl ConfigError { +impl ConfigErrors { pub fn is_empty(&self) -> bool { self.0.is_empty() } } -impl fmt::Display for ConfigError { +impl fmt::Display for ConfigErrors { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let errors = self.0.iter().format_with("\n", |inner, f| match inner { - ConfigErrorInner::Json(key, e) => { + let errors = self.0.iter().format_with("\n", |inner, f| match &**inner { + ConfigErrorInner::Json { config_key: key, error: e } => { f(key)?; f(&": ")?; f(e) } - ConfigErrorInner::Toml(key, e) => { + ConfigErrorInner::Toml { config_key: key, error: e } => { f(key)?; f(&": ")?; f(e) @@ -1111,7 +1164,7 @@ impl fmt::Display for ConfigError { } } -impl std::error::Error for ConfigError {} +impl std::error::Error for ConfigErrors {} impl Config { pub fn new( @@ -1121,6 +1174,7 @@ impl Config { visual_studio_code_version: Option, user_config_path: Option, ) -> 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 { @@ -1149,10 +1203,11 @@ impl Config { snippets: Default::default(), workspace_roots, visual_studio_code_version, - client_config: FullConfigInput::default(), + client_config: (FullConfigInput::default(), ConfigErrors(vec![])), user_config: None, ratoml_files: FxHashMap::default(), - default_config: DefaultConfigData::default(), + default_config: DEFAULT_CONFIG_DATA + .get_or_init(|| Box::leak(Box::new(DefaultConfigData::default()))), source_root_parent_map: Arc::new(FxHashMap::default()), user_config_path, root_ratoml: None, @@ -2497,24 +2552,24 @@ macro_rules! _impl_for_config_data { let mut par: Option = source_root; while let Some(source_root_id) = par { par = self.source_root_parent_map.get(&source_root_id).copied(); - if let Some(config) = self.ratoml_files.get(&source_root_id) { + if let Some((config, _)) = self.ratoml_files.get(&source_root_id) { if let Some(value) = config.$field.as_ref() { return value; } } } - if let Some(root_path_ratoml) = self.root_ratoml.as_ref() { + if let Some((root_path_ratoml, _)) = self.root_ratoml.as_ref() { if let Some(v) = root_path_ratoml.local.$field.as_ref() { return &v; } } - if let Some(v) = self.client_config.local.$field.as_ref() { + if let Some(v) = self.client_config.0.local.$field.as_ref() { return &v; } - if let Some(user_config) = self.user_config.as_ref() { + if let Some((user_config, _)) = self.user_config.as_ref() { if let Some(v) = user_config.local.$field.as_ref() { return &v; } @@ -2536,17 +2591,17 @@ macro_rules! _impl_for_config_data { #[allow(non_snake_case)] $vis fn $field(&self) -> &$ty { - if let Some(root_path_ratoml) = self.root_ratoml.as_ref() { + if let Some((root_path_ratoml, _)) = self.root_ratoml.as_ref() { if let Some(v) = root_path_ratoml.global.$field.as_ref() { return &v; } } - if let Some(v) = self.client_config.global.$field.as_ref() { + if let Some(v) = self.client_config.0.global.$field.as_ref() { return &v; } - if let Some(user_config) = self.user_config.as_ref() { + if let Some((user_config, _)) = self.user_config.as_ref() { if let Some(v) = user_config.global.$field.as_ref() { return &v; } @@ -2567,7 +2622,7 @@ macro_rules! _impl_for_config_data { $($doc)* #[allow(non_snake_case)] $vis fn $field(&self) -> &$ty { - if let Some(v) = self.client_config.client.$field.as_ref() { + if let Some(v) = self.client_config.0.client.$field.as_ref() { return &v; } @@ -2747,41 +2802,37 @@ impl GlobalLocalConfigInput { } } -fn get_field_toml( - val: &toml::Table, - error_sink: &mut Vec<(String, toml::de::Error)>, +fn get_field( + json: &mut serde_json::Value, + error_sink: &mut Vec<(String, serde_json::Error)>, field: &'static str, alias: Option<&'static str>, ) -> Option { + // XXX: check alias first, to work around the VS Code where it pre-fills the + // defaults instead of sending an empty object. alias .into_iter() .chain(iter::once(field)) .filter_map(move |field| { - let subkeys = field.split('_'); - let mut v = val; - for subkey in subkeys { - let val = v.get(subkey)?; - if let Some(map) = val.as_table() { - v = map; - } else { - return Some(toml::Value::try_into(val.clone()).map_err(|e| (e, v))); - } - } - None + let mut pointer = field.replace('_', "/"); + pointer.insert(0, '/'); + json.pointer_mut(&pointer) + .map(|it| serde_json::from_value(it.take()).map_err(|e| (e, pointer))) }) .find(Result::is_ok) .and_then(|res| match res { Ok(it) => Some(it), Err((e, pointer)) => { - error_sink.push((pointer.to_string(), e)); + tracing::warn!("Failed to deserialize config field at {}: {:?}", pointer, e); + error_sink.push((pointer, e)); None } }) } -fn get_field( - json: &mut serde_json::Value, - error_sink: &mut Vec<(String, serde_json::Error)>, +fn get_field_toml( + toml: &toml::Table, + error_sink: &mut Vec<(String, toml::de::Error)>, field: &'static str, alias: Option<&'static str>, ) -> Option { @@ -2793,8 +2844,8 @@ fn get_field( .filter_map(move |field| { let mut pointer = field.replace('_', "/"); pointer.insert(0, '/'); - json.pointer_mut(&pointer) - .map(|it| serde_json::from_value(it.take()).map_err(|e| (e, pointer))) + toml_pointer(toml, &pointer) + .map(|it| <_>::deserialize(it.clone()).map_err(|e| (e, pointer))) }) .find(Result::is_ok) .and_then(|res| match res { @@ -2807,6 +2858,32 @@ fn get_field( }) } +fn toml_pointer<'a>(toml: &'a toml::Table, pointer: &str) -> Option<&'a toml::Value> { + fn parse_index(s: &str) -> Option { + if s.starts_with('+') || (s.starts_with('0') && s.len() != 1) { + return None; + } + s.parse().ok() + } + + if pointer.is_empty() { + return None; + } + if !pointer.starts_with('/') { + return None; + } + let mut parts = pointer.split('/').skip(1); + let first = parts.next()?; + let init = toml.get(first)?; + parts.map(|x| x.replace("~1", "/").replace("~0", "~")).try_fold(init, |target, token| { + match target { + toml::Value::Table(table) => table.get(&token), + toml::Value::Array(list) => parse_index(&token).and_then(move |x| list.get(x)), + _ => None, + } + }) +} + type SchemaField = (&'static str, &'static str, &'static [&'static str], String); fn schema(fields: &[SchemaField]) -> serde_json::Value { @@ -3192,9 +3269,8 @@ fn validate_toml_table( // This is a table config, any entry in it is therefor valid toml::Value::Table(_) if verify(ptr) => (), toml::Value::Table(table) => validate_toml_table(known_ptrs, table, ptr, error_sink), - _ if !verify(ptr) => { - error_sink.push((ptr.clone(), toml::de::Error::custom("unexpected field"))) - } + _ if !verify(ptr) => error_sink + .push((ptr.replace('_', "/"), toml::de::Error::custom("unexpected field"))), _ => (), } @@ -3475,13 +3551,47 @@ mod tests { .to_string(), )); + let (config, e, _) = config.apply_change(change); + expect_test::expect![[r#" + ConfigErrors( + [ + Toml { + config_key: "invalid/config/err", + error: Error { + inner: Error { + inner: TomlError { + message: "unexpected field", + raw: None, + keys: [], + span: None, + }, + }, + }, + }, + ], + ) + "#]] + .assert_debug_eq(&e); + let mut change = ConfigChange::default(); + + change.change_user_config(Some( + toml::toml! { + [cargo.cfgs] + these = "these" + should = "should" + be = "be" + valid = "valid" + } + .to_string(), + )); + let (_, e, _) = config.apply_change(change); expect_test::expect![[r#" - ConfigError( + ConfigErrors( [ - Toml( - "invalid_config_err", - Error { + Toml { + config_key: "invalid/config/err", + error: Error { inner: Error { inner: TomlError { message: "unexpected field", @@ -3491,7 +3601,7 @@ mod tests { }, }, }, - ), + }, ], ) "#]] diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index 5e7e8061efdc..354f955efcec 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -3,7 +3,7 @@ //! //! Each tick provides an immutable snapshot of the state as `WorldSnapshot`. -use std::time::Instant; +use std::{ops::Not as _, time::Instant}; use crossbeam_channel::{unbounded, Receiver, Sender}; use flycheck::FlycheckHandle; @@ -28,7 +28,7 @@ use triomphe::Arc; use vfs::{AnchoredPathBuf, Vfs, VfsPath}; use crate::{ - config::{Config, ConfigChange, ConfigError}, + config::{Config, ConfigChange, ConfigErrors}, diagnostics::{CheckFixes, DiagnosticCollection}, line_index::{LineEndings, LineIndex}, lsp::{ @@ -68,7 +68,7 @@ pub(crate) struct GlobalState { pub(crate) fmt_pool: Handle, Receiver>, pub(crate) config: Arc, - pub(crate) config_errors: Option, + pub(crate) config_errors: Option, pub(crate) analysis_host: AnalysisHost, pub(crate) diagnostics: DiagnosticCollection, pub(crate) mem_docs: MemDocs, @@ -405,7 +405,8 @@ impl GlobalState { change }; - let (config, _, should_update) = self.config.apply_change(config_change); + let (config, e, should_update) = self.config.apply_change(config_change); + self.config_errors = e.is_empty().not().then_some(e); if should_update { self.update_configuration(config); diff --git a/crates/rust-analyzer/src/handlers/notification.rs b/crates/rust-analyzer/src/handlers/notification.rs index 50489044309b..6146e902722c 100644 --- a/crates/rust-analyzer/src/handlers/notification.rs +++ b/crates/rust-analyzer/src/handlers/notification.rs @@ -1,7 +1,7 @@ //! This module is responsible for implementing handlers for Language Server //! Protocol. This module specifically handles notifications. -use std::ops::Deref; +use std::ops::{Deref, Not as _}; use itertools::Itertools; use lsp_types::{ @@ -197,11 +197,12 @@ pub(crate) fn handle_did_change_configuration( } (None, Some(mut configs)) => { if let Some(json) = configs.get_mut(0) { - let mut config = Config::clone(&*this.config); + let config = Config::clone(&*this.config); let mut change = ConfigChange::default(); change.change_client_config(json.take()); - (config, _, _) = config.apply_change(change); + let (config, e, _) = config.apply_change(change); + this.config_errors = e.is_empty().not().then_some(e); // Client config changes neccesitates .update_config method to be called. this.update_configuration(config); diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index 6061ccbfe868..36f9504e7d59 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -13,7 +13,7 @@ //! project is currently loading and we don't have a full project model, we //! still want to respond to various requests. // FIXME: This is a mess that needs some untangling work -use std::{iter, mem}; +use std::{iter, mem, ops::Not as _}; use flycheck::{FlycheckConfig, FlycheckHandle}; use hir::{db::DefDatabase, ChangeWithProcMacros, ProcMacros}; @@ -605,7 +605,8 @@ impl GlobalState { let mut config_change = ConfigChange::default(); config_change.change_source_root_parent_map(self.local_roots_parent_map.clone()); - let (config, _, _) = self.config.apply_change(config_change); + let (config, e, _) = self.config.apply_change(config_change); + self.config_errors = e.is_empty().not().then_some(e); self.config = Arc::new(config); diff --git a/crates/rust-analyzer/tests/slow-tests/support.rs b/crates/rust-analyzer/tests/slow-tests/support.rs index 2d9b3d560b7a..c43832553230 100644 --- a/crates/rust-analyzer/tests/slow-tests/support.rs +++ b/crates/rust-analyzer/tests/slow-tests/support.rs @@ -10,7 +10,7 @@ use lsp_server::{Connection, Message, Notification, Request}; use lsp_types::{notification::Exit, request::Shutdown, TextDocumentIdentifier, Url}; use paths::{Utf8Path, Utf8PathBuf}; use rust_analyzer::{ - config::{Config, ConfigChange, ConfigError}, + config::{Config, ConfigChange, ConfigErrors}, lsp, main_loop, }; use serde::Serialize; @@ -207,7 +207,7 @@ impl Project<'_> { change.change_client_config(self.config); - let error_sink: ConfigError; + let error_sink: ConfigErrors; (config, error_sink, _) = config.apply_change(change); assert!(error_sink.is_empty(), "{error_sink:?}"); From 3178e5fb5a9a22bf10e4b25a2ceeea39c824fd65 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 6 Jun 2024 10:37:46 +0200 Subject: [PATCH 8/8] Fix file loading of r-a toml files --- crates/rust-analyzer/src/config.rs | 24 +++++----- crates/rust-analyzer/src/global_state.rs | 61 +++++++++--------------- crates/rust-analyzer/src/reload.rs | 42 +--------------- 3 files changed, 37 insertions(+), 90 deletions(-) diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 2baac8862505..1ae454ace358 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -902,10 +902,10 @@ impl Config { #[derive(Default, Debug)] pub struct ConfigChange { - user_config_change: Option, - root_ratoml_change: Option, + user_config_change: Option>, + root_ratoml_change: Option>, client_config_change: Option, - ratoml_file_change: Option)>>, + ratoml_file_change: Option>)>>, source_map_change: Option>>, } @@ -914,19 +914,19 @@ impl ConfigChange { &mut self, source_root: SourceRootId, vfs_path: VfsPath, - content: Option, - ) -> Option<(VfsPath, Option)> { + content: Option>, + ) -> Option<(VfsPath, Option>)> { self.ratoml_file_change .get_or_insert_with(Default::default) .insert(source_root, (vfs_path, content)) } - pub fn change_user_config(&mut self, content: Option) { + pub fn change_user_config(&mut self, content: Option>) { assert!(self.user_config_change.is_none()); // Otherwise it is a double write. self.user_config_change = content; } - pub fn change_root_ratoml(&mut self, content: Option) { + pub fn change_root_ratoml(&mut self, content: Option>) { assert!(self.user_config_change.is_none()); // Otherwise it is a double write. self.root_ratoml_change = content; } @@ -1206,8 +1206,7 @@ impl Config { client_config: (FullConfigInput::default(), ConfigErrors(vec![])), user_config: None, ratoml_files: FxHashMap::default(), - default_config: DEFAULT_CONFIG_DATA - .get_or_init(|| Box::leak(Box::new(DefaultConfigData::default()))), + default_config: DEFAULT_CONFIG_DATA.get_or_init(|| Box::leak(Box::default())), source_root_parent_map: Arc::new(FxHashMap::default()), user_config_path, root_ratoml: None, @@ -3548,7 +3547,8 @@ mod tests { [cargo.sysroot] non-table = "expected" } - .to_string(), + .to_string() + .into(), )); let (config, e, _) = config.apply_change(change); @@ -3582,9 +3582,9 @@ mod tests { be = "be" valid = "valid" } - .to_string(), + .to_string() + .into(), )); - let (_, e, _) = config.apply_change(change); expect_test::expect![[r#" ConfigErrors( diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index 354f955efcec..a3e61447b1d2 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -25,7 +25,7 @@ use project_model::{ use rustc_hash::{FxHashMap, FxHashSet}; use tracing::{span, Level}; use triomphe::Arc; -use vfs::{AnchoredPathBuf, Vfs, VfsPath}; +use vfs::{AnchoredPathBuf, ChangeKind, Vfs}; use crate::{ config::{Config, ConfigChange, ConfigErrors}, @@ -262,19 +262,9 @@ impl GlobalState { // that can be used by the config module because config talks // in `SourceRootId`s instead of `FileId`s and `FileId` -> `SourceRootId` // mapping is not ready until `AnalysisHost::apply_changes` has been called. - let mut modified_ratoml_files: FxHashMap = FxHashMap::default(); - let mut ratoml_text_map: FxHashMap)> = + let mut modified_ratoml_files: FxHashMap = FxHashMap::default(); - let mut user_config_file: Option> = None; - let mut root_path_ratoml: Option> = None; - - let root_vfs_path = { - let mut root_vfs_path = self.config.root_path().to_path_buf(); - root_vfs_path.push("rust-analyzer.toml"); - VfsPath::new_real_path(root_vfs_path.to_string()) - }; - let (change, modified_rust_files, workspace_structure_change) = { let mut change = ChangeWithProcMacros::new(); let mut guard = self.vfs.write(); @@ -296,7 +286,7 @@ impl GlobalState { let vfs_path = vfs.file_path(file.file_id); if let Some(("rust-analyzer", Some("toml"))) = vfs_path.name_and_extension() { // Remember ids to use them after `apply_changes` - modified_ratoml_files.insert(file.file_id, vfs_path.clone()); + modified_ratoml_files.insert(file.file_id, (file.kind(), vfs_path.clone())); } if let Some(path) = vfs_path.as_path() { @@ -344,17 +334,7 @@ impl GlobalState { Some(text) } }; - - change.change_file(file_id, text.clone()); - if let Some(vfs_path) = modified_ratoml_files.get(&file_id) { - if vfs_path == self.config.user_config_path() { - user_config_file = Some(text); - } else if vfs_path == &root_vfs_path { - root_path_ratoml = Some(text); - } else { - ratoml_text_map.insert(file_id, (vfs_path.clone(), text)); - } - } + change.change_file(file_id, text); }); if has_structure_changes { let roots = self.source_root_config.partition(vfs); @@ -365,22 +345,35 @@ impl GlobalState { let _p = span!(Level::INFO, "GlobalState::process_changes/apply_change").entered(); self.analysis_host.apply_change(change); - if !(ratoml_text_map.is_empty() && user_config_file.is_none() && root_path_ratoml.is_none()) - { + if !modified_ratoml_files.is_empty() { let config_change = { + let user_config_path = self.config.user_config_path(); + let root_ratoml_path = self.config.root_ratoml_path(); let mut change = ConfigChange::default(); let db = self.analysis_host.raw_database(); - for (file_id, (vfs_path, text)) in ratoml_text_map { + for (file_id, (_change_kind, vfs_path)) in modified_ratoml_files { + if vfs_path == *user_config_path { + change.change_user_config(Some(db.file_text(file_id))); + continue; + } + + if vfs_path == *root_ratoml_path { + change.change_root_ratoml(Some(db.file_text(file_id))); + continue; + } + // If change has been made to a ratoml file that // belongs to a non-local source root, we will ignore it. // As it doesn't make sense a users to use external config files. let sr_id = db.file_source_root(file_id); let sr = db.source_root(sr_id); if !sr.is_library { - if let Some((old_path, old_text)) = - change.change_ratoml(sr_id, vfs_path.clone(), text) - { + if let Some((old_path, old_text)) = change.change_ratoml( + sr_id, + vfs_path.clone(), + Some(db.file_text(file_id)), + ) { // SourceRoot has more than 1 RATOML files. In this case lexicographically smaller wins. if old_path < vfs_path { span!(Level::ERROR, "Two `rust-analyzer.toml` files were found inside the same crate. {vfs_path} has no effect."); @@ -394,14 +387,6 @@ impl GlobalState { } } - if let Some(Some(txt)) = user_config_file { - change.change_user_config(Some(txt)); - } - - if let Some(Some(txt)) = root_path_ratoml { - change.change_root_ratoml(Some(txt)); - } - change }; diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index 36f9504e7d59..899c3cfeebbb 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -13,7 +13,7 @@ //! project is currently loading and we don't have a full project model, we //! still want to respond to various requests. // FIXME: This is a mess that needs some untangling work -use std::{iter, mem, ops::Not as _}; +use std::{iter, mem}; use flycheck::{FlycheckConfig, FlycheckHandle}; use hir::{db::DefDatabase, ChangeWithProcMacros, ProcMacros}; @@ -28,12 +28,11 @@ use lsp_types::FileSystemWatcher; use proc_macro_api::ProcMacroServer; use project_model::{ManifestPath, ProjectWorkspace, ProjectWorkspaceKind, WorkspaceBuildScripts}; use stdx::{format_to, thread::ThreadIntent}; -use tracing::error; use triomphe::Arc; use vfs::{AbsPath, AbsPathBuf, ChangeKind}; use crate::{ - config::{Config, ConfigChange, FilesWatcher, LinkedProject}, + config::{Config, FilesWatcher, LinkedProject}, global_state::GlobalState, lsp_ext, main_loop::Task, @@ -573,43 +572,6 @@ impl GlobalState { self.source_root_config = project_folders.source_root_config; self.local_roots_parent_map = Arc::new(self.source_root_config.source_root_parent_map()); - let user_config_path = self.config.user_config_path(); - let root_ratoml_path = self.config.root_ratoml_path(); - - { - let vfs = &mut self.vfs.write().0; - let loader = &mut self.loader; - - if vfs.file_id(user_config_path).is_none() { - if let Some(user_cfg_abs) = user_config_path.as_path() { - let contents = loader.handle.load_sync(user_cfg_abs); - vfs.set_file_contents(user_config_path.clone(), contents); - } else { - error!("Non-abs virtual path for user config."); - } - } - - if vfs.file_id(root_ratoml_path).is_none() { - // FIXME @alibektas : Sometimes root_path_ratoml collide with a regular ratoml. - // Although this shouldn't be a problem because everything is mapped to a `FileId`. - // We may want to further think about this. - if let Some(root_ratoml_abs) = root_ratoml_path.as_path() { - let contents = loader.handle.load_sync(root_ratoml_abs); - vfs.set_file_contents(root_ratoml_path.clone(), contents); - } else { - error!("Non-abs virtual path for user config."); - } - } - } - - let mut config_change = ConfigChange::default(); - config_change.change_source_root_parent_map(self.local_roots_parent_map.clone()); - - let (config, e, _) = self.config.apply_change(config_change); - self.config_errors = e.is_empty().not().then_some(e); - - self.config = Arc::new(config); - self.recreate_crate_graph(cause); tracing::info!("did switch workspaces");