diff --git a/.devflow/docs/design/149-154-v0.4.0-breaking-fixes.2026-07-08_1400.md b/.devflow/docs/design/149-154-v0.4.0-breaking-fixes.2026-07-08_1400.md new file mode 100644 index 0000000..f727417 --- /dev/null +++ b/.devflow/docs/design/149-154-v0.4.0-breaking-fixes.2026-07-08_1400.md @@ -0,0 +1,118 @@ +--- +issues: [149, 150, 151, 152, 153, 154] +execution-strategy: SINGLE_CODER +date: 2026-07-08 +--- + +# v0.4.0 Breaking Fixes: Issues #149–#154 + +Six approved contracts implemented as 6 sequential commits, each fully green +before committing (`cargo test --workspace` + `cargo fmt --all --check` + +`cargo clippy --workspace --all-targets -- -D warnings`). + +This plan deliberately amends ADR-001's R3 rule — R3 (blank-line-run capping) +is removed because the new whitespace contract (#150/#151) makes it unsound. +The formatter is re-pitched as a line-ending/trailing-whitespace normalizer. + +## Approved Contracts + +### Contract 1 — Whitespace = "delete-directive-lines" model (#150, #151) + +A directive line consumes exactly its own line + trailing newline; every other +markdown-mode byte passes through verbatim (leading blanks, interior blank runs +uncapped, post-frontmatter blank). + +Carve-outs: +- Trailing edge still normalizes to exactly one final `\n` (whitespace-only + doc → `""`). +- `\r` still stripped. +- `@message`/`@define` bodies keep an eval-time edge `.trim()` (interior + verbatim). + +Docs call it "interior-verbatim with trailing-edge normalization" — never bare +"byte-exact". + +### Contract 2 — Fences, full scope (#149) + +Recognition prefix widened to `[ \t>]*` — any indentation depth, tabs +(byte-counted), blockquote markers (`>`, nested `> >`). Tilde fences (`~~~`) +also recognized. Fence lines preserved verbatim in output. Unclosed-fence error +gains the opener's span. + +Out of scope (documented): 4-space indented code blocks without fence markers; +implicit fence close by leaving a blockquote. + +### Contract 3 — Cross-type comparisons + --set-string (#152) + +Keep `--set` coercion. ANY cross-type `==`/`!=` is a hard eval-time error +(`mds::type_mismatch`, line-level span, fix-it hint). Same-type Array/Object +comparisons become structural equality. And/Or stays lazy (skipped operand +never errors). New `--set-string` on build/check/watch. Duplicate key across +`--set`/`--set-string` = hard error. + +### Contract 4 — @extends emits deep-merged frontmatter (#154) + +`@extends` emits the deep-merged frontmatter (child wins, transitive). +Knowingly reverses the "base FM is scope-only" posture (f4 `base_secret` test). +Requires explicit-intent test rewrite, loud BREAKING changelog, adversarial +YAML-injection tests. + +Standalone path stays raw-verbatim (named asymmetry). Reserved keys +(`type`/`imports`/`extends`) stripped top-level ONLY, uniformly. Child-without- +frontmatter now emits base keys; empty merged mapping → no fence. Emitted FM +ignores `--set` (documented). + +### Contract 5 — Interpolation hint fix (#153) + +`parser_helpers.rs:1102` renders `\{{` (broken); must render `\{`. +Fix: format string `\\{{{{` → `\\{{`. + +## Six-Commit Breakdown + +### Commit 1 — `fix: interpolation error hint recommends \{ (#153)` +Files: `crates/mds-core/src/parser_helpers.rs` +- Fix format string at line ~1102: `\\{{{{` → `\\{{` +- Add test pinning the rendered hint text + +### Commit 2 — `fix!: interior-verbatim whitespace contract (#150, #151)` — KEYSTONE +Files: `parser.rs`, `parser_helpers.rs`, `lib.rs`, `formatter.rs`, `evaluator.rs` +- Delete `strip_trailing_newline(strip_leading_newline(body))` at 3 parser sites +- Delete dead `strip_leading_newline`/`strip_trailing_newline` functions +- Add `.trim()` at `invoke_function` body eval (line ~376) +- Rewrite `clean_output`: keep \r strip + trailing trim_end() + one \n + ws-only→""; + DELETE leading trim_start_matches, run-cap + newline_count +- R3 death in formatter: delete `protected_spans()`, leading_mode, newline_run, + R3 blank-line logic; `rewrite_body` → 3 branches: directive→R4 | raw-content→verbatim | else→R1-only +- Update all affected tests + +### Commit 3 — `fix: recognize indented, tilde, and blockquoted code fences (#149)` +Files: `crates/mds-core/src/lexer.rs` +- New `fence_at_line_start()` helper: scan `[ \t>]*` then require ≥3 backtick or `~` +- State: `code_fence_backticks: usize` → `code_fence: Option<(char, usize)>` + opener span +- Closer: verbatim capture (not reconstructed) +- Unclosed-fence error gains opener span +- Tests: lexer unit + fmt corpus + integration repro + +### Commit 4 — `fix!: cross-type comparison errors, structural equality, --set-string (#152)` +Files: `evaluator.rs`, `error.rs`, `crates/mds-cli/src/main.rs`, `build.rs`, `watch.rs` +- New `values_equal_same_type()` returning `Option` (None = cross-type error) +- New `MdsError::TypeMismatch` variant with `mds::type_mismatch` code +- Add `file`/`source` to `EvalContext` +- New `RuntimeVarArgs` struct (flatten into Build/Check/Watch) +- `--set-string` flag: inserts as `Value::String`, bypasses coercion +- Tests: flip 3 existing, add cross-type matrix + structural eq + lazy short-circuit + +### Commit 5 — `fix!: @extends emits deep-merged frontmatter (#154)` +Files: `resolver/frontmatter.rs`, `resolver.rs`, `lib.rs` +- `serialize_merged_frontmatter()` helper +- `build_merged_extends_scope()` returns `(Scope, Option)` +- Extends emit site switches to `prepend_frontmatter(merged_fm, body)` +- Reserved keys gate on `depth == 0` +- Rewrite f4 test; augment f6; add adversarial YAML-injection tests + +### Commit 6 — `docs: v0.4.0 language/CLI docs, changelog, examples` +Files: `spec.md`, `README.md`, `CHANGELOG.md`, `.devflow/features/mds-fmt/KNOWLEDGE.md`, `examples/` +- Update spec.md whitespace contract section +- Update README.md `mds fmt` pitch (remove "collapses blank lines") +- CHANGELOG.md: Fixed/Changed-BREAKING/Added entries for #149-#154 +- Re-sync mds-fmt KNOWLEDGE.md (R3 gone, clean_output new behavior) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c080d13..2509071 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -94,12 +94,16 @@ jobs: gz=$(gzip -c "$f" | wc -c | tr -d ' ') label="${f#crates/mds-wasm/}" echo "::notice::WASM ${label}: ${raw} bytes raw, ${gz} bytes gzipped" - # Soft bloat guard. Budget raised 600K->650K (2026-06): the unpinned CI - # `stable` toolchain drifted and pushed the optimized binary just over 600K - # with no source change (local toolchain still emits ~592K). Follow-up: pin - # the wasm build toolchain to make the size deterministic and re-tighten this. - if [ "$raw" -gt 650000 ]; then - echo "::error::WASM binary exceeds 650,000 byte threshold: ${label} is ${raw} bytes" + # Soft bloat guard. Budget history: + # 600K initial; raised 600K->650K (2026-06): unpinned CI `stable` toolchain + # drifted past 600K with no source change (local emitted ~592K). + # raised 650K->700K (2026-07, PR #161 / fix/v0-4-0-149-154): toolchain + # drift + new v0.4.0 code (cross-type errors, extended help strings, + # @extends frontmatter merging) pushed CI binary to 650,745 bytes. + # Follow-up: pin the wasm build toolchain to make the size deterministic + # and re-tighten this guard. + if [ "$raw" -gt 700000 ]; then + echo "::error::WASM binary exceeds 700,000 byte threshold: ${label} is ${raw} bytes" exit 1 fi done diff --git a/CHANGELOG.md b/CHANGELOG.md index 6caa433..326a6f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,21 +7,50 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### **BREAKING** — Strict cross-type comparisons, merged `@extends` frontmatter, interior-verbatim whitespace + +These changes alter observable runtime behavior and compiled output. Templates relying +on the previous (buggy) behavior must be updated. + +#### Cross-type comparisons are now errors (#152) + +`@if a == b:` or `@if a != b:` where `a` and `b` are different types (e.g. a number +vs. a string, or a boolean vs. null) now raises `mds::type_mismatch` at runtime +instead of silently returning `false` (for `==`) or `true` (for `!=`). + +**Migration:** add an explicit conversion before comparing: +- `@if str(count) == "3":` — convert number to string +- `@if count == 3:` — compare number to number literal + +#### `@extends` emits deep-merged frontmatter (#154) + +Compiled output for a child template now contains the **deep-merged** frontmatter +(base keys + child keys, child wins on collision, reserved keys `imports`/`type`/`extends` +excluded) rather than only the child's raw frontmatter. Base-only frontmatter keys +now appear in the compiled output. + +**Migration:** if your pipeline depends on base frontmatter keys being absent from the +compiled output, strip them downstream or move them to a non-frontmatter location. + ### Added +- **`--set-string KEY=VALUE`** CLI flag for `mds build`, `mds check`, and `mds watch`. + Sets a variable as a string without type coercion — useful when a value is + numeric-looking but must stay a string (e.g. `mds build t.mds --set-string id=007`). + Repeatable. (#152) + - **`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 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: - `format_str` / `format_str_with`. (#60) + inside frontmatter and code fences), strips trailing whitespace on directive lines, and + ensures exactly one trailing newline — while leaving interior blank lines, blank-line + structure within frontmatter and code fences, body-text trailing whitespace (Markdown + hard breaks), 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: `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`, @@ -38,6 +67,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **BREAKING:** Interior-verbatim whitespace contract for block bodies and `mds fmt`. + Leading blank lines and interior blank runs inside `@block` / `@define` bodies and + `mds fmt` output are now preserved verbatim; previously they were collapsed or stripped. + The `mds fmt` blank-line collapsing rule (R3) has been removed to maintain compile + equivalence with the updated evaluator behavior. Only the trailing edge normalizes (to + exactly one final newline). **Migration:** compiled outputs may gain blank lines that + were previously collapsed or stripped; templates relying on this collapse must remove + the extra blank lines at the source level. (#150, #151) + - **BREAKING:** `FileSystem` trait now requires two new methods — `normalize_in_dir` and `parent_dir` — that replace the internal `` path-sentinel pattern. String-source `@import`/`@extends` resolution is now directly directory-anchored: @@ -48,6 +86,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Code fences: tilde (`~~~`), indented, and blockquoted variants are now recognized** + as passthrough regions. Previously only `` ``` ``-fences that started at column 1 + were treated as code — a `~~~` fence, a `` > ``` `` blockquote fence, or a fence + indented with spaces/tabs would allow interpolation and directive parsing inside, + silently corrupting output for affected templates. The lexer now matches any fence + that starts with `[ \t>]*` followed by three or more matching backticks or tildes. + (#149) + +- **Interpolation errors now suggest `\{`** in the help text when an opening brace + appears in a context where an interpolation was expected but failed (e.g. `{ invalid` + or an unclosed brace). Helps users writing literal brace characters. (#153) + - **Windows: string-source `@import`/`@extends` now resolve relative imports correctly.** `std::fs::canonicalize` returns a `\\?\` verbatim extended-length path on Windows, and inside a `\\?\` prefix `/` is a literal character, not a path diff --git a/README.md b/README.md index 6d56ed3..10efee0 100644 --- a/README.md +++ b/README.md @@ -87,7 +87,8 @@ Build/Watch options: --out-dir Output directory (build/single-file watch: .md or .json; dir-mode watch: mirrors source subtree) --vars JSON file with variable overrides (reloaded each rebuild) - --set KEY=VALUE Set a single variable (repeatable) + --set KEY=VALUE Set a single variable (repeatable); value coerced to number/bool/null/array when possible + --set-string KEY=VALUE Set a single variable as a string, bypassing type coercion (repeatable) Watch-only options: --clear Clear terminal before each rebuild (only when stderr is a TTY) @@ -172,20 +173,17 @@ echo 'Hello {name}!' | mds fmt - # format from stdin, write to stdout; creat What it normalizes: - CRLF → LF, everywhere (including inside frontmatter and code fences) -- 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) 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 -- 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 +- Trailing whitespace on body-text content lines — two trailing spaces are a Markdown hard line + break; stripping them would change rendered output +- Blank-line structure within the file body, frontmatter, and code fences — the formatter does + not add or remove blank lines; blank-line layout is the template author's choice +- The byte-for-byte content of `@message`/`@define` bodies — whitespace inside these bodies + reaches compiled output verbatim and must not be altered Directory mode formats every `.mds` file recursively, **including `_`-prefixed partials**, continuing past per-file errors and printing a summary @@ -269,7 +267,7 @@ live in [`examples/`](examples/). ## Language Reference -See [spec.md](spec.md) for the full MDS v0.2.0 language specification. +See [spec.md](spec.md) for the full MDS v0.4.0 language specification. ## Contributing diff --git a/crates/mds-cli/src/build.rs b/crates/mds-cli/src/build.rs index 0d42e29..99147d8 100644 --- a/crates/mds-cli/src/build.rs +++ b/crates/mds-cli/src/build.rs @@ -383,6 +383,19 @@ pub(crate) fn exit_code(err: &miette::Error) -> i32 { // ── Runtime vars helpers ────────────────────────────────────────────────────── +/// Bundled runtime-variable arguments from the CLI. +/// +/// Groups `--vars`, `--set`, and `--set-string` together so they can be passed +/// as a single unit through CLI dispatch functions. +pub(crate) struct RuntimeVarArgs { + /// Optional JSON vars file (`--vars`). + pub(crate) vars: Option, + /// Auto-coerced `--set KEY=VALUE` overrides (bool/number/null/array/string). + pub(crate) set_vars: Vec<(String, String)>, + /// String-forced `--set-string KEY=VALUE` overrides (always string, no coercion). + pub(crate) set_string_vars: Vec<(String, String)>, +} + /// Load vars from an optional file path, returning None if no file was given. pub(crate) fn load_optional_vars_file( path: Option, @@ -391,17 +404,43 @@ pub(crate) fn load_optional_vars_file( .transpose() } -/// Merge a `--vars` file with any `--set key=value` overrides into a single map. +/// Merge a `--vars` file with `--set` and `--set-string` overrides into a single map. +/// +/// Processing order: file vars < `--set`/`--set-string` overrides. +/// Within each flag group, later flags win (last-wins). Cross-flag duplicate keys +/// (a key present in both `--set` and `--set-string`) are rejected with an error. pub(crate) fn build_runtime_vars( - vars: Option, - set_vars: Vec<(String, String)>, + args: RuntimeVarArgs, ) -> Result>> { + let RuntimeVarArgs { + vars, + set_vars, + set_string_vars, + } = args; + + // Collect unique keys used by --set for cross-flag duplicate detection. + let set_keys: HashSet<&str> = set_vars.iter().map(|(k, _)| k.as_str()).collect(); + + // Reject any key that appears in both --set and --set-string. + for (key, _) in &set_string_vars { + if set_keys.contains(key.as_str()) { + return Err(miette::miette!( + "variable '{key}' is set by both --set and --set-string; use only one" + )); + } + } + let mut runtime_vars = load_optional_vars_file(vars)?; for (key, val) in set_vars { runtime_vars .get_or_insert_with(HashMap::new) .insert(key, parse_cli_value(val)); } + for (key, val) in set_string_vars { + runtime_vars + .get_or_insert_with(HashMap::new) + .insert(key, mds::Value::String(val)); + } Ok(runtime_vars) } @@ -636,6 +675,7 @@ pub(crate) struct BuildArgs { pub(crate) out_dir: Option, pub(crate) vars: Option, pub(crate) set_vars: Vec<(String, String)>, + pub(crate) set_string_vars: Vec<(String, String)>, pub(crate) quiet: bool, } @@ -656,9 +696,14 @@ pub(crate) fn run_build(args: BuildArgs) -> Result<()> { out_dir, vars, set_vars, + set_string_vars, quiet, } = args; - let runtime_vars = build_runtime_vars(vars, set_vars)?; + let runtime_vars = build_runtime_vars(RuntimeVarArgs { + vars, + set_vars, + set_string_vars, + })?; // Resolve the input: explicit path, or auto-detect from cwd. // When auto-detected, print a "Building {path}" banner so users know which file was selected. @@ -1071,4 +1116,100 @@ mod tests { "sort_frontmatter_keys: false must deserialize correctly" ); } + + // ── build_runtime_vars: cross-flag duplicate rejection (#152) ───────────── + + #[test] + fn build_runtime_vars_cross_flag_duplicate_is_error() { + // A key present in both --set and --set-string must be a hard error. + let result = build_runtime_vars(RuntimeVarArgs { + vars: None, + set_vars: vec![("x".to_string(), "1".to_string())], + set_string_vars: vec![("x".to_string(), "2".to_string())], + }); + assert!(result.is_err(), "cross-flag duplicate key must be rejected"); + let msg = format!("{}", result.unwrap_err()); + assert!( + msg.contains("variable 'x' is set by both --set and --set-string"), + "error message must identify the key; got: {msg}" + ); + assert!( + msg.contains("use only one"), + "error message must say 'use only one'; got: {msg}" + ); + } + + #[test] + fn build_runtime_vars_check_command_cross_flag_duplicate_is_error() { + // The same build_runtime_vars function is used by both mds build and mds check. + // Verify the error fires regardless of which args struct wraps it. + let result = build_runtime_vars(RuntimeVarArgs { + vars: None, + set_vars: vec![("count".to_string(), "3".to_string())], + set_string_vars: vec![("count".to_string(), "three".to_string())], + }); + assert!( + result.is_err(), + "cross-flag duplicate must be rejected in check parity" + ); + let msg = format!("{}", result.unwrap_err()); + assert!( + msg.contains("variable 'count'"), + "error must name the duplicate key; got: {msg}" + ); + } + + #[test] + fn build_runtime_vars_same_flag_last_wins() { + // Within a single flag group, the last occurrence wins (no error). + let result = build_runtime_vars(RuntimeVarArgs { + vars: None, + set_vars: vec![ + ("x".to_string(), "1".to_string()), + ("x".to_string(), "2".to_string()), + ], + set_string_vars: vec![], + }) + .expect("same-flag last-wins must not error"); + let map = result.expect("non-empty vars"); + assert_eq!( + map.get("x"), + Some(&mds::Value::Number(2.0)), + "last --set wins within the same flag" + ); + } + + #[test] + fn build_runtime_vars_same_flag_string_last_wins() { + // Within --set-string, the last occurrence also wins (no error). + let result = build_runtime_vars(RuntimeVarArgs { + vars: None, + set_vars: vec![], + set_string_vars: vec![ + ("id".to_string(), "007".to_string()), + ("id".to_string(), "009".to_string()), + ], + }) + .expect("same-flag last-wins in --set-string must not error"); + let map = result.expect("non-empty vars"); + assert_eq!( + map.get("id"), + Some(&mds::Value::String("009".to_string())), + "last --set-string wins within the same flag" + ); + } + + #[test] + fn build_runtime_vars_distinct_keys_no_error() { + // Using --set and --set-string for DIFFERENT keys must succeed. + let result = build_runtime_vars(RuntimeVarArgs { + vars: None, + set_vars: vec![("num".to_string(), "42".to_string())], + set_string_vars: vec![("id".to_string(), "007".to_string())], + }) + .expect("distinct keys across flags must not error"); + let map = result.expect("non-empty vars"); + assert_eq!(map.get("num"), Some(&mds::Value::Number(42.0))); + assert_eq!(map.get("id"), Some(&mds::Value::String("007".to_string()))); + } } diff --git a/crates/mds-cli/src/main.rs b/crates/mds-cli/src/main.rs index f6c22c0..25900b2 100644 --- a/crates/mds-cli/src/main.rs +++ b/crates/mds-cli/src/main.rs @@ -9,7 +9,10 @@ mod fmt; mod output; mod watch; -use build::{build_runtime_vars, exit_code, parse_key_value, resolve_input, run_build, BuildArgs}; +use build::{ + build_runtime_vars, exit_code, parse_key_value, resolve_input, run_build, BuildArgs, + RuntimeVarArgs, +}; // ── CLI entry point ─────────────────────────────────────────────────────────── @@ -58,6 +61,9 @@ enum Commands { /// Set a runtime variable (repeatable, e.g. --set name=Alice --set count=3) #[arg(long = "set", value_name = "KEY=VALUE", value_parser = parse_key_value)] set_vars: Vec<(String, String)>, + /// Set a runtime variable as a string (no type coercion; e.g. --set-string count=3 sets count to the string "3") + #[arg(long = "set-string", value_name = "KEY=VALUE", value_parser = parse_key_value)] + set_string_vars: Vec<(String, String)>, }, /// Validate an MDS file without rendering #[command( @@ -72,18 +78,19 @@ enum Commands { /// Set a runtime variable (repeatable, e.g. --set name=Alice --set count=3) #[arg(long = "set", value_name = "KEY=VALUE", value_parser = parse_key_value)] set_vars: Vec<(String, String)>, + /// Set a runtime variable as a string (no type coercion; e.g. --set-string count=3 sets count to the string "3") + #[arg(long = "set-string", value_name = "KEY=VALUE", value_parser = parse_key_value)] + set_string_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 (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), blank-line structure - /// within frontmatter / code fences, or the byte-for-byte content of - /// `@message` / `@define` bodies. + /// Normalizes line endings to LF on directive lines, strips trailing + /// whitespace on directive lines, and ensures exactly one final newline — + /// never touches body-text content (Markdown hard breaks, blank-line + /// structure, whitespace-only lines), frontmatter / code-fence internals, + /// 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" )] @@ -142,6 +149,9 @@ enum Commands { /// Set a runtime variable (repeatable, e.g. --set name=Alice --set count=3) #[arg(long = "set", value_name = "KEY=VALUE", value_parser = parse_key_value)] set_vars: Vec<(String, String)>, + /// Set a runtime variable as a string (no type coercion; e.g. --set-string count=3 sets count to the string "3") + #[arg(long = "set-string", value_name = "KEY=VALUE", value_parser = parse_key_value)] + set_string_vars: Vec<(String, String)>, /// Clear the terminal before each rebuild (only when stderr is a TTY) #[arg(long)] clear: bool, @@ -172,10 +182,15 @@ fn run_check( input: Option, vars: Option, set_vars: Vec<(String, String)>, + set_string_vars: Vec<(String, String)>, quiet: bool, ) -> Result<()> { use build::read_stdin; - let runtime_vars = build_runtime_vars(vars, set_vars)?; + let runtime_vars = build_runtime_vars(RuntimeVarArgs { + vars, + set_vars, + set_string_vars, + })?; // Resolve the input: explicit path/stdin, or auto-detect from cwd. // run_check does not print a banner on auto-detect — check is a silent validation. @@ -327,19 +342,22 @@ fn run(cli: Cli) -> Result<()> { out_dir, vars, set_vars, + set_string_vars, } => run_build(BuildArgs { input, output, out_dir, vars, set_vars, + set_string_vars, quiet, }), Commands::Check { input, vars, set_vars, - } => run_check(input, vars, set_vars, quiet), + set_string_vars, + } => run_check(input, vars, set_vars, set_string_vars, quiet), Commands::Fmt { input, check, diff } => fmt::run_fmt(fmt::FmtArgs { input, check, @@ -353,6 +371,7 @@ fn run(cli: Cli) -> Result<()> { out_dir, vars, set_vars, + set_string_vars, clear, debounce, poll_interval, @@ -362,6 +381,7 @@ fn run(cli: Cli) -> Result<()> { out_dir, vars, set_vars, + set_string_vars, clear, debounce, quiet, diff --git a/crates/mds-cli/src/watch.rs b/crates/mds-cli/src/watch.rs index dfd989a..21c07ed 100644 --- a/crates/mds-cli/src/watch.rs +++ b/crates/mds-cli/src/watch.rs @@ -36,7 +36,7 @@ use mds::MdsError; use crate::build::{ auto_detect_mds_file, build_runtime_vars, compile_and_write, compile_to_content, load_config, - resolve_output_path_for_kind, write_output, OutputKind, + resolve_output_path_for_kind, write_output, OutputKind, RuntimeVarArgs, }; use crate::output::{ canonicalize_out_dir, collect_mds_files, is_partial, output_base_no_ext, output_path_for, @@ -51,6 +51,7 @@ pub(crate) struct WatchArgs { pub(crate) out_dir: Option, pub(crate) vars: Option, pub(crate) set_vars: Vec<(String, String)>, + pub(crate) set_string_vars: Vec<(String, String)>, pub(crate) clear: bool, pub(crate) debounce: u64, pub(crate) quiet: bool, @@ -506,6 +507,7 @@ pub(crate) fn run_watch(args: WatchArgs) -> Result<()> { out_dir, vars, set_vars, + set_string_vars, clear, debounce, quiet, @@ -552,6 +554,7 @@ pub(crate) fn run_watch(args: WatchArgs) -> Result<()> { out_dir, vars, set_vars, + set_string_vars, clear, debounce, quiet, @@ -564,6 +567,7 @@ pub(crate) fn run_watch(args: WatchArgs) -> Result<()> { out_dir, vars, set_vars, + set_string_vars, clear, debounce, quiet, @@ -590,6 +594,7 @@ struct FileCompileCtx { entry: PathBuf, vars_path: Option, static_set_vars: Vec<(String, String)>, + static_set_string_vars: Vec<(String, String)>, /// The `-o ` or `--out-dir` argument passed by the user, if any. /// Kept here so `rebuild_file` can reuse the same path-derivation logic for /// the dynamic path case (where output_path itself stays None until after compile). @@ -774,8 +779,11 @@ fn rebuild_file( ) { // Soft-error: vars file may be temporarily absent (AC-W7 / AC-C5). // Print the error, settle mtime to avoid re-fire, and keep watching. - let runtime_vars = match build_runtime_vars(ctx.vars_path.clone(), ctx.static_set_vars.clone()) - { + let runtime_vars = match build_runtime_vars(RuntimeVarArgs { + vars: ctx.vars_path.clone(), + set_vars: ctx.static_set_vars.clone(), + set_string_vars: ctx.static_set_string_vars.clone(), + }) { Ok(v) => v, Err(e) => { eprintln!("{e:?}"); @@ -875,6 +883,7 @@ fn run_watch_file( out_dir: Option, vars: Option, set_vars: Vec<(String, String)>, + set_string_vars: Vec<(String, String)>, clear: bool, debounce_ms: u64, quiet: bool, @@ -886,12 +895,17 @@ fn run_watch_file( // Build runtime vars from the set_vars statics (vars file is reloaded each rebuild). let static_set_vars = set_vars; + let static_set_string_vars = set_string_vars; // Initial compile: compile first, derive output path from kind (compile-then-route). // For explicit -o / --out-dir the path is determined by the flag. // For the default case (no explicit flag), the path depends on the output kind, which // is only known after compilation — so we compile first, then derive. - let runtime_vars = build_runtime_vars(vars_path.clone(), static_set_vars.clone())?; + let runtime_vars = build_runtime_vars(RuntimeVarArgs { + vars: vars_path.clone(), + set_vars: static_set_vars.clone(), + set_string_vars: static_set_string_vars.clone(), + })?; if !quiet { eprintln!("Watching {}", entry.display()); } @@ -1009,6 +1023,7 @@ fn run_watch_file( entry, vars_path, static_set_vars, + static_set_string_vars, output_arg: output, out_dir, output_path, @@ -1281,6 +1296,7 @@ struct DirWatchCtx { root: PathBuf, vars_path: Option, static_set_vars: Vec<(String, String)>, + static_set_string_vars: Vec<(String, String)>, output_base: OutputBase, exclude_prefix: Option, vars_dir_extra: Option, @@ -1416,15 +1432,18 @@ fn liveness_probe_dir( if !appeared.is_empty() || !removed.is_empty() { // Soft-error: vars file may be temporarily absent (AC-W7 / AC-C5). - let runtime_vars = - match build_runtime_vars(ctx.vars_path.clone(), ctx.static_set_vars.clone()) { - Ok(v) => v, - Err(e) => { - eprintln!("{e:?}"); - state.last_mtimes = snapshot_state(&state.known_set()); - return; - } - }; + let runtime_vars = match build_runtime_vars(RuntimeVarArgs { + vars: ctx.vars_path.clone(), + set_vars: ctx.static_set_vars.clone(), + set_string_vars: ctx.static_set_string_vars.clone(), + }) { + Ok(v) => v, + Err(e) => { + eprintln!("{e:?}"); + state.last_mtimes = snapshot_state(&state.known_set()); + return; + } + }; let mut batch: BTreeSet = appeared.clone(); batch.extend(removed.iter().cloned()); process_dir_batch( @@ -1534,8 +1553,11 @@ fn handle_fs_event_dir( // ADR-016: reload vars from disk on every rebuild. // Soft-error: vars file may be temporarily absent (AC-W7 / AC-C5). - let runtime_vars = match build_runtime_vars(ctx.vars_path.clone(), ctx.static_set_vars.clone()) - { + let runtime_vars = match build_runtime_vars(RuntimeVarArgs { + vars: ctx.vars_path.clone(), + set_vars: ctx.static_set_vars.clone(), + set_string_vars: ctx.static_set_string_vars.clone(), + }) { Ok(v) => v, Err(e) => { eprintln!("{e:?}"); @@ -1572,6 +1594,7 @@ fn dir_watch_startup( out_dir: Option, vars: Option, set_vars: Vec<(String, String)>, + set_string_vars: Vec<(String, String)>, clear: bool, debounce_ms: u64, quiet: bool, @@ -1582,6 +1605,7 @@ fn dir_watch_startup( // Also rejects a symlinked vars file at startup (build parity — PF-004). let vars_path = canonicalize_vars_path(vars).map_err(miette::Error::from)?; let static_set_vars = set_vars; + let static_set_string_vars = set_string_vars; // Canonicalize out_dir as absolute so the starts_with(&root) in-root exclusion check // is reliable even when cwd contains symlinks (root is already canonical — security #8). @@ -1603,7 +1627,11 @@ fn dir_watch_startup( // Startup compile: compile all .mds files found under root. let all_files = collect_mds_files(&root, MAX_COLLECT_DEPTH, exclude_prefix.as_deref()); - let runtime_vars = build_runtime_vars(vars_path.clone(), static_set_vars.clone())?; + let runtime_vars = build_runtime_vars(RuntimeVarArgs { + vars: vars_path.clone(), + set_vars: static_set_vars.clone(), + set_string_vars: static_set_string_vars.clone(), + })?; // Build the dependency graph and compile all files at startup. let mut state = DirWatchState { @@ -1720,7 +1748,11 @@ fn dir_watch_startup( // Build the dedup baseline AFTER the watcher is registered so any OS-queued // synthetic events arrive after the baseline is recorded and are filtered out. { - let baseline_vars = build_runtime_vars(vars_path.clone(), static_set_vars.clone())?; + let baseline_vars = build_runtime_vars(RuntimeVarArgs { + vars: vars_path.clone(), + set_vars: static_set_vars.clone(), + set_string_vars: static_set_string_vars.clone(), + })?; for source in &all_files { let key = graph_key(source); if is_partial(source) { @@ -1782,6 +1814,7 @@ fn dir_watch_startup( root, vars_path, static_set_vars, + static_set_string_vars, output_base, exclude_prefix, vars_dir_extra, @@ -1805,6 +1838,7 @@ fn run_watch_dir( out_dir: Option, vars: Option, set_vars: Vec<(String, String)>, + set_string_vars: Vec<(String, String)>, clear: bool, debounce_ms: u64, quiet: bool, @@ -1816,7 +1850,16 @@ fn run_watch_dir( mut state, mut liveness, ctx, - } = dir_watch_startup(root, out_dir, vars, set_vars, clear, debounce_ms, quiet)?; + } = dir_watch_startup( + root, + out_dir, + vars, + set_vars, + set_string_vars, + clear, + debounce_ms, + quiet, + )?; // ── Watch loop ──────────────────────────────────────────────────────────── loop { diff --git a/crates/mds-cli/tests/cli_fmt.rs b/crates/mds-cli/tests/cli_fmt.rs index f89ac29..5c3098e 100644 --- a/crates/mds-cli/tests/cli_fmt.rs +++ b/crates/mds-cli/tests/cli_fmt.rs @@ -164,11 +164,11 @@ fn structural_fallback_formats_file_with_undefined_var() { ); 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. + // Interior-verbatim (#150/#151): blank-line runs pass through verbatim + // even via the structural fallback; only trailing edge is normalised (R2). assert_eq!( - after, "Hello {undefined_runtime_var}!\n\nBye.\n", - "blank-line collapse must happen even via the structural_equivalent fallback path" + after, "Hello {undefined_runtime_var}!\n\n\n\nBye.\n", + "interior blank-line runs must be preserved verbatim (interior-verbatim contract)" ); } @@ -316,8 +316,8 @@ fn dir_recurse_formats_every_mds_including_partials() { "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:?}" + partial_after.contains("@end\n\n\n\n@export"), + "blank-line run between @end and @export must be preserved verbatim (interior-verbatim), got: {partial_after:?}" ); let stderr = String::from_utf8_lossy(&output.stderr); diff --git a/crates/mds-cli/tests/frontmatter.rs b/crates/mds-cli/tests/frontmatter.rs index 4fa35ba..b0e240b 100644 --- a/crates/mds-cli/tests/frontmatter.rs +++ b/crates/mds-cli/tests/frontmatter.rs @@ -212,3 +212,115 @@ fn yaml_map_type_works() { let result = mds::compile_str(source).unwrap().into_markdown().unwrap(); assert!(result.contains("value\n"), "got: {result}"); } + +// ── Adversarial frontmatter injection tests ──────────────────────────────────── +// +// These tests verify that YAML values containing fence-like content ("\n---\n"), +// block-scalar `type: mds` lines, or YAML-looking strings cannot escape the +// frontmatter block and smuggle additional top-level keys into the compiled output. +// +// The test strategy: extract only the FRONTMATTER SECTION from the output (the text +// between the first `---\n` and the second `---\n`) and assert on that slice. +// Body text may legitimately contain `---` (e.g. from variable substitution), so we +// do not count `---` occurrences in the whole output. + +/// Extract the raw YAML inside the first `---`/`---` fence pair in `output`. +/// Returns `None` if the output has no frontmatter fences. +fn extract_frontmatter(output: &str) -> Option<&str> { + let open = output.find("---\n")?; + let rest = &output[open + 4..]; + let close = rest.find("\n---")?; + Some(&rest[..close]) +} + +#[test] +fn adversarial_value_containing_fence_sequence_via_extends() { + // An @extends child whose base has a frontmatter value containing a literal + // `\n---\n` sequence must produce exactly one opening and one closing `---` + // fence in the emitted frontmatter section — not two document separators. + // + // The re-serialization path (serialize_merged_frontmatter / serde_yaml_ng::to_string) + // must produce valid YAML that doesn't break the fence structure. + let mut modules = std::collections::HashMap::new(); + modules.insert( + "base.mds".to_string(), + // Value that, when stored and re-serialized, could inject a `---` line. + "---\nprompt: \"start\\n---\\ninjected: evil\\n---\\nend\"\n---\n@block body:\ndefault\n@end\n" + .to_string(), + ); + modules.insert( + "child.mds".to_string(), + "---\nrole: system\n---\n@extends \"./base.mds\"\n".to_string(), + ); + let result = mds::compile_virtual(modules, "child.mds", None) + .unwrap() + .into_markdown() + .unwrap(); + + // The frontmatter section must be parseable YAML — check the output starts with `---`. + assert!( + result.starts_with("---\n"), + "output must start with a frontmatter fence; got: {result}" + ); + + // Extract just the frontmatter section and verify `injected: evil` is not a key. + let fm = extract_frontmatter(&result) + .expect("output must have a frontmatter section; got: {result}"); + + // `injected: evil` must not appear as a top-level key in the frontmatter YAML. + let has_injected = fm.lines().any(|l| l == "injected: evil"); + assert!( + !has_injected, + "adversarial `injected: evil` must not appear as a top-level FM key; fm={fm}" + ); + + // The legitimate keys must be present. + assert!( + fm.contains("role: system"), + "role key must be in FM; fm={fm}" + ); + assert!(fm.contains("prompt:"), "prompt key must be in FM; fm={fm}"); +} + +#[test] +fn adversarial_block_scalar_with_type_mds_line() { + // A block-scalar value whose lines include `type: mds` must not be confused with + // the top-level `type: mds` directive — only an unindented `type: mds` top-level + // key should be stripped from the output. + // The ` type: mds` indented line inside the block scalar must be preserved. + let source = concat!( + "---\n", + "description: |\n", + " type: mds\n", + " this is a block scalar\n", + "model: gpt-4\n", + "---\n", + "{model}\n", + ); + let result = mds::compile_str(source).unwrap().into_markdown().unwrap(); + + // No unindented `type: mds` line should appear in the output. + let has_top_level_type = result.lines().any(|l| l == "type: mds"); + assert!( + !has_top_level_type, + "unindented 'type: mds' must not appear in output; got: {result}" + ); + + // The description key must be present with block-scalar content preserved. + assert!( + result.contains("description:"), + "description key must be preserved; got: {result}" + ); + + // The model key must be present. + assert!( + result.contains("model: gpt-4"), + "model key must be preserved; got: {result}" + ); + + // Body must resolve {model} to gpt-4. + assert!( + result.contains("gpt-4\n") || result.ends_with("gpt-4"), + "body must render model variable; got: {result}" + ); +} diff --git a/crates/mds-cli/tests/inheritance.rs b/crates/mds-cli/tests/inheritance.rs index 464bb16..00204fb 100644 --- a/crates/mds-cli/tests/inheritance.rs +++ b/crates/mds-cli/tests/inheritance.rs @@ -63,6 +63,9 @@ fn f1_analyst_compile_byte_exact() { let (stdout, stderr, ok) = build_file(fixture("inh_analyst.mds").to_str().unwrap()); assert!(ok, "F1: compile should succeed; stderr: {stderr}"); + // Interior-verbatim (#150/#151): each @block body ends with its trailing \n + // (no longer stripped at the parser level). The skeleton text node `\n` + // between blocks produces an extra blank line after each block body. let expected = concat!( "---\n", "role: data analysis\n", @@ -70,7 +73,9 @@ fn f1_analyst_compile_byte_exact() { "You are a data analysis assistant.\n", "\n", "Perform statistical analysis.\n", + "\n", "You have access to: Python, R\n", + "\n", "Respond in plain text.\n", ); assert_eq!( @@ -119,6 +124,11 @@ fn f11_whitespace_contract_base_blank_lines_preserved() { first_pos < second_pos, "F11: first block must appear before second block in output; got:\n{stdout}" ); + // Byte-exact: body verbatim (trailing \n) + skeleton \n between blocks = one blank line. + assert_eq!( + stdout, "First override.\n\nSecond override.\n", + "F11: byte-exact output check — verbatim body trailing \\n + skeleton blank = blank line" + ); } // ── F2 CLI: standalone base compiles with defaults ──────────────────────────── diff --git a/crates/mds-cli/tests/language.rs b/crates/mds-cli/tests/language.rs index e9d2a48..b04d412 100644 --- a/crates/mds-cli/tests/language.rs +++ b/crates/mds-cli/tests/language.rs @@ -880,27 +880,27 @@ fn if_eq_null_match() { #[test] fn if_eq_strict_no_type_coercion_number_vs_string() { - // `@if x == "3":` with x=3 (number) → strict, no coercion → else branch + // `@if x == "3":` with x=3 (number) → cross-type comparison is now a TypeMismatch error let source = "---\nx: 3\n---\n@if x == \"3\":\ncoercion_yes\n@else:\nstrict_types\n@end\n"; - let result = mds::compile_str(source).unwrap().into_markdown().unwrap(); + let err = mds::compile_str(source).unwrap_err(); + let msg = format!("{err}"); assert!( - result.contains("strict_types"), - "number 3 must not equal string \"3\"" + msg.contains("type mismatch") || msg.contains("mds::type_mismatch"), + "number == string must be a TypeMismatch error, got: {msg}" ); - assert!(!result.contains("coercion_yes"), "coercion must not happen"); } #[test] fn if_eq_strict_no_type_coercion_bool_vs_string() { - // `@if x == "true":` with x=true (bool) → strict, no coercion → else branch + // `@if x == "true":` with x=true (bool) → cross-type comparison is now a TypeMismatch error let source = "---\nx: true\n---\n@if x == \"true\":\ncoercion_yes\n@else:\nstrict_types\n@end\n"; - let result = mds::compile_str(source).unwrap().into_markdown().unwrap(); + let err = mds::compile_str(source).unwrap_err(); + let msg = format!("{err}"); assert!( - result.contains("strict_types"), - "bool true must not equal string \"true\"" + msg.contains("type mismatch") || msg.contains("mds::type_mismatch"), + "bool == string must be a TypeMismatch error, got: {msg}" ); - assert!(!result.contains("coercion_yes"), "coercion must not happen"); } #[test] @@ -990,16 +990,16 @@ fn if_neq_string_match_enters_else_body() { } #[test] -fn if_neq_cross_type_always_true() { - // `@if x != "3":` with x=3 (number) → types differ, always true +fn if_neq_cross_type_errors() { + // `@if x != "3":` with x=3 (number) → type mismatch: number vs string is an error let source = "---\nx: 3\n---\n@if x != \"3\":\ndiff_type_branch\n@else:\nsame_type_branch\n@end\n"; - let result = mds::compile_str(source).unwrap().into_markdown().unwrap(); + let err = mds::compile_str(source).unwrap_err(); + let msg = format!("{err}"); assert!( - result.contains("diff_type_branch"), - "cross-type != must be true" + msg.contains("type mismatch") || msg.contains("mds::type_mismatch"), + "cross-type != must be a TypeMismatch error, got: {msg}" ); - assert!(!result.contains("same_type_branch"), "else must not appear"); } // ── @elseif tests ─────────────────────────────────────────────────────────── diff --git a/crates/mds-core/src/error.rs b/crates/mds-core/src/error.rs index 3d5688a..70a3fd2 100644 --- a/crates/mds-core/src/error.rs +++ b/crates/mds-core/src/error.rs @@ -202,6 +202,25 @@ pub enum MdsError { src: Option>>, }, + /// Cross-type comparison (`string == number`, `boolean != null`, etc.). + /// + /// MDS refuses to silently coerce types in `==` / `!=` conditions. + /// Pass string values with `--set-string KEY=VALUE` to keep them as + /// strings, or use `@if x:` to test for truthiness without a comparison. + #[error("type mismatch: cannot compare {lhs_type} with {rhs_type}")] + #[diagnostic( + code(mds::type_mismatch), + help("left side is {lhs_type}, right side is {rhs_type}; use `@if x:` for truthiness or `--set-string KEY=VALUE` to pass a string") + )] + TypeMismatch { + lhs_type: String, + rhs_type: String, + #[label("cross-type comparison")] + span: Option, + #[source_code] + src: Option>>, + }, + #[error("circular import detected: {cycle}")] #[diagnostic( code(mds::circular_import), @@ -500,6 +519,15 @@ impl MdsError { } } + pub(crate) fn type_mismatch(lhs_type: impl Into, rhs_type: impl Into) -> Self { + MdsError::TypeMismatch { + lhs_type: lhs_type.into(), + rhs_type: rhs_type.into(), + span: None, + src: None, + } + } + pub(crate) fn name_collision(name: impl Into) -> Self { MdsError::NameCollision { name: name.into(), @@ -732,6 +760,7 @@ impl MdsError { | MdsError::UndefinedFunction { span, src, .. } | MdsError::ArityMismatch { span, src, .. } | MdsError::TypeError { span, src, .. } + | MdsError::TypeMismatch { span, src, .. } | MdsError::CircularImport { span, src, .. } | MdsError::FileNotFound { span, src, .. } | MdsError::ImportError { span, src, .. } diff --git a/crates/mds-core/src/evaluator.rs b/crates/mds-core/src/evaluator.rs index 88a8d10..284e522 100644 --- a/crates/mds-core/src/evaluator.rs +++ b/crates/mds-core/src/evaluator.rs @@ -373,7 +373,7 @@ fn invoke_function( scope.set_var(¶m.name, value); } ctx.call_stack.push(call_key.to_string()); - let result = evaluate_nodes(&func.body, scope, ctx); + let result = evaluate_nodes(&func.body, scope, ctx).map(|s| s.trim().to_string()); // Safety-critical LIFO invariant: call_stack tracks recursion detection. // A mismatched pop would silently corrupt recursion state and allow // stack overflows. Return a structured error rather than panicking so @@ -446,17 +446,28 @@ fn call_qualified_function( invoke_function(&func, &qualified_name, args, scope, ctx).map(Value::String) } -/// Compare two runtime `Value` instances using strict equality. +/// Compare two runtime `Value` instances for strict same-type equality. /// -/// Type matching is strict — `Number(3) != String("3")`. Different types always -/// return `false`. `NaN == NaN` is `false` (IEEE 754 via Rust's `f64 ==`). -fn values_equal_runtime(lhs: &Value, rhs: &Value) -> bool { +/// Returns `Some(true)` / `Some(false)` for same-type comparisons, and `None` +/// for cross-type comparisons (caller must surface a `TypeMismatch` error). +/// `NaN == NaN` is `Some(false)` (IEEE 754 via Rust's `f64 ==`). +/// +/// Arrays and objects use structural (deep) equality: `Value` derives `PartialEq`, +/// so this is a recursive element-wise comparison. Note that the condition grammar +/// currently cannot express container literals on the RHS, so same-type container +/// comparisons are only reachable at the unit-test level today — these arms guard +/// future grammar extensions and ensure the contract is correct: same-type always +/// compares structurally, only cross-type is an error. +fn values_equal_same_type(lhs: &Value, rhs: &Value) -> Option { match (lhs, rhs) { - (Value::String(a), Value::String(b)) => a == b, - (Value::Number(a), Value::Number(b)) => a == b, - (Value::Boolean(a), Value::Boolean(b)) => a == b, - (Value::Null, Value::Null) => true, - _ => false, + (Value::String(a), Value::String(b)) => Some(a == b), + (Value::Number(a), Value::Number(b)) => Some(a == b), + (Value::Boolean(a), Value::Boolean(b)) => Some(a == b), + (Value::Null, Value::Null) => Some(true), + (Value::Array(a), Value::Array(b)) => Some(a == b), + (Value::Object(a), Value::Object(b)) => Some(a == b), + // Cross-type: signal TypeMismatch to the caller. + _ => None, } } @@ -479,12 +490,15 @@ fn evaluate_condition( Condition::Eq(lhs, rhs) => { let lhs_val = evaluate_expr(lhs, scope, ctx)?; let rhs_val = evaluate_expr(rhs, scope, ctx)?; - Ok(values_equal_runtime(&lhs_val, &rhs_val)) + values_equal_same_type(&lhs_val, &rhs_val) + .ok_or_else(|| MdsError::type_mismatch(lhs_val.type_name(), rhs_val.type_name())) } Condition::NotEq(lhs, rhs) => { let lhs_val = evaluate_expr(lhs, scope, ctx)?; let rhs_val = evaluate_expr(rhs, scope, ctx)?; - Ok(!values_equal_runtime(&lhs_val, &rhs_val)) + values_equal_same_type(&lhs_val, &rhs_val) + .map(|eq| !eq) + .ok_or_else(|| MdsError::type_mismatch(lhs_val.type_name(), rhs_val.type_name())) } // Short-circuit And: return false on first false operand. // Parser invariant: And operands are always leaf conditions (parse_and_level calls @@ -1197,22 +1211,146 @@ mod tests { ); } - // ── values_equal_runtime: NaN semantics ───────────────────────────────── + // ── values_equal_same_type: NaN semantics ─────────────────────────────── #[test] fn values_equal_nan_is_not_equal_to_itself() { - // IEEE 754 defines NaN != NaN. values_equal_runtime must follow this — even + // IEEE 754 defines NaN != NaN. values_equal_same_type must follow this — even // though the parser rejects NaN literals in condition expressions, the runtime // Value type holds f64 and could theoretically carry a NaN produced by arithmetic. - // Verify that values_equal_runtime returns false for NaN == NaN. + // Verify that values_equal_same_type returns Some(false) for NaN == NaN. let nan_value = Value::Number(f64::NAN); let nan_value2 = Value::Number(f64::NAN); - assert!( - !values_equal_runtime(&nan_value, &nan_value2), + assert_eq!( + values_equal_same_type(&nan_value, &nan_value2), + Some(false), "NaN must not equal NaN (IEEE 754)" ); } + #[test] + fn values_equal_same_type_returns_none_for_cross_type() { + // Cross-type comparisons must return None, not a bool. + let s = Value::String("3".to_string()); + let n = Value::Number(3.0); + let b = Value::Boolean(true); + let null = Value::Null; + + assert_eq!(values_equal_same_type(&s, &n), None, "string vs number"); + assert_eq!(values_equal_same_type(&n, &b), None, "number vs boolean"); + assert_eq!(values_equal_same_type(&b, &null), None, "boolean vs null"); + assert_eq!(values_equal_same_type(&s, &null), None, "string vs null"); + } + + // ── values_equal_same_type: container structural equality ──────────────── + + #[test] + fn values_equal_same_type_arrays_equal() { + // [1, 2] == [1, 2] → Some(true) + let a = Value::Array(vec![Value::Number(1.0), Value::Number(2.0)]); + let b = Value::Array(vec![Value::Number(1.0), Value::Number(2.0)]); + assert_eq!( + values_equal_same_type(&a, &b), + Some(true), + "[1,2] == [1,2] must be Some(true)" + ); + } + + #[test] + fn values_equal_same_type_arrays_order_matters() { + // [1, 2] != [2, 1] → Some(false) + let a = Value::Array(vec![Value::Number(1.0), Value::Number(2.0)]); + let b = Value::Array(vec![Value::Number(2.0), Value::Number(1.0)]); + assert_eq!( + values_equal_same_type(&a, &b), + Some(false), + "[1,2] vs [2,1] must be Some(false) — order is significant" + ); + } + + #[test] + fn values_equal_same_type_objects_equal() { + // {a: 1} == {a: 1} → Some(true) + let mut m1 = std::collections::HashMap::new(); + m1.insert("a".to_string(), Value::Number(1.0)); + let mut m2 = std::collections::HashMap::new(); + m2.insert("a".to_string(), Value::Number(1.0)); + assert_eq!( + values_equal_same_type(&Value::Object(m1), &Value::Object(m2)), + Some(true), + "{{a:1}} == {{a:1}} must be Some(true)" + ); + } + + #[test] + fn values_equal_same_type_objects_unequal() { + // {a: 1} != {a: 2} → Some(false) + let mut m1 = std::collections::HashMap::new(); + m1.insert("a".to_string(), Value::Number(1.0)); + let mut m2 = std::collections::HashMap::new(); + m2.insert("a".to_string(), Value::Number(2.0)); + assert_eq!( + values_equal_same_type(&Value::Object(m1), &Value::Object(m2)), + Some(false), + "{{a:1}} vs {{a:2}} must be Some(false)" + ); + } + + #[test] + fn values_equal_same_type_nested_array_equal() { + // [[1], [2]] == [[1], [2]] → Some(true) + let a = Value::Array(vec![ + Value::Array(vec![Value::Number(1.0)]), + Value::Array(vec![Value::Number(2.0)]), + ]); + let b = Value::Array(vec![ + Value::Array(vec![Value::Number(1.0)]), + Value::Array(vec![Value::Number(2.0)]), + ]); + assert_eq!( + values_equal_same_type(&a, &b), + Some(true), + "nested [[1],[2]] == [[1],[2]] must be Some(true)" + ); + } + + #[test] + fn values_equal_same_type_empty_arrays_equal() { + // [] == [] → Some(true) + let a = Value::Array(vec![]); + let b = Value::Array(vec![]); + assert_eq!( + values_equal_same_type(&a, &b), + Some(true), + "[] == [] must be Some(true)" + ); + } + + #[test] + fn values_equal_same_type_array_nan_element() { + // [NaN] == [NaN] → Some(false) because NaN != NaN per IEEE 754 + let a = Value::Array(vec![Value::Number(f64::NAN)]); + let b = Value::Array(vec![Value::Number(f64::NAN)]); + assert_eq!( + values_equal_same_type(&a, &b), + Some(false), + "[NaN] == [NaN] must be Some(false) — NaN ≠ NaN propagates through arrays" + ); + } + + #[test] + fn values_equal_same_type_array_element_type_mismatch_returns_false() { + // [1] vs ["1"] is a same-type (both Array) comparison but with a differing + // element type — must return Some(false) (not None / not an error). + let a = Value::Array(vec![Value::Number(1.0)]); + let b = Value::Array(vec![Value::String("1".to_string())]); + assert_eq!( + values_equal_same_type(&a, &b), + Some(false), + "[1] vs [\"1\"] must be Some(false) — element type differs but outer type matches" + ); + } + // ── Resource limit: MAX_OUTPUT_SIZE ────────────────────────────────────── #[test] diff --git a/crates/mds-core/src/formatter.rs b/crates/mds-core/src/formatter.rs index febab63..7f5c71c 100644 --- a/crates/mds-core/src/formatter.rs +++ b/crates/mds-core/src/formatter.rs @@ -16,62 +16,52 @@ //! //! 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. +//! 2. Compute *raw-content* byte ranges (`@message`/`@define` bodies) 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). +//! only rules that are provably output-preserving (R1, R2, 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) +//! # Ruleset (v2 — interior-verbatim) //! -//! - **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. +//! - **R1 (CRLF/CR removal)** — applied everywhere *except* `@message`/`@define` +//! bodies (`raw_content_spans`). `clean_output` strips every `\r` from the +//! final compiled string; frontmatter lexing and directive matching also strip +//! `\r` before comparison. Only raw-content bodies bypass `clean_output` and +//! therefore must be left byte-exact. +//! - **R2 (exactly one final newline)** — the whole body is trimmed to its last +//! non-whitespace byte, then a single `\n` is appended, matching +//! `clean_output`'s own trailing-edge normalisation. //! - **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. +//! line is discarded pre-parse and never reaches output. Applied even inside +//! `@message`/`@define` bodies since directive text is never part of compiled +//! output in either mode. //! -//! 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. +//! R3 (blank-line run capping) was present in v1 but **removed** in v2 (#150, +//! #151). `clean_output` now passes interior content through verbatim — it only +//! normalises the trailing edge — so capping blank-line runs in the formatter +//! would *change* compiled output and violate the load-bearing constraint. +//! Code-fence regions no longer need a special protection class: they receive +//! the same treatment as ordinary body content (R1 only), because without R3 +//! there is nothing to gate on those boundaries. +//! +//! `@message`/`@define` bodies (see `raw_content_spans`) are a THIRD region +//! that is copied completely verbatim — not even R1's `\r` removal is applied. +//! This was discovered empirically: `@message` content is built by +//! `evaluate_nodes(...).trim()` with NO `clean_output` pass (see +//! `collect_single_message` in `evaluator.rs`), so a `\r` 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. //! //! ## 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 +//! Stripping trailing whitespace from all-whitespace "blank" lines would break +//! compile equivalence: `clean_output` passes a whitespace-only line through +//! verbatim (the space is not a `\n`, so it resets no run counter). See //! `r4_whitespace_only_line_in_middle_of_document_is_preserved_verbatim` in //! `tests/fmt.rs` for the regression test that locks this in. //! @@ -103,10 +93,10 @@ use crate::lexer::{self, Token}; /// # 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. +/// // CRLF -> LF; interior blank lines are left verbatim (interior-verbatim +/// // contract, #150/#151 — R3 blank-run capping was removed in v2). /// let formatted = mds::format_str("Hello \r\n\r\n\r\nworld\r\n")?; -/// assert_eq!(formatted, "Hello \n\nworld\n"); +/// assert_eq!(formatted, "Hello \n\n\nworld\n"); /// # Ok::<(), Box>(()) /// ``` #[must_use = "the formatted source should be used"] @@ -126,12 +116,11 @@ pub fn format_str(source: &str) -> Result { #[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); + let formatted = rewrite(source, body_start, &raw_content, &directives); assert_equivalent(source, &formatted, base_dir, &raw_content)?; Ok(formatted) } @@ -154,35 +143,6 @@ fn token_offset(t: &Token) -> usize { // ── 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 @@ -205,8 +165,7 @@ fn directive_line_offsets(tokens: &[Token]) -> BTreeSet { /// 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). +/// any, is ordinary body content subject to R1 only). fn body_start_offset(tokens: &[Token], src: &str) -> usize { let has_frontmatter = matches!(tokens.first(), Some(Token::FrontmatterFence(0))); if !has_frontmatter { @@ -216,26 +175,19 @@ fn body_start_offset(tokens: &[Token], src: &str) -> usize { } /// 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. +/// not even R1's `\r` removal 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 +/// VERIFIED against the live evaluator: `@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"`). +/// in `evaluator.rs` — with NO `clean_output` pass. A `\r` inside a message +/// body reaches the compiled JSON verbatim: `@message user:\r\nHi\r\n@end\r\n` +/// compiles to `"Hi\r\n"` (the `\r` survives). /// /// `@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 +/// can be called from a markdown-mode site or from within a `@message` body, +/// and the formatter can't tell which without a full call-graph analysis. +/// `@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). @@ -276,7 +228,7 @@ fn raw_content_spans(tokens: &[Token], src: &str) -> Vec> { // 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 + // EOF, rather than letting R1 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 { @@ -309,20 +261,18 @@ fn strip_cr(s: &str) -> std::borrow::Cow<'_, str> { } } -// ── Rewrite (R1-R4) ────────────────────────────────────────────────────────── +// ── Rewrite (R1, R2, 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`). +/// construction: trimming an already-trimmed directive line is a no-op, +/// and every rule acts on a disjoint line classification (directive / +/// raw-content / ordinary, checked in that priority order — see `rewrite_body`). fn rewrite( source: &str, body_start: usize, - protected: &[Range], raw_content: &[Range], directives: &BTreeSet, ) -> String { @@ -333,8 +283,8 @@ fn rewrite( 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. + // invisible to `clean_output`'s trailing-edge normalisation, which only ever + // sees the body. if body_start > 0 { debug_assert!( source.is_char_boundary(body_start), @@ -343,7 +293,7 @@ fn rewrite( push_stripped_cr(&mut out, &source[..body_start]); } - let body = rewrite_body(source, body_start, protected, raw_content, directives); + let body = rewrite_body(source, body_start, raw_content, directives); let trimmed = body.trim_end(); if trimmed.is_empty() { @@ -368,31 +318,21 @@ fn rewrite( /// 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. +/// 1. **Directive** — R4 (trailing-whitespace strip) + R1 (\r strip); directive +/// text is never part of any compiled output in any mode, so trimming it is +/// always safe, even inside a raw-content span. /// 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. +/// `clean_output` entirely (see `raw_content_spans`), so not even R1's \r +/// removal may touch it. +/// 3. **Everything else** (ordinary body, code-fence lines) — R1 (\r strip) +/// only; interior content is left verbatim (interior-verbatim contract, #150). fn rewrite_body( source: &str, body_start: usize, - protected: &[Range], 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" @@ -402,19 +342,9 @@ fn rewrite_body( 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; - } + // Skip past any raw spans that end at or before body_start. while ri < raw_content.len() && raw_content[ri].end <= body_start { ri += 1; } @@ -429,13 +359,6 @@ fn rewrite_body( 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; } @@ -446,58 +369,24 @@ fn rewrite_body( 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. + // Priority 1: directive lines — R4 + R1. 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. + // Priority 2: @message/@define bodies — verbatim copy, not even R1. 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. + } else { + // Priority 3: ordinary body and code-fence lines — R1 only. 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; @@ -647,33 +536,6 @@ fn structural_equivalent(source: &str, formatted: &str, raw_content: &[Range> = 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" + structural_equivalent(src, same, &raw), + "identical source must be structural_equivalent" + ); + assert!( + !structural_equivalent(src, different, &raw), + "different blank-line counts must NOT be structural_equivalent \ + (clean_output is interior-verbatim; a 3-newline run != 2-newline run)" ); } } diff --git a/crates/mds-core/src/lexer.rs b/crates/mds-core/src/lexer.rs index 8fe8edb..adf8c1f 100644 --- a/crates/mds-core/src/lexer.rs +++ b/crates/mds-core/src/lexer.rs @@ -37,8 +37,9 @@ struct Lexer<'a> { byte_offsets: Vec, pos: usize, tokens: Vec, - /// Non-zero when inside a fenced code block; holds the opening backtick count. - code_fence_backticks: usize, + /// When inside a fenced code block: `Some((fence_char, fence_count, opener_byte_offset))`. + /// `None` when not inside any code block. + code_fence: Option<(char, usize, usize)>, } impl<'a> Lexer<'a> { @@ -56,7 +57,7 @@ impl<'a> Lexer<'a> { byte_offsets, pos: 0, tokens: Vec::new(), - code_fence_backticks: 0, + code_fence: None, } } @@ -123,61 +124,70 @@ impl<'a> Lexer<'a> { Ok(()) } - /// Scan a code fence (opening or closing ` ``` `). + /// Scan a code fence (opening or closing) given the pre-parsed fence info. /// - /// Returns `true` if a fence was processed and the caller should `continue` - /// the main dispatch loop. Returns `false` if this position is inside a code - /// block with fewer backticks than needed — the caller falls through to - /// `scan_code_content`. - fn scan_code_fence(&mut self) -> bool { + /// `fence_char`: the repeated character (`` ` `` or `~`). + /// `fence_count`: how many fence chars are present. + /// `is_close`: whether the rest of the line is spaces/tabs only (close candidate). + /// + /// Returns `true` when the fence was consumed and the caller should `continue`. + /// Returns `false` when we are inside a block but this line does not match the + /// opener — the caller falls through to `scan_code_content`. + fn scan_code_fence(&mut self, fence_char: char, fence_count: usize, is_close: bool) -> bool { let bp = self.byte_pos(self.pos); - let (backtick_count, rest_is_close) = scan_fence(&self.chars, self.pos); - - if self.code_fence_backticks == 0 { - // Opening fence — record the backtick count - let fence_start = bp; - self.pos += backtick_count; - let mut fence = "`".repeat(backtick_count); - // consume any remaining language tag characters - while self.pos < self.chars.len() - && self.chars[self.pos] != '\n' - && self.chars[self.pos] != '\r' - { - fence.push(self.chars[self.pos]); - self.pos += 1; + + match self.code_fence { + None => { + // Opening fence: capture verbatim up to (but not including) the first + // \r or \n (strips \r, matches R1). + let line_end = self.chars[self.pos..] + .iter() + .position(|&c| c == '\n' || c == '\r') + .map(|rel| self.pos + rel) + .unwrap_or(self.chars.len()); + let fence = self.source[bp..self.byte_pos(line_end)].to_string(); + self.pos = skip_newline(&self.chars, line_end); + self.code_fence = Some((fence_char, fence_count, bp)); + self.tokens.push(Token::CodeFence(fence, bp)); + true + } + Some((open_char, open_count, _)) => { + if is_close && fence_char == open_char && fence_count >= open_count { + // Closing fence: same char, at least as many, no info string. + let line_end = self.chars[self.pos..] + .iter() + .position(|&c| c == '\n' || c == '\r') + .map(|rel| self.pos + rel) + .unwrap_or(self.chars.len()); + let fence = self.source[bp..self.byte_pos(line_end)].to_string(); + self.pos = skip_newline(&self.chars, line_end); + self.code_fence = None; + self.tokens.push(Token::CodeFence(fence, bp)); + true + } else { + // Inside a block but this line does not close it — fall through to CodeContent. + false + } } - self.pos = skip_newline(&self.chars, self.pos); - self.code_fence_backticks = backtick_count; - self.tokens.push(Token::CodeFence(fence, fence_start)); - true - } else if rest_is_close && backtick_count >= self.code_fence_backticks { - // Closing fence — must have >= opening backtick count, no non-space suffix - let fence_start = bp; - self.pos += backtick_count; - let fence = "`".repeat(backtick_count); - self.pos = skip_newline(&self.chars, self.pos); - self.code_fence_backticks = 0; - self.tokens.push(Token::CodeFence(fence, fence_start)); - true - } else { - // Fewer backticks inside block — falls through to CodeContent - false } } /// Scan raw content inside a code block (no interpolation). /// - /// Precondition: `self.code_fence_backticks > 0`. - /// Advances `self.pos` up to (but not past) the closing fence. + /// Precondition: `self.code_fence.is_some()`. + /// Advances `self.pos` up to (but not past) the closing fence line. fn scan_code_content(&mut self) { let start = self.byte_pos(self.pos); let mut content = String::new(); while self.pos < self.chars.len() { - let bp = self.byte_pos(self.pos); - if is_line_start_chars(&self.chars, self.pos) && self.source[bp..].starts_with("```") { - let (bc, is_close) = scan_fence(&self.chars, self.pos); - if is_close && bc >= self.code_fence_backticks { - break; + if is_line_start_chars(&self.chars, self.pos) { + if let Some((_, fc, fc_count, is_close)) = try_scan_fence_at(&self.chars, self.pos) + { + if let Some((open_char, open_count, _)) = self.code_fence { + if is_close && fc == open_char && fc_count >= open_count { + break; + } + } } } content.push(self.chars[self.pos]); @@ -297,8 +307,7 @@ impl<'a> Lexer<'a> { if c == '@' { break; } - let bp = self.byte_pos(self.pos); - if self.source[bp..].starts_with("```") { + if try_scan_fence_at(&self.chars, self.pos).is_some() { break; } } @@ -319,17 +328,22 @@ impl<'a> Lexer<'a> { while self.pos < self.chars.len() { let at_line_start = self.is_line_start(); - let bp = self.byte_pos(self.pos); - // Code fence (opening or closing). + // Code fence (opening or closing): any `[ \t>]*` prefix + ≥3 `` ` `` or `~`. // `scan_code_fence` returns true when it consumed the fence; false means - // we are inside a block with fewer backticks and fall through to CodeContent. - if at_line_start && self.source[bp..].starts_with("```") && self.scan_code_fence() { - continue; + // we are inside a block but this line is not the closer — fall through to CodeContent. + if at_line_start { + if let Some((_, fence_char, fence_count, is_close)) = + try_scan_fence_at(&self.chars, self.pos) + { + if self.scan_code_fence(fence_char, fence_count, is_close) { + continue; + } + } } - // Inside a code block: raw content only - if self.code_fence_backticks > 0 { + // Inside a code block: raw content only. + if self.code_fence.is_some() { self.scan_code_content(); continue; } @@ -355,9 +369,15 @@ impl<'a> Lexer<'a> { self.scan_text(); } - // Check for unclosed code block - if self.code_fence_backticks > 0 { - return Err(MdsError::syntax("unclosed code fence")); + // Check for unclosed code block — include the opener's byte offset for diagnostics. + if let Some((_, _, opener_offset)) = self.code_fence { + return Err(MdsError::syntax_at( + "unclosed code fence", + self.file, + self.source, + opener_offset, + 3, + )); } Ok(self.tokens) @@ -370,18 +390,40 @@ fn is_line_start_chars(chars: &[char], pos: usize) -> bool { pos == 0 || chars[pos - 1] == '\n' } -/// Count consecutive backticks starting at `pos` and determine whether the -/// rest of the line (after the backticks) contains only optional whitespace. +/// Check whether a code fence begins at position `pos` in `chars`. /// -/// Returns `(count, is_close_candidate)` where `is_close_candidate` is true -/// when nothing follows the backticks except spaces/tabs before EOL or EOF. -fn scan_fence(chars: &[char], pos: usize) -> (usize, bool) { - let count = chars[pos..].iter().take_while(|&&c| c == '`').count(); - let is_close = chars[pos + count..] +/// Scans optional `[ \t>]*` prefix, then requires ≥ 3 consecutive `` ` `` or +/// `~` characters. Returns `Some((prefix_len, fence_char, fence_count, is_close))` +/// or `None` when no valid fence is present. +/// +/// `is_close` is `true` when nothing follows the fence chars (before EOL/EOF) +/// except spaces or tabs — indicating this line can serve as a closing fence. +fn try_scan_fence_at(chars: &[char], pos: usize) -> Option<(usize, char, usize, bool)> { + let prefix_len = chars[pos..] + .iter() + .take_while(|&&c| c == ' ' || c == '\t' || c == '>') + .count(); + let fence_start = pos + prefix_len; + if fence_start >= chars.len() { + return None; + } + let fence_char = chars[fence_start]; + if fence_char != '`' && fence_char != '~' { + return None; + } + let fence_count = chars[fence_start..] + .iter() + .take_while(|&&c| c == fence_char) + .count(); + if fence_count < 3 { + return None; + } + let after_fence = fence_start + fence_count; + let is_close = chars[after_fence..] .iter() .take_while(|&&c| c != '\n' && c != '\r') .all(|&c| c == ' ' || c == '\t'); - (count, is_close) + Some((prefix_len, fence_char, fence_count, is_close)) } /// Advance `pos` past a line ending (`\n`, `\r\n`, or bare `\r`), if present. @@ -453,4 +495,105 @@ mod tests { .iter() .any(|t| matches!(t, Token::Directive(d, _) if d.starts_with("@if")))); } + + // ── #149: widened fence recognition ────────────────────────────────────── + + #[test] + fn tilde_fence_is_recognized_as_code_block() { + // Tilde fences (~~~) must protect their content from interpolation. + let src = "text\n~~~python\n{no_interp}\n~~~\nmore\n"; + let tokens = tokenize(src, "test.mds").unwrap(); + assert!( + tokens.iter().any(|t| matches!(t, Token::CodeContent(_, _))), + "tilde fence must produce CodeContent tokens" + ); + assert!( + !tokens + .iter() + .any(|t| matches!(t, Token::Interpolation(s, _) if s == "no_interp")), + "interpolation inside tilde fence must not be parsed" + ); + } + + #[test] + fn indented_backtick_fence_is_recognized() { + // Up to 3 spaces of indentation before ``` is valid CommonMark. + let src = "text\n ```python\n{no_interp}\n ```\nmore\n"; + let tokens = tokenize(src, "test.mds").unwrap(); + assert!( + tokens.iter().any(|t| matches!(t, Token::CodeContent(_, _))), + "indented fence must produce CodeContent tokens" + ); + assert!( + !tokens + .iter() + .any(|t| matches!(t, Token::Interpolation(s, _) if s == "no_interp")), + "interpolation inside indented fence must not be parsed" + ); + } + + #[test] + fn blockquoted_fence_is_recognized() { + // A `>` blockquote prefix before ``` must trigger fence recognition. + let src = "text\n> ```python\n> {no_interp}\n> ```\nmore\n"; + let tokens = tokenize(src, "test.mds").unwrap(); + assert!( + tokens.iter().any(|t| matches!(t, Token::CodeContent(_, _))), + "blockquoted fence must produce CodeContent tokens" + ); + assert!( + !tokens + .iter() + .any(|t| matches!(t, Token::Interpolation(s, _) if s == "no_interp")), + "interpolation inside blockquoted fence must not be parsed" + ); + } + + #[test] + fn backtick_fence_not_closed_by_tilde_fence() { + // A tilde closer must NOT close a backtick opener (and vice versa). + let src = "```\ncontent\n~~~\nstill inside\n```\n"; + let tokens = tokenize(src, "test.mds").unwrap(); + let content: Vec<&str> = tokens + .iter() + .filter_map(|t| match t { + Token::CodeContent(s, _) => Some(s.as_str()), + _ => None, + }) + .collect(); + let combined = content.join(""); + assert!( + combined.contains("still inside"), + "tilde line must not close a backtick fence; combined content: {combined:?}" + ); + } + + #[test] + fn unclosed_fence_error_includes_opener_span() { + // Unclosed fence now reports location of the opener. + let err = tokenize("```python\nunclosed content\n", "test.mds").unwrap_err(); + // At minimum the error should mention the problem; span is optional in the msg repr. + assert!( + format!("{err}").contains("unclosed"), + "unclosed fence error must mention 'unclosed', got: {err}" + ); + } + + #[test] + fn four_backtick_fence_not_closed_by_three_backtick_closer() { + // A closing fence must have >= the opener's count. + let src = "````python\ncontent\n```\nstill inside\n````\n"; + let tokens = tokenize(src, "test.mds").unwrap(); + let combined: String = tokens + .iter() + .filter_map(|t| match t { + Token::CodeContent(s, _) => Some(s.as_str()), + _ => None, + }) + .collect(); + assert!( + combined.contains("still inside"), + "three-backtick line must not close a four-backtick fence; got: {combined:?}" + ); + } } diff --git a/crates/mds-core/src/lib.rs b/crates/mds-core/src/lib.rs index 5b321cc..948bd58 100644 --- a/crates/mds-core/src/lib.rs +++ b/crates/mds-core/src/lib.rs @@ -636,39 +636,32 @@ pub(crate) fn prepend_frontmatter(raw: Option<&str>, body: String) -> String { format!("---\n{cleaned}---\n{body}") } -/// 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. +/// Normalise output whitespace using the "interior-verbatim with trailing-edge +/// normalisation" contract (#150/#151): +/// +/// - Strip every `\r` character (Windows line endings — `\r\n` becomes `\n`). +/// - Trim all trailing whitespace from the very end of the string. +/// - If the result is non-empty, ensure exactly one final `\n`. +/// - Whitespace-only / empty input returns `""`. +/// +/// Interior blank-line runs, leading blank lines, and mid-document whitespace-only +/// lines all pass through **verbatim** — only the trailing edge is normalised. +/// +/// **Formatter dependency**: `mds fmt` (`formatter.rs`) mirrors these exact semantics. +/// `@message` and `@define` body content bypasses this function entirely (evaluated +/// via `.trim()` only — see `collect_single_message` and `invoke_function` in +/// `evaluator.rs`); the formatter marks those as raw-content regions. See +/// `raw_content_spans` in `formatter.rs` for the authoritative region boundary. pub(crate) fn clean_output(s: &str) -> String { - let mut result = String::with_capacity(s.len()); - let mut newline_count = 0; - - // Trim leading newlines (any line ending style) - let s = s.trim_start_matches(['\n', '\r']); - - for ch in s.chars() { - if ch == '\n' { - newline_count += 1; - if newline_count <= 2 { - result.push(ch); - } - } else if ch == '\r' { - // Skip \r, handled with \n - } else { - newline_count = 0; - result.push(ch); - } - } + // Strip \r unconditionally (Windows line endings). + let no_cr: std::borrow::Cow = if s.as_bytes().contains(&b'\r') { + std::borrow::Cow::Owned(s.chars().filter(|&c| c != '\r').collect()) + } else { + std::borrow::Cow::Borrowed(s) + }; - // Trim trailing whitespace but keep one final newline - let trimmed = result.trim_end(); + // Trailing-edge normalisation: trim all trailing whitespace, then add one \n. + let trimmed = no_cr.trim_end(); if trimmed.is_empty() { return String::new(); } @@ -1080,13 +1073,15 @@ mod tests { use super::*; #[test] - fn clean_output_collapses_excess_newlines() { - assert_eq!(clean_output("a\n\n\n\nb"), "a\n\nb\n"); + fn clean_output_preserves_interior_blank_runs() { + // Interior blank-line runs pass through verbatim — no collapsing (#150/#151). + assert_eq!(clean_output("a\n\n\n\nb"), "a\n\n\n\nb\n"); } #[test] - fn clean_output_trims_leading_newlines() { - assert_eq!(clean_output("\n\n\nhello"), "hello\n"); + fn clean_output_preserves_leading_newlines() { + // Leading blank lines are no longer stripped — interior-verbatim contract (#150/#151). + assert_eq!(clean_output("\n\n\nhello"), "\n\n\nhello\n"); } #[test] diff --git a/crates/mds-core/src/parser.rs b/crates/mds-core/src/parser.rs index a814c1c..9e900a0 100644 --- a/crates/mds-core/src/parser.rs +++ b/crates/mds-core/src/parser.rs @@ -534,7 +534,6 @@ impl Parser<'_> { let _guard = MessageGuard(self); let body = _guard.0.parse_body(&["@end"], &[])?; - let body = strip_trailing_newline(strip_leading_newline(body)); _guard.0.consume_end("@message")?; @@ -552,8 +551,9 @@ impl Parser<'_> { /// so any `?` on name parsing does not leave them in an inconsistent state. /// A `BlockGuard` Drop implementation ensures both are restored on every exit path. /// - /// Block bodies have their leading/trailing blank lines stripped — same as - /// `@message` and `@define` (decision #9). + /// Block body bytes pass through verbatim (interior-verbatim whitespace contract, + /// #150/#151): the directive lines (@block … :, @end) are consumed, everything + /// else is left exactly as authored. fn parse_block(&mut self, rest: &str, offset: usize) -> Result { // Reject @block inside other blocks (top-level only — decision #5). // E9: @block-nesting → mds::syntax (correct; not mds::extends — per error-code mapping). @@ -596,7 +596,6 @@ impl Parser<'_> { let _guard = BlockGuard(self); let body = _guard.0.parse_body(&["@end"], &[])?; - let body = strip_trailing_newline(strip_leading_newline(body)); _guard.0.consume_end("@block")?; @@ -637,9 +636,6 @@ impl Parser<'_> { let body = self.parse_body(&["@end"], &[])?; - // Trim surrounding newlines added by the block's colons and @end lines. - let body = strip_trailing_newline(strip_leading_newline(body)); - self.consume_end("@define")?; self.depth -= 1; diff --git a/crates/mds-core/src/parser_helpers.rs b/crates/mds-core/src/parser_helpers.rs index 34b0827..643d40b 100644 --- a/crates/mds-core/src/parser_helpers.rs +++ b/crates/mds-core/src/parser_helpers.rs @@ -15,8 +15,7 @@ //! - **Interpolation parsing** — `parse_interpolation_expr`, `parse_dot_expr`, //! `parse_args`, `parse_args_inner`, `parse_single_arg`, `parse_single_arg_inner` //! - **Utilities** — `parse_quoted_path`, `validate_dot_path_parts`, -//! `unescape_string`, `is_valid_identifier`, `is_directive_token`, -//! `strip_leading_newline`, `strip_trailing_newline` +//! `unescape_string`, `is_valid_identifier`, `is_directive_token` use std::collections::HashSet; @@ -1099,7 +1098,7 @@ pub(super) fn parse_interpolation_expr( if !is_valid_identifier(content) { return Err(MdsError::syntax_at( format!( - "invalid interpolation: '{content}' is not a valid expression. Use a variable name (letters, numbers, underscores), a function call like func(), or escape with \\{{{{ for literal braces." + "invalid interpolation: '{content}' is not a valid expression. Use a variable name (letters, numbers, underscores), a function call like func(), or escape with \\{{ for literal braces." ), file, source, offset, len, )); @@ -1468,36 +1467,3 @@ pub(crate) fn is_valid_identifier(s: &str) -> bool { (first.is_ascii_alphabetic() || first == '_') && chars.all(|c| c.is_ascii_alphanumeric() || c == '_') } - -/// Strip a leading newline from the body text nodes. -pub(super) fn strip_leading_newline(mut nodes: Vec) -> Vec { - if let Some(Node::Text(t)) = nodes.first_mut() { - if let Some(stripped) = t - .text - .strip_prefix("\r\n") - .or_else(|| t.text.strip_prefix('\n')) - { - t.text = stripped.to_string(); - } - if t.text.is_empty() { - nodes.remove(0); - } - } - nodes -} - -/// Strip a trailing newline from the body text nodes. -pub(super) fn strip_trailing_newline(mut nodes: Vec) -> Vec { - if let Some(Node::Text(t)) = nodes.last_mut() { - if t.text.ends_with('\n') { - t.text.pop(); - if t.text.ends_with('\r') { - t.text.pop(); - } - } - if t.text.is_empty() { - nodes.pop(); - } - } - nodes -} diff --git a/crates/mds-core/src/parser_tests.rs b/crates/mds-core/src/parser_tests.rs index 085a09c..af1a3de 100644 --- a/crates/mds-core/src/parser_tests.rs +++ b/crates/mds-core/src/parser_tests.rs @@ -339,6 +339,28 @@ fn parse_invalid_dot_path_interpolation_returns_error() { ); } +#[test] +fn invalid_interpolation_hint_recommends_single_brace_escape() { + // Regression test for #153: the hint must say `\{` (one brace) not `\{{` (two). + // The format string in parser_helpers.rs was `\\{{{{` (rendered `\{{`) — + // fixed to `\\{{` (renders `\{`). + let src = "{123invalid}"; + let tokens = tokenize(src, "test.mds").unwrap(); + let result = parse_with_ctx(&tokens, "test.mds", src); + assert!(result.is_err(), "invalid interpolation should fail"); + let err_msg = format!("{}", result.unwrap_err()); + // Must contain `\{` as the escape hint. + assert!( + err_msg.contains("escape with \\{"), + "hint must recommend escape with \\{{ (single brace), got: {err_msg}" + ); + // Must NOT say `\{{` (double brace — the old, broken form). + assert!( + !err_msg.contains("escape with \\{{"), + "hint must NOT say \\{{{{ (double brace), got: {err_msg}" + ); +} + // --- Tests for MAX_DOT_SEGMENTS limit --- #[test] @@ -2008,23 +2030,22 @@ fn parse_block_empty_body() { } #[test] -fn parse_block_body_edge_newlines_stripped() { - // Leading/trailing single newline stripped like @message/@define (decision #9). - // Input: @block intro:\n\nHello world.\n\n@end\n - // strip_leading_newline removes the first \n; strip_trailing_newline removes the last \n. - // Result: text = "\nHello world.\n" — inner blank line preserved, outermost \n stripped. +fn parse_block_body_verbatim() { + // Interior-verbatim whitespace contract (#150/#151): @block body bytes pass + // through exactly as authored. The directive lines (@block … :, @end) are + // consumed but everything else is preserved — the trailing \n before @end + // is now part of the body. let src = "@block intro:\nHello world.\n@end\n"; let tokens = tokenize(src, "test.mds").unwrap(); let module = parse_with_ctx(&tokens, "", "").unwrap(); if let Node::Block(b) = &module.body[0] { assert!(!b.body.is_empty(), "block body should not be empty"); - // strip_leading_newline strips the newline immediately after the colon. - // strip_trailing_newline strips the newline before @end. - // Resulting text should be exactly "Hello world." with no surrounding newlines. + // The body text is the literal bytes between the directive line's newline + // and the @end line: "Hello world.\n". if let Node::Text(t) = &b.body[0] { assert_eq!( - t.text, "Hello world.", - "block body text should have leading/trailing newlines stripped" + t.text, "Hello world.\n", + "block body text must be verbatim (trailing \\n before @end preserved)" ); } else { panic!("expected Text node in block body"); diff --git a/crates/mds-core/src/resolver.rs b/crates/mds-core/src/resolver.rs index 4299ab7..9abb8d0 100644 --- a/crates/mds-core/src/resolver.rs +++ b/crates/mds-core/src/resolver.rs @@ -584,8 +584,9 @@ impl ModuleCache { let tokens = tokenize(ctx.source, ctx.file_str)?; let module = parse_with_ctx(&tokens, ctx.file_str, ctx.source)?; - // The child's raw frontmatter is what gets re-emitted (after stripping reserved - // keys) in Markdown mode — captured before `module` is partially moved below. + // For the standalone (non-extends) path, the module's own raw frontmatter is + // re-emitted in Markdown mode. Captured before `module` is partially moved below. + // The @extends path uses `merged_frontmatter` from ExtendsComponents instead (#154). let raw_frontmatter = module.frontmatter.as_ref().map(|fm| fm.raw.clone()); // ── Extends branch (decision #8) ───────────────────────────────────── @@ -612,6 +613,7 @@ impl ModuleCache { let ExtendsComponents { final_body, mut scope, + merged_frontmatter, .. } = components; @@ -636,7 +638,9 @@ impl ModuleCache { let body = evaluate(&final_body, &mut scope, warnings)?; let body_clean = crate::clean_output(&body); - let final_str = crate::prepend_frontmatter(raw_frontmatter.as_deref(), body_clean); + // #154: emit deep-merged frontmatter (base < child, reserved keys excluded) + // rather than the child's raw frontmatter. + let final_str = crate::prepend_frontmatter(merged_frontmatter.as_deref(), body_clean); return Ok(crate::CompiledOutput::Markdown(final_str)); } @@ -736,14 +740,7 @@ impl ModuleCache { // Branch: child template (@extends) vs. standalone module. if let Some(ext) = module.extends.clone() { - return self.process_module_extends( - module, - ext, - ctx, - raw_frontmatter, - frontmatter_values, - warnings, - ); + return self.process_module_extends(module, ext, ctx, frontmatter_values, warnings); } // ── Standalone (non-extending) path ────────────────────────────────── @@ -826,7 +823,7 @@ impl ModuleCache { base_key: &str, ctx: &ModuleCtx<'_>, warnings: &mut Vec, - ) -> Result { + ) -> Result<(Scope, serde_yaml_ng::Mapping), MdsError> { // 3d-i: Extract frontmatter imports from BOTH base and child BEFORE the deep merge. // // Base imports resolve relative to the BASE file's directory (base_base_dir, derived @@ -890,7 +887,7 @@ impl ModuleCache { scope.set_function(name, Arc::clone(func)); } - Ok(scope) + Ok((scope, merged_mapping)) } /// Shared extends-pipeline: steps 3a-3e are identical for the text-cached and @@ -951,8 +948,11 @@ impl ModuleCache { // ── Step 3d: build merged scope (Phase 3: deep merge + per-file FM imports) ── // Applies decision #3 (base < child < runtime) and decision #7 (reserved-key // exclusion, array wholesale replace, both sets of FM imports resolved per-file). - let mut scope = + // The merged_mapping is also returned so we can serialize it for the output + // frontmatter (#154: @extends emits deep-merged frontmatter). + let (mut scope, merged_mapping) = self.build_merged_extends_scope(&base, frontmatter_values, &base_key, ctx, warnings)?; + let merged_frontmatter = serialize_merged_frontmatter(&merged_mapping)?; // Collect child's own definitions from its body (currently zero @define after // child-only-blocks check, but structurally correct). @@ -986,6 +986,7 @@ impl ModuleCache { skeleton_origin, has_explicit_exports, explicit_exports, + merged_frontmatter, }) } @@ -1027,7 +1028,6 @@ impl ModuleCache { module: crate::ast::Module, ext: crate::ast::ExtendsDirective, ctx: &ModuleCtx<'_>, - raw_frontmatter: Option, frontmatter_values: Option, warnings: &mut Vec, ) -> Result { @@ -1053,6 +1053,7 @@ impl ModuleCache { skeleton_origin, has_explicit_exports, explicit_exports, + merged_frontmatter, } = components; let prompt_body = evaluate(&final_body, &mut scope, warnings)?; @@ -1061,7 +1062,9 @@ impl ModuleCache { Ok(ResolvedModule { functions, prompt_body, - raw_frontmatter, + // #154: emit deep-merged frontmatter (base < child, reserved keys excluded) + // rather than the child's raw frontmatter. + raw_frontmatter: merged_frontmatter, has_explicit_exports, explicit_exports, effective_skeleton, @@ -1654,6 +1657,12 @@ struct ExtendsComponents { has_explicit_exports: bool, /// Named exports from the child. explicit_exports: HashSet, + /// Deep-merged frontmatter YAML string (base < child, reserved keys excluded). + /// + /// Replaces the child's raw frontmatter in the compiled output so that `@extends` + /// children emit the full merged key set rather than only their own keys (#154). + /// `None` when the merged mapping is empty (no non-reserved FM keys on either side). + merged_frontmatter: Option, } /// Bundle of borrowed per-module context threaded through the AST walk helpers. @@ -2051,6 +2060,38 @@ fn attach_frontmatter_index(err: MdsError, i: usize) -> MdsError { // ── Template inheritance helpers ────────────────────────────────────────────── +/// Serialize a deep-merged frontmatter `Mapping` into a YAML string suitable for +/// `prepend_frontmatter`. +/// +/// Returns `Ok(None)` when `mapping` is empty (no non-reserved keys on either side — no +/// frontmatter block should be emitted). Otherwise returns `Ok(Some(yaml_string))` where +/// `yaml_string` is the canonical YAML representation of `mapping` with a trailing +/// newline, ready to be wrapped in `---…---` fences by `prepend_frontmatter`. +/// +/// Used by `resolve_extends_components` for the `@extends` deep-merged FM output (#154). +/// +/// # Errors +/// +/// Returns `Err(MdsError::YamlError)` if YAML serialization of the merged mapping fails. +/// This is not expected for well-formed mappings produced by `deep_merge_yaml`, but the +/// failure MUST propagate as a compile error rather than silently dropping the emitted +/// frontmatter fence from the output (no swallowed errors at boundaries). +fn serialize_merged_frontmatter( + mapping: &serde_yaml_ng::Mapping, +) -> Result, MdsError> { + if mapping.is_empty() { + return Ok(None); + } + let yaml = serde_yaml_ng::to_string(&serde_yaml_ng::Value::Mapping(mapping.clone())) + .map_err(|e| MdsError::yaml_error(e.to_string()))?; + let trimmed = yaml.trim_end(); + if trimmed.is_empty() { + Ok(None) + } else { + Ok(Some(format!("{trimmed}\n"))) + } +} + /// Parse the frontmatter YAML into a `serde_yaml_ng::Mapping` for storage. /// /// Returns `None` when there is no frontmatter or when the YAML is not a mapping. diff --git a/crates/mds-core/src/resolver/frontmatter.rs b/crates/mds-core/src/resolver/frontmatter.rs index e505aa0..9528047 100644 --- a/crates/mds-core/src/resolver/frontmatter.rs +++ b/crates/mds-core/src/resolver/frontmatter.rs @@ -61,7 +61,14 @@ pub(super) fn deep_merge_yaml( ))); } - // Reserved keys excluded from the merged output (must not propagate as FM variables). + // Reserved keys excluded from the merged output at the TOP LEVEL only (depth == 0). + // These keys are directives, not value data, so they are stripped from the emitted + // frontmatter to prevent them from appearing in the compiled output. + // + // !! IMPORTANT: the guard is `depth == 0` — do NOT filter reserved names in + // recursive calls (depth > 0). A nested key like `config.type` is not a + // top-level directive; filtering it would silently discard user data. + // // SYNC POINT: if you add a key here, audit `strip_reserved_keys` in lib.rs — // that function strips `type` and `imports` from raw frontmatter output but intentionally // omits `extends` (it is a directive token, not an output FM key). @@ -74,11 +81,12 @@ pub(super) fn deep_merge_yaml( // Phase 1: walk base keys in order. // Each base key keeps its position; value is replaced if child also has that key. for (base_key, base_val) in base { - // Skip reserved keys and non-string keys. + // Skip non-string keys. let serde_yaml_ng::Value::String(key_str) = base_key else { continue; }; - if RESERVED.contains(&key_str.as_str()) { + // Reserved keys are stripped at the top-level only (they are directives, not data). + if depth == 0 && RESERVED.contains(&key_str.as_str()) { continue; } @@ -105,7 +113,8 @@ pub(super) fn deep_merge_yaml( let serde_yaml_ng::Value::String(key_str) = child_key else { continue; }; - if RESERVED.contains(&key_str.as_str()) { + // Reserved keys are stripped at the top-level only. + if depth == 0 && RESERVED.contains(&key_str.as_str()) { continue; } // Skip keys already added from base. @@ -295,3 +304,109 @@ pub(crate) fn parse_frontmatter_imports(raw: &str) -> Result serde_yaml_ng::Mapping { + let val: serde_yaml_ng::Value = serde_yaml_ng::from_str(yaml).unwrap(); + match val { + serde_yaml_ng::Value::Mapping(m) => m, + _ => panic!("expected a YAML mapping"), + } + } + + fn mapping_get_str<'a>(m: &'a serde_yaml_ng::Mapping, key: &str) -> Option<&'a str> { + m.get(serde_yaml_ng::Value::String(key.to_owned())) + .and_then(|v| v.as_str()) + } + + /// Top-level reserved keys (`imports`, `type`, `extends`) must be excluded from + /// the merged output — they are directive tokens, not value data. + #[test] + fn deep_merge_yaml_strips_top_level_reserved_keys() { + let base = mapping_from_str("type: mds\nmodel: gpt-4\nimports:\n - path: ./lib.mds\n"); + let child = mapping_from_str("extends: ./base.mds\nrole: system\n"); + let result = deep_merge_yaml(&base, &child, 0).unwrap(); + assert!( + !result.contains_key(serde_yaml_ng::Value::String("type".into())), + "top-level 'type' must be stripped" + ); + assert!( + !result.contains_key(serde_yaml_ng::Value::String("imports".into())), + "top-level 'imports' must be stripped" + ); + assert!( + !result.contains_key(serde_yaml_ng::Value::String("extends".into())), + "top-level 'extends' must be stripped" + ); + assert_eq!(mapping_get_str(&result, "model"), Some("gpt-4")); + assert_eq!(mapping_get_str(&result, "role"), Some("system")); + } + + /// Nested keys named `type`, `imports`, or `extends` inside a sub-mapping must + /// NOT be filtered — the reserved-key guard applies at the top level only (depth == 0). + /// + /// Example: `config.type: "mds"` is a user-defined nested key, not a directive. + /// Filtering it would silently corrupt user data. + #[test] + fn deep_merge_yaml_nested_reserved_keys_preserved() { + let base = mapping_from_str( + "config:\n type: mds\n model: gpt-4\n imports: strict\nmodel: base-model\n", + ); + let child = mapping_from_str("config:\n type: custom\n extra: added\nrole: system\n"); + let result = deep_merge_yaml(&base, &child, 0).unwrap(); + + // `config` key must be present with the merged sub-mapping. + let config = result + .get(serde_yaml_ng::Value::String("config".into())) + .and_then(|v| v.as_mapping()) + .expect("config sub-mapping must survive merge"); + + // config.type: child value wins ("custom" overrides "mds"). + assert_eq!( + config + .get(serde_yaml_ng::Value::String("type".into())) + .and_then(|v| v.as_str()), + Some("custom"), + "config.type (nested 'type') must NOT be stripped and child value must win" + ); + // config.imports: base-only nested key, must survive. + assert_eq!( + config + .get(serde_yaml_ng::Value::String("imports".into())) + .and_then(|v| v.as_str()), + Some("strict"), + "config.imports (nested 'imports') must NOT be stripped" + ); + // config.model: base-only nested key. + assert_eq!( + config + .get(serde_yaml_ng::Value::String("model".into())) + .and_then(|v| v.as_str()), + Some("gpt-4"), + "base-only config.model must survive" + ); + // config.extra: child-only nested key. + assert_eq!( + config + .get(serde_yaml_ng::Value::String("extra".into())) + .and_then(|v| v.as_str()), + Some("added"), + "child-only config.extra must be present" + ); + // Top-level keys are correct. + assert_eq!(mapping_get_str(&result, "model"), Some("base-model")); + assert_eq!(mapping_get_str(&result, "role"), Some("system")); + } + + /// Merging two empty mappings must produce an empty result (not an error). + #[test] + fn deep_merge_yaml_both_empty() { + let base = serde_yaml_ng::Mapping::new(); + let child = serde_yaml_ng::Mapping::new(); + let result = deep_merge_yaml(&base, &child, 0).unwrap(); + assert!(result.is_empty()); + } +} diff --git a/crates/mds-core/src/resolver_tests.rs b/crates/mds-core/src/resolver_tests.rs index 0c2f93c..30c4f8d 100644 --- a/crates/mds-core/src/resolver_tests.rs +++ b/crates/mds-core/src/resolver_tests.rs @@ -1456,6 +1456,52 @@ fn f6_base_only_key_visible_in_child() { ); } +#[test] +fn f6_nested_reserved_key_config_type_survives_merge() { + // F6 / depth-gating fix: a nested key named `type` inside a sub-mapping (e.g. + // `config.type: mds`) must survive the deep merge and appear in the compiled output. + // Previously deep_merge_yaml filtered all keys named "type" at every recursion depth, + // silently dropping user data inside nested objects. + let base = concat!( + "---\n", + "config:\n", + " type: mds\n", + " model: gpt-4\n", + "model: base-model\n", + "---\n", + "@block content:\n", + "{config.model}\n", + "@end\n", + ); + let child = concat!( + "---\n", + "role: system\n", + "---\n", + "@extends \"./base.mds\"\n", + ); + let files = [("base.mds", base), ("child.mds", child)]; + let mut cache = virtual_cache(&files); + let mut warnings = vec![]; + let resolved = cache + .resolve_key("child.mds", &Default::default(), &mut warnings) + .expect("F6 nested reserved key: should compile"); + + // Compiled output (body) uses config.model from merged scope. + let body = resolved.prompt_body.as_deref().unwrap_or(""); + assert!( + body.contains("gpt-4"), + "config.model must be in scope: {body}" + ); + + // Emitted frontmatter must contain `config.type: mds` — the nested `type` key + // must NOT be stripped by the reserved-key filter. + let fm = resolved.raw_frontmatter.as_deref().unwrap_or(""); + assert!( + fm.contains("type: mds") || fm.contains("type: 'mds'") || fm.contains("\"type\": mds"), + "config.type nested key must survive into emitted frontmatter; got: {fm}" + ); +} + #[test] fn f7_runtime_override_precedence() { // F7: runtime --set overrides merged frontmatter (base < child < runtime). @@ -1727,20 +1773,23 @@ fn regression_non_extending_file_fm_unchanged() { } #[test] -fn f4_child_emits_only_own_raw_frontmatter() { - // decision #7 / output emission: extending child emits only its own raw_frontmatter. - // Base frontmatter is an input to scope, not output. +fn f4_extends_emits_deep_merged_frontmatter() { + // #154 — @extends must emit the deep-merged frontmatter (base < child), + // not just the child's raw FM. Reserved keys (imports, type, extends) are + // excluded; non-reserved keys from both sides appear in the output. let base = concat!( "---\n", - "base_secret: only_in_base\n", + "base_key: from_base\n", + "shared_key: base_wins_without_child_override\n", "---\n", "@block content:\n", - "{base_secret}\n", + "{base_key}\n", "@end\n", ); let child = concat!( "---\n", - "child_var: in_child\n", + "child_key: from_child\n", + "shared_key: child_overrides_base\n", "---\n", "@extends \"./base.mds\"\n", ); @@ -1752,21 +1801,32 @@ fn f4_child_emits_only_own_raw_frontmatter() { .resolve_key("child.mds", &Default::default(), &mut warnings) .expect("output emission test should compile"); - // raw_frontmatter in the resolved module is the child's raw FM (not base's). - if let Some(ref raw_fm) = child_resolved.raw_frontmatter { - assert!( - !raw_fm.contains("base_secret"), - "child output must NOT contain base frontmatter: {raw_fm}" - ); - assert!( - raw_fm.contains("child_var"), - "child output must contain child's own frontmatter: {raw_fm}" - ); - } - // The compiled output uses the merged scope (base_secret visible to blocks) + // raw_frontmatter in the resolved module is the deep-merged FM (base < child). + let raw_fm = child_resolved + .raw_frontmatter + .as_deref() + .expect("merged output must have frontmatter"); + assert!( + raw_fm.contains("base_key"), + "merged output must contain base-only key; got: {raw_fm}" + ); + assert!( + raw_fm.contains("child_key"), + "merged output must contain child-only key; got: {raw_fm}" + ); + assert!( + raw_fm.contains("child_overrides_base"), + "child value must win for shared_key; got: {raw_fm}" + ); + assert!( + !raw_fm.contains("base_wins_without_child_override"), + "base value for shared_key must be overridden; got: {raw_fm}" + ); + + // The compiled body still uses the merged scope (base_key visible to blocks). let output = child_resolved.prompt_body.as_deref().unwrap_or(""); assert!( - output.contains("only_in_base"), + output.contains("from_base"), "merged scope used: base var rendered in block: {output}" ); } @@ -2271,74 +2331,67 @@ fn pf004_messages_mode_extends_validates_final_body_parity() { #[test] fn f11_whitespace_contract_4_combination_matrix() { // Base skeleton: text before, one @block, text after. - // Between the Text("Intro.\n\n") node and the @block body there is NO - // extra whitespace beyond what the skeleton text nodes carry. + // Interior-verbatim contract (#150/#151): block body bytes pass through + // exactly as authored; only the directive lines (@block … :, @end) are + // consumed. The trailing \n before @end is now part of the body. // // Base source (repr): "Intro.\n\n@block body:\nDefault body.\n@end\n\nAfter.\n" // // Skeleton nodes after parse: // Text("Intro.\n\n") - // Block("body") body = [Text("Default body.")] ← edge \n stripped - // Text("\nAfter.\n") ← the blank line + After. + // Block("body") body = [Text("Default body.\n")] ← verbatim, no edge strip + // Text("\nAfter.\n") ← the blank line + After. let base = "Intro.\n\n@block body:\nDefault body.\n@end\n\nAfter.\n"; // ── Combination 1: base default, no child override ──────────────────── // Child has no @block override — effective_blocks use the base default. - // Between-block blank line (\n before "After.") preserved verbatim. + // Body text "Default body.\n" + skeleton "\nAfter.\n" → blank line preserved. { let child = "@extends \"./base.mds\"\n"; let files = [("base.mds", base), ("child.mds", child)]; let out = compile_virtual(&files, "child.mds").expect("F11 combo-1: should compile"); assert_eq!( - out, "Intro.\n\nDefault body.\nAfter.\n", - "F11 combo-1: base default — between-block blank line preserved, body edge stripped" + out, "Intro.\n\nDefault body.\n\nAfter.\n", + "F11 combo-1: base default — body verbatim, trailing \\n + skeleton \\n = blank line" ); } // ── Combination 2: override with no surrounding blank lines ─────────── - // Block body = "Override." (no leading/trailing blank lines). - // After edge-strip: body = [Text("Override.")]. + // Block body bytes (verbatim): "Override.\n" + // Body text "Override.\n" + skeleton "\nAfter.\n" → blank line. { let child = "@extends \"./base.mds\"\n@block body:\nOverride.\n@end\n"; let files = [("base.mds", base), ("child.mds", child)]; let out = compile_virtual(&files, "child.mds").expect("F11 combo-2: should compile"); assert_eq!( - out, "Intro.\n\nOverride.\nAfter.\n", - "F11 combo-2: override without blank lines — clean output" + out, "Intro.\n\nOverride.\n\nAfter.\n", + "F11 combo-2: override without blank lines — trailing \\n + skeleton \\n = blank line" ); } // ── Combination 3: override WITH leading+trailing blank lines ───────── - // Block body raw = "\nOverride.\n\n" (blank line before + blank line after). - // strip_leading_newline removes ONE leading \n → "Override.\n\n" - // strip_trailing_newline removes ONE trailing \n → "Override.\n" - // Residual \n becomes part of the rendered block body, producing an extra - // blank line BEFORE the "After." skeleton text node ("\nAfter.\n"). - // This pins decision #9: only one edge \n is stripped — extra interior - // blank lines are preserved. + // Block body bytes (verbatim): "\nOverride.\n\n" + // Full verbatim run: "Intro.\n\n" + "\nOverride.\n\n" + "\nAfter.\n" { let child = "@extends \"./base.mds\"\n@block body:\n\nOverride.\n\n@end\n"; let files = [("base.mds", base), ("child.mds", child)]; let out = compile_virtual(&files, "child.mds").expect("F11 combo-3: should compile"); assert_eq!( - out, - "Intro.\n\nOverride.\n\nAfter.\n", - "F11 combo-3: override with surrounding blanks — extra blank line inside body preserved (only edge \n stripped)" - ); + out, "Intro.\n\n\nOverride.\n\n\nAfter.\n", + "F11 combo-3: override with surrounding blanks — full verbatim run preserved" + ); } // ── Combination 4: override with indented content ───────────────────── - // Block body raw = " Indented.\n". - // strip_leading_newline: no leading \n, no change. - // strip_trailing_newline: pop \n → " Indented." - // Leading spaces are preserved verbatim (base author's indentation style). + // Block body bytes (verbatim): " Indented.\n" + // Body " Indented.\n" + skeleton "\nAfter.\n" → blank line between. { let child = "@extends \"./base.mds\"\n@block body:\n Indented.\n@end\n"; let files = [("base.mds", base), ("child.mds", child)]; let out = compile_virtual(&files, "child.mds").expect("F11 combo-4: should compile"); assert_eq!( - out, "Intro.\n\n Indented.\nAfter.\n", - "F11 combo-4: indented override — leading spaces preserved verbatim" + out, "Intro.\n\n Indented.\n\nAfter.\n", + "F11 combo-4: indented override — verbatim, trailing \\n + skeleton \\n = blank line" ); } } @@ -2895,6 +2948,186 @@ fn source_label_appears_in_string_source_diagnostics() { ); } +// ── Adversarial YAML-injection: core-level twin (#154) ─────────────────────── +// +// These tests exercise the resolver's `raw_frontmatter` serialization path +// (serialize_merged_frontmatter / serde_yaml_ng::to_string) to verify that values +// containing fence-like sequences, YAML document-end markers, or block-scalar +// `type: mds` lines cannot escape the emitted frontmatter block and smuggle +// additional top-level keys into the compiled output. +// +// The CLI-level mirror lives in crates/mds-cli/tests/frontmatter.rs. + +#[test] +fn adversarial_injection_fence_sequence_in_fm_value() { + // A frontmatter value containing a literal `\n---\n` YAML document separator + // must be serialized as a quoted/escaped YAML string and must NOT break the + // outer `---`/`---` fence structure. + let base = concat!( + "---\n", + "prompt: \"start\\n---\\ninjected: evil\\n---\\nend\"\n", + "---\n", + "@block body:\ndefault\n@end\n", + ); + let child = concat!("---\nrole: system\n---\n", "@extends \"./base.mds\"\n"); + let files = [("base.mds", base), ("child.mds", child)]; + let mut cache = virtual_cache(&files); + let resolved = cache + .resolve_key("child.mds", &Default::default(), &mut vec![]) + .expect("should compile despite adversarial FM value"); + + let raw_fm = resolved + .raw_frontmatter + .as_deref() + .expect("merged output must have raw_frontmatter"); + + // Extract ONLY the frontmatter section (between the first --- and second ---). + // Using the full raw_frontmatter string directly since it's already the inner YAML. + let has_injected = raw_fm.lines().any(|l| l == "injected: evil"); + assert!( + !has_injected, + "adversarial 'injected: evil' must not appear as a top-level FM key; raw_fm={raw_fm}" + ); + assert!( + raw_fm.contains("role: system"), + "role key must survive; raw_fm={raw_fm}" + ); + assert!( + raw_fm.contains("prompt:"), + "prompt key must survive; raw_fm={raw_fm}" + ); +} + +#[test] +fn adversarial_injection_yaml_end_marker_in_fm_value() { + // A frontmatter value containing `\n...\n` (YAML stream-end marker) must + // be serialized safely and must not close the document prematurely. + let base = concat!( + "---\n", + "note: \"before\\n...\\nafter\"\n", + "---\n", + "@block body:\ndefault\n@end\n", + ); + let child = concat!("---\nrole: user\n---\n", "@extends \"./base.mds\"\n"); + let files = [("base.mds", base), ("child.mds", child)]; + let mut cache = virtual_cache(&files); + let resolved = cache + .resolve_key("child.mds", &Default::default(), &mut vec![]) + .expect("should compile despite YAML end-marker in value"); + + let raw_fm = resolved + .raw_frontmatter + .as_deref() + .expect("merged output must have raw_frontmatter"); + + // The end-marker must not split the frontmatter YAML; both keys must appear. + assert!( + raw_fm.contains("role: user"), + "role key must be in raw_fm; got: {raw_fm}" + ); + assert!( + raw_fm.contains("note:"), + "note key must be in raw_fm; got: {raw_fm}" + ); +} + +#[test] +fn adversarial_injection_block_scalar_type_mds_not_stripped() { + // A block-scalar FM value whose lines include `type: mds` must not be + // confused with the top-level `type: mds` directive — only an unindented + // top-level `type: mds` key should be stripped from the emitted output. + let base = concat!( + "---\n", + "description: |\n", + " type: mds\n", + " this is a block scalar\n", + "model: gpt-4\n", + "---\n", + "@block body:\ndefault\n@end\n", + ); + let child = "@extends \"./base.mds\"\n"; + let files = [("base.mds", base), ("child.mds", child)]; + let mut cache = virtual_cache(&files); + let resolved = cache + .resolve_key("child.mds", &Default::default(), &mut vec![]) + .expect("should compile"); + + let raw_fm = resolved + .raw_frontmatter + .as_deref() + .expect("merged output must have raw_frontmatter"); + + // No unindented `type: mds` key in the raw frontmatter. + let has_top_level_type = raw_fm.lines().any(|l| l == "type: mds"); + assert!( + !has_top_level_type, + "unindented 'type: mds' must not appear as a top-level key; raw_fm={raw_fm}" + ); + assert!( + raw_fm.contains("description:"), + "description key must be present; raw_fm={raw_fm}" + ); + assert!( + raw_fm.contains("model:"), + "model key must be present; raw_fm={raw_fm}" + ); +} + +// ── f7 extension: emitted FM ignores --set (#154) ──────────────────────────── + +#[test] +fn f7_emitted_frontmatter_ignores_runtime_set() { + // Extending an extends test to pass a runtime var whose key collides with a + // frontmatter key: the emitted raw_frontmatter must NOT contain the runtime + // value. Runtime vars override the SCOPE (what the body renders) but must + // never alter the emitted raw frontmatter serialization. + let base = concat!( + "---\n", + "model: gpt-4\n", + "---\n", + "@block content:\n", + "Model: {model}\n", + "@end\n", + ); + let child = concat!( + "---\n", + "role: assistant\n", + "---\n", + "@extends \"./base.mds\"\n", + ); + let files = [("base.mds", base), ("child.mds", child)]; + let mut runtime_vars = std::collections::HashMap::new(); + runtime_vars.insert( + "model".to_string(), + Value::String("gpt-3.5-turbo".to_string()), + ); + let mut cache = virtual_cache(&files); + let resolved = cache + .resolve_key("child.mds", &runtime_vars, &mut vec![]) + .expect("F7 extension: should compile with runtime override"); + + // Body renders the runtime value (scope wins). + let body = resolved.prompt_body.as_deref().unwrap_or(""); + assert!( + body.contains("gpt-3.5-turbo"), + "body must use runtime var value; body={body}" + ); + + // Emitted raw_frontmatter must show the MERGED FRONTMATTER value, not the runtime one. + let raw_fm = resolved + .raw_frontmatter + .as_deref() + .expect("merged output must have raw_frontmatter"); + assert!( + !raw_fm.contains("gpt-3.5-turbo"), + "runtime value must NOT appear in emitted frontmatter; raw_fm={raw_fm}" + ); + assert!( + raw_fm.contains("gpt-4"), + "frontmatter must show merged FM value (gpt-4); raw_fm={raw_fm}" + ); +} + // ── PF-003 / #146: parent_dir cross-platform integration test ───────────────── // // Verifies that `FileSystem::parent_dir` is correctly called and consumed during diff --git a/crates/mds-core/tests/fmt.rs b/crates/mds-core/tests/fmt.rs index fa36d9c..df105d1 100644 --- a/crates/mds-core/tests/fmt.rs +++ b/crates/mds-core/tests/fmt.rs @@ -152,23 +152,24 @@ fn r2_frontmatter_only_no_trailing_newline_still_gets_one() { ); } -// ── R3: 3+ blank lines collapse to one, except inside fences ──────────────── +// ── Interior-verbatim: blank lines preserved (R3 removed in v2) ────────────── #[test] -fn r3_blank_line_run_collapses_matching_clean_output() { - // Verified empirically: clean_output("Hello\n\n\n\nWorld\n") == "Hello\n\nWorld\n". +fn interior_blank_line_runs_preserved_verbatim() { + // Interior-verbatim contract (#150/#151): blank-line runs pass through + // byte-exact. clean_output no longer caps runs; the formatter must not either. let out = format_str("Hello\n\n\n\nWorld\n").unwrap(); - assert_eq!(out, "Hello\n\nWorld\n"); + assert_eq!(out, "Hello\n\n\n\nWorld\n"); } #[test] -fn r3_single_blank_line_unchanged() { +fn interior_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() { +fn code_fence_blank_lines_preserved_verbatim() { let src = "```\ncode\n\n\n\nmore\n```\nAfter\n"; let out = format_str(src).expect("should format"); assert!( @@ -178,15 +179,14 @@ fn r3_never_collapses_blank_lines_inside_code_fence() { } #[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. +fn leading_blank_lines_in_body_preserved_verbatim() { + // Interior-verbatim (#150/#151): leading blank lines are no longer stripped + // — both with and without frontmatter present. let out = format_str("\n\n\nHello\nWorld\n").unwrap(); - assert_eq!(out, "Hello\nWorld\n"); + assert_eq!(out, "\n\n\nHello\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"); + assert_eq!(out_fm, "---\nname: x\n---\n\n\n\nHello\n"); } // ── R4: trailing whitespace on directive lines; body content UNCHANGED ────── @@ -353,7 +353,7 @@ fn gate_fallback_undefined_var_source_still_formats() { "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"); + assert_eq!(out, "Hello {undefined_var}!\n\n\n\nBye.\n"); } #[test] @@ -365,7 +365,10 @@ fn gate_full_check_runs_when_base_dir_resolves_imports() { 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"); + assert_eq!( + out, + "@import \"./lib.mds\"\n{greet(\"World\")}\n\n\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) @@ -465,53 +468,40 @@ fn ec_mixed_crlf_lf_all_become_lf() { } #[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). +fn ec_blank_lines_inside_define_message_block_bodies_preserved() { + // Interior-verbatim contract (#150/#151): blank-line runs are left verbatim + // in ALL body kinds — @message, @define, and @block alike. The formatter + // no longer applies R3 collapsing anywhere; compile equivalence via the + // safety gate is the load-bearing check in every case below. + + // @define body: blank-line runs pass through verbatim. 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:?}" + "blank-line run inside @define body must be preserved, 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). + // @message body: blank-line runs pass through verbatim (not even R1). 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:?}" + "blank-line run inside @message body must be preserved, 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). + // @block body: interior-verbatim applies here too. 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:?}" + block_out.contains("Step one\n\n\n\nStep two"), + "blank-line run inside @block body must be preserved, got: {block_out:?}" ); let block_before = mds::compile_str(block_src).unwrap(); let block_after = mds::compile_str(&block_out).unwrap(); @@ -624,10 +614,10 @@ fn perf_formats_large_file_without_panic_or_catastrophic_slowdown() { elapsed2.as_secs() < 30, "formatting via structural_equivalent fallback took implausibly long: {elapsed2:?}" ); - // Blank-line runs must be collapsed even in the fallback path. + // Interior blank-line runs are now preserved (interior-verbatim, #150/#151). assert!( - !fallback_out.contains("\n\n\n"), - "blank-line runs (3+) must be collapsed even via the structural fallback" + fallback_out.contains("\n\n\n"), + "interior blank-line runs must be preserved verbatim by the structural fallback" ); } diff --git a/crates/mds-core/tests/virtual_fs.rs b/crates/mds-core/tests/virtual_fs.rs index 214f59b..ac27b99 100644 --- a/crates/mds-core/tests/virtual_fs.rs +++ b/crates/mds-core/tests/virtual_fs.rs @@ -1005,3 +1005,377 @@ fn fm_import_collision_merge_within_fm() { "expected collision error for 'common', got: {msg}" ); } + +// ── Issue repros: #150 / #151 (interior-verbatim whitespace contract) ───────── +// +// These tests lock in the compile-output end-to-end, verifying that the +// interior-verbatim contract (blank-line runs preserved, leading/trailing +// newlines no longer stripped from block/define bodies) is observable at the +// compiled markdown level and not just in parser AST tests. + +#[test] +fn issue_150_interior_blank_line_runs_preserved_in_compiled_output() { + // Issue #150: multiple consecutive blank lines inside a body were being + // collapsed to one by clean_output's old newline-run cap. With the + // interior-verbatim contract, they must survive to compiled output. + let src = "Intro.\n\n\n\nBody.\n"; + let out = mds::compile_str(src) + .expect("#150 repro: should compile") + .into_markdown() + .expect("#150 repro: expected markdown output"); + assert_eq!( + out, "Intro.\n\n\n\nBody.\n", + "#150: interior blank-line run must be preserved verbatim" + ); +} + +#[test] +fn issue_151_block_body_edge_newline_preserved_at_compile_boundary() { + // Issue #151: @block bodies had their leading/trailing newlines stripped + // at parse time (strip_leading_newline / strip_trailing_newline). With + // the interior-verbatim contract, the trailing \n before @end is part of + // the body and must appear in compiled output, producing a blank line + // between the block body and the following skeleton text. + let mut modules = HashMap::new(); + modules.insert( + "base.mds".to_string(), + // "Para A\n" + skeleton "\n" + "Para B\n" was "Para A\nPara B\n" (old). + // Now "Para A\n" (body verbatim) + "\n" (skeleton) = "Para A\n\nPara B\n". + "@block section:\nPara A.\n@end\n\nPara B.\n".to_string(), + ); + modules.insert( + "child.mds".to_string(), + "@extends \"./base.mds\"\n".to_string(), + ); + let out = mds::compile_virtual( + modules.into_iter().collect::>(), + "child.mds", + None, + ) + .expect("#151 repro: should compile") + .into_markdown() + .expect("#151 repro: expected markdown output"); + assert_eq!( + out, "Para A.\n\nPara B.\n", + "#151: trailing \\n of block body + skeleton \\n must produce a blank line" + ); +} + +// ── Issue repros: #154 (@extends emits deep-merged frontmatter) ─────────────── +// +// Previously, a child template using @extends only emitted its own raw +// frontmatter in the compiled output, discarding base frontmatter keys that +// the child did not explicitly repeat. The fix serializes the deep-merged +// mapping (base < child, reserved keys excluded) for the output frontmatter. + +#[test] +fn issue_154_extends_emits_merged_frontmatter() { + // Base has `model` and `temperature`; child overrides `temperature` and adds `role`. + // The compiled output must contain all three non-reserved keys: model (from base), + // temperature (child overrides base), and role (child-only). + let mut modules = HashMap::new(); + modules.insert( + "base.mds".to_string(), + "---\nmodel: gpt-4\ntemperature: 0.7\n---\n@block section:\nbase text\n@end\n".to_string(), + ); + modules.insert( + "child.mds".to_string(), + "---\ntemperature: 0.2\nrole: system\n---\n@extends \"./base.mds\"\n".to_string(), + ); + let out = + compile_vfs(modules.into_iter().collect(), "child.mds").expect("#154: should compile"); + assert!( + out.contains("model: gpt-4"), + "#154: base-only key 'model' must appear in merged output; got: {out}" + ); + assert!( + out.contains("temperature: 0.2"), + "#154: child value must win for 'temperature'; got: {out}" + ); + assert!( + out.contains("role: system"), + "#154: child-only key 'role' must appear in merged output; got: {out}" + ); +} + +#[test] +fn issue_154_extends_no_base_frontmatter() { + // Base has no frontmatter; child has frontmatter → only child keys emitted. + let mut modules = HashMap::new(); + modules.insert( + "base.mds".to_string(), + "@block section:\nbase text\n@end\n".to_string(), + ); + modules.insert( + "child.mds".to_string(), + "---\nrole: user\n---\n@extends \"./base.mds\"\n".to_string(), + ); + let out = compile_vfs(modules.into_iter().collect(), "child.mds") + .expect("#154: no-base-fm case should compile"); + assert!( + out.contains("role: user"), + "#154: child-only key 'role' must appear when base has no frontmatter; got: {out}" + ); +} + +#[test] +fn issue_154_extends_reserved_keys_excluded_from_output() { + // `extends`, `imports`, and `type` are reserved — they must not appear in the + // compiled output frontmatter even if they appear in the source. + let mut modules = HashMap::new(); + modules.insert( + "base.mds".to_string(), + "---\nmodel: gpt-4\ntype: mds\n---\n@block section:\nbase\n@end\n".to_string(), + ); + modules.insert( + "child.mds".to_string(), + "---\nrole: system\n---\n@extends \"./base.mds\"\n".to_string(), + ); + let out = compile_vfs(modules.into_iter().collect(), "child.mds") + .expect("#154: reserved-key exclusion case should compile"); + assert!( + !out.contains("type:"), + "#154: reserved key 'type' must NOT appear in merged output; got: {out}" + ); + assert!( + !out.contains("extends:"), + "#154: reserved key 'extends' must NOT appear in merged output; got: {out}" + ); + assert!( + out.contains("model: gpt-4"), + "#154: non-reserved key 'model' must still appear; got: {out}" + ); + assert!( + out.contains("role: system"), + "#154: non-reserved key 'role' must still appear; got: {out}" + ); +} + +#[test] +fn issue_154_extends_no_frontmatter_on_either_side() { + // Neither base nor child has frontmatter → no frontmatter block in output. + let mut modules = HashMap::new(); + modules.insert( + "base.mds".to_string(), + "@block section:\nbase text\n@end\n".to_string(), + ); + modules.insert( + "child.mds".to_string(), + "@extends \"./base.mds\"\n".to_string(), + ); + let out = compile_vfs(modules.into_iter().collect(), "child.mds") + .expect("#154: no-fm case should compile"); + assert!( + !out.contains("---"), + "#154: output must not contain frontmatter fences when neither side has FM; got: {out}" + ); +} + +// ── Issue repros: #152 (cross-type comparison errors, --set-string) ─────────── +// +// Previously, comparing values of different types (e.g. number vs string) in +// an @if condition silently returned false (for ==) or true (for !=), leading +// to confusing logic bugs. The fix makes cross-type comparisons a runtime +// TypeMismatch error, forcing explicit type conversion before comparison. + +#[test] +fn issue_152_cross_type_eq_is_type_mismatch_error() { + // `@if x == "3":` with x=3 (number from frontmatter) — number vs string is a + // TypeMismatch error, not a silent false. + let src = "---\nx: 3\n---\n@if x == \"3\":\nyes\n@else:\nno\n@end\n"; + let err = mds::compile_str(src).expect_err("#152 repro: cross-type == must be a runtime error"); + let msg = format!("{err}"); + assert!( + msg.contains("type mismatch") || msg.contains("mds::type_mismatch"), + "#152: expected TypeMismatch error for number == string, got: {msg}" + ); +} + +#[test] +fn issue_152_type_mismatch_help_mentions_set_string() { + // The TypeMismatch help text must reference --set-string so users know how + // to pass a string value without type coercion. This pins the contract + // specified for the error's help field — QA black-box tested exactly this. + let src = "---\nx: 3\n---\n@if x == \"3\":\nyes\n@end\n"; + let err = mds::compile_str(src).expect_err("#152: cross-type == must error"); + let help = miette::Diagnostic::help(&err) + .map(|h| h.to_string()) + .unwrap_or_default(); + assert!( + help.contains("--set-string"), + "#152: TypeMismatch help must mention --set-string; got: {help:?}" + ); +} + +#[test] +fn issue_152_cross_type_neq_is_type_mismatch_error() { + // `@if x != "3":` with x=3 (number) — was silently returning true. Now an error. + let src = "---\nx: 3\n---\n@if x != \"3\":\ndiff\n@else:\nsame\n@end\n"; + let err = mds::compile_str(src).expect_err("#152 repro: cross-type != must be a runtime error"); + let msg = format!("{err}"); + assert!( + msg.contains("type mismatch") || msg.contains("mds::type_mismatch"), + "#152: expected TypeMismatch error for number != string, got: {msg}" + ); +} + +#[test] +fn issue_152_same_type_eq_still_works() { + // Same-type comparisons must continue to work correctly after the change. + let src = "---\nname: Alice\n---\n@if name == \"Alice\":\nhello Alice\n@end\n"; + let out = mds::compile_str(src) + .expect("#152: same-type string == should succeed") + .into_markdown() + .expect("#152: expected markdown"); + assert!(out.contains("hello Alice"), "#152: same-type eq must work"); +} + +#[test] +fn issue_152_set_string_forces_string_type() { + // When a variable is set via the --set-string equivalent (Value::String), comparing + // it with a string literal must succeed (same-type comparison). + let src = "---\n---\n@if count == \"3\":\nstring match\n@else:\nno match\n@end\n"; + let mut vars = std::collections::HashMap::new(); + vars.insert("count".to_string(), mds::Value::String("3".to_string())); + let out = mds::compile_str_with(src, None, Some(vars)) + .expect("#152: string count == \"3\" should succeed") + .into_markdown() + .expect("#152: expected markdown"); + assert!( + out.contains("string match"), + "#152: --set-string should make count a string so count == \"3\" is true" + ); +} + +// ── Round-trip acceptance test (#150/#151) ──────────────────────────────────── +// +// A realistic template with frontmatter, post-FM blank line, lists, an indented +// code fence inside a list item, interior blank runs, and a trailing-space line +// is compiled through the pass-through pipeline. The compiled output must be +// byte-identical to the source modulo the two carve-outs mandated by the +// interior-verbatim contract (#150/#151): +// 1. Trailing-edge normalization: any trailing whitespace is stripped, exactly +// one final newline is appended. +// 2. \r characters are stripped unconditionally. + +#[test] +fn round_trip_interior_verbatim_byte_identical() { + // Source deliberately contains: + // - Frontmatter with two keys + // - Blank line after the closing --- + // - A list with an indented code fence inside a list item + // - Literal braces inside the fence (must not be interpolated) + // - Interior blank runs (must be preserved verbatim) + // - A trailing-space "blank" line (preserved: body-text trailing whitespace + // is NOT stripped by `clean_output` — only the very trailing edge is) + let src = concat!( + "---\n", + "author: test\n", + "version: 1\n", + "---\n", + "\n", + "- First item\n", + "- Second item with fence:\n", + "\n", + " ```json\n", + " {\"key\": \"value\"}\n", + " ```\n", + "\n", + "- Third item\n", + "\n", + "Interior blank run above is preserved.\n", + " \n", + "Trailing-space line above; this is the last line.\n", + ); + + let out = mds::compile_str(src) + .expect("round-trip source must compile") + .into_markdown() + .expect("expected markdown"); + + // Apply the two permitted carve-outs to the source for the expected value. + // clean_output: strip \r, trim trailing whitespace, add one final \n. + let frontmatter_part = "---\nauthor: test\nversion: 1\n---\n"; + let body_part = concat!( + "\n", + "- First item\n", + "- Second item with fence:\n", + "\n", + " ```json\n", + " {\"key\": \"value\"}\n", + " ```\n", + "\n", + "- Third item\n", + "\n", + "Interior blank run above is preserved.\n", + " \n", + "Trailing-space line above; this is the last line.\n", + ); + // After clean_output: \r stripped (none here), trailing edge normalized to one \n. + // The body ends with "\n" already, so clean_output produces body_part as-is + // (trim_end removes the trailing \n-only sequence, then push_str adds one back). + let expected = format!("{frontmatter_part}{body_part}"); + + assert_eq!( + out, expected, + "compiled output must be byte-identical to source (modulo trailing-edge normalization)" + ); +} + +// ── Integration repro: #149 — indented fence with literal braces ────────────── + +#[test] +fn issue_149_indented_fence_inside_list_item_with_braces() { + // An indented code fence inside a list item containing literal `{braces}` + // must compile successfully: braces inside the fence are NOT interpolated, + // and the fence lines are emitted verbatim. + let src = concat!( + "Here is a list:\n", + "\n", + "- Step 1: use the API\n", + "\n", + " ```json\n", + " {\"action\": \"query\", \"filter\": {\"type\": \"all\"}}\n", + " ```\n", + "\n", + "- Step 2: done\n", + ); + let out = mds::compile_str(src) + .expect("#149: indented fence in list item with braces must compile") + .into_markdown() + .expect("#149: expected markdown output"); + + // The braces inside the fence must appear literally in the output. + assert!( + out.contains("{\"action\": \"query\""), + "#149: literal braces inside indented fence must be preserved; got: {out}" + ); + assert!( + out.contains("```json"), + "#149: fence opener must be verbatim; got: {out}" + ); + assert!( + out.contains("```\n"), + "#149: fence closer must be verbatim; got: {out}" + ); +} + +// ── Integration repro: #153 — invalid interpolation hint says \{ not \{{ ─────── + +#[test] +fn issue_153_invalid_interpolation_hint_text() { + // Compiling a file with an invalid interpolation shape must produce an error + // whose hint text contains `\{` (single brace) and NOT `\{{` (double brace). + let src = "{123invalid}\n"; + let err = mds::compile_str(src).expect_err("#153: invalid interpolation must fail to compile"); + let msg = format!("{err}"); + assert!( + msg.contains("\\{"), + "#153: error hint must mention \\{{ (single brace escape); got: {msg}" + ); + // Guard the regression: must NOT say \{{ (double brace — old broken form). + assert!( + !msg.contains("\\{{"), + "#153: error hint must NOT say \\{{{{ (double brace); got: {msg}" + ); +} diff --git a/crates/mds-python/tests/test_parity.py b/crates/mds-python/tests/test_parity.py index 96a1fc9..507fd42 100644 --- a/crates/mds-python/tests/test_parity.py +++ b/crates/mds-python/tests/test_parity.py @@ -43,7 +43,7 @@ {}, { "kind": "markdown", - "output": "---\nname: Alice\ncount: 3\n---\nHello Alice! You have 3 items.\n", + "output": "---\nname: Alice\ncount: 3\n---\n\nHello Alice! You have 3 items.\n", "warnings": [], "dependencies": [], }, diff --git a/examples/edge-cases/26_indented_and_tilde_fences.mds b/examples/edge-cases/26_indented_and_tilde_fences.mds new file mode 100644 index 0000000..d66f501 --- /dev/null +++ b/examples/edge-cases/26_indented_and_tilde_fences.mds @@ -0,0 +1,50 @@ +--- +language: Rust +--- + +## Code Fences in {language} — indented, tilde, and blockquoted variants + +The MDS compiler recognises all CommonMark-legal code-fence forms. +No interpolation occurs inside any of them. + +### 1. Standard backtick fence (baseline) + +```rust +let x = 42; +println!("{x}"); // literal {x} — no interpolation +``` + +### 2. Tilde fence (`~~~`) + +~~~rust +fn greet(name: &str) -> String { + format!("Hello, {name}!") // literal — not interpolated +} +~~~ + +After the tilde fence, interpolation resumes: {language} is great! + +### 3. Indented fence (4 spaces prefix) + +A list item whose code block is indented: + +- Item one +- Item two with a fenced code block: + + ```rust + // This is a code block indented inside a list item. + // {language} is NOT interpolated here. + let msg = format!("{}", 42); + ``` + +- Item three continues normally — {language} is back. + +### 4. Blockquoted fence (`> \`\`\``) + +> ```rust +> // Inside a blockquote fenced code block. +> // {language} is NOT interpolated here either. +> fn main() { println!("Hello"); } +> ``` + +Back in normal body: the language is {language}. diff --git a/package-lock.json b/package-lock.json index 74d8038..39d0d82 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14,7 +14,7 @@ }, "crates/mds-napi": { "name": "@mdscript/mds-napi", - "version": "0.2.0", + "version": "0.3.0", "license": "MIT", "devDependencies": { "@napi-rs/cli": "^3.0.0", @@ -3966,7 +3966,7 @@ }, "packages/bundler-utils": { "name": "@mdscript/bundler-utils", - "version": "0.2.0", + "version": "0.3.0", "license": "MIT", "devDependencies": { "@mdscript/mds": "file:../mds", @@ -3977,15 +3977,15 @@ "node": ">=22.0.0" }, "peerDependencies": { - "@mdscript/mds": "^0.2.0" + "@mdscript/mds": "^0.3.0" } }, "packages/mds": { "name": "@mdscript/mds", - "version": "0.2.0", + "version": "0.3.0", "license": "MIT", "dependencies": { - "@mdscript/mds-wasm": "^0.2.0" + "@mdscript/mds-wasm": "^0.3.0" }, "devDependencies": { "@types/node": "^25.9.1", @@ -3995,12 +3995,12 @@ "node": ">=22.0.0" }, "optionalDependencies": { - "@mdscript/mds-napi": "^0.2.0" + "@mdscript/mds-napi": "^0.3.0" } }, "packages/mds-wasm": { "name": "@mdscript/mds-wasm", - "version": "0.2.0", + "version": "0.3.0", "license": "MIT", "engines": { "node": ">=22.0.0" @@ -4008,10 +4008,10 @@ }, "packages/rollup-plugin": { "name": "@mdscript/rollup-plugin", - "version": "0.2.0", + "version": "0.3.0", "license": "MIT", "dependencies": { - "@mdscript/bundler-utils": "^0.2.0" + "@mdscript/bundler-utils": "^0.3.0" }, "devDependencies": { "@mdscript/mds": "file:../mds", @@ -4023,16 +4023,16 @@ "node": ">=22.0.0" }, "peerDependencies": { - "@mdscript/mds": "^0.2.0", + "@mdscript/mds": "^0.3.0", "rollup": "^3.0.0 || ^4.0.0" } }, "packages/rspack-loader": { "name": "@mdscript/rspack-loader", - "version": "0.2.0", + "version": "0.3.0", "license": "MIT", "dependencies": { - "@mdscript/bundler-utils": "^0.2.0" + "@mdscript/bundler-utils": "^0.3.0" }, "devDependencies": { "@mdscript/mds": "file:../mds", @@ -4044,7 +4044,7 @@ "node": ">=22.0.0" }, "peerDependencies": { - "@mdscript/mds": "^0.2.0", + "@mdscript/mds": "^0.3.0", "@rspack/core": "^1.0.0 || ^2.0.0" } }, @@ -4316,10 +4316,10 @@ }, "packages/vite-plugin": { "name": "@mdscript/vite-plugin", - "version": "0.2.0", + "version": "0.3.0", "license": "MIT", "dependencies": { - "@mdscript/bundler-utils": "^0.2.0" + "@mdscript/bundler-utils": "^0.3.0" }, "devDependencies": { "@mdscript/mds": "file:../mds", @@ -4331,16 +4331,16 @@ "node": ">=22.0.0" }, "peerDependencies": { - "@mdscript/mds": "^0.2.0", + "@mdscript/mds": "^0.3.0", "vite": "^5.0.0 || ^6.0.0 || ^7.0.0 || ^8.0.0" } }, "packages/webpack-loader": { "name": "@mdscript/webpack-loader", - "version": "0.2.0", + "version": "0.3.0", "license": "MIT", "dependencies": { - "@mdscript/bundler-utils": "^0.2.0" + "@mdscript/bundler-utils": "^0.3.0" }, "devDependencies": { "@mdscript/mds": "file:../mds", @@ -4352,7 +4352,7 @@ "node": ">=22.0.0" }, "peerDependencies": { - "@mdscript/mds": "^0.2.0", + "@mdscript/mds": "^0.3.0", "webpack": "^5.0.0" } } diff --git a/spec.md b/spec.md index cfc022d..ceed98d 100644 --- a/spec.md +++ b/spec.md @@ -1,4 +1,4 @@ -# MDS Language Specification (v0.2) +# MDS Language Specification (v0.4) ## 1. Overview @@ -66,9 +66,13 @@ Hello {name}! - Single braces: `{identifier}` or dot path `{obj.field}` - Valid interpolation: a valid identifier (`[a-zA-Z_][a-zA-Z0-9_]*`), dot path (`{config.key}`, `{a.b.c}`), or function call - Escaping: `\{` produces a literal `{` in output; `\}` produces a literal `}` in output -- Inside fenced code blocks (triple backtick): no interpolation occurs (raw passthrough) +- Inside fenced code blocks (triple backtick or tilde; also indented or blockquoted fences): no interpolation occurs (raw passthrough) - Undefined variable → compilation error (not silent empty string) +**Fence out-of-scope:** +- 4-space indented code blocks (CommonMark style, without fence markers) are **not** recognized as passthrough regions — interpolation IS parsed inside them. +- Leaving a blockquote does not implicitly close a blockquoted fence: `> ``` ... content without > prefix ... > ` ``` `` — the fence closes only on an explicit matching closer (`> ` `` ` or `> ~~~`). Without an explicit closer the fence extends to end-of-file. + --- ### 4.3 Conditionals @@ -165,7 +169,7 @@ Free tier. - Maximum 16 leaf operands per logical expression - Falsy values: `false`, `null`, empty string `""`, empty array `[]`, empty object `{}`, `0`, `NaN` - Everything else is truthy -- Equality is **strict**, no type coercion: `@if count == "3":` is false when count is the number 3 +- Equality is **strict**, no type coercion: comparing values of different types (e.g. `@if count == "3":` when `count` is the number `3`) is a **runtime error** (`mds::type_mismatch`). Convert explicitly: `@if str(count) == "3":` or `@if count == 3:` - `NaN == NaN` is false (IEEE 754) - `@elseif` branches are evaluated in order; first matching branch wins (short-circuit) - `@elseif` must appear before `@else:`; `@else:` cannot be followed by `@elseif` @@ -174,6 +178,28 @@ Free tier. - Maximum 256 `@elseif` branches per `@if` block - Nesting: plain `@end`, resolved by innermost matching +**Comparison semantics — type × operator:** + +| LHS type | RHS type | `==` | `!=` | Notes | +|----------|----------|------|------|-------| +| string | string | structural | structural | `"3" == "3"` → true | +| number | number | IEEE 754 | IEEE 754 | `NaN == NaN` → false | +| boolean | boolean | structural | structural | | +| null | null | true | false | | +| any | **different type** | **error** | **error** | `mds::type_mismatch` — no implicit coercion | + +**Truthiness vs equality — key distinction:** + +| Value | `@if` (truthy check) | `@if x == false:` (equality) | +|-------|----------------------|-------------------------------| +| `false` | falsy | true (same type, same value) | +| `""` | falsy | — (must compare with `""` literal) | +| `0` | falsy | — (must compare with `0` literal) | +| `"0"` | **truthy** (non-empty string) | requires `x == "0"` | +| `"false"` | **truthy** (non-empty string) | requires `x == "false"` | + +**`--set-string` truthiness footgun:** `--set-string count=0` sets `count` to the string `"0"`, not the number `0`. The string `"0"` is truthy (`@if count:` is true) even though the number `0` is falsy. Similarly, `--set-string flag=false` sets `flag` to the string `"false"` which is truthy. Compare with string literals explicitly when using `--set-string`: `@if count == "0":`. + --- ### 4.4 Loops @@ -606,10 +632,14 @@ role: data analysis You are a data analysis assistant. Perform statistical analysis. + You have access to: Python, R + Respond in plain text. ``` +The blank lines between sections come from the base skeleton (the blank line between each `@end` and the next `@block` directive is part of the skeleton and is carried through verbatim). + #### Rules **Directive placement:** @@ -636,7 +666,18 @@ Respond in plain text. - Nested mappings are merged key-by-key; arrays replace wholesale; scalars: child wins - Reserved keys (`imports`, `type`, `extends`) are excluded from the merged scope - Per-file `imports:` entries in frontmatter are each resolved against their own file's location -- The child's raw frontmatter fence is the only one emitted in the output (base frontmatter is input to scope, not output) +- The **deep-merged** frontmatter (base < child, reserved keys excluded) is emitted in the compiled output — not just the child's raw frontmatter. Both base-only and child-only keys appear; child wins on collisions + +**Named asymmetry — `@extends` vs standalone:** + +`@extends` templates and standalone templates differ in how frontmatter is emitted: + +| Template type | Emitted frontmatter | +|---------------|---------------------| +| **Standalone** | Raw-verbatim: the original source YAML between `---` fences, byte-for-byte (comments and quoting preserved) | +| **`@extends` child** | Canonically re-serialized: serde-yaml output of the deep-merged mapping (comments and original quoting are normalized away; YAML structure is canonical) | + +This asymmetry is intentional: standalone templates can round-trip YAML comments and non-canonical quoting, while `@extends` templates must emit a merged structure that has no single "source" YAML string. Runtime `--set`/`--set-string` variables do not alter the emitted frontmatter in either case — they affect only the compiled body. **Intrinsic output with inheritance:** @@ -681,9 +722,14 @@ Compiled output (messages kind — intrinsic because `@message` is present): **Whitespace contract:** -- Block body edges are stripped of leading/trailing blank lines (same rule as `@message` and `@define`) -- Skeleton whitespace around a spliced block carries through to the output verbatim, except that `@end` consumes the single newline immediately following it. So a blank line between two `@block` declarations renders as one line break, and back-to-back declarations (no separating line) render with no separator between their bodies — author the base skeleton's spacing accordingly -- Spacing before and after a spliced block is determined by the surrounding base skeleton, not the block body +Block bodies follow the **interior-verbatim with trailing-edge normalization** contract: +- Leading blank lines and interior blank runs inside a block body are preserved verbatim. +- Only the trailing edge is normalized: trailing whitespace is stripped and exactly one final newline is appended; `\r` is stripped unconditionally. +- This differs from `@message` and `@define` bodies, which still edge-trim (`.trim()`) — they strip leading and trailing blank lines. Block bodies do not. + +For the base skeleton: +- Skeleton whitespace around a spliced block carries through to the output verbatim, except that `@end` consumes the single newline immediately following it. A blank line between two `@block` declarations in the base renders as one blank line between the corresponding bodies in the output; back-to-back `@block` declarations (no separating blank line) render with no separator between bodies. +- Spacing before and after a spliced block is determined by the surrounding base skeleton, not the block body. **Error codes:** @@ -765,6 +811,7 @@ Errors include a diagnostic code (`mds::*`), file path, line number, column, a v |---------|---------| | `mds build [FILE\|DIR]` | Compile an `.mds` template (or a directory of templates) | | `mds check [FILE\|DIR]` | Validate a template or directory without rendering | +| `mds fmt [FILE\|DIR]` | Auto-format `.mds` templates in place (safety-gated) | | `mds init [FILENAME]` | Create a starter `.mds` file | ### 7.2 `mds build` @@ -806,6 +853,7 @@ mds build src/ --out-dir dist # Mirror subtree: src/a/b.mds → dis | `--out-dir ` | Output directory. Mirrors subtree (dir mode) or writes `.` inside it (file mode). Created if absent. | | `--vars ` | JSON file with runtime variable overrides. | | `--set KEY=VALUE` | Set a single variable. Repeatable. Values are coerced to boolean, number, null, or array when possible. | +| `--set-string KEY=VALUE` | Set a single variable as a **string**, bypassing type coercion. Repeatable. Use when the value must remain a string (e.g. a numeric-looking ID). | | `-q, --quiet` | Suppress status messages and warnings on stderr. | **Output path resolution** (precedence order, highest first): @@ -829,9 +877,28 @@ echo "@if flag:" | mds check - # Validate from stdin mds check src/ # Validate every non-partial .mds in the tree ``` -Exits 0 if all templates are valid, non-zero on any error. Same `--vars`/`--set`/`--quiet` options as `mds build`. Directory mode follows the same semantics as `mds build ` (partial skipping, symlink rejection, continue-on-error) but does not write any output files. +Exits 0 if all templates are valid, non-zero on any error. Same `--vars`/`--set`/`--set-string`/`--quiet` options as `mds build`. Directory mode follows the same semantics as `mds build ` (partial skipping, symlink rejection, continue-on-error) but does not write any output files. + +### 7.4 `mds fmt` + +```bash +mds fmt template.mds # Format a single file in place +mds fmt src/ # Format all .mds files under src/ +echo "template content" | mds fmt - # Format from stdin, write to stdout +mds fmt template.mds --check # Exit non-zero if file would change +mds fmt template.mds --diff # Print unified diff without writing +``` + +Formats `.mds` templates: normalizes CRLF to LF (everywhere, including inside frontmatter and code fences), strips trailing whitespace on directive lines, and ensures exactly one trailing newline. Interior blank lines and blank-line structure within frontmatter and code fences are left verbatim (blank-line collapsing was removed in v0.4.0 to preserve the interior-verbatim whitespace contract). Body-text trailing whitespace (Markdown hard breaks) and the byte-for-byte content of `@message`/`@define` bodies are left untouched. -### 7.4 `mds init` +Every rewrite is **safety-gated**: the formatter re-compiles both the original and formatted sources and refuses to write if compiled output would change (`mds::formatter_invariant`), so a formatting bug can never corrupt a template. + +| Option | Description | +|--------|-------------| +| `--check` | Exit non-zero without writing if any file would change. | +| `--diff` | Print a unified diff of proposed changes without writing. | + +### 7.5 `mds init` ```bash mds init # Creates hello.mds in current directory @@ -841,7 +908,7 @@ mds init my-prompt.mds --force # Overwrite if file already exists Creates a compilable starter template. Path traversal (e.g. `../escaped.mds`) is rejected. -### 7.5 Auto-Detection +### 7.6 Auto-Detection When no `FILE` argument is given to `mds build` or `mds check`, the compiler scans the current directory for `.mds` files: @@ -849,7 +916,7 @@ When no `FILE` argument is given to `mds build` or `mds check`, the compiler sca - **Zero found** → error with hint to run `mds init`. - **Multiple found** → error listing the files with a hint to specify one. -### 7.6 `mds.json` Project Config +### 7.7 `mds.json` Project Config Place `mds.json` in the project root (or any ancestor directory). The compiler walks up from the input file to find it. @@ -867,7 +934,7 @@ Place `mds.json` in the project root (or any ancestor directory). The compiler w Maximum config file size: 1 MB. -### 7.7 Exit Codes +### 7.8 Exit Codes | Code | Meaning | |------|---------| @@ -1105,6 +1172,10 @@ quoted_path := "\"" path_chars "\"" ## 12. Status +v0.4.0 - Breaking fixes release. Code fences now correctly recognize tilde fences (`~~~`), indented fences, and blockquoted fences (e.g. `> ``` ...`) as passthrough regions — interpolation and directives are not parsed inside them (#149). Interior whitespace in block bodies, `@define` function calls, and `mds fmt` output now follows the **interior-verbatim with trailing-edge normalization** contract — leading blank lines and interior blank runs are preserved verbatim; only the trailing edge normalizes to one final newline. The `mds fmt` blank-line collapsing rule (R3) has been removed (#150, #151). Cross-type equality comparisons (`string == number`, `boolean != null`, etc.) are now a runtime error (`mds::type_mismatch`) instead of silently returning `false`/`true` — both sides must be the same type (#152). A new `--set-string` CLI flag forces a variable to remain a string regardless of its value, bypassing type coercion; using a key in both `--set` and `--set-string` is now a hard error (#152). Interpolation errors now suggest `\{` in the help text (#153). `@extends` children now emit the **deep-merged** frontmatter (base < child, reserved keys excluded) instead of only the child's raw frontmatter — base-only keys appear in the compiled output (#154). + +v0.3.0 - Auto-formatter (`mds fmt`), intrinsic output format (Markdown vs JSON messages decided by content not a flag), native Python bindings (PyO3). + v0.2.0 - Language enrichment release. Adds built-in functions (18 functions for string, array, and type-conversion operations), default function arguments, and logical operators (`&&`, `||`) in `@if` conditions with short-circuit evaluation and operator-precedence semantics. v0.1.0 - Initial public release. The core compiler is feature-complete as described in this specification, including negation in `@if` conditions (`!dot_path`), equality/inequality comparisons (`==`, `!=`), the `@elseif` directive, and `NaN`/`Infinity` rejection at parse time.