close
Skip to content

Commit 5db8f0c

Browse files
committed
fix(config): report the effective rule lists from config get global.*
Per-rule `enabled` outranks the global enable/disable lists, and the lists are canonicalized, so what a config file writes is not always what the run uses. `read_global_key` took only the sourced config, which made `[global] disable = ["MD013"]` plus `[MD013] enabled = true` report MD013 as disabled while the same config linted with it. The value now comes from the effective config and the provenance from the sourced record, which are the two different questions the output answers.
1 parent 51e26ea commit 5db8f0c

2 files changed

Lines changed: 114 additions & 33 deletions

File tree

‎src/commands/config.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ fn handle_config_get(key: &str, config_path: Option<&str>, no_config: bool, inli
105105

106106
// Handle GLOBAL keys
107107
if section_part.eq_ignore_ascii_case("global") {
108-
match rumdl_config::read_global_key(&sourced.global, &normalized_field) {
108+
match rumdl_config::read_global_key(&final_config.global, &sourced.global, &normalized_field) {
109109
Some(rumdl_config::GlobalKeyValue::Set(value, source)) => {
110110
print_config_value("global", &normalized_field, &value, source);
111111
}

‎src/config/global_keys.rs‎

Lines changed: 113 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use std::str::FromStr;
1717
use super::flavor::{MarkdownFlavor, normalize_key};
1818
use super::registry::RuleRegistry;
1919
use super::source_tracking::{ConfigSource, SourcedGlobalConfig, SourcedValue};
20+
use super::types::GlobalConfig;
2021
use crate::types::LineLength;
2122

2223
/// Global configuration keys that hold plain values (normalized kebab-case).
@@ -60,42 +61,60 @@ pub enum GlobalKeyValue {
6061
/// The read half of [`apply_global_key`], kept beside it so the two cannot drift:
6162
/// every key in [`GLOBAL_VALUE_KEYS`] answers here. Returns `None` only for a key
6263
/// that is not a global setting at all.
63-
pub fn read_global_key(global: &SourcedGlobalConfig, norm_key: &str) -> Option<GlobalKeyValue> {
64-
let strings = |sv: &SourcedValue<Vec<String>>| {
64+
///
65+
/// The value comes from `effective` and the provenance from `sourced`, because the
66+
/// two answer different questions. `sourced` records what the config files said;
67+
/// `effective` is what the run actually uses, after
68+
/// [`Config::apply_per_rule_enabled`](crate::config::Config::apply_per_rule_enabled)
69+
/// has folded per-rule `enabled` into the rule lists and after those lists have been
70+
/// canonicalized. Reporting the sourced value would tell a user that MD013 is
71+
/// disabled while the very same config runs it.
72+
pub fn read_global_key(
73+
effective: &GlobalConfig,
74+
sourced: &SourcedGlobalConfig,
75+
norm_key: &str,
76+
) -> Option<GlobalKeyValue> {
77+
let strings = |value: &[String], sv: &SourcedValue<Vec<String>>| {
6578
GlobalKeyValue::Set(
66-
toml::Value::Array(sv.value.iter().map(|s| toml::Value::String(s.clone())).collect()),
79+
toml::Value::Array(value.iter().map(|s| toml::Value::String(s.clone())).collect()),
6780
sv.source,
6881
)
6982
};
70-
let boolean = |sv: &SourcedValue<bool>| GlobalKeyValue::Set(toml::Value::Boolean(sv.value), sv.source);
71-
let optional_string = |slot: &Option<SourcedValue<String>>| match slot {
72-
Some(sv) => GlobalKeyValue::Set(toml::Value::String(sv.value.clone()), sv.source),
83+
let boolean = |value: bool, sv: &SourcedValue<bool>| GlobalKeyValue::Set(toml::Value::Boolean(value), sv.source);
84+
let optional_string = |value: &Option<String>, slot: &Option<SourcedValue<String>>| match value {
85+
Some(value) => GlobalKeyValue::Set(
86+
toml::Value::String(value.clone()),
87+
slot.as_ref().map_or(ConfigSource::Default, |sv| sv.source),
88+
),
7389
None => GlobalKeyValue::Unset,
7490
};
7591

7692
Some(match norm_key {
77-
"enable" => strings(&global.enable),
78-
"disable" => strings(&global.disable),
79-
"include" => strings(&global.include),
80-
"exclude" => strings(&global.exclude),
81-
"extend-enable" => strings(&global.extend_enable),
82-
"extend-disable" => strings(&global.extend_disable),
83-
"fixable" => strings(&global.fixable),
84-
"unfixable" => strings(&global.unfixable),
85-
"respect-gitignore" => boolean(&global.respect_gitignore),
86-
"force-exclude" => boolean(&global.force_exclude),
87-
"cache" => boolean(&global.cache),
88-
"editorconfig" => boolean(&global.editorconfig),
93+
"enable" => strings(&effective.enable, &sourced.enable),
94+
"disable" => strings(&effective.disable, &sourced.disable),
95+
"include" => strings(&effective.include, &sourced.include),
96+
"exclude" => strings(&effective.exclude, &sourced.exclude),
97+
"extend-enable" => strings(&effective.extend_enable, &sourced.extend_enable),
98+
"extend-disable" => strings(&effective.extend_disable, &sourced.extend_disable),
99+
"fixable" => strings(&effective.fixable, &sourced.fixable),
100+
"unfixable" => strings(&effective.unfixable, &sourced.unfixable),
101+
"respect-gitignore" => boolean(effective.respect_gitignore, &sourced.respect_gitignore),
102+
"force-exclude" => {
103+
// The field is deprecated and inert, but it is still a key a config may
104+
// carry, so `config get` answers for it rather than calling it unknown.
105+
#[allow(deprecated)]
106+
let value = effective.force_exclude;
107+
boolean(value, &sourced.force_exclude)
108+
}
109+
"cache" => boolean(effective.cache, &sourced.cache),
110+
"editorconfig" => boolean(effective.editorconfig, &sourced.editorconfig),
89111
"line-length" => GlobalKeyValue::Set(
90-
toml::Value::Integer(global.line_length.value.get() as i64),
91-
global.line_length.source,
92-
),
93-
"output-format" => optional_string(&global.output_format),
94-
"cache-dir" => optional_string(&global.cache_dir),
95-
"flavor" => GlobalKeyValue::Set(
96-
toml::Value::String(global.flavor.value.to_string()),
97-
global.flavor.source,
112+
toml::Value::Integer(effective.line_length.get() as i64),
113+
sourced.line_length.source,
98114
),
115+
"output-format" => optional_string(&effective.output_format, &sourced.output_format),
116+
"cache-dir" => optional_string(&effective.cache_dir, &sourced.cache_dir),
117+
"flavor" => GlobalKeyValue::Set(toml::Value::String(effective.flavor.to_string()), sourced.flavor.source),
99118
_ => return None,
100119
})
101120
}
@@ -245,6 +264,18 @@ mod tests {
245264
(global, outcome)
246265
}
247266

267+
/// The effective config a sourced one produces, built through the same conversion
268+
/// the CLI uses rather than by mirroring fields here, so a field this test reads
269+
/// cannot silently stop tracking the real one.
270+
fn effective(sourced: &SourcedGlobalConfig) -> GlobalConfig {
271+
let sourced = crate::config::SourcedConfig {
272+
global: sourced.clone(),
273+
..Default::default()
274+
};
275+
let config: crate::config::Config = sourced.into_validated_unchecked().into();
276+
config.global
277+
}
278+
248279
#[test]
249280
fn every_global_key_is_recognized() {
250281
// The key list and the dispatch must stay in lockstep: every listed
@@ -265,19 +296,21 @@ mod tests {
265296
// The read half must cover the same key list as the write half, or
266297
// `rumdl config get global.<key>` calls a real setting unknown.
267298
let global = SourcedGlobalConfig::default();
299+
let config = effective(&global);
268300
for key in GLOBAL_VALUE_KEYS {
269301
assert!(
270-
read_global_key(&global, key).is_some(),
302+
read_global_key(&config, &global, key).is_some(),
271303
"key '{key}' is listed but cannot be read back"
272304
);
273305
}
274-
assert!(read_global_key(&global, "not-a-key").is_none());
306+
assert!(read_global_key(&config, &global, "not-a-key").is_none());
275307
}
276308

277309
#[test]
278310
fn a_set_value_reads_back_with_its_provenance() {
279311
let (global, _) = apply("line-length", &toml::Value::Integer(120));
280-
let Some(GlobalKeyValue::Set(value, source)) = read_global_key(&global, "line-length") else {
312+
let Some(GlobalKeyValue::Set(value, source)) = read_global_key(&effective(&global), &global, "line-length")
313+
else {
281314
panic!("a set line-length must read back as Set");
282315
};
283316
assert_eq!(value, toml::Value::Integer(120));
@@ -288,21 +321,69 @@ mod tests {
288321
fn an_unset_optional_key_reads_back_as_unset_not_missing() {
289322
let global = SourcedGlobalConfig::default();
290323
assert!(matches!(
291-
read_global_key(&global, "output-format"),
324+
read_global_key(&effective(&global), &global, "output-format"),
292325
Some(GlobalKeyValue::Unset)
293326
));
294327
assert!(matches!(
295-
read_global_key(&global, "cache-dir"),
328+
read_global_key(&effective(&global), &global, "cache-dir"),
296329
Some(GlobalKeyValue::Unset)
297330
));
298331

299332
let (global, _) = apply("output-format", &toml::Value::String("json".to_string()));
300333
assert!(matches!(
301-
read_global_key(&global, "output-format"),
334+
read_global_key(&effective(&global), &global, "output-format"),
302335
Some(GlobalKeyValue::Set(toml::Value::String(_), _))
303336
));
304337
}
305338

339+
#[test]
340+
fn a_rule_list_reads_back_as_the_run_will_use_it() {
341+
// `[global] disable = ["MD013"]` with `[MD013] enabled = true` runs MD013:
342+
// per-rule `enabled` outranks the global list. Reporting the list as the config
343+
// file wrote it would name a rule as disabled while the same config lints with
344+
// it, so the read reports the list the run actually uses.
345+
let (global, _) = apply(
346+
"disable",
347+
&toml::Value::Array(vec![toml::Value::String("MD013".to_string())]),
348+
);
349+
let mut sourced = crate::config::SourcedConfig {
350+
global,
351+
..Default::default()
352+
};
353+
sourced.rules.entry("MD013".to_string()).or_default().values.insert(
354+
"enabled".to_string(),
355+
SourcedValue::new(toml::Value::Boolean(true), ConfigSource::ProjectConfig),
356+
);
357+
let config: crate::config::Config = sourced.clone().into_validated_unchecked().into();
358+
359+
let Some(GlobalKeyValue::Set(disabled, _)) = read_global_key(&config.global, &sourced.global, "disable") else {
360+
panic!("disable must read back as Set");
361+
};
362+
assert_eq!(
363+
disabled,
364+
toml::Value::Array(vec![]),
365+
"MD013 is enabled by its own section, so it is not in the effective disable list"
366+
);
367+
368+
// Control: without the per-rule override the rule stays disabled and listed.
369+
let (global, _) = apply(
370+
"disable",
371+
&toml::Value::Array(vec![toml::Value::String("MD013".to_string())]),
372+
);
373+
let sourced = crate::config::SourcedConfig {
374+
global,
375+
..Default::default()
376+
};
377+
let config: crate::config::Config = sourced.clone().into_validated_unchecked().into();
378+
let Some(GlobalKeyValue::Set(disabled, _)) = read_global_key(&config.global, &sourced.global, "disable") else {
379+
panic!("disable must read back as Set");
380+
};
381+
assert_eq!(
382+
disabled,
383+
toml::Value::Array(vec![toml::Value::String("MD013".to_string())])
384+
);
385+
}
386+
306387
#[test]
307388
fn applies_values_with_origin() {
308389
let (global, outcome) = apply("line-length", &toml::Value::Integer(120));

0 commit comments

Comments
 (0)