Skip to content

Commit a59dae7

Browse files
authored
Merge pull request #130 from me-cr/fix/config-file-unknown-fields
Capture and log unknown fields from config file
2 parents 12d3c11 + e220afa commit a59dae7

File tree

5 files changed

+103
-20
lines changed

5 files changed

+103
-20
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
- Fixed issue where statements under top-level in-eachs were not correctly tracked.
1717
- Moved storage of reference->symbol mapping to on-demand timing, should significantly speed
1818
up device analysises
19+
- Unknown fields in the lint configuration file are now detected and reported as errors, helping users identify and correct typos or unsupported configuration options.
1920
- CLI tool DFA now uses default one-indexed line count for reporting warnings on analyzed files.
2021
`--zero-indexed` flag can be set to `true` when executing DFA for using zero-indexed counting if required.
2122

src/actions/mod.rs

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -342,17 +342,14 @@ impl <O: Output> InitActionContext<O> {
342342
pid: u32,
343343
_client_supports_cmd_run: bool,
344344
) -> InitActionContext<O> {
345-
let lint_config = Arc::new(Mutex::new(
346-
config.lock().unwrap().lint_cfg_path.clone()
347-
.and_then(maybe_parse_lint_cfg)
348-
.unwrap_or_default()));
345+
349346
InitActionContext {
350347
vfs,
351348
analysis,
352349
analysis_queue: Arc::new(AnalysisQueue::init()),
353350
current_notifier: Arc::default(),
354351
config,
355-
lint_config,
352+
lint_config: Arc::new(Mutex::new(LintCfg::default())),
356353
jobs: Arc::default(),
357354
direct_opens: Arc::default(),
358355
quiescent: Arc::new(AtomicBool::new(false)),
@@ -388,7 +385,8 @@ impl <O: Output> InitActionContext<O> {
388385
fn init(&mut self,
389386
_init_options: InitializationOptions,
390387
out: O) {
391-
self.update_compilation_info(&out)
388+
self.update_compilation_info(&out);
389+
self.update_linter_config(&out);
392390
}
393391

394392
pub fn update_workspaces(&self,
@@ -401,13 +399,17 @@ impl <O: Output> InitActionContext<O> {
401399
}
402400
}
403401

404-
fn update_linter_config(&self, _out: &O) {
402+
fn update_linter_config(&self, out: &O) {
405403
trace!("Updating linter config");
406404
if let Ok(config) = self.config.lock() {
407-
*self.lint_config.lock().unwrap() =
408-
config.lint_cfg_path.clone()
409-
.and_then(maybe_parse_lint_cfg)
410-
.unwrap_or_default();
405+
if let Some(ref lint_path) = config.lint_cfg_path {
406+
if let Some(cfg) = maybe_parse_lint_cfg(lint_path.clone(), out) {
407+
*self.lint_config.lock().unwrap() = cfg;
408+
}
409+
} else {
410+
// If no lint config path is set, use default
411+
*self.lint_config.lock().unwrap() = LintCfg::default();
412+
}
411413
}
412414
}
413415

src/actions/notifications.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,7 @@ impl BlockingNotificationAction for DidChangeWatchedFiles {
249249
if let Some(file_watch) = FileWatch::new(ctx) {
250250
if params.changes.iter().any(|c| file_watch.is_relevant(c)) {
251251
ctx.update_compilation_info(&out);
252+
ctx.update_linter_config(&out);
252253
}
253254
}
254255
Ok(())

src/lint/mod.rs

Lines changed: 72 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,19 +22,27 @@ use crate::lint::rules::indentation::{MAX_LENGTH_DEFAULT,
2222
INDENTATION_LEVEL_DEFAULT,
2323
setup_indentation_size
2424
};
25+
use crate::server::{maybe_notify_unknown_lint_fields, Output};
2526

26-
pub fn parse_lint_cfg(path: PathBuf) -> Result<LintCfg, String> {
27+
pub fn parse_lint_cfg(path: PathBuf) -> Result<(LintCfg, Vec<String>), String> {
2728
debug!("Reading Lint configuration from {:?}", path);
28-
let file_content = fs::read_to_string(path).map_err(
29-
|e|e.to_string())?;
29+
let file_content = fs::read_to_string(path).map_err(|e| e.to_string())?;
3030
trace!("Content is {:?}", file_content);
31-
serde_json::from_str(&file_content)
32-
.map_err(|e|e.to_string())
31+
32+
let val: serde_json::Value = serde_json::from_str(&file_content)
33+
.map_err(|e| e.to_string())?;
34+
35+
let mut unknowns = Vec::new();
36+
let cfg = LintCfg::try_deserialize(&val, &mut unknowns)?;
37+
38+
Ok((cfg, unknowns))
3339
}
3440

35-
pub fn maybe_parse_lint_cfg(path: PathBuf) -> Option<LintCfg> {
41+
pub fn maybe_parse_lint_cfg<O: Output>(path: PathBuf, out: &O) -> Option<LintCfg> {
3642
match parse_lint_cfg(path) {
37-
Ok(mut cfg) => {
43+
Ok((mut cfg, unknowns)) => {
44+
// Send visible warning to client
45+
maybe_notify_unknown_lint_fields(out, &unknowns);
3846
setup_indentation_size(&mut cfg);
3947
Some(cfg)
4048
},
@@ -45,9 +53,10 @@ pub fn maybe_parse_lint_cfg(path: PathBuf) -> Option<LintCfg> {
4553
}
4654
}
4755

56+
57+
4858
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
4959
#[serde(default)]
50-
#[serde(deny_unknown_fields)]
5160
pub struct LintCfg {
5261
#[serde(default)]
5362
pub sp_brace: Option<SpBraceOptions>,
@@ -81,6 +90,21 @@ pub struct LintCfg {
8190
pub annotate_lints: bool,
8291
}
8392

93+
impl LintCfg {
94+
pub fn try_deserialize(
95+
val: &serde_json::Value,
96+
unknowns: &mut Vec<String>,
97+
) -> Result<LintCfg, String> {
98+
// Use serde_ignored to automatically track unknown fields
99+
match serde_ignored::deserialize(val, |json_field| {
100+
unknowns.push(json_field.to_string());
101+
}) {
102+
Ok(cfg) => Ok(cfg),
103+
Err(e) => Err(e.to_string()),
104+
}
105+
}
106+
}
107+
84108
fn get_true() -> bool {
85109
true
86110
}
@@ -421,8 +445,47 @@ pub mod tests {
421445
let example_path = format!("{}{}",
422446
env!("CARGO_MANIFEST_DIR"),
423447
EXAMPLE_CFG);
424-
let example_cfg = parse_lint_cfg(example_path.into()).unwrap();
448+
let (example_cfg, unknowns) = parse_lint_cfg(example_path.into()).unwrap();
425449
assert_eq!(example_cfg, LintCfg::default());
450+
// Assert that there are no unknown fields in the example config:
451+
assert!(unknowns.is_empty(), "Example config should not have unknown fields: {:?}", unknowns);
452+
}
453+
454+
#[test]
455+
fn test_unknown_fields_detection() {
456+
use crate::lint::LintCfg;
457+
458+
// JSON with unknown fields
459+
let json_with_unknowns = r#"{
460+
"sp_brace": {},
461+
"unknown_field_1": true,
462+
"indent_size": {"indentation_spaces": 4},
463+
"another_unknown": "value"
464+
}"#;
465+
466+
let val: serde_json::Value = serde_json::from_str(json_with_unknowns).unwrap();
467+
let mut unknowns = Vec::new();
468+
let result = LintCfg::try_deserialize(&val, &mut unknowns);
469+
470+
assert!(result.is_ok());
471+
let cfg = result.unwrap();
472+
473+
// Assert that unknown fields were detected
474+
assert_eq!(unknowns.len(), 2);
475+
assert!(unknowns.contains(&"unknown_field_1".to_string()));
476+
assert!(unknowns.contains(&"another_unknown".to_string()));
477+
478+
// Assert the final LintCfg matches expected json (the known fields)
479+
let expected_json = r#"{
480+
"sp_brace": {},
481+
"indent_size": {"indentation_spaces": 4}
482+
}"#;
483+
let expected_val: serde_json::Value = serde_json::from_str(expected_json).unwrap();
484+
let mut expected_unknowns = Vec::new();
485+
let expected_cfg = LintCfg::try_deserialize(&expected_val, &mut expected_unknowns).unwrap();
486+
487+
assert_eq!(cfg, expected_cfg);
488+
assert!(expected_unknowns.is_empty()); // No unknown fields in the expected config
426489
}
427490

428491
#[test]

src/server/mod.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,22 @@ pub(crate) fn maybe_notify_deprecated_configs<O: Output>(out: &O, keys: &[String
164164
}
165165
}
166166

167+
pub(crate) fn maybe_notify_unknown_lint_fields<O: Output>(out: &O, unknowns: &[String]) {
168+
if !unknowns.is_empty() {
169+
let fields_list = unknowns.join(", ");
170+
let message = format!(
171+
"Unknown lint configuration field{}: {}. These will be ignored.",
172+
if unknowns.len() > 1 { "s" } else { "" },
173+
fields_list
174+
);
175+
176+
out.notify(Notification::<ShowMessage>::new(ShowMessageParams {
177+
typ: MessageType::ERROR,
178+
message,
179+
}));
180+
}
181+
}
182+
167183
pub(crate) fn maybe_notify_duplicated_configs<O: Output>(
168184
out: &O,
169185
dups: &std::collections::HashMap<String, Vec<String>>,

0 commit comments

Comments
 (0)