user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Auto merge of #17483 - alibektas:ratoml/fixes, r=alibektas

Browse source 
minor : fixes for ratoml module

This is a follow-up PR to #17058.

- Parse errors are reflected as such by defining a new variant called `ConfigError::ParseError`
- New error collection has been added to store config level agnostic errors.

EDIT : Some things that this PR promised to solve are removed and will be addressed by other PRs
This commit is contained in:
bors 2024-07-23 14:13:45 +00:00
commit c611e440d4
3 changed files with 92 additions and 115 deletions
Download patch file Download diff file Expand all files Collapse all files

119
src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs
View file

@ -779,7 +779,6 @@ pub struct Config {
/// | 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<(GlobalLocalConfigInput, ConfigErrors)>,
@ -795,6 +794,12 @@ pub struct Config {
/// Clone of the value that is stored inside a `GlobalState`.
source_root_parent_map: Arc<FxHashMap<SourceRootId, SourceRootId>>,
/// Use case : It is an error to have an empty value for `check_command`.
/// Since it is a `global` command at the moment, its final value can only be determined by
/// traversing through `global` configs and the `client` config. However the non-null value constraint
/// is config level agnostic, so this requires an independent error storage
validation_errors: ConfigErrors,
detached_files: Vec<AbsPathBuf>,
}
@ -824,6 +829,7 @@ impl Config {
/// The return tuple's bool component signals whether the `GlobalState` should call its `update_configuration()` method.
fn apply_change_with_sink(&self, change: ConfigChange) -> (Config, bool) {
let mut config = self.clone();
config.validation_errors = ConfigErrors::default();
let mut should_update = false;
@ -852,6 +858,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_json::<Vec<Utf8PathBuf>>(
@ -867,6 +874,37 @@ impl Config {
patch_old_style::patch_json_for_outdated_configs(&mut json);
// IMPORTANT : This holds as long as ` completion_snippets_custom` is declared `client`.
config.snippets.clear();
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 => json_errors.push((
name.to_owned(),
<serde_json::Error as serde::de::Error>::custom(format!(
"snippet {name} is invalid or triggers are missing",
)),
)),
}
}
config.client_config = (
FullConfigInput::from_json(json, &mut json_errors),
ConfigErrors(
@ -906,8 +944,15 @@ impl Config {
));
should_update = true;
}
// FIXME
Err(_) => (),
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()]),
));
}
}
}
@ -942,8 +987,18 @@ impl Config {
),
);
}
// FIXME
Err(_) => (),
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()]),
));
}
}
}
}
@ -953,48 +1008,13 @@ impl Config {
config.source_root_parent_map = source_root_map;
}
// IMPORTANT : This holds as long as ` completion_snippets_custom` is declared `client`.
config.snippets.clear();
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,
};
#[allow(clippy::single_match)]
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),
// FIXME
// None => error_sink.0.push(ConfigErrorInner::Json {
// config_key: "".to_owned(),
// error: <serde_json::Error as serde::de::Error>::custom(format!(
// "snippet {name} is invalid or triggers are missing",
// )),
// }),
None => (),
}
if config.check_command().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"),
}));
}
// 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)
}
@ -1012,6 +1032,7 @@ impl Config {
.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()))
.chain(config.validation_errors.0.iter())
.cloned()
.collect(),
);
@ -1259,9 +1280,10 @@ pub struct ClientCommandsConfig {
pub enum ConfigErrorInner {
Json { config_key: String, error: serde_json::Error },
Toml { config_key: String, error: toml::de::Error },
ParseError { reason: String },
}
#[derive(Clone, Debug)]
#[derive(Clone, Debug, Default)]
pub struct ConfigErrors(Vec<Arc<ConfigErrorInner>>);
impl ConfigErrors {
@ -1283,6 +1305,7 @@ impl fmt::Display for ConfigErrors {
f(&": ")?;
f(e)
}
ConfigErrorInner::ParseError { reason } => f(reason),
});
write!(f, "invalid config value{}:\n{}", if self.0.len() == 1 { "" } else { "s" }, errors)
}
@ -1336,6 +1359,7 @@ impl Config {
root_ratoml: None,
root_ratoml_path,
detached_files: Default::default(),
validation_errors: Default::default(),
}
}
@ -2575,6 +2599,7 @@ macro_rules! _impl_for_config_data {
}
}
&self.default_config.global.$field
}
)*
@ -3304,7 +3329,7 @@ fn validate_toml_table(
ptr.push_str(k);
match v {
// This is a table config, any entry in it is therefor valid
// This is a table config, any entry in it is therefore 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

8
src/tools/rust-analyzer/crates/rust-analyzer/src/main_loop.rs
View file

@ -14,7 +14,7 @@ use ide_db::base_db::{SourceDatabase, SourceDatabaseExt, VfsPath};
use lsp_server::{Connection, Notification, Request};
use lsp_types::{notification::Notification as _, TextDocumentIdentifier};
use stdx::thread::ThreadIntent;
use tracing::{span, Level};
use tracing::{error, span, Level};
use vfs::{AbsPathBuf, FileId};
use crate::{
@ -674,7 +674,7 @@ impl GlobalState {
self.fetch_workspaces_queue
.op_completed(Some((workspaces, force_reload_crate_graph)));
if let Err(e) = self.fetch_workspace_error() {
tracing::error!("FetchWorkspaceError:\n{e}");
error!("FetchWorkspaceError:\n{e}");
}
self.switch_workspaces("fetched workspace".to_owned());
(Progress::End, None)
@ -719,7 +719,7 @@ impl GlobalState {
BuildDataProgress::End(build_data_result) => {
self.fetch_build_data_queue.op_completed(build_data_result);
if let Err(e) = self.fetch_build_data_error() {
tracing::error!("FetchBuildDataError:\n{e}");
error!("FetchBuildDataError:\n{e}");
}
self.switch_workspaces("fetched build data".to_owned());
@ -937,7 +937,7 @@ impl GlobalState {
diag.fix,
),
Err(err) => {
tracing::error!(
error!(
"flycheck {id}: File with cargo diagnostic not found in VFS: {}",
err
);

80
src/tools/rust-analyzer/crates/rust-analyzer/tests/slow-tests/ratoml.rs
View file

@ -30,8 +30,6 @@ impl RatomlTest {
const EMIT_MUST_USE: &'static str = r#"assist.emitMustUse = true"#;
const EMIT_MUST_NOT_USE: &'static str = r#"assist.emitMustUse = false"#;
const GLOBAL_TRAIT_ASSOC_ITEMS_ZERO: &'static str = r#"hover.show.traitAssocItems = 0"#;
fn new(
fixtures: Vec<&str>,
roots: Vec<&str>,
@ -180,25 +178,6 @@ impl RatomlTest {
}
}
// /// 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]
@ -420,15 +399,6 @@ assist.emitMustUse = true"#,
server.delete(2);
assert!(!server.query(QueryType::Local, 1));
}
// #[test]
// fn delete_user_config() {
// todo!()
// }
// #[test]
// fn modify_client_config() {
// todo!()
// }
#[test]
fn ratoml_inherit_config_from_ws_root() {
@ -609,6 +579,7 @@ 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() {
let mut server = RatomlTest::new(
vec![
@ -837,6 +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."]
fn ratoml_in_root_is_global() {
let server = RatomlTest::new(
vec![
@ -849,30 +821,23 @@ edition = "2021"
"#,
r#"
//- /rust-analyzer.toml
hover.show.traitAssocItems = 4
rustfmt.rangeFormatting.enable = true
"#,
r#"
//- /p1/src/lib.rs
trait RandomTrait {
type B;
fn abc() -> i32;
fn def() -> i64;
}
fn main() {
let a = RandomTrait;
todo!()
}"#,
],
vec![],
None,
);
server.query(QueryType::Global, 2);
assert!(server.query(QueryType::Global, 2));
}
#[allow(unused)]
// #[test]
// FIXME: Re-enable this test when we have a global config we can check again
#[test]
#[ignore = "Root ratomls are not being looked for on startup. Fix this."]
fn ratoml_root_is_updateable() {
let mut server = RatomlTest::new(
vec![
@ -885,18 +850,12 @@ edition = "2021"
"#,
r#"
//- /rust-analyzer.toml
hover.show.traitAssocItems = 4
"#,
rustfmt.rangeFormatting.enable = true
"#,
r#"
//- /p1/src/lib.rs
trait RandomTrait {
type B;
fn abc() -> i32;
fn def() -> i64;
}
fn main() {
let a = RandomTrait;
todo!()
}"#,
],
vec![],
@ -904,13 +863,12 @@ fn main() {
);
assert!(server.query(QueryType::Global, 2));
server.edit(1, RatomlTest::GLOBAL_TRAIT_ASSOC_ITEMS_ZERO.to_owned());
server.edit(1, "rustfmt.rangeFormatting.enable = false".to_owned());
assert!(!server.query(QueryType::Global, 2));
}
#[allow(unused)]
// #[test]
// FIXME: Re-enable this test when we have a global config we can check again
#[test]
#[ignore = "Root ratomls are not being looked for on startup. Fix this."]
fn ratoml_root_is_deletable() {
let mut server = RatomlTest::new(
vec![
@ -923,18 +881,12 @@ edition = "2021"
"#,
r#"
//- /rust-analyzer.toml
hover.show.traitAssocItems = 4
"#,
rustfmt.rangeFormatting.enable = true
"#,
r#"
//- /p1/src/lib.rs
trait RandomTrait {
type B;
fn abc() -> i32;
fn def() -> i64;
}
fn main() {
let a = RandomTrait;
todo!()
}"#,
],
vec![],