Skip to content

minor : fixes for ratoml module #17483

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 23, 2024
Merged

minor : fixes for ratoml module #17483

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
97bad35
Minor fixes for ratoml module
alibektas Jun 21, 2024
4f8735f
Apply changes to ratoml/fixes
alibektas Jul 21, 2024
db2ba64
Add FIXME to root ratoml tests.
alibektas Jul 21, 2024
784d46e
rename config::ConfigChange::other_errors to validation_errors
alibektas Jul 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 73 additions & 47 deletions crates/rust-analyzer/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -782,7 +782,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)>,

Expand All @@ -798,6 +797,13 @@ 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
/// FIXME : bad name I know...
other_errors: ConfigErrors,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validation_errors?


detached_files: Vec<AbsPathBuf>,
}

Expand Down Expand Up @@ -827,6 +833,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.other_errors = ConfigErrors::default();

let mut should_update = false;

Expand Down Expand Up @@ -855,6 +862,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::<Vec<Utf8PathBuf>>(
Expand All @@ -870,6 +878,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(
Expand Down Expand Up @@ -909,8 +948,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()]),
));
}
}
}

Expand Down Expand Up @@ -945,8 +991,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()]),
));
}
}
}
}
Expand All @@ -956,48 +1012,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.other_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)
}

Expand All @@ -1015,6 +1036,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.other_errors.0.iter())
.cloned()
.collect(),
);
Expand Down Expand Up @@ -1262,9 +1284,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 {
Expand All @@ -1286,6 +1309,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)
}
Expand Down Expand Up @@ -1339,6 +1363,7 @@ impl Config {
root_ratoml: None,
root_ratoml_path,
detached_files: Default::default(),
other_errors: Default::default(),
}
}

Expand Down Expand Up @@ -2580,6 +2605,7 @@ macro_rules! _impl_for_config_data {
}
}


&self.default_config.global.$field
}
)*
Expand Down Expand Up @@ -3309,7 +3335,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
Expand Down
8 changes: 4 additions & 4 deletions crates/rust-analyzer/src/main_loop.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -937,7 +937,7 @@ impl GlobalState {
diag.fix,
),
Err(err) => {
tracing::error!(
error!(
"flycheck {id}: File with cargo diagnostic not found in VFS: {}",
err
);
Expand Down
Loading