Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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.
- CLI tool DFA now uses default one-indexed line count for reporting warnings on analyzed files.
`--zero-indexed` flag can be set to `true` when executing DFA for using zero-indexed counting if required.

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