Skip to content

Commit 8ec7bba

Browse files
committed
fix(config): accept every config key a rule actually reads
Config validation read `default_config_section()`, the same table that backs `rumdl config --defaults` and `rumdl explain`. That table is a listing of defaults, so a key whose default cannot be written down is absent from it: an unset `Option` has no value a user could copy, and inventing one would read as a real setting. Those keys were therefore rejected as "Unknown option" even though the rule deserialized and honored them. MD001's `front-matter-title-pattern` worked and was warned about on every run. Validation now reads a separate `Rule::config_schema()`, derived from the same struct `from_config` deserializes, so the accepted keys cannot drift from the honored ones. A key with no representable default carries a sentinel there: the name is recognized while its type check is skipped. Sentinels contain a NUL byte and are unwritable as TOML, which is what keeps them out of the listing. The two halves are pinned against each other rather than spot-checked: every published default must be accepted by the validator, every schema key must survive a real load-and-validate, no user-facing table may carry a sentinel, and `rumdl config --defaults` must produce a config rumdl itself accepts. Reported in #794.
1 parent 78f4aa0 commit 8ec7bba

30 files changed

Lines changed: 436 additions & 276 deletions

docs/md001.md

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,48 @@ Prevents skipping heading levels (like jumping from # to ### without ##).
4444

4545
## Configuration
4646

47-
This rule has no configuration options.
47+
```toml
48+
[MD001]
49+
front-matter-title = true # Count a title in front matter as an implicit level 1 heading (default: true)
50+
front-matter-title-pattern = "^(title|header):" # Regex matching the front matter line that holds the title
51+
```
52+
53+
### `front-matter-title`
54+
55+
A document whose front matter carries a title starts at level 1 already, so its
56+
first body heading should be a level 2:
57+
58+
```markdown
59+
---
60+
title: Getting Started
61+
---
62+
63+
## Installation
64+
```
65+
66+
With `front-matter-title = false` the front matter is ignored and that document may
67+
open at any level.
68+
69+
### `front-matter-title-pattern`
70+
71+
Set this when the title lives under a different key. The pattern is matched against
72+
the front matter lines, and replaces the default `title:` lookup:
73+
74+
```toml
75+
[MD001]
76+
front-matter-title-pattern = "^header:"
77+
```
78+
79+
```markdown
80+
---
81+
header: Getting Started
82+
---
83+
84+
### Installation
85+
```
86+
87+
That jumps from the implicit level 1 to a level 3 and is flagged. An empty pattern
88+
means "no pattern": the default `title:` lookup applies.
4889

4990
## Automatic fixes
5091

src/config/registry.rs

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ impl RuleRegistry {
3939
let mut rule_aliases = std::collections::BTreeMap::new();
4040

4141
for rule in rules {
42-
let norm_name = if let Some((name, toml::Value::Table(mut table))) = rule.default_config_section() {
43-
let norm_name = normalize_key(&name); // Normalize the name from default_config_section
42+
let norm_name = if let Some((name, toml::Value::Table(mut table))) = rule.config_schema() {
43+
let norm_name = normalize_key(&name); // Normalize the name from config_schema
4444
// Overwrite polymorphic keys with the sentinel so the validator skips
4545
// type checking for fields whose deserializer accepts multiple TOML
4646
// types. The clean default is preserved for `rumdl config --defaults`
@@ -115,24 +115,26 @@ impl RuleRegistry {
115115
})
116116
}
117117

118-
/// Get the expected value type for a rule's configuration key, trying variants.
119-
/// Returns `None` for sentinel values (nullable Option fields, polymorphic fields
120-
/// that accept multiple TOML types), which signals the caller to skip type checking
121-
/// for that key while still recognizing the key as valid.
122-
pub fn expected_value_for(&self, rule: &str, key: &str) -> Option<&toml::Value> {
118+
/// Resolve a key as the user wrote it to the schema key it names, trying the rule's
119+
/// aliases and the separator/case variants.
120+
///
121+
/// Returns `None` when the rule does not accept the key at all. A key that resolves
122+
/// may still carry a sentinel value, so this answers "is this key known?" where
123+
/// [`RuleRegistry::expected_value_for`] answers "what type must it be?".
124+
pub fn canonical_config_key(&self, rule: &str, key: &str) -> Option<&str> {
123125
let schema = self.rule_schemas.get(rule)?;
124126

125127
// Check if this key is an alias
126128
if let Some(aliases) = self.rule_aliases.get(rule)
127129
&& let Some(canonical_key) = aliases.get(key)
128-
&& let Some(value) = schema.get(canonical_key)
130+
&& let Some((schema_key, _)) = schema.get_key_value(canonical_key)
129131
{
130-
return filter_type_check_sentinels(value);
132+
return Some(schema_key);
131133
}
132134

133135
// Try the original key
134-
if let Some(value) = schema.get(key) {
135-
return filter_type_check_sentinels(value);
136+
if let Some((schema_key, _)) = schema.get_key_value(key) {
137+
return Some(schema_key);
136138
}
137139

138140
// Try key variants
@@ -143,14 +145,25 @@ impl RuleRegistry {
143145
];
144146

145147
for variant in &key_variants {
146-
if let Some(value) = schema.get(variant) {
147-
return filter_type_check_sentinels(value);
148+
if let Some((schema_key, _)) = schema.get_key_value(variant) {
149+
return Some(schema_key);
148150
}
149151
}
150152

151153
None
152154
}
153155

156+
/// Get the expected value type for a rule's configuration key, trying variants.
157+
/// Returns `None` both for an unknown key and for sentinel values (nullable Option
158+
/// fields, polymorphic fields that accept multiple TOML types), which signals the
159+
/// caller to skip type checking while still recognizing the key as valid. Use
160+
/// [`RuleRegistry::canonical_config_key`] to tell those two cases apart.
161+
pub fn expected_value_for(&self, rule: &str, key: &str) -> Option<&toml::Value> {
162+
let schema = self.rule_schemas.get(rule)?;
163+
let canonical = self.canonical_config_key(rule, key)?;
164+
filter_type_check_sentinels(schema.get(canonical)?)
165+
}
166+
154167
/// Resolve any rule name (canonical or alias) to its canonical form
155168
/// Returns None if the rule name is not recognized
156169
///

src/rule.rs

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -237,10 +237,30 @@ pub trait Rule: DynClone + Send + Sync {
237237
/// Returns the rule name and default config table if the rule has config.
238238
/// If a rule implements this, it MUST be defined on the `impl Rule for ...` block,
239239
/// not just the inherent impl.
240+
///
241+
/// This is user-facing: it backs `rumdl config`, `rumdl config --defaults` and
242+
/// `rumdl explain`, so every value here must be a real default the user could write
243+
/// back into a config file. A key whose default cannot be written down (an unset
244+
/// `Option`) is therefore absent. Config *validation* reads [`Rule::config_schema`]
245+
/// instead, which keeps such keys.
240246
fn default_config_section(&self) -> Option<(String, toml::Value)> {
241247
None
242248
}
243249

250+
/// Returns the rule name and every config key the rule accepts, for validation.
251+
///
252+
/// A key with no representable default (an unset `Option`, or a deserializer that
253+
/// accepts several TOML types) carries a sentinel value: the key name is recognized
254+
/// while its type check is skipped. Sentinels contain a NUL byte and must never
255+
/// reach user-facing output, which is why this is separate from
256+
/// [`Rule::default_config_section`].
257+
///
258+
/// Defaults to the user-facing table, which is correct for a rule whose every key
259+
/// has a representable default.
260+
fn config_schema(&self) -> Option<(String, toml::Value)> {
261+
self.default_config_section()
262+
}
263+
244264
/// Returns config key aliases for this rule
245265
/// This allows rules to accept alternative config key names for backwards compatibility
246266
fn config_aliases(&self) -> Option<std::collections::HashMap<String, String>> {
@@ -252,9 +272,8 @@ pub trait Rule: DynClone + Send + Sync {
252272
/// default that can only encode one variant, so the validator would reject the
253273
/// alternative form. The registry replaces the schema entry for each listed key
254274
/// with a polymorphic sentinel so type checking is skipped while the key name
255-
/// is still validated. Keep `default_config_section()` returning clean defaults
256-
/// — the sentinel is a schema concern and must not leak into user-facing output
257-
/// like `rumdl config --defaults`.
275+
/// is still validated. The registry rewrites [`Rule::config_schema`], so
276+
/// `default_config_section()` keeps returning the clean user-facing default.
258277
fn polymorphic_config_keys(&self) -> &'static [&'static str] {
259278
&[]
260279
}

src/rule_config_serde.rs

Lines changed: 40 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ pub fn is_polymorphic_sentinel(value: &toml::Value) -> bool {
108108
/// Construct a polymorphic sentinel TOML value. Used by the `RuleRegistry` to overwrite
109109
/// schema entries for keys returned by `Rule::polymorphic_config_keys()`, so the validator
110110
/// skips the type check rather than flagging the alternative form as invalid. Rules must
111-
/// not call this from `default_config_section()` — the sentinel is a schema-only concern
111+
/// not call this from `default_config_section()`: a sentinel belongs to `config_schema()`
112112
/// and would leak into user-facing output (e.g. `rumdl config --defaults`).
113113
pub fn polymorphic_sentinel_value() -> toml::Value {
114114
toml::Value::String(POLYMORPHIC_SENTINEL.to_string())
@@ -136,12 +136,13 @@ pub fn config_schema_table<T: RuleConfig>(config: &T) -> Option<toml::map::Map<S
136136
Some(table)
137137
}
138138

139-
/// Default config section for a rule backed by a serde `RuleConfig` struct.
139+
/// User-facing default config section for a rule backed by a serde `RuleConfig` struct.
140140
///
141141
/// Serializes `T::default()` through the JSON→TOML path, which drops nullable
142-
/// (`None`) fields. Returns `None` when no fields remain. Rules whose
143-
/// `Option`-typed keys must stay visible to config validation use
144-
/// [`nullable_config_section_for`] instead.
142+
/// (`None`) fields: an unset `Option` has no default a user could write down, and
143+
/// inventing one (an empty string, a zero) would read as a real setting. Returns
144+
/// `None` when no fields remain. Config validation reads [`config_schema_for`], which
145+
/// keeps those keys.
145146
pub fn default_config_section_for<T: RuleConfig>() -> Option<(String, toml::Value)> {
146147
let json_value = serde_json::to_value(T::default()).ok()?;
147148
let toml_value = json_to_toml_value(&json_value)?;
@@ -151,44 +152,55 @@ pub fn default_config_section_for<T: RuleConfig>() -> Option<(String, toml::Valu
151152
}
152153
}
153154

154-
/// Default config section that keeps nullable (`None`) fields visible as
155-
/// schema sentinels, so key validation recognizes `Option`-typed settings.
156-
/// Returns `None` when no fields remain, like [`default_config_section_for`].
157-
pub fn nullable_config_section_for<T: RuleConfig>() -> Option<(String, toml::Value)> {
155+
/// Validation schema for a rule backed by a serde `RuleConfig` struct: every field the
156+
/// struct will deserialize, with nullable (`None`) fields kept as sentinels so key
157+
/// validation recognizes `Option`-typed settings. Returns `None` when the struct has no
158+
/// serialized fields, like [`default_config_section_for`].
159+
pub fn config_schema_for<T: RuleConfig>() -> Option<(String, toml::Value)> {
158160
let table = config_schema_table(&T::default())?;
159161
if table.is_empty() {
160162
return None;
161163
}
162164
Some((T::RULE_NAME.to_string(), toml::Value::Table(table)))
163165
}
164166

165-
/// Implements `default_config_section` and `from_config` for a rule backed by
166-
/// a serde `RuleConfig` struct. Use inside the rule's `impl Rule` block; the
167-
/// rule must provide a `from_config_struct(config)` constructor.
167+
/// Implements `config_schema` for a rule backed by a serde `RuleConfig` struct, so the
168+
/// set of keys config validation accepts is derived from the same struct `from_config`
169+
/// deserializes and cannot drift from it. Use inside the rule's `impl Rule` block.
168170
///
169-
/// The default arm drops nullable (`None`) fields from the config section;
170-
/// the `nullable` arm keeps them visible as schema sentinels so config
171-
/// validation recognizes `Option`-typed keys.
171+
/// Rules that also want the derived user-facing defaults use
172+
/// [`impl_rule_config_sections`]; this macro alone is for a rule that presents its
173+
/// defaults differently (ordering, commentary) but still deserializes through serde.
172174
#[macro_export]
173-
macro_rules! impl_rule_config_methods {
175+
macro_rules! impl_rule_config_schema {
176+
($config_ty:ty) => {
177+
fn config_schema(&self) -> Option<(String, toml::Value)> {
178+
$crate::rule_config_serde::config_schema_for::<$config_ty>()
179+
}
180+
};
181+
}
182+
183+
/// Implements `default_config_section` and `config_schema` for a rule backed by a serde
184+
/// `RuleConfig` struct. Use inside the rule's `impl Rule` block. Rules that also want
185+
/// the derived `from_config` use [`impl_rule_config_methods`].
186+
#[macro_export]
187+
macro_rules! impl_rule_config_sections {
174188
($config_ty:ty) => {
175189
fn default_config_section(&self) -> Option<(String, toml::Value)> {
176190
$crate::rule_config_serde::default_config_section_for::<$config_ty>()
177191
}
178192

179-
fn from_config(config: &$crate::config::Config) -> Box<dyn $crate::rule::Rule>
180-
where
181-
Self: Sized,
182-
{
183-
Box::new(Self::from_config_struct(
184-
$crate::rule_config_serde::load_rule_config::<$config_ty>(config),
185-
))
186-
}
193+
$crate::impl_rule_config_schema!($config_ty);
187194
};
188-
($config_ty:ty, nullable) => {
189-
fn default_config_section(&self) -> Option<(String, toml::Value)> {
190-
$crate::rule_config_serde::nullable_config_section_for::<$config_ty>()
191-
}
195+
}
196+
197+
/// Implements `default_config_section`, `config_schema` and `from_config` for a rule
198+
/// backed by a serde `RuleConfig` struct. Use inside the rule's `impl Rule` block; the
199+
/// rule must provide a `from_config_struct(config)` constructor.
200+
#[macro_export]
201+
macro_rules! impl_rule_config_methods {
202+
($config_ty:ty) => {
203+
$crate::impl_rule_config_sections!($config_ty);
192204

193205
fn from_config(config: &$crate::config::Config) -> Box<dyn $crate::rule::Rule>
194206
where

src/rules/md001_heading_increment.rs

Lines changed: 48 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,43 @@
11
use crate::HeadingStyle;
22
use crate::rule::{Fix, LintError, LintResult, LintWarning, Rule, RuleCategory, Severity};
3+
use crate::rule_config_serde::RuleConfig;
34
use crate::rules::front_matter_utils::FrontMatterUtils;
45
use crate::rules::heading_utils::HeadingUtils;
56
use crate::utils::range_utils::calculate_heading_range;
67
use regex::Regex;
8+
use serde::{Deserialize, Serialize};
9+
10+
/// Configuration for MD001 (Heading increment)
11+
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
12+
#[serde(rename_all = "kebab-case")]
13+
pub(super) struct MD001Config {
14+
/// Whether a title field in front matter counts as an implicit level-1 heading
15+
#[serde(default = "default_front_matter_title", alias = "front_matter_title")]
16+
pub front_matter_title: bool,
17+
18+
/// Regex matching the front matter line that holds the title. When set, it replaces
19+
/// the default `title:` lookup, so a document keying its title differently still
20+
/// gets the implicit level-1 heading.
21+
#[serde(default, alias = "front_matter_title_pattern")]
22+
pub front_matter_title_pattern: Option<String>,
23+
}
24+
25+
fn default_front_matter_title() -> bool {
26+
true
27+
}
28+
29+
impl Default for MD001Config {
30+
fn default() -> Self {
31+
Self {
32+
front_matter_title: default_front_matter_title(),
33+
front_matter_title_pattern: None,
34+
}
35+
}
36+
}
37+
38+
impl RuleConfig for MD001Config {
39+
const RULE_NAME: &'static str = "MD001";
40+
}
741

842
/// Rule MD001: Heading levels should only increment by one level at a time
943
///
@@ -128,6 +162,16 @@ impl MD001HeadingIncrement {
128162
}
129163
}
130164

165+
/// An empty pattern is read as "no pattern": compiled, it would match every line
166+
/// and turn any front matter at all into an implicit level-1 heading.
167+
fn from_config_struct(config: MD001Config, values_withheld: bool) -> Self {
168+
Self::with_pattern_from(
169+
config.front_matter_title,
170+
config.front_matter_title_pattern.filter(|p| !p.is_empty()),
171+
values_withheld,
172+
)
173+
}
174+
131175
/// Check if the document has a front matter title field
132176
fn has_front_matter_title(&self, content: &str) -> bool {
133177
if !self.front_matter_title {
@@ -291,44 +335,14 @@ impl Rule for MD001HeadingIncrement {
291335
where
292336
Self: Sized,
293337
{
294-
// Get MD001 config section
295-
let (front_matter_title, front_matter_title_pattern) = if let Some(rule_config) = config.rules.get("MD001") {
296-
let fmt = rule_config
297-
.values
298-
.get("front-matter-title")
299-
.or_else(|| rule_config.values.get("front_matter_title"))
300-
.and_then(toml::Value::as_bool)
301-
.unwrap_or(true);
302-
303-
let pattern = rule_config
304-
.values
305-
.get("front-matter-title-pattern")
306-
.or_else(|| rule_config.values.get("front_matter_title_pattern"))
307-
.and_then(|v| v.as_str())
308-
.filter(|s: &&str| !s.is_empty())
309-
.map(String::from);
310-
311-
(fmt, pattern)
312-
} else {
313-
(true, None)
314-
};
315-
316-
Box::new(MD001HeadingIncrement::with_pattern_from(
317-
front_matter_title,
318-
front_matter_title_pattern,
338+
let rule_config = crate::rule_config_serde::load_rule_config::<MD001Config>(config);
339+
Box::new(Self::from_config_struct(
340+
rule_config,
319341
config.withheld_rule_values.contains("MD001"),
320342
))
321343
}
322344

323-
fn default_config_section(&self) -> Option<(String, toml::Value)> {
324-
Some((
325-
"MD001".to_string(),
326-
toml::toml! {
327-
front-matter-title = true
328-
}
329-
.into(),
330-
))
331-
}
345+
crate::impl_rule_config_sections!(MD001Config);
332346
}
333347

334348
#[cfg(test)]

0 commit comments

Comments
 (0)