Skip to content

Commit 400b164

Browse files
jasonforaljasonforal
committed
Refactor StaticAnalysisConfigFile to handle both v1 and v2 configurations
1 parent 6169fbe commit 400b164

3 files changed

Lines changed: 161 additions & 81 deletions

File tree

‎crates/bins/src/bin/datadog_static_analyzer_server/ide/configuration_file/static_analysis_config_file.rs‎

Lines changed: 147 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,37 @@ use super::comment_preserver::{prettify_yaml, reconcile_comments};
22
use super::error::ConfigFileError;
33
use indexmap::IndexMap;
44
use itertools::Itertools;
5-
use kernel::config::common::{PathConfig, PathPattern, RuleConfig, RulesetConfig};
6-
use kernel::config::file_v1;
7-
use kernel::config::file_v1::{config_file_to_yaml, parse_config_file};
5+
use kernel::config::common::{
6+
parse_any_schema_yaml, ConfigError, PathConfig, PathPattern, RuleConfig, RulesetConfig,
7+
WithVersion,
8+
};
9+
use kernel::config::file_v1::config_file_to_yaml;
10+
use kernel::config::{file_v1, file_v2};
811
use kernel::utils::decode_base64_string;
9-
use std::{borrow::Cow, fmt::Debug, ops::Deref};
12+
use std::{borrow::Cow, fmt::Debug};
1013
use tracing::instrument;
1114

12-
#[derive(Debug, Default, Clone, PartialEq)]
15+
const WILDCARD_IGNORE: &str = "**";
16+
17+
#[derive(Debug, Clone, PartialEq)]
1318
pub struct StaticAnalysisConfigFile {
14-
config_file: file_v1::ConfigFile,
19+
config_file: WithVersion<file_v1::ConfigFile, file_v2::YamlConfigFile>,
1520
original_content: Option<String>,
1621
}
1722

23+
impl Default for StaticAnalysisConfigFile {
24+
fn default() -> Self {
25+
Self {
26+
config_file: WithVersion::V1(Default::default()),
27+
original_content: None,
28+
}
29+
}
30+
}
31+
1832
impl From<file_v1::ConfigFile> for StaticAnalysisConfigFile {
1933
fn from(value: file_v1::ConfigFile) -> Self {
2034
Self {
21-
config_file: value,
35+
config_file: WithVersion::V1(value),
2236
original_content: None,
2337
}
2438
}
@@ -28,30 +42,44 @@ impl TryFrom<String> for StaticAnalysisConfigFile {
2842
type Error = ConfigFileError;
2943

3044
fn try_from(base64_str: String) -> Result<Self, Self::Error> {
45+
use serde::de::Error;
3146
let content = decode_base64_string(base64_str)?;
3247
if content.trim().is_empty() {
3348
return Ok(Self::default());
3449
}
35-
let config = parse_config_file(&content)?;
50+
let parsed = parse_any_schema_yaml(&content).map_err(|err| {
51+
match err {
52+
// Artificially represent this as a "parse" error for backwards compatibility.
53+
ConfigError::UnsupportedSchema(_) => ConfigFileError::Parser {
54+
source: serde_yaml::Error::custom(err),
55+
},
56+
ConfigError::Parse(err) => ConfigFileError::Parser { source: err },
57+
}
58+
})?;
59+
let config_file = match parsed {
60+
WithVersion::V1(yaml) => WithVersion::V1(file_v1::ConfigFile::from(yaml)),
61+
WithVersion::V2(yaml) => WithVersion::V2(yaml),
62+
};
3663
Ok(Self {
37-
config_file: config,
64+
config_file,
3865
original_content: Some(content),
3966
})
4067
}
4168
}
4269

43-
impl Deref for StaticAnalysisConfigFile {
44-
type Target = file_v1::ConfigFile;
45-
fn deref(&self) -> &Self::Target {
46-
&self.config_file
47-
}
70+
/// Returns a vec representing an ignored path (via the `**` glob).
71+
fn create_ignored_path() -> Vec<String> {
72+
vec![WILDCARD_IGNORE.to_string()]
4873
}
4974

5075
fn create_ignored_pattern() -> Vec<PathPattern> {
51-
vec![PathPattern {
52-
prefix: "**".into(),
53-
glob: None,
54-
}]
76+
create_ignored_path()
77+
.into_iter()
78+
.map(|path_str| PathPattern {
79+
prefix: std::path::PathBuf::from(path_str),
80+
glob: None,
81+
})
82+
.collect()
5583
}
5684

5785
fn create_ignored_rule() -> RuleConfig {
@@ -113,32 +141,48 @@ impl StaticAnalysisConfigFile {
113141

114142
#[instrument(skip(self))]
115143
pub fn ignore_rule(&mut self, rule: Cow<str>) {
116-
let config = &mut self.config_file;
117-
if let Some((ruleset_name, rule_name)) = rule.split_once('/') {
118-
// the ruleset may exist and contain other rules so we
119-
// can't update it blindly
120-
if let Some(existing_ruleset) = config.rulesets.get_mut(ruleset_name) {
121-
// if the rule already exists we need to see if the rule was already present.
122-
// if that's the case, we need to keep the old properties
123-
if let Some(existing_rule) = existing_ruleset.rules.get_mut(rule_name) {
124-
existing_rule.paths.ignore = create_ignored_pattern();
144+
let Some((ruleset_name, rule_name)) = rule.split_once('/') else {
145+
return;
146+
};
147+
match &mut self.config_file {
148+
WithVersion::V1(config) => {
149+
// the ruleset may exist and contain other rules so we
150+
// can't update it blindly
151+
if let Some(existing_ruleset) = config.rulesets.get_mut(ruleset_name) {
152+
// if the rule already exists we need to see if the rule was already present.
153+
// if that's the case, we need to keep the old properties
154+
if let Some(existing_rule) = existing_ruleset.rules.get_mut(rule_name) {
155+
existing_rule.paths.ignore = create_ignored_pattern();
156+
} else {
157+
existing_ruleset
158+
.rules
159+
.insert(rule_name.to_string(), create_ignored_rule());
160+
}
125161
} else {
126-
existing_ruleset
127-
.rules
128-
.insert(rule_name.to_string(), create_ignored_rule());
162+
// we can add the new ruleset
163+
let mut rules_to_ignore = IndexMap::new();
164+
rules_to_ignore.insert(rule_name.to_string(), create_ignored_rule());
165+
166+
config.rulesets.insert(
167+
ruleset_name.to_string(),
168+
RulesetConfig {
169+
rules: rules_to_ignore,
170+
..Default::default()
171+
},
172+
);
129173
}
130-
} else {
131-
// we can add the new ruleset
132-
let mut rules_to_ignore = IndexMap::new();
133-
rules_to_ignore.insert(rule_name.to_string(), create_ignored_rule());
134-
135-
config.rulesets.insert(
136-
ruleset_name.to_string(),
137-
RulesetConfig {
138-
rules: rules_to_ignore,
139-
..Default::default()
140-
},
141-
);
174+
}
175+
WithVersion::V2(config) => {
176+
// (All logic for this is translated from the V1 match arm)
177+
let map = config.ruleset_configs.get_or_insert_default();
178+
let ruleset_config = map.0.entry(ruleset_name.to_string()).or_default();
179+
let rule_config = ruleset_config
180+
.rule_configs
181+
.get_or_insert_default()
182+
.0
183+
.entry(rule_name.to_string())
184+
.or_default();
185+
rule_config.path_config.ignore_paths = Some(create_ignored_path());
142186
}
143187
}
144188
}
@@ -193,12 +237,29 @@ impl StaticAnalysisConfigFile {
193237

194238
#[instrument(skip(self))]
195239
pub fn add_rulesets(&mut self, rulesets: &[impl AsRef<str> + Debug]) {
196-
let config = &mut self.config_file;
197-
for ruleset in rulesets {
198-
if !config.rulesets.contains_key(ruleset.as_ref()) {
199-
config
200-
.rulesets
201-
.insert(ruleset.as_ref().to_owned(), RulesetConfig::default());
240+
match &mut self.config_file {
241+
WithVersion::V1(config) => {
242+
for ruleset in rulesets {
243+
if !config.rulesets.contains_key(ruleset.as_ref()) {
244+
config
245+
.rulesets
246+
.insert(ruleset.as_ref().to_owned(), RulesetConfig::default());
247+
}
248+
}
249+
}
250+
WithVersion::V2(config) => {
251+
if rulesets.is_empty() {
252+
return;
253+
}
254+
let list = config
255+
.use_rulesets
256+
.get_or_insert_with(|| Vec::with_capacity(rulesets.len()));
257+
for ruleset_name in rulesets {
258+
// if list.iter().find(|&name| ruleset_name.as_ref() ==)
259+
if !list.iter().any(|name| ruleset_name.as_ref() == name) {
260+
list.push(ruleset_name.as_ref().to_string())
261+
}
262+
}
202263
}
203264
}
204265
}
@@ -224,19 +285,32 @@ impl StaticAnalysisConfigFile {
224285
/// ```
225286
#[instrument]
226287
pub fn to_rulesets(config_content_base64: String) -> Vec<String> {
227-
let config = match Self::try_from(config_content_base64) {
288+
let parsed = match Self::try_from(config_content_base64) {
228289
Ok(config) => config,
229290
Err(e) => {
230291
tracing::error!(error =?e, "Error trying to parse config file");
231292
return vec![];
232293
}
233294
};
234-
config.rulesets.iter().map(|rs| rs.0.clone()).collect()
295+
match parsed.config_file {
296+
WithVersion::V1(config) => config.rulesets.iter().map(|rs| rs.0.clone()).collect(),
297+
WithVersion::V2(config) => config.use_rulesets.clone().unwrap_or_default(),
298+
}
235299
}
236300

237301
#[instrument(skip(self))]
238302
pub fn is_onboarding_allowed(&self) -> bool {
239-
self.paths.only.is_none() && self.paths.ignore.is_empty()
303+
match &self.config_file {
304+
WithVersion::V1(config) => {
305+
config.paths.only.is_none() && config.paths.ignore.is_empty()
306+
}
307+
WithVersion::V2(config) => {
308+
config.global_config.is_none()
309+
|| config.global_config.as_ref().is_some_and(|c| {
310+
c.path_config.ignore_paths.is_none() && c.path_config.only_paths.is_none()
311+
})
312+
}
313+
}
240314
}
241315

242316
/// Serializes the `StaticAnalysisConfigFile` into a YAML string.
@@ -251,31 +325,34 @@ impl StaticAnalysisConfigFile {
251325
/// Returns a `ConfigFileError` if something goes wrong.
252326
#[instrument(skip(self))]
253327
pub fn to_string(&self) -> Result<String, ConfigFileError> {
254-
let str = config_file_to_yaml(self)?;
255-
// fix null maps, note that str will not have comments and it will be using the default serde format.
256-
let fixed = str
257-
.lines()
258-
.map(|l| {
259-
if l.ends_with(": null") {
260-
l.replace(": null", ":")
261-
} else {
262-
l.to_string()
263-
}
264-
})
265-
.join("\n");
266-
328+
let yaml = match &self.config_file {
329+
WithVersion::V1(config) => {
330+
let str = config_file_to_yaml(config)?;
331+
// fix null maps, note that str will not have comments and it will be using the default serde format.
332+
str.lines()
333+
.map(|l| {
334+
if l.ends_with(": null") {
335+
l.replace(": null", ":")
336+
} else {
337+
l.to_string()
338+
}
339+
})
340+
.join("\n")
341+
}
342+
WithVersion::V2(config) => serde_yaml::to_string(&config)?,
343+
};
267344
// reconcile and format
268345
// NOTE: if this fails, we're going to return the content
269346
// and swallow the error.
270347
self.original_content
271348
.as_ref()
272349
.map_or_else(
273-
|| prettify_yaml(&fixed),
274-
|original_content| reconcile_comments(original_content, &fixed, true),
350+
|| prettify_yaml(&yaml),
351+
|original_content| reconcile_comments(original_content, &yaml, true),
275352
)
276353
.or_else(|e| {
277354
tracing::debug!(error = ?e, "Reconciliation or formatting error: {}", e.to_string());
278-
Ok::<String, ConfigFileError>(fixed)
355+
Ok::<String, ConfigFileError>(yaml)
279356
})
280357
}
281358
}
@@ -853,3 +930,6 @@ rulesets:
853930
assert_eq!(config.trim(), expected.trim());
854931
}
855932
}
933+
934+
// test behaviors
935+
// add empty rulesets to v2 doesn't instantiate a use_rulesets

‎crates/static-analysis-kernel/src/config/file_v1.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,7 @@ impl<'de> Deserialize<'de> for YamlRuleCategory {
495495
// A map from string to value that disallows repeated keys when deserializing.
496496
#[derive(Debug, Clone, Serialize, Default, PartialEq, Eq)]
497497
#[serde(transparent)]
498-
pub(crate) struct UniqueKeyMap<V>(pub(crate) IndexMap<String, V>);
498+
pub struct UniqueKeyMap<V>(pub IndexMap<String, V>);
499499

500500
impl<V> UniqueKeyMap<V> {
501501
fn is_empty(&self) -> bool {

‎crates/static-analysis-kernel/src/config/file_v2.rs‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,13 @@ pub struct YamlConfigFile {
2828
#[serde(skip_serializing_if = "Option::is_none")]
2929
pub(crate) use_default_rulesets: Option<bool>,
3030
#[serde(skip_serializing_if = "Option::is_none")]
31-
pub(crate) use_rulesets: Option<Vec<String>>,
31+
pub use_rulesets: Option<Vec<String>>,
3232
#[serde(skip_serializing_if = "Option::is_none")]
3333
pub(crate) ignore_rulesets: Option<Vec<String>>,
3434
#[serde(skip_serializing_if = "Option::is_none")]
35-
pub(crate) ruleset_configs: Option<UniqueKeyMap<YamlRulesetConfig>>,
35+
pub ruleset_configs: Option<UniqueKeyMap<YamlRulesetConfig>>,
3636
#[serde(skip_serializing_if = "Option::is_none")]
37-
pub(crate) global_config: Option<YamlGlobalConfig>,
37+
pub global_config: Option<YamlGlobalConfig>,
3838
}
3939

4040
#[derive(Debug, Clone, PartialEq)]
@@ -64,9 +64,9 @@ impl From<YamlConfigFile> for ConfigFile {
6464
#[derive(Debug, Clone, Deserialize, Serialize, Default, Eq, PartialEq)]
6565
#[serde(rename_all = "kebab-case")]
6666
#[serde(deny_unknown_fields)]
67-
pub(crate) struct YamlGlobalConfig {
67+
pub struct YamlGlobalConfig {
6868
#[serde(flatten)]
69-
pub(crate) path_config: YamlPathConfig,
69+
pub path_config: YamlPathConfig,
7070
#[serde(skip_serializing_if = "Option::is_none")]
7171
pub(crate) use_gitignore: Option<bool>,
7272
#[serde(skip_serializing_if = "Option::is_none")]
@@ -97,11 +97,11 @@ impl From<YamlGlobalConfig> for GlobalConfig {
9797
#[derive(Debug, Clone, Deserialize, Serialize, Default, PartialEq)]
9898
#[serde(rename_all = "kebab-case")]
9999
#[serde(deny_unknown_fields)]
100-
pub(crate) struct YamlRulesetConfig {
100+
pub struct YamlRulesetConfig {
101101
#[serde(flatten)]
102-
pub(crate) path_config: YamlPathConfig,
102+
pub path_config: YamlPathConfig,
103103
#[serde(skip_serializing_if = "Option::is_none")]
104-
pub(crate) rule_configs: Option<UniqueKeyMap<YamlRuleConfig>>,
104+
pub rule_configs: Option<UniqueKeyMap<YamlRuleConfig>>,
105105
}
106106

107107
impl From<YamlRulesetConfig> for RulesetConfig {
@@ -122,9 +122,9 @@ impl From<YamlRulesetConfig> for RulesetConfig {
122122
#[derive(Debug, Clone, Deserialize, Serialize, Default, PartialEq)]
123123
#[serde(rename_all = "kebab-case")]
124124
#[serde(deny_unknown_fields)]
125-
pub(crate) struct YamlRuleConfig {
125+
pub struct YamlRuleConfig {
126126
#[serde(flatten)]
127-
pub(crate) path_config: YamlPathConfig,
127+
pub path_config: YamlPathConfig,
128128
#[serde(skip_serializing_if = "Option::is_none")]
129129
pub(crate) arguments: Option<UniqueKeyMap<file_v1::YamlBySubtree<AnyAsString>>>,
130130
#[serde(skip_serializing_if = "Option::is_none")]
@@ -154,11 +154,11 @@ impl From<YamlRuleConfig> for RuleConfig {
154154
#[derive(Debug, Clone, Deserialize, Serialize, Default, PartialEq, Eq)]
155155
#[serde(rename_all = "kebab-case")]
156156
#[serde(deny_unknown_fields)]
157-
pub(crate) struct YamlPathConfig {
157+
pub struct YamlPathConfig {
158158
#[serde(skip_serializing_if = "Option::is_none")]
159-
pub(crate) only_paths: Option<Vec<String>>,
159+
pub only_paths: Option<Vec<String>>,
160160
#[serde(skip_serializing_if = "Option::is_none")]
161-
pub(crate) ignore_paths: Option<Vec<String>>,
161+
pub ignore_paths: Option<Vec<String>>,
162162
}
163163

164164
impl From<YamlPathConfig> for Option<PathConfig> {

0 commit comments

Comments
 (0)