Skip to content

Commit ad4e35a

Browse files
committed
Minor fixes for ratoml module
- 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.
1 parent 402e176 commit ad4e35a

File tree

3 files changed

+90
-115
lines changed

3 files changed

+90
-115
lines changed

src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs

Lines changed: 74 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -782,7 +782,6 @@ pub struct Config {
782782
/// | Windows | `{FOLDERID_RoamingAppData}` | C:\Users\Alice\AppData\Roaming |
783783
user_config_path: VfsPath,
784784

785-
/// FIXME @alibektas : Change this to sth better.
786785
/// Config node whose values apply to **every** Rust project.
787786
user_config: Option<(GlobalLocalConfigInput, ConfigErrors)>,
788787

@@ -798,6 +797,13 @@ pub struct Config {
798797
/// Clone of the value that is stored inside a `GlobalState`.
799798
source_root_parent_map: Arc<FxHashMap<SourceRootId, SourceRootId>>,
800799

800+
/// Use case : It is an error to have an empty value for `check_command`.
801+
/// Since it is a `global` command at the moment, its final value can only be determined by
802+
/// traversing through `global` configs and the `client` config. However the non-null value constraint
803+
/// is config level agnostic, so this requires an independent error storage
804+
/// FIXME : bad name I know...
805+
other_errors: ConfigErrors,
806+
801807
detached_files: Vec<AbsPathBuf>,
802808
}
803809

@@ -827,6 +833,7 @@ impl Config {
827833
/// The return tuple's bool component signals whether the `GlobalState` should call its `update_configuration()` method.
828834
fn apply_change_with_sink(&self, change: ConfigChange) -> (Config, bool) {
829835
let mut config = self.clone();
836+
config.other_errors = ConfigErrors::default();
830837

831838
let mut should_update = false;
832839

@@ -855,6 +862,7 @@ impl Config {
855862

856863
if let Some(mut json) = change.client_config_change {
857864
tracing::info!("updating config from JSON: {:#}", json);
865+
858866
if !(json.is_null() || json.as_object().map_or(false, |it| it.is_empty())) {
859867
let mut json_errors = vec![];
860868
let detached_files = get_field::<Vec<Utf8PathBuf>>(
@@ -870,6 +878,38 @@ impl Config {
870878

871879
patch_old_style::patch_json_for_outdated_configs(&mut json);
872880

881+
// IMPORTANT : This holds as long as ` completion_snippets_custom` is declared `client`.
882+
config.snippets.clear();
883+
884+
let snips = self.completion_snippets_custom().to_owned();
885+
886+
for (name, def) in snips.iter() {
887+
if def.prefix.is_empty() && def.postfix.is_empty() {
888+
continue;
889+
}
890+
let scope = match def.scope {
891+
SnippetScopeDef::Expr => SnippetScope::Expr,
892+
SnippetScopeDef::Type => SnippetScope::Type,
893+
SnippetScopeDef::Item => SnippetScope::Item,
894+
};
895+
#[allow(clippy::single_match)]
896+
match Snippet::new(
897+
&def.prefix,
898+
&def.postfix,
899+
&def.body,
900+
def.description.as_ref().unwrap_or(name),
901+
&def.requires,
902+
scope,
903+
) {
904+
Some(snippet) => config.snippets.push(snippet),
905+
None => json_errors.push((
906+
name.to_owned(),
907+
<serde_json::Error as serde::de::Error>::custom(format!(
908+
"snippet {name} is invalid or triggers are missing",
909+
)),
910+
)),
911+
}
912+
}
873913
config.client_config = (
874914
FullConfigInput::from_json(json, &mut json_errors),
875915
ConfigErrors(
@@ -909,8 +949,15 @@ impl Config {
909949
));
910950
should_update = true;
911951
}
912-
// FIXME
913-
Err(_) => (),
952+
Err(e) => {
953+
config.root_ratoml = Some((
954+
GlobalLocalConfigInput::from_toml(toml::map::Map::default(), &mut vec![]),
955+
ConfigErrors(vec![ConfigErrorInner::ParseError {
956+
reason: e.message().to_owned(),
957+
}
958+
.into()]),
959+
));
960+
}
914961
}
915962
}
916963

@@ -945,8 +992,18 @@ impl Config {
945992
),
946993
);
947994
}
948-
// FIXME
949-
Err(_) => (),
995+
Err(e) => {
996+
config.root_ratoml = Some((
997+
GlobalLocalConfigInput::from_toml(
998+
toml::map::Map::default(),
999+
&mut vec![],
1000+
),
1001+
ConfigErrors(vec![ConfigErrorInner::ParseError {
1002+
reason: e.message().to_owned(),
1003+
}
1004+
.into()]),
1005+
));
1006+
}
9501007
}
9511008
}
9521009
}
@@ -956,48 +1013,13 @@ impl Config {
9561013
config.source_root_parent_map = source_root_map;
9571014
}
9581015

959-
// IMPORTANT : This holds as long as ` completion_snippets_custom` is declared `client`.
960-
config.snippets.clear();
961-
962-
let snips = self.completion_snippets_custom().to_owned();
963-
964-
for (name, def) in snips.iter() {
965-
if def.prefix.is_empty() && def.postfix.is_empty() {
966-
continue;
967-
}
968-
let scope = match def.scope {
969-
SnippetScopeDef::Expr => SnippetScope::Expr,
970-
SnippetScopeDef::Type => SnippetScope::Type,
971-
SnippetScopeDef::Item => SnippetScope::Item,
972-
};
973-
#[allow(clippy::single_match)]
974-
match Snippet::new(
975-
&def.prefix,
976-
&def.postfix,
977-
&def.body,
978-
def.description.as_ref().unwrap_or(name),
979-
&def.requires,
980-
scope,
981-
) {
982-
Some(snippet) => config.snippets.push(snippet),
983-
// FIXME
984-
// None => error_sink.0.push(ConfigErrorInner::Json {
985-
// config_key: "".to_owned(),
986-
// error: <serde_json::Error as serde::de::Error>::custom(format!(
987-
// "snippet {name} is invalid or triggers are missing",
988-
// )),
989-
// }),
990-
None => (),
991-
}
1016+
if config.check_command().is_empty() {
1017+
config.other_errors.0.push(Arc::new(ConfigErrorInner::Json {
1018+
config_key: "/check/command".to_owned(),
1019+
error: serde_json::Error::custom("expected a non-empty string"),
1020+
}));
9921021
}
9931022

994-
// FIXME: bring this back
995-
// if config.check_command().is_empty() {
996-
// error_sink.0.push(ConfigErrorInner::Json {
997-
// config_key: "/check/command".to_owned(),
998-
// error: serde_json::Error::custom("expected a non-empty string"),
999-
// });
1000-
// }
10011023
(config, should_update)
10021024
}
10031025

@@ -1015,6 +1037,7 @@ impl Config {
10151037
.chain(config.root_ratoml.as_ref().into_iter().flat_map(|it| it.1 .0.iter()))
10161038
.chain(config.user_config.as_ref().into_iter().flat_map(|it| it.1 .0.iter()))
10171039
.chain(config.ratoml_files.values().flat_map(|it| it.1 .0.iter()))
1040+
.chain(config.other_errors.0.iter())
10181041
.cloned()
10191042
.collect(),
10201043
);
@@ -1262,9 +1285,10 @@ pub struct ClientCommandsConfig {
12621285
pub enum ConfigErrorInner {
12631286
Json { config_key: String, error: serde_json::Error },
12641287
Toml { config_key: String, error: toml::de::Error },
1288+
ParseError { reason: String },
12651289
}
12661290

1267-
#[derive(Clone, Debug)]
1291+
#[derive(Clone, Debug, Default)]
12681292
pub struct ConfigErrors(Vec<Arc<ConfigErrorInner>>);
12691293

12701294
impl ConfigErrors {
@@ -1286,6 +1310,7 @@ impl fmt::Display for ConfigErrors {
12861310
f(&": ")?;
12871311
f(e)
12881312
}
1313+
ConfigErrorInner::ParseError { reason } => f(reason),
12891314
});
12901315
write!(f, "invalid config value{}:\n{}", if self.0.len() == 1 { "" } else { "s" }, errors)
12911316
}
@@ -1339,6 +1364,7 @@ impl Config {
13391364
root_ratoml: None,
13401365
root_ratoml_path,
13411366
detached_files: Default::default(),
1367+
other_errors: Default::default(),
13421368
}
13431369
}
13441370

@@ -2580,6 +2606,7 @@ macro_rules! _impl_for_config_data {
25802606
}
25812607
}
25822608

2609+
25832610
&self.default_config.global.$field
25842611
}
25852612
)*
@@ -3309,7 +3336,7 @@ fn validate_toml_table(
33093336
ptr.push_str(k);
33103337

33113338
match v {
3312-
// This is a table config, any entry in it is therefor valid
3339+
// This is a table config, any entry in it is therefore valid
33133340
toml::Value::Table(_) if verify(ptr) => (),
33143341
toml::Value::Table(table) => validate_toml_table(known_ptrs, table, ptr, error_sink),
33153342
_ if !verify(ptr) => error_sink

src/tools/rust-analyzer/crates/rust-analyzer/src/main_loop.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use ide_db::base_db::{SourceDatabase, SourceDatabaseExt, VfsPath};
1414
use lsp_server::{Connection, Notification, Request};
1515
use lsp_types::{notification::Notification as _, TextDocumentIdentifier};
1616
use stdx::thread::ThreadIntent;
17-
use tracing::{span, Level};
17+
use tracing::{error, span, Level};
1818
use vfs::{AbsPathBuf, FileId};
1919

2020
use crate::{
@@ -674,7 +674,7 @@ impl GlobalState {
674674
self.fetch_workspaces_queue
675675
.op_completed(Some((workspaces, force_reload_crate_graph)));
676676
if let Err(e) = self.fetch_workspace_error() {
677-
tracing::error!("FetchWorkspaceError:\n{e}");
677+
error!("FetchWorkspaceError:\n{e}");
678678
}
679679
self.switch_workspaces("fetched workspace".to_owned());
680680
(Progress::End, None)
@@ -719,7 +719,7 @@ impl GlobalState {
719719
BuildDataProgress::End(build_data_result) => {
720720
self.fetch_build_data_queue.op_completed(build_data_result);
721721
if let Err(e) = self.fetch_build_data_error() {
722-
tracing::error!("FetchBuildDataError:\n{e}");
722+
error!("FetchBuildDataError:\n{e}");
723723
}
724724

725725
self.switch_workspaces("fetched build data".to_owned());
@@ -937,7 +937,7 @@ impl GlobalState {
937937
diag.fix,
938938
),
939939
Err(err) => {
940-
tracing::error!(
940+
error!(
941941
"flycheck {id}: File with cargo diagnostic not found in VFS: {}",
942942
err
943943
);

0 commit comments

Comments
 (0)