From 6b96cb93890545f63f17ed86520d6585b2549a06 Mon Sep 17 00:00:00 2001 From: Roberto Huertas Date: Sun, 28 Jul 2024 02:25:58 +0200 Subject: [PATCH 01/23] feat(config): preserve comments on modification and better formattting --- Cargo.toml | 3 +- crates/bins/Cargo.toml | 5 +- .../comment_preserver/mod.rs | 3 + .../comment_preserver/models.rs | 40 ++++ .../comment_preserver/reconciler.rs | 209 ++++++++++++++++++ .../ide/configuration_file/error.rs | 8 + .../ide/configuration_file/mod.rs | 1 + .../static_analysis_config_file.rs | 40 +++- crates/static-analysis-kernel/Cargo.toml | 2 +- 9 files changed, 296 insertions(+), 15 deletions(-) create mode 100644 crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/mod.rs create mode 100644 crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/models.rs create mode 100644 crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs diff --git a/Cargo.toml b/Cargo.toml index fae319ab..e9d2965b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,7 +5,7 @@ default-members = [ "crates/cli", "crates/static-analysis-kernel", "crates/static-analysis-server", - "crates/secrets" + "crates/secrets", ] resolver = "2" @@ -39,3 +39,4 @@ sha2 = "0.10.7" num_cpus = "1.15.0" tracing = "0.1.40" uuid = { version = "1.6.1", features = ["v4"] } +tree-sitter = "0.22.6" diff --git a/crates/bins/Cargo.toml b/crates/bins/Cargo.toml index 496e4d30..2f4d7e5a 100644 --- a/crates/bins/Cargo.toml +++ b/crates/bins/Cargo.toml @@ -48,9 +48,11 @@ serde_json = { workspace = true } tracing = { workspace = true } uuid = { workspace = true } indexmap = { workspace = true } -num_cpus = { workspace = true} +num_cpus = { workspace = true } serde = { workspace = true } serde_yaml = { workspace = true } +tree-sitter = { workspace = true } + # other terminal-emoji = "0.4.1" getopts = "0.2.21" @@ -59,6 +61,7 @@ rayon = "1.7.0" rocket = { version = "=0.5.0", features = ["json"] } tracing-subscriber = { version = "0.3.18", features = ["fmt", "env-filter"] } thiserror = "1" +pretty_yaml = "0.4" # For linux and macos, we need the vendored ssl diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/mod.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/mod.rs new file mode 100644 index 00000000..1de2ab01 --- /dev/null +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/mod.rs @@ -0,0 +1,3 @@ +mod models; +mod reconciler; +pub use reconciler::{reconcile_comments, ReconcileError}; diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/models.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/models.rs new file mode 100644 index 00000000..7e1d1401 --- /dev/null +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/models.rs @@ -0,0 +1,40 @@ +#[derive(Debug, Clone, Default)] +pub struct Line { + pub row: usize, + pub content: String, +} + +impl Line { + pub fn new(row: usize, content: String) -> Self { + Self { row, content } + } +} + +#[derive(Debug, Clone)] +pub enum Comment { + Inline { + line: Line, + original_content: String, + }, + Block { + line: Line, + above_line: Option, + below_line: Option, + }, +} + +impl Comment { + pub const fn get_line(&self) -> &Line { + match self { + Self::Inline { + line, + original_content: _, + } + | Self::Block { + line, + above_line: _, + below_line: _, + } => line, + } + } +} diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs new file mode 100644 index 00000000..428edeb1 --- /dev/null +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs @@ -0,0 +1,209 @@ +use super::models::{Comment, Line}; +use kernel::analysis::tree_sitter::get_tree; +use kernel::model::common::Language; +use thiserror::Error; +use tree_sitter::Node; + +#[derive(Debug, Error)] +#[error("Reconciler error")] +pub struct ReconcileError { + #[from] + pub source: anyhow::Error, +} + +pub fn reconcile_comments( + original_content: &str, + new_content: &str, +) -> Result { + // parse the original content and look for comments + let tree = get_tree(original_content, &Language::Yaml).ok_or_else(|| { + anyhow::anyhow!("Failed to parse the original content with the tree-sitter parser") + })?; + + let root_node = tree.root_node(); + + let mut comments = vec![]; + extract_comments_from_node(root_node, original_content, &mut comments); + + let reconciled_content = reconcile(new_content, &comments); + + // make it pretty + let options = pretty_yaml::config::FormatOptions::default(); + let formatted = pretty_yaml::format_text(&reconciled_content, &options) + .map_err(|e| anyhow::anyhow!("Failed to format the reconciled content: {}", e))?; + + Ok(formatted) +} + +fn extract_comments_from_node(node: Node<'_>, source: &str, comments: &mut Vec) { + if node.kind() == "comment" { + let start_byte = node.start_byte(); + let end_byte = node.end_byte(); + let comment = &source[start_byte..end_byte]; + let row = node.start_position().row; + + let prev = node.prev_sibling(); + let final_comment = prev + .and_then(|p| { + if p.start_position().row == row { + Some(p) + } else { + None + } + }) + .map_or_else( + || Comment::Block { + line: Line::new(row, comment.to_string()), + above_line: prev.map(|prev| { + let content = get_line_content(source, prev.end_byte()); + Line::new(row, content.to_string()) + }), + below_line: node.next_sibling().map(|next| { + let content = get_line_content(source, next.start_byte()); + Line::new(next.start_position().row, content.to_string()) + }), + }, + |original| Comment::Inline { + line: Line::new(row, comment.to_string()), + original_content: source[original.start_byte()..start_byte - 1].to_string(), + }, + ); + comments.push(final_comment); + } + + for child in node.children(&mut node.walk()) { + extract_comments_from_node(child, source, comments); + } +} + +fn reconcile(modified: &str, comments: &[Comment]) -> String { + let mut lines: Vec = modified.lines().map(ToString::to_string).collect(); + let lines_len = lines.len(); + + for comment in comments { + let line = comment.get_line(); + if line.row < lines_len { + match comment { + Comment::Inline { + line, + original_content, + } => { + manage_inline_comment(&mut lines, line, original_content); + } + Comment::Block { + line, + above_line, + below_line, + } => manage_block_comment(&mut lines, line, above_line, below_line), + } + } + } + // rejoin the lines again + lines.join("\n") +} + +fn manage_inline_comment(lines: &mut [String], line: &Line, original_content: &str) { + // for comments added to a node line, we can detect the row and the original content, and then just go to that line, + // if the content of the line is the same as the original content, we can add the comment to the end of the line. + // if the content of the line is different, we have to look for the original content in the document, as it may have been moved + // if we find it, we add the comment to the end of the line, if we don't find it, we add the comment to the end of the line of the original content even if the content is different. + let current_content = &lines[line.row]; + if current_content.starts_with(original_content) { + // line is ok, just add the comment + let comment_added = format!("{} {}", lines[line.row], line.content.clone()); + lines[line.row] = comment_added; + } else { + // content is different, try to find the original content in another line + if let Some((row, found_line)) = lines + .iter() + .enumerate() + .find(|(_, l)| l.starts_with(original_content)) + { + // we found it, add the comment + let comment_added = format!("{} {}", found_line, line.content.clone()); + lines[row] = comment_added.to_string(); + } else { + // ignore comment or add it to the original line? + // TODO: add option for the user to decide what to do? + } + } +} + +fn manage_block_comment( + lines: &mut Vec, + line: &Line, + above_line: &Option, + below_line: &Option, +) { + // block comment + // we check the line in the original content, if the content is the same and the line above and below are the same, we add the comment. + match (above_line, below_line) { + (Some(above_line), Some(below_line)) => { + // iterate from the start and try to find a couple of lines + let (trimmed_above, trimmed_below) = + (above_line.content.trim(), below_line.content.trim()); + let found = lines.iter().enumerate().find(|(i, l)| { + lines.get(i + 1).map_or(false, |next| { + l.trim().starts_with(trimmed_above) && next.trim().starts_with(trimmed_below) + }) + }); + if let Some(found) = found { + // add the comment in the line below + lines.insert(found.0 + 1, line.content.clone()); + } else { + // if not found, some lines may have been inserted in between. + // as most usually comments are placed above the line or in the line (not usually below) + // we will test for the the below line + search_and_insert_if_found(&below_line.content, lines, &line.content); + } + } + (Some(above_line), None) => { + // most probably was the last line + // start searching from the end + let trimmed = above_line.content.trim(); + let found = lines + .iter() + .enumerate() + .rev() + .find(|(_, l)| l.trim().starts_with(trimmed)); + if let Some(found) = found { + // add the comment in the line below + lines.insert(found.0 + 1, line.content.clone()); + } + } + (None, Some(below_line)) => { + // most probably was the first line + // start searching from the beginning + search_and_insert_if_found(&below_line.content, lines, &line.content); + } + (None, None) => { + // we have a block comment with no context, just add it to the line + // NOTE: potentially do nothing, this case should not happen + lines.insert(line.row, line.content.clone()); + } + } +} + +fn search_and_insert_if_found(content: &str, lines: &mut Vec, comment: &str) { + let trimmed = content.trim(); + let found = lines + .iter() + .enumerate() + .find(|(_, l)| l.trim().starts_with(trimmed)); + + if let Some(found) = found { + // add the comment in the line + lines.insert(found.0, comment.to_owned()); + } +} + +fn get_line_content(source: &str, byte_offset: usize) -> &str { + let start = source[..byte_offset].rfind('\n').map_or(0, |pos| pos + 1); + let end = source[byte_offset..] + .find('\n') + .map_or(source.len(), |pos| byte_offset + pos); + let content = &source[start..end]; + content + .find('#') + .map_or(content, |index| content[..index].trim_end()) +} diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/error.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/error.rs index 25f26a1c..612196f1 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/error.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/error.rs @@ -1,3 +1,4 @@ +use super::comment_preserver::ReconcileError; use rocket::{http::ContentType, response::Responder, Response}; use serde_json::json; use thiserror::Error; @@ -12,6 +13,12 @@ pub enum ConfigFileError { }, #[error("Error decoding base64 string")] Decoder { source: anyhow::Error }, + + #[error("Error reconciling comments")] + CommentReconciler { + #[from] + source: ReconcileError, + }, } impl From for ConfigFileError { @@ -37,6 +44,7 @@ impl ConfigFileError { match self { Self::Parser { .. } => 1, Self::Decoder { .. } => 2, + Self::CommentReconciler { .. } => 3, } } } diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/mod.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/mod.rs index 2dd9ef32..027f3b90 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/mod.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/mod.rs @@ -1,3 +1,4 @@ +mod comment_preserver; pub mod endpoints; mod error; mod models; diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs index 22d9f489..3e2d9eaf 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs @@ -1,3 +1,4 @@ +use super::comment_preserver::reconcile_comments; use super::error::ConfigFileError; use indexmap::IndexMap; use kernel::config_file::config_file_to_yaml; @@ -10,11 +11,17 @@ use std::{borrow::Cow, fmt::Debug, ops::Deref}; use tracing::instrument; #[derive(Debug, Default, Clone, PartialEq)] -pub struct StaticAnalysisConfigFile(ConfigFile); +pub struct StaticAnalysisConfigFile { + config_file: ConfigFile, + original_content: Option, +} impl From for StaticAnalysisConfigFile { fn from(value: ConfigFile) -> Self { - Self(value) + Self { + config_file: value, + original_content: None, + } } } @@ -27,14 +34,17 @@ impl TryFrom for StaticAnalysisConfigFile { return Ok(Self::default()); } let config = parse_config_file(&content)?; - Ok(config.into()) + Ok(Self { + config_file: config, + original_content: Some(content), + }) } } impl Deref for StaticAnalysisConfigFile { type Target = ConfigFile; fn deref(&self) -> &Self::Target { - &self.0 + &self.config_file } } @@ -104,7 +114,7 @@ impl StaticAnalysisConfigFile { #[instrument(skip(self))] pub fn ignore_rule(&mut self, rule: Cow) { - let config = &mut self.0; + let config = &mut self.config_file; if let Some((ruleset_name, rule_name)) = rule.split_once('/') { // the ruleset may exist and contain other rules so we // can't update it blindly @@ -168,14 +178,15 @@ impl StaticAnalysisConfigFile { rulesets: &[impl AsRef + Debug], config_content_base64: Option, ) -> Result { - let mut config = if config_content_base64.is_some() { - Self::try_from(config_content_base64.unwrap()).map_err(|e| { + let mut original_content: Option = None; + + let mut config = config_content_base64.map_or(Ok(Self::default()), |content| { + original_content = Some(content.clone()); + Self::try_from(content).map_err(|e| { tracing::error!(error =?e, "Error trying to parse config file"); e }) - } else { - Ok(Self(ConfigFile::default())) - }?; + })?; config.add_rulesets(rulesets); config.to_string().map_err(|e| { @@ -186,7 +197,7 @@ impl StaticAnalysisConfigFile { #[instrument(skip(self))] pub fn add_rulesets(&mut self, rulesets: &[impl AsRef + Debug]) { - let config = &mut self.0; + let config = &mut self.config_file; for ruleset in rulesets { if !config.rulesets.contains_key(ruleset.as_ref()) { config @@ -237,7 +248,12 @@ impl StaticAnalysisConfigFile { let str = config_file_to_yaml(self)?; // fix null maps let fixed = str.replace(": null", ":"); - Ok(fixed) + // preserve the comments if the original content is provided + if let Some(original_content) = &self.original_content { + reconcile_comments(original_content, &fixed).map_err(Into::into) + } else { + Ok(fixed) + } } } diff --git a/crates/static-analysis-kernel/Cargo.toml b/crates/static-analysis-kernel/Cargo.toml index f78dadee..7a0e0b9b 100644 --- a/crates/static-analysis-kernel/Cargo.toml +++ b/crates/static-analysis-kernel/Cargo.toml @@ -14,6 +14,7 @@ derive_builder = { workspace = true } serde-sarif = { workspace = true } sha2 = { workspace = true } indexmap = { workspace = true } +tree-sitter = { workspace = true } # other deno_core = "0.196.0" @@ -21,7 +22,6 @@ globset = "0.4.14" sequence_trie = "0.3.6" serde_yaml = "0.9.21" thiserror = "1.0.59" -tree-sitter = "0.22.6" [build-dependencies] cc = "1.0.97" From 2efcdb3530c077aef0dc884c741bc074c139281a Mon Sep 17 00:00:00 2001 From: Roberto Huertas Date: Sun, 28 Jul 2024 11:01:58 +0200 Subject: [PATCH 02/23] fixed some tests --- .../comment_preserver/reconciler.rs | 5 +- .../static_analysis_config_file.rs | 58 +++++++++---------- 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs index 428edeb1..df04cc99 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs @@ -63,9 +63,10 @@ fn extract_comments_from_node(node: Node<'_>, source: &str, comments: &mut Vec Date: Sun, 28 Jul 2024 12:21:21 +0200 Subject: [PATCH 03/23] support for multiline comments --- .../comment_preserver/reconciler.rs | 85 ++++++++++++++----- 1 file changed, 62 insertions(+), 23 deletions(-) diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs index df04cc99..e172a0dc 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs @@ -1,3 +1,5 @@ +use std::collections::HashSet; + use super::models::{Comment, Line}; use kernel::analysis::tree_sitter::get_tree; use kernel::model::common::Language; @@ -23,7 +25,8 @@ pub fn reconcile_comments( let root_node = tree.root_node(); let mut comments = vec![]; - extract_comments_from_node(root_node, original_content, &mut comments); + let mut visited = HashSet::new(); + extract_comments_from_node(root_node, original_content, &mut comments, &mut visited); let reconciled_content = reconcile(new_content, &comments); @@ -35,15 +38,47 @@ pub fn reconcile_comments( Ok(formatted) } -fn extract_comments_from_node(node: Node<'_>, source: &str, comments: &mut Vec) { +fn get_related_comments<'a, 'b>( + next: Option>, + source: &str, + visited: &'b mut HashSet>, + comment: &mut String, +) -> Option> { + if let Some(next) = next { + if next.kind() == "comment" { + // get the comment + let content = &source[next.start_byte()..next.end_byte()]; + *comment = format!("{}\n{}", comment, content); + // println!("Related comment: {}", content); + visited.insert(next); + get_related_comments(next.next_sibling(), source, visited, comment) + } else { + Some(next) + } + } else { + None + } +} + +fn extract_comments_from_node<'a, 'b>( + node: Node<'a>, + source: &str, + comments: &mut Vec, + visited: &'b mut HashSet>, +) { if node.kind() == "comment" { + if visited.contains(&node) { + return; + } + visited.insert(node); + let start_byte = node.start_byte(); let end_byte = node.end_byte(); let comment = &source[start_byte..end_byte]; let row = node.start_position().row; - let prev = node.prev_sibling(); - let final_comment = prev + let prev_sibling = node.prev_sibling(); + let final_comment = prev_sibling .and_then(|p| { if p.start_position().row == row { Some(p) @@ -52,45 +87,49 @@ fn extract_comments_from_node(node: Node<'_>, source: &str, comments: &mut Vec String { let mut lines: Vec = modified.lines().map(ToString::to_string).collect(); - let lines_len = lines.len(); for comment in comments { let line = comment.get_line(); - if line.row < lines_len { + if line.row < lines.len() { match comment { Comment::Inline { line, original_content, - } => { - manage_inline_comment(&mut lines, line, original_content); - } + } => manage_inline_comment(&mut lines, line, original_content), + Comment::Block { line, above_line, From f53b98e49254f20df4fbd2f4c6bcae54bcd3bb69 Mon Sep 17 00:00:00 2001 From: Roberto Huertas Date: Sun, 28 Jul 2024 12:32:28 +0200 Subject: [PATCH 04/23] tests passing - not complete --- .../static_analysis_config_file.rs | 148 +++++++++--------- 1 file changed, 75 insertions(+), 73 deletions(-) diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs index 3cbfcd38..f6a0e0e2 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs @@ -287,20 +287,20 @@ rulesets: #[test] fn it_works_complex() { let content = to_encoded_content( - r" + r#" schema-version: v1 rulesets: -- java-security -- java-1 -- ruleset1: - rules: - rule2: + - java-security + - java-1 + - ruleset1: + rules: + rule2: only: - - foo/bar - rule1: + - foo/bar + rule1: ignore: - - '**' -", + - "**" +"#, ); let rulesets = StaticAnalysisConfigFile::to_rulesets(content); assert_eq!(rulesets, vec!["java-security", "java-1", "ruleset1"]); @@ -411,20 +411,20 @@ rulesets: #[test] fn it_works_complex() { let content = to_encoded_content( - r" + r#" schema-version: v1 rulesets: -- java-security -- java-1 -- ruleset1: - rules: - rule2: - only: - - foo/bar - rule1: - ignore: - - '**' -", + - java-security + - java-1 + - ruleset1: + rules: + rule2: + only: + - foo/bar + rule1: + ignore: + - "**" +"#, ); let config = StaticAnalysisConfigFile::with_added_rulesets( &["ruleset1", "ruleset2", "a-ruleset3"], @@ -432,22 +432,22 @@ rulesets: ) .unwrap(); - let expected = r" + let expected = r#" schema-version: v1 rulesets: -- java-security -- java-1 -- ruleset1: - rules: - rule2: - only: - - foo/bar - rule1: - ignore: - - '**' -- ruleset2 -- a-ruleset3 -"; + - java-security + - java-1 + - ruleset1: + rules: + rule2: + only: + - foo/bar + rule1: + ignore: + - "**" + - ruleset2 + - a-ruleset3 +"#; assert_eq!(config.trim(), expected.trim()); } @@ -490,17 +490,17 @@ rulesets: StaticAnalysisConfigFile::with_ignored_rule("ruleset1/rule1".into(), content) .unwrap(); - let expected = r" + let expected = r#" schema-version: v1 rulesets: -- java-1 -- java-security -- ruleset1: - rules: - rule1: - ignore: - - '**' -"; + - java-1 + - java-security + - ruleset1: + rules: + rule1: + ignore: + - "**" +"#; assert_eq!(config.trim(), expected.trim()); } @@ -519,16 +519,17 @@ rulesets: StaticAnalysisConfigFile::with_ignored_rule("ruleset1/rule1".into(), content) .unwrap(); - let expected = r" + let expected = r#" schema-version: v1 rulesets: -- java-1 -- java-security -- ruleset1: - rules: - rule1: - ignore: - - '**'"; + - java-1 + - java-security + - ruleset1: + rules: + rule1: + ignore: + - "**" +"#; assert_eq!(config.trim(), expected.trim()); } @@ -552,18 +553,19 @@ rulesets: StaticAnalysisConfigFile::with_ignored_rule("ruleset1/rule1".into(), content) .unwrap(); - let expected = r" + let expected = r#" schema-version: v1 rulesets: -- java-1 -- java-security -- ruleset1: - rules: - rule1: - only: - - foo/bar - ignore: - - '**'"; + - java-1 + - java-security + - ruleset1: + rules: + rule1: + only: + - foo/bar + ignore: + - "**" +"#; assert_eq!(config.trim(), expected.trim()); } @@ -675,18 +677,18 @@ rulesets: StaticAnalysisConfigFile::with_ignored_rule("ruleset1/rule2".into(), content) .unwrap(); - let expected = r" + let expected = r#" schema-version: v1 rulesets: -- java-security -- java-1 -- ruleset1: - rules: - rule2: - ignore: - - '**' - severity: ERROR -"; + - java-security + - java-1 + - ruleset1: + rules: + rule2: + ignore: + - "**" + severity: ERROR +"#; assert_eq!(config.trim(), expected.trim()); } From c14708b87031ec6028c141ae8459383ec015384f Mon Sep 17 00:00:00 2001 From: Roberto Huertas Date: Sun, 28 Jul 2024 20:31:44 +0200 Subject: [PATCH 05/23] fixed issue with inline comments --- .../comment_preserver/models.rs | 16 ---------- .../comment_preserver/reconciler.rs | 32 +++++++++---------- 2 files changed, 16 insertions(+), 32 deletions(-) diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/models.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/models.rs index 7e1d1401..2b5f99e5 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/models.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/models.rs @@ -22,19 +22,3 @@ pub enum Comment { below_line: Option, }, } - -impl Comment { - pub const fn get_line(&self) -> &Line { - match self { - Self::Inline { - line, - original_content: _, - } - | Self::Block { - line, - above_line: _, - below_line: _, - } => line, - } - } -} diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs index e172a0dc..fe235c18 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs @@ -122,20 +122,17 @@ fn reconcile(modified: &str, comments: &[Comment]) -> String { let mut lines: Vec = modified.lines().map(ToString::to_string).collect(); for comment in comments { - let line = comment.get_line(); - if line.row < lines.len() { - match comment { - Comment::Inline { - line, - original_content, - } => manage_inline_comment(&mut lines, line, original_content), - - Comment::Block { - line, - above_line, - below_line, - } => manage_block_comment(&mut lines, line, above_line, below_line), - } + match comment { + Comment::Inline { + line, + original_content, + } => manage_inline_comment(&mut lines, line, original_content), + + Comment::Block { + line, + above_line, + below_line, + } => manage_block_comment(&mut lines, line, above_line, below_line), } } // rejoin the lines again @@ -147,8 +144,11 @@ fn manage_inline_comment(lines: &mut [String], line: &Line, original_content: &s // if the content of the line is the same as the original content, we can add the comment to the end of the line. // if the content of the line is different, we have to look for the original content in the document, as it may have been moved // if we find it, we add the comment to the end of the line, if we don't find it, we add the comment to the end of the line of the original content even if the content is different. - let current_content = &lines[line.row]; - if current_content.starts_with(original_content) { + let current_content = &lines.get(line.row); + if current_content + .filter(|c| c.starts_with(original_content)) + .is_some() + { // line is ok, just add the comment let comment_added = format!("{} {}", lines[line.row], line.content.clone()); lines[line.row] = comment_added; From c5d1ee938695e879536ce5f588692f2c3766bf03 Mon Sep 17 00:00:00 2001 From: Roberto Huertas Date: Sun, 28 Jul 2024 21:44:11 +0200 Subject: [PATCH 06/23] fixed issue with inline comments, prettify and tests --- .../comment_preserver/mod.rs | 2 +- .../comment_preserver/reconciler.rs | 97 ++++++++++++++++--- .../static_analysis_config_file.rs | 15 +-- 3 files changed, 95 insertions(+), 19 deletions(-) diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/mod.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/mod.rs index 1de2ab01..9d6e08c7 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/mod.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/mod.rs @@ -1,3 +1,3 @@ mod models; mod reconciler; -pub use reconciler::{reconcile_comments, ReconcileError}; +pub use reconciler::{prettify_yaml, reconcile_comments, ReconcileError}; diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs index fe235c18..922fe46a 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs @@ -13,9 +13,17 @@ pub struct ReconcileError { pub source: anyhow::Error, } +pub fn prettify_yaml(content: &str) -> Result { + let options = pretty_yaml::config::FormatOptions::default(); + pretty_yaml::format_text(content, &options).map_err(|e| ReconcileError { + source: anyhow::anyhow!("Failed to format the content: {}", e), + }) +} + pub fn reconcile_comments( original_content: &str, new_content: &str, + prettify: bool, ) -> Result { // parse the original content and look for comments let tree = get_tree(original_content, &Language::Yaml).ok_or_else(|| { @@ -30,12 +38,12 @@ pub fn reconcile_comments( let reconciled_content = reconcile(new_content, &comments); - // make it pretty - let options = pretty_yaml::config::FormatOptions::default(); - let formatted = pretty_yaml::format_text(&reconciled_content, &options) - .map_err(|e| anyhow::anyhow!("Failed to format the reconciled content: {}", e))?; - - Ok(formatted) + // make it pretty? + if prettify { + prettify_yaml(&reconciled_content) + } else { + Ok(reconciled_content) + } } fn get_related_comments<'a, 'b>( @@ -49,7 +57,6 @@ fn get_related_comments<'a, 'b>( // get the comment let content = &source[next.start_byte()..next.end_byte()]; *comment = format!("{}\n{}", comment, content); - // println!("Related comment: {}", content); visited.insert(next); get_related_comments(next.next_sibling(), source, visited, comment) } else { @@ -105,11 +112,12 @@ fn extract_comments_from_node<'a, 'b>( }), } }, - |prev_sibling| Comment::Inline { + |_prev_sibling| Comment::Inline { line: Line::new(row, comment.to_string()), - original_content: source[prev_sibling.start_byte()..start_byte - 1].to_string(), + original_content: get_line_content(source, start_byte).trim().to_string(), }, ); + comments.push(final_comment); } @@ -157,14 +165,13 @@ fn manage_inline_comment(lines: &mut [String], line: &Line, original_content: &s if let Some((row, found_line)) = lines .iter() .enumerate() - .find(|(_, l)| l.starts_with(original_content)) + .find(|(_, l)| l.trim().starts_with(original_content)) { // we found it, add the comment let comment_added = format!("{} {}", found_line, line.content.clone()); lines[row] = comment_added.to_string(); } else { // ignore comment or add it to the original line? - // TODO: add option for the user to decide what to do? } } } @@ -247,3 +254,71 @@ fn get_line_content(source: &str, byte_offset: usize) -> &str { .find('#') .map_or(content, |index| content[..index].trim_end()) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn it_works() { + let original_content = r#" +schema-version: v1 +rulesets: + # this is a comment above java-security + - java-security + - java-1 # inline comment for java-1 + # multi1 + # multi2 + # multi3 + # multi4 + - ruleset1: + rules: + rule2: # this is rule 2 comment + only: + - foo/bar + rule1: + ignore: + - "**" +"#; + + let modified = r#" +schema-version: v1 +rulesets: + - java-security + - java-1 + - java-2 + - ruleset1: + rules: + rule2: + only: + - foo/bar + rule3: + ignore: + - "**" +"#; + + let expected = r#" +schema-version: v1 +rulesets: + # this is a comment above java-security + - java-security + - java-1 # inline comment for java-1 + - java-2 + # multi1 + # multi2 + # multi3 + # multi4 + - ruleset1: + rules: + rule2: # this is rule 2 comment + only: + - foo/bar + rule3: + ignore: + - "**" +"#; + + let result = reconcile_comments(original_content, modified, true).unwrap(); + assert_eq!(result.trim(), expected.trim()); + } +} diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs index f6a0e0e2..c674cf82 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs @@ -1,4 +1,4 @@ -use super::comment_preserver::reconcile_comments; +use super::comment_preserver::{prettify_yaml, reconcile_comments}; use super::error::ConfigFileError; use indexmap::IndexMap; use kernel::config_file::config_file_to_yaml; @@ -250,9 +250,10 @@ impl StaticAnalysisConfigFile { let fixed = str.replace(": null", ":"); // preserve the comments if the original content is provided if let Some(original_content) = &self.original_content { - reconcile_comments(original_content, &fixed).map_err(Into::into) + reconcile_comments(original_content, &fixed, true).map_err(Into::into) } else { - Ok(fixed) + // prettify the content + prettify_yaml(&fixed).map_err(Into::into) } } } @@ -357,9 +358,9 @@ rulesets: let expected = r" schema-version: v1 rulesets: -- ruleset1 -- ruleset2 -- a-ruleset3 + - ruleset1 + - ruleset2 + - a-ruleset3 "; assert_eq!(config.trim(), expected.trim()); } @@ -374,7 +375,7 @@ rulesets: let expected = r" schema-version: v1 rulesets: -- ruleset1 + - ruleset1 "; assert_eq!(config.trim(), expected.trim()); } From 8af29a924ae14f3c86574013ffc8d16fccf19297 Mon Sep 17 00:00:00 2001 From: Roberto Huertas Date: Mon, 29 Jul 2024 11:53:01 +0200 Subject: [PATCH 07/23] added separated tests and removed lifetime --- .../comment_preserver/reconciler.rs | 126 +++++++++++++++++- 1 file changed, 121 insertions(+), 5 deletions(-) diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs index 922fe46a..3a38be34 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs @@ -46,10 +46,10 @@ pub fn reconcile_comments( } } -fn get_related_comments<'a, 'b>( +fn get_related_comments<'a>( next: Option>, source: &str, - visited: &'b mut HashSet>, + visited: &mut HashSet>, comment: &mut String, ) -> Option> { if let Some(next) = next { @@ -67,11 +67,11 @@ fn get_related_comments<'a, 'b>( } } -fn extract_comments_from_node<'a, 'b>( +fn extract_comments_from_node<'a>( node: Node<'a>, source: &str, comments: &mut Vec, - visited: &'b mut HashSet>, + visited: &mut HashSet>, ) { if node.kind() == "comment" { if visited.contains(&node) { @@ -260,7 +260,123 @@ mod tests { use super::*; #[test] - fn it_works() { + fn it_works_for_inline_comments() { + let original_content = r#" +schema-version: v1 +rulesets: + - java-security + - java-1 # inline comment for java-1 + - ruleset1: + rules: + rule2: # this is rule 2 comment + only: + - foo/bar + rule1: + ignore: + - "**" +"#; + + let modified = r#" +schema-version: v1 +rulesets: + - java-security + - java-1 + - java-2 + - ruleset1: + rules: + rule2: + only: + - foo/bar + rule3: + ignore: + - "**" +"#; + + let expected = r#" +schema-version: v1 +rulesets: + - java-security + - java-1 # inline comment for java-1 + - java-2 + - ruleset1: + rules: + rule2: # this is rule 2 comment + only: + - foo/bar + rule3: + ignore: + - "**" +"#; + + let result = reconcile_comments(original_content, modified, true).unwrap(); + assert_eq!(result.trim(), expected.trim()); + } + + #[test] + fn it_works_for_block_comments() { + let original_content = r#" +schema-version: v1 +rulesets: + # this is a comment above java-security + - java-security + - java-1 + # multi1 + # multi2 + # multi3 + # multi4 + - ruleset1: + rules: + rule2: + only: + - foo/bar + rule1: + ignore: + - "**" +"#; + + let modified = r#" +schema-version: v1 +rulesets: + - java-security + - java-1 + - java-2 + - ruleset1: + rules: + rule2: + only: + - foo/bar + rule3: + ignore: + - "**" +"#; + + let expected = r#" +schema-version: v1 +rulesets: + # this is a comment above java-security + - java-security + - java-1 + - java-2 + # multi1 + # multi2 + # multi3 + # multi4 + - ruleset1: + rules: + rule2: + only: + - foo/bar + rule3: + ignore: + - "**" +"#; + + let result = reconcile_comments(original_content, modified, true).unwrap(); + assert_eq!(result.trim(), expected.trim()); + } + + #[test] + fn it_works_mixed() { let original_content = r#" schema-version: v1 rulesets: From 5bceb079b3eeb9ca0660ea2af2b40db5146232ff Mon Sep 17 00:00:00 2001 From: Roberto Huertas Date: Mon, 29 Jul 2024 12:02:25 +0200 Subject: [PATCH 08/23] warnings from clippy --- crates/cli/src/datadog_utils.rs | 3 ++- crates/cli/src/file_utils.rs | 1 + crates/static-analysis-kernel/src/model/analysis.rs | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/cli/src/datadog_utils.rs b/crates/cli/src/datadog_utils.rs index fc6a3438..53cb78b4 100644 --- a/crates/cli/src/datadog_utils.rs +++ b/crates/cli/src/datadog_utils.rs @@ -287,7 +287,8 @@ pub fn get_all_default_rulesets(use_staging: bool, debug: bool) -> Result bool { match &self.ignore_file { AllRules => { From 2932721fd61fed9d4b026ac64d51bc972facc2cd Mon Sep 17 00:00:00 2001 From: Roberto Huertas Date: Mon, 29 Jul 2024 13:52:55 +0200 Subject: [PATCH 09/23] avoid passing the previous node --- .../comment_preserver/reconciler.rs | 65 +++++++++---------- 1 file changed, 30 insertions(+), 35 deletions(-) diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs index 3a38be34..2d1edaa8 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs @@ -85,38 +85,33 @@ fn extract_comments_from_node<'a>( let row = node.start_position().row; let prev_sibling = node.prev_sibling(); - let final_comment = prev_sibling - .and_then(|p| { - if p.start_position().row == row { - Some(p) - } else { - None - } - }) - .map_or_else( - || { - // this can be a multiline comment - // let's keep adding lines until next_sibling is not comment - let mut comment = comment.to_string(); - let last_node = - get_related_comments(node.next_sibling(), source, visited, &mut comment); - Comment::Block { - line: Line::new(row, comment), - above_line: prev_sibling.map(|prev| { - let content = get_line_content(source, prev.end_byte()); - Line::new(row, content.to_string()) - }), - below_line: last_node.map(|next| { - let content = get_line_content(source, next.start_byte()); - Line::new(next.start_position().row, content.to_string()) - }), - } - }, - |_prev_sibling| Comment::Inline { - line: Line::new(row, comment.to_string()), - original_content: get_line_content(source, start_byte).trim().to_string(), - }, - ); + + let final_comment = if prev_sibling + .filter(|p| p.start_position().row == row) + .is_some() + { + Comment::Inline { + line: Line::new(row, comment.to_string()), + original_content: get_line_content(source, start_byte).trim().to_string(), + } + } else { + // this can be a multiline comment + // let's keep adding lines until next_sibling is not comment + let mut comment = comment.to_string(); + let last_node = + get_related_comments(node.next_sibling(), source, visited, &mut comment); + Comment::Block { + line: Line::new(row, comment), + above_line: prev_sibling.map(|prev| { + let content = get_line_content(source, prev.end_byte()); + Line::new(row, content.to_string()) + }), + below_line: last_node.map(|next| { + let content = get_line_content(source, next.start_byte()); + Line::new(next.start_position().row, content.to_string()) + }), + } + }; comments.push(final_comment); } @@ -150,8 +145,8 @@ fn reconcile(modified: &str, comments: &[Comment]) -> String { fn manage_inline_comment(lines: &mut [String], line: &Line, original_content: &str) { // for comments added to a node line, we can detect the row and the original content, and then just go to that line, // if the content of the line is the same as the original content, we can add the comment to the end of the line. - // if the content of the line is different, we have to look for the original content in the document, as it may have been moved - // if we find it, we add the comment to the end of the line, if we don't find it, we add the comment to the end of the line of the original content even if the content is different. + // if the content of the line is different, we have to look for the original content in the document (as it may have been moved) + // if we find it, we add the comment to the end of the line, if we don't find it, we ignore the comment. let current_content = &lines.get(line.row); if current_content .filter(|c| c.starts_with(original_content)) @@ -171,7 +166,7 @@ fn manage_inline_comment(lines: &mut [String], line: &Line, original_content: &s let comment_added = format!("{} {}", found_line, line.content.clone()); lines[row] = comment_added.to_string(); } else { - // ignore comment or add it to the original line? + // ignore comment (or add it to the original line?) } } } From 1c5d537997daeb109db427bbe528999f35978c7e Mon Sep 17 00:00:00 2001 From: Roberto Huertas Date: Mon, 29 Jul 2024 21:32:16 +0200 Subject: [PATCH 10/23] comments and more tests --- .../comment_preserver/reconciler.rs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs index 2d1edaa8..8e4691ea 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs @@ -20,6 +20,27 @@ pub fn prettify_yaml(content: &str) -> Result { }) } +/// This is a best-effort comment reconciler, which uses a simple diff algorithm to try to locate +/// comments in the new content. +/// +/// The algorithm applies for the majority of the cases affecting the static analysis configuration files but has some limitations: +/// +/// * Repeated elements may lead to false positives if there are lines added before or between the existing content. +/// * If the original content uses a different syntax than the one emitted by the serializer, we may not be able to determine the location of those comments (e.g. dictionaries and list can be represented in an abbreviated form) +/// +/// +/// # Returns +/// +/// If successful, it returns a `Result` containing a `String`. The `String` is the reconciled configuration +/// file content. +/// +/// # Errors +/// +/// This function will return an error of type `ReconcileError` if: +/// +/// * There's an issue getting the tree-sitter tree +/// * There's an issue trying to apply the format to the reconciled yaml content +/// pub fn reconcile_comments( original_content: &str, new_content: &str, @@ -332,6 +353,7 @@ rulesets: let modified = r#" schema-version: v1 rulesets: + - java-0 - java-security - java-1 - java-2 @@ -348,6 +370,7 @@ rulesets: let expected = r#" schema-version: v1 rulesets: + - java-0 # this is a comment above java-security - java-security - java-1 @@ -395,6 +418,7 @@ rulesets: let modified = r#" schema-version: v1 rulesets: + - java-0 - java-security - java-1 - java-2 @@ -411,6 +435,7 @@ rulesets: let expected = r#" schema-version: v1 rulesets: + - java-0 # this is a comment above java-security - java-security - java-1 # inline comment for java-1 From 4613d95ec48b1ab25fc0afabe5ef820a0648252f Mon Sep 17 00:00:00 2001 From: Roberto Huertas Date: Mon, 29 Jul 2024 21:37:02 +0200 Subject: [PATCH 11/23] specification for inline comments --- .../comment_preserver/reconciler.rs | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs index 8e4691ea..8ad77e2d 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs @@ -25,7 +25,7 @@ pub fn prettify_yaml(content: &str) -> Result { /// /// The algorithm applies for the majority of the cases affecting the static analysis configuration files but has some limitations: /// -/// * Repeated elements may lead to false positives if there are lines added before or between the existing content. +/// * Repeated elements with `inline` comments may lead to false positives if there are lines added before or between the existing content. /// * If the original content uses a different syntax than the one emitted by the serializer, we may not be able to determine the location of those comments (e.g. dictionaries and list can be represented in an abbreviated form) /// /// @@ -457,4 +457,30 @@ rulesets: let result = reconcile_comments(original_content, modified, true).unwrap(); assert_eq!(result.trim(), expected.trim()); } + + #[test] + fn it_works_for_repeated_keys_with_inline_comments_if_no_additions_before_comment_occurrence() { + let original_content = r#" +schema-version: v1 +rulesets: + - java-1 # inline comment for java-1 +"#; + + let modified = r#" +schema-version: v1 +rulesets: + - java-1 + - java-2 +"#; + + let expected = r#" +schema-version: v1 +rulesets: + - java-1 # inline comment for java-1 + - java-2 +"#; + + let result = reconcile_comments(original_content, modified, true).unwrap(); + assert_eq!(result.trim(), expected.trim()); + } } From 21e0c76983a05119cdc4a2ca7ecce1aab878d7ef Mon Sep 17 00:00:00 2001 From: Roberto Huertas Date: Mon, 29 Jul 2024 22:01:00 +0200 Subject: [PATCH 12/23] fixed case for inline --- .../comment_preserver/reconciler.rs | 52 ++++++++++++++++--- 1 file changed, 45 insertions(+), 7 deletions(-) diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs index 8ad77e2d..6294a16e 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs @@ -170,7 +170,7 @@ fn manage_inline_comment(lines: &mut [String], line: &Line, original_content: &s // if we find it, we add the comment to the end of the line, if we don't find it, we ignore the comment. let current_content = &lines.get(line.row); if current_content - .filter(|c| c.starts_with(original_content)) + .filter(|c| c.trim().starts_with(original_content)) .is_some() { // line is ok, just add the comment @@ -459,28 +459,66 @@ rulesets: } #[test] - fn it_works_for_repeated_keys_with_inline_comments_if_no_additions_before_comment_occurrence() { + fn it_works_for_repeated_keys_with_inline_comments_if_no_additions_before_comment_occurrence2() + { let original_content = r#" schema-version: v1 rulesets: - - java-1 # inline comment for java-1 + - rule-1 # inline 1 + - rule-1 # inline 2 "#; let modified = r#" schema-version: v1 rulesets: - - java-1 - - java-2 + - rule-1 + - rule-1 + - rule-2 "#; let expected = r#" schema-version: v1 rulesets: - - java-1 # inline comment for java-1 - - java-2 + - rule-1 # inline 1 + - rule-1 # inline 2 + - rule-2 "#; let result = reconcile_comments(original_content, modified, true).unwrap(); assert_eq!(result.trim(), expected.trim()); } + + mod inline_limitations { + use super::*; + + #[test] + fn it_does_not_work_for_repeated_keys_with_inline_comments_if_additions_before_comment_occurrence( + ) { + let original_content = r#" +schema-version: v1 +rulesets: + - java-1 # inline comment 1 + - java-1 # inline comment 2 +"#; + + let modified = r#" +schema-version: v1 +rulesets: + - java-0 + - java-1 + - java-1 +"#; + + let expected = r#" +schema-version: v1 +rulesets: + - java-0 + - java-1 # inline comment 1 + - java-1 # inline comment 2 +"#; + + let result = reconcile_comments(original_content, modified, true).unwrap(); + assert_ne!(result.trim(), expected.trim()); + } + } } From 5d0b5f368de8ab467194841b4d8038432a08f51b Mon Sep 17 00:00:00 2001 From: Roberto Huertas Date: Mon, 29 Jul 2024 22:01:42 +0200 Subject: [PATCH 13/23] change name of test --- .../ide/configuration_file/comment_preserver/reconciler.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs index 6294a16e..2afbe121 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs @@ -459,8 +459,7 @@ rulesets: } #[test] - fn it_works_for_repeated_keys_with_inline_comments_if_no_additions_before_comment_occurrence2() - { + fn it_works_for_repeated_keys_with_inline_comments_if_no_additions_before_comment_occurrence() { let original_content = r#" schema-version: v1 rulesets: From f3f9b943581335767034a2343e43eb9cde913787 Mon Sep 17 00:00:00 2001 From: Roberto Huertas Date: Mon, 29 Jul 2024 23:34:59 +0200 Subject: [PATCH 14/23] new test case supported --- .../comment_preserver/reconciler.rs | 49 ++++++++++--------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs index 2afbe121..4adf05eb 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs @@ -25,7 +25,7 @@ pub fn prettify_yaml(content: &str) -> Result { /// /// The algorithm applies for the majority of the cases affecting the static analysis configuration files but has some limitations: /// -/// * Repeated elements with `inline` comments may lead to false positives if there are lines added before or between the existing content. +/// * Repeated elements with `inline` comments may lead to false positives in some edge cases. /// * If the original content uses a different syntax than the one emitted by the serializer, we may not be able to determine the location of those comments (e.g. dictionaries and list can be represented in an abbreviated form) /// /// @@ -47,6 +47,7 @@ pub fn reconcile_comments( prettify: bool, ) -> Result { // parse the original content and look for comments + let original_content = original_content.trim(); let tree = get_tree(original_content, &Language::Yaml).ok_or_else(|| { anyhow::anyhow!("Failed to parse the original content with the tree-sitter parser") })?; @@ -163,6 +164,11 @@ fn reconcile(modified: &str, comments: &[Comment]) -> String { lines.join("\n") } +fn starts_with_and_no_comment(text: &str, pat: &str) -> bool { + let trimmed = text.trim(); + trimmed.starts_with(pat) && !trimmed.contains('#') +} + fn manage_inline_comment(lines: &mut [String], line: &Line, original_content: &str) { // for comments added to a node line, we can detect the row and the original content, and then just go to that line, // if the content of the line is the same as the original content, we can add the comment to the end of the line. @@ -170,7 +176,7 @@ fn manage_inline_comment(lines: &mut [String], line: &Line, original_content: &s // if we find it, we add the comment to the end of the line, if we don't find it, we ignore the comment. let current_content = &lines.get(line.row); if current_content - .filter(|c| c.trim().starts_with(original_content)) + .filter(|c| starts_with_and_no_comment(c, original_content)) .is_some() { // line is ok, just add the comment @@ -178,10 +184,11 @@ fn manage_inline_comment(lines: &mut [String], line: &Line, original_content: &s lines[line.row] = comment_added; } else { // content is different, try to find the original content in another line + if let Some((row, found_line)) = lines .iter() .enumerate() - .find(|(_, l)| l.trim().starts_with(original_content)) + .find(|(_, l)| starts_with_and_no_comment(l, original_content)) { // we found it, add the comment let comment_added = format!("{} {}", found_line, line.content.clone()); @@ -487,37 +494,33 @@ rulesets: assert_eq!(result.trim(), expected.trim()); } - mod inline_limitations { - use super::*; - - #[test] - fn it_does_not_work_for_repeated_keys_with_inline_comments_if_additions_before_comment_occurrence( - ) { - let original_content = r#" + #[test] + fn it_does_work_for_repeated_keys_with_inline_comments_if_additions_before_comment_occurrence() + { + let original_content = r#" schema-version: v1 rulesets: - - java-1 # inline comment 1 - - java-1 # inline comment 2 + - java-1 # inline comment 1 + - java-1 # inline comment 2 "#; - let modified = r#" + let modified = r#" schema-version: v1 rulesets: - - java-0 - - java-1 - - java-1 + - java-0 + - java-1 + - java-1 "#; - let expected = r#" + let expected = r#" schema-version: v1 rulesets: - - java-0 - - java-1 # inline comment 1 - - java-1 # inline comment 2 + - java-0 + - java-1 # inline comment 1 + - java-1 # inline comment 2 "#; - let result = reconcile_comments(original_content, modified, true).unwrap(); - assert_ne!(result.trim(), expected.trim()); - } + let result = reconcile_comments(original_content, modified, true).unwrap(); + assert_eq!(result.trim(), expected.trim()); } } From 256d30aa88264d3a17a765fdf311c54d1144f6ff Mon Sep 17 00:00:00 2001 From: Roberto Huertas Date: Tue, 30 Jul 2024 00:19:54 +0200 Subject: [PATCH 15/23] another case covered and better docs --- .../comment_preserver/reconciler.rs | 79 ++++++++++++++++++- 1 file changed, 75 insertions(+), 4 deletions(-) diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs index 4adf05eb..d1d07822 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs @@ -25,7 +25,7 @@ pub fn prettify_yaml(content: &str) -> Result { /// /// The algorithm applies for the majority of the cases affecting the static analysis configuration files but has some limitations: /// -/// * Repeated elements with `inline` comments may lead to false positives in some edge cases. +/// * Repeated elements with `inline` comments may lead to false positives in some edge cases (see mode inline_limitations in tests for more info) /// * If the original content uses a different syntax than the one emitted by the serializer, we may not be able to determine the location of those comments (e.g. dictionaries and list can be represented in an abbreviated form) /// /// @@ -46,8 +46,9 @@ pub fn reconcile_comments( new_content: &str, prettify: bool, ) -> Result { - // parse the original content and look for comments let original_content = original_content.trim(); + let new_content = new_content.trim(); + // parse the original content and look for comments let tree = get_tree(original_content, &Language::Yaml).ok_or_else(|| { anyhow::anyhow!("Failed to parse the original content with the tree-sitter parser") })?; @@ -175,6 +176,7 @@ fn manage_inline_comment(lines: &mut [String], line: &Line, original_content: &s // if the content of the line is different, we have to look for the original content in the document (as it may have been moved) // if we find it, we add the comment to the end of the line, if we don't find it, we ignore the comment. let current_content = &lines.get(line.row); + if current_content .filter(|c| starts_with_and_no_comment(c, original_content)) .is_some() @@ -184,7 +186,6 @@ fn manage_inline_comment(lines: &mut [String], line: &Line, original_content: &s lines[line.row] = comment_added; } else { // content is different, try to find the original content in another line - if let Some((row, found_line)) = lines .iter() .enumerate() @@ -466,7 +467,8 @@ rulesets: } #[test] - fn it_works_for_repeated_keys_with_inline_comments_if_no_additions_before_comment_occurrence() { + fn it_works_for_repeated_keys_with_inline_comments_if_no_additions_before_comment_occurrence_case_1( + ) { let original_content = r#" schema-version: v1 rulesets: @@ -494,6 +496,39 @@ rulesets: assert_eq!(result.trim(), expected.trim()); } + #[test] + fn it_works_for_repeated_keys_with_inline_comments_if_no_additions_before_comment_occurrence_case_2( + ) { + let original_content = r#" +schema-version: v1 +rulesets: + - java-1 # inline comment 1 + - java-1 + - java-1 # inline comment 2 +"#; + + let modified = r#" +schema-version: v1 +rulesets: + - java-1 + - java-1 + - java-1 + - java-2 +"#; + + let expected = r#" +schema-version: v1 +rulesets: + - java-1 # inline comment 1 + - java-1 + - java-1 # inline comment 2 + - java-2 +"#; + + let result = reconcile_comments(original_content, modified, true).unwrap(); + assert_eq!(result.trim(), expected.trim()); + } + #[test] fn it_does_work_for_repeated_keys_with_inline_comments_if_additions_before_comment_occurrence() { @@ -523,4 +558,40 @@ rulesets: let result = reconcile_comments(original_content, modified, true).unwrap(); assert_eq!(result.trim(), expected.trim()); } + + mod inline_limitations { + use super::*; + + #[test] + fn it_does_not_work_for_repeated_keys_with_inline_comments_case_1() { + let original_content = r#" +schema-version: v1 +rulesets: + - java-1 # inline comment 1 + - java-1 + - java-1 # inline comment 2 +"#; + + let modified = r#" +schema-version: v1 +rulesets: + - java-0 + - java-1 + - java-1 + - java-1 +"#; + + let expected = r#" +schema-version: v1 +rulesets: + - java-0 + - java-1 # inline comment 1 + - java-1 + - java-1 # inline comment 2 +"#; + + let result = reconcile_comments(original_content, modified, true).unwrap(); + assert_ne!(result.trim(), expected.trim()); + } + } } From 50e11bbf05b557a9762677954f4aa2768fceb1eb Mon Sep 17 00:00:00 2001 From: Roberto Huertas Date: Tue, 30 Jul 2024 01:02:17 +0200 Subject: [PATCH 16/23] improve comment detection --- .../ide/configuration_file/comment_preserver/reconciler.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs index d1d07822..85f73c93 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs @@ -167,7 +167,7 @@ fn reconcile(modified: &str, comments: &[Comment]) -> String { fn starts_with_and_no_comment(text: &str, pat: &str) -> bool { let trimmed = text.trim(); - trimmed.starts_with(pat) && !trimmed.contains('#') + trimmed.starts_with(pat) && !trimmed.contains(" #") } fn manage_inline_comment(lines: &mut [String], line: &Line, original_content: &str) { @@ -275,7 +275,7 @@ fn get_line_content(source: &str, byte_offset: usize) -> &str { .map_or(source.len(), |pos| byte_offset + pos); let content = &source[start..end]; content - .find('#') + .find(" #") .map_or(content, |index| content[..index].trim_end()) } From 56d503be1419f4ce0ca8a937dd22fdd3dd90dc4a Mon Sep 17 00:00:00 2001 From: robertohuertasm Date: Tue, 30 Jul 2024 15:49:45 +0200 Subject: [PATCH 17/23] returning the serialized yaml even if formatting and reconciliation fails avoid map_err --- .../static_analysis_config_file.rs | 31 ++++++++++++++----- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs index c674cf82..e8d88e00 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs @@ -244,17 +244,34 @@ impl StaticAnalysisConfigFile { } /// Serializes the `StaticAnalysisConfigFile` into a YAML string. + /// + /// # Returns + /// + /// This function will try to prettify/format the yaml and preserve the existing comments. + /// If it fails to do so, it will return a raw yaml with the default format and without comments. + /// + /// # Errors + /// + /// Returns a `ConfigFileError` if something goes wrong. + #[instrument(skip(self))] pub fn to_string(&self) -> Result { let str = config_file_to_yaml(self)?; // fix null maps let fixed = str.replace(": null", ":"); - // preserve the comments if the original content is provided - if let Some(original_content) = &self.original_content { - reconcile_comments(original_content, &fixed, true).map_err(Into::into) - } else { - // prettify the content - prettify_yaml(&fixed).map_err(Into::into) - } + + // reconcile and format + // NOTE: if this fails, we're going to return the content + // and swallow the error. + self.original_content + .as_ref() + .map_or_else( + || prettify_yaml(&fixed), + |original_content| reconcile_comments(original_content, &fixed, true), + ) + .or_else(|e| { + tracing::debug!(error = ?e, "Reconciliation error: {}", e.to_string()); + Ok::(fixed) + }) } } From bca9165042d9b511808f61789d57d898509248fd Mon Sep 17 00:00:00 2001 From: Roberto Huertas Date: Tue, 30 Jul 2024 15:56:07 +0200 Subject: [PATCH 18/23] better messaging --- .../ide/configuration_file/static_analysis_config_file.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs index e8d88e00..1816ad11 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs @@ -269,7 +269,7 @@ impl StaticAnalysisConfigFile { |original_content| reconcile_comments(original_content, &fixed, true), ) .or_else(|e| { - tracing::debug!(error = ?e, "Reconciliation error: {}", e.to_string()); + tracing::debug!(error = ?e, "Reconciliation or formatting error: {}", e.to_string()); Ok::(fixed) }) } From 174109e9488ed48f8fa5a676cae1a0a8015be3ed Mon Sep 17 00:00:00 2001 From: Roberto Huertas Date: Tue, 30 Jul 2024 17:07:05 +0200 Subject: [PATCH 19/23] remove null only from maps --- .../static_analysis_config_file.rs | 52 ++++++++++++++++++- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs index 1816ad11..d55b3501 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs @@ -1,6 +1,7 @@ use super::comment_preserver::{prettify_yaml, reconcile_comments}; use super::error::ConfigFileError; use indexmap::IndexMap; +use itertools::Itertools; use kernel::config_file::config_file_to_yaml; use kernel::{ config_file::parse_config_file, @@ -256,8 +257,17 @@ impl StaticAnalysisConfigFile { #[instrument(skip(self))] pub fn to_string(&self) -> Result { let str = config_file_to_yaml(self)?; - // fix null maps - let fixed = str.replace(": null", ":"); + // fix null maps, note that str will not have comments and it will be using the default serde format. + let fixed = str + .lines() + .map(|l| { + if l.ends_with(": null") { + l.replace(": null", ":") + } else { + l.to_string() + } + }) + .join("\n"); // reconcile and format // NOTE: if this fails, we're going to return the content @@ -809,4 +819,42 @@ rulesets: let config = super::StaticAnalysisConfigFile::try_from(String::new()).unwrap(); assert_eq!(config, expected); } + + #[test] + fn it_removes_null_on_maps_only() { + let content = to_encoded_content( + r#" +schema-version: v1 +rulesets: +- java-security +- java-1 +- ruleset1: + rules: + rule2: + only: + - "foo/bar: null" +"#, + ); + let config = super::StaticAnalysisConfigFile::with_added_rulesets( + &["ruleset1", "ruleset2", "a-ruleset3"], + Some(content), + ) + .unwrap(); + + let expected = r#" +schema-version: v1 +rulesets: + - java-security + - java-1 + - ruleset1: + rules: + rule2: + only: + - "foo/bar: null" + - ruleset2 + - a-ruleset3 +"#; + + assert_eq!(config.trim(), expected.trim()); + } } From edb99ffc48292ba0f21944b0d71a2dc8699af774 Mon Sep 17 00:00:00 2001 From: Roberto Huertas Date: Tue, 30 Jul 2024 18:55:30 +0200 Subject: [PATCH 20/23] fixed some lint issues --- .../bins/src/bin/datadog-static-analyzer-server.rs | 2 +- crates/bins/src/bin/datadog-static-analyzer.rs | 2 +- .../src/bin/datadog_static_analyzer_server/cli.rs | 6 +++--- .../bin/datadog_static_analyzer_server/endpoints.rs | 4 ++-- .../bin/datadog_static_analyzer_server/fairings.rs | 11 +++++------ .../configuration_file/comment_preserver/models.rs | 2 +- .../comment_preserver/reconciler.rs | 12 +++++------- .../ide/configuration_file/error.rs | 2 +- .../static_analysis_config_file.rs | 3 --- .../bin/datadog_static_analyzer_server/ide/mod.rs | 2 ++ 10 files changed, 21 insertions(+), 25 deletions(-) diff --git a/crates/bins/src/bin/datadog-static-analyzer-server.rs b/crates/bins/src/bin/datadog-static-analyzer-server.rs index 3bc84e9e..6f2c222e 100644 --- a/crates/bins/src/bin/datadog-static-analyzer-server.rs +++ b/crates/bins/src/bin/datadog-static-analyzer-server.rs @@ -2,5 +2,5 @@ mod datadog_static_analyzer_server; #[rocket::main] async fn main() { - datadog_static_analyzer_server::start().await + datadog_static_analyzer_server::start().await; } diff --git a/crates/bins/src/bin/datadog-static-analyzer.rs b/crates/bins/src/bin/datadog-static-analyzer.rs index 411675c6..df2a2f38 100644 --- a/crates/bins/src/bin/datadog-static-analyzer.rs +++ b/crates/bins/src/bin/datadog-static-analyzer.rs @@ -500,7 +500,7 @@ fn main() -> Result<()> { "Analyzing {}, {} files detected", language, files_for_language.len() - ) + ); } // take the relative path for the analysis diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/cli.rs b/crates/bins/src/bin/datadog_static_analyzer_server/cli.rs index 51fb4cef..3d6e0690 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/cli.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/cli.rs @@ -8,7 +8,7 @@ use std::{env, process, thread}; use super::state::ServerState; use super::utils::get_current_timestamp_ms; -fn print_usage(program: &str, opts: Options) { +fn print_usage(program: &str, opts: &Options) { let brief = format!("Usage: {} [options]", program); print!("{}", opts.usage(&brief)); } @@ -61,7 +61,7 @@ pub fn prepare_rocket() -> (Rocket, ServerState, Sender) { } if matches.opt_present("h") { - print_usage(&program, opts); + print_usage(&program, &opts); process::exit(0); } @@ -122,7 +122,7 @@ pub fn prepare_rocket() -> (Rocket, ServerState, Sender) { if latest_timestamp > 0 && current_timestamp > latest_timestamp + timeout_ms { eprintln!("exiting because of timeout, trying to exit gracefully"); - shutdown_handle.clone().notify(); + shutdown_handle.notify(); // we give 10 seconds for the process to terminate // if it does not, we abort thread::sleep(Duration::from_secs(10)); diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/endpoints.rs b/crates/bins/src/bin/datadog_static_analyzer_server/endpoints.rs index 080f5f00..3d1260cd 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/endpoints.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/endpoints.rs @@ -149,9 +149,9 @@ async fn serve_static( /// Catches all OPTION requests in order to get the CORS related Fairing triggered. #[rocket::options("/<_..>")] -fn get_options() -> String { +const fn get_options() -> String { /* Intentionally left empty */ - "".to_string() + String::new() } /// Simple ping method that will return "pong" as response. diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/fairings.rs b/crates/bins/src/bin/datadog_static_analyzer_server/fairings.rs index 4bbdc817..abd247ae 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/fairings.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/fairings.rs @@ -96,9 +96,9 @@ impl Fairing for KeepAlive { } } -/// Provides functionality to associate a UUIDv4 per request. +/// Provides functionality to associate a `UUIDv4` per request. /// -/// Rocket 0.5.0 does not generate per-request IDs (see: https://github.com/rwf2/Rocket/issues/21) +/// Rocket 0.5.0 does not generate per-request IDs (see: [#21](https://github.com/rwf2/Rocket/issues/21)) /// Until upstream natively supports this, we use a custom [Fairing] to generate one. pub struct TracingFairing; @@ -122,10 +122,10 @@ impl TraceSpan { } } -/// A newtype Option representing a [Span] that is used to conform to the [Request::local_cache] API +/// A newtype Option representing a [Span] that is used to conform to the [`Request::local_cache`] API struct FairingTraceSpan(Option); -/// A newtype Option representing a [String] request ID that is used to conform to the [Request::local_cache] API +/// A newtype Option representing a [String] request ID that is used to conform to the [`Request::local_cache`] API struct FairingRequestId(Option); #[rocket::async_trait] @@ -141,8 +141,7 @@ impl Fairing for TracingFairing { let request_id = req .headers() .get_one("X-Request-Id") - .map(String::from) - .unwrap_or_else(|| Uuid::new_v4().to_string()); + .map_or_else(|| Uuid::new_v4().to_string(), String::from); let request_span = tracing::info_span!( "http_request", diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/models.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/models.rs index 2b5f99e5..50a1b7a8 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/models.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/models.rs @@ -5,7 +5,7 @@ pub struct Line { } impl Line { - pub fn new(row: usize, content: String) -> Self { + pub const fn new(row: usize, content: String) -> Self { Self { row, content } } } diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs index 85f73c93..26702870 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs @@ -25,7 +25,7 @@ pub fn prettify_yaml(content: &str) -> Result { /// /// The algorithm applies for the majority of the cases affecting the static analysis configuration files but has some limitations: /// -/// * Repeated elements with `inline` comments may lead to false positives in some edge cases (see mode inline_limitations in tests for more info) +/// * Repeated elements with `inline` comments may lead to false positives in some edge cases (see mode `inline_limitations` in tests for more info) /// * If the original content uses a different syntax than the one emitted by the serializer, we may not be able to determine the location of those comments (e.g. dictionaries and list can be represented in an abbreviated form) /// /// @@ -75,19 +75,17 @@ fn get_related_comments<'a>( visited: &mut HashSet>, comment: &mut String, ) -> Option> { - if let Some(next) = next { + next.and_then(|next| { if next.kind() == "comment" { // get the comment let content = &source[next.start_byte()..next.end_byte()]; - *comment = format!("{}\n{}", comment, content); + *comment = format!("{comment}\n{content}"); visited.insert(next); get_related_comments(next.next_sibling(), source, visited, comment) } else { Some(next) } - } else { - None - } + }) } fn extract_comments_from_node<'a>( @@ -193,7 +191,7 @@ fn manage_inline_comment(lines: &mut [String], line: &Line, original_content: &s { // we found it, add the comment let comment_added = format!("{} {}", found_line, line.content.clone()); - lines[row] = comment_added.to_string(); + lines[row] = comment_added; } else { // ignore comment (or add it to the original line?) } diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/error.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/error.rs index 612196f1..d1a1d5e2 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/error.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/error.rs @@ -40,7 +40,7 @@ impl<'r> Responder<'r, 'static> for ConfigFileError { } impl ConfigFileError { - pub fn code(&self) -> u16 { + pub const fn code(&self) -> u16 { match self { Self::Parser { .. } => 1, Self::Decoder { .. } => 2, diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs index d55b3501..39882461 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs @@ -179,10 +179,7 @@ impl StaticAnalysisConfigFile { rulesets: &[impl AsRef + Debug], config_content_base64: Option, ) -> Result { - let mut original_content: Option = None; - let mut config = config_content_base64.map_or(Ok(Self::default()), |content| { - original_content = Some(content.clone()); Self::try_from(content).map_err(|e| { tracing::error!(error =?e, "Error trying to parse config file"); e diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/mod.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/mod.rs index eaa5339e..b4b6d179 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/mod.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/mod.rs @@ -1,3 +1,5 @@ +#![allow(clippy::module_name_repetitions)] + mod configuration_file; use rocket::Route; From e07765034251e0c375c67ff585f9e6246490761d Mon Sep 17 00:00:00 2001 From: Roberto Huertas Date: Wed, 31 Jul 2024 14:09:50 +0200 Subject: [PATCH 21/23] added dependency to 3rd party manifest --- LICENSE-3rdparty.csv | 1 + 1 file changed, 1 insertion(+) diff --git a/LICENSE-3rdparty.csv b/LICENSE-3rdparty.csv index aebed79c..84dcffcb 100644 --- a/LICENSE-3rdparty.csv +++ b/LICENSE-3rdparty.csv @@ -13,6 +13,7 @@ num_cpus,https://github.com/seanmonstar/num_cpus,MIT, Copyright (c) 2015 Sean Mc percent-encoding,https://github.com/servo/rust-url/,MIT, Copyright (c) 2013-2022 The rust-url developers prettytable-rs,https://github.com/phsym/prettytable-rs/,BSD-3-Clause,Copyright (c) 2022 Pierre-Henri Symoneaux rayon,https://crates.io/crates/rayon,MIT,Copyright (c) 2010 The Rust Project Developers +pretty_yaml, https://github.com/g-plane/pretty_yaml, MIT, Copyright (c) 2024-present Pig Fang rocket,https://github.com/SergioBenitez/Rocket,Apache-2.0,Copyright 2016 Sergio Benitez tree-sitter,https://github.com/tree-sitter/tree-sitter,MIT,2014 Max Brunsfeld sarif-rs,https://github.com/psastras/sarif-rs,MIT,Copyright (c) 2021 Paul Sastrasinh From b61e46c07009395f75950dc6724147d635871c43 Mon Sep 17 00:00:00 2001 From: Roberto Huertas Date: Wed, 31 Jul 2024 14:12:23 +0200 Subject: [PATCH 22/23] avoid contains + insert --- .../ide/configuration_file/comment_preserver/reconciler.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs index 26702870..c8554613 100644 --- a/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs +++ b/crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/comment_preserver/reconciler.rs @@ -95,10 +95,9 @@ fn extract_comments_from_node<'a>( visited: &mut HashSet>, ) { if node.kind() == "comment" { - if visited.contains(&node) { + if !visited.insert(node) { return; } - visited.insert(node); let start_byte = node.start_byte(); let end_byte = node.end_byte(); From 23ecab2190fef3ebe95304c49cdfc50a929522a6 Mon Sep 17 00:00:00 2001 From: Roberto Huertas Date: Wed, 31 Jul 2024 14:16:26 +0200 Subject: [PATCH 23/23] Update LICENSE-3rdparty.csv MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jacobo TarrĂ­o <158186053+jacobotb@users.noreply.github.com> --- LICENSE-3rdparty.csv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/LICENSE-3rdparty.csv b/LICENSE-3rdparty.csv index 84dcffcb..f5a551a6 100644 --- a/LICENSE-3rdparty.csv +++ b/LICENSE-3rdparty.csv @@ -13,7 +13,7 @@ num_cpus,https://github.com/seanmonstar/num_cpus,MIT, Copyright (c) 2015 Sean Mc percent-encoding,https://github.com/servo/rust-url/,MIT, Copyright (c) 2013-2022 The rust-url developers prettytable-rs,https://github.com/phsym/prettytable-rs/,BSD-3-Clause,Copyright (c) 2022 Pierre-Henri Symoneaux rayon,https://crates.io/crates/rayon,MIT,Copyright (c) 2010 The Rust Project Developers -pretty_yaml, https://github.com/g-plane/pretty_yaml, MIT, Copyright (c) 2024-present Pig Fang +pretty_yaml,https://github.com/g-plane/pretty_yaml,MIT,Copyright (c) 2024-present Pig Fang rocket,https://github.com/SergioBenitez/Rocket,Apache-2.0,Copyright 2016 Sergio Benitez tree-sitter,https://github.com/tree-sitter/tree-sitter,MIT,2014 Max Brunsfeld sarif-rs,https://github.com/psastras/sarif-rs,MIT,Copyright (c) 2021 Paul Sastrasinh