From 043cf19241cf03a795de9f3339c05b0f8424631e Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Fri, 3 Jul 2026 11:37:43 +0300 Subject: [PATCH 01/15] feat(core): mds fmt formatting engine (format_str/format_str_with) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a span-guided, line-oriented source rewriter (never a token/AST reconstructor, since the lexer is lossy) implementing a conservative R1-R4 ruleset: CRLF/CR removal, exactly-one-final-newline, 3+ blank-line-run collapsing matching clean_output's own algorithm exactly, and directive trailing-whitespace stripping. A runtime safety gate re-compiles both the original and formatted source and returns MdsError::FormatterInvariant on any divergence rather than silently writing a non-equivalent result. Two invariants were verified empirically against the live compiler rather than assumed from prose, and reshaped the ruleset: - A whitespace-only "blank" line is NOT inert mid-document (clean_output only fully strips leading/trailing runs, and mid-stream only collapses truly-empty newline runs) — so blank-line whitespace stripping was dropped as unsound; body-text trailing whitespace (Markdown hard breaks) is never touched. - @message/@define bodies bypass clean_output entirely (built via `evaluate_nodes(...).trim()`, no newline-run capping, no \r removal), so they get a third "raw content" region alongside frontmatter/code fences — copied byte-for-byte, including nested code fences. Task: feat/60-mds-fmt-auto-formatter --- crates/mds-core/src/error.rs | 23 +- crates/mds-core/src/formatter.rs | 732 +++++++++++++++++++++++++++ crates/mds-core/src/lib.rs | 2 + crates/mds-core/tests/api_surface.rs | 22 +- crates/mds-core/tests/fmt.rs | 551 ++++++++++++++++++++ 5 files changed, 1328 insertions(+), 2 deletions(-) create mode 100644 crates/mds-core/src/formatter.rs create mode 100644 crates/mds-core/tests/fmt.rs diff --git a/crates/mds-core/src/error.rs b/crates/mds-core/src/error.rs index f312b7b..335c422 100644 --- a/crates/mds-core/src/error.rs +++ b/crates/mds-core/src/error.rs @@ -331,6 +331,18 @@ pub enum MdsError { #[error("expected messages output, but template produced markdown")] #[diagnostic(code(mds::expected_messages))] ExpectedMessages, + + /// The formatter's rewritten source failed the compile-equivalence safety + /// gate: either the formatted source failed to compile when the original + /// did, or the two compiled to different output. This signals a formatter + /// bug, not a problem with the input template — the CLI must not write the + /// file when this occurs. + #[error("formatter produced non-equivalent output: {detail}")] + #[diagnostic( + code(mds::formatter_invariant), + help("this indicates a bug in `mds fmt` itself; please file an issue") + )] + FormatterInvariant { detail: String }, } impl MdsError { @@ -671,6 +683,14 @@ impl MdsError { MdsError::NotMdsFile { path: path.into() } } + /// Construct a `FormatterInvariant` error, signaling that the formatter's + /// rewritten source failed the compile-equivalence safety gate. + pub(crate) fn formatter_invariant(detail: impl Into) -> Self { + MdsError::FormatterInvariant { + detail: detail.into(), + } + } + /// Construct a `MixedContent` error whose span points at the offending /// top-level prose / interpolation. /// @@ -745,7 +765,8 @@ impl MdsError { | MdsError::YamlError { .. } | MdsError::JsonError { .. } | MdsError::ExpectedMarkdown - | MdsError::ExpectedMessages => None, + | MdsError::ExpectedMessages + | MdsError::FormatterInvariant { .. } => None, }; SerializedError { diff --git a/crates/mds-core/src/formatter.rs b/crates/mds-core/src/formatter.rs new file mode 100644 index 0000000..ab84ac1 --- /dev/null +++ b/crates/mds-core/src/formatter.rs @@ -0,0 +1,732 @@ +//! `mds fmt` engine — an opinionated, safety-gated auto-formatter for MDS source. +//! +//! # The load-bearing constraint +//! +//! MDS is a *content language*: the formatter MUST produce byte-identical compile +//! output (`compile(fmt(src)).output == compile(src).output`) and MUST be +//! idempotent (`fmt(fmt(src)) == fmt(src)`). The token stream is lossy (the lexer +//! trims interpolation inner whitespace, discards the newline after a directive +//! line / code fence, and filters frontmatter `\r`) so source can never be +//! reconstructed from tokens/AST alone without inventing or dropping whitespace +//! the author actually wrote. This module therefore rewrites the ORIGINAL SOURCE +//! STRING directly, line by line, copying any byte without a proven-safe rule +//! verbatim — it never reconstructs from tokens. +//! +//! # Approach +//! +//! 1. Tokenize the source (surfaces syntax errors; reuses the lexer's fence / +//! frontmatter state machine rather than re-implementing it). +//! 2. Compute *protected* byte ranges (frontmatter + code fence regions, derived +//! from consecutive token offsets) and the set of *directive line* start +//! offsets. +//! 3. Rewrite the source in a single left-to-right, line-oriented pass, applying +//! only rules that are provably output-preserving (R1-R4 below). +//! 4. Run a runtime safety gate (`assert_equivalent`) that re-compiles both the +//! original and formatted source and hard-errors (`MdsError::FormatterInvariant`) +//! on any divergence, rather than silently returning a wrong result. +//! +//! # Ruleset (v1) +//! +//! - **R1 (CRLF/CR removal)** — applied to the WHOLE file, including protected +//! regions. This is the one transform allowed to cross a protected boundary: +//! `clean_output` strips every `\r` from the final compiled string regardless +//! of where it came from, frontmatter lexing already filters `\r` out of its +//! captured content, and directive matching trims `\r` before comparison — so +//! no downstream consumer of a `\r` byte can ever observe it. Removing it from +//! the source up front cannot change compiled output. +//! - **R2 (exactly one final newline)** — empty or whitespace-only input becomes +//! `""`, matching `clean_output`'s own leading/trailing-trim behavior (verified +//! against the live compiler: `clean_output(" \n") == ""`). +//! - **R3 (blank-line run capping)** — mirrors `clean_output`'s own newline-run +//! cap *exactly* (verified empirically, not just inferred from its doc +//! comment): a run of `N` consecutive raw `\n` characters keeps only +//! `min(N, 2)` of them, and leading blank lines at the very start of the body +//! (immediately after frontmatter, or at offset 0 if there is none) are +//! elided entirely rather than merely capped — never applied inside protected +//! regions. +//! - **R4 (directive trailing-whitespace strip)** — safe because the parser +//! calls `dir.trim()` before matching, so trailing whitespace on a directive +//! line is discarded pre-parse and never reaches output. +//! +//! R1 and R3 additionally exclude a THIRD region beyond frontmatter/code +//! fences: `@message` and `@define` bodies (see `raw_content_spans`). This +//! was discovered empirically, not anticipated up front — `@message` content +//! is built by `evaluate_nodes(...).trim()` with NO `clean_output` pass (see +//! `collect_single_message` in `evaluator.rs`), so a `\r` or an uncollapsed +//! blank-line run inside one reaches the compiled message JSON verbatim. +//! `@define` bodies get the same conservative treatment because a function +//! can be called from a markdown-mode site OR from within a `@message` body, +//! and the formatter cannot tell which without a full call-graph analysis. +//! Directive lines (R4) remain safe inside both, since directive text is +//! never part of compiled output in either mode. +//! +//! ## Deliberately NOT implemented: blank-line whitespace stripping +//! +//! An earlier reading of this ruleset called for stripping trailing whitespace +//! from *any* all-whitespace "blank" line. Verified against the live compiler, +//! that is unsound in the general case: `clean_output`'s per-character loop +//! treats a bare space as ordinary content — it resets the newline-run counter +//! and is pushed through verbatim — so a whitespace-only line in the MIDDLE of +//! a document (not the absolute start or end) survives compilation byte for +//! byte (`printf 'Hello\n \nWorld\n' | mds build -` preserves the three +//! spaces). Stripping it in the formatter would silently break compile +//! equivalence, which this module treats as the non-negotiable constraint that +//! overrides any individual rule's literal description. See +//! `r4_whitespace_only_line_in_middle_of_document_is_preserved_verbatim` in +//! `tests/fmt.rs` for the regression test that locks this in. +//! +//! # Deferred to a future version (NOT implemented here) +//! +//! Interpolation `{ x }` -> `{x}` trimming, frontmatter key sorting, `@import` +//! grouping/reordering, blank-line insertion around directive blocks, body +//! hard-break normalization, directive internal spacing. + +use std::collections::BTreeSet; +use std::ops::Range; +use std::path::Path; + +use crate::error::MdsError; +use crate::lexer::{self, Token}; + +/// Format MDS source code, returning the rewritten source string. +/// +/// Equivalent to [`format_str_with`] with `base_dir = None` (imports resolve +/// against the current working directory, matching [`crate::compile_str`]). +/// +/// # Errors +/// +/// Returns `Err` if `source` has a syntax error (never a garbled string), or if +/// the rewritten source fails the internal compile-equivalence safety gate +/// (`MdsError::FormatterInvariant` — signals a formatter bug; callers must not +/// write the file when this occurs). +/// +/// # Examples +/// +/// ```rust +/// // CRLF -> LF, and a run of 3 raw newlines caps to 2 (one blank line +/// // survives), exactly matching the compiler's own `clean_output` pass. +/// let formatted = mds::format_str("Hello \r\n\r\n\r\nworld\r\n")?; +/// assert_eq!(formatted, "Hello \n\nworld\n"); +/// # Ok::<(), Box>(()) +/// ``` +#[must_use = "the formatted source should be used"] +pub fn format_str(source: &str) -> Result { + format_str_with(source, None) +} + +/// Format MDS source code with an explicit `@import` base directory. +/// +/// `base_dir` sets the root for resolving `@import` paths during the +/// compile-equivalence safety gate; defaults to the current directory when +/// `None`, matching [`crate::compile_str_with`]. +/// +/// # Errors +/// +/// See [`format_str`]. +#[must_use = "the formatted source should be used"] +pub fn format_str_with(source: &str, base_dir: Option<&Path>) -> Result { + let tokens = lexer::tokenize(source, "")?; + let protected = protected_spans(&tokens, source); + let raw_content = raw_content_spans(&tokens, source); + let directives = directive_line_offsets(&tokens); + let body_start = body_start_offset(&tokens, source); + + let formatted = rewrite(source, body_start, &protected, &raw_content, &directives); + assert_equivalent(source, &formatted, base_dir, &raw_content)?; + Ok(formatted) +} + +// ── Token offset helper ──────────────────────────────────────────────────────── + +/// Extract the byte offset carried by every [`Token`] variant. +fn token_offset(t: &Token) -> usize { + match t { + Token::Text(_, o) + | Token::Interpolation(_, o) + | Token::EscapedBrace(o) + | Token::Directive(_, o) + | Token::FrontmatterFence(o) + | Token::FrontmatterContent(_, o) + | Token::CodeFence(_, o) + | Token::CodeContent(_, o) => *o, + } +} + +// ── Region model ───────────────────────────────────────────────────────────── + +/// Compute the protected byte ranges: the union of Frontmatter* and Code* +/// region ranges. +/// +/// Each token's region runs from its own start offset to the next token's +/// start offset (or `src.len()` for the last token) — the lexer already ran +/// the fence/frontmatter state machine, so this reuses its offsets rather than +/// re-detecting fences. +fn protected_spans(tokens: &[Token], src: &str) -> Vec> { + let mut spans = Vec::new(); + for (i, t) in tokens.iter().enumerate() { + let is_protected = matches!( + t, + Token::FrontmatterFence(_) + | Token::FrontmatterContent(_, _) + | Token::CodeFence(_, _) + | Token::CodeContent(_, _) + ); + if !is_protected { + continue; + } + let start = token_offset(t); + let end = tokens.get(i + 1).map(token_offset).unwrap_or(src.len()); + if end > start { + spans.push(start..end); + } + } + spans +} + +/// Compute the start byte offset of every directive line. +/// +/// A `Token::Directive` is only ever lexed at line-start, outside code, so its +/// own offset IS the directive line's start offset. +fn directive_line_offsets(tokens: &[Token]) -> BTreeSet { + tokens + .iter() + .filter_map(|t| match t { + Token::Directive(_, o) => Some(*o), + _ => None, + }) + .collect() +} + +/// Compute the byte offset where the compiled "body" begins. +/// +/// `clean_output` (see `lib.rs`) only ever runs on the body — the raw +/// frontmatter is stripped off before it and reattached verbatim afterward via +/// `prepend_frontmatter`. A leading frontmatter block is always exactly three +/// consecutive tokens (`FrontmatterFence(0)`, `FrontmatterContent`, +/// `FrontmatterFence`), so the body starts at the fourth token when present. +/// Returns `0` when there is no leading frontmatter (a leading code fence, if +/// any, is ordinary body content — it participates in the same leading/middle +/// blank-line handling as everything else). +fn body_start_offset(tokens: &[Token], src: &str) -> usize { + let has_frontmatter = matches!(tokens.first(), Some(Token::FrontmatterFence(0))); + if !has_frontmatter { + return 0; + } + tokens.get(3).map(token_offset).unwrap_or(src.len()) +} + +/// Compute byte ranges within which content must be copied byte-for-byte: +/// neither R1's `\r` removal nor R3's blank-line-run capping may apply. These +/// are the bodies of `@message` and `@define` blocks. +/// +/// VERIFIED against the live evaluator (not inferred from the doc comment on +/// `clean_output`): `@message` content is produced by +/// `evaluate_nodes(&block.body, ...)?.trim()` — see `collect_single_message` +/// in `evaluator.rs` — with NO `clean_output` pass. Unlike markdown-mode +/// output, this means a `\r` or an uncollapsed blank-line run inside a +/// message body reaches the compiled JSON verbatim. Confirmed empirically: +/// `@message user:\r\nHi\r\nthere\r\n@end\r\n` compiles to message content +/// `"Hi\r\nthere"` (the `\r` survives), and `@message user:\nHi\n\n\n\nthere\n@end\n` +/// compiles to `"Hi\n\n\n\nthere"` (all 4 raw newlines survive uncapped, +/// unlike markdown mode's `"Hi\n\nthere"`). +/// +/// `@define` bodies get the same conservative treatment: a defined function +/// can be called from a markdown-mode site (where the surrounding +/// `clean_output` pass makes pre-collapsing harmless) or from within a +/// `@message` body (where it does not) — the formatter can't tell which +/// without a full call-graph analysis, so every `@define` body is treated as +/// raw. `@if`/`@for`/`@block` don't themselves bypass `clean_output`; they +/// only inherit raw-content status by virtue of being lexically nested inside +/// a `@message`/`@define` span, which the stack below naturally captures +/// (the outer span's byte range already covers everything nested within it). +fn raw_content_spans(tokens: &[Token], src: &str) -> Vec> { + // (is_message_or_define, start_offset) per currently-open block. + let mut stack: Vec<(bool, usize)> = Vec::new(); + let mut spans = Vec::new(); + // Defensive cap: the parser separately enforces MAX_NESTING_DEPTH (64) at + // parse time, but this function only sees tokens (pre-parse), so it + // cannot rely on that having been checked yet — bound the stack itself. + const MAX_STACK: usize = 4096; + + for t in tokens { + let Token::Directive(d, offset) = t else { + continue; + }; + let trimmed = d.trim(); + + if trimmed == "@end" { + if let Some((is_raw, start)) = stack.pop() { + if is_raw { + spans.push(start..*offset); + } + } + continue; + } + + let is_raw_opener = trimmed.starts_with("@message ") || trimmed.starts_with("@define "); + let is_any_opener = is_raw_opener + || trimmed.starts_with("@if ") + || trimmed.starts_with("@for ") + || trimmed.starts_with("@block "); + + if is_any_opener && stack.len() < MAX_STACK { + stack.push((is_raw_opener, *offset)); + } + } + + // Defensive: an unclosed raw-kind block (malformed input the parser will + // separately reject) still marks its remaining content as raw through + // EOF, rather than letting R1/R3 touch content that never had a chance + // to be proven equivalent via the safety gate's real-compile path. + for (is_raw, start) in stack { + if is_raw { + spans.push(start..src.len()); + } + } + + spans.sort_by_key(|r| r.start); + spans +} + +// ── \r stripping (R1) ──────────────────────────────────────────────────────── + +/// Append `s` to `out` with every `\r` character removed (R1). Allocation-free +/// in the common case where `s` contains no `\r`. +fn push_stripped_cr(out: &mut String, s: &str) { + if s.as_bytes().contains(&b'\r') { + out.extend(s.chars().filter(|&c| c != '\r')); + } else { + out.push_str(s); + } +} + +/// Return `s` with every `\r` character removed (R1), borrowing when possible. +fn strip_cr(s: &str) -> std::borrow::Cow<'_, str> { + if s.as_bytes().contains(&b'\r') { + std::borrow::Cow::Owned(s.chars().filter(|&c| c != '\r').collect()) + } else { + std::borrow::Cow::Borrowed(s) + } +} + +// ── Rewrite (R1-R4) ────────────────────────────────────────────────────────── + +/// Rewrite `source` into its formatted form. +/// +/// Single left-to-right pass over the original source (no per-line substring +/// re-scans of already-visited bytes, so this stays linear). Idempotent by +/// construction: R3's cap can't create a new collapsible run, trimming an +/// already-trimmed directive line is a no-op, and every rule acts on a +/// disjoint line classification (raw-content / protected / directive / blank +/// / content, checked in that priority order — see `rewrite_body`). +fn rewrite( + source: &str, + body_start: usize, + protected: &[Range], + raw_content: &[Range], + directives: &BTreeSet, +) -> String { + if source.is_empty() { + return String::new(); + } + + let mut out = String::with_capacity(source.len() + 1); + + // The leading frontmatter span (if any) is copied verbatim, mod \r — it is + // invisible to `clean_output`'s leading/trailing-trim and newline-run + // capping, which only ever see the body. + if body_start > 0 { + push_stripped_cr(&mut out, &source[..body_start]); + } + + let body = rewrite_body(source, body_start, protected, raw_content, directives); + let trimmed = body.trim_end(); + + if trimmed.is_empty() { + // Whole body is empty/whitespace-only -> matches clean_output(body) == "". + if out.is_empty() { + return String::new(); + } + // Frontmatter-only document: ensure exactly one final newline even in + // the edge case where the source had none (e.g. truncated right after + // the closing `---` fence). + if !out.ends_with('\n') { + out.push('\n'); + } + return out; + } + + out.push_str(trimmed); + out.push('\n'); + out +} + +/// Rewrite the body portion of `source` (from `body_start` to the end). +/// +/// Each line is classified in priority order: +/// 1. **Directive** — R4 (trailing-whitespace strip) applies unconditionally; +/// directive text is never part of any compiled output, in markdown OR +/// messages mode, so trimming it is always safe. +/// 2. **Raw content** (`@message`/`@define` bodies) — copied byte-for-byte, +/// including any nested code-fence content: this content bypasses +/// `clean_output` entirely (see `raw_content_spans`), so neither R1 nor R3 +/// may touch it. +/// 3. **Protected** (frontmatter never reaches here; code-fence lines do) — +/// R1 (\r strip) applies, R3 does not. +/// 4. **Ordinary content** — R1 and R3 (leading-blank elision + blank-run +/// capping) both apply. +fn rewrite_body( + source: &str, + body_start: usize, + protected: &[Range], + raw_content: &[Range], + directives: &BTreeSet, +) -> String { + let end = source.len(); + let mut out = String::with_capacity(end.saturating_sub(body_start)); + + let mut pos = body_start; + let mut leading_mode = true; + // Mirrors clean_output's own `newline_count`: sits at 1 immediately after + // any non-blank line (that line's own terminator counts as the first `\n` + // of a potential run), then increments per subsequent blank line. + let mut newline_run: usize = 0; + let mut pi: usize = 0; + let mut ri: usize = 0; + + // Skip past any protected/raw spans that end at or before body_start (the + // frontmatter's own spans, when present). + while pi < protected.len() && protected[pi].end <= body_start { + pi += 1; + } + while ri < raw_content.len() && raw_content[ri].end <= body_start { + ri += 1; + } + + while pos < end { + let line_start = pos; + let line_end = source[pos..].find('\n').map(|rel| pos + rel).unwrap_or(end); + let had_newline = line_end < end; + let next_pos = if had_newline { line_end + 1 } else { line_end }; + + while pi < protected.len() && protected[pi].end <= line_start { + pi += 1; + } + let is_protected = pi < protected.len() + && protected[pi].start <= line_start + && line_start < protected[pi].end; + + while ri < raw_content.len() && raw_content[ri].end <= line_start { + ri += 1; + } + let is_raw_content = ri < raw_content.len() + && raw_content[ri].start <= line_start + && line_start < raw_content[ri].end; + + let raw_line = &source[line_start..line_end]; + + if directives.contains(&line_start) { + // Priority 1: directive lines are never part of any compiled + // output (markdown or messages mode), so R4's trailing-whitespace + // strip is always safe, even inside a raw-content span. + let no_cr = strip_cr(raw_line); + out.push_str(no_cr.trim_end()); + if had_newline { + out.push('\n'); + } + leading_mode = false; + newline_run = 1; + } else if is_raw_content { + // Priority 2: @message/@define body content bypasses + // clean_output entirely (see raw_content_spans) -- copy exactly, + // not even R1's \r removal. + out.push_str(raw_line); + if had_newline { + out.push('\n'); + } + leading_mode = false; + newline_run = 1; + } else if is_protected { + // Priority 3: frontmatter (never reaches here) / code-fence + // content -- R1 applies, R3 does not. + push_stripped_cr(&mut out, raw_line); + if had_newline { + out.push('\n'); + } + leading_mode = false; + newline_run = 1; + } else { + let no_cr = strip_cr(raw_line); + if no_cr.is_empty() { + // Truly blank line (zero-width -- nothing between the newlines, + // not merely whitespace; see module docs for why a + // whitespace-only line is NOT treated as blank here). + if leading_mode { + // Elide entirely: no content, no newline. + } else { + newline_run += 1; + if newline_run <= 2 && had_newline { + out.push('\n'); + } + // newline_run > 2: drop this blank line's newline (R3 cap). + } + } else { + out.push_str(&no_cr); + if had_newline { + out.push('\n'); + } + leading_mode = false; + newline_run = 1; + } + } + + pos = next_pos; + } + + out +} + +// ── Safety gate ────────────────────────────────────────────────────────────── + +/// Verify that `formatted` compiles to the same output as `source`. +/// +/// When `source` compiles standalone, this is an exact check: both are +/// compiled with the same `base_dir` and their [`crate::CompiledOutput`]s must +/// match. `compile_str_collecting_warnings` (not `compile_str_with`) is used +/// so the gate never duplicates warnings to stderr on every format call. +/// +/// When `source` does NOT compile standalone (a minority case — typically an +/// undefined runtime variable; imports still resolve via `base_dir`), falls +/// back to a structural, rule-aware token comparison so the formatter can +/// still succeed on templates that only work with runtime vars supplied at +/// render time. +fn assert_equivalent( + source: &str, + formatted: &str, + base_dir: Option<&Path>, + raw_content: &[Range], +) -> Result<(), MdsError> { + match crate::compile_str_collecting_warnings(source, base_dir, None) { + Ok(orig) => match crate::compile_str_collecting_warnings(formatted, base_dir, None) { + Ok(after) if after.output == orig.output => Ok(()), + Ok(_) => Err(MdsError::formatter_invariant( + "formatted source compiles to different output than the original", + )), + Err(e) => Err(MdsError::formatter_invariant(format!( + "formatted source failed to compile though the original succeeded: {e}" + ))), + }, + Err(_) => { + if structural_equivalent(source, formatted, raw_content) { + Ok(()) + } else { + Err(MdsError::formatter_invariant( + "formatted source diverges structurally from the original, and the \ + original does not compile standalone to verify equivalence directly", + )) + } + } + } +} + +/// Rule-aware structural comparison used when neither `source` nor +/// `formatted` can be compiled standalone (e.g. an undefined runtime +/// variable). Re-tokenizes both and compares token-for-token: `Directive` +/// content after `.trim()`, Frontmatter*/Code* content after `\r` removal +/// (both always safe, matching R4 and R1), and everything else exactly +/// EXCEPT `Text` tokens outside a raw-content span, which are compared after +/// `crate::clean_output` (the SAME function the real compiler applies to +/// markdown-mode output). `Text` tokens whose SOURCE offset falls inside +/// `raw_content` (a `@message`/`@define` body -- see `raw_content_spans`) are +/// compared exactly instead, since that content bypasses `clean_output` +/// entirely; `rewrite` already guarantees such tokens are byte-identical, but +/// comparing them exactly here (rather than via `clean_output`, which would +/// incorrectly treat some byte differences as insignificant) keeps this +/// fallback correct in its own right rather than merely accidentally correct +/// because of that upstream guarantee. +fn structural_equivalent(source: &str, formatted: &str, raw_content: &[Range]) -> bool { + let (Ok(src_tokens), Ok(fmt_tokens)) = + (lexer::tokenize(source, ""), lexer::tokenize(formatted, "")) + else { + return false; + }; + + if src_tokens.len() != fmt_tokens.len() { + return false; + } + + src_tokens + .iter() + .zip(fmt_tokens.iter()) + .all(|(a, b)| match (a, b) { + (Token::Text(ta, oa), Token::Text(tb, _)) => { + if raw_content.iter().any(|r| r.contains(oa)) { + ta == tb + } else { + crate::clean_output(ta) == crate::clean_output(tb) + } + } + (Token::Interpolation(ia, _), Token::Interpolation(ib, _)) => ia == ib, + (Token::EscapedBrace(_), Token::EscapedBrace(_)) => true, + (Token::Directive(da, _), Token::Directive(db, _)) => da.trim() == db.trim(), + (Token::FrontmatterFence(_), Token::FrontmatterFence(_)) => true, + (Token::FrontmatterContent(ca, _), Token::FrontmatterContent(cb, _)) => { + ca.replace('\r', "") == cb.replace('\r', "") + } + (Token::CodeFence(fa, _), Token::CodeFence(fb, _)) => fa == fb, + (Token::CodeContent(ca, oa), Token::CodeContent(cb, _)) => { + if raw_content.iter().any(|r| r.contains(oa)) { + ca == cb + } else { + ca.replace('\r', "") == cb.replace('\r', "") + } + } + _ => false, + }) +} + +// ── Tests ──────────────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn protected_spans_covers_frontmatter_and_code_fence() { + let src = "---\nname: x\n---\n```\ncode\n```\nAfter\n"; + let tokens = lexer::tokenize(src, "").unwrap(); + let spans = protected_spans(&tokens, src); + // Every byte of the frontmatter block and the code fence block should + // be covered by some protected span; "After\n" should not be. + let covers = |offset: usize| spans.iter().any(|r| r.contains(&offset)); + assert!(covers(0), "frontmatter fence start should be protected"); + assert!( + covers(src.find("name").unwrap()), + "frontmatter content should be protected" + ); + assert!( + covers(src.find("```").unwrap()), + "code fence should be protected" + ); + assert!( + covers(src.find("code").unwrap()), + "code content should be protected" + ); + assert!( + !covers(src.find("After").unwrap()), + "body text after the fence should not be protected" + ); + } + + #[test] + fn directive_line_offsets_finds_at_directives_only() { + let src = "@if x:\nBody\n@end\n"; + let tokens = lexer::tokenize(src, "").unwrap(); + let offsets = directive_line_offsets(&tokens); + assert_eq!(offsets.len(), 2, "expected @if and @end, got: {offsets:?}"); + assert!( + offsets.contains(&0), + "expected offset 0 for @if, got: {offsets:?}" + ); + } + + #[test] + fn body_start_offset_zero_without_frontmatter() { + let src = "Hello\n"; + let tokens = lexer::tokenize(src, "").unwrap(); + assert_eq!(body_start_offset(&tokens, src), 0); + } + + #[test] + fn body_start_offset_after_closing_fence_with_frontmatter() { + let src = "---\nname: x\n---\nHello\n"; + let tokens = lexer::tokenize(src, "").unwrap(); + let start = body_start_offset(&tokens, src); + assert_eq!(&src[start..], "Hello\n"); + } + + #[test] + fn body_start_offset_end_of_source_when_frontmatter_only() { + let src = "---\nname: x\n---\n"; + let tokens = lexer::tokenize(src, "").unwrap(); + assert_eq!(body_start_offset(&tokens, src), src.len()); + } + + #[test] + fn raw_content_spans_covers_message_body_not_surrounding_text() { + let src = "Before\n@message user:\nHi there\n@end\nAfter\n"; + let tokens = lexer::tokenize(src, "").unwrap(); + let spans = raw_content_spans(&tokens, src); + let covers = |offset: usize| spans.iter().any(|r| r.contains(&offset)); + assert!( + covers(src.find("Hi there").unwrap()), + "message body should be raw content" + ); + assert!( + !covers(src.find("Before").unwrap()), + "text before @message must not be raw" + ); + assert!( + !covers(src.find("After").unwrap()), + "text after @end must not be raw" + ); + } + + #[test] + fn raw_content_spans_covers_define_body() { + let src = "@define greet(x):\nHello {x}\n@end\n"; + let tokens = lexer::tokenize(src, "").unwrap(); + let spans = raw_content_spans(&tokens, src); + assert!(spans + .iter() + .any(|r| r.contains(&src.find("Hello").unwrap()))); + } + + #[test] + fn raw_content_spans_excludes_standalone_if_and_block() { + // @if and @block bodies are NOT raw content on their own -- only + // @message/@define, and anything lexically nested inside one of them. + let src = "@if x:\nBody\n@end\n@block b:\nStuff\n@end\n"; + let tokens = lexer::tokenize(src, "").unwrap(); + let spans = raw_content_spans(&tokens, src); + assert!(spans.is_empty(), "expected no raw spans, got: {spans:?}"); + } + + #[test] + fn raw_content_spans_covers_nested_if_inside_message() { + // Content nested inside @if, which is itself nested inside @message, + // is STILL raw -- the outer @message span covers it structurally. + let src = "@message user:\n@if x:\nNested\n@end\n@end\n"; + let tokens = lexer::tokenize(src, "").unwrap(); + let spans = raw_content_spans(&tokens, src); + assert!(spans + .iter() + .any(|r| r.contains(&src.find("Nested").unwrap()))); + } + + #[test] + fn raw_content_spans_defensive_unclosed_message_covers_to_eof() { + // Malformed input (missing @end) still marks the remainder as raw + // rather than leaving it to R1/R3, even though the parser will + // separately reject this at compile time. + let src = "@message user:\nHi there"; + let tokens = lexer::tokenize(src, "").unwrap(); + let spans = raw_content_spans(&tokens, src); + assert!(spans + .iter() + .any(|r| r.contains(&src.find("Hi there").unwrap()))); + } + + #[test] + fn push_stripped_cr_removes_embedded_and_trailing_cr() { + let mut out = String::new(); + push_stripped_cr(&mut out, "a\rb\r"); + assert_eq!(out, "ab"); + } + + #[test] + fn strip_cr_borrows_when_no_cr_present() { + let s = "no carriage returns here"; + assert!(matches!(strip_cr(s), std::borrow::Cow::Borrowed(_))); + } +} diff --git a/crates/mds-core/src/lib.rs b/crates/mds-core/src/lib.rs index b97364e..1b54b65 100644 --- a/crates/mds-core/src/lib.rs +++ b/crates/mds-core/src/lib.rs @@ -44,6 +44,7 @@ pub(crate) mod ast; pub(crate) mod builtins; pub(crate) mod error; pub(crate) mod evaluator; +pub(crate) mod formatter; pub(crate) mod fs; pub(crate) mod lexer; pub(crate) mod limits; @@ -54,6 +55,7 @@ pub(crate) mod scope; pub(crate) mod validator; pub(crate) mod value; +pub use formatter::{format_str, format_str_with}; pub use fs::{FileSystem, NativeFs, VirtualFs}; pub use options::{ format_unknown_keys_error, json_type_name, parse_json_vars, reject_unknown_json_keys, VarsError, diff --git a/crates/mds-core/tests/api_surface.rs b/crates/mds-core/tests/api_surface.rs index 9ca1bbd..076c056 100644 --- a/crates/mds-core/tests/api_surface.rs +++ b/crates/mds-core/tests/api_surface.rs @@ -8,6 +8,10 @@ use mds::{ #[test] fn public_functions_exist() { + let _: fn(&str) -> Result = mds::format_str; + let _: fn(&str, Option<&Path>) -> Result = mds::format_str_with; + let _ = mds::format_str("Hello!\n"); + let _ = mds::format_str_with("Hello!\n", None); let _ = mds::compile_str("---\nname: World\n---\nHello {name}!\n"); let _ = mds::compile_str_with("Hello!\n", None, None); let _ = mds::compile_str_collecting_warnings("Hello!\n", None, None); @@ -117,6 +121,9 @@ fn mds_error_variants_exist() { span: None, src: None, }; + let _ = MdsError::FormatterInvariant { + detail: "test".to_string(), + }; #[allow(unreachable_patterns)] match (MdsError::Io { @@ -138,11 +145,24 @@ fn mds_error_variants_exist() { | MdsError::JsonError { .. } | MdsError::Recursion { .. } | MdsError::ExportError { .. } - | MdsError::BuiltinError { .. } => {} + | MdsError::BuiltinError { .. } + | MdsError::FormatterInvariant { .. } => {} _ => {} } } +#[test] +fn formatter_invariant_has_diagnostic_code() { + let err = MdsError::FormatterInvariant { + detail: "test detail".to_string(), + }; + let code = miette::Diagnostic::code(&err) + .map(|c| c.to_string()) + .unwrap_or_default(); + assert_eq!(code, "mds::formatter_invariant"); + assert!(format!("{err}").contains("test detail")); +} + #[test] fn value_trait_impls() { let s = Value::from("hello"); diff --git a/crates/mds-core/tests/fmt.rs b/crates/mds-core/tests/fmt.rs new file mode 100644 index 0000000..420560f --- /dev/null +++ b/crates/mds-core/tests/fmt.rs @@ -0,0 +1,551 @@ +//! Integration tests for the `mds fmt` engine (`format_str` / `format_str_with`). +//! +//! MDS is a content language: the formatter MUST produce byte-identical compile +//! output (`compile(fmt(src)).output == compile(src).output`) and MUST be +//! idempotent (`fmt(fmt(src)) == fmt(src)`). These tests verify that invariant +//! across a small corpus plus rule-specific and edge-case scenarios. +//! +//! Several exact whitespace-collapsing thresholds below were verified empirically +//! against the live `clean_output` compiler pass (not just inferred from prose), +//! per the plan's own instruction to verify claims against live code. See the +//! `r3_*` and `r4_*` tests and their comments for the specific behavior locked in. + +use mds::{format_str, format_str_with, MdsError}; + +// ── Small corpus of representative, syntactically-valid MDS snippets ───────── +// +// Every entry here is known to tokenize AND compile without runtime vars or +// imports (self-contained). Import/base_dir scenarios get their own dedicated +// tests below (they need a real tempdir). +const CORPUS: &[&str] = &[ + "Hello world!\n", + "---\nname: World\n---\nHello {name}!\n", + "---\npremium: true\n---\n@if premium:\nThanks!\n@end\n", + "---\nitems: [a, b, c]\n---\n@for item in items:\n- {item}\n@end\n", + "```python\ndef f():\n pass\n```\n", + "@define greet(x):\nHello {x}!\n@end\n{greet(\"World\")}\n", + "@message user:\nHi!\n@end\n", + "Line with \\{escaped\\} braces.\n", + "", + " \n", + "No trailing newline", + "---\nname: x\n---\n", + "---\npremium: true\n---\nBefore\n\n\n\nAfter @if premium: inline text {premium}\n@end\n", +]; + +fn compiled_markdown(src: &str) -> Option { + mds::compile_str(src) + .ok() + .and_then(|r| r.into_markdown().ok()) +} + +// ── AC-EF-1: idempotence ────────────────────────────────────────────────────── + +#[test] +fn idempotent_across_corpus() { + for src in CORPUS { + let once = format_str(src).unwrap_or_else(|e| panic!("format_str failed for {src:?}: {e}")); + let twice = format_str(&once) + .unwrap_or_else(|e| panic!("format_str (2nd pass) failed for {once:?}: {e}")); + assert_eq!(once, twice, "not idempotent for input: {src:?}"); + } +} + +// ── AC-EF-2: compile-equivalence ────────────────────────────────────────────── + +#[test] +fn compile_equivalent_across_corpus() { + for src in CORPUS { + let Some(orig_md) = compiled_markdown(src) else { + // Skip entries that don't compile standalone (none currently, but + // keeps this test robust if the corpus grows to include one). + continue; + }; + let formatted = format_str(src).expect("format_str should succeed for corpus entry"); + let formatted_md = compiled_markdown(&formatted).unwrap_or_else(|| { + panic!("formatted output of {src:?} failed to compile: {formatted:?}") + }); + assert_eq!( + orig_md, formatted_md, + "compile output changed for input: {src:?}\nformatted: {formatted:?}" + ); + } +} + +// ── API surface (also pinned in api_surface.rs) ─────────────────────────────── + +#[test] +fn format_str_returns_result_string_mdserror() { + let _: Result = format_str("Hello!\n"); +} + +#[test] +fn format_str_with_accepts_optional_base_dir() { + let _: Result = format_str_with("Hello!\n", None); +} + +// ── R1: CRLF/CR removal, including inside protected regions ───────────────── + +#[test] +fn r1_crlf_converted_to_lf_everywhere_including_frontmatter_and_fences() { + let src = "---\r\nname: x\r\n---\r\nHello\r\n```\r\ncode\r\n```\r\n"; + let out = format_str(src).expect("should format"); + assert!( + !out.contains('\r'), + "no \\r should survive formatting: {out:?}" + ); + assert_eq!(out, "---\nname: x\n---\nHello\n```\ncode\n```\n"); +} + +#[test] +fn r1_bare_cr_without_lf_is_removed() { + // A bare \r (no following \n) must also be removed, matching clean_output's + // unconditional \r-skip (verified: clean_output deletes every \r it sees, + // \n-paired or not). + let src = "a\rb\n"; + let out = format_str(src).expect("should format"); + assert!(!out.contains('\r')); +} + +// ── R2: exactly one final newline; empty/whitespace-only -> empty ─────────── + +#[test] +fn r2_empty_input_yields_empty_output() { + assert_eq!(format_str("").unwrap(), ""); +} + +#[test] +fn r2_whitespace_only_input_yields_empty_output() { + // Matches clean_output(" \n") == "" (verified empirically against the + // live compiler: `printf ' \n' | mds build -` produces zero bytes). + assert_eq!(format_str(" \n").unwrap(), ""); + assert_eq!(format_str("\n\n\n").unwrap(), ""); + assert_eq!(format_str(" \n\n\n").unwrap(), ""); +} + +#[test] +fn r2_missing_trailing_newline_is_added() { + assert_eq!(format_str("Hello").unwrap(), "Hello\n"); +} + +#[test] +fn r2_excess_trailing_newlines_collapse_to_one() { + assert_eq!(format_str("Hello\n\n\n").unwrap(), "Hello\n"); +} + +#[test] +fn r2_frontmatter_only_no_trailing_newline_still_gets_one() { + // Edge case: source ends exactly at the closing frontmatter fence with no + // trailing newline. The overall document must still end in exactly one \n. + let out = format_str("---\nname: x\n---").expect("should format"); + assert!( + out.ends_with('\n'), + "expected trailing newline, got: {out:?}" + ); +} + +// ── R3: 3+ blank lines collapse to one, except inside fences ──────────────── + +#[test] +fn r3_blank_line_run_collapses_matching_clean_output() { + // Verified empirically: clean_output("Hello\n\n\n\nWorld\n") == "Hello\n\nWorld\n". + let out = format_str("Hello\n\n\n\nWorld\n").unwrap(); + assert_eq!(out, "Hello\n\nWorld\n"); +} + +#[test] +fn r3_single_blank_line_unchanged() { + let out = format_str("Hello\n\nWorld\n").unwrap(); + assert_eq!(out, "Hello\n\nWorld\n"); +} + +#[test] +fn r3_never_collapses_blank_lines_inside_code_fence() { + let src = "```\ncode\n\n\n\nmore\n```\nAfter\n"; + let out = format_str(src).expect("should format"); + assert!( + out.contains("code\n\n\n\nmore"), + "blank lines inside a code fence must be preserved verbatim, got: {out:?}" + ); +} + +#[test] +fn r3_leading_blank_lines_in_body_fully_stripped() { + // Verified empirically: clean_output of a body with leading blank lines + // removes them ENTIRELY (not capped at one) -- both with and without + // frontmatter present. + let out = format_str("\n\n\nHello\nWorld\n").unwrap(); + assert_eq!(out, "Hello\nWorld\n"); + + let out_fm = format_str("---\nname: x\n---\n\n\n\nHello\n").unwrap(); + assert_eq!(out_fm, "---\nname: x\n---\nHello\n"); +} + +// ── R4: trailing whitespace on directive lines; body content UNCHANGED ────── + +#[test] +fn r4_directive_trailing_whitespace_stripped() { + let src = "---\npremium: true\n---\n@if premium: \nThanks!\n@end\t\n"; + let out = format_str(src).expect("should format"); + assert!( + out.contains("@if premium:\n"), + "directive trailing spaces should be stripped, got: {out:?}" + ); + assert!( + out.contains("@end\n") && !out.contains("@end\t"), + "directive trailing tab should be stripped, got: {out:?}" + ); +} + +#[test] +fn r4_headline_body_content_trailing_spaces_survive_markdown_hard_break() { + // THE canonical case from the plan: two trailing spaces on a body-content + // line form a Markdown hard line break and MUST survive formatting + // byte-for-byte, unlike a directive line's trailing whitespace. + let src = "line \nnext\n"; + let out = format_str(src).expect("should format"); + assert_eq!( + out, "line \nnext\n", + "trailing spaces on content lines must be preserved" + ); +} + +#[test] +fn r4_whitespace_only_line_in_middle_of_document_is_preserved_verbatim() { + // DELIBERATE, VERIFIED DEVIATION from a literal "strip all blank-line + // whitespace" reading of R4: empirically, clean_output does NOT treat a + // whitespace-only line as inert when it sits between two content lines -- + // `printf 'Hello\n \nWorld\n' | mds build -` preserves the 3 spaces + // byte-for-byte (clean_output's per-char loop only collapses truly-EMPTY + // \n runs; a space character resets its counter and is pushed verbatim, + // and clean_output's final trim_end() only touches the absolute end of + // the document, never the middle). Stripping this line would therefore + // change compiled output and violate AC-EF-2, which the plan states is + // the overriding, non-negotiable constraint. So this formatter leaves an + // isolated whitespace-only "blank" line completely untouched. The + // `format_str_with(' \n', ...)` whole-document case is still covered by + // R2 above (whitespace-only whole input -> empty). + let src = "Hello\n \nWorld\n"; + let out = format_str(src).expect("should format"); + assert_eq!( + out, src, + "an isolated whitespace-only line must not be touched (see comment)" + ); + // Compile-equivalence must hold regardless. + let compiled_before = mds::compile_str(src).unwrap().into_markdown().unwrap(); + let compiled_after = mds::compile_str(&out).unwrap().into_markdown().unwrap(); + assert_eq!(compiled_before, compiled_after); +} + +#[test] +fn r4_trailing_tabs_on_directive_and_blank_lines_stripped_body_tabs_preserved() { + let src = "---\npremium: true\n---\n@if premium:\t\t\nkeep\ttab\there\n@end\n"; + let out = format_str(src).expect("should format"); + assert!(out.contains("@if premium:\n"), "got: {out:?}"); + assert!( + out.contains("keep\ttab\there\n"), + "body-content tabs must be preserved, got: {out:?}" + ); +} + +// ── R7: protected regions byte-identical after format except \r removal ───── + +#[test] +fn r7_frontmatter_content_byte_identical_mod_cr() { + let src = "---\nname: World\nimports:\n - path: ./lib.mds\ntype: mds\n---\nHello {name}!\n"; + let out = format_str(src).expect("should format"); + assert!( + out.starts_with("---\nname: World\nimports:\n - path: ./lib.mds\ntype: mds\n---\n"), + "frontmatter must be byte-identical (not reformatted/stripped), got: {out:?}" + ); +} + +#[test] +fn r7_code_fence_content_byte_identical_mod_cr() { + let src = "```python\ndef f( x ):\n return x\n```\n"; + let out = format_str(src).expect("should format"); + assert_eq!(out, src, "code fence content must be byte-identical"); +} + +// ── AC-EF-8: syntax errors surfaced, never a garbled string ───────────────── + +#[test] +fn syntax_error_unclosed_code_fence_is_err() { + let err = format_str("```\ncode without a close\n").unwrap_err(); + assert!(matches!(err, MdsError::Syntax { .. })); +} + +#[test] +fn syntax_error_unclosed_interpolation_is_err() { + let err = format_str("Hello {name\n").unwrap_err(); + assert!(matches!(err, MdsError::Syntax { .. })); +} + +#[test] +fn syntax_error_unclosed_frontmatter_is_err() { + let err = format_str("---\nname: x\nHello\n").unwrap_err(); + assert!(matches!(err, MdsError::Syntax { .. })); +} + +// ── T1-gate-fallback: undefined-var source still formats; import source uses full gate ── + +#[test] +fn gate_fallback_undefined_var_source_still_formats() { + // `mds::compile_str` fails (undefined variable), so format_str_with must + // fall back to the structural check rather than hard-failing. + let src = "Hello {undefined_var}!\n\n\n\nBye.\n"; + assert!( + mds::compile_str(src).is_err(), + "sanity: source must not compile standalone" + ); + let out = format_str(src).expect("format_str must still succeed via structural fallback"); + assert_eq!(out, "Hello {undefined_var}!\n\nBye.\n"); +} + +#[test] +fn gate_full_check_runs_when_base_dir_resolves_imports() { + let dir = tempfile::tempdir().unwrap(); + let lib_path = dir.path().join("lib.mds"); + std::fs::write(&lib_path, "@define greet(x):\nHello {x}!\n@end\n").unwrap(); + + let src = "@import \"./lib.mds\"\n{greet(\"World\")}\n\n\n\nBye.\n"; + let out = + format_str_with(src, Some(dir.path())).expect("format_str_with should resolve imports"); + assert_eq!(out, "@import \"./lib.mds\"\n{greet(\"World\")}\n\nBye.\n"); + + // Compile-equivalence via the REAL gate: compile both with the same base_dir. + let before = mds::compile_str_with(src, Some(dir.path()), None) + .unwrap() + .into_markdown() + .unwrap(); + let after = mds::compile_str_with(&out, Some(dir.path()), None) + .unwrap() + .into_markdown() + .unwrap(); + assert_eq!(before, after); +} + +// ── Edge cases (EC-1..EC-8ish; EC-9 partial/parent lives in cli_fmt.rs) ────── + +#[test] +fn ec_empty_and_whitespace_only_input_to_empty_output() { + assert_eq!(format_str("").unwrap(), ""); + assert_eq!(format_str(" ").unwrap(), ""); + assert_eq!(format_str("\t\t").unwrap(), ""); +} + +#[test] +fn ec_missing_trailing_newline_added() { + assert_eq!( + format_str("no newline at all").unwrap(), + "no newline at all\n" + ); +} + +#[test] +fn ec_frontmatter_only_file() { + let out = format_str("---\nname: World\n---\n").unwrap(); + assert_eq!(out, "---\nname: World\n---\n"); +} + +#[test] +fn ec_file_ending_mid_directive_at_eof_no_trailing_newline() { + let src = "---\npremium: true\n---\n@if premium:\nBody\n@end"; + let out = format_str(src).expect("should format despite no trailing newline"); + assert!(out.ends_with('\n')); + assert!(out.ends_with("@end\n")); +} + +#[test] +fn ec_unclosed_fence_interp_frontmatter_all_error() { + assert!(format_str("```\nunclosed").is_err()); + assert!(format_str("{unclosed").is_err()); + assert!(format_str("---\nunclosed").is_err()); +} + +#[test] +fn ec_unclosed_input_is_not_written_by_caller_contract() { + // format_str returning Err (not a garbled Ok(String)) is the contract the + // CLI relies on to avoid ever writing a corrupted file. + match format_str("```\nunclosed") { + Err(MdsError::Syntax { .. }) => {} + other => panic!("expected Syntax error, got: {other:?}"), + } +} + +#[test] +fn ec_utf8_bom_before_frontmatter_fence_left_verbatim() { + // A UTF-8 BOM before `---` means the source does NOT start with the exact + // byte sequence "---\n"/"---\r\n" the lexer requires, so frontmatter is + // never recognized -- consistent with the compiler. The whole thing + // (BOM included) is ordinary body text. + let src = "\u{FEFF}---\nname: x\n---\nHello\n"; + let out = format_str(src).expect("should format"); + assert!( + out.starts_with('\u{FEFF}'), + "BOM must be preserved verbatim, got: {out:?}" + ); + // Must remain compile-equivalent (BOM-prefixed source is not treated as + // frontmatter by the compiler either). + let before = mds::compile_str(src).unwrap().into_markdown().unwrap(); + let after = mds::compile_str(&out).unwrap().into_markdown().unwrap(); + assert_eq!(before, after); +} + +#[test] +fn ec_multibyte_utf8_byte_offset_correctness() { + let src = "---\npremium: true\n---\n@if premium:\nCafé 日本語 emoji: \u{1F600}\n@end\n\n\n\nBye \u{1F600}\n"; + let out = format_str(src).expect("should format without panicking on multi-byte offsets"); + assert!(out.contains("Café 日本語 emoji: \u{1F600}")); + assert!(out.contains("Bye \u{1F600}\n")); + let before = mds::compile_str(src).unwrap().into_markdown().unwrap(); + let after = mds::compile_str(&out).unwrap().into_markdown().unwrap(); + assert_eq!(before, after); +} + +#[test] +fn ec_mixed_crlf_lf_all_become_lf() { + let src = "Hello\r\nWorld\nAgain\r\n"; + let out = format_str(src).unwrap(); + assert_eq!(out, "Hello\nWorld\nAgain\n"); +} + +#[test] +fn ec_blank_line_collapse_inside_define_message_block_bodies_is_safe() { + // R3 collapsing is safe to apply inside a standalone @block body (still + // ordinary markdown-mode content, subject to the normal clean_output + // pass) but is DELIBERATELY NOT applied inside @message or @define + // bodies. Verified empirically against the live evaluator/compiler (not + // just inferred): `@message` content is built via + // `evaluate_nodes(...).trim()` with NO clean_output pass, so an + // uncollapsed blank-line run survives verbatim in the compiled message + // JSON (`@message user:\nHi\n\n\n\nthere\n@end\n` compiles to content + // `"Hi\n\n\n\nthere"`, all 4 raw newlines intact). `@define` bodies get + // the same conservative treatment because a function's body can be + // called from EITHER a markdown-mode site (clean_output applies) or a + // @message-mode site (it does not) -- the formatter can't tell which + // without a full call-graph analysis, so it never collapses inside one. + // In all three cases the direct compile-equivalence assertion below is + // the thing that actually matters; the "contains" checks document + // exactly what the formatter chose to do (and, for the define/message + // cases, that it correctly chose NOT to touch the blank-line run). + + // @define body: collapsing must NOT happen (conservative exemption). + let define_src = "@define greet(x):\nHello\n\n\n\n{x}!\n@end\n{greet(\"World\")}\n"; + let define_out = format_str(define_src).expect("should format"); + assert!( + define_out.contains("Hello\n\n\n\n{x}!"), + "blank-line run inside @define body must NOT collapse, got: {define_out:?}" + ); + let define_before = mds::compile_str(define_src).unwrap(); + let define_after = mds::compile_str(&define_out).unwrap(); + assert_eq!(define_before.output, define_after.output); + + // @message body: collapsing must NOT happen (verified unsafe above). + let message_src = "@message user:\nHi\n\n\n\nthere\n@end\n"; + let message_out = format_str(message_src).expect("should format"); + assert!( + message_out.contains("Hi\n\n\n\nthere"), + "blank-line run inside @message body must NOT collapse, got: {message_out:?}" + ); + let message_before = mds::compile_str(message_src).unwrap(); + let message_after = mds::compile_str(&message_out).unwrap(); + assert_eq!(message_before.output, message_after.output); + + // @block body (standalone mode: renders its default body inline). + let block_src = "@block instructions:\nStep one\n\n\n\nStep two\n@end\n"; + let block_out = format_str(block_src).expect("should format"); + assert!( + block_out.contains("Step one\n\nStep two"), + "blank-line run inside @block body should collapse, got: {block_out:?}" + ); + let block_before = mds::compile_str(block_src).unwrap(); + let block_after = mds::compile_str(&block_out).unwrap(); + assert_eq!(block_before.output, block_after.output); +} + +#[test] +fn message_body_carriage_return_is_preserved_not_stripped() { + // Verified empirically: `@message` content bypasses clean_output's \r + // removal entirely -- a \r inside a message body reaches the compiled + // JSON verbatim (as the literal two-character sequence \r\n in the + // JSON-escaped string). The formatter must therefore NOT apply R1 + // inside a message body, unlike everywhere else in the document. + let src = "@message user:\r\nHi\r\nthere\r\n@end\r\n"; + let out = format_str(src).expect("should format"); + assert!( + out.contains("Hi\r\nthere"), + "\\r inside a @message body must be preserved verbatim, got: {out:?}" + ); + let before = mds::compile_str(src).unwrap(); + let after = mds::compile_str(&out).unwrap(); + assert_eq!(before.output, after.output); +} + +#[test] +fn code_fence_nested_inside_message_body_is_fully_raw_not_just_r1_protected() { + // A code fence nested inside a @message body is doubly-exempt: it would + // normally be "protected" (R1 \r-strip applies, R3 doesn't), but because + // its content flows into the message's raw (non-clean_output'd) string, + // even R1's \r-strip must not apply to it here. This locks in the + // priority ordering in `rewrite_body` (raw-content is checked before + // protected). + let src = "@message user:\n```\r\ncode\r\n```\r\n@end\r\n"; + let out = format_str(src).expect("should format"); + assert!( + out.contains("```\r\ncode\r\n```\r\n"), + "\\r inside a fence nested in a @message body must be preserved verbatim, got: {out:?}" + ); + let before = mds::compile_str(src).unwrap(); + let after = mds::compile_str(&out).unwrap(); + assert_eq!(before.output, after.output); +} + +// ── Idempotency for malformed/degenerate inputs must still error consistently ─ + +#[test] +fn idempotency_not_claimed_for_non_tokenizing_input() { + // Not a formal AC, but documents the boundary: malformed input errors both + // times rather than being "fixed" by a first pass. + assert!(format_str("```\nunclosed").is_err()); +} + +// ── Perf: linear time on a large mixed file (AC-PERF-1) ────────────────────── + +#[test] +fn perf_linear_one_megabyte_file_formats_under_two_seconds() { + let mut src = String::from("---\npremium: true\n---\n"); + // Mix directives, prose, and a code block, repeated to reach ~1MB. + let chunk = "@if premium:\nSome prose line with plain text.\n@end\n\n\n\n```text\nblock\n```\n"; + while src.len() < 1_000_000 { + src.push_str(chunk); + } + src.push_str("Final line.\n"); + + let start = std::time::Instant::now(); + let out = format_str(&src).expect("should format a large file"); + let elapsed = start.elapsed(); + + assert!(!out.is_empty()); + assert!( + elapsed.as_secs_f64() < 2.0, + "formatting ~1MB took too long: {elapsed:?}" + ); +} + +// ── Runtime-vars-independent: format_str_with never needs runtime vars ─────── + +#[test] +fn format_str_with_none_base_dir_matches_format_str() { + for src in CORPUS { + let a = format_str(src); + let b = format_str_with(src, None); + match (a, b) { + (Ok(a), Ok(b)) => assert_eq!(a, b, "format_str/format_str_with diverged for {src:?}"), + (Err(_), Err(_)) => {} + (a, b) => panic!( + "format_str and format_str_with disagreed on Ok/Err for {src:?}: {a:?} vs {b:?}" + ), + } + } +} From e012a89a400101531f6fec00aa8fbb95d0c73fce Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Fri, 3 Jul 2026 11:37:56 +0300 Subject: [PATCH 02/15] feat(cli): add mds fmt subcommand Wires the mds-core formatting engine into the CLI: `mds fmt` on a file, directory (recursive, INCLUDING _-prefixed partials -- a deliberate divergence from build/check, since formatting rewrites source rather than emitting compiled output), or stdin (`-`, as a filter). `--check` is read-only and exits non-zero if anything would change; `--diff` is read-only and prints a unified diff (colorized via raw ANSI only on a TTY, using the `similar` crate -- a genuinely zero-transitive-dependency, Apache-2.0, MSRV-1.85 pin); the two combine. Writes only when content actually changes (preserves mtime). Reuses the existing exit_code mapping (0/1/2/3, no new codes) and the existing resolve_input/read_stdin/collect_mds_files helpers. Adds a FmtConfig `mds.json` section (sort_frontmatter_keys, default true) as forward-compat scaffolding for a rule deferred to a future version -- not yet consulted by any formatting behavior. Task: feat/60-mds-fmt-auto-formatter --- Cargo.lock | 20 + Cargo.toml | 5 + crates/mds-cli/Cargo.toml | 1 + crates/mds-cli/src/build.rs | 41 + crates/mds-cli/src/fmt.rs | 426 +++++++++ crates/mds-cli/src/main.rs | 30 + crates/mds-cli/src/watch.rs | 2 + crates/mds-cli/tests/cli_commands.rs | 111 +++ crates/mds-cli/tests/cli_fmt.rs | 856 ++++++++++++++++++ .../mds-cli/tests/fixtures/_fmt_partial.mds | 7 + .../tests/fixtures/fmt_body_hardbreak.mds | 2 + .../mds-cli/tests/fixtures/fmt_formatted.mds | 6 + .../mds-cli/tests/fixtures/fmt_malformed.mds | 2 + .../fixtures/fmt_parent_includes_partial.mds | 2 + .../tests/fixtures/fmt_unformatted.mds | 10 + 15 files changed, 1521 insertions(+) create mode 100644 crates/mds-cli/src/fmt.rs create mode 100644 crates/mds-cli/tests/cli_fmt.rs create mode 100644 crates/mds-cli/tests/fixtures/_fmt_partial.mds create mode 100644 crates/mds-cli/tests/fixtures/fmt_body_hardbreak.mds create mode 100644 crates/mds-cli/tests/fixtures/fmt_formatted.mds create mode 100644 crates/mds-cli/tests/fixtures/fmt_malformed.mds create mode 100644 crates/mds-cli/tests/fixtures/fmt_parent_includes_partial.mds create mode 100644 crates/mds-cli/tests/fixtures/fmt_unformatted.mds diff --git a/Cargo.lock b/Cargo.lock index c9384a2..8dd9cc6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -129,6 +129,16 @@ dependencies = [ "objc2", ] +[[package]] +name = "bstr" +version = "1.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5cee35f73844aa3014bb606320a6c1f010249dbdf43342fe54b5a4f6a8ed4b79" +dependencies = [ + "memchr", + "serde_core", +] + [[package]] name = "bumpalo" version = "3.20.2" @@ -558,6 +568,7 @@ dependencies = [ "notify", "serde", "serde_json", + "similar", "tempfile", ] @@ -1084,6 +1095,15 @@ version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64" +[[package]] +name = "similar" +version = "3.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6505efef05804732ed8a3f2d4f279429eb485bd69d5b0cc6b19cc02005cda16" +dependencies = [ + "bstr", +] + [[package]] name = "slab" version = "0.4.12" diff --git a/Cargo.toml b/Cargo.toml index be09d46..a92d0c8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -52,6 +52,11 @@ pythonize = "=0.29.0" # would enforce this acceptance in CI (suggested follow-up, out of scope for this batch). notify = "8" ctrlc = "3.5" +# similar: pure-Rust diffing (mds-cli `fmt --diff`). Apache-2.0 (MIT-compatible, no +# copyleft). 3.1.1 released 2026-05-23 (~40d before this pin) — past the routine 30d +# soak window. rust-version = "1.85" in its published Cargo.toml, within the workspace +# MSRV (1.88). Only the default features ("std", "text") are used. +similar = "3.1" # panic = "unwind" is required workspace-wide because mds-wasm and mds-napi use # catch_unwind at the JS boundary to convert panics into structured JS errors. diff --git a/crates/mds-cli/Cargo.toml b/crates/mds-cli/Cargo.toml index 9e4444f..6e3d712 100644 --- a/crates/mds-cli/Cargo.toml +++ b/crates/mds-cli/Cargo.toml @@ -22,6 +22,7 @@ serde_json = { workspace = true } miette = { workspace = true, features = ["fancy"] } notify = { workspace = true } ctrlc = { workspace = true } +similar = { workspace = true } [dev-dependencies] tempfile = { workspace = true } diff --git a/crates/mds-cli/src/build.rs b/crates/mds-cli/src/build.rs index c080f69..88801cc 100644 --- a/crates/mds-cli/src/build.rs +++ b/crates/mds-cli/src/build.rs @@ -17,6 +17,15 @@ use serde::Deserialize; pub(crate) struct MdsConfig { #[serde(default)] pub(crate) build: BuildConfig, + /// Loaded (and validated — a malformed `fmt` section still fails config + /// loading) for forward-compatibility, but not yet consulted by `mds fmt` + /// — see `FmtConfig`. + #[allow( + dead_code, + reason = "scaffolding for a rule not implemented until a future version" + )] + #[serde(default)] + pub(crate) fmt: FmtConfig, } #[derive(Debug, Default, Deserialize)] @@ -24,6 +33,37 @@ pub(crate) struct BuildConfig { pub(crate) output_dir: Option, } +/// Forward-compatibility scaffolding for `mds fmt` configuration. +/// +/// `sort_frontmatter_keys` is not wired into any formatting behavior yet — the +/// v1 ruleset (R1-R4, see `mds-core`'s `formatter` module) deliberately defers +/// frontmatter key sorting to a future version, and there is intentionally no +/// matching CLI flag (a no-op flag would be a clippy/UX liability). The field +/// exists now purely so `{"fmt": {"sort_frontmatter_keys": false}}` parses +/// cleanly today and this won't need a breaking `mds.json` schema change once +/// the rule ships. +#[derive(Debug, Deserialize)] +pub(crate) struct FmtConfig { + #[allow( + dead_code, + reason = "scaffolding for a rule not implemented until a future version" + )] + #[serde(default = "default_sort_frontmatter_keys")] + pub(crate) sort_frontmatter_keys: bool, +} + +impl Default for FmtConfig { + fn default() -> Self { + Self { + sort_frontmatter_keys: default_sort_frontmatter_keys(), + } + } +} + +fn default_sort_frontmatter_keys() -> bool { + true +} + /// Maximum allowed size for `mds.json` (1 MB) to prevent runaway memory use. const MAX_CONFIG_SIZE: u64 = 1024 * 1024; @@ -971,6 +1011,7 @@ mod tests { build: BuildConfig { output_dir: Some("build".to_string()), }, + ..Default::default() }, PathBuf::from("/project"), )); diff --git a/crates/mds-cli/src/fmt.rs b/crates/mds-cli/src/fmt.rs new file mode 100644 index 0000000..c9c27cf --- /dev/null +++ b/crates/mds-cli/src/fmt.rs @@ -0,0 +1,426 @@ +//! `fmt` subcommand — opinionated, safety-gated auto-formatter (issue #60). +//! +//! Dispatches on the resolved input: `-` (stdin, base_dir = None, formats to +//! stdout as a filter), a directory (recurses, INCLUDING partials — formatting +//! rewrites source, unlike build/check which skip partials because they don't +//! emit their own compiled output), or a single file (base_dir = the file's +//! parent, matching how imports resolve for that file when compiled normally). +//! +//! # Channel discipline +//! +//! - Plain filter-mode formatted content (stdin, no flags) and `--diff` output +//! go to STDOUT. +//! - All status lines, summaries, and errors go to STDERR. +//! - `--quiet` suppresses status/summaries but never errors (AC-CF-8). +//! +//! # Exit codes (reuses the existing `exit_code` mapping — no new codes) +//! +//! - 0: formatted OK / nothing to change / diff preview +//! - 1: `--check` found something that would change (after printing the +//! summary), OR a format/parse error (`MdsError` non-io, including +//! `FormatterInvariant`) via `Err` -> `exit_code` +//! - 2: file not found / not `.mds` / I/O / bad UTF-8 +//! - 3: oversized source + +use std::io::{IsTerminal, Write as _}; +use std::path::{Path, PathBuf}; + +use mds::{FileSystem, MdsError}; +use miette::Result; + +use crate::build::{load_config, read_stdin, resolve_input}; +use crate::output::collect_mds_files; + +/// Directory recursion depth cap, matching `run_build_directory` / `run_check_directory`. +const MAX_DEPTH: usize = 64; + +pub(crate) struct FmtArgs { + pub(crate) input: Option, + pub(crate) check: bool, + pub(crate) diff: bool, + pub(crate) quiet: bool, +} + +pub(crate) fn run_fmt(args: FmtArgs) -> Result<()> { + let FmtArgs { + input, + check, + diff, + quiet, + } = args; + + let (input, auto_detected) = resolve_input(input)?; + if auto_detected && !quiet { + eprintln!("Formatting {}", input.display()); + } + + if input == Path::new("-") { + return run_fmt_stdin(check, diff, quiet); + } + + if input.is_dir() { + // Reject a symlinked directory root, matching build/check parity (commit aa0c538). + if input + .symlink_metadata() + .map(|m| m.file_type().is_symlink()) + .unwrap_or(false) + { + return Err(miette::miette!( + "directory argument must not be a symlink: {}", + input.display() + )); + } + return run_fmt_directory(&input, check, diff, quiet); + } + + ensure_mds_extension(&input)?; + run_fmt_file(&input, check, diff, quiet) +} + +/// Reject an explicit-file input whose extension isn't `.mds` (AC-CF, exit 2). +/// +/// `MdsError::NotMdsFile` already maps to exit code 2 via `exit_code` — reused +/// rather than inventing a new error shape. +fn ensure_mds_extension(path: &Path) -> Result<()> { + let is_mds = path.extension().and_then(|e| e.to_str()) == Some("mds"); + if is_mds { + Ok(()) + } else { + Err(miette::Error::from(MdsError::NotMdsFile { + path: path.display().to_string(), + })) + } +} + +/// Read the raw source of `path` for formatting: symlink-checked, size-capped, +/// UTF-8-validated (PF-004 parity with `read_stdin` and the resolver). +/// +/// `mds fmt` needs the RAW, unparsed source text (not a compiled result), so +/// it can't go through `mds::compile*`. Reuses `NativeFs::check_symlink` +/// (never re-implements canonicalize — PF-003) and `NativeFs::read`'s +/// TOCTOU-safe read-then-size-check instead of a bare `std::fs::read`. +fn read_source_file(path: &Path) -> Result { + let canonical = mds::NativeFs::check_symlink(path).map_err(miette::Error::from)?; + let path_str = canonical + .to_str() + .ok_or_else(|| miette::miette!("path is not valid UTF-8: {}", path.display()))?; + mds::NativeFs::new() + .read(path_str) + .map_err(miette::Error::from) +} + +/// The result of formatting one file's content: whether it changed, and the +/// formatted string. Callers apply mode-specific policy (write-if-changed, +/// `--check` tally, `--diff` rendering) on top of this. +struct FmtResult { + formatted: String, + changed: bool, +} + +fn format_source(source: &str, base_dir: Option<&Path>) -> Result { + let formatted = mds::format_str_with(source, base_dir).map_err(miette::Error::from)?; + let changed = formatted != source; + Ok(FmtResult { formatted, changed }) +} + +// ── stdin mode ─────────────────────────────────────────────────────────────── + +fn run_fmt_stdin(check: bool, diff: bool, quiet: bool) -> Result<()> { + let (source, cwd) = read_stdin()?; + let result = format_source(&source, Some(&cwd))?; + + if diff { + print_diff(&render_diff(&source, &result.formatted, ""))?; + } else if !check { + // Plain filter mode: formatted content is the output. + write_stdout(&result.formatted)?; + } + + if check && result.changed { + if !quiet { + eprintln!("Would reformat: "); + } + std::process::exit(1); + } + Ok(()) +} + +// ── single-file mode ───────────────────────────────────────────────────────── + +fn run_fmt_file(path: &Path, check: bool, diff: bool, quiet: bool) -> Result<()> { + let source = read_source_file(path)?; + let base_dir = path.parent(); + let result = format_source(&source, base_dir)?; + + if diff && result.changed { + let label = path.display().to_string(); + print_diff(&render_diff(&source, &result.formatted, &label))?; + } + + let read_only = check || diff; + if !read_only { + if result.changed { + std::fs::write(path, &result.formatted) + .map_err(|e| miette::miette!("cannot write {}: {e}", path.display()))?; + if !quiet { + eprintln!("Formatted: {}", path.display()); + } + } else if !quiet { + eprintln!("Unchanged: {}", path.display()); + } + return Ok(()); + } + + if check { + if result.changed { + if !quiet { + eprintln!("Would reformat: {}", path.display()); + } + std::process::exit(1); + } + if !quiet { + eprintln!("Unchanged: {}", path.display()); + } + } + Ok(()) +} + +// ── directory mode ─────────────────────────────────────────────────────────── + +/// Format every `.mds` file under `dir`, INCLUDING `_`-prefixed partials. +/// +/// Deliberate divergence from `run_build_directory` / `run_check_directory`: +/// `is_partial` governs output EMISSION (a partial produces no standalone +/// compiled file), but formatting rewrites SOURCE, and a partial's source is +/// just as much a candidate for reformatting as any other file. +/// +/// Continue-on-error: a per-file failure does not abort the run. Non-zero +/// exit when any file failed, or (under `--check`) when any file would change. +fn run_fmt_directory(dir: &Path, check: bool, diff: bool, quiet: bool) -> Result<()> { + // Validate mds.json even though `fmt` doesn't act on its `fmt` section's + // content yet — consistent with build/check, which also fail loudly on a + // malformed config rather than silently ignoring it. + let _ = load_config(dir)?; + + let files = collect_mds_files(dir, MAX_DEPTH, None); + + if files.is_empty() { + if !quiet { + eprintln!("No .mds files found in {}", dir.display()); + } + return Ok(()); + } + + let read_only = check || diff; + let mut changed_count: usize = 0; + let mut unchanged_count: usize = 0; + let mut fail_count: usize = 0; + + for file in &files { + let source = match read_source_file(file) { + Ok(s) => s, + Err(e) => { + eprintln!("{e:?}"); + fail_count += 1; + continue; + } + }; + let base_dir = file.parent(); + let result = match format_source(&source, base_dir) { + Ok(r) => r, + Err(e) => { + eprintln!("{e:?}"); + fail_count += 1; + continue; + } + }; + + if diff && result.changed { + let label = file.display().to_string(); + if let Err(e) = print_diff(&render_diff(&source, &result.formatted, &label)) { + eprintln!("{e:?}"); + } + } + + if read_only { + if result.changed { + changed_count += 1; + } else { + unchanged_count += 1; + } + continue; + } + + if !result.changed { + unchanged_count += 1; + continue; + } + match std::fs::write(file, &result.formatted) { + Ok(()) => { + if !quiet { + eprintln!("Formatted: {}", file.display()); + } + changed_count += 1; + } + Err(e) => { + eprintln!("error: cannot write {}: {e}", file.display()); + fail_count += 1; + } + } + } + + if read_only { + if !quiet || changed_count > 0 || fail_count > 0 { + eprintln!("{changed_count} would reformat, {fail_count} failed"); + } + } else if !quiet || fail_count > 0 { + eprintln!("{changed_count} formatted, {unchanged_count} unchanged, {fail_count} failed"); + } + + if fail_count > 0 || (check && changed_count > 0) { + std::process::exit(1); + } + Ok(()) +} + +// ── stdout writing (broken-pipe-safe) ──────────────────────────────────────── + +/// Write `s` to stdout, treating a broken pipe as a clean early exit rather +/// than an error — matches Unix filter conventions (e.g. `mds fmt - | head +/// -n1` closing the pipe early must not surface as a crash or failure). +fn write_stdout(s: &str) -> Result<()> { + let mut stdout = std::io::stdout(); + if let Err(e) = stdout.write_all(s.as_bytes()) { + if e.kind() == std::io::ErrorKind::BrokenPipe { + return Ok(()); + } + return Err(miette::miette!("cannot write to stdout: {e}")); + } + if let Err(e) = stdout.flush() { + if e.kind() != std::io::ErrorKind::BrokenPipe { + return Err(miette::miette!("cannot flush stdout: {e}")); + } + } + Ok(()) +} + +fn print_diff(rendered: &str) -> Result<()> { + if rendered.is_empty() { + return Ok(()); + } + write_stdout(rendered) +} + +// ── unified diff rendering ─────────────────────────────────────────────────── + +/// Render a unified diff between `original` and `formatted`, colorized with +/// raw ANSI escapes ONLY when stdout is a terminal (mirrors `watch.rs`'s +/// `clear_terminal`, which does the same TTY gate for its own raw escapes; +/// this repo has no color crate dependency). +fn render_diff(original: &str, formatted: &str, label: &str) -> String { + let diff = similar::TextDiff::from_lines(original, formatted); + let unified = diff + .unified_diff() + .context_radius(3) + .header(label, label) + .to_string(); + + if unified.is_empty() || !std::io::stdout().is_terminal() { + return unified; + } + colorize_unified_diff(&unified) +} + +fn colorize_unified_diff(unified: &str) -> String { + const RED: &str = "\x1b[31m"; + const GREEN: &str = "\x1b[32m"; + const CYAN: &str = "\x1b[36m"; + const RESET: &str = "\x1b[0m"; + + let mut out = String::with_capacity(unified.len() + 64); + for line in unified.split_inclusive('\n') { + let color = if line.starts_with("+++") || line.starts_with("---") || line.starts_with("@@") + { + CYAN + } else if line.starts_with('+') { + GREEN + } else if line.starts_with('-') { + RED + } else { + "" + }; + if color.is_empty() { + out.push_str(line); + continue; + } + out.push_str(color); + match line.strip_suffix('\n') { + Some(stripped) => { + out.push_str(stripped); + out.push_str(RESET); + out.push('\n'); + } + None => { + out.push_str(line); + out.push_str(RESET); + } + } + } + out +} + +// ── Unit tests ──────────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn ensure_mds_extension_accepts_dot_mds() { + assert!(ensure_mds_extension(Path::new("foo.mds")).is_ok()); + } + + #[test] + fn ensure_mds_extension_rejects_other_extensions() { + assert!(ensure_mds_extension(Path::new("foo.txt")).is_err()); + assert!(ensure_mds_extension(Path::new("foo")).is_err()); + } + + #[test] + fn format_source_detects_no_change() { + let result = format_source("Hello!\n", None).unwrap(); + assert!(!result.changed); + assert_eq!(result.formatted, "Hello!\n"); + } + + #[test] + fn format_source_detects_change() { + let result = format_source("Hello!\r\n\r\n\r\n\r\nBye.\r\n", None).unwrap(); + assert!(result.changed); + assert!(!result.formatted.contains('\r')); + } + + #[test] + fn render_diff_empty_for_identical_input() { + let rendered = render_diff("same\n", "same\n", "label"); + assert!(rendered.is_empty()); + } + + #[test] + fn render_diff_contains_unified_markers_for_changed_input() { + let rendered = render_diff("a\n", "b\n", "label"); + assert!(rendered.contains("---")); + assert!(rendered.contains("+++")); + assert!(rendered.contains("-a")); + assert!(rendered.contains("+b")); + } + + #[test] + fn colorize_unified_diff_wraps_add_remove_lines_with_ansi_when_requested() { + let unified = "--- a\n+++ b\n@@ -1 +1 @@\n-old\n+new\n"; + let colorized = colorize_unified_diff(unified); + assert!(colorized.contains("\x1b[32m+new\x1b[0m")); + assert!(colorized.contains("\x1b[31m-old\x1b[0m")); + assert!(colorized.contains("\x1b[36m--- a\x1b[0m")); + } +} diff --git a/crates/mds-cli/src/main.rs b/crates/mds-cli/src/main.rs index b33ee4d..6c6586c 100644 --- a/crates/mds-cli/src/main.rs +++ b/crates/mds-cli/src/main.rs @@ -5,6 +5,7 @@ use clap::{Parser, Subcommand}; use miette::Result; mod build; +mod fmt; mod output; mod watch; @@ -72,6 +73,29 @@ enum Commands { #[arg(long = "set", value_name = "KEY=VALUE", value_parser = parse_key_value)] set_vars: Vec<(String, String)>, }, + /// Reformat MDS file(s) in place (opinionated, safety-gated) + /// + /// Rewrites are guaranteed compile-equivalent: a safety gate re-compiles the + /// formatted source and refuses to write if it would change compiled output. + /// Normalizes CRLF to LF, collapses runs of 3+ blank lines to one, strips + /// trailing whitespace on directive lines, and ensures exactly one final + /// newline — never touches body-text trailing whitespace (Markdown hard + /// breaks, including on whitespace-only "blank" lines) or the byte-for-byte + /// content of frontmatter / code fences / `@message` / `@define` bodies. + #[command( + after_help = "Examples:\n mds fmt Auto-detect and format the .mds file in current dir\n mds fmt template.mds Format a file in place\n mds fmt . Format every .mds file recursively (incl. partials)\n mds fmt --check template.mds Exit 1 if the file would change; writes nothing\n mds fmt --diff template.mds Print a unified diff of pending changes; writes nothing\n mds fmt --check --diff . Show diffs for every file that would change, exit 1 if any would\n echo \"Hello {name}!\" | mds fmt - Format from stdin, write to stdout; creates no file" + )] + Fmt { + /// Input .mds file, directory, or "-" for stdin (omit to auto-detect in current directory) + input: Option, + /// Read-only: exit non-zero if any file would change; never writes + #[arg(long)] + check: bool, + /// Read-only: print a unified diff of pending changes; never writes. + /// Combines with --check (diff is the rendering, check is the exit behavior). + #[arg(long)] + diff: bool, + }, /// Create a starter MDS file Init { /// Output filename @@ -314,6 +338,12 @@ fn run(cli: Cli) -> Result<()> { vars, set_vars, } => run_check(input, vars, set_vars, quiet), + Commands::Fmt { input, check, diff } => fmt::run_fmt(fmt::FmtArgs { + input, + check, + diff, + quiet, + }), Commands::Init { filename, force } => run_init(filename, force, quiet), Commands::Watch { input, diff --git a/crates/mds-cli/src/watch.rs b/crates/mds-cli/src/watch.rs index 0774b96..dfd989a 100644 --- a/crates/mds-cli/src/watch.rs +++ b/crates/mds-cli/src/watch.rs @@ -2317,6 +2317,7 @@ mod tests { build: BuildConfig { output_dir: Some("dist".to_string()), }, + ..Default::default() }, PathBuf::from("/project"), )); @@ -2336,6 +2337,7 @@ mod tests { build: BuildConfig { output_dir: Some("../bad".to_string()), }, + ..Default::default() }, PathBuf::from("/project"), )); diff --git a/crates/mds-cli/tests/cli_commands.rs b/crates/mds-cli/tests/cli_commands.rs index 96343e0..725dd60 100644 --- a/crates/mds-cli/tests/cli_commands.rs +++ b/crates/mds-cli/tests/cli_commands.rs @@ -383,6 +383,117 @@ fn exit_code_syntax_error() { ); } +// ── mds fmt exit codes (issue #60) ──────────────────────────────────────────── + +#[test] +fn exit_code_fmt_success() { + let dir = tempfile::tempdir().unwrap(); + let src = dir.path().join("clean.mds"); + std::fs::write(&src, "Hello!\n").unwrap(); + let status = mds_bin() + .arg("fmt") + .arg(&src) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .expect("failed to run mds"); + assert!( + status.success(), + "expected exit code 0 for an already-clean file" + ); +} + +#[test] +fn exit_code_fmt_check_dirty() { + let dir = tempfile::tempdir().unwrap(); + let src = dir.path().join("dirty.mds"); + std::fs::write(&src, "Hello!\r\n\r\n\r\n\r\nBye.\r\n").unwrap(); + let status = mds_bin() + .args(["fmt", "--check"]) + .arg(&src) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .expect("failed to run mds"); + assert_eq!( + status.code(), + Some(1), + "expected exit code 1 when --check finds a file that would change" + ); +} + +#[test] +fn exit_code_fmt_syntax_error() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("bad.mds"); + std::fs::write(&path, "```\nunclosed fence\n").unwrap(); + let status = mds_bin() + .arg("fmt") + .arg(&path) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .expect("failed to run mds"); + assert_eq!( + status.code(), + Some(1), + "expected exit code 1 for a syntax error" + ); +} + +#[test] +fn exit_code_fmt_file_not_found() { + let status = mds_bin() + .args(["fmt", "/tmp/no_such_fmt_file_98765.mds"]) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .expect("failed to run mds"); + assert_eq!( + status.code(), + Some(2), + "expected exit code 2 for file-not-found" + ); +} + +#[test] +fn exit_code_fmt_not_mds_extension() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("notes.txt"); + std::fs::write(&path, "plain text\n").unwrap(); + let status = mds_bin() + .arg("fmt") + .arg(&path) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .expect("failed to run mds"); + assert_eq!( + status.code(), + Some(2), + "expected exit code 2 for a non-.mds explicit file" + ); +} + +#[test] +fn exit_code_fmt_oversized() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("huge.mds"); + std::fs::write(&path, "x".repeat(10 * 1024 * 1024 + 1)).unwrap(); + let status = mds_bin() + .arg("fmt") + .arg(&path) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .expect("failed to run mds"); + assert_eq!( + status.code(), + Some(3), + "expected exit code 3 for oversized source" + ); +} + // Directory input: an empty directory exits 0 (prints "No .mds files found"). // Directory build is now supported — the old "must fail" test is updated to match new behavior. #[test] diff --git a/crates/mds-cli/tests/cli_fmt.rs b/crates/mds-cli/tests/cli_fmt.rs new file mode 100644 index 0000000..9c42cf6 --- /dev/null +++ b/crates/mds-cli/tests/cli_fmt.rs @@ -0,0 +1,856 @@ +//! Integration tests for `mds fmt` (issue #60). +//! +//! Coverage maps to AC-CF-1..8 from the execution plan: +//! - AC-CF-1: file.mds rewrites in place only when content changes, unchanged +//! keeps mtime, prints Formatted/Unchanged to stderr +//! - AC-CF-2: `-` reads stdin -> stdout, creates no file +//! - AC-CF-3: `.` formats every .mds recursively INCLUDING _partials, +//! continue-on-error, summary `N formatted, M unchanged, K failed`, exit 1 +//! iff any failed +//! - AC-CF-4: --check exits 0 when clean / 1 when any would change or fails, +//! NEVER writes +//! - AC-CF-5: --diff prints unified diff to stdout (colorized only on TTY), +//! writes nothing, exit 0 unless read/parse error -> 1 +//! - AC-CF-6: --check --diff prints diffs AND exits non-zero when changes exist +//! - AC-CF-7: no-arg auto-detects single .mds in cwd, errors on zero/many +//! - AC-CF-8: -q/--quiet suppresses status but never errors + +mod common; +use common::{fixture, mds_bin}; + +use std::fs; +use std::path::Path; +use std::time::Duration; + +fn read_fixture(name: &str) -> String { + fs::read_to_string(fixture(name)).expect("read fixture") +} + +fn fmt_file(path: &Path, extra_args: &[&str]) -> std::process::Output { + mds_bin() + .arg("fmt") + .arg(path) + .args(extra_args) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap() +} + +fn fmt_dir(path: &Path, extra_args: &[&str]) -> std::process::Output { + mds_bin() + .arg("fmt") + .arg(path) + .args(extra_args) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap() +} + +// ── AC-CF-1: single-file in-place rewrite, mtime, status lines ────────────── + +#[test] +fn inplace_rewrites_only_when_changed() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("unformatted.mds"); + fs::write(&target, read_fixture("fmt_unformatted.mds")).unwrap(); + let original = fs::read_to_string(&target).unwrap(); + + let output = fmt_file(&target, &[]); + assert!( + output.status.success(), + "fmt should succeed; stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let after = fs::read_to_string(&target).unwrap(); + assert_ne!(after, original, "content should have changed"); + assert!(!after.contains('\r'), "CRLF should be gone: {after:?}"); + + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("Formatted"), + "expected a Formatted status line, got: {stderr}" + ); +} + +#[test] +fn inplace_unchanged_file_preserves_mtime_and_reports_unchanged() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("clean.mds"); + fs::write(&target, read_fixture("fmt_formatted.mds")).unwrap(); + + let before_mtime = fs::metadata(&target).unwrap().modified().unwrap(); + // Ensure the filesystem clock would visibly move if the file were rewritten. + std::thread::sleep(Duration::from_millis(1100)); + + let output = fmt_file(&target, &[]); + assert!( + output.status.success(), + "fmt should succeed on an already-clean file" + ); + + let after_mtime = fs::metadata(&target).unwrap().modified().unwrap(); + assert_eq!( + before_mtime, after_mtime, + "mtime must not change for an unchanged file" + ); + + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("Unchanged"), + "expected an Unchanged status line, got: {stderr}" + ); +} + +#[test] +fn format_then_check_is_idempotent() { + // T2-idem: format a file, then --check it -- must report clean (exit 0). + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("idem.mds"); + fs::write(&target, read_fixture("fmt_unformatted.mds")).unwrap(); + + let first = fmt_file(&target, &[]); + assert!(first.status.success()); + + let check = fmt_file(&target, &["--check"]); + assert!( + check.status.success(), + "second pass under --check should report clean; stderr: {}", + String::from_utf8_lossy(&check.stderr) + ); +} + +// ── AC-CF-2: stdin -> stdout, no file created ──────────────────────────────── + +fn fmt_stdin(input: &str, extra_args: &[&str]) -> std::process::Output { + use std::io::Write; + let mut child = mds_bin() + .arg("fmt") + .arg("-") + .args(extra_args) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .unwrap(); + child + .stdin + .take() + .unwrap() + .write_all(input.as_bytes()) + .unwrap(); + child.wait_with_output().unwrap() +} + +#[test] +fn stdin_reads_and_writes_stdout_creates_no_file() { + let dir = tempfile::tempdir().unwrap(); + let before: Vec<_> = fs::read_dir(dir.path()).unwrap().collect(); + assert!(before.is_empty()); + + let input = read_fixture("fmt_unformatted.mds"); + let output = mds_bin() + .current_dir(dir.path()) + .arg("fmt") + .arg("-") + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .and_then(|mut child| { + use std::io::Write; + child.stdin.take().unwrap().write_all(input.as_bytes())?; + child.wait_with_output() + }) + .unwrap(); + + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).unwrap(); + assert!( + !stdout.contains('\r'), + "stdout should be reformatted: {stdout:?}" + ); + + let after: Vec<_> = fs::read_dir(dir.path()).unwrap().collect(); + assert!(after.is_empty(), "fmt - must not create any file in cwd"); +} + +#[test] +fn stdin_filter_mode_is_idempotent() { + // T2-filter-idem: pipe formatted output back through `fmt -` -> identical. + let input = read_fixture("fmt_unformatted.mds"); + let first = fmt_stdin(&input, &[]); + assert!(first.status.success()); + let once = String::from_utf8(first.stdout).unwrap(); + + let second = fmt_stdin(&once, &[]); + assert!(second.status.success()); + let twice = String::from_utf8(second.stdout).unwrap(); + + assert_eq!(once, twice, "stdin filter mode must be idempotent"); +} + +#[test] +fn stdin_into_closed_pipe_does_not_panic() { + // Piping into `true` closes the read end almost immediately; writing the + // formatted result to stdout must handle a broken pipe gracefully. + use std::io::Write; + let mut child = mds_bin() + .arg("fmt") + .arg("-") + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .unwrap(); + + // Drop stdout immediately to force a broken pipe on write. + drop(child.stdout.take()); + + let big_input = read_fixture("fmt_unformatted.mds").repeat(1000); + // Ignore write errors -- the point is the CHILD process must not panic. + let _ = child.stdin.take().unwrap().write_all(big_input.as_bytes()); + + let output = child.wait_with_output().unwrap(); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + !stderr.contains("panicked"), + "must not panic on broken pipe, got stderr: {stderr}" + ); +} + +// ── AC-CF-3: directory mode, partials included, continue-on-error, summary ── + +#[test] +fn dir_recurse_formats_every_mds_including_partials() { + let dir = tempfile::tempdir().unwrap(); + fs::write( + dir.path().join("main.mds"), + read_fixture("fmt_unformatted.mds"), + ) + .unwrap(); + fs::write( + dir.path().join("_partial.mds"), + read_fixture("_fmt_partial.mds"), + ) + .unwrap(); + + let output = fmt_dir(dir.path(), &[]); + assert!( + output.status.success(), + "dir fmt should succeed; stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let main_after = fs::read_to_string(dir.path().join("main.mds")).unwrap(); + assert!(!main_after.contains('\r'), "main.mds should be reformatted"); + + // Partials ARE formatted (deliberate divergence from build/check). The + // partial's directive lines (@define/@end/@export) and the blank-line run + // between @end and @export are OUTSIDE the @define body and must be + // reformatted; the "Hello {x}!\r\n" line INSIDE the @define body + // deliberately keeps its \r (raw-content exemption -- see formatter.rs's + // module docs and the `message_body_carriage_return_is_preserved`-style + // tests in mds-core/tests/fmt.rs), so this asserts the directive-level + // change rather than a blanket "no \\r anywhere". + let partial_after = fs::read_to_string(dir.path().join("_partial.mds")).unwrap(); + assert_ne!( + partial_after, + read_fixture("_fmt_partial.mds"), + "_partial.mds should ALSO be reformatted" + ); + assert!( + partial_after.contains("@define greet(x):\n"), + "directive trailing ws/\\r should be stripped even in a partial, got: {partial_after:?}" + ); + assert!( + partial_after.contains("@end\n\n@export"), + "blank-line run between @end and @export should collapse, got: {partial_after:?}" + ); + + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("formatted") && stderr.contains("unchanged") && stderr.contains("failed"), + "expected a summary with formatted/unchanged/failed counts, got: {stderr}" + ); +} + +#[test] +fn dir_continue_on_error_summary_nonzero_exit() { + let dir = tempfile::tempdir().unwrap(); + fs::write(dir.path().join("good.mds"), "Hello, world!\n").unwrap(); + fs::write(dir.path().join("bad.mds"), "```\nunclosed fence\n").unwrap(); + + let output = fmt_dir(dir.path(), &[]); + assert!( + !output.status.success(), + "dir fmt with a failing file must exit non-zero" + ); + + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("1 failed") || stderr.contains("failed"), + "summary must mention the failure, got: {stderr}" + ); +} + +#[test] +fn dir_already_formatted_tree_reports_all_unchanged_exit_zero() { + let dir = tempfile::tempdir().unwrap(); + fs::write( + dir.path().join("clean.mds"), + read_fixture("fmt_formatted.mds"), + ) + .unwrap(); + + let output = fmt_dir(dir.path(), &[]); + assert!(output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("1 unchanged"), + "expected '1 unchanged' in summary, got: {stderr}" + ); +} + +// ── AC-CF-9 (EC-9): partial + parent that @includes it stay compile-equivalent ─ + +#[test] +fn parent_that_includes_partial_compiles_unchanged_after_formatting_both() { + let dir = tempfile::tempdir().unwrap(); + fs::write( + dir.path().join("_fmt_partial.mds"), + read_fixture("_fmt_partial.mds"), + ) + .unwrap(); + let parent = dir.path().join("parent.mds"); + fs::write(&parent, read_fixture("fmt_parent_includes_partial.mds")).unwrap(); + + let before = mds_bin() + .arg("build") + .arg(&parent) + .arg("-o") + .arg("-") + .output() + .unwrap(); + assert!(before.status.success(), "pre-format build should succeed"); + + let output = fmt_dir(dir.path(), &[]); + assert!( + output.status.success(), + "formatting the dir should succeed; stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let after = mds_bin() + .arg("build") + .arg(&parent) + .arg("-o") + .arg("-") + .output() + .unwrap(); + assert!(after.status.success(), "post-format build should succeed"); + assert_eq!( + before.stdout, after.stdout, + "compiled output must be unchanged after formatting the partial + parent" + ); +} + +// ── AC-CF-4: --check exits 0 clean / 1 dirty, never writes ────────────────── + +#[test] +fn check_clean_file_exits_zero_no_write() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("clean.mds"); + fs::write(&target, read_fixture("fmt_formatted.mds")).unwrap(); + let before = fs::read_to_string(&target).unwrap(); + let before_mtime = fs::metadata(&target).unwrap().modified().unwrap(); + + let output = fmt_file(&target, &["--check"]); + assert!(output.status.success(), "check on clean file should exit 0"); + + assert_eq!( + fs::read_to_string(&target).unwrap(), + before, + "must not write" + ); + assert_eq!( + fs::metadata(&target).unwrap().modified().unwrap(), + before_mtime + ); +} + +#[test] +fn check_dirty_file_exits_nonzero_no_write() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("dirty.mds"); + fs::write(&target, read_fixture("fmt_unformatted.mds")).unwrap(); + let before = fs::read_to_string(&target).unwrap(); + + let output = fmt_file(&target, &["--check"]); + assert!( + !output.status.success(), + "check on a file that would change must exit non-zero" + ); + assert_eq!( + fs::read_to_string(&target).unwrap(), + before, + "--check must never write" + ); +} + +#[test] +fn dir_check_reports_would_reformat_and_exits_nonzero() { + let dir = tempfile::tempdir().unwrap(); + fs::write( + dir.path().join("dirty.mds"), + read_fixture("fmt_unformatted.mds"), + ) + .unwrap(); + + let output = fmt_dir(dir.path(), &["--check"]); + assert!( + !output.status.success(), + "dir --check with dirty files must exit non-zero" + ); + + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("would reformat"), + "expected 'would reformat' summary wording under --check, got: {stderr}" + ); + + // No file should have been touched. + let content = fs::read_to_string(dir.path().join("dirty.mds")).unwrap(); + assert!( + content.contains('\r'), + "--check must never write, CRLF should survive" + ); +} + +// ── AC-CF-5: --diff prints unified diff, writes nothing ───────────────────── + +#[test] +fn diff_prints_unified_diff_to_stdout_writes_nothing() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("dirty.mds"); + fs::write(&target, read_fixture("fmt_unformatted.mds")).unwrap(); + let before = fs::read_to_string(&target).unwrap(); + + let output = fmt_file(&target, &["--diff"]); + assert!( + output.status.success(), + "--diff alone should exit 0 even when changes exist" + ); + + let stdout = String::from_utf8(output.stdout).unwrap(); + assert!( + stdout.contains("---"), + "expected unified diff header, got: {stdout:?}" + ); + assert!( + stdout.contains("+++"), + "expected unified diff header, got: {stdout:?}" + ); + assert!( + stdout.lines().any(|l| l.starts_with('-')) || stdout.lines().any(|l| l.starts_with('+')), + "expected +/- diff lines, got: {stdout:?}" + ); + + assert_eq!( + fs::read_to_string(&target).unwrap(), + before, + "--diff must never write" + ); +} + +#[test] +fn diff_on_clean_file_prints_nothing_and_exits_zero() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("clean.mds"); + fs::write(&target, read_fixture("fmt_formatted.mds")).unwrap(); + + let output = fmt_file(&target, &["--diff"]); + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).unwrap(); + assert!( + stdout.trim().is_empty(), + "no diff expected for a clean file, got: {stdout:?}" + ); +} + +#[test] +fn diff_on_malformed_file_exits_nonzero() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("bad.mds"); + fs::write(&target, read_fixture("fmt_malformed.mds")).unwrap(); + + let output = fmt_file(&target, &["--diff"]); + assert!( + !output.status.success(), + "--diff on unparseable input must exit non-zero" + ); +} + +// ── AC-CF-6: --check --diff combined ───────────────────────────────────────── + +#[test] +fn check_and_diff_combined_prints_diff_and_exits_nonzero() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("dirty.mds"); + fs::write(&target, read_fixture("fmt_unformatted.mds")).unwrap(); + + let output = fmt_file(&target, &["--check", "--diff"]); + assert!( + !output.status.success(), + "--check --diff must exit non-zero when changes exist" + ); + let stdout = String::from_utf8(output.stdout).unwrap(); + assert!( + stdout.contains("---") && stdout.contains("+++"), + "expected a diff, got: {stdout:?}" + ); +} + +// ── AC-CF-7: no-arg auto-detect ────────────────────────────────────────────── + +#[test] +fn autodetect_single_mds_file_in_cwd() { + let dir = tempfile::tempdir().unwrap(); + fs::write( + dir.path().join("only.mds"), + read_fixture("fmt_unformatted.mds"), + ) + .unwrap(); + + let output = mds_bin() + .current_dir(dir.path()) + .arg("fmt") + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + assert!( + output.status.success(), + "auto-detect should succeed; stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + let content = fs::read_to_string(dir.path().join("only.mds")).unwrap(); + assert!( + !content.contains('\r'), + "auto-detected file should be formatted" + ); +} + +#[test] +fn autodetect_errors_on_zero_files() { + let dir = tempfile::tempdir().unwrap(); + let output = mds_bin() + .current_dir(dir.path()) + .arg("fmt") + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + assert!( + !output.status.success(), + "auto-detect with zero .mds files must fail" + ); +} + +#[test] +fn autodetect_errors_on_multiple_files() { + let dir = tempfile::tempdir().unwrap(); + fs::write(dir.path().join("a.mds"), "Hello!\n").unwrap(); + fs::write(dir.path().join("b.mds"), "Hello!\n").unwrap(); + let output = mds_bin() + .current_dir(dir.path()) + .arg("fmt") + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + assert!( + !output.status.success(), + "auto-detect with multiple .mds files must fail" + ); +} + +// ── AC-CF-8: --quiet suppresses status but never errors ───────────────────── + +#[test] +fn quiet_suppresses_status_line() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("dirty.mds"); + fs::write(&target, read_fixture("fmt_unformatted.mds")).unwrap(); + + let output = mds_bin() + .arg("--quiet") + .arg("fmt") + .arg(&target) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + assert!(output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.trim().is_empty(), + "quiet should suppress status, got: {stderr}" + ); +} + +#[test] +fn quiet_does_not_suppress_errors() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("bad.mds"); + fs::write(&target, read_fixture("fmt_malformed.mds")).unwrap(); + + let output = mds_bin() + .arg("--quiet") + .arg("fmt") + .arg(&target) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + assert!( + !output.status.success(), + "malformed input must still fail under --quiet" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + !stderr.trim().is_empty(), + "errors must still be printed under --quiet" + ); +} + +// ── Malformed input / missing / not-.mds / symlink / oversized / bad-utf8 ─── + +#[test] +fn malformed_single_file_exits_one_with_diagnostic() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("bad.mds"); + fs::write(&target, read_fixture("fmt_malformed.mds")).unwrap(); + + let output = fmt_file(&target, &[]); + assert_eq!( + output.status.code(), + Some(1), + "syntax error should map to exit 1" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("syntax") || stderr.contains("fence"), + "expected a diagnostic mentioning the syntax problem, got: {stderr}" + ); + // Must not have written a garbled file. + let content = fs::read_to_string(&target).unwrap(); + assert_eq!( + content, + read_fixture("fmt_malformed.mds"), + "malformed file must be untouched" + ); +} + +#[test] +fn dir_malformed_file_continues_and_reports_one_failed() { + let dir = tempfile::tempdir().unwrap(); + fs::write( + dir.path().join("bad.mds"), + read_fixture("fmt_malformed.mds"), + ) + .unwrap(); + fs::write( + dir.path().join("good.mds"), + read_fixture("fmt_unformatted.mds"), + ) + .unwrap(); + + let output = fmt_dir(dir.path(), &[]); + assert!(!output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("1 failed"), + "expected '1 failed' in summary, got: {stderr}" + ); + + // good.mds must still have been formatted (continue-on-error). + let good_after = fs::read_to_string(dir.path().join("good.mds")).unwrap(); + assert!( + !good_after.contains('\r'), + "good.mds should still be formatted" + ); +} + +#[test] +fn missing_file_exits_two() { + let output = fmt_file(Path::new("/tmp/no_such_fmt_target_xyz.mds"), &[]); + assert_eq!(output.status.code(), Some(2)); +} + +#[test] +fn non_mds_extension_rejected_exits_two() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("notes.txt"); + fs::write(&target, "just some text\n").unwrap(); + + let output = fmt_file(&target, &[]); + assert_eq!( + output.status.code(), + Some(2), + "non-.mds explicit file must be rejected" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("mds") || stderr.contains("extension"), + "expected a not-an-mds-file diagnostic, got: {stderr}" + ); +} + +#[test] +#[cfg(unix)] +fn symlinked_file_rejected() { + let dir = tempfile::tempdir().unwrap(); + let real = dir.path().join("real.mds"); + fs::write(&real, "Hello!\n").unwrap(); + let link = dir.path().join("link.mds"); + std::os::unix::fs::symlink(&real, &link).unwrap(); + + let output = fmt_file(&link, &[]); + assert!(!output.status.success(), "symlinked file must be rejected"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("symlink"), + "expected symlink diagnostic, got: {stderr}" + ); +} + +#[test] +#[cfg(unix)] +fn symlinked_directory_root_rejected() { + let real_dir = tempfile::tempdir().unwrap(); + fs::write(real_dir.path().join("page.mds"), "Hello!\n").unwrap(); + let link_parent = tempfile::tempdir().unwrap(); + let link_path = link_parent.path().join("linked"); + std::os::unix::fs::symlink(real_dir.path(), &link_path).unwrap(); + + let output = fmt_dir(&link_path, &[]); + assert!( + !output.status.success(), + "symlinked directory root must be rejected" + ); +} + +#[test] +fn oversized_file_exits_three() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("huge.mds"); + let content = "x".repeat(10 * 1024 * 1024 + 1); + fs::write(&target, &content).unwrap(); + + let output = fmt_file(&target, &[]); + assert_eq!( + output.status.code(), + Some(3), + "oversized source should map to exit 3" + ); +} + +#[test] +#[cfg(unix)] +fn bad_utf8_exits_two() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("bad_utf8.mds"); + std::fs::write(&target, [0xFF, 0xFE, b'\n']).unwrap(); + + let output = fmt_file(&target, &[]); + assert_eq!( + output.status.code(), + Some(2), + "invalid UTF-8 should map to exit 2" + ); +} + +// ── Channel discipline: capture stdout/stderr separately ──────────────────── + +#[test] +fn channels_stdout_only_content_stderr_only_status() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("dirty.mds"); + fs::write(&target, read_fixture("fmt_unformatted.mds")).unwrap(); + + // Write mode: nothing on stdout, status on stderr. + let output = fmt_file(&target, &[]); + assert!( + output.stdout.is_empty(), + "write mode must not print to stdout" + ); + assert!( + !output.stderr.is_empty(), + "write mode should print status to stderr" + ); +} + +#[test] +fn channels_diff_goes_to_stdout_summary_to_stderr_in_dir_mode() { + let dir = tempfile::tempdir().unwrap(); + fs::write( + dir.path().join("dirty.mds"), + read_fixture("fmt_unformatted.mds"), + ) + .unwrap(); + + let output = fmt_dir(dir.path(), &["--diff"]); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stdout.contains("---") || stdout.contains("+++"), + "diff should be on stdout: {stdout}" + ); + assert!( + !stderr.contains("---") && !stderr.contains("+++"), + "diff must not leak to stderr: {stderr}" + ); +} + +// ── Config: mds.json `fmt` section ─────────────────────────────────────────── + +#[test] +fn fmt_config_section_parses_and_pre_existing_configs_still_load() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("dirty.mds"); + fs::write(&target, read_fixture("fmt_unformatted.mds")).unwrap(); + fs::write( + dir.path().join("mds.json"), + r#"{"build": {"output_dir": "dist"}, "fmt": {"sort_frontmatter_keys": false}}"#, + ) + .unwrap(); + + let output = fmt_file(&target, &[]); + assert!( + output.status.success(), + "fmt should succeed with a fmt-section config present; stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); +} + +#[test] +fn pre_existing_config_without_fmt_section_still_loads() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("dirty.mds"); + fs::write(&target, read_fixture("fmt_unformatted.mds")).unwrap(); + fs::write( + dir.path().join("mds.json"), + r#"{"build": {"output_dir": "dist"}}"#, + ) + .unwrap(); + + let output = fmt_file(&target, &[]); + assert!( + output.status.success(), + "fmt should succeed with a pre-existing (no fmt section) config; stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); +} diff --git a/crates/mds-cli/tests/fixtures/_fmt_partial.mds b/crates/mds-cli/tests/fixtures/_fmt_partial.mds new file mode 100644 index 0000000..de7a7a9 --- /dev/null +++ b/crates/mds-cli/tests/fixtures/_fmt_partial.mds @@ -0,0 +1,7 @@ +@define greet(x): +Hello {x}! +@end + + + +@export greet diff --git a/crates/mds-cli/tests/fixtures/fmt_body_hardbreak.mds b/crates/mds-cli/tests/fixtures/fmt_body_hardbreak.mds new file mode 100644 index 0000000..5b155f6 --- /dev/null +++ b/crates/mds-cli/tests/fixtures/fmt_body_hardbreak.mds @@ -0,0 +1,2 @@ +Line with hard break. +Next line. diff --git a/crates/mds-cli/tests/fixtures/fmt_formatted.mds b/crates/mds-cli/tests/fixtures/fmt_formatted.mds new file mode 100644 index 0000000..dca9925 --- /dev/null +++ b/crates/mds-cli/tests/fixtures/fmt_formatted.mds @@ -0,0 +1,6 @@ +--- +name: World +--- +@if name: +Hello {name}! +@end diff --git a/crates/mds-cli/tests/fixtures/fmt_malformed.mds b/crates/mds-cli/tests/fixtures/fmt_malformed.mds new file mode 100644 index 0000000..060716d --- /dev/null +++ b/crates/mds-cli/tests/fixtures/fmt_malformed.mds @@ -0,0 +1,2 @@ +```text +unclosed fence content diff --git a/crates/mds-cli/tests/fixtures/fmt_parent_includes_partial.mds b/crates/mds-cli/tests/fixtures/fmt_parent_includes_partial.mds new file mode 100644 index 0000000..2988b05 --- /dev/null +++ b/crates/mds-cli/tests/fixtures/fmt_parent_includes_partial.mds @@ -0,0 +1,2 @@ +@import { greet } from "./_fmt_partial.mds" +{greet("World")} diff --git a/crates/mds-cli/tests/fixtures/fmt_unformatted.mds b/crates/mds-cli/tests/fixtures/fmt_unformatted.mds new file mode 100644 index 0000000..d92906c --- /dev/null +++ b/crates/mds-cli/tests/fixtures/fmt_unformatted.mds @@ -0,0 +1,10 @@ +--- +name: World +--- +@if name: +Hello {name}! + + + +Goodbye. +@end From 0e60fd898a296919e3e651464298ba3528383d2d Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Fri, 3 Jul 2026 11:38:02 +0300 Subject: [PATCH 03/15] docs: document mds fmt in README and CHANGELOG Adds mds fmt to the CLI Reference command list, a Fmt options block, the exit-code note for --check, and a "Formatting with mds fmt" subsection covering in-place/dir/--check/--diff/stdin usage, what gets normalized vs. deliberately left untouched, and the mds.json fmt section. Stamps the CHANGELOG [Unreleased] entry (#60). Task: feat/60-mds-fmt-auto-formatter --- CHANGELOG.md | 13 +++++++++++++ README.md | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0566456..3b72d67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **`mds fmt`** — an opinionated, safety-gated auto-formatter for `.mds` templates. Every + rewrite is guaranteed compile-equivalent: a runtime safety gate re-compiles the formatted + source and refuses to write if it would change compiled output (`mds::formatter_invariant`) + rather than silently corrupting a template. Normalizes CRLF to LF (including inside + frontmatter and code fences), collapses runs of 3+ blank lines to one, strips trailing + whitespace on directive lines, and ensures exactly one final newline — while leaving + body-text trailing whitespace (Markdown hard breaks) and the byte-for-byte content of + frontmatter / code fences / `@message` / `@define` bodies untouched. Supports a single file, + a directory (recursive, including `_`-prefixed partials), or stdin (`-`, as a filter); + `--check` exits non-zero without writing when anything would change, and `--diff` prints a + unified diff (colorized on a TTY) without writing. New public `mds-core` API: + `format_str` / `format_str_with`. (#60) + - **Native Python bindings** (`crates/mds-python`, PyO3 + maturin), to be distributed as `mdscript` on PyPI. Seven functions — `compile`, `compile_file`, `compile_virtual`, `check`, `check_file`, `check_virtual`, and `scan_imports` — diff --git a/README.md b/README.md index a40a658..15aa6ec 100644 --- a/README.md +++ b/README.md @@ -75,6 +75,7 @@ Unlike general-purpose template engines, MDS is Markdown-native: no delimiters t mds build [FILE|DIR] [OPTIONS] Compile an MDS template or directory to Markdown / JSON mds watch [FILE|DIR] [OPTIONS] Watch and auto-recompile on save mds check [FILE|DIR] [OPTIONS] Validate without rendering +mds fmt [FILE|DIR] [OPTIONS] Reformat MDS file(s) in place (opinionated, safety-gated) mds init [FILENAME] Create a starter MDS file Global options: @@ -96,15 +97,24 @@ Watch-only options: The watcher self-heals after a watched dir/root is deleted and recreated; --poll-interval controls how quickly it detects recovery. +Fmt options: + --check Read-only: exit non-zero if any file would change; never writes + --diff Read-only: print a unified diff of pending changes; never writes + (colorized only when stdout is a terminal). Combines with --check — + --diff controls what's printed, --check controls the exit code. + Exit codes: - 0 Success (or clean Ctrl+C in watch mode) - 1 Template error (syntax, undefined variable, arity mismatch) + 0 Success (or clean Ctrl+C in watch mode; or a clean `fmt --check` / `fmt --diff` preview) + 1 Template error (syntax, undefined variable, arity mismatch), or `fmt --check` found a + file that would change 2 I/O error (file not found, not an MDS file), or invalid CLI argument (clap parse error) 3 Resource limit exceeded ``` **Directory mode** (`mds build ` / `mds check `): every non-partial `.mds` file under the directory is compiled. `_`-prefixed files are partials — tracked as dependencies but never emitted to their own output. Output mirrors the source subtree (e.g. `src/a/b/foo.mds` → `dist/a/b/foo.md`). Symlinks are rejected. Errors are per-file and do not abort the run; a summary is printed and the exit code is non-zero if any file fails. Stale output files (compiled outputs with no corresponding source) are cleaned up automatically. The output extension is intrinsic: `.md` for Markdown templates, `.json` for templates with `@message` blocks. +`mds fmt ` follows the same directory-mode conventions (recursive, symlinks rejected, continue-on-error, non-zero exit summary) with one deliberate difference: it formats `_`-prefixed **partials too** — formatting rewrites source, not compiled output, and a partial's source is just as much a candidate for reformatting as any other file. + ### Live preview with `mds watch` Watch a single file and recompile whenever it (or any of its imports) changes: @@ -143,6 +153,47 @@ shared partial rebuilds **all transitive importers** automatically. - Ctrl+C exits with code 0 and prints `Stopped watching.` - `--vars` file is reloaded from disk on every rebuild; edits to it trigger a recompile. +### Formatting with `mds fmt` + +An opinionated, safety-gated auto-formatter. Every rewrite is guaranteed **compile-equivalent**: +before writing anything, `mds fmt` re-compiles the formatted source and refuses to write if it +would change compiled output — a formatter bug surfaces as a clean error, never a silent +corruption of your template. + +```bash +mds fmt template.mds # format a file in place +mds fmt . # format every .mds file recursively (partials included) +mds fmt --check template.mds # exit 1 if the file would change; never writes — for CI +mds fmt --diff template.mds # print a unified diff of pending changes; never writes +mds fmt --check --diff . # show diffs for every file that would change; exit 1 if any would +echo 'Hello {name}!' | mds fmt - # format from stdin, write to stdout; creates no file +``` + +What it normalizes: + +- CRLF → LF, everywhere (including inside frontmatter and code fences) +- Runs of 3+ blank lines collapse to one; leading blank lines in the body are removed entirely +- Trailing whitespace on `@if`/`@for`/`@define`/… directive lines is stripped +- Exactly one final newline (empty or whitespace-only input formats to an empty file) + +What it deliberately leaves untouched: + +- Trailing whitespace on body-text content lines, including whitespace-only "blank" lines — two + trailing spaces are a Markdown hard line break, and stripping them (anywhere in body text, + including a stray blank line) can change rendered output +- The byte-for-byte content of frontmatter, code fences, and `@message`/`@define` bodies (the + latter two can be consumed in a context that bypasses the compiler's own whitespace + normalization, so the formatter never touches them) + +Directory mode formats every `.mds` file recursively, **including `_`-prefixed partials**, +continuing past per-file errors and printing a summary +(`N formatted, M unchanged, K failed`, or `N would reformat, K failed` under `--check`). A file +is only written (and its mtime touched) when its content actually changes. Status lines and +summaries go to stderr; `--diff` output and stdin filter-mode content go to stdout; `--quiet` +suppresses status but never errors. Reads a `fmt` section from `mds.json` +(`{"fmt": {"sort_frontmatter_keys": true}}`) for forward compatibility — the field doesn't drive +any formatting behavior yet; frontmatter key sorting is deferred to a future version. + ## Bundler Integration Import `.mds` templates directly in Vite, Rollup, Webpack, and Rspack projects: From 886d56537bf4298a6e5290b094a795d6aa87268c Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Fri, 3 Jul 2026 19:04:03 +0300 Subject: [PATCH 04/15] refactor(core): surface unclosed-block errors as Syntax, not FormatterInvariant The assert_equivalent gate now explicitly handles parse-level Syntax errors (like unclosed @message/@if/@for/@define/@block blocks) by surfacing them verbatim, matching mds build/check behavior. This prevents misleading FormatterInvariant reports for what is ordinary author error (missing @end). Engine fmt tests increased to 43 with two new syntax-error tests: - syntax_error_unclosed_directive_blocks_surface_syntax_not_formatter_invariant: validates that 5 block type variants all surface Syntax, not a silent pass or a FormatterInvariant. - syntax_error_unclosed_message_with_trailing_ws_is_syntax_not_formatter_invariant: validates the specific case where trailing ws on the final body line previously triggered a FormatterInvariant (rewrite diverged from the raw region), now a clean Syntax error. Co-Authored-By: Claude --- crates/mds-core/src/formatter.rs | 30 ++++++++++++++++---- crates/mds-core/tests/fmt.rs | 48 +++++++++++++++++++++++++++++++- 2 files changed, 72 insertions(+), 6 deletions(-) diff --git a/crates/mds-core/src/formatter.rs b/crates/mds-core/src/formatter.rs index ab84ac1..e9ede6b 100644 --- a/crates/mds-core/src/formatter.rs +++ b/crates/mds-core/src/formatter.rs @@ -495,11 +495,23 @@ fn rewrite_body( /// match. `compile_str_collecting_warnings` (not `compile_str_with`) is used /// so the gate never duplicates warnings to stderr on every format call. /// -/// When `source` does NOT compile standalone (a minority case — typically an -/// undefined runtime variable; imports still resolve via `base_dir`), falls -/// back to a structural, rule-aware token comparison so the formatter can -/// still succeed on templates that only work with runtime vars supplied at -/// render time. +/// When `source` does NOT compile standalone the reason matters: +/// +/// - A [`MdsError::Syntax`] means the source is malformed at the lex/parse +/// level — most notably an unclosed `@message`/`@if`/`@for`/`@define`/ +/// `@block` block, which (unlike an unclosed code fence or interpolation) +/// tokenizes cleanly and only fails when the parser looks for the matching +/// `@end`. There is no well-formed program to format, so the real syntax +/// error is surfaced verbatim — matching `mds build` / `mds check` — instead +/// of silently emitting a still-broken file or a misleading +/// `FormatterInvariant` for what is ordinary author error. +/// - Any OTHER compile error (a minority case — typically an undefined runtime +/// variable or function; imports still resolve via `base_dir`) means the +/// source parsed into a well-formed token stream and only failed later during +/// name resolution / evaluation. Those templates are legitimately formattable, +/// so the gate falls back to a structural, rule-aware token comparison and +/// still succeeds on sources that only compile with runtime vars supplied at +/// render time. fn assert_equivalent( source: &str, formatted: &str, @@ -516,6 +528,14 @@ fn assert_equivalent( "formatted source failed to compile though the original succeeded: {e}" ))), }, + // A lex/parse `Syntax` error means the source is genuinely malformed + // (e.g. an unclosed `@message`/`@if`/`@for` block, which tokenizes but + // fails at parse time). There is nothing safe to format: surface the + // real error rather than papering over it with the structural check. + Err(e @ MdsError::Syntax { .. }) => Err(e), + // Any other compile failure (undefined var/fn, unresolved import, …) + // means the token stream is well-formed and only later analysis failed, + // so the rule-aware structural comparison is a meaningful fallback. Err(_) => { if structural_equivalent(source, formatted, raw_content) { Ok(()) diff --git a/crates/mds-core/tests/fmt.rs b/crates/mds-core/tests/fmt.rs index 420560f..d4bcb7e 100644 --- a/crates/mds-core/tests/fmt.rs +++ b/crates/mds-core/tests/fmt.rs @@ -30,7 +30,7 @@ const CORPUS: &[&str] = &[ " \n", "No trailing newline", "---\nname: x\n---\n", - "---\npremium: true\n---\nBefore\n\n\n\nAfter @if premium: inline text {premium}\n@end\n", + "---\npremium: true\n---\nBefore\n\n\n\nAfter\n@if premium:\ninline text {premium}\n@end\n", ]; fn compiled_markdown(src: &str) -> Option { @@ -287,6 +287,52 @@ fn syntax_error_unclosed_frontmatter_is_err() { assert!(matches!(err, MdsError::Syntax { .. })); } +// ── AC-EF-8 (parse-level): unclosed directive blocks surface Syntax, never +// a silent format or a misleading FormatterInvariant ────────────────────── +// +// Unlike an unclosed code fence / interpolation / frontmatter (all of which +// fail in the LEXER, so `format_str` errors before it ever rewrites), an +// unclosed `@message`/`@if`/`@for`/`@define`/`@block` tokenizes cleanly and +// only fails at PARSE time. Such malformed input must still be rejected with +// the real syntax error — exactly like `mds build`/`mds check` — rather than +// being silently accepted (when the rewrite is a no-op) or reported as a +// `FormatterInvariant` "please file a bug" (when the whole-body trim happens +// to touch the unclosed block's trailing whitespace). + +#[test] +fn syntax_error_unclosed_directive_blocks_surface_syntax_not_formatter_invariant() { + // "Clean" variants (rewrite is a no-op): previously silently returned Ok. + for src in [ + "@message user:\nHi\n", + "@if x:\nHi\n", + "@for a in items:\n- {a}\n", + "@define greet(x):\nHello {x}\n", + "@block b:\nStuff\n", + ] { + match format_str(src) { + Err(MdsError::Syntax { .. }) => {} + other => panic!("expected Syntax error for unclosed block {src:?}, got: {other:?}"), + } + } +} + +#[test] +fn syntax_error_unclosed_message_with_trailing_ws_is_syntax_not_formatter_invariant() { + // Trailing whitespace on the final body line makes the whole-body trim_end + // diverge from the byte-exact raw region -> this is the exact input that + // previously produced `FormatterInvariant`. It must now be a clean Syntax + // error (the source is simply missing an `@end`). + let err = format_str("@message user:\nHi ").unwrap_err(); + assert!( + matches!(err, MdsError::Syntax { .. }), + "expected Syntax error, got: {err:?}" + ); + assert!( + !matches!(err, MdsError::FormatterInvariant { .. }), + "unclosed @message is author error, not a formatter bug" + ); +} + // ── T1-gate-fallback: undefined-var source still formats; import source uses full gate ── #[test] From 1bd44a4f832ad6f727ff96f68efd9e50847bfba4 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Fri, 3 Jul 2026 19:04:08 +0300 Subject: [PATCH 05/15] test(cli): consolidate fmt_file/fmt_dir into a single fmt_path helper The fmt_file and fmt_dir helpers were byte-identical; mds fmt dispatches on filesystem type (file, dir, or stdin) at runtime, so a single fmt_path helper eliminates duplication and improves clarity. CLI fmt tests increased to 35 with one new test: - unclosed_directive_block_exits_one_with_syntax_not_formatter_invariant: validates that the CLI surfaces the same Syntax error for unclosed @message blocks as the core formatter (not a FormatterInvariant nor a silent format). Co-Authored-By: Claude --- crates/mds-cli/tests/cli_fmt.rs | 105 ++++++++++++++++++++------------ 1 file changed, 66 insertions(+), 39 deletions(-) diff --git a/crates/mds-cli/tests/cli_fmt.rs b/crates/mds-cli/tests/cli_fmt.rs index 9c42cf6..22f4bfe 100644 --- a/crates/mds-cli/tests/cli_fmt.rs +++ b/crates/mds-cli/tests/cli_fmt.rs @@ -26,18 +26,12 @@ fn read_fixture(name: &str) -> String { fs::read_to_string(fixture(name)).expect("read fixture") } -fn fmt_file(path: &Path, extra_args: &[&str]) -> std::process::Output { - mds_bin() - .arg("fmt") - .arg(path) - .args(extra_args) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .output() - .unwrap() -} - -fn fmt_dir(path: &Path, extra_args: &[&str]) -> std::process::Output { +/// Run `mds fmt `, capturing stdout/stderr separately. +/// +/// `mds fmt` dispatches on the filesystem type of `path` (file, dir, or `-` +/// for stdin — see the dedicated `fmt_stdin` helper below), so one helper +/// covers both single-file and directory-mode invocations. +fn fmt_path(path: &Path, extra_args: &[&str]) -> std::process::Output { mds_bin() .arg("fmt") .arg(path) @@ -57,7 +51,7 @@ fn inplace_rewrites_only_when_changed() { fs::write(&target, read_fixture("fmt_unformatted.mds")).unwrap(); let original = fs::read_to_string(&target).unwrap(); - let output = fmt_file(&target, &[]); + let output = fmt_path(&target, &[]); assert!( output.status.success(), "fmt should succeed; stderr: {}", @@ -85,7 +79,7 @@ fn inplace_unchanged_file_preserves_mtime_and_reports_unchanged() { // Ensure the filesystem clock would visibly move if the file were rewritten. std::thread::sleep(Duration::from_millis(1100)); - let output = fmt_file(&target, &[]); + let output = fmt_path(&target, &[]); assert!( output.status.success(), "fmt should succeed on an already-clean file" @@ -111,10 +105,10 @@ fn format_then_check_is_idempotent() { let target = dir.path().join("idem.mds"); fs::write(&target, read_fixture("fmt_unformatted.mds")).unwrap(); - let first = fmt_file(&target, &[]); + let first = fmt_path(&target, &[]); assert!(first.status.success()); - let check = fmt_file(&target, &["--check"]); + let check = fmt_path(&target, &["--check"]); assert!( check.status.success(), "second pass under --check should report clean; stderr: {}", @@ -237,7 +231,7 @@ fn dir_recurse_formats_every_mds_including_partials() { ) .unwrap(); - let output = fmt_dir(dir.path(), &[]); + let output = fmt_path(dir.path(), &[]); assert!( output.status.success(), "dir fmt should succeed; stderr: {}", @@ -283,7 +277,7 @@ fn dir_continue_on_error_summary_nonzero_exit() { fs::write(dir.path().join("good.mds"), "Hello, world!\n").unwrap(); fs::write(dir.path().join("bad.mds"), "```\nunclosed fence\n").unwrap(); - let output = fmt_dir(dir.path(), &[]); + let output = fmt_path(dir.path(), &[]); assert!( !output.status.success(), "dir fmt with a failing file must exit non-zero" @@ -305,7 +299,7 @@ fn dir_already_formatted_tree_reports_all_unchanged_exit_zero() { ) .unwrap(); - let output = fmt_dir(dir.path(), &[]); + let output = fmt_path(dir.path(), &[]); assert!(output.status.success()); let stderr = String::from_utf8_lossy(&output.stderr); assert!( @@ -336,7 +330,7 @@ fn parent_that_includes_partial_compiles_unchanged_after_formatting_both() { .unwrap(); assert!(before.status.success(), "pre-format build should succeed"); - let output = fmt_dir(dir.path(), &[]); + let output = fmt_path(dir.path(), &[]); assert!( output.status.success(), "formatting the dir should succeed; stderr: {}", @@ -367,7 +361,7 @@ fn check_clean_file_exits_zero_no_write() { let before = fs::read_to_string(&target).unwrap(); let before_mtime = fs::metadata(&target).unwrap().modified().unwrap(); - let output = fmt_file(&target, &["--check"]); + let output = fmt_path(&target, &["--check"]); assert!(output.status.success(), "check on clean file should exit 0"); assert_eq!( @@ -388,7 +382,7 @@ fn check_dirty_file_exits_nonzero_no_write() { fs::write(&target, read_fixture("fmt_unformatted.mds")).unwrap(); let before = fs::read_to_string(&target).unwrap(); - let output = fmt_file(&target, &["--check"]); + let output = fmt_path(&target, &["--check"]); assert!( !output.status.success(), "check on a file that would change must exit non-zero" @@ -409,7 +403,7 @@ fn dir_check_reports_would_reformat_and_exits_nonzero() { ) .unwrap(); - let output = fmt_dir(dir.path(), &["--check"]); + let output = fmt_path(dir.path(), &["--check"]); assert!( !output.status.success(), "dir --check with dirty files must exit non-zero" @@ -438,7 +432,7 @@ fn diff_prints_unified_diff_to_stdout_writes_nothing() { fs::write(&target, read_fixture("fmt_unformatted.mds")).unwrap(); let before = fs::read_to_string(&target).unwrap(); - let output = fmt_file(&target, &["--diff"]); + let output = fmt_path(&target, &["--diff"]); assert!( output.status.success(), "--diff alone should exit 0 even when changes exist" @@ -471,7 +465,7 @@ fn diff_on_clean_file_prints_nothing_and_exits_zero() { let target = dir.path().join("clean.mds"); fs::write(&target, read_fixture("fmt_formatted.mds")).unwrap(); - let output = fmt_file(&target, &["--diff"]); + let output = fmt_path(&target, &["--diff"]); assert!(output.status.success()); let stdout = String::from_utf8(output.stdout).unwrap(); assert!( @@ -486,7 +480,7 @@ fn diff_on_malformed_file_exits_nonzero() { let target = dir.path().join("bad.mds"); fs::write(&target, read_fixture("fmt_malformed.mds")).unwrap(); - let output = fmt_file(&target, &["--diff"]); + let output = fmt_path(&target, &["--diff"]); assert!( !output.status.success(), "--diff on unparseable input must exit non-zero" @@ -501,7 +495,7 @@ fn check_and_diff_combined_prints_diff_and_exits_nonzero() { let target = dir.path().join("dirty.mds"); fs::write(&target, read_fixture("fmt_unformatted.mds")).unwrap(); - let output = fmt_file(&target, &["--check", "--diff"]); + let output = fmt_path(&target, &["--check", "--diff"]); assert!( !output.status.success(), "--check --diff must exit non-zero when changes exist" @@ -634,7 +628,7 @@ fn malformed_single_file_exits_one_with_diagnostic() { let target = dir.path().join("bad.mds"); fs::write(&target, read_fixture("fmt_malformed.mds")).unwrap(); - let output = fmt_file(&target, &[]); + let output = fmt_path(&target, &[]); assert_eq!( output.status.code(), Some(1), @@ -654,6 +648,39 @@ fn malformed_single_file_exits_one_with_diagnostic() { ); } +#[test] +fn unclosed_directive_block_exits_one_with_syntax_not_formatter_invariant() { + // An unclosed `@message` (parse-level malformation, unlike the unclosed + // code fence in fmt_malformed.mds which fails in the lexer) must be + // rejected with the real syntax error and leave the file untouched — + // not silently "formatted" nor reported as a formatter bug. + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("unclosed.mds"); + let src = "@message user:\nHi there\n"; + fs::write(&target, src).unwrap(); + + let output = fmt_path(&target, &[]); + assert_eq!( + output.status.code(), + Some(1), + "unclosed block should map to exit 1" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("syntax") && stderr.contains("@end"), + "expected a syntax diagnostic about the missing @end, got: {stderr}" + ); + assert!( + !stderr.contains("formatter_invariant") && !stderr.contains("file an issue"), + "author error must not be reported as a formatter bug, got: {stderr}" + ); + assert_eq!( + fs::read_to_string(&target).unwrap(), + src, + "malformed file must be left untouched" + ); +} + #[test] fn dir_malformed_file_continues_and_reports_one_failed() { let dir = tempfile::tempdir().unwrap(); @@ -668,7 +695,7 @@ fn dir_malformed_file_continues_and_reports_one_failed() { ) .unwrap(); - let output = fmt_dir(dir.path(), &[]); + let output = fmt_path(dir.path(), &[]); assert!(!output.status.success()); let stderr = String::from_utf8_lossy(&output.stderr); assert!( @@ -686,7 +713,7 @@ fn dir_malformed_file_continues_and_reports_one_failed() { #[test] fn missing_file_exits_two() { - let output = fmt_file(Path::new("/tmp/no_such_fmt_target_xyz.mds"), &[]); + let output = fmt_path(Path::new("/tmp/no_such_fmt_target_xyz.mds"), &[]); assert_eq!(output.status.code(), Some(2)); } @@ -696,7 +723,7 @@ fn non_mds_extension_rejected_exits_two() { let target = dir.path().join("notes.txt"); fs::write(&target, "just some text\n").unwrap(); - let output = fmt_file(&target, &[]); + let output = fmt_path(&target, &[]); assert_eq!( output.status.code(), Some(2), @@ -718,7 +745,7 @@ fn symlinked_file_rejected() { let link = dir.path().join("link.mds"); std::os::unix::fs::symlink(&real, &link).unwrap(); - let output = fmt_file(&link, &[]); + let output = fmt_path(&link, &[]); assert!(!output.status.success(), "symlinked file must be rejected"); let stderr = String::from_utf8_lossy(&output.stderr); assert!( @@ -736,7 +763,7 @@ fn symlinked_directory_root_rejected() { let link_path = link_parent.path().join("linked"); std::os::unix::fs::symlink(real_dir.path(), &link_path).unwrap(); - let output = fmt_dir(&link_path, &[]); + let output = fmt_path(&link_path, &[]); assert!( !output.status.success(), "symlinked directory root must be rejected" @@ -750,7 +777,7 @@ fn oversized_file_exits_three() { let content = "x".repeat(10 * 1024 * 1024 + 1); fs::write(&target, &content).unwrap(); - let output = fmt_file(&target, &[]); + let output = fmt_path(&target, &[]); assert_eq!( output.status.code(), Some(3), @@ -765,7 +792,7 @@ fn bad_utf8_exits_two() { let target = dir.path().join("bad_utf8.mds"); std::fs::write(&target, [0xFF, 0xFE, b'\n']).unwrap(); - let output = fmt_file(&target, &[]); + let output = fmt_path(&target, &[]); assert_eq!( output.status.code(), Some(2), @@ -782,7 +809,7 @@ fn channels_stdout_only_content_stderr_only_status() { fs::write(&target, read_fixture("fmt_unformatted.mds")).unwrap(); // Write mode: nothing on stdout, status on stderr. - let output = fmt_file(&target, &[]); + let output = fmt_path(&target, &[]); assert!( output.stdout.is_empty(), "write mode must not print to stdout" @@ -802,7 +829,7 @@ fn channels_diff_goes_to_stdout_summary_to_stderr_in_dir_mode() { ) .unwrap(); - let output = fmt_dir(dir.path(), &["--diff"]); + let output = fmt_path(dir.path(), &["--diff"]); let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); assert!( @@ -828,7 +855,7 @@ fn fmt_config_section_parses_and_pre_existing_configs_still_load() { ) .unwrap(); - let output = fmt_file(&target, &[]); + let output = fmt_path(&target, &[]); assert!( output.status.success(), "fmt should succeed with a fmt-section config present; stderr: {}", @@ -847,7 +874,7 @@ fn pre_existing_config_without_fmt_section_still_loads() { ) .unwrap(); - let output = fmt_file(&target, &[]); + let output = fmt_path(&target, &[]); assert!( output.status.success(), "fmt should succeed with a pre-existing (no fmt section) config; stderr: {}", From c32964bd2c1659b6442a331209dbbe408f1ecf58 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 7 Jul 2026 20:08:50 +0300 Subject: [PATCH 06/15] =?UTF-8?q?refactor(core):=20rename=20FormatterInvar?= =?UTF-8?q?iant=20field=20`detail`=20=E2=86=92=20`message`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `FormatterInvariant` was the lone free-form-string variant in `MdsError` not named `message`; every other variant (Syntax, Io, ResourceLimit, BuiltinError, ImportError, ExportError, Extends, YamlError, JsonError) uses `message`. Rename for consistency before the variant ships in a public release. No binding (napi/wasm/python) exposes this variant yet, so this is a no-cost breaking change while still in [Unreleased]. Update the `#[error]` format string, the `formatter_invariant()` helper parameter, and the two struct-literal construction sites in tests/api_surface.rs. The `{ .. }` match arm in tests/fmt.rs needs no change. Co-Authored-By: Claude --- crates/mds-core/src/error.rs | 8 ++++---- crates/mds-core/tests/api_surface.rs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/mds-core/src/error.rs b/crates/mds-core/src/error.rs index 335c422..3d5688a 100644 --- a/crates/mds-core/src/error.rs +++ b/crates/mds-core/src/error.rs @@ -337,12 +337,12 @@ pub enum MdsError { /// did, or the two compiled to different output. This signals a formatter /// bug, not a problem with the input template — the CLI must not write the /// file when this occurs. - #[error("formatter produced non-equivalent output: {detail}")] + #[error("formatter produced non-equivalent output: {message}")] #[diagnostic( code(mds::formatter_invariant), help("this indicates a bug in `mds fmt` itself; please file an issue") )] - FormatterInvariant { detail: String }, + FormatterInvariant { message: String }, } impl MdsError { @@ -685,9 +685,9 @@ impl MdsError { /// Construct a `FormatterInvariant` error, signaling that the formatter's /// rewritten source failed the compile-equivalence safety gate. - pub(crate) fn formatter_invariant(detail: impl Into) -> Self { + pub(crate) fn formatter_invariant(message: impl Into) -> Self { MdsError::FormatterInvariant { - detail: detail.into(), + message: message.into(), } } diff --git a/crates/mds-core/tests/api_surface.rs b/crates/mds-core/tests/api_surface.rs index 076c056..7a833e8 100644 --- a/crates/mds-core/tests/api_surface.rs +++ b/crates/mds-core/tests/api_surface.rs @@ -122,7 +122,7 @@ fn mds_error_variants_exist() { src: None, }; let _ = MdsError::FormatterInvariant { - detail: "test".to_string(), + message: "test".to_string(), }; #[allow(unreachable_patterns)] @@ -154,7 +154,7 @@ fn mds_error_variants_exist() { #[test] fn formatter_invariant_has_diagnostic_code() { let err = MdsError::FormatterInvariant { - detail: "test detail".to_string(), + message: "test detail".to_string(), }; let code = miette::Diagnostic::code(&err) .map(|c| c.to_string()) From d6fad0d3c27e173283c18644a1334c953ff4ea35 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 7 Jul 2026 20:10:19 +0300 Subject: [PATCH 07/15] feat(core): add debug_assert! slice/span invariants in formatter (R1) The byte-range rewrite in formatter.rs relies on three unstated invariants with no production assertion: 1. `body_start` must be a char boundary before `source[..body_start]` 2. `pos` (advancing cursor in rewrite_body's loop) must be a char boundary before `source[pos..]` 3. `protected` and `raw_content` spans must be sorted and non-overlapping for the advancing cursors (pi / ri) to be correct Add zero-cost `debug_assert!` guards at each site, matching the project's reliability.md / rust.md mandates (debug_assert! for invariants in hot paths). A future lexer regression will now fail loudly in debug rather than silently mis-slicing. Co-Authored-By: Claude --- crates/mds-core/src/formatter.rs | 114 ++++++++++++++++++++++++++++++- 1 file changed, 112 insertions(+), 2 deletions(-) diff --git a/crates/mds-core/src/formatter.rs b/crates/mds-core/src/formatter.rs index e9ede6b..febab63 100644 --- a/crates/mds-core/src/formatter.rs +++ b/crates/mds-core/src/formatter.rs @@ -336,6 +336,10 @@ fn rewrite( // invisible to `clean_output`'s leading/trailing-trim and newline-run // capping, which only ever see the body. if body_start > 0 { + debug_assert!( + source.is_char_boundary(body_start), + "body_start ({body_start}) must be a char boundary in source" + ); push_stripped_cr(&mut out, &source[..body_start]); } @@ -382,6 +386,18 @@ fn rewrite_body( raw_content: &[Range], directives: &BTreeSet, ) -> String { + // Invariant: spans must be sorted and non-overlapping so the advancing + // cursors (pi / ri) never overshoot. A future lexer regression would cause + // silent mis-classification rather than a crash without these guards. + debug_assert!( + protected.windows(2).all(|w| w[0].end <= w[1].start), + "protected spans must be sorted and non-overlapping" + ); + debug_assert!( + raw_content.windows(2).all(|w| w[0].end <= w[1].start), + "raw_content spans must be sorted and non-overlapping" + ); + let end = source.len(); let mut out = String::with_capacity(end.saturating_sub(body_start)); @@ -404,6 +420,10 @@ fn rewrite_body( } while pos < end { + debug_assert!( + source.is_char_boundary(pos), + "byte offset {pos} is not a char boundary in source" + ); let line_start = pos; let line_end = source[pos..].find('\n').map(|rel| pos + rel).unwrap_or(end); let had_newline = line_end < end; @@ -549,6 +569,18 @@ fn assert_equivalent( } } +/// Returns `true` when `offset` falls inside any span in `raw_content`. +/// +/// Requires `raw_content` to be sorted by start offset and non-overlapping +/// (as guaranteed by [`raw_content_spans`]), enabling O(log S) binary search +/// rather than an O(S) linear scan per call. +fn in_raw_content(raw_content: &[Range], offset: usize) -> bool { + // partition_point returns the first index where r.end > offset, i.e. the + // first span that has NOT already ended before our offset. + let idx = raw_content.partition_point(|r| r.end <= offset); + idx < raw_content.len() && raw_content[idx].start <= offset +} + /// Rule-aware structural comparison used when neither `source` nor /// `formatted` can be compiled standalone (e.g. an undefined runtime /// variable). Re-tokenizes both and compares token-for-token: `Directive` @@ -564,6 +596,10 @@ fn assert_equivalent( /// incorrectly treat some byte differences as insignificant) keeps this /// fallback correct in its own right rather than merely accidentally correct /// because of that upstream guarantee. +/// +/// The raw-content span lookup uses [`in_raw_content`] (binary search, O(log S)) +/// rather than a linear scan, since spans are sorted by [`raw_content_spans`] +/// and token offsets from the source tokenization are monotonically increasing. fn structural_equivalent(source: &str, formatted: &str, raw_content: &[Range]) -> bool { let (Ok(src_tokens), Ok(fmt_tokens)) = (lexer::tokenize(source, ""), lexer::tokenize(formatted, "")) @@ -580,7 +616,7 @@ fn structural_equivalent(source: &str, formatted: &str, raw_content: &[Range { - if raw_content.iter().any(|r| r.contains(oa)) { + if in_raw_content(raw_content, *oa) { ta == tb } else { crate::clean_output(ta) == crate::clean_output(tb) @@ -595,7 +631,7 @@ fn structural_equivalent(source: &str, formatted: &str, raw_content: &[Range fa == fb, (Token::CodeContent(ca, oa), Token::CodeContent(cb, _)) => { - if raw_content.iter().any(|r| r.contains(oa)) { + if in_raw_content(raw_content, *oa) { ca == cb } else { ca.replace('\r', "") == cb.replace('\r', "") @@ -749,4 +785,78 @@ mod tests { let s = "no carriage returns here"; assert!(matches!(strip_cr(s), std::borrow::Cow::Borrowed(_))); } + + // ── in_raw_content ──────────────────────────────────────────────────────── + + #[test] + fn in_raw_content_empty_spans_always_false() { + assert!(!in_raw_content(&[], 0)); + assert!(!in_raw_content(&[], 42)); + } + + #[test] + fn in_raw_content_inside_span() { + // span covers bytes 10..20 + let span = 10_usize..20_usize; + let spans = std::slice::from_ref(&span); + assert!(in_raw_content(spans, 10), "start of span"); + assert!(in_raw_content(spans, 15), "middle of span"); + assert!(!in_raw_content(spans, 20), "end (exclusive) not inside"); + assert!(!in_raw_content(spans, 9), "just before span"); + } + + #[test] + fn in_raw_content_multiple_sorted_spans() { + let spans = vec![5_usize..10_usize, 20_usize..30_usize]; + assert!(!in_raw_content(&spans, 4)); + assert!(in_raw_content(&spans, 5)); + assert!(in_raw_content(&spans, 9)); + assert!(!in_raw_content(&spans, 10)); // gap between spans + assert!(!in_raw_content(&spans, 19)); + assert!(in_raw_content(&spans, 20)); + assert!(in_raw_content(&spans, 29)); + assert!(!in_raw_content(&spans, 30)); + } + + // ── structural_equivalent ───────────────────────────────────────────────── + + #[test] + fn structural_equivalent_inside_raw_span_is_byte_exact_not_clean_output() { + // A @message body is raw content: blank-line runs are NOT normalised by + // clean_output there. structural_equivalent must compare those tokens + // byte-for-byte, NOT via clean_output. + let src = "@message user:\nHi\n\n\nthere\n@end\n"; + // Same text -- must be considered equivalent. + let same = "@message user:\nHi\n\n\nthere\n@end\n"; + // Collapsed blank lines inside the body -- must NOT be considered equivalent. + let collapsed = "@message user:\nHi\nthere\n@end\n"; + + let tokens = lexer::tokenize(src, "").unwrap(); + let raw = raw_content_spans(&tokens, src); + + assert!( + structural_equivalent(src, same, &raw), + "identical source should be structural_equivalent" + ); + assert!( + !structural_equivalent(src, collapsed, &raw), + "collapsing blank lines inside a raw-content span must NOT be \ + considered structural_equivalent" + ); + } + + #[test] + fn structural_equivalent_outside_raw_span_uses_clean_output() { + // Outside @message/@define bodies, clean_output normalisation is applied: + // three raw newlines vs two are structurally equivalent (R3 cap). + let src = "Hello\n\n\nworld\n"; + let capped = "Hello\n\nworld\n"; + let raw: Vec> = vec![]; + + assert!( + structural_equivalent(src, capped, &raw), + "text outside raw spans is compared via clean_output; \ + a 3-newline run and a 2-newline run should be equivalent" + ); + } } From d2c3fb9c50cbb20b36bf643595afe7d3232f1daf Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 7 Jul 2026 20:10:38 +0300 Subject: [PATCH 08/15] =?UTF-8?q?docs(core):=20back-reference=20fmt?= =?UTF-8?q?=E2=86=92clean=5Foutput=20dependency=20in=20lib.rs=20and=20eval?= =?UTF-8?q?uator.rs=20(A1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The formatter's safety model has a critical three-way dependency that was documented only in formatter.rs and KNOWLEDGE.md: - clean_output (lib.rs) is the ceiling of what mds fmt may change in markdown-mode content - @message/@define bodies bypass clean_output entirely (evaluator.rs line 845: plain `.trim()`, no clean_output call), so their content reaches compiled JSON verbatim - formatter.rs::raw_content_spans protects those bodies from R1/R3 transforms precisely because of this bypass Adding back-reference comments at both dependents (clean_output and collect_single_message) makes the coupling visible locally. The A1 shared-constant extraction (block-opener keywords duplicated between formatter.rs and parser.rs) is deferred to tech debt — the back-ref comments deliver the core value with no behavioural risk. Co-Authored-By: Claude --- crates/mds-core/src/evaluator.rs | 7 +++++++ crates/mds-core/src/lib.rs | 8 ++++++++ 2 files changed, 15 insertions(+) diff --git a/crates/mds-core/src/evaluator.rs b/crates/mds-core/src/evaluator.rs index 5bd793d..88a8d10 100644 --- a/crates/mds-core/src/evaluator.rs +++ b/crates/mds-core/src/evaluator.rs @@ -842,6 +842,13 @@ fn collect_single_message( } // Evaluate the body as plain text and trim surrounding whitespace. + // NOTE: this path uses only `.trim()`, NOT `clean_output` — `\r` characters + // and blank-line runs inside a @message body survive verbatim into the + // compiled JSON output. `mds fmt` relies on this: `raw_content_spans` in + // `formatter.rs` marks @message and @define bodies as raw content to prevent + // R1 (`\r` strip) and R3 (blank-line-run cap) from silently changing compiled + // output. Adding a `clean_output` call here would break the formatter's + // safety model without any corresponding formatter change. let content = evaluate_nodes(&block.body, scope, ctx)?.trim().to_string(); // Skip empty messages. diff --git a/crates/mds-core/src/lib.rs b/crates/mds-core/src/lib.rs index 1b54b65..5b321cc 100644 --- a/crates/mds-core/src/lib.rs +++ b/crates/mds-core/src/lib.rs @@ -638,6 +638,14 @@ pub(crate) fn prepend_frontmatter(raw: Option<&str>, body: String) -> String { /// Clean up output whitespace: collapse 3+ consecutive newlines to 2 (one blank line), /// and trim leading/trailing blank lines. +/// +/// **Formatter dependency**: `mds fmt` (`formatter.rs`) treats this function as the +/// ceiling of what it may safely change in markdown-mode content — any transform the +/// formatter applies must produce output that survives this normalisation unchanged. +/// Critically, `@message` and `@define` body content bypasses this function entirely +/// (routed through `.trim()` only in `collect_single_message`, `evaluator.rs`), so +/// the formatter protects those regions as raw content; see `raw_content_spans` in +/// `formatter.rs` for the authoritative source of that distinction. pub(crate) fn clean_output(s: &str) -> String { let mut result = String::with_capacity(s.len()); let mut newline_count = 0; From a0d4b9e80e70e98f7e7c6f42fc0f89a69564ab51 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 7 Jul 2026 20:22:16 +0300 Subject: [PATCH 09/15] refactor(cli): bundle fmt flags into FmtFlags; scope MAX_DEPTH locally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three positional bool params (check, diff, quiet) were threaded through run_fmt_stdin / run_fmt_file / run_fmt_directory in identical order — a silent-transposition hazard since all three are `bool` and the compiler cannot distinguish them. Bundle them into a private `FmtFlags { check, diff, quiet }` struct (#[derive(Clone, Copy)]); build it once in run_fmt and pass it to all three helpers. Each helper destructures it at the top to keep the function body unchanged. Also moves `MAX_DEPTH` from module scope into run_fmt_directory, matching the function-local placement used by run_build_directory / run_check_directory in build.rs. Co-Authored-By: Claude --- crates/mds-cli/src/fmt.rs | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/crates/mds-cli/src/fmt.rs b/crates/mds-cli/src/fmt.rs index c9c27cf..aea1dfe 100644 --- a/crates/mds-cli/src/fmt.rs +++ b/crates/mds-cli/src/fmt.rs @@ -31,9 +31,6 @@ use miette::Result; use crate::build::{load_config, read_stdin, resolve_input}; use crate::output::collect_mds_files; -/// Directory recursion depth cap, matching `run_build_directory` / `run_check_directory`. -const MAX_DEPTH: usize = 64; - pub(crate) struct FmtArgs { pub(crate) input: Option, pub(crate) check: bool, @@ -41,6 +38,15 @@ pub(crate) struct FmtArgs { pub(crate) quiet: bool, } +/// Bundled mode flags passed to every per-input helper — avoids the silent +/// transposition hazard of three consecutive positional `bool` parameters. +#[derive(Clone, Copy)] +struct FmtFlags { + check: bool, + diff: bool, + quiet: bool, +} + pub(crate) fn run_fmt(args: FmtArgs) -> Result<()> { let FmtArgs { input, @@ -54,8 +60,10 @@ pub(crate) fn run_fmt(args: FmtArgs) -> Result<()> { eprintln!("Formatting {}", input.display()); } + let flags = FmtFlags { check, diff, quiet }; + if input == Path::new("-") { - return run_fmt_stdin(check, diff, quiet); + return run_fmt_stdin(flags); } if input.is_dir() { @@ -70,11 +78,11 @@ pub(crate) fn run_fmt(args: FmtArgs) -> Result<()> { input.display() )); } - return run_fmt_directory(&input, check, diff, quiet); + return run_fmt_directory(&input, flags); } ensure_mds_extension(&input)?; - run_fmt_file(&input, check, diff, quiet) + run_fmt_file(&input, flags) } /// Reject an explicit-file input whose extension isn't `.mds` (AC-CF, exit 2). @@ -125,7 +133,8 @@ fn format_source(source: &str, base_dir: Option<&Path>) -> Result { // ── stdin mode ─────────────────────────────────────────────────────────────── -fn run_fmt_stdin(check: bool, diff: bool, quiet: bool) -> Result<()> { +fn run_fmt_stdin(flags: FmtFlags) -> Result<()> { + let FmtFlags { check, diff, quiet } = flags; let (source, cwd) = read_stdin()?; let result = format_source(&source, Some(&cwd))?; @@ -147,7 +156,8 @@ fn run_fmt_stdin(check: bool, diff: bool, quiet: bool) -> Result<()> { // ── single-file mode ───────────────────────────────────────────────────────── -fn run_fmt_file(path: &Path, check: bool, diff: bool, quiet: bool) -> Result<()> { +fn run_fmt_file(path: &Path, flags: FmtFlags) -> Result<()> { + let FmtFlags { check, diff, quiet } = flags; let source = read_source_file(path)?; let base_dir = path.parent(); let result = format_source(&source, base_dir)?; @@ -196,7 +206,13 @@ fn run_fmt_file(path: &Path, check: bool, diff: bool, quiet: bool) -> Result<()> /// /// Continue-on-error: a per-file failure does not abort the run. Non-zero /// exit when any file failed, or (under `--check`) when any file would change. -fn run_fmt_directory(dir: &Path, check: bool, diff: bool, quiet: bool) -> Result<()> { +fn run_fmt_directory(dir: &Path, flags: FmtFlags) -> Result<()> { + // Directory recursion depth cap, matching `run_build_directory` / + // `run_check_directory` which also declare MAX_DEPTH as a function-local + // constant (build.rs line ~744). + const MAX_DEPTH: usize = 64; + let FmtFlags { check, diff, quiet } = flags; + // Validate mds.json even though `fmt` doesn't act on its `fmt` section's // content yet — consistent with build/check, which also fail loudly on a // malformed config rather than silently ignoring it. From 662d157c03408a2f501af0f91f8850319505fe64 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 7 Jul 2026 20:23:21 +0300 Subject: [PATCH 10/15] refactor(cli): extract format_one_file helper; count diff-output failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-file loop in run_fmt_directory interleaved six concerns (read, format, diff render, read-only tally, write, write-tally) across ~85 lines while mutating three counters across six branches, making it hard to audit each branch in isolation. Extract format_one_file(file, flags) -> FileOutcome, which encapsulates the full per-file read → format → (optional) diff → (optional) write sequence and returns a typed outcome enum. run_fmt_directory's loop is now a single match expression that tallies the outcome. As part of this extraction, a diff-output failure (non-broken-pipe stdout error from print_diff) now returns FileOutcome::Failed and increments fail_count — previously such errors were logged to stderr but the file was still tallied as "would reformat", leaving fail_count unaffected and the exit code misleadingly clean (RU2). Co-Authored-By: Claude --- crates/mds-cli/src/fmt.rs | 137 +++++++++++++++++++++++--------------- 1 file changed, 82 insertions(+), 55 deletions(-) diff --git a/crates/mds-cli/src/fmt.rs b/crates/mds-cli/src/fmt.rs index aea1dfe..be75420 100644 --- a/crates/mds-cli/src/fmt.rs +++ b/crates/mds-cli/src/fmt.rs @@ -197,6 +197,79 @@ fn run_fmt_file(path: &Path, flags: FmtFlags) -> Result<()> { // ── directory mode ─────────────────────────────────────────────────────────── +/// Outcome of formatting a single file in directory mode; tallied by the caller. +enum FileOutcome { + /// Normal mode: the file was reformatted and written. + Formatted, + /// Normal mode: the file was already formatted (no write needed). + Unchanged, + /// `--check` / `--diff` mode: the file would change. + WouldChange, + /// `--check` / `--diff` mode: the file is already formatted. + NoChange, + /// Any per-file error (read, format, diff-output, or write). + Failed, +} + +/// Format one file in directory mode: read → format → (optional) diff → (optional) write. +/// +/// All per-file error and status lines are printed as side effects so the +/// directory loop only needs to tally the returned [`FileOutcome`]. +/// +/// A diff-output failure (non-broken-pipe stdout error) is returned as +/// [`FileOutcome::Failed`] and counted in `fail_count` — consistent with how +/// read and format errors are treated in the surrounding loop. +fn format_one_file(file: &Path, flags: FmtFlags) -> FileOutcome { + let FmtFlags { check, diff, quiet } = flags; + let source = match read_source_file(file) { + Ok(s) => s, + Err(e) => { + eprintln!("{e:?}"); + return FileOutcome::Failed; + } + }; + let base_dir = file.parent(); + let result = match format_source(&source, base_dir) { + Ok(r) => r, + Err(e) => { + eprintln!("{e:?}"); + return FileOutcome::Failed; + } + }; + + if diff && result.changed { + let label = file.display().to_string(); + if let Err(e) = print_diff(&render_diff(&source, &result.formatted, &label)) { + eprintln!("{e:?}"); + return FileOutcome::Failed; + } + } + + let read_only = check || diff; + if read_only { + if result.changed { + FileOutcome::WouldChange + } else { + FileOutcome::NoChange + } + } else if !result.changed { + FileOutcome::Unchanged + } else { + match std::fs::write(file, &result.formatted) { + Ok(()) => { + if !quiet { + eprintln!("Formatted: {}", file.display()); + } + FileOutcome::Formatted + } + Err(e) => { + eprintln!("error: cannot write {}: {e}", file.display()); + FileOutcome::Failed + } + } + } +} + /// Format every `.mds` file under `dir`, INCLUDING `_`-prefixed partials. /// /// Deliberate divergence from `run_build_directory` / `run_check_directory`: @@ -211,7 +284,6 @@ fn run_fmt_directory(dir: &Path, flags: FmtFlags) -> Result<()> { // `run_check_directory` which also declare MAX_DEPTH as a function-local // constant (build.rs line ~744). const MAX_DEPTH: usize = 64; - let FmtFlags { check, diff, quiet } = flags; // Validate mds.json even though `fmt` doesn't act on its `fmt` section's // content yet — consistent with build/check, which also fail loudly on a @@ -221,79 +293,34 @@ fn run_fmt_directory(dir: &Path, flags: FmtFlags) -> Result<()> { let files = collect_mds_files(dir, MAX_DEPTH, None); if files.is_empty() { - if !quiet { + if !flags.quiet { eprintln!("No .mds files found in {}", dir.display()); } return Ok(()); } - let read_only = check || diff; + let read_only = flags.check || flags.diff; let mut changed_count: usize = 0; let mut unchanged_count: usize = 0; let mut fail_count: usize = 0; for file in &files { - let source = match read_source_file(file) { - Ok(s) => s, - Err(e) => { - eprintln!("{e:?}"); - fail_count += 1; - continue; - } - }; - let base_dir = file.parent(); - let result = match format_source(&source, base_dir) { - Ok(r) => r, - Err(e) => { - eprintln!("{e:?}"); - fail_count += 1; - continue; - } - }; - - if diff && result.changed { - let label = file.display().to_string(); - if let Err(e) = print_diff(&render_diff(&source, &result.formatted, &label)) { - eprintln!("{e:?}"); - } - } - - if read_only { - if result.changed { - changed_count += 1; - } else { - unchanged_count += 1; - } - continue; - } - - if !result.changed { - unchanged_count += 1; - continue; - } - match std::fs::write(file, &result.formatted) { - Ok(()) => { - if !quiet { - eprintln!("Formatted: {}", file.display()); - } - changed_count += 1; - } - Err(e) => { - eprintln!("error: cannot write {}: {e}", file.display()); - fail_count += 1; - } + match format_one_file(file, flags) { + FileOutcome::Formatted | FileOutcome::WouldChange => changed_count += 1, + FileOutcome::Unchanged | FileOutcome::NoChange => unchanged_count += 1, + FileOutcome::Failed => fail_count += 1, } } if read_only { - if !quiet || changed_count > 0 || fail_count > 0 { + if !flags.quiet || changed_count > 0 || fail_count > 0 { eprintln!("{changed_count} would reformat, {fail_count} failed"); } - } else if !quiet || fail_count > 0 { + } else if !flags.quiet || fail_count > 0 { eprintln!("{changed_count} formatted, {unchanged_count} unchanged, {fail_count} failed"); } - if fail_count > 0 || (check && changed_count > 0) { + if fail_count > 0 || (flags.check && changed_count > 0) { std::process::exit(1); } Ok(()) From 8678ce6d9b40252603645a58bd28a5db1b08ad79 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 7 Jul 2026 20:23:41 +0300 Subject: [PATCH 11/15] fix(cli): suppress --check summary under --quiet when no failures The directory-mode --check summary used `!quiet || changed_count > 0 || fail_count > 0`, which forced "N would reformat, K failed" to stderr even when --quiet was set and files only *would* change (no errors). This contradicts the documented --quiet contract (suppress status/summaries, never errors) and the single-file --check path, which is fully silent under --quiet. Drop the `changed_count > 0` disjunct: `!flags.quiet || fail_count > 0` matches the non-read_only branch's guard, sibling run_check_directory's guard, and the KB-stated --quiet semantics. Co-Authored-By: Claude --- crates/mds-cli/src/fmt.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/mds-cli/src/fmt.rs b/crates/mds-cli/src/fmt.rs index be75420..441ed46 100644 --- a/crates/mds-cli/src/fmt.rs +++ b/crates/mds-cli/src/fmt.rs @@ -313,7 +313,13 @@ fn run_fmt_directory(dir: &Path, flags: FmtFlags) -> Result<()> { } if read_only { - if !flags.quiet || changed_count > 0 || fail_count > 0 { + // The `changed_count > 0` disjunct was deliberately removed: --quiet + // suppresses status/summaries (including "N would reformat"), never + // errors. Only fail_count > 0 forces a summary under --quiet, matching + // the non-read_only branch's `!quiet || fail_count > 0` contract and + // the single-file --check path (which is fully silent under --quiet, + // exiting 1 with no message when a file would change). + if !flags.quiet || fail_count > 0 { eprintln!("{changed_count} would reformat, {fail_count} failed"); } } else if !flags.quiet || fail_count > 0 { From 581a2bdee721d578107a6ba53af1a06ab0042020 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 7 Jul 2026 20:26:55 +0300 Subject: [PATCH 12/15] fix(cli): use hunk-state machine in colorize_unified_diff to fix mis-coloring The previous implementation keyed colorization on the rendered string prefix: `starts_with("---")` / `starts_with("+++")` for CYAN (file headers). A content line that starts with `-- ` or `++` produces a diff line like `--- comment` or `+++ foo`, which matched the file-header check and was rendered CYAN instead of RED/GREEN. Cosmetic, TTY-only, but incorrect. Replace with a state machine: track whether a `@@` hunk marker has been seen. Before the first `@@`, `---`/`+++` are file headers (CYAN). After `@@`, `-` lines are removals (RED) and `+` lines are additions (GREEN), regardless of what their remaining content looks like. Adds a regression test that exercises both the file-header path and the within-hunk path for content starting with extra dashes/pluses. Co-Authored-By: Claude --- crates/mds-cli/src/fmt.rs | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/crates/mds-cli/src/fmt.rs b/crates/mds-cli/src/fmt.rs index 441ed46..19c9932 100644 --- a/crates/mds-cli/src/fmt.rs +++ b/crates/mds-cli/src/fmt.rs @@ -387,13 +387,23 @@ fn colorize_unified_diff(unified: &str) -> String { const RESET: &str = "\x1b[0m"; let mut out = String::with_capacity(unified.len() + 64); + // `---`/`+++` file headers appear before the first `@@` hunk marker. + // Inside a hunk a removed line starts with `-` and an added line starts + // with `+` — but if that line's *content* itself starts with `-` or `+`, + // the rendered diff line looks like `---` or `+++`, which the old global + // prefix check mis-colored as CYAN (file header) instead of RED/GREEN. + // A state machine keyed on the first `@@` avoids the ambiguity. + let mut in_hunk = false; for line in unified.split_inclusive('\n') { - let color = if line.starts_with("+++") || line.starts_with("---") || line.starts_with("@@") - { + let color = if line.starts_with("@@") { + in_hunk = true; + CYAN + } else if !in_hunk && (line.starts_with("---") || line.starts_with("+++")) { + // File header — always precedes the first @@ hunk. CYAN - } else if line.starts_with('+') { + } else if in_hunk && line.starts_with('+') { GREEN - } else if line.starts_with('-') { + } else if in_hunk && line.starts_with('-') { RED } else { "" @@ -472,4 +482,22 @@ mod tests { assert!(colorized.contains("\x1b[31m-old\x1b[0m")); assert!(colorized.contains("\x1b[36m--- a\x1b[0m")); } + + #[test] + fn colorize_unified_diff_correctly_colors_content_starting_with_dashes_or_pluses() { + // Regression: a removed line whose content starts with "-- " produces + // "--- ..." in the rendered unified diff. The old global prefix check + // matched `starts_with("---")` and mis-colored it CYAN (file header) + // instead of RED (removal). Same defect for "++" content → "+++ " → + // mis-colored CYAN instead of GREEN. + let unified = "--- a\n+++ b\n@@ -1,2 +1,2 @@\n--- dashes content\n+++ plus content\n"; + let colorized = colorize_unified_diff(unified); + // Inside the hunk: removal of a line whose content starts with "-- " + assert!(colorized.contains("\x1b[31m--- dashes content\x1b[0m")); + // Inside the hunk: addition of a line whose content starts with "++" + assert!(colorized.contains("\x1b[32m+++ plus content\x1b[0m")); + // File headers (before @@) must still be CYAN + assert!(colorized.contains("\x1b[36m--- a\x1b[0m")); + assert!(colorized.contains("\x1b[36m+++ b\x1b[0m")); + } } From 0bec62f3aed1dbfc644e33379590f0f0d4bc3469 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 7 Jul 2026 20:34:24 +0300 Subject: [PATCH 13/15] docs(fmt): fix blank-line threshold and CRLF/protected-region accuracy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two factual inaccuracies in user-facing mds fmt docs: 1. Blank-line threshold (DO1): all three locations said "3+ blank lines collapse to one." The actual R3 rule is min(N,2) newlines, meaning TWO or more consecutive blank lines (= three or more newlines) collapse to one — not three. Two blank lines in a row were incorrectly implied to survive formatting unchanged. 2. CRLF vs byte-for-byte contradiction (DO2): docs claimed frontmatter, code fences, AND @message/@define bodies are "byte-for-byte untouched" while also stating CRLF is normalized "including inside frontmatter and code fences." These contradict: R1 (CRLF strip) does cross frontmatter/ code-fence boundaries; only @message/@define bodies are truly byte-for-byte (neither R1 nor R3 applies inside raw_content_spans). Fixed by distinguishing the two cases in each location. Ground truth: formatter.rs module doc (lines 30-60) and KNOWLEDGE.md Rule 1/Rule 3 — both already stated the correct behavior. Co-Authored-By: Claude --- CHANGELOG.md | 11 ++++++----- README.md | 11 +++++++---- crates/mds-cli/src/main.rs | 8 +++++--- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b72d67..6caa433 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,11 +12,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **`mds fmt`** — an opinionated, safety-gated auto-formatter for `.mds` templates. Every rewrite is guaranteed compile-equivalent: a runtime safety gate re-compiles the formatted source and refuses to write if it would change compiled output (`mds::formatter_invariant`) - rather than silently corrupting a template. Normalizes CRLF to LF (including inside - frontmatter and code fences), collapses runs of 3+ blank lines to one, strips trailing - whitespace on directive lines, and ensures exactly one final newline — while leaving - body-text trailing whitespace (Markdown hard breaks) and the byte-for-byte content of - frontmatter / code fences / `@message` / `@define` bodies untouched. Supports a single file, + rather than silently corrupting a template. Normalizes CRLF to LF everywhere (including + inside frontmatter and code fences), collapses runs of two or more consecutive blank lines + to a single blank line, strips trailing whitespace on directive lines, and ensures exactly + one final newline — while leaving body-text trailing whitespace (Markdown hard breaks), + blank-line structure within frontmatter and code fences, and the byte-for-byte content of + `@message`/`@define` bodies untouched. Supports a single file, a directory (recursive, including `_`-prefixed partials), or stdin (`-`, as a filter); `--check` exits non-zero without writing when anything would change, and `--diff` prints a unified diff (colorized on a TTY) without writing. New public `mds-core` API: diff --git a/README.md b/README.md index 15aa6ec..6d56ed3 100644 --- a/README.md +++ b/README.md @@ -172,7 +172,7 @@ echo 'Hello {name}!' | mds fmt - # format from stdin, write to stdout; creat What it normalizes: - CRLF → LF, everywhere (including inside frontmatter and code fences) -- Runs of 3+ blank lines collapse to one; leading blank lines in the body are removed entirely +- Two or more consecutive blank lines collapse to a single blank line; leading blank lines in the body are removed entirely - Trailing whitespace on `@if`/`@for`/`@define`/… directive lines is stripped - Exactly one final newline (empty or whitespace-only input formats to an empty file) @@ -181,9 +181,11 @@ What it deliberately leaves untouched: - Trailing whitespace on body-text content lines, including whitespace-only "blank" lines — two trailing spaces are a Markdown hard line break, and stripping them (anywhere in body text, including a stray blank line) can change rendered output -- The byte-for-byte content of frontmatter, code fences, and `@message`/`@define` bodies (the - latter two can be consumed in a context that bypasses the compiler's own whitespace - normalization, so the formatter never touches them) +- Blank-line structure within frontmatter and code fences — CRLF is normalized there (the same + as everywhere else in the file), but blank lines are not collapsed inside these regions +- The byte-for-byte content of `@message`/`@define` bodies — neither CRLF normalization nor + blank-line collapsing is applied inside them, because these bodies bypass the compiler's own + whitespace normalization and their content reaches compiled output verbatim Directory mode formats every `.mds` file recursively, **including `_`-prefixed partials**, continuing past per-file errors and printing a summary @@ -257,6 +259,7 @@ try { ```rust let output = mds::compile(Path::new("template.mds"), None)?; let output = mds::compile_str("---\nname: World\n---\nHello {name}!\n")?; +let formatted = mds::format_str("Hello {name}!\n")?; ``` ## Examples diff --git a/crates/mds-cli/src/main.rs b/crates/mds-cli/src/main.rs index 6c6586c..a130b9d 100644 --- a/crates/mds-cli/src/main.rs +++ b/crates/mds-cli/src/main.rs @@ -77,11 +77,13 @@ enum Commands { /// /// Rewrites are guaranteed compile-equivalent: a safety gate re-compiles the /// formatted source and refuses to write if it would change compiled output. - /// Normalizes CRLF to LF, collapses runs of 3+ blank lines to one, strips + /// Normalizes CRLF to LF (including inside frontmatter and code fences), + /// collapses runs of 2+ consecutive blank lines to a single blank line, strips /// trailing whitespace on directive lines, and ensures exactly one final /// newline — never touches body-text trailing whitespace (Markdown hard - /// breaks, including on whitespace-only "blank" lines) or the byte-for-byte - /// content of frontmatter / code fences / `@message` / `@define` bodies. + /// breaks, including on whitespace-only "blank" lines), blank-line structure + /// within frontmatter / code fences, or the byte-for-byte content of + /// `@message` / `@define` bodies. #[command( after_help = "Examples:\n mds fmt Auto-detect and format the .mds file in current dir\n mds fmt template.mds Format a file in place\n mds fmt . Format every .mds file recursively (incl. partials)\n mds fmt --check template.mds Exit 1 if the file would change; writes nothing\n mds fmt --diff template.mds Print a unified diff of pending changes; writes nothing\n mds fmt --check --diff . Show diffs for every file that would change, exit 1 if any would\n echo \"Hello {name}!\" | mds fmt - Format from stdin, write to stdout; creates no file" )] From 79698ae70fd1afac7157c302b60a791893218d6f Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 7 Jul 2026 20:47:46 +0300 Subject: [PATCH 14/15] test(fmt): strengthen safety-gate coverage + adversarial edge cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves batch R-tests: T1 (safety-gate paths): - Add CLI-level integration test `structural_fallback_formats_file_with_undefined_var` (cli_fmt.rs): proves path 3 of assert_equivalent works end-to-end for files with undefined runtime vars. Syntax-propagation + file-not-written paths were already covered; FormatterInvariant end-to-end is deferred (requires a formatter bug — see comment in cli_fmt.rs). T2/P2 (perf test honesty + fallback coverage): - Rename `perf_linear_one_megabyte_file_formats_under_two_seconds` → `perf_formats_large_file_without_panic_or_catastrophic_slowdown` - Drop the tight 2.0s wall-clock SLA (CI-flaky); replace with a 30s catastrophic-regression smoke check - Add a ~300 KB undefined-var input exercising the structural_equivalent fallback path at scale (previously the perf test only exercised the double-compile gate path) T3 (adversarial corner cases): - `t3_deep_nesting_for_inside_if_inside_message_*`: @for inside @if inside @message — blank-line run between nested @end lines inside the raw-content span must NOT collapse - `t3_run_of_consecutive_whitespace_only_lines_preserved_verbatim`: 3 consecutive whitespace-only lines — distinct from the single-line case already tested - `t3_bom_plus_crlf_bom_preserved_cr_stripped`: UTF-8 BOM + CRLF combination — BOM survives verbatim, all \r stripped T4 (mtime test discriminating power): - `inplace_unchanged_file_preserves_mtime_and_reports_unchanged`: also assert file bytes are identical (in addition to mtime) to prove no rewrite happened T5 (malformed fmt config): - Add `fmt_config_malformed_bool_field_fails_loading` and `fmt_config_valid_section_loads_cleanly` to build.rs's test module T7 (dead fixture): - Delete `fmt_body_hardbreak.mds` — confirmed unreferenced in the entire test tree T8a (corpus count guard): - Assert at least 10 corpus entries actually ran `compile_equivalent_across_corpus` T8b (diff test robustness): - Add `@@` hunk-header assertion to `diff_prints_unified_diff_to_stdout_writes_nothing` (more discriminating than `---` alone, which frontmatter content can satisfy) Co-Authored-By: Claude --- crates/mds-cli/src/build.rs | 41 +++++ crates/mds-cli/tests/cli_fmt.rs | 67 +++++++- .../tests/fixtures/fmt_body_hardbreak.mds | 2 - crates/mds-core/tests/fmt.rs | 162 +++++++++++++++++- 4 files changed, 262 insertions(+), 10 deletions(-) delete mode 100644 crates/mds-cli/tests/fixtures/fmt_body_hardbreak.mds diff --git a/crates/mds-cli/src/build.rs b/crates/mds-cli/src/build.rs index 88801cc..0d42e29 100644 --- a/crates/mds-cli/src/build.rs +++ b/crates/mds-cli/src/build.rs @@ -1030,4 +1030,45 @@ mod tests { "-o should win over mds.json config" ); } + + // ── T5: malformed fmt config fails config loading ───────────────────────── + + #[test] + fn fmt_config_malformed_bool_field_fails_loading() { + // `FmtConfig.sort_frontmatter_keys` is a `bool`. Supplying a string + // value must cause `serde_json` to reject the config, so `load_config` + // returns `Err` rather than silently using the default. This ensures a + // bad `mds.json` is reported loudly rather than quietly ignored. + let dir = tempfile::tempdir().unwrap(); + std::fs::write( + dir.path().join("mds.json"), + r#"{"fmt": {"sort_frontmatter_keys": "not-a-bool"}}"#, + ) + .unwrap(); + + let result = load_config(dir.path()); + assert!( + result.is_err(), + "a malformed fmt config (wrong type for sort_frontmatter_keys) must fail config loading" + ); + } + + #[test] + fn fmt_config_valid_section_loads_cleanly() { + // Complement to the malformed test: a well-typed fmt section must parse + // without error and produce the expected field value. + let dir = tempfile::tempdir().unwrap(); + std::fs::write( + dir.path().join("mds.json"), + r#"{"fmt": {"sort_frontmatter_keys": false}}"#, + ) + .unwrap(); + + let result = load_config(dir.path()).expect("valid fmt config must load"); + let (config, _) = result.expect("mds.json must be found"); + assert!( + !config.fmt.sort_frontmatter_keys, + "sort_frontmatter_keys: false must deserialize correctly" + ); + } } diff --git a/crates/mds-cli/tests/cli_fmt.rs b/crates/mds-cli/tests/cli_fmt.rs index 22f4bfe..f89ac29 100644 --- a/crates/mds-cli/tests/cli_fmt.rs +++ b/crates/mds-cli/tests/cli_fmt.rs @@ -75,6 +75,7 @@ fn inplace_unchanged_file_preserves_mtime_and_reports_unchanged() { let target = dir.path().join("clean.mds"); fs::write(&target, read_fixture("fmt_formatted.mds")).unwrap(); + let before_bytes = fs::read(&target).unwrap(); let before_mtime = fs::metadata(&target).unwrap().modified().unwrap(); // Ensure the filesystem clock would visibly move if the file were rewritten. std::thread::sleep(Duration::from_millis(1100)); @@ -85,6 +86,15 @@ fn inplace_unchanged_file_preserves_mtime_and_reports_unchanged() { "fmt should succeed on an already-clean file" ); + // Assert both that the mtime is unchanged AND that the bytes are identical: + // the mtime check alone is not enough to distinguish "same content written" + // from "write skipped", because some filesystems may round mtime. + let after_bytes = fs::read(&target).unwrap(); + assert_eq!( + before_bytes, after_bytes, + "file bytes must be identical for an already-clean file (no rewrite should occur)" + ); + let after_mtime = fs::metadata(&target).unwrap().modified().unwrap(); assert_eq!( before_mtime, after_mtime, @@ -116,6 +126,52 @@ fn format_then_check_is_idempotent() { ); } +// ── T1: safety-gate path coverage (Syntax propagation + structural fallback) ── +// +// FormatterInvariant end-to-end is deliberately untested here: triggering it +// requires a formatter bug (the gate fires only when the formatter itself +// diverges from the original output). That cannot be induced without a test +// hook inside formatter.rs, which falls outside the scope of test-only changes. +// The white-box unit tests for `structural_equivalent` inside formatter.rs's +// own `#[cfg(test)]` module cover both branches of that function directly. +// +// What we DO test here: +// - Syntax propagation: `unclosed_directive_block_exits_one_with_syntax_not_formatter_invariant` +// (already present above) proves the gate surfaces real syntax errors and +// leaves the file untouched. +// - structural_equivalent fallback: the test below proves that a source file +// referencing an undefined runtime variable (which doesn't compile standalone) +// is still formatted via the token-comparison fallback, not rejected. + +#[test] +fn structural_fallback_formats_file_with_undefined_var() { + // A source with an undefined runtime variable does not compile standalone, + // so the safety gate cannot do a real recompile-and-diff. It must fall + // back to the structural_equivalent token comparison and succeed, because + // real template files are often written for runtime vars supplied at render + // time — refusing to format them would make `mds fmt` useless on templates. + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("template.mds"); + let src = "Hello {undefined_runtime_var}!\n\n\n\nBye.\n"; + fs::write(&target, src).unwrap(); + + let output = fmt_path(&target, &[]); + assert!( + output.status.success(), + "fmt must succeed on a file with undefined runtime var (structural fallback); \ + stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let after = fs::read_to_string(&target).unwrap(); + // The blank-line run (4 newlines = 3 blank lines) must still be collapsed + // to a single blank line (2 newlines) even via the structural fallback. + assert_eq!( + after, "Hello {undefined_runtime_var}!\n\nBye.\n", + "blank-line collapse must happen even via the structural_equivalent fallback path" + ); +} + // ── AC-CF-2: stdin -> stdout, no file created ──────────────────────────────── fn fmt_stdin(input: &str, extra_args: &[&str]) -> std::process::Output { @@ -439,13 +495,20 @@ fn diff_prints_unified_diff_to_stdout_writes_nothing() { ); let stdout = String::from_utf8(output.stdout).unwrap(); + // `---`/`+++` are the unified diff file headers; `@@` is the hunk header. + // Checking `@@` is more discriminating than `---` alone, which could be + // satisfied by frontmatter content that starts with `---`. assert!( stdout.contains("---"), - "expected unified diff header, got: {stdout:?}" + "expected unified diff --- header, got: {stdout:?}" ); assert!( stdout.contains("+++"), - "expected unified diff header, got: {stdout:?}" + "expected unified diff +++ header, got: {stdout:?}" + ); + assert!( + stdout.contains("@@"), + "expected unified diff @@ hunk header, got: {stdout:?}" ); assert!( stdout.lines().any(|l| l.starts_with('-')) || stdout.lines().any(|l| l.starts_with('+')), diff --git a/crates/mds-cli/tests/fixtures/fmt_body_hardbreak.mds b/crates/mds-cli/tests/fixtures/fmt_body_hardbreak.mds deleted file mode 100644 index 5b155f6..0000000 --- a/crates/mds-cli/tests/fixtures/fmt_body_hardbreak.mds +++ /dev/null @@ -1,2 +0,0 @@ -Line with hard break. -Next line. diff --git a/crates/mds-core/tests/fmt.rs b/crates/mds-core/tests/fmt.rs index d4bcb7e..fa36d9c 100644 --- a/crates/mds-core/tests/fmt.rs +++ b/crates/mds-core/tests/fmt.rs @@ -55,12 +55,14 @@ fn idempotent_across_corpus() { #[test] fn compile_equivalent_across_corpus() { + let mut ran = 0_usize; for src in CORPUS { let Some(orig_md) = compiled_markdown(src) else { // Skip entries that don't compile standalone (none currently, but // keeps this test robust if the corpus grows to include one). continue; }; + ran += 1; let formatted = format_str(src).expect("format_str should succeed for corpus entry"); let formatted_md = compiled_markdown(&formatted).unwrap_or_else(|| { panic!("formatted output of {src:?} failed to compile: {formatted:?}") @@ -70,6 +72,12 @@ fn compile_equivalent_across_corpus() { "compile output changed for input: {src:?}\nformatted: {formatted:?}" ); } + // Guard against the corpus accidentally being all non-compiling entries + // (which would make the loop silently pass without checking anything). + assert!( + ran >= 10, + "expected at least 10 corpus entries to compile standalone, only {ran} did" + ); } // ── API surface (also pinned in api_surface.rs) ─────────────────────────────── @@ -556,12 +564,26 @@ fn idempotency_not_claimed_for_non_tokenizing_input() { assert!(format_str("```\nunclosed").is_err()); } -// ── Perf: linear time on a large mixed file (AC-PERF-1) ────────────────────── +// ── Perf: smoke-check for catastrophic regressions on large files (AC-PERF-1) ─ +// +// NOTE ON WALL-CLOCK BOUNDS: We do NOT assert a tight timing SLA here because +// absolute wall-clock limits are flaky under CI load (the previous "< 2s" bound +// could fire on a loaded runner even with O(n) behaviour). The intent of this +// test is to catch catastrophic O(n²) or worse regressions, not to enforce a +// specific throughput target. The 30-second limit below is a "something went +// very wrong" smoke check only. +// +// TWO GATE PATHS are exercised: +// 1. Double-compile (frontmatter provides all vars → original compiles +// standalone → real-recompile gate path is taken). +// 2. structural_equivalent fallback (undefined-var input → original doesn't +// compile → token-comparison fallback is taken). +// Having both here ensures neither path contains a hidden O(n²) loop. #[test] -fn perf_linear_one_megabyte_file_formats_under_two_seconds() { +fn perf_formats_large_file_without_panic_or_catastrophic_slowdown() { + // Path 1: source compiles standalone → real-recompile gate. let mut src = String::from("---\npremium: true\n---\n"); - // Mix directives, prose, and a code block, repeated to reach ~1MB. let chunk = "@if premium:\nSome prose line with plain text.\n@end\n\n\n\n```text\nblock\n```\n"; while src.len() < 1_000_000 { src.push_str(chunk); @@ -571,14 +593,142 @@ fn perf_linear_one_megabyte_file_formats_under_two_seconds() { let start = std::time::Instant::now(); let out = format_str(&src).expect("should format a large file"); let elapsed = start.elapsed(); - assert!(!out.is_empty()); + // Catastrophic-regression smoke check — NOT a throughput SLA. + assert!( + elapsed.as_secs() < 30, + "formatting ~1MB (double-compile path) took implausibly long: {elapsed:?}" + ); + + // Path 2: source does NOT compile standalone → structural_equivalent fallback. + // Uses an undefined variable so compile_str fails; format_str must still succeed. + let mut fallback_src = String::new(); + let fallback_chunk = + "Hello {undefined_runtime_var}!\n@if undefined_runtime_var:\nYes.\n@end\n\n\n\n"; + while fallback_src.len() < 300_000 { + fallback_src.push_str(fallback_chunk); + } + fallback_src.push_str("Done.\n"); + + assert!( + mds::compile_str(&fallback_src).is_err(), + "sanity: source with undefined var must not compile standalone" + ); + + let start2 = std::time::Instant::now(); + let fallback_out = + format_str(&fallback_src).expect("structural_equivalent fallback must succeed"); + let elapsed2 = start2.elapsed(); + assert!(!fallback_out.is_empty()); + assert!( + elapsed2.as_secs() < 30, + "formatting via structural_equivalent fallback took implausibly long: {elapsed2:?}" + ); + // Blank-line runs must be collapsed even in the fallback path. + assert!( + !fallback_out.contains("\n\n\n"), + "blank-line runs (3+) must be collapsed even via the structural fallback" + ); +} + +// ── T3: adversarial corner cases ───────────────────────────────────────────── + +#[test] +fn t3_deep_nesting_for_inside_if_inside_message_compile_equivalence_and_idempotence() { + // Verifies that @for nested inside @if, itself nested inside @message, is + // handled correctly: the entire inner content is within the @message's + // raw-content span, so R3 blank-line collapse must NOT apply to blank + // lines sandwiched between the nested @end directives. + // + // The blank-line run between the @for's @end and the @if's @end (all inside + // @message) must be preserved verbatim — collapsing it would change the + // compiled message content. + let src = "---\nitems: [a, b, c]\npremium: true\n---\n\ + @message user:\n\ + @if premium:\n\ + @for item in items:\n\ + - {item}\n\ + @end\n\ + \n\n\n\ + @end\n\ + @end\n"; + + let once = format_str(src).unwrap_or_else(|e| panic!("format_str failed for nested src: {e}")); + let twice = format_str(&once).unwrap_or_else(|e| panic!("format_str 2nd pass failed: {e}")); + assert_eq!(once, twice, "nested @for/@if/@message must be idempotent"); + + // The 3 blank lines between for's @end and if's @end are inside the + // @message raw-content span — they must not be collapsed. assert!( - elapsed.as_secs_f64() < 2.0, - "formatting ~1MB took too long: {elapsed:?}" + once.contains("@end\n\n\n\n@end\n"), + "blank-line run inside @message body (between nested @end lines) must \ + not be collapsed by R3, got: {once:?}" + ); + + // Compile-equivalence: the output (messages mode) must be identical. + let before = mds::compile_str(src).unwrap(); + let after = mds::compile_str(&once).unwrap(); + assert_eq!( + before.output, after.output, + "deeply nested @message/@if/@for must remain compile-equivalent after formatting" ); } +#[test] +fn t3_run_of_consecutive_whitespace_only_lines_preserved_verbatim() { + // A RUN of consecutive whitespace-only lines (not truly empty `\n` lines) + // in the middle of a document must be preserved verbatim, even though a + // naive "collapse blank-looking lines" rule might collapse them. + // + // clean_output's per-char loop resets newline_count on ANY non-`\n` + // character — including a space. So each whitespace-only line (which + // contains at least one space) resets the counter; these lines are NOT + // "blank" in the R3 sense and must pass through byte-for-byte. + // + // This is distinct from the single-line case already covered by + // `r4_whitespace_only_line_in_middle_of_document_is_preserved_verbatim`. + let src = "Hello\n \n \n \nWorld\n"; + let out = format_str(src).expect("should format"); + assert_eq!( + out, src, + "a run of consecutive whitespace-only lines must be preserved verbatim" + ); + // Compile-equivalence: clean_output treats these lines as content. + let before = mds::compile_str(src).unwrap().into_markdown().unwrap(); + let after = mds::compile_str(&out).unwrap().into_markdown().unwrap(); + assert_eq!( + before, after, + "compile output must be unchanged (whitespace-only lines are not blank to clean_output)" + ); +} + +#[test] +fn t3_bom_plus_crlf_bom_preserved_cr_stripped() { + // A source with a UTF-8 BOM AND CRLF line endings: R1 must strip `\r` + // but the BOM (`\u{FEFF}`) must survive verbatim. Since the BOM prefix + // means the lexer doesn't recognise the `---` frontmatter fence (it sees + // `\u{FEFF}---`), the whole file is body text — consistent with the + // compiler's own behaviour (no special BOM handling at the lex layer). + let src = "\u{FEFF}Hello\r\nWorld\r\n---\r\nfm key: val\r\n---\r\n"; + let out = format_str(src).expect("should format BOM+CRLF input"); + + assert!( + out.starts_with('\u{FEFF}'), + "UTF-8 BOM must be preserved verbatim, got: {out:?}" + ); + assert!( + !out.contains('\r'), + "all \\r must be stripped by R1 (including from CRLF pairs), got: {out:?}" + ); + // Idempotence: a second pass must not change anything. + let twice = format_str(&out).expect("second pass should format"); + assert_eq!(out, twice, "BOM+CRLF formatting must be idempotent"); + // Compile-equivalence. + let before = mds::compile_str(src).unwrap().into_markdown().unwrap(); + let after = mds::compile_str(&out).unwrap().into_markdown().unwrap(); + assert_eq!(before, after); +} + // ── Runtime-vars-independent: format_str_with never needs runtime vars ─────── #[test] From c3eaec87119e1b6ab576f5e323b575781355afc9 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 7 Jul 2026 20:56:42 +0300 Subject: [PATCH 15/15] style: remove tombstone comment about test relocation from main.rs "The unit tests that were in main.rs have moved to build.rs." describes a past transition, not current state. The following line already explains what matters: this file holds only integration-level wiring. --- crates/mds-cli/src/main.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/mds-cli/src/main.rs b/crates/mds-cli/src/main.rs index a130b9d..f6c22c0 100644 --- a/crates/mds-cli/src/main.rs +++ b/crates/mds-cli/src/main.rs @@ -370,5 +370,4 @@ fn run(cli: Cli) -> Result<()> { } } -// The unit tests that were in main.rs have moved to build.rs. // This file only contains integration-level wiring that is covered by the integration tests.