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.
This commit is contained in:
Ali Bektas 2024-07-27 23:52:40 +02:00
parent 1aece03d16
commit d7bedc85b3
5 changed files with 66 additions and 80 deletions

View file

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

View file

@ -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<SourceRootId, (GlobalLocalConfigInput, ConfigErrors)>,
workspace_ratoml: FxHashMap<SourceRootId, (GlobalLocalConfigInput, ConfigErrors)>,
/// For every `SourceRoot` there can be at most one RATOML file.
ratoml_files: FxHashMap<SourceRootId, (LocalConfigInput, ConfigErrors)>,
krate_ratoml: FxHashMap<SourceRootId, (LocalConfigInput, ConfigErrors)>,
/// Clone of the value that is stored inside a `GlobalState`.
source_root_parent_map: Arc<FxHashMap<SourceRootId, SourceRootId>>,
@ -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<Arc<str>>,
) -> Option<(VfsPath, Option<Arc<str>>)> {
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<Arc<str>>,
) -> Option<(VfsPath, Option<Arc<str>>)> {
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<SourceRootId>) -> 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<SourceRootId>) -> &$ty {
let mut par: Option<SourceRootId> = 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<SourceRootId> = None;
let mut next: Option<SourceRootId> = 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(&current).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<SourceRootId>) -> &$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<SourceRootId> = None;
let mut next: Option<SourceRootId> = 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(&current).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;

View file

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

View file

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

View file

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