Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
- Fixed issue where statements under top-level in-eachs were not correctly tracked.
- Moved storage of reference->symbol mapping to on-demand timing, should significantly speed
up device analysises
- Unknown fields in the lint configuration file are now detected and reported as errors, helping users identify and correct typos or unsupported configuration options.

## 0.9.12
- Added 'simics\_util\_vect' as a known provisional (with no DLS semantics)
Expand Down
24 changes: 13 additions & 11 deletions src/actions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,17 +342,14 @@ impl <O: Output> InitActionContext<O> {
pid: u32,
_client_supports_cmd_run: bool,
) -> InitActionContext<O> {
let lint_config = Arc::new(Mutex::new(
config.lock().unwrap().lint_cfg_path.clone()
.and_then(maybe_parse_lint_cfg)
.unwrap_or_default()));

InitActionContext {
vfs,
analysis,
analysis_queue: Arc::new(AnalysisQueue::init()),
current_notifier: Arc::default(),
config,
lint_config,
lint_config: Arc::new(Mutex::new(LintCfg::default())),
jobs: Arc::default(),
direct_opens: Arc::default(),
quiescent: Arc::new(AtomicBool::new(false)),
Expand Down Expand Up @@ -388,7 +385,8 @@ impl <O: Output> InitActionContext<O> {
fn init(&mut self,
_init_options: InitializationOptions,
out: O) {
self.update_compilation_info(&out)
self.update_compilation_info(&out);
self.update_linter_config(&out);
}

pub fn update_workspaces(&self,
Expand All @@ -401,13 +399,17 @@ impl <O: Output> InitActionContext<O> {
}
}

fn update_linter_config(&self, _out: &O) {
fn update_linter_config(&self, out: &O) {
trace!("Updating linter config");
if let Ok(config) = self.config.lock() {
*self.lint_config.lock().unwrap() =
config.lint_cfg_path.clone()
.and_then(maybe_parse_lint_cfg)
.unwrap_or_default();
if let Some(ref lint_path) = config.lint_cfg_path {
if let Some(cfg) = maybe_parse_lint_cfg(lint_path.clone(), out) {
*self.lint_config.lock().unwrap() = cfg;
}
} else {
// If no lint config path is set, use default
*self.lint_config.lock().unwrap() = LintCfg::default();
}
}
}

Expand Down
1 change: 1 addition & 0 deletions src/actions/notifications.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ impl BlockingNotificationAction for DidChangeWatchedFiles {
if let Some(file_watch) = FileWatch::new(ctx) {
if params.changes.iter().any(|c| file_watch.is_relevant(c)) {
ctx.update_compilation_info(&out);
ctx.update_linter_config(&out);
Copy link
Contributor

Choose a reason for hiding this comment

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

@JonatanWaern We were considering adding linting config file to FileWatch, but I found that an effort was being made in this PR: #122. Are you planning on working on this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, eventually I will take a second(third?) look at making file watching work properly. ATM it is entirely non-functional and my previous attempts didn't result in a clear cause. It is not a high priority atm, though.

}
}
Ok(())
Expand Down
81 changes: 72 additions & 9 deletions src/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,27 @@ use crate::lint::rules::indentation::{MAX_LENGTH_DEFAULT,
INDENTATION_LEVEL_DEFAULT,
setup_indentation_size
};
use crate::server::{maybe_notify_unknown_lint_fields, Output};

pub fn parse_lint_cfg(path: PathBuf) -> Result<LintCfg, String> {
pub fn parse_lint_cfg(path: PathBuf) -> Result<(LintCfg, Vec<String>), String> {
debug!("Reading Lint configuration from {:?}", path);
let file_content = fs::read_to_string(path).map_err(
|e|e.to_string())?;
let file_content = fs::read_to_string(path).map_err(|e| e.to_string())?;
trace!("Content is {:?}", file_content);
serde_json::from_str(&file_content)
.map_err(|e|e.to_string())

let val: serde_json::Value = serde_json::from_str(&file_content)
.map_err(|e| e.to_string())?;

let mut unknowns = Vec::new();
let cfg = LintCfg::try_deserialize(&val, &mut unknowns)?;

Ok((cfg, unknowns))
}

pub fn maybe_parse_lint_cfg(path: PathBuf) -> Option<LintCfg> {
pub fn maybe_parse_lint_cfg<O: Output>(path: PathBuf, out: &O) -> Option<LintCfg> {
match parse_lint_cfg(path) {
Ok(mut cfg) => {
Ok((mut cfg, unknowns)) => {
// Send visible warning to client
maybe_notify_unknown_lint_fields(out, &unknowns);
setup_indentation_size(&mut cfg);
Some(cfg)
},
Expand All @@ -45,9 +53,10 @@ pub fn maybe_parse_lint_cfg(path: PathBuf) -> Option<LintCfg> {
}
}



#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
#[serde(default)]
#[serde(deny_unknown_fields)]
pub struct LintCfg {
#[serde(default)]
pub sp_brace: Option<SpBraceOptions>,
Expand Down Expand Up @@ -81,6 +90,21 @@ pub struct LintCfg {
pub annotate_lints: bool,
}

impl LintCfg {
pub fn try_deserialize(
val: &serde_json::Value,
unknowns: &mut Vec<String>,
) -> Result<LintCfg, String> {
// Use serde_ignored to automatically track unknown fields
match serde_ignored::deserialize(val, |json_field| {
unknowns.push(json_field.to_string());
}) {
Ok(cfg) => Ok(cfg),
Err(e) => Err(e.to_string()),
}
}
}

fn get_true() -> bool {
true
}
Expand Down Expand Up @@ -421,8 +445,47 @@ pub mod tests {
let example_path = format!("{}{}",
env!("CARGO_MANIFEST_DIR"),
EXAMPLE_CFG);
let example_cfg = parse_lint_cfg(example_path.into()).unwrap();
let (example_cfg, unknowns) = parse_lint_cfg(example_path.into()).unwrap();
assert_eq!(example_cfg, LintCfg::default());
// Assert that there are no unknown fields in the example config:
assert!(unknowns.is_empty(), "Example config should not have unknown fields: {:?}", unknowns);
}

#[test]
fn test_unknown_fields_detection() {
use crate::lint::LintCfg;

// JSON with unknown fields
let json_with_unknowns = r#"{
"sp_brace": {},
"unknown_field_1": true,
"indent_size": {"indentation_spaces": 4},
"another_unknown": "value"
}"#;

let val: serde_json::Value = serde_json::from_str(json_with_unknowns).unwrap();
let mut unknowns = Vec::new();
let result = LintCfg::try_deserialize(&val, &mut unknowns);

assert!(result.is_ok());
let cfg = result.unwrap();

// Assert that unknown fields were detected
assert_eq!(unknowns.len(), 2);
assert!(unknowns.contains(&"unknown_field_1".to_string()));
assert!(unknowns.contains(&"another_unknown".to_string()));

// Assert the final LintCfg matches expected json (the known fields)
let expected_json = r#"{
"sp_brace": {},
"indent_size": {"indentation_spaces": 4}
}"#;
let expected_val: serde_json::Value = serde_json::from_str(expected_json).unwrap();
let mut expected_unknowns = Vec::new();
let expected_cfg = LintCfg::try_deserialize(&expected_val, &mut expected_unknowns).unwrap();

assert_eq!(cfg, expected_cfg);
assert!(expected_unknowns.is_empty()); // No unknown fields in the expected config
}

#[test]
Expand Down
16 changes: 16 additions & 0 deletions src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,22 @@ pub(crate) fn maybe_notify_deprecated_configs<O: Output>(out: &O, keys: &[String
}
}

pub(crate) fn maybe_notify_unknown_lint_fields<O: Output>(out: &O, unknowns: &[String]) {
if !unknowns.is_empty() {
let fields_list = unknowns.join(", ");
let message = format!(
"Unknown lint configuration field{}: {}. These will be ignored.",
if unknowns.len() > 1 { "s" } else { "" },
fields_list
);

out.notify(Notification::<ShowMessage>::new(ShowMessageParams {
typ: MessageType::ERROR,
message,
}));
}
}

pub(crate) fn maybe_notify_duplicated_configs<O: Output>(
out: &O,
dups: &std::collections::HashMap<String, Vec<String>>,
Expand Down