From 93e50c988e6f555c5169363895db14953e168cb9 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Fri, 26 Jul 2024 02:38:02 +0200 Subject: [PATCH 1/7] Turn root_ratoml into workspace_ratomls --- .../crates/rust-analyzer/src/config.rs | 124 +++++++++--------- .../crates/rust-analyzer/src/global_state.rs | 32 +++-- .../crates/rust-analyzer/src/reload.rs | 1 - .../rust-analyzer/tests/slow-tests/ratoml.rs | 2 +- 4 files changed, 84 insertions(+), 75 deletions(-) diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs index 4a09ef21adc7..6724b6a7add3 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs @@ -779,11 +779,8 @@ pub struct Config { /// Config node whose values apply to **every** Rust project. 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<(GlobalLocalConfigInput, ConfigErrors)>, + /// TODO : This file can be used to make global changes while having only a workspace-wide scope. + workspace_ratoml_change: FxHashMap, /// For every `SourceRoot` there can be at most one RATOML file. ratoml_files: FxHashMap, @@ -917,38 +914,44 @@ impl Config { 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) => { + if let Some(change) = change.workspace_ratoml_change { + tracing::info!("updating root ra-toml config"); + for (source_root_id, (_, text)) in change { + if let Some(text) = text { 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), - ConfigErrors( - toml_errors - .into_iter() - .map(|(a, b)| ConfigErrorInner::Toml { config_key: a, error: b }) - .map(Arc::new) - .collect(), - ), - )); - should_update = true; - } - Err(e) => { - config.root_ratoml = Some(( - GlobalLocalConfigInput::from_toml(toml::map::Map::default(), &mut vec![]), - ConfigErrors(vec![ConfigErrorInner::ParseError { - reason: e.message().to_owned(), + match toml::from_str(&text) { + Ok(table) => { + validate_toml_table( + GlobalLocalConfigInput::FIELDS, + &table, + &mut String::new(), + &mut toml_errors, + ); + config.workspace_ratoml_change.insert( + source_root_id, + ( + 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; } - .into()]), - )); + Err(e) => { + config.validation_errors.0.push( + ConfigErrorInner::ParseError { reason: e.message().to_owned() } + .into(), + ); + } + } } } } @@ -958,7 +961,6 @@ impl Config { 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( @@ -985,16 +987,10 @@ impl Config { ); } Err(e) => { - config.root_ratoml = Some(( - GlobalLocalConfigInput::from_toml( - toml::map::Map::default(), - &mut vec![], - ), - ConfigErrors(vec![ConfigErrorInner::ParseError { - reason: e.message().to_owned(), - } - .into()]), - )); + config.validation_errors.0.push( + ConfigErrorInner::ParseError { reason: e.message().to_owned() } + .into(), + ); } } } @@ -1026,7 +1022,13 @@ impl Config { .1 .0 .iter() - .chain(config.root_ratoml.as_ref().into_iter().flat_map(|it| it.1 .0.iter())) + .chain( + config + .workspace_ratoml_change + .values() + .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())) .chain(config.validation_errors.0.iter()) @@ -1055,8 +1057,8 @@ impl Config { #[derive(Default, Debug)] pub struct ConfigChange { user_config_change: Option>, - root_ratoml_change: Option>, client_config_change: Option, + workspace_ratoml_change: Option>)>>, ratoml_file_change: Option>)>>, source_map_change: Option>>, } @@ -1078,9 +1080,15 @@ impl ConfigChange { self.user_config_change = content; } - pub fn change_root_ratoml(&mut self, content: Option>) { - assert!(self.root_ratoml_change.is_none()); // Otherwise it is a double write. - self.root_ratoml_change = content; + pub fn change_workspace_ratoml( + &mut self, + source_root: SourceRootId, + vfs_path: VfsPath, + content: Option>, + ) -> Option<(VfsPath, Option>)> { + self.workspace_ratoml_change + .get_or_insert_with(Default::default) + .insert(source_root, (vfs_path, content)) } pub fn change_client_config(&mut self, change: serde_json::Value) { @@ -1333,11 +1341,6 @@ impl Config { // 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: ClientCapabilities::new(caps), @@ -1352,10 +1355,9 @@ impl Config { source_root_parent_map: Arc::new(FxHashMap::default()), user_config: None, user_config_path, - root_ratoml: None, - root_ratoml_path, detached_files: Default::default(), validation_errors: Default::default(), + workspace_ratoml_change: Default::default(), } } @@ -1398,10 +1400,6 @@ impl Config { &self.root_path } - pub fn root_ratoml_path(&self) -> &VfsPath { - &self.root_ratoml_path - } - pub fn caps(&self) -> &ClientCapabilities { &self.caps } @@ -3591,7 +3589,7 @@ mod tests { let mut change = ConfigChange::default(); - change.change_root_ratoml(Some( + change.change_user_config(Some( toml::toml! { [cargo.cfgs] these = "these" diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/global_state.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/global_state.rs index 9fd9bee5377a..d78e76035824 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/global_state.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/global_state.rs @@ -10,6 +10,7 @@ use flycheck::{project_json, FlycheckHandle}; use hir::ChangeWithProcMacros; use ide::{Analysis, AnalysisHost, Cancellable, FileId, SourceRootId}; use ide_db::base_db::{CrateId, ProcMacroPaths, SourceDatabaseExt}; +use itertools::Itertools; use load_cargo::SourceRootConfig; use lsp_types::{SemanticTokens, Url}; use nohash_hasher::IntMap; @@ -22,7 +23,7 @@ use project_model::{ManifestPath, ProjectWorkspace, ProjectWorkspaceKind, Worksp use rustc_hash::{FxHashMap, FxHashSet}; use tracing::{span, trace, Level}; use triomphe::Arc; -use vfs::{AbsPathBuf, AnchoredPathBuf, ChangeKind, Vfs}; +use vfs::{AbsPathBuf, AnchoredPathBuf, ChangeKind, Vfs, VfsPath}; use crate::{ config::{Config, ConfigChange, ConfigErrors}, @@ -382,26 +383,37 @@ impl GlobalState { { 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(); + // FIXME @alibektas : This is silly. There is abs no reason to use VfsPaths when there is SourceRoots. But how + // do I resolve a "workspace_root" to its corresponding id without having to rely on a cargo.toml's ( or project json etc.) file id? + let workspace_roots = self + .workspaces + .iter() + .map(|ws| VfsPath::from(ws.workspace_root().to_owned())) + .collect_vec(); + 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))); + // If change has been made to a ratoml file that + // belongs to a non-local source root, we will ignore it. + let sr_id = db.file_source_root(file_id); + let sr = db.source_root(sr_id); + + if workspace_roots.contains(&vfs_path) { + change.change_workspace_ratoml( + sr_id, + vfs_path, + 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, @@ -430,7 +442,7 @@ impl GlobalState { if should_update { self.update_configuration(config); } else { - // No global or client level config was changed. So we can just naively replace config. + // No global or client level config was changed. So we can naively replace config. self.config = Arc::new(config); } } diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/reload.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/reload.rs index fb16f28a14bd..b2d360c62db8 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/reload.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/reload.rs @@ -541,7 +541,6 @@ impl GlobalState { watchers.extend( iter::once(self.config.user_config_path().as_path()) - .chain(iter::once(self.config.root_ratoml_path().as_path())) .chain(self.workspaces.iter().map(|ws| ws.manifest().map(ManifestPath::as_ref))) .flatten() .map(|glob_pattern| lsp_types::FileSystemWatcher { diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/tests/slow-tests/ratoml.rs b/src/tools/rust-analyzer/crates/rust-analyzer/tests/slow-tests/ratoml.rs index 3b05138e1872..298e10fb9e02 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/tests/slow-tests/ratoml.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/tests/slow-tests/ratoml.rs @@ -808,7 +808,7 @@ enum Value { /// Having a ratoml file at the root of a project enables /// configuring global level configurations as well. #[test] -#[ignore = "Root ratomls are not being looked for on startup. Fix this."] +// #[ignore = "Root ratomls are not being looked for on startup. Fix this."] fn ratoml_in_root_is_global() { let server = RatomlTest::new( vec![ From c9cb05c41285950e193a0b8fbd5598b89dae48fb Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Fri, 26 Jul 2024 03:04:29 +0200 Subject: [PATCH 2/7] Globals too are asking for sourcerootid --- .../crates/rust-analyzer/src/config.rs | 177 +++++++++--------- .../crates/rust-analyzer/src/reload.rs | 2 +- 2 files changed, 93 insertions(+), 86 deletions(-) diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs index 6724b6a7add3..36419d4a97b2 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs @@ -1001,7 +1001,7 @@ impl Config { config.source_root_parent_map = source_root_map; } - if config.check_command().is_empty() { + if config.check_command(None).is_empty() { config.validation_errors.0.push(Arc::new(ConfigErrorInner::Json { config_key: "/check/command".to_owned(), error: serde_json::Error::custom("expected a non-empty string"), @@ -1453,11 +1453,11 @@ impl Config { pub fn diagnostics(&self, source_root: Option) -> DiagnosticsConfig { DiagnosticsConfig { - enabled: *self.diagnostics_enable(), + enabled: *self.diagnostics_enable(source_root), proc_attr_macros_enabled: self.expand_proc_attr_macros(), proc_macros_enabled: *self.procMacro_enable(), - disable_experimental: !self.diagnostics_experimental_enable(), - disabled: self.diagnostics_disabled().clone(), + disable_experimental: !self.diagnostics_experimental_enable(source_root), + disabled: self.diagnostics_disabled(source_root).clone(), expr_fill_default: match self.assist_expressionFillDefault(source_root) { ExprFillDefaultDef::Todo => ExprFillDefaultMode::Todo, ExprFillDefaultDef::Default => ExprFillDefaultMode::Default, @@ -1467,7 +1467,7 @@ impl Config { prefer_no_std: self.imports_preferNoStd(source_root).to_owned(), prefer_prelude: self.imports_preferPrelude(source_root).to_owned(), prefer_absolute: self.imports_prefixExternPrelude(source_root).to_owned(), - style_lints: self.diagnostics_styleLints_enable().to_owned(), + style_lints: self.diagnostics_styleLints_enable(source_root).to_owned(), term_search_fuel: self.assist_termSearch_fuel(source_root).to_owned() as u64, term_search_borrowck: self.assist_termSearch_borrowcheck(source_root).to_owned(), } @@ -1660,11 +1660,11 @@ impl Config { } pub fn has_linked_projects(&self) -> bool { - !self.linkedProjects().is_empty() + !self.linkedProjects(None).is_empty() } pub fn linked_manifests(&self) -> impl Iterator + '_ { - self.linkedProjects().iter().filter_map(|it| match it { + self.linkedProjects(None).iter().filter_map(|it| match it { ManifestOrProjectJson::Manifest(p) => Some(&**p), // despite having a buildfile, using this variant as a manifest // will fail. @@ -1674,20 +1674,20 @@ impl Config { } pub fn has_linked_project_jsons(&self) -> bool { - self.linkedProjects() + self.linkedProjects(None) .iter() .any(|it| matches!(it, ManifestOrProjectJson::ProjectJson { .. })) } pub fn discover_workspace_config(&self) -> Option<&DiscoverWorkspaceConfig> { - self.workspace_discoverConfig().as_ref() + self.workspace_discoverConfig(None).as_ref() } pub fn linked_or_discovered_projects(&self) -> Vec { - match self.linkedProjects().as_slice() { + match self.linkedProjects(None).as_slice() { [] => { let exclude_dirs: Vec<_> = - self.files_excludeDirs().iter().map(|p| self.root_path.join(p)).collect(); + self.files_excludeDirs(None).iter().map(|p| self.root_path.join(p)).collect(); self.discovered_projects .iter() .filter(|project| { @@ -1722,48 +1722,48 @@ impl Config { } pub fn prefill_caches(&self) -> bool { - self.cachePriming_enable().to_owned() + self.cachePriming_enable(None).to_owned() } pub fn publish_diagnostics(&self) -> bool { - self.diagnostics_enable().to_owned() + self.diagnostics_enable(None).to_owned() } pub fn diagnostics_map(&self) -> DiagnosticsMapConfig { DiagnosticsMapConfig { - remap_prefix: self.diagnostics_remapPrefix().clone(), - warnings_as_info: self.diagnostics_warningsAsInfo().clone(), - warnings_as_hint: self.diagnostics_warningsAsHint().clone(), - check_ignore: self.check_ignore().clone(), + remap_prefix: self.diagnostics_remapPrefix(None).clone(), + warnings_as_info: self.diagnostics_warningsAsInfo(None).clone(), + warnings_as_hint: self.diagnostics_warningsAsHint(None).clone(), + check_ignore: self.check_ignore(None).clone(), } } pub fn extra_args(&self) -> &Vec { - self.cargo_extraArgs() + self.cargo_extraArgs(None) } pub fn extra_env(&self) -> &FxHashMap { - self.cargo_extraEnv() + self.cargo_extraEnv(None) } pub fn check_extra_args(&self) -> Vec { let mut extra_args = self.extra_args().clone(); - extra_args.extend_from_slice(self.check_extraArgs()); + extra_args.extend_from_slice(self.check_extraArgs(None)); extra_args } pub fn check_extra_env(&self) -> FxHashMap { - let mut extra_env = self.cargo_extraEnv().clone(); - extra_env.extend(self.check_extraEnv().clone()); + let mut extra_env = self.cargo_extraEnv(None).clone(); + extra_env.extend(self.check_extraEnv(None).clone()); extra_env } pub fn lru_parse_query_capacity(&self) -> Option { - self.lru_capacity().to_owned() + self.lru_capacity(None).to_owned() } pub fn lru_query_capacities_config(&self) -> Option<&FxHashMap, u16>> { - self.lru_query_capacities().is_empty().not().then(|| self.lru_query_capacities()) + self.lru_query_capacities(None).is_empty().not().then(|| self.lru_query_capacities(None)) } pub fn proc_macro_srv(&self) -> Option { @@ -1772,7 +1772,7 @@ impl Config { } pub fn ignored_proc_macros(&self) -> &FxHashMap, Box<[Box]>> { - self.procMacro_ignored() + self.procMacro_ignored(None) } pub fn expand_proc_macros(&self) -> bool { @@ -1787,7 +1787,11 @@ impl Config { } _ => FilesWatcher::Server, }, - exclude: self.files_excludeDirs().iter().map(|it| self.root_path.join(it)).collect(), + exclude: self + .files_excludeDirs(None) + .iter() + .map(|it| self.root_path.join(it)) + .collect(), } } @@ -1798,22 +1802,22 @@ impl Config { } pub fn cargo_autoreload_config(&self) -> bool { - self.cargo_autoreload().to_owned() + self.cargo_autoreload(None).to_owned() } pub fn run_build_scripts(&self) -> bool { - self.cargo_buildScripts_enable().to_owned() || self.procMacro_enable().to_owned() + self.cargo_buildScripts_enable(None).to_owned() || self.procMacro_enable().to_owned() } pub fn cargo(&self) -> CargoConfig { - let rustc_source = self.rustc_source().as_ref().map(|rustc_src| { + let rustc_source = self.rustc_source(None).as_ref().map(|rustc_src| { if rustc_src == "discover" { RustLibSource::Discover } else { RustLibSource::Path(self.root_path.join(rustc_src)) } }); - let sysroot = self.cargo_sysroot().as_ref().map(|sysroot| { + let sysroot = self.cargo_sysroot(None).as_ref().map(|sysroot| { if sysroot == "discover" { RustLibSource::Discover } else { @@ -1821,26 +1825,26 @@ impl Config { } }); let sysroot_src = - self.cargo_sysrootSrc().as_ref().map(|sysroot| self.root_path.join(sysroot)); - let sysroot_query_metadata = self.cargo_sysrootQueryMetadata(); + self.cargo_sysrootSrc(None).as_ref().map(|sysroot| self.root_path.join(sysroot)); + let sysroot_query_metadata = self.cargo_sysrootQueryMetadata(None); CargoConfig { - all_targets: *self.cargo_allTargets(), - features: match &self.cargo_features() { + all_targets: *self.cargo_allTargets(None), + features: match &self.cargo_features(None) { CargoFeaturesDef::All => CargoFeatures::All, CargoFeaturesDef::Selected(features) => CargoFeatures::Selected { features: features.clone(), - no_default_features: self.cargo_noDefaultFeatures().to_owned(), + no_default_features: self.cargo_noDefaultFeatures(None).to_owned(), }, }, - target: self.cargo_target().clone(), + target: self.cargo_target(None).clone(), sysroot, sysroot_query_metadata: *sysroot_query_metadata, sysroot_src, rustc_source, cfg_overrides: project_model::CfgOverrides { global: CfgDiff::new( - self.cargo_cfgs() + self.cargo_cfgs(None) .iter() .map(|(key, val)| match val { Some(val) => CfgAtom::KeyValue { @@ -1855,49 +1859,49 @@ impl Config { .unwrap(), selective: Default::default(), }, - wrap_rustc_in_build_scripts: *self.cargo_buildScripts_useRustcWrapper(), - invocation_strategy: match self.cargo_buildScripts_invocationStrategy() { + wrap_rustc_in_build_scripts: *self.cargo_buildScripts_useRustcWrapper(None), + invocation_strategy: match self.cargo_buildScripts_invocationStrategy(None) { InvocationStrategy::Once => project_model::InvocationStrategy::Once, InvocationStrategy::PerWorkspace => project_model::InvocationStrategy::PerWorkspace, }, - invocation_location: match self.cargo_buildScripts_invocationLocation() { + invocation_location: match self.cargo_buildScripts_invocationLocation(None) { InvocationLocation::Root => { project_model::InvocationLocation::Root(self.root_path.clone()) } InvocationLocation::Workspace => project_model::InvocationLocation::Workspace, }, - run_build_script_command: self.cargo_buildScripts_overrideCommand().clone(), - extra_args: self.cargo_extraArgs().clone(), - extra_env: self.cargo_extraEnv().clone(), + run_build_script_command: self.cargo_buildScripts_overrideCommand(None).clone(), + extra_args: self.cargo_extraArgs(None).clone(), + extra_env: self.cargo_extraEnv(None).clone(), target_dir: self.target_dir_from_config(), } } pub fn rustfmt(&self) -> RustfmtConfig { - match &self.rustfmt_overrideCommand() { + match &self.rustfmt_overrideCommand(None) { Some(args) if !args.is_empty() => { let mut args = args.clone(); let command = args.remove(0); RustfmtConfig::CustomCommand { command, args } } Some(_) | None => RustfmtConfig::Rustfmt { - extra_args: self.rustfmt_extraArgs().clone(), - enable_range_formatting: *self.rustfmt_rangeFormatting_enable(), + extra_args: self.rustfmt_extraArgs(None).clone(), + enable_range_formatting: *self.rustfmt_rangeFormatting_enable(None), }, } } pub fn flycheck_workspace(&self) -> bool { - *self.check_workspace() + *self.check_workspace(None) } pub fn cargo_test_options(&self) -> CargoOptions { CargoOptions { - target_triples: self.cargo_target().clone().into_iter().collect(), + target_triples: self.cargo_target(None).clone().into_iter().collect(), all_targets: false, - no_default_features: *self.cargo_noDefaultFeatures(), - all_features: matches!(self.cargo_features(), CargoFeaturesDef::All), - features: match self.cargo_features().clone() { + no_default_features: *self.cargo_noDefaultFeatures(None), + all_features: matches!(self.cargo_features(None), CargoFeaturesDef::All), + features: match self.cargo_features(None).clone() { CargoFeaturesDef::All => vec![], CargoFeaturesDef::Selected(it) => it, }, @@ -1908,7 +1912,7 @@ impl Config { } pub fn flycheck(&self) -> FlycheckConfig { - match &self.check_overrideCommand() { + match &self.check_overrideCommand(None) { Some(args) if !args.is_empty() => { let mut args = args.clone(); let command = args.remove(0); @@ -1916,13 +1920,13 @@ impl Config { command, args, extra_env: self.check_extra_env(), - invocation_strategy: match self.check_invocationStrategy() { + invocation_strategy: match self.check_invocationStrategy(None) { InvocationStrategy::Once => flycheck::InvocationStrategy::Once, InvocationStrategy::PerWorkspace => { flycheck::InvocationStrategy::PerWorkspace } }, - invocation_location: match self.check_invocationLocation() { + invocation_location: match self.check_invocationLocation(None) { InvocationLocation::Root => { flycheck::InvocationLocation::Root(self.root_path.clone()) } @@ -1931,28 +1935,30 @@ impl Config { } } Some(_) | None => FlycheckConfig::CargoCommand { - command: self.check_command().clone(), + command: self.check_command(None).clone(), options: CargoOptions { target_triples: self - .check_targets() + .check_targets(None) .clone() .and_then(|targets| match &targets.0[..] { [] => None, targets => Some(targets.into()), }) - .unwrap_or_else(|| self.cargo_target().clone().into_iter().collect()), - all_targets: self.check_allTargets().unwrap_or(*self.cargo_allTargets()), + .unwrap_or_else(|| self.cargo_target(None).clone().into_iter().collect()), + all_targets: self + .check_allTargets(None) + .unwrap_or(*self.cargo_allTargets(None)), no_default_features: self - .check_noDefaultFeatures() - .unwrap_or(*self.cargo_noDefaultFeatures()), + .check_noDefaultFeatures(None) + .unwrap_or(*self.cargo_noDefaultFeatures(None)), all_features: matches!( - self.check_features().as_ref().unwrap_or(self.cargo_features()), + self.check_features(None).as_ref().unwrap_or(self.cargo_features(None)), CargoFeaturesDef::All ), features: match self - .check_features() + .check_features(None) .clone() - .unwrap_or_else(|| self.cargo_features().clone()) + .unwrap_or_else(|| self.cargo_features(None).clone()) { CargoFeaturesDef::All => vec![], CargoFeaturesDef::Selected(it) => it, @@ -1967,7 +1973,7 @@ impl Config { } fn target_dir_from_config(&self) -> Option { - self.cargo_targetDir().as_ref().and_then(|target_dir| match target_dir { + self.cargo_targetDir(None).as_ref().and_then(|target_dir| match target_dir { TargetDirectory::UseSubdirectory(true) => { Some(Utf8PathBuf::from("target/rust-analyzer")) } @@ -1978,18 +1984,18 @@ impl Config { } pub fn check_on_save(&self) -> bool { - *self.checkOnSave() + *self.checkOnSave(None) } pub fn script_rebuild_on_save(&self) -> bool { - *self.cargo_buildScripts_rebuildOnSave() + *self.cargo_buildScripts_rebuildOnSave(None) } pub fn runnables(&self) -> RunnablesConfig { RunnablesConfig { - override_cargo: self.runnables_command().clone(), - cargo_extra_args: self.runnables_extraArgs().clone(), - extra_test_binary_args: self.runnables_extraTestBinaryArgs().clone(), + override_cargo: self.runnables_command(None).clone(), + cargo_extra_args: self.runnables_extraArgs(None).clone(), + extra_test_binary_args: self.runnables_extraTestBinaryArgs(None).clone(), } } @@ -2060,7 +2066,7 @@ impl Config { } pub fn prime_caches_num_threads(&self) -> usize { - match self.cachePriming_numThreads() { + match self.cachePriming_numThreads(None) { NumThreads::Concrete(0) | NumThreads::Physical => num_cpus::get_physical(), &NumThreads::Concrete(n) => n, NumThreads::Logical => num_cpus::get(), @@ -2544,11 +2550,12 @@ macro_rules! _impl_for_config_data { } } - if let Some((root_path_ratoml, _)) = self.root_ratoml.as_ref() { - if let Some(v) = root_path_ratoml.local.$field.as_ref() { - return &v; - } - } + // TODO + // 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.0.local.$field.as_ref() { return &v; @@ -2574,13 +2581,13 @@ macro_rules! _impl_for_config_data { $( $($doc)* #[allow(non_snake_case)] - $vis fn $field(&self) -> &$ty { - - if let Some((root_path_ratoml, _)) = self.root_ratoml.as_ref() { - if let Some(v) = root_path_ratoml.global.$field.as_ref() { - return &v; - } - } + $vis fn $field(&self, source_root : Option) -> &$ty { + // TODO + // 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.0.global.$field.as_ref() { return &v; @@ -3522,7 +3529,7 @@ mod tests { })); (config, _, _) = config.apply_change(change); - assert_eq!(config.cargo_targetDir(), &None); + assert_eq!(config.cargo_targetDir(None), &None); assert!( matches!(config.flycheck(), FlycheckConfig::CargoCommand { options, .. } if options.target_dir.is_none()) ); @@ -3545,7 +3552,7 @@ mod tests { (config, _, _) = config.apply_change(change); - assert_eq!(config.cargo_targetDir(), &Some(TargetDirectory::UseSubdirectory(true))); + assert_eq!(config.cargo_targetDir(None), &Some(TargetDirectory::UseSubdirectory(true))); assert!( matches!(config.flycheck(), FlycheckConfig::CargoCommand { options, .. } if options.target_dir == Some(Utf8PathBuf::from("target/rust-analyzer"))) ); @@ -3569,7 +3576,7 @@ mod tests { (config, _, _) = config.apply_change(change); assert_eq!( - config.cargo_targetDir(), + config.cargo_targetDir(None), &Some(TargetDirectory::Directory(Utf8PathBuf::from("other_folder"))) ); assert!( diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/reload.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/reload.rs index b2d360c62db8..2d358afab39e 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/reload.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/reload.rs @@ -110,7 +110,7 @@ impl GlobalState { }; let mut message = String::new(); - if !self.config.cargo_autoreload() + if !self.config.cargo_autoreload(None) && self.is_quiescent() && self.fetch_workspaces_queue.op_requested() && self.config.discover_workspace_config().is_none() From 6c98665d91fb85fe78752f24d733c7d3671c60ea Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Sat, 27 Jul 2024 23:52:40 +0200 Subject: [PATCH 3/7] Down to 1 failing test And that is due to a case where we have two ratomls in a source root, one of which is a `workspace_ratoml` and the other one is simple old ratoml. Since we are not checking to see if the source root is already populated with workspace ratoml, this test fails. Due to principles of clear code I believe it is reasonable to not have two HashMaps that are almost for the exact same thing. So next commit should remove `workspace_ratoml` and merge it with `krate_ratoml`s. --- .../crates/rust-analyzer/src/capabilities.rs | 2 +- .../crates/rust-analyzer/src/config.rs | 78 +++++++++++-------- .../crates/rust-analyzer/src/global_state.rs | 15 +++- .../rust-analyzer/src/handlers/request.rs | 5 +- .../rust-analyzer/tests/slow-tests/ratoml.rs | 46 ++--------- 5 files changed, 66 insertions(+), 80 deletions(-) diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/capabilities.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/capabilities.rs index 212294b5d326..9610808c27e1 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/capabilities.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/capabilities.rs @@ -67,7 +67,7 @@ pub fn server_capabilities(config: &Config) -> ServerCapabilities { code_action_provider: Some(config.caps().code_action_capabilities()), code_lens_provider: Some(CodeLensOptions { resolve_provider: Some(true) }), document_formatting_provider: Some(OneOf::Left(true)), - document_range_formatting_provider: match config.rustfmt() { + document_range_formatting_provider: match config.rustfmt(None) { RustfmtConfig::Rustfmt { enable_range_formatting: true, .. } => Some(OneOf::Left(true)), _ => Some(OneOf::Left(false)), }, diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs index 36419d4a97b2..d667ed94976f 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs @@ -780,10 +780,10 @@ pub struct Config { user_config: Option<(GlobalLocalConfigInput, ConfigErrors)>, /// TODO : This file can be used to make global changes while having only a workspace-wide scope. - workspace_ratoml_change: FxHashMap, + workspace_ratoml: FxHashMap, /// For every `SourceRoot` there can be at most one RATOML file. - ratoml_files: FxHashMap, + krate_ratoml: FxHashMap, /// Clone of the value that is stored inside a `GlobalState`. source_root_parent_map: Arc>, @@ -927,7 +927,7 @@ impl Config { &mut String::new(), &mut toml_errors, ); - config.workspace_ratoml_change.insert( + config.workspace_ratoml.insert( source_root_id, ( GlobalLocalConfigInput::from_toml(table, &mut toml_errors), @@ -969,7 +969,7 @@ impl Config { &mut String::new(), &mut toml_errors, ); - config.ratoml_files.insert( + config.krate_ratoml.insert( source_root_id, ( LocalConfigInput::from_toml(&table, &mut toml_errors), @@ -1022,15 +1022,9 @@ impl Config { .1 .0 .iter() - .chain( - config - .workspace_ratoml_change - .values() - .into_iter() - .flat_map(|it| it.1 .0.iter()), - ) + .chain(config.workspace_ratoml.values().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())) + .chain(config.krate_ratoml.values().flat_map(|it| it.1 .0.iter())) .chain(config.validation_errors.0.iter()) .cloned() .collect(), @@ -1070,6 +1064,7 @@ impl ConfigChange { vfs_path: VfsPath, content: Option>, ) -> Option<(VfsPath, Option>)> { + dbg!("change_ratoml", &vfs_path); self.ratoml_file_change .get_or_insert_with(Default::default) .insert(source_root, (vfs_path, content)) @@ -1086,6 +1081,7 @@ impl ConfigChange { vfs_path: VfsPath, content: Option>, ) -> Option<(VfsPath, Option>)> { + dbg!("change_workspace", &vfs_path); self.workspace_ratoml_change .get_or_insert_with(Default::default) .insert(source_root, (vfs_path, content)) @@ -1350,14 +1346,14 @@ impl Config { workspace_roots, visual_studio_code_version, client_config: (FullConfigInput::default(), ConfigErrors(vec![])), - ratoml_files: FxHashMap::default(), + krate_ratoml: FxHashMap::default(), default_config: DEFAULT_CONFIG_DATA.get_or_init(|| Box::leak(Box::default())), source_root_parent_map: Arc::new(FxHashMap::default()), user_config: None, user_config_path, detached_files: Default::default(), validation_errors: Default::default(), - workspace_ratoml_change: Default::default(), + workspace_ratoml: Default::default(), } } @@ -1877,7 +1873,7 @@ impl Config { } } - pub fn rustfmt(&self) -> RustfmtConfig { + pub fn rustfmt(&self, source_root_id: Option) -> RustfmtConfig { match &self.rustfmt_overrideCommand(None) { Some(args) if !args.is_empty() => { let mut args = args.clone(); @@ -1885,8 +1881,8 @@ impl Config { RustfmtConfig::CustomCommand { command, args } } Some(_) | None => RustfmtConfig::Rustfmt { - extra_args: self.rustfmt_extraArgs(None).clone(), - enable_range_formatting: *self.rustfmt_rangeFormatting_enable(None), + extra_args: self.rustfmt_extraArgs(source_root_id).clone(), + enable_range_formatting: *self.rustfmt_rangeFormatting_enable(source_root_id), }, } } @@ -2540,22 +2536,28 @@ macro_rules! _impl_for_config_data { $($doc)* #[allow(non_snake_case)] $vis fn $field(&self, source_root: Option) -> &$ty { - 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) { + let follow = if stringify!($field) == "assist_emitMustUse" { dbg!("YEY"); true } else {false}; + let mut current: Option = None; + let mut next: Option = source_root; + if follow { dbg!(&self.krate_ratoml);} + while let Some(source_root_id) = next { + current = next; + next = self.source_root_parent_map.get(&source_root_id).copied(); + if let Some((config, _)) = self.krate_ratoml.get(&source_root_id) { if let Some(value) = config.$field.as_ref() { return value; } } } - // TODO - // 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(current) = current { + if follow { dbg!(&self.workspace_ratoml);} + if let Some((root_path_ratoml, _)) = self.workspace_ratoml.get(¤t).as_ref() { + if let Some(v) = root_path_ratoml.local.$field.as_ref() { + return &v; + } + } + } if let Some(v) = self.client_config.0.local.$field.as_ref() { return &v; @@ -2582,12 +2584,22 @@ macro_rules! _impl_for_config_data { $($doc)* #[allow(non_snake_case)] $vis fn $field(&self, source_root : Option) -> &$ty { - // TODO - // if let Some((root_path_ratoml, _)) = self.root_ratoml.as_ref() { - // if let Some(v) = root_path_ratoml.global.$field.as_ref() { - // return &v; - // } - // } + let follow = if stringify!($field) == "rustfmt_extraArgs" { dbg!("YEY"); true } else {false}; + let mut current: Option = None; + let mut next: Option = source_root; + while let Some(source_root_id) = next { + current = next; + next = self.source_root_parent_map.get(&source_root_id).copied(); + } + + if let Some(current) = current { + if follow { dbg!(&self.workspace_ratoml);} + if let Some((root_path_ratoml, _)) = self.workspace_ratoml.get(¤t).as_ref() { + if let Some(v) = root_path_ratoml.global.$field.as_ref() { + return &v; + } + } + } if let Some(v) = self.client_config.0.global.$field.as_ref() { return &v; diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/global_state.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/global_state.rs index d78e76035824..5bfdb67af28a 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/global_state.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/global_state.rs @@ -386,12 +386,18 @@ impl GlobalState { let mut change = ConfigChange::default(); let db = self.analysis_host.raw_database(); - // FIXME @alibektas : This is silly. There is abs no reason to use VfsPaths when there is SourceRoots. But how + // FIXME @alibektas : This is silly. There is no reason to use VfsPaths when there is SourceRoots. But how // do I resolve a "workspace_root" to its corresponding id without having to rely on a cargo.toml's ( or project json etc.) file id? - let workspace_roots = self + let workspace_ratoml_paths = self .workspaces .iter() - .map(|ws| VfsPath::from(ws.workspace_root().to_owned())) + .map(|ws| { + VfsPath::from({ + let mut p = ws.workspace_root().to_owned(); + p.push("rust-analyzer.toml"); + p + }) + }) .collect_vec(); for (file_id, (_change_kind, vfs_path)) in modified_ratoml_files { @@ -405,7 +411,7 @@ impl GlobalState { let sr_id = db.file_source_root(file_id); let sr = db.source_root(sr_id); - if workspace_roots.contains(&vfs_path) { + if workspace_ratoml_paths.contains(&vfs_path) { change.change_workspace_ratoml( sr_id, vfs_path, @@ -422,6 +428,7 @@ impl GlobalState { ) { // SourceRoot has more than 1 RATOML files. In this case lexicographically smaller wins. if old_path < vfs_path { + dbg!("HARBIDEN"); 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(sr_id, old_path, old_text); diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/handlers/request.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/handlers/request.rs index 44ff273b5ae0..9680a5c82795 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/handlers/request.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/handlers/request.rs @@ -2090,8 +2090,9 @@ fn run_rustfmt( let edition = editions.iter().copied().max(); let line_index = snap.file_line_index(file_id)?; + let sr = snap.analysis.source_root_id(file_id)?; - let mut command = match snap.config.rustfmt() { + let mut command = match snap.config.rustfmt(Some(sr)) { RustfmtConfig::Rustfmt { extra_args, enable_range_formatting } => { // FIXME: Set RUSTUP_TOOLCHAIN let mut cmd = process::Command::new(toolchain::Tool::Rustfmt.path()); @@ -2280,7 +2281,7 @@ pub(crate) fn internal_testing_fetch_config( serde_json::to_value(match &*params.config { "local" => state.config.assist(source_root).assist_emit_must_use, "global" => matches!( - state.config.rustfmt(), + state.config.rustfmt(source_root), RustfmtConfig::Rustfmt { enable_range_formatting: true, .. } ), _ => return Err(anyhow::anyhow!("Unknown test config key: {}", params.config)), diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/tests/slow-tests/ratoml.rs b/src/tools/rust-analyzer/crates/rust-analyzer/tests/slow-tests/ratoml.rs index 298e10fb9e02..8c111723fb28 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/tests/slow-tests/ratoml.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/tests/slow-tests/ratoml.rs @@ -700,37 +700,6 @@ fn ratoml_multiple_ratoml_in_single_source_root() { ); assert!(server.query(QueryType::Local, 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::Local, 3)); } /// If a root is non-local, so we cannot find what its parent is @@ -808,7 +777,6 @@ enum Value { /// Having a ratoml file at the root of a project enables /// configuring global level configurations as well. #[test] -// #[ignore = "Root ratomls are not being looked for on startup. Fix this."] fn ratoml_in_root_is_global() { let server = RatomlTest::new( vec![ @@ -820,7 +788,7 @@ version = "0.1.0" edition = "2021" "#, r#" -//- /rust-analyzer.toml +//- /p1/rust-analyzer.toml rustfmt.rangeFormatting.enable = true "#, r#" @@ -829,7 +797,7 @@ fn main() { todo!() }"#, ], - vec![], + vec!["p1"], None, ); @@ -837,7 +805,6 @@ fn main() { } #[test] -#[ignore = "Root ratomls are not being looked for on startup. Fix this."] fn ratoml_root_is_updateable() { let mut server = RatomlTest::new( vec![ @@ -849,7 +816,7 @@ version = "0.1.0" edition = "2021" "#, r#" -//- /rust-analyzer.toml +//- /p1/rust-analyzer.toml rustfmt.rangeFormatting.enable = true "#, r#" @@ -858,7 +825,7 @@ fn main() { todo!() }"#, ], - vec![], + vec!["p1"], None, ); @@ -868,7 +835,6 @@ fn main() { } #[test] -#[ignore = "Root ratomls are not being looked for on startup. Fix this."] fn ratoml_root_is_deletable() { let mut server = RatomlTest::new( vec![ @@ -880,7 +846,7 @@ version = "0.1.0" edition = "2021" "#, r#" -//- /rust-analyzer.toml +//- /p1/rust-analyzer.toml rustfmt.rangeFormatting.enable = true "#, r#" @@ -889,7 +855,7 @@ fn main() { todo!() }"#, ], - vec![], + vec!["p1"], None, ); From adefc44f31658c886fc84f8ce08e65470bf31289 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Mon, 29 Jul 2024 02:50:53 +0200 Subject: [PATCH 4/7] Combine krate_ratoml and workspace_ratomls into one --- .../crates/rust-analyzer/src/config.rs | 279 +++++++++--------- .../crates/rust-analyzer/src/global_state.rs | 46 +-- 2 files changed, 163 insertions(+), 162 deletions(-) diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs index d667ed94976f..abb112da6052 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs @@ -751,6 +751,18 @@ config_data! { } } +#[derive(Debug)] +pub enum RatomlFileKind { + Workspace, + Crate, +} + +#[derive(Debug, Clone)] +enum RatomlFile { + Workspace(GlobalLocalConfigInput), + Crate(LocalConfigInput), +} + #[derive(Debug, Clone)] pub struct Config { discovered_projects: Vec, @@ -779,11 +791,7 @@ pub struct Config { /// Config node whose values apply to **every** Rust project. user_config: Option<(GlobalLocalConfigInput, ConfigErrors)>, - /// TODO : This file can be used to make global changes while having only a workspace-wide scope. - workspace_ratoml: FxHashMap, - - /// For every `SourceRoot` there can be at most one RATOML file. - krate_ratoml: FxHashMap, + ratoml_file: FxHashMap, /// Clone of the value that is stored inside a `GlobalState`. source_root_parent_map: Arc>, @@ -914,83 +922,95 @@ impl Config { should_update = true; } - if let Some(change) = change.workspace_ratoml_change { - tracing::info!("updating root ra-toml config"); - for (source_root_id, (_, text)) in change { - if let Some(text) = text { - let mut toml_errors = vec![]; - match toml::from_str(&text) { - Ok(table) => { - validate_toml_table( - GlobalLocalConfigInput::FIELDS, - &table, - &mut String::new(), - &mut toml_errors, - ); - config.workspace_ratoml.insert( - source_root_id, - ( - 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) => { - config.validation_errors.0.push( - ConfigErrorInner::ParseError { reason: e.message().to_owned() } - .into(), - ); + if let Some(change) = change.ratoml_file_change { + for (source_root_id, (kind, _, text)) in change { + match kind { + RatomlFileKind::Crate => { + if let Some(text) = text { + let mut toml_errors = vec![]; + tracing::info!("updating ra-toml config: {:#}", text); + match toml::from_str(&text) { + Ok(table) => { + validate_toml_table( + &[LocalConfigInput::FIELDS], + &table, + &mut String::new(), + &mut toml_errors, + ); + config.ratoml_file.insert( + source_root_id, + ( + RatomlFile::Crate(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) => { + config.validation_errors.0.push( + ConfigErrorInner::ParseError { + reason: e.message().to_owned(), + } + .into(), + ); + } + } } } - } - } - } - - 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); - match toml::from_str(&text) { - Ok(table) => { - validate_toml_table( - &[LocalConfigInput::FIELDS], - &table, - &mut String::new(), - &mut toml_errors, - ); - config.krate_ratoml.insert( - source_root_id, - ( - 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) => { - config.validation_errors.0.push( - ConfigErrorInner::ParseError { reason: e.message().to_owned() } - .into(), - ); + RatomlFileKind::Workspace => { + if let Some(text) = text { + let mut toml_errors = vec![]; + match toml::from_str(&text) { + Ok(table) => { + validate_toml_table( + GlobalLocalConfigInput::FIELDS, + &table, + &mut String::new(), + &mut toml_errors, + ); + config.ratoml_file.insert( + source_root_id, + ( + RatomlFile::Workspace( + 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) => { + config.validation_errors.0.push( + ConfigErrorInner::ParseError { + reason: e.message().to_owned(), + } + .into(), + ); + } + } } } } @@ -1022,9 +1042,8 @@ impl Config { .1 .0 .iter() - .chain(config.workspace_ratoml.values().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.krate_ratoml.values().flat_map(|it| it.1 .0.iter())) + .chain(config.ratoml_file.values().flat_map(|it| it.1 .0.iter())) .chain(config.validation_errors.0.iter()) .cloned() .collect(), @@ -1052,8 +1071,8 @@ impl Config { pub struct ConfigChange { user_config_change: Option>, client_config_change: Option, - workspace_ratoml_change: Option>)>>, - ratoml_file_change: Option>)>>, + ratoml_file_change: + Option>)>>, source_map_change: Option>>, } @@ -1063,11 +1082,10 @@ impl ConfigChange { source_root: SourceRootId, vfs_path: VfsPath, content: Option>, - ) -> Option<(VfsPath, Option>)> { - dbg!("change_ratoml", &vfs_path); + ) -> Option<(RatomlFileKind, VfsPath, Option>)> { self.ratoml_file_change .get_or_insert_with(Default::default) - .insert(source_root, (vfs_path, content)) + .insert(source_root, (RatomlFileKind::Crate, vfs_path, content)) } pub fn change_user_config(&mut self, content: Option>) { @@ -1080,11 +1098,10 @@ impl ConfigChange { source_root: SourceRootId, vfs_path: VfsPath, content: Option>, - ) -> Option<(VfsPath, Option>)> { - dbg!("change_workspace", &vfs_path); - self.workspace_ratoml_change + ) -> Option<(RatomlFileKind, VfsPath, Option>)> { + self.ratoml_file_change .get_or_insert_with(Default::default) - .insert(source_root, (vfs_path, content)) + .insert(source_root, (RatomlFileKind::Workspace, vfs_path, content)) } pub fn change_client_config(&mut self, change: serde_json::Value) { @@ -1346,14 +1363,13 @@ impl Config { workspace_roots, visual_studio_code_version, client_config: (FullConfigInput::default(), ConfigErrors(vec![])), - krate_ratoml: FxHashMap::default(), default_config: DEFAULT_CONFIG_DATA.get_or_init(|| Box::leak(Box::default())), source_root_parent_map: Arc::new(FxHashMap::default()), user_config: None, user_config_path, detached_files: Default::default(), validation_errors: Default::default(), - workspace_ratoml: Default::default(), + ratoml_file: Default::default(), } } @@ -1874,7 +1890,7 @@ impl Config { } pub fn rustfmt(&self, source_root_id: Option) -> RustfmtConfig { - match &self.rustfmt_overrideCommand(None) { + match &self.rustfmt_overrideCommand(source_root_id) { Some(args) if !args.is_empty() => { let mut args = args.clone(); let command = args.remove(0); @@ -2536,27 +2552,23 @@ macro_rules! _impl_for_config_data { $($doc)* #[allow(non_snake_case)] $vis fn $field(&self, source_root: Option) -> &$ty { - let follow = if stringify!($field) == "assist_emitMustUse" { dbg!("YEY"); true } else {false}; - let mut current: Option = None; - let mut next: Option = source_root; - if follow { dbg!(&self.krate_ratoml);} - while let Some(source_root_id) = next { - current = next; - next = self.source_root_parent_map.get(&source_root_id).copied(); - if let Some((config, _)) = self.krate_ratoml.get(&source_root_id) { - if let Some(value) = config.$field.as_ref() { - return value; - } - } - } - - if let Some(current) = current { - if follow { dbg!(&self.workspace_ratoml);} - if let Some((root_path_ratoml, _)) = self.workspace_ratoml.get(¤t).as_ref() { - if let Some(v) = root_path_ratoml.local.$field.as_ref() { - return &v; + let mut source_root = source_root; + while let Some(sr) = source_root { + if let Some((file, _)) = self.ratoml_file.get(&sr) { + match file { + RatomlFile::Workspace(config) => { + if let Some(v) = config.local.$field.as_ref() { + return &v; + } + }, + RatomlFile::Crate(config) => { + if let Some(value) = config.$field.as_ref() { + return value; + } + } } } + source_root = self.source_root_parent_map.get(&sr).copied(); } if let Some(v) = self.client_config.0.local.$field.as_ref() { @@ -2584,21 +2596,20 @@ macro_rules! _impl_for_config_data { $($doc)* #[allow(non_snake_case)] $vis fn $field(&self, source_root : Option) -> &$ty { - let follow = if stringify!($field) == "rustfmt_extraArgs" { dbg!("YEY"); true } else {false}; - let mut current: Option = None; - let mut next: Option = source_root; - while let Some(source_root_id) = next { - current = next; - next = self.source_root_parent_map.get(&source_root_id).copied(); - } - - if let Some(current) = current { - if follow { dbg!(&self.workspace_ratoml);} - if let Some((root_path_ratoml, _)) = self.workspace_ratoml.get(¤t).as_ref() { - if let Some(v) = root_path_ratoml.global.$field.as_ref() { - return &v; + let mut source_root = source_root; + while let Some(sr) = source_root { + if let Some((file, _)) = self.ratoml_file.get(&sr) { + match file { + RatomlFile::Workspace(config) => { + if let Some(v) = config.global.$field.as_ref() { + return &v; + } + }, + _ => () } } + + source_root = self.source_root_parent_map.get(&sr).copied(); } if let Some(v) = self.client_config.0.global.$field.as_ref() { @@ -3667,21 +3678,7 @@ mod tests { let (_, 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); diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/global_state.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/global_state.rs index 5bfdb67af28a..f1dde104fce4 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/global_state.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/global_state.rs @@ -26,13 +26,10 @@ use triomphe::Arc; use vfs::{AbsPathBuf, AnchoredPathBuf, ChangeKind, Vfs, VfsPath}; use crate::{ - config::{Config, ConfigChange, ConfigErrors}, + config::{Config, ConfigChange, ConfigErrors, RatomlFileKind}, diagnostics::{CheckFixes, DiagnosticCollection}, line_index::{LineEndings, LineIndex}, - lsp::{ - from_proto::{self}, - to_proto::url_from_abs_path, - }, + lsp::{from_proto, to_proto::url_from_abs_path}, lsp_ext, main_loop::Task, mem_docs::MemDocs, @@ -411,27 +408,34 @@ impl GlobalState { let sr_id = db.file_source_root(file_id); let sr = db.source_root(sr_id); - if workspace_ratoml_paths.contains(&vfs_path) { - change.change_workspace_ratoml( - sr_id, - vfs_path, - Some(db.file_text(file_id)), - ); - continue; - } - if !sr.is_library { - if let Some((old_path, old_text)) = change.change_ratoml( - sr_id, - vfs_path.clone(), - Some(db.file_text(file_id)), - ) { + let entry = if workspace_ratoml_paths.contains(&vfs_path) { + change.change_workspace_ratoml( + sr_id, + vfs_path.clone(), + Some(db.file_text(file_id)), + ) + } else { + change.change_ratoml( + sr_id, + vfs_path.clone(), + Some(db.file_text(file_id)), + ) + }; + + if let Some((kind, old_path, old_text)) = entry { // SourceRoot has more than 1 RATOML files. In this case lexicographically smaller wins. if old_path < vfs_path { - dbg!("HARBIDEN"); 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(sr_id, old_path, old_text); + match kind { + RatomlFileKind::Crate => { + change.change_ratoml(sr_id, old_path, old_text); + } + RatomlFileKind::Workspace => { + change.change_workspace_ratoml(sr_id, old_path, old_text); + } + } } } } else { From 59c465b6e244e7f878d9c0d3a8785d9403818dcc Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Mon, 29 Jul 2024 03:43:27 +0200 Subject: [PATCH 5/7] add skip_slow_tests to ratoml tests --- .../rust-analyzer/tests/slow-tests/ratoml.rs | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/tests/slow-tests/ratoml.rs b/src/tools/rust-analyzer/crates/rust-analyzer/tests/slow-tests/ratoml.rs index 8c111723fb28..c06ba9eee147 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/tests/slow-tests/ratoml.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/tests/slow-tests/ratoml.rs @@ -10,6 +10,7 @@ use paths::Utf8PathBuf; use rust_analyzer::lsp::ext::{InternalTestingFetchConfig, InternalTestingFetchConfigParams}; use serde_json::json; +use test_utils::skip_slow_tests; enum QueryType { Local, @@ -182,6 +183,10 @@ impl RatomlTest { /// the client config. #[test] fn ratoml_client_config_basic() { + if skip_slow_tests() { + return; + } + let server = RatomlTest::new( vec![ r#" @@ -283,6 +288,10 @@ enum Value { #[test] #[ignore = "the user config is currently not being watched on startup, fix this"] fn ratoml_user_config_detected() { + if skip_slow_tests() { + return; + } + let server = RatomlTest::new( vec![ r#" @@ -312,6 +321,10 @@ enum Value { #[test] #[ignore = "the user config is currently not being watched on startup, fix this"] fn ratoml_create_user_config() { + if skip_slow_tests() { + return; + } + let mut server = RatomlTest::new( vec![ r#" @@ -343,6 +356,10 @@ enum Value { #[test] #[ignore = "the user config is currently not being watched on startup, fix this"] fn ratoml_modify_user_config() { + if skip_slow_tests() { + return; + } + let mut server = RatomlTest::new( vec![ r#" @@ -373,6 +390,10 @@ assist.emitMustUse = true"#, #[test] #[ignore = "the user config is currently not being watched on startup, fix this"] fn ratoml_delete_user_config() { + if skip_slow_tests() { + return; + } + let mut server = RatomlTest::new( vec![ r#" @@ -402,6 +423,10 @@ assist.emitMustUse = true"#, #[test] fn ratoml_inherit_config_from_ws_root() { + if skip_slow_tests() { + return; + } + let server = RatomlTest::new( vec![ r#" @@ -445,6 +470,10 @@ pub fn add(left: usize, right: usize) -> usize { #[test] fn ratoml_modify_ratoml_at_ws_root() { + if skip_slow_tests() { + return; + } + let mut server = RatomlTest::new( vec![ r#" @@ -490,6 +519,10 @@ pub fn add(left: usize, right: usize) -> usize { #[test] fn ratoml_delete_ratoml_at_ws_root() { + if skip_slow_tests() { + return; + } + let mut server = RatomlTest::new( vec![ r#" @@ -535,6 +568,10 @@ pub fn add(left: usize, right: usize) -> usize { #[test] fn ratoml_add_immediate_child_to_ws_root() { + if skip_slow_tests() { + return; + } + let mut server = RatomlTest::new( vec![ r#" @@ -581,6 +618,10 @@ pub fn add(left: usize, right: usize) -> usize { #[test] #[ignore = "Root ratomls are not being looked for on startup. Fix this."] fn ratoml_rm_ws_root_ratoml_child_has_client_as_parent_now() { + if skip_slow_tests() { + return; + } + let mut server = RatomlTest::new( vec![ r#" @@ -626,6 +667,10 @@ pub fn add(left: usize, right: usize) -> usize { #[test] fn ratoml_crates_both_roots() { + if skip_slow_tests() { + return; + } + let server = RatomlTest::new( vec![ r#" @@ -670,6 +715,10 @@ enum Value { #[test] fn ratoml_multiple_ratoml_in_single_source_root() { + if skip_slow_tests() { + return; + } + let server = RatomlTest::new( vec![ r#" @@ -778,6 +827,10 @@ fn ratoml_multiple_ratoml_in_single_source_root() { /// configuring global level configurations as well. #[test] fn ratoml_in_root_is_global() { + if skip_slow_tests() { + return; + } + let server = RatomlTest::new( vec![ r#" @@ -806,6 +859,10 @@ fn main() { #[test] fn ratoml_root_is_updateable() { + if skip_slow_tests() { + return; + } + let mut server = RatomlTest::new( vec![ r#" @@ -836,6 +893,10 @@ fn main() { #[test] fn ratoml_root_is_deletable() { + if skip_slow_tests() { + return; + } + let mut server = RatomlTest::new( vec![ r#" From 45ef4f2c32cb9a192b4cabfee8701eec42b64f20 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Mon, 29 Jul 2024 03:46:02 +0200 Subject: [PATCH 6/7] Remove unnec copying of source_root_ids --- .../rust-analyzer/crates/rust-analyzer/src/config.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs index abb112da6052..996f48482aa1 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs @@ -2552,7 +2552,7 @@ macro_rules! _impl_for_config_data { $($doc)* #[allow(non_snake_case)] $vis fn $field(&self, source_root: Option) -> &$ty { - let mut source_root = source_root; + let mut source_root = source_root.as_ref(); while let Some(sr) = source_root { if let Some((file, _)) = self.ratoml_file.get(&sr) { match file { @@ -2568,7 +2568,7 @@ macro_rules! _impl_for_config_data { } } } - source_root = self.source_root_parent_map.get(&sr).copied(); + source_root = self.source_root_parent_map.get(&sr); } if let Some(v) = self.client_config.0.local.$field.as_ref() { @@ -2596,7 +2596,7 @@ macro_rules! _impl_for_config_data { $($doc)* #[allow(non_snake_case)] $vis fn $field(&self, source_root : Option) -> &$ty { - let mut source_root = source_root; + let mut source_root = source_root.as_ref(); while let Some(sr) = source_root { if let Some((file, _)) = self.ratoml_file.get(&sr) { match file { @@ -2609,7 +2609,7 @@ macro_rules! _impl_for_config_data { } } - source_root = self.source_root_parent_map.get(&sr).copied(); + source_root = self.source_root_parent_map.get(&sr); } if let Some(v) = self.client_config.0.global.$field.as_ref() { From dc471c16b2a2d6ae68cb7668228d95a17afa6cbd Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Mon, 29 Jul 2024 17:00:31 +0200 Subject: [PATCH 7/7] Remove clippy errors --- .../crates/rust-analyzer/src/config.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs index 996f48482aa1..8743c4faff69 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs @@ -758,6 +758,8 @@ pub enum RatomlFileKind { } #[derive(Debug, Clone)] +// FIXME @alibektas : Seems like a clippy warning of this sort should tell that combining different ConfigInputs into one enum was not a good idea. +#[allow(clippy::large_enum_variant)] enum RatomlFile { Workspace(GlobalLocalConfigInput), Crate(LocalConfigInput), @@ -2598,14 +2600,9 @@ macro_rules! _impl_for_config_data { $vis fn $field(&self, source_root : Option) -> &$ty { let mut source_root = source_root.as_ref(); while let Some(sr) = source_root { - if let Some((file, _)) = self.ratoml_file.get(&sr) { - match file { - RatomlFile::Workspace(config) => { - if let Some(v) = config.global.$field.as_ref() { - return &v; - } - }, - _ => () + if let Some((RatomlFile::Workspace(config), _)) = self.ratoml_file.get(&sr) { + if let Some(v) = config.global.$field.as_ref() { + return &v; } }