From 16339e1a092abf0a5197bea4ef3292bc6d464e56 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Thu, 13 Aug 2026 10:39:42 +0200 Subject: [PATCH 01/32] =?UTF-8?q?feat(lint):=20JSON=20wire=20contract=20?= =?UTF-8?q?=E2=80=94=20sort=20diagnostics,=20stdin=20label,=20name-anchore?= =?UTF-8?q?d=20spans=20(#202,=20#203,=20#211)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **#211 — Uniform `` sentinel across all CLI surfaces** Adds `STDIN_DISPLAY_LABEL: &str = ""` to `output.rs` (AD-211-3) as the single definition of the stdin source-identity sentinel. Replaces five scattered conventions: `input.mds` (internal VFS key), bare `stdin`, `` (resolver), and the hardcoded `""` in `build.rs`. - `set_diag_display_path` called after every `lint_str_with` to relabel `diag.file` from `STRING_SOURCE_MAP_LABEL` → `STDIN_DISPLAY_LABEL` at the CLI output boundary (AD-211-4). The internal constant is NOT changed. - `StdinRelabeledError` wrapper overrides `miette::Diagnostic::source_code()` to relabel `` → `` in analysis-failure rendered output (AD-211-5), working around the `with_source_code()` fallback semantics in miette-7.6.0. - `emit_analysis_failure_json_or_stderr` gains `stdin_source: Option<(&str, &str)>` parameter; file-mode callers pass `None`. - `build.rs` now references `crate::output::STDIN_DISPLAY_LABEL` instead of a hardcoded `""` literal. **#202 — Diagnostics sorted by byte offset (wire contract)** Adds `sort_diagnostics` (stable sort by `(file, span.offset)`) called in `LintResultBuilder::build` after truncation, before JSON emission (AD-202-1). No-span diagnostics sort to the end of their file group (AD-202-3). The sort is stable so equal-offset diagnostics preserve rule-insertion order (AD-202-2). Fixes the `to_canonical_json` rustdoc to document `fix_edits` in the schema and the ordering guarantee. **#203 — `unused-import` span anchors at the unused name for selective imports** `ImportDirective::Selective` gains `name_offsets: Vec` (AD-203-1 / PF-012). `parse_import_directive` computes per-name byte offsets in a single pass alongside name collection (no desync possible for sparse inputs like `{ a, , b }`). The corrected delta formula uses `trim_start` (not `trim`) to measure the byte distance from directive start to `{`. `ImportFact` threads `name_offsets` through to `unused_import::check`, which now passes the per-name offset and `name.len()` to `make_diag` for Selective forms. Alias forms are unchanged. Tests: AC-P1-01, AC-P1-14, AC-P1-15, AC-P1-19 — 13 new unit tests + 3 CLI tests. All 1992 nextest + 50 doctests pass. Zero clippy warnings. No control bytes. --- CHANGELOG.md | 36 ++++ crates/mds-cli/src/build.rs | 3 +- crates/mds-cli/src/lint.rs | 159 +++++++++++++++--- crates/mds-cli/src/output.rs | 16 ++ crates/mds-cli/tests/cli_lint.rs | 76 ++++++++- crates/mds-cli/tests/print_discipline.rs | 7 + crates/mds-core/src/ast.rs | 11 ++ crates/mds-core/src/lint/diagnostic.rs | 148 +++++++++++++++- crates/mds-core/src/lint/facts.rs | 12 ++ .../mds-core/src/lint/rules/unused_import.rs | 143 ++++++++++++++-- crates/mds-core/src/parser_helpers.rs | 32 +++- crates/mds-core/src/resolver.rs | 1 + 12 files changed, 604 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6f0e2b..6f2b675 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,42 @@ via struct literals. Use the named constructor or builder listed for each: this method instead of assembling sanitized copies itself, keeping the escape logic co-located with the struct definition (PF-014). +#### Lint JSON wire contract: diagnostics sorted by byte offset (#202) + +Within each `files[].diagnostics` array in `mds lint --format json` output, +diagnostics are now ordered by ascending byte offset (`span.offset`). Previously +the order was rule-insertion order (implementation-defined). This is a **wire +contract change**: consumers that relied on a fixed rule-application order may +see reordered JSON output. + +- Diagnostics without a span sort to the end of their file group. +- Equal-offset diagnostics preserve the previous rule-insertion order (stable sort). +- File groups themselves remain lexicographically ordered (BTreeMap). + +#### Lint JSON wire contract: stdin source key is always `""` (#211) + +`mds lint --format json -` now emits `""` in the `files[].file` key. +Previously this field emitted `"input.mds"` (the internal VFS sentinel), which +was an implementation detail leaking into the public wire contract. + +Human-readable diagnostic output (stderr) now also consistently shows `` +as the source identity in span headers and status lines (e.g. +`Would fix: `, diff headers). + +#### `unused-import` diagnostic spans anchor at the unused name (#203) + +For selective imports (`@import { name1, name2 } from "path"`), the +`unused-import` diagnostic span now anchors at the **unused name's first byte** +rather than at the `@import` keyword. The `span.length` covers only the name +token. + +Before: `{ "offset": 0, "length": 7 }` (always the `@import` keyword) +After: `{ "offset": 10, "length": 5 }` (the name, e.g. `greet` in + `@import { greet } from ...`) + +Alias imports (`@import "path" as alias`) are unchanged — their span still +covers the `@import` keyword. + #### New `fix_edits` field on `LintDiagnostic` `LintDiagnostic` gains an additive `fix_edits` field (null when not fixable; diff --git a/crates/mds-cli/src/build.rs b/crates/mds-cli/src/build.rs index 318c6f4..0de7934 100644 --- a/crates/mds-cli/src/build.rs +++ b/crates/mds-cli/src/build.rs @@ -990,7 +990,8 @@ pub(crate) fn apply_source_map_file_label( if stdin_label { for src in &mut sm.sources { if src == STRING_SOURCE_MAP_LABEL { - *src = "".to_string(); + // AD-211-3: use the centralised sentinel from output.rs. + *src = crate::output::STDIN_DISPLAY_LABEL.to_string(); } } } diff --git a/crates/mds-cli/src/lint.rs b/crates/mds-cli/src/lint.rs index 158da37..5d6b512 100644 --- a/crates/mds-cli/src/lint.rs +++ b/crates/mds-cli/src/lint.rs @@ -39,7 +39,7 @@ use crate::build::{ }; use crate::output::{ atomic_write_file, collect_mds_files_detailed, eprint_error, eprint_warning, - render_unified_diff, safe_file_display, safe_inline, safe_path, + render_unified_diff, safe_file_display, safe_inline, safe_path, STDIN_DISPLAY_LABEL, }; /// Known lint rule names — used to warn about unknown names in mds.json config. @@ -165,7 +165,7 @@ fn do_lint(args: LintArgs) -> Result<()> { input.display() ), }; - emit_analysis_failure_json_or_stderr(&mds_err, format); + emit_analysis_failure_json_or_stderr(&mds_err, format, None); std::process::exit(2); } return run_lint_directory(&input, flags, runtime_vars); @@ -177,7 +177,7 @@ fn do_lint(args: LintArgs) -> Result<()> { // Route through emit_analysis_failure_json_or_stderr so --format json produces the // correct error envelope (L-CLI-JSON4 / AC-F-14). Do NOT use `?` here. if let Err(mds_err) = ensure_existing_mds_file(&input) { - emit_analysis_failure_json_or_stderr(&mds_err, format); + emit_analysis_failure_json_or_stderr(&mds_err, format, None); std::process::exit(mds_error_exit_code(&mds_err)); } run_lint_file(&input, flags, runtime_vars) @@ -227,7 +227,12 @@ fn load_lint_config(dir: &Path) -> Result { /// This function replaces the field with the caller-supplied relative path so /// the JSON output uses distinct, navigable paths. /// -/// Call this immediately after every `mds::lint` that runs in directory mode. +/// **AD-211-4 (stdin relabel):** also used for stdin mode — called with +/// `STDIN_DISPLAY_LABEL` immediately after every `mds::lint_str_with` call so +/// that `diag.file` in the JSON wire output reads `""` rather than the +/// internal VFS key `"input.mds"` (`STRING_SOURCE_MAP_LABEL`). +/// +/// Call this immediately after every `mds::lint` / `mds::lint_str_with` call. fn set_diag_display_path(result: &mut mds::LintResult, display: &str) { for diag in &mut result.diagnostics { diag.file = Some(display.to_string()); @@ -637,19 +642,34 @@ fn run_lint_stdin( let mds_err = MdsError::Io { message: format!("{e}"), }; - emit_analysis_failure_json_or_stderr(&mds_err, format); + // AD-211-5: config errors (MdsError::Io) have no embedded NamedSource, + // so stdin_source = Some(...) is a no-op for relabelling purposes but + // keeps the pattern consistent across all stdin failure paths. + emit_analysis_failure_json_or_stderr( + &mds_err, + format, + Some((STDIN_DISPLAY_LABEL, &source)), + ); std::process::exit(2); } }; - let result = match mds::lint_str_with(&source, Some(&cwd), runtime_vars.clone(), &config) { + let mut result = match mds::lint_str_with(&source, Some(&cwd), runtime_vars.clone(), &config) { Ok(r) => r, Err(e) => { - emit_analysis_failure_json_or_stderr(&e, format); + // AD-211-5: relabel → in the rendered failure envelope. + emit_analysis_failure_json_or_stderr(&e, format, Some((STDIN_DISPLAY_LABEL, &source))); std::process::exit(mds_error_exit_code(&e)); } }; + // AD-211-4 / AD-211-1: relabel diag.file from STRING_SOURCE_MAP_LABEL → + // STDIN_DISPLAY_LABEL at the CLI output boundary. fix.rs never reads diag.file + // (verified: zero reads in fix.rs), so this relabel is safe upstream of both + // preview_fixes and plan_and_apply_fixes. The JSON wire output's "files[].file" + // key therefore emits "" for every stdin lint, satisfying AC-P1-01. + set_diag_display_path(&mut result, STDIN_DISPLAY_LABEL); + if fix { // ── Preview path: --fix --check and/or --fix --diff (never writes source) ─── // Mirrors run_lint_file's preview path so stdin honours --check / --diff the @@ -659,12 +679,12 @@ fn run_lint_stdin( match preview { PreviewOutcome::WouldFix(ref fixed) => { if diff { - let diff_str = render_unified_diff(&source, fixed, "stdin"); + let diff_str = render_unified_diff(&source, fixed, STDIN_DISPLAY_LABEL); let _ = write_stdout(&diff_str); } if check { if !quiet { - eprintln!("Would fix: stdin"); + eprintln!("Would fix: {STDIN_DISPLAY_LABEL}"); } std::process::exit(1); } @@ -678,8 +698,10 @@ fn run_lint_stdin( } // After diff-only preview, or when nothing would change / fix rejected: // render diagnostics of the original result and exit by severity. + // AD-211-1: pass STDIN_DISPLAY_LABEL so span context renders "", not + // the internal STRING_SOURCE_MAP_LABEL ("input.mds"). let named_source = if format == LintFormat::Human { - Some((mds::STRING_SOURCE_MAP_LABEL, source.as_str())) + Some((STDIN_DISPLAY_LABEL, source.as_str())) } else { None }; @@ -703,7 +725,7 @@ fn run_lint_stdin( } => { if !quiet { eprintln!( - "Partially fixed: stdin ({applied_count} of {total_count} fixes applied)" + "Partially fixed: {STDIN_DISPLAY_LABEL} ({applied_count} of {total_count} fixes applied)" ); } (new_source, residual) @@ -715,7 +737,8 @@ fn run_lint_stdin( FixFileOutcome::NothingToFix { original } => (source, original), }; // Stdin diagnostics: pass source text for span context rendering. - let named_source = (mds::STRING_SOURCE_MAP_LABEL, output_src.as_str()); + // AD-211-1: use STDIN_DISPLAY_LABEL so source frame header reads "". + let named_source = (STDIN_DISPLAY_LABEL, output_src.as_str()); render_result_human(&diag_result, quiet, named_source); let _ = write_stdout(&output_src); exit_by_severity(&diag_result); @@ -723,8 +746,9 @@ fn run_lint_stdin( } // Report-only mode: pass stdin source for span context rendering. + // AD-211-1: use STDIN_DISPLAY_LABEL so span source frame reads "". let named_source = if format == LintFormat::Human { - Some((mds::STRING_SOURCE_MAP_LABEL, source.as_str())) + Some((STDIN_DISPLAY_LABEL, source.as_str())) } else { None }; @@ -757,7 +781,7 @@ fn run_lint_file( let mds_err = MdsError::Io { message: format!("{e}"), }; - emit_analysis_failure_json_or_stderr(&mds_err, format); + emit_analysis_failure_json_or_stderr(&mds_err, format, None); std::process::exit(2); } }; @@ -765,7 +789,7 @@ fn run_lint_file( let source = match read_source_file(path) { Ok(s) => s, Err(e) => { - emit_analysis_failure_json_or_stderr(&e, format); + emit_analysis_failure_json_or_stderr(&e, format, None); std::process::exit(mds_error_exit_code(&e)); } }; @@ -777,7 +801,7 @@ fn run_lint_file( let result = match mds::lint(path, runtime_vars.clone(), &config) { Ok(r) => r, Err(e) => { - emit_analysis_failure_json_or_stderr(&e, format); + emit_analysis_failure_json_or_stderr(&e, format, None); std::process::exit(mds_error_exit_code(&e)); } }; @@ -1416,9 +1440,89 @@ fn emit_result( } } -/// Emit an `MdsError` analysis failure. +// ── Analysis-failure rendering ──────────────────────────────────────────────── + +/// AD-211-5: thin wrapper that overrides `source_code()` to relabel the embedded +/// `NamedSource` in an `MdsError` when rendering analysis failures for stdin input. +/// +/// `resolve_source_intrinsic` sets `ctx.file_str = ""` so errors it produces +/// carry `NamedSource::new("", src)`. Replacing it at this boundary (not in +/// core) matches the "sanitize miette inputs, not rendered output" rule (PF-014) and +/// the "relabel at the CLI output boundary" discipline (AD-211-1). +/// +/// Delegates all `Diagnostic` methods to `inner` except `source_code`, which returns +/// the pre-built replacement `NamedSource` (or `None` when the inner error had no +/// embedded source — avoids miette trying to render spans against a missing source). +struct StdinRelabeledError { + inner: MdsError, + /// `Some(named)` when `inner` had embedded source code (so spans still render). + /// `None` when `inner` had no source code (MdsError::Io and similar). + source: Option>, +} + +impl std::fmt::Display for StdinRelabeledError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + std::fmt::Display::fmt(&self.inner, f) + } +} + +impl std::fmt::Debug for StdinRelabeledError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + std::fmt::Debug::fmt(&self.inner, f) + } +} + +impl std::error::Error for StdinRelabeledError {} + +impl miette::Diagnostic for StdinRelabeledError { + fn code<'a>(&'a self) -> Option> { + miette::Diagnostic::code(&self.inner) + } + fn severity(&self) -> Option { + miette::Diagnostic::severity(&self.inner) + } + fn help<'a>(&'a self) -> Option> { + miette::Diagnostic::help(&self.inner) + } + fn url<'a>(&'a self) -> Option> { + miette::Diagnostic::url(&self.inner) + } + fn labels<'a>(&'a self) -> Option + 'a>> { + miette::Diagnostic::labels(&self.inner) + } + fn source_code(&self) -> Option<&dyn miette::SourceCode> { + // Return the relabeled NamedSource only when the inner error actually has + // embedded source code; otherwise return None so miette skips code-frame + // rendering entirely. + self.source.as_ref().map(|s| s as &dyn miette::SourceCode) + } + fn related<'a>(&'a self) -> Option + 'a>> { + miette::Diagnostic::related(&self.inner) + } + fn diagnostic_source(&self) -> Option<&dyn miette::Diagnostic> { + miette::Diagnostic::diagnostic_source(&self.inner) + } +} + +/// AD-211-5 (2026-08-12 ruling): this envelope is the single CLI choke-point for +/// analysis failures (config load, IO, resolution, parse). When `stdin_source` is +/// `Some((display_label, source_text))`, the embedded source identity in the rendered +/// output is replaced with `display_label` (e.g. `STDIN_DISPLAY_LABEL = ""`), +/// so every CLI diagnostic context for stdin input uses the uniform sentinel instead +/// of the core's internal `SOURCE_LABEL` (`""`) that `resolve_source_intrinsic` +/// embeds in `MdsError` spans. This is a pure label swap at the output boundary — +/// core keeps `""` as `ctx.file_str` for non-stdin paths, and the source +/// content used for span rendering is unchanged. +/// +/// For errors from a file source, pass `stdin_source: None`; the error's embedded +/// `NamedSource` (which already carries the correct filename) is used as-is. +/// /// JSON format → stdout envelope; human → stderr via miette. -fn emit_analysis_failure_json_or_stderr(e: &MdsError, format: LintFormat) { +fn emit_analysis_failure_json_or_stderr( + e: &MdsError, + format: LintFormat, + stdin_source: Option<(&str, &str)>, +) { if format == LintFormat::Json { let envelope = serde_json::json!({ "version": 1, @@ -1431,7 +1535,24 @@ fn emit_analysis_failure_json_or_stderr(e: &MdsError, format: LintFormat) { } else { // Route through the single render choke point (avoids PF-004 / // architecture-6: hand-rolled sanitize_control_chars bypass). - eprint_error(miette::Report::from(e.clone())); + let report = match stdin_source { + Some((label, src)) => { + // AD-211-5: override the embedded NamedSource with the stdin + // sentinel. miette's WithSourceCode wrapper (used by with_source_code) + // returns self.error.source_code().or(Some(&self.source_code)), so the + // inner diagnostic's source_code takes priority. Instead, wrap the + // error in a delegate that REPLACES source_code() with the relabeled + // NamedSource — the same "sanitize inputs, not outputs" discipline as + // named_source_for_render elsewhere (PF-014). + let named = mds::named_source_for_render(label, src); + miette::Report::new(StdinRelabeledError { + inner: e.clone(), + source: miette::Diagnostic::source_code(e).map(|_| named), + }) + } + None => miette::Report::from(e.clone()), + }; + eprint_error(report); } } diff --git a/crates/mds-cli/src/output.rs b/crates/mds-cli/src/output.rs index 6bcd46d..37a413a 100644 --- a/crates/mds-cli/src/output.rs +++ b/crates/mds-cli/src/output.rs @@ -24,6 +24,22 @@ use miette::Result; use crate::build::{MdsConfig, OutputKind}; +// ── Stdin display sentinel ──────────────────────────────────────────────────── + +/// AD-211-1 / AD-211-3: the single stdin source-identity sentinel used by every +/// CLI diagnostic context. +/// +/// Every user-visible emission of stdin's source identity — human diagnostics, JSON +/// `files[].file`, fix-preview status lines, diff headers, source-map `sources[]`, +/// and the analysis-failure envelope — uses this exact string. The remap is applied +/// at the CLI output boundary; `crates/mds-core` continues to carry `"input.mds"` +/// (STRING_SOURCE_MAP_LABEL) as the internal VFS entry key, which is NOT changed. +/// +/// Centralised here so the CLI has exactly one definition of the sentinel (AD-211-3), +/// replacing the five previous scattered literals including the hardcoded `""` +/// in `build.rs:993`. +pub(crate) const STDIN_DISPLAY_LABEL: &str = ""; + // ── Output base for directory mode ──────────────────────────────────────────── /// Describes where directory-mode output files are written. diff --git a/crates/mds-cli/tests/cli_lint.rs b/crates/mds-cli/tests/cli_lint.rs index 8c62f35..e68aa1e 100644 --- a/crates/mds-cli/tests/cli_lint.rs +++ b/crates/mds-cli/tests/cli_lint.rs @@ -1274,10 +1274,11 @@ fn stdin_lint_diagnostic_includes_code_frame() { "diagnostic must appear in stdin report-only mode; got: {stderr}" ); - // "input.mds" must appear: miette renders it as the file reference in the span header. + // "" must appear: AD-211-1 relabels the span header from the internal + // STRING_SOURCE_MAP_LABEL ("input.mds") to the uniform CLI sentinel. assert!( - stderr.contains("input.mds"), - "stdin mode must show 'input.mds' in the code frame; got: {stderr}" + stderr.contains(""), + "stdin mode must show '' in the code frame; got: {stderr}" ); // At least one token from the source must appear in the code frame context. @@ -1288,6 +1289,75 @@ fn stdin_lint_diagnostic_includes_code_frame() { ); } +// ── AC-P1-01: stdin JSON wire `files[].file` emits "" ───────────────── +// +// Pins issue #211: every stdin lint path must emit `""` in the JSON +// `files[].file` key, not the internal VFS sentinel `"input.mds"`. +// +// Covers: run_lint_stdin report-only mode with --format json. + +#[test] +fn stdin_json_wire_file_key_is_stdin_sentinel() { + // Source with a known lint finding that needs no file imports so lint_str_with + // succeeds and emits a regular diagnostic JSON (not an analysis-failure envelope). + // duplicate-export fires without any resolver look-ups. + let source = "@define greet(name):\n Hello {{name}}!\n@end\n\n@export greet\n@export greet\n"; + let out = lint_stdin(source, &["--format", "json"]); + + let stdout = String::from_utf8_lossy(&out.stdout); + let v: serde_json::Value = serde_json::from_str(&stdout).expect("stdout must be valid JSON"); + + let files = v["files"].as_array().expect("JSON must have 'files' array"); + assert!( + !files.is_empty(), + "stdin JSON must have at least one file group (duplicate-export fires); got: {stdout}" + ); + for entry in files { + let file_key = entry["file"] + .as_str() + .expect("each file entry must have a 'file' string"); + assert_eq!( + file_key, "", + "AC-P1-01: JSON files[].file must be '' for stdin input, not '{file_key}'" + ); + } +} + +// ── AC-P1-14/#202: JSON wire diagnostics sorted by byte offset ─────────────── +// +// Pins issue #202: within a file group, diagnostics must appear in ascending +// byte-offset order regardless of the order rules were applied. + +#[test] +fn stdin_json_diagnostics_sorted_by_offset() { + // Two diagnostics at different offsets: the one at the lower offset must + // come first. Use a source with two distinct export violations placed at + // known positions. + let source = "@define greet(name):\n Hello {{name}}!\n@end\n\n@export greet\n@export greet\n"; + let out = lint_stdin(source, &["--format", "json"]); + + let stdout = String::from_utf8_lossy(&out.stdout); + let v: serde_json::Value = serde_json::from_str(&stdout).expect("stdout must be valid JSON"); + + let files = v["files"].as_array().expect("JSON must have 'files' array"); + for entry in files { + let diags = entry["diagnostics"] + .as_array() + .expect("must have 'diagnostics'"); + let offsets: Vec = diags + .iter() + .filter_map(|d| d["span"]["offset"].as_i64()) + .collect(); + let mut sorted = offsets.clone(); + sorted.sort_unstable(); + assert_eq!( + offsets, sorted, + "AC-P1-14: diagnostics must be in ascending byte-offset order; \ + got offsets: {offsets:?}" + ); + } +} + // ── Test (i): Auto-detect hint names the invoking subcommand ───────────────── // // Pins bugs 22/23: auto_detect_mds_file and resolve_input now take a `subcommand: diff --git a/crates/mds-cli/tests/print_discipline.rs b/crates/mds-cli/tests/print_discipline.rs index 5f8c3c4..693ce55 100644 --- a/crates/mds-cli/tests/print_discipline.rs +++ b/crates/mds-cli/tests/print_discipline.rs @@ -279,6 +279,13 @@ const ALLOWED_UNSANITIZED: &[(&str, &str, &str)] = &[ "`usize` count of `.mds` files the default-exclusion walker skipped \ (hidden dirs, node_modules); produced by `collect_mds_files_detailed`.", ), + ( + "lint.rs", + "STDIN_DISPLAY_LABEL", + "`&'static str` compile-time constant defined in `output.rs` as `\"\"`. \ + It is the uniform stdin source-identity sentinel (AD-211-3 / issue #211); \ + it contains only ASCII printable characters and cannot carry hostile bytes.", + ), ( "lint.rs", "applied_count", diff --git a/crates/mds-core/src/ast.rs b/crates/mds-core/src/ast.rs index 0386f9b..3cb4cb5 100644 --- a/crates/mds-core/src/ast.rs +++ b/crates/mds-core/src/ast.rs @@ -357,6 +357,17 @@ pub enum ImportDirective { names: Vec, path: String, offset: usize, + /// Byte offset of each name within the source file (parallel to `names`). + /// + /// **AD-203-1 / PF-012:** computed by `parse_import_directive` in a single + /// pass alongside name collection so the two vectors never desync. Used by + /// the `unused-import` rule to anchor the diagnostic span at the unused name + /// rather than at the `@import` keyword. + /// + /// This field is intentionally excluded from structural equality (see + /// `structural_eq.rs` — `Selective` uses `..` pattern) because it is a + /// span annotation, not part of the semantic content of the directive. + name_offsets: Vec, }, } diff --git a/crates/mds-core/src/lint/diagnostic.rs b/crates/mds-core/src/lint/diagnostic.rs index 64940c5..742b6a1 100644 --- a/crates/mds-core/src/lint/diagnostic.rs +++ b/crates/mds-core/src/lint/diagnostic.rs @@ -696,7 +696,8 @@ impl LintResult { /// "message": "...", /// "help": "...", /// "fixable": false, - /// "span": { "offset": 0, "length": 5, "line": 1, "column": 1 } + /// "span": { "offset": 0, "length": 5, "line": 1, "column": 1 }, + /// "fix_edits": [{ "start": 0, "end": 7, "new_text": "" }] /// } /// ] /// } @@ -710,6 +711,13 @@ impl LintResult { /// Some(..)`). /// The `fixable` field reflects tier semantics: `true` for Tier A rules and for /// Tier B rules when the file is standalone, `false` otherwise. + /// The `fix_edits` field is `null` when no machine-applicable fix is available. + /// + /// **AD-202-3 (ordering guarantee):** within each file group, diagnostics are + /// ordered by ascending byte offset (`span.offset`). Diagnostics without a span + /// sort to the end of the group. The sort is stable so equal-offset diagnostics + /// preserve rule-insertion order. Files are ordered lexicographically by file + /// path (BTreeMap property). /// /// **NEVER** build this JSON via `format!()` — use `serde_json::json!()` so /// control characters in message/help are serialized safely. @@ -827,14 +835,43 @@ impl LintResultBuilder { } pub(crate) fn build(self, is_standalone: bool) -> LintResult { + let mut diagnostics = self.diagnostics; + // AD-202-1: sort after truncation (some diagnostics may have been dropped), + // before JSON emission. Stable sort preserves rule-insertion order for + // equal-offset diagnostics (AD-202-2). + sort_diagnostics(&mut diagnostics); LintResult { - diagnostics: self.diagnostics, + diagnostics, truncated: self.truncated, is_standalone, } } } +// ── Diagnostic ordering ─────────────────────────────────────────────────────── + +/// Sort `diagnostics` in-place by `(file, span.offset)` — ascending, stable. +/// +/// **AD-202-1:** called in `LintResultBuilder::build`, after truncation, before +/// JSON emission. +/// +/// **AD-202-2:** the sort is stable (`sort_by`, not `sort_unstable_by`) so +/// equal-offset diagnostics preserve the insertion order established by the rule +/// dispatch loop — rule tests that assert exact ordering remain deterministic. +/// +/// **AD-202-3:** diagnostics without a span sort to the end of their file group +/// (we use `usize::MAX` as a sentinel offset). Diagnostics without a file sort +/// to the end of the overall list. +fn sort_diagnostics(diagnostics: &mut [LintDiagnostic]) { + diagnostics.sort_by(|a, b| { + let file_a = a.file.as_deref().unwrap_or("\u{FFFF}"); + let file_b = b.file.as_deref().unwrap_or("\u{FFFF}"); + let off_a = a.span.as_ref().map(|s| s.offset).unwrap_or(usize::MAX); + let off_b = b.span.as_ref().map(|s| s.offset).unwrap_or(usize::MAX); + file_a.cmp(file_b).then(off_a.cmp(&off_b)) + }); +} + // ── sanitize_control_chars ──────────────────────────────────────────────────── /// Which escape class a sanitizer call applies. @@ -2043,4 +2080,111 @@ mod tests { "fix_edits must be None in the sanitized clone" ); } + + // ── AC-P1-14 / AD-202-1: sort by byte offset ───────────────────────────── + + fn make_span_diag(file: Option<&str>, offset: Option, rule: &str) -> LintDiagnostic { + LintDiagnostic { + rule: rule.to_string(), + severity: Severity::Warn, + message: format!("{rule} at {offset:?}"), + help: None, + span: offset.map(|o| SerializedSpan { + offset: o, + length: 1, + line: None, + column: None, + }), + file: file.map(str::to_string), + fix_removals: None, + fix_edits: None, + } + } + + /// AC-P1-14: `LintResultBuilder::build` sorts diagnostics by ascending byte + /// offset within each file. + #[test] + fn build_sorts_diagnostics_by_offset() { + let mut builder = LintResultBuilder::new(); + // Insert in reverse offset order. + builder.push(make_span_diag(Some("a.mds"), Some(20), "r1")); + builder.push(make_span_diag(Some("a.mds"), Some(5), "r2")); + builder.push(make_span_diag(Some("a.mds"), Some(10), "r3")); + let result = builder.build(false); + let offsets: Vec<_> = result + .diagnostics + .iter() + .map(|d| d.span.as_ref().map(|s| s.offset)) + .collect(); + assert_eq!( + offsets, + vec![Some(5), Some(10), Some(20)], + "diagnostics must be ordered by ascending byte offset; got: {:?}", + offsets + ); + } + + /// AC-P1-14 (multi-file): files are ordered lexicographically; within a file + /// diagnostics are ordered by byte offset. + #[test] + fn build_sorts_multi_file_diagnostics() { + let mut builder = LintResultBuilder::new(); + builder.push(make_span_diag(Some("b.mds"), Some(1), "r1")); + builder.push(make_span_diag(Some("a.mds"), Some(10), "r2")); + builder.push(make_span_diag(Some("a.mds"), Some(3), "r3")); + let result = builder.build(false); + let keys: Vec<_> = result + .diagnostics + .iter() + .map(|d| (d.file.as_deref(), d.span.as_ref().map(|s| s.offset))) + .collect(); + assert_eq!( + keys, + vec![ + (Some("a.mds"), Some(3)), + (Some("a.mds"), Some(10)), + (Some("b.mds"), Some(1)), + ], + "multi-file diagnostics must sort by (file, offset); got: {:?}", + keys + ); + } + + /// AC-P1-15 / AD-202-2: diagnostics with the same file and offset preserve + /// insertion order (stable sort). + #[test] + fn build_stable_sort_preserves_insertion_order_for_equal_offsets() { + let mut builder = LintResultBuilder::new(); + builder.push(make_span_diag(Some("a.mds"), Some(5), "first")); + builder.push(make_span_diag(Some("a.mds"), Some(5), "second")); + builder.push(make_span_diag(Some("a.mds"), Some(5), "third")); + let result = builder.build(false); + let rules: Vec<_> = result.diagnostics.iter().map(|d| d.rule.as_str()).collect(); + assert_eq!( + rules, + vec!["first", "second", "third"], + "equal-offset diagnostics must preserve insertion order; got: {:?}", + rules + ); + } + + /// AC-P1-14 (no-span): diagnostics without a span sort to the end of their + /// file group. + #[test] + fn build_no_span_sorts_to_end() { + let mut builder = LintResultBuilder::new(); + builder.push(make_span_diag(Some("a.mds"), None, "no-span")); + builder.push(make_span_diag(Some("a.mds"), Some(3), "has-span")); + let result = builder.build(false); + assert_eq!( + result.diagnostics[0].rule, "has-span", + "spanned diagnostic must sort before no-span; got: {:?}", + result.diagnostics + ); + assert_eq!( + result.diagnostics[1].rule, "no-span", + "no-span diagnostic must sort to the end; got: {:?}", + result.diagnostics + ); + } } diff --git a/crates/mds-core/src/lint/facts.rs b/crates/mds-core/src/lint/facts.rs index da64b50..e74ba01 100644 --- a/crates/mds-core/src/lint/facts.rs +++ b/crates/mds-core/src/lint/facts.rs @@ -32,6 +32,12 @@ pub struct ImportFact { pub alias: Option, /// Names for `@import { name1, name2 } from "path"` forms. pub names: Vec, + /// Byte offset of each name in `names` within the source file (Selective only). + /// + /// **AD-203-1 / PF-012:** parallel to `names`; always `Vec::new()` for Alias + /// and Merge forms. Used by the `unused-import` rule to anchor the diagnostic + /// span at the unused name rather than at the `@import` keyword. + pub name_offsets: Vec, /// Byte offset of the `@import` token in the source. pub offset: usize, } @@ -415,6 +421,7 @@ fn collect_import_fact(imp: &ImportDirective, ctx: &mut AnalysisContext) { kind: ImportKind::Alias, alias: Some(alias.clone()), names: vec![], + name_offsets: vec![], offset: *offset, }); } @@ -424,6 +431,7 @@ fn collect_import_fact(imp: &ImportDirective, ctx: &mut AnalysisContext) { kind: ImportKind::Merge, alias: None, names: vec![], + name_offsets: vec![], offset: *offset, }); } @@ -431,12 +439,16 @@ fn collect_import_fact(imp: &ImportDirective, ctx: &mut AnalysisContext) { names, path, offset, + name_offsets, } => { ctx.imports.push(ImportFact { path: path.clone(), kind: ImportKind::Selective, alias: None, names: names.clone(), + // AD-203-1: thread per-name offsets through to the rule so the + // unused-import diagnostic can anchor at the name, not @import. + name_offsets: name_offsets.clone(), offset: *offset, }); } diff --git a/crates/mds-core/src/lint/rules/unused_import.rs b/crates/mds-core/src/lint/rules/unused_import.rs index 7535869..0b2c188 100644 --- a/crates/mds-core/src/lint/rules/unused_import.rs +++ b/crates/mds-core/src/lint/rules/unused_import.rs @@ -109,6 +109,7 @@ pub(crate) fn check( .to_string(), ), imp.offset, + "@import".len(), )) { return; @@ -116,12 +117,16 @@ pub(crate) fn check( } ImportKind::Selective => { // Per-name flagging: each name checked individually. - for name in &imp.names { + // AD-203-1 / PF-012: anchor the span at the name, not @import. + for (i, name) in imp.names.iter().enumerate() { let is_used = ctx.used_calls.contains(name) || ctx.used_vars.contains(name) || reexport_names.contains(name); - if !is_used - && !builder.push(make_diag( + if !is_used { + // Prefer the per-name offset; fall back to @import offset + // if name_offsets is unexpectedly short (defensive). + let name_offset = imp.name_offsets.get(i).copied().unwrap_or(imp.offset); + if !builder.push(make_diag( severity, filename, format!( @@ -132,10 +137,11 @@ pub(crate) fn check( "Remove '{}' from the selective import or use it in the body.", name )), - imp.offset, - )) - { - return; + name_offset, + name.len(), + )) { + return; + } } } } @@ -147,14 +153,24 @@ fn resolve_severity(config: &LintConfig) -> Severity { config.severity_for(RULE).copied().unwrap_or(Severity::Warn) } -/// Build an unused-import diagnostic. The span always covers the `@import` keyword -/// (length = 7), so `offset` is the only caller-supplied span parameter. +/// Build an unused-import diagnostic. +/// +/// `offset` is the byte position of the span anchor within the source. +/// `length` is the byte length of the highlighted token. +/// +/// For Alias and Merge forms the caller passes `imp.offset` / +/// `"@import".len()` so the span covers the `@import` keyword. +/// +/// For Selective forms the caller passes the per-name offset from +/// `imp.name_offsets` and `name.len()` so the span covers the unused name +/// (AD-203-1 / PF-012). fn make_diag( severity: Severity, filename: &str, message: String, help: Option, offset: usize, + length: usize, ) -> LintDiagnostic { LintDiagnostic { rule: RULE.to_string(), @@ -163,7 +179,7 @@ fn make_diag( help, span: Some(SerializedSpan { offset, - length: "@import".len(), + length, line: None, column: None, }), @@ -343,4 +359,111 @@ mod tests { check(&module, &ctx, "test.mds", &config, &mut builder); assert!(builder.build(false).diagnostics.is_empty()); } + + // ── AC-P1-19 / AD-203-1: span anchors at the unused name ───────────────── + + /// AC-P1-19: for a single unused name in a selective import, the span offset + /// must point at the name's first byte, not at the `@import` keyword. + /// + /// Source: `@import { greet } from "./lib.mds"\n` + /// 0123456789012345... + /// ^ 'greet' starts at byte 10 (after "@import { ") + #[test] + fn selective_span_anchors_at_name_not_at_import_keyword() { + let src = "@import { greet } from \"./lib.mds\"\nHello!\n"; + let diags = lint_src(src); + let diag = diags + .iter() + .find(|d| d.rule == RULE && d.message.contains("greet")) + .expect("unused-import diagnostic for 'greet' must fire"); + let span = diag.span.as_ref().expect("span must be present"); + + // "@import { " = 10 bytes before 'greet'. + let expected_offset = "@import { ".len(); + assert_eq!( + span.offset, expected_offset, + "span.offset must point at the name 'greet' (byte {}), not at @import (byte 0); \ + got span.offset={}", + expected_offset, span.offset + ); + assert_eq!( + span.length, + "greet".len(), + "span.length must equal the name length; got span.length={}", + span.length + ); + } + + /// AC-P1-19 (second name): in a multi-name selective import, each unused name + /// has an independently anchored span. + /// + /// Source: `@import { foo, bar } from "./lib.mds"\n` + /// 0123456789012345678... + /// ^ 'foo' at 10, 'bar' at 15 + #[test] + fn selective_multi_name_each_span_anchored_independently() { + let src = "@import { foo, bar } from \"./lib.mds\"\nHello!\n"; + let diags = lint_src(src); + + let foo_diag = diags + .iter() + .find(|d| d.rule == RULE && d.message.contains("'foo'")) + .expect("diagnostic for 'foo' must fire"); + let bar_diag = diags + .iter() + .find(|d| d.rule == RULE && d.message.contains("'bar'")) + .expect("diagnostic for 'bar' must fire"); + + let foo_span = foo_diag + .span + .as_ref() + .expect("span for 'foo' must be present"); + let bar_span = bar_diag + .span + .as_ref() + .expect("span for 'bar' must be present"); + + // "@import { " = 10 bytes. + assert_eq!( + foo_span.offset, + "@import { ".len(), + "span for 'foo' must start at byte {}; got {}", + "@import { ".len(), + foo_span.offset + ); + assert_eq!(foo_span.length, "foo".len()); + + // "@import { foo, " = 15 bytes. + assert_eq!( + bar_span.offset, + "@import { foo, ".len(), + "span for 'bar' must start at byte {}; got {}", + "@import { foo, ".len(), + bar_span.offset + ); + assert_eq!(bar_span.length, "bar".len()); + } + + /// Alias form still anchors at the `@import` keyword (not changed by #203). + #[test] + fn alias_span_anchors_at_import_keyword() { + let src = "@import \"./lib.mds\" as lib\nHello!\n"; + let diags = lint_src(src); + let diag = diags + .iter() + .find(|d| d.rule == RULE) + .expect("unused alias import diagnostic must fire"); + let span = diag.span.as_ref().expect("span must be present"); + assert_eq!( + span.offset, 0, + "alias span must start at byte 0 (@import); got {}", + span.offset + ); + assert_eq!( + span.length, + "@import".len(), + "alias span.length must equal '@import' length; got {}", + span.length + ); + } } diff --git a/crates/mds-core/src/parser_helpers.rs b/crates/mds-core/src/parser_helpers.rs index 9c969ed..4af112a 100644 --- a/crates/mds-core/src/parser_helpers.rs +++ b/crates/mds-core/src/parser_helpers.rs @@ -813,11 +813,32 @@ pub(super) fn parse_import_directive(directive: &str, offset: usize) -> Result = names_str - .split(',') - .map(|n| n.trim().to_string()) - .filter(|n| !n.is_empty()) - .collect(); + + // AD-203-1 / PF-012: compute per-name byte offsets in a single pass + // alongside name collection so the two vectors cannot desync. + // + // We use trim_start (not trim) to get the exact byte distance from the + // start of `directive` to the `{`: trim() on the right side might collapse + // trailing whitespace and shift the length, yielding a wrong delta. + let delta = directive.len() - directive.trim_start_matches("@import").trim_start().len(); + // `names_str` begins at `offset + delta + 1` (past the `{`). + let names_str_start = offset + delta + 1; + + let mut names: Vec = Vec::new(); + let mut name_offsets: Vec = Vec::new(); + let mut cursor = 0usize; // byte cursor within `names_str` + for seg in names_str.split(',') { + let seg_byte_len = seg.len(); + let trimmed = seg.trim(); + if !trimmed.is_empty() { + // Count leading whitespace bytes within this segment so we land + // on the first byte of the identifier, not on the space before it. + let leading_ws = seg.len() - seg.trim_start().len(); + name_offsets.push(names_str_start + cursor + leading_ws); + names.push(trimmed.to_string()); + } + cursor += seg_byte_len + 1; // +1 for the comma separator + } for name in &names { if !is_valid_identifier(name) { @@ -837,6 +858,7 @@ pub(super) fn parse_import_directive(directive: &str, offset: usize) -> Result self.resolve_selective_import(names, path, *offset, scope, ctx, warnings), } } From d150f65ba34f2a83286075e217a802158ccd00e1 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Thu, 13 Aug 2026 10:48:14 +0200 Subject: [PATCH 02/32] refactor(lint): remove one-shot intermediates in offset loop and error relabel - `parse_import_directive`: drop `seg_byte_len` local (used only at the end of the loop body) and call `seg.len()` directly on the advance line. - `emit_analysis_failure_json_or_stderr`: inline `named` into the `StdinRelabeledError` struct initialiser; it was created and consumed on consecutive lines with no intervening use. Behaviour is unchanged; fmt and clippy pass; source-hygiene gate clean. --- crates/mds-cli/src/lint.rs | 4 ++-- crates/mds-core/src/parser_helpers.rs | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/mds-cli/src/lint.rs b/crates/mds-cli/src/lint.rs index 5d6b512..3d6bb80 100644 --- a/crates/mds-cli/src/lint.rs +++ b/crates/mds-cli/src/lint.rs @@ -1544,10 +1544,10 @@ fn emit_analysis_failure_json_or_stderr( // error in a delegate that REPLACES source_code() with the relabeled // NamedSource — the same "sanitize inputs, not outputs" discipline as // named_source_for_render elsewhere (PF-014). - let named = mds::named_source_for_render(label, src); miette::Report::new(StdinRelabeledError { inner: e.clone(), - source: miette::Diagnostic::source_code(e).map(|_| named), + source: miette::Diagnostic::source_code(e) + .map(|_| mds::named_source_for_render(label, src)), }) } None => miette::Report::from(e.clone()), diff --git a/crates/mds-core/src/parser_helpers.rs b/crates/mds-core/src/parser_helpers.rs index 4af112a..542c693 100644 --- a/crates/mds-core/src/parser_helpers.rs +++ b/crates/mds-core/src/parser_helpers.rs @@ -828,7 +828,6 @@ pub(super) fn parse_import_directive(directive: &str, offset: usize) -> Result = Vec::new(); let mut cursor = 0usize; // byte cursor within `names_str` for seg in names_str.split(',') { - let seg_byte_len = seg.len(); let trimmed = seg.trim(); if !trimmed.is_empty() { // Count leading whitespace bytes within this segment so we land @@ -837,7 +836,7 @@ pub(super) fn parse_import_directive(directive: &str, offset: usize) -> Result Date: Thu, 13 Aug 2026 11:12:00 +0200 Subject: [PATCH 03/32] fix: address self-review issues (#202, #203, #211) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P0 — the AC-P1-04 / §0a leg that was never implemented. `mds check -` and `mds build -` still rendered `[:1:1]` for stdin; reproduced per plan §5 step 1a, then fixed. `StdinRelabeledError` + `relabel_stdin_error` move from `lint.rs` to `output.rs` and are applied at `run_check` and both `compile_str_with_deps_opts` stdin sites. `build::exit_code` unwraps the wrapper so a render-only label swap cannot change a process exit code. P0 — the #202 wire-ordering test passed vacuously. Its fixture produced exactly one diagnostic, so `offsets == sorted` held with the sort deleted. Replaced with a fixture whose offset order inverts `run_rules` dispatch order, plus a non-vacuity guard and an explicit rule-position assertion. P0 — #203's span anchoring had no test for the desync and robustness cases the plan flagged as blocker-class. Added slice-based positive controls (AC-P1-14/15/16) covering empty and trailing comma segments, prefix and path name collisions, irregular whitespace, CRLF and multi-byte prefixes. All verified non-vacuous against planted bugs. P1 — sort key no longer uses a `\u{FFFF}` sentinel for a missing file, which mis-ordered against astral-plane filenames; AD-203-3's `debug_assert_eq!` added at the construction site; the desync fallback now degrades to the whole `@import` span instead of `name.len()` bytes of the keyword. Added: AC-P1-07 (analysis-failure label, both channels), AC-P1-03 (fix-preview sentinel), AC-P1-09/11 (cross-surface order, determinism), AC-P1-10 (files[] path order), AC-P1-12 (truncation is not offset-ranked), AC-P1-18 (formatter safety gate ignores name_offsets), AC-P1-20 (WIRE escaping positive control). P2 — AD-202-x rustdoc IDs and AC-P1-xx test citations corrected against the plan; `main.rs` / `fmt.rs` stdin literals point at `STDIN_DISPLAY_LABEL` (AD-211-3); CHANGELOG rewritten as the wave's single wire-change ledger with a before/after snippet and the AD-211-5 leg. --- CHANGELOG.md | 91 ++-- crates/mds-cli/src/build.rs | 18 +- crates/mds-cli/src/fmt.rs | 13 +- crates/mds-cli/src/lint.rs | 122 ++--- crates/mds-cli/src/main.rs | 8 +- crates/mds-cli/src/output.rs | 107 ++++- crates/mds-cli/tests/cli_lint.rs | 431 +++++++++++++++++- crates/mds-cli/tests/print_discipline.rs | 14 + crates/mds-core/src/formatter.rs | 39 ++ crates/mds-core/src/lint/diagnostic.rs | 172 +++++-- .../mds-core/src/lint/rules/unused_import.rs | 341 +++++++++++--- crates/mds-core/src/parser_helpers.rs | 36 +- crates/mds-core/src/parser_tests.rs | 25 + 13 files changed, 1166 insertions(+), 251 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f2b675..a19df2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,41 +78,70 @@ via struct literals. Use the named constructor or builder listed for each: this method instead of assembling sanitized copies itself, keeping the escape logic co-located with the struct definition (PF-014). -#### Lint JSON wire contract: diagnostics sorted by byte offset (#202) +#### Lint JSON wire contract (#202, #203, #211) + +> This block is the **single wire-change ledger** for the lint JSON envelope. +> Later changes to `mds lint --format json` append here rather than opening a +> parallel section, so a consumer has one place to read. + +**Before / after**, for `mds lint - --format json` on a source with one unused +selective import: + +```jsonc +// before +{ "version": 1, "truncated": false, "files": [ { "file": "input.mds", + "diagnostics": [ + { "rule": "duplicate-export", "span": { "offset": 59, "length": 7 } }, + { "rule": "unused-import", "span": { "offset": 0, "length": 7 } } + ] } ] } + +// after +{ "version": 1, "truncated": false, "files": [ { "file": "", + "diagnostics": [ + { "rule": "unused-import", "span": { "offset": 10, "length": 5 } }, + { "rule": "duplicate-export", "span": { "offset": 59, "length": 7 } } + ] } ] } +``` + +**A consumer breaks if it** keys off `files[].file == "input.mds"` for CLI stdin +output, matches `` in an `error.message` or rendered frame, relies on +`diagnostics[]` arriving in rule-execution order, or assumes `unused-import` +spans have length 7. -Within each `files[].diagnostics` array in `mds lint --format json` output, -diagnostics are now ordered by ascending byte offset (`span.offset`). Previously -the order was rule-insertion order (implementation-defined). This is a **wire -contract change**: consumers that relied on a fixed rule-application order may -see reordered JSON output. +**1. Diagnostics are sorted by byte offset (#202).** Within each +`files[].diagnostics` array, diagnostics are ordered by ascending `span.offset`. +Previously the order was rule-execution order (implementation-defined). - Diagnostics without a span sort to the end of their file group. -- Equal-offset diagnostics preserve the previous rule-insertion order (stable sort). +- Equal-offset diagnostics preserve rule-execution order (stable sort). - File groups themselves remain lexicographically ordered (BTreeMap). - -#### Lint JSON wire contract: stdin source key is always `""` (#211) - -`mds lint --format json -` now emits `""` in the `files[].file` key. -Previously this field emitted `"input.mds"` (the internal VFS sentinel), which -was an implementation detail leaking into the public wire contract. - -Human-readable diagnostic output (stderr) now also consistently shows `` -as the source identity in span headers and status lines (e.g. -`Would fix: `, diff headers). - -#### `unused-import` diagnostic spans anchor at the unused name (#203) - -For selective imports (`@import { name1, name2 } from "path"`), the -`unused-import` diagnostic span now anchors at the **unused name's first byte** -rather than at the `@import` keyword. The `span.length` covers only the name -token. - -Before: `{ "offset": 0, "length": 7 }` (always the `@import` keyword) -After: `{ "offset": 10, "length": 5 }` (the name, e.g. `greet` in - `@import { greet } from ...`) - -Alias imports (`@import "path" as alias`) are unchanged — their span still -covers the `@import` keyword. +- Ordering is established on `LintResult.diagnostics` itself, so the CLI human + path and the napi / WASM / Python surfaces observe the same order. +- **Truncation is unchanged and is NOT offset-ranked.** When `truncated` is + `true`, the retained diagnostics are still the first `MAX_DIAGNOSTICS` (1,000) + in rule-execution order, re-sorted afterwards — not the 1,000 smallest offsets. + +**2. The stdin source identity is always `` (#211).** Every CLI context +that names a stdin source now uses the single sentinel ``: + +- the JSON `files[].file` key (previously `"input.mds"`, the internal VFS key); +- human diagnostic frames for `mds lint -` (previously `input.mds`); +- fix-preview status lines and diff headers (previously bare `stdin`); +- the **analysis-failure envelope** — a stdin source that fails the check gate + used to render `:L:C`, the resolver's internal label. `mds check -` and + `mds build -` rendered `` on the same path and now render `` + too, so all four subcommands agree. + +`mds::STRING_SOURCE_MAP_LABEL` is **unchanged** and remains `"input.mds"`: it is a +virtual-FS entry key, not a display label. The napi, WASM and Python lint APIs +continue to report `"input.mds"` for string-source input. The relabel is applied +only at the CLI output boundary. + +**3. `unused-import` spans anchor at the unused name (#203).** For selective +imports (`@import { name1, name2 } from "path"`), the span now covers the unused +name rather than the `@import` keyword, and `span.length` is the name's length +instead of a constant 7. Alias imports (`@import "path" as alias`) are unchanged — +their span still covers the `@import` keyword. #### New `fix_edits` field on `LintDiagnostic` diff --git a/crates/mds-cli/src/build.rs b/crates/mds-cli/src/build.rs index 0de7934..17e6cdb 100644 --- a/crates/mds-cli/src/build.rs +++ b/crates/mds-cli/src/build.rs @@ -431,7 +431,15 @@ pub(crate) fn parse_cli_value(val: String) -> mds::Value { /// and correctly fall through to exit code 1. Only `MdsError` values converted via /// `.map_err(miette::Error::from)` are categorized. pub(crate) fn exit_code(err: &miette::Error) -> i32 { - if let Some(mds_err) = err.downcast_ref::() { + // AD-211-5: `StdinRelabeledError` is a render-only wrapper around an `MdsError`. + // It must be unwrapped here or wrapping an error to fix its DISPLAY label would + // silently change its EXIT CODE (a wrapped FileNotFound would fall through to 1 + // instead of 2). The label swap is not allowed to have behavioural side effects. + let mds_err = err.downcast_ref::().or_else(|| { + err.downcast_ref::() + .map(crate::output::StdinRelabeledError::inner) + }); + if let Some(mds_err) = mds_err { match mds_err { MdsError::Io { .. } | MdsError::FileNotFound { .. } | MdsError::NotMdsFile { .. } => 2, MdsError::ResourceLimit { .. } => 3, @@ -699,8 +707,11 @@ pub(crate) fn compile_to_content( // Stdin: compile from source string using cwd as base_dir. // read_stdin enforces MAX_FILE_SIZE (PF-004). let (source, cwd) = read_stdin()?; + // AD-211-1 / AD-211-5: a string-source compile labels its errors `` + // (resolver's SOURCE_LABEL). Relabel to the uniform CLI sentinel here, at the + // boundary that knows the input was stdin. mds::compile_str_with_deps_opts(&source, Some(&cwd), runtime_vars, opts) - .map_err(miette::Error::from)? + .map_err(|e| crate::output::relabel_stdin_error(&e, &source))? } else { // File path: compile_with_deps_opts routes through the resolver which enforces // MAX_FILE_SIZE and check_symlink (PF-004 compliance). @@ -1160,8 +1171,9 @@ pub(crate) fn run_build(args: BuildArgs) -> Result<()> { .with_source_map_base(source_map_base); let (source, cwd) = read_stdin()?; + // AD-211-1 / AD-211-5: same stdin relabel as `compile_to_content`. let result = mds::compile_str_with_deps_opts(&source, Some(&cwd), runtime_vars, opts) - .map_err(miette::Error::from)?; + .map_err(|e| crate::output::relabel_stdin_error(&e, &source))?; if !quiet { for w in &result.warnings { crate::output::eprint_warning(w); diff --git a/crates/mds-cli/src/fmt.rs b/crates/mds-cli/src/fmt.rs index 4aa7b1c..c00f2ce 100644 --- a/crates/mds-cli/src/fmt.rs +++ b/crates/mds-cli/src/fmt.rs @@ -128,12 +128,19 @@ fn format_source_named( // ── stdin mode ─────────────────────────────────────────────────────────────── fn run_fmt_stdin(flags: FmtFlags) -> Result<()> { + use crate::output::STDIN_DISPLAY_LABEL; + let FmtFlags { check, diff, quiet } = flags; let (source, cwd) = read_stdin()?; - let result = format_source_named(&source, Some(&cwd), "")?; + // AD-211-3: one definition of the stdin sentinel, shared with lint/check/build. + let result = format_source_named(&source, Some(&cwd), STDIN_DISPLAY_LABEL)?; if diff { - print_diff(&render_unified_diff(&source, &result.formatted, ""))?; + print_diff(&render_unified_diff( + &source, + &result.formatted, + STDIN_DISPLAY_LABEL, + ))?; } else if !check { // Plain filter mode: formatted content is the output. write_stdout(&result.formatted)?; @@ -141,7 +148,7 @@ fn run_fmt_stdin(flags: FmtFlags) -> Result<()> { if check && result.changed { if !quiet { - eprintln!("Would reformat: "); + eprintln!("Would reformat: {STDIN_DISPLAY_LABEL}"); } std::process::exit(1); } diff --git a/crates/mds-cli/src/lint.rs b/crates/mds-cli/src/lint.rs index 3d6bb80..b8f3fdf 100644 --- a/crates/mds-cli/src/lint.rs +++ b/crates/mds-cli/src/lint.rs @@ -39,7 +39,8 @@ use crate::build::{ }; use crate::output::{ atomic_write_file, collect_mds_files_detailed, eprint_error, eprint_warning, - render_unified_diff, safe_file_display, safe_inline, safe_path, STDIN_DISPLAY_LABEL, + relabel_stdin_error, render_unified_diff, safe_file_display, safe_inline, safe_path, + STDIN_DISPLAY_LABEL, }; /// Known lint rule names — used to warn about unknown names in mds.json config. @@ -642,14 +643,11 @@ fn run_lint_stdin( let mds_err = MdsError::Io { message: format!("{e}"), }; - // AD-211-5: config errors (MdsError::Io) have no embedded NamedSource, - // so stdin_source = Some(...) is a no-op for relabelling purposes but - // keeps the pattern consistent across all stdin failure paths. - emit_analysis_failure_json_or_stderr( - &mds_err, - format, - Some((STDIN_DISPLAY_LABEL, &source)), - ); + // AD-211-5: config errors (MdsError::Io) carry no embedded NamedSource, + // so the relabel is a no-op here. Passed anyway so the envelope rule holds + // for EVERY stdin failure path — a future error variant routed here that + // does carry a source inherits the sentinel instead of needing a new call. + emit_analysis_failure_json_or_stderr(&mds_err, format, Some(&source)); std::process::exit(2); } }; @@ -658,7 +656,7 @@ fn run_lint_stdin( Ok(r) => r, Err(e) => { // AD-211-5: relabel → in the rendered failure envelope. - emit_analysis_failure_json_or_stderr(&e, format, Some((STDIN_DISPLAY_LABEL, &source))); + emit_analysis_failure_json_or_stderr(&e, format, Some(&source)); std::process::exit(mds_error_exit_code(&e)); } }; @@ -1440,79 +1438,24 @@ fn emit_result( } } -// ── Analysis-failure rendering ──────────────────────────────────────────────── - -/// AD-211-5: thin wrapper that overrides `source_code()` to relabel the embedded -/// `NamedSource` in an `MdsError` when rendering analysis failures for stdin input. +/// AD-211-5 (2026-08-12 ruling): this envelope is the single CLI choke-point for +/// **lint's** analysis failures (config load, IO, resolution, parse). When +/// `stdin_source` is `Some(source_text)` the embedded source identity in the rendered +/// output is replaced with [`STDIN_DISPLAY_LABEL`], so every CLI diagnostic context +/// for stdin input uses the uniform sentinel instead of the core's internal +/// `SOURCE_LABEL` (`""`) that `resolve_source_intrinsic` embeds in `MdsError` +/// spans. /// -/// `resolve_source_intrinsic` sets `ctx.file_str = ""` so errors it produces -/// carry `NamedSource::new("", src)`. Replacing it at this boundary (not in -/// core) matches the "sanitize miette inputs, not rendered output" rule (PF-014) and -/// the "relabel at the CLI output boundary" discipline (AD-211-1). +/// **State it as a rule about this envelope, not about stdin:** every +/// `MdsError` reaching this function for a stdin run labels its source ``. +/// Any error later routed here — a config rejection, a new IO failure — inherits +/// that label instead of inventing a second convention. /// -/// Delegates all `Diagnostic` methods to `inner` except `source_code`, which returns -/// the pre-built replacement `NamedSource` (or `None` when the inner error had no -/// embedded source — avoids miette trying to render spans against a missing source). -struct StdinRelabeledError { - inner: MdsError, - /// `Some(named)` when `inner` had embedded source code (so spans still render). - /// `None` when `inner` had no source code (MdsError::Io and similar). - source: Option>, -} - -impl std::fmt::Display for StdinRelabeledError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - std::fmt::Display::fmt(&self.inner, f) - } -} - -impl std::fmt::Debug for StdinRelabeledError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - std::fmt::Debug::fmt(&self.inner, f) - } -} - -impl std::error::Error for StdinRelabeledError {} - -impl miette::Diagnostic for StdinRelabeledError { - fn code<'a>(&'a self) -> Option> { - miette::Diagnostic::code(&self.inner) - } - fn severity(&self) -> Option { - miette::Diagnostic::severity(&self.inner) - } - fn help<'a>(&'a self) -> Option> { - miette::Diagnostic::help(&self.inner) - } - fn url<'a>(&'a self) -> Option> { - miette::Diagnostic::url(&self.inner) - } - fn labels<'a>(&'a self) -> Option + 'a>> { - miette::Diagnostic::labels(&self.inner) - } - fn source_code(&self) -> Option<&dyn miette::SourceCode> { - // Return the relabeled NamedSource only when the inner error actually has - // embedded source code; otherwise return None so miette skips code-frame - // rendering entirely. - self.source.as_ref().map(|s| s as &dyn miette::SourceCode) - } - fn related<'a>(&'a self) -> Option + 'a>> { - miette::Diagnostic::related(&self.inner) - } - fn diagnostic_source(&self) -> Option<&dyn miette::Diagnostic> { - miette::Diagnostic::diagnostic_source(&self.inner) - } -} - -/// AD-211-5 (2026-08-12 ruling): this envelope is the single CLI choke-point for -/// analysis failures (config load, IO, resolution, parse). When `stdin_source` is -/// `Some((display_label, source_text))`, the embedded source identity in the rendered -/// output is replaced with `display_label` (e.g. `STDIN_DISPLAY_LABEL = ""`), -/// so every CLI diagnostic context for stdin input uses the uniform sentinel instead -/// of the core's internal `SOURCE_LABEL` (`""`) that `resolve_source_intrinsic` -/// embeds in `MdsError` spans. This is a pure label swap at the output boundary — -/// core keeps `""` as `ctx.file_str` for non-stdin paths, and the source -/// content used for span rendering is unchanged. +/// The JSON leg needs no relabel and takes none: `MdsError::serialize()` emits +/// `code` / `message` / `help` / `span`, and no `MdsError` `Display` template +/// interpolates `ctx.file_str`, so the source identity never reaches +/// `error.message`. `cli_lint.rs::stdin_analysis_failure_labels_source_as_stdin` +/// pins that on both channels rather than leaving it as an assumption. /// /// For errors from a file source, pass `stdin_source: None`; the error's embedded /// `NamedSource` (which already carries the correct filename) is used as-is. @@ -1521,7 +1464,7 @@ impl miette::Diagnostic for StdinRelabeledError { fn emit_analysis_failure_json_or_stderr( e: &MdsError, format: LintFormat, - stdin_source: Option<(&str, &str)>, + stdin_source: Option<&str>, ) { if format == LintFormat::Json { let envelope = serde_json::json!({ @@ -1536,20 +1479,7 @@ fn emit_analysis_failure_json_or_stderr( // Route through the single render choke point (avoids PF-004 / // architecture-6: hand-rolled sanitize_control_chars bypass). let report = match stdin_source { - Some((label, src)) => { - // AD-211-5: override the embedded NamedSource with the stdin - // sentinel. miette's WithSourceCode wrapper (used by with_source_code) - // returns self.error.source_code().or(Some(&self.source_code)), so the - // inner diagnostic's source_code takes priority. Instead, wrap the - // error in a delegate that REPLACES source_code() with the relabeled - // NamedSource — the same "sanitize inputs, not outputs" discipline as - // named_source_for_render elsewhere (PF-014). - miette::Report::new(StdinRelabeledError { - inner: e.clone(), - source: miette::Diagnostic::source_code(e) - .map(|_| mds::named_source_for_render(label, src)), - }) - } + Some(src) => relabel_stdin_error(e, src), None => miette::Report::from(e.clone()), }; eprint_error(report); diff --git a/crates/mds-cli/src/main.rs b/crates/mds-cli/src/main.rs index 3a55377..fa5c183 100644 --- a/crates/mds-cli/src/main.rs +++ b/crates/mds-cli/src/main.rs @@ -272,13 +272,17 @@ fn run_check( // Single-file / stdin path. if input == std::path::Path::new("-") { let (source, cwd) = read_stdin()?; + // AD-211-1 / AD-211-5: a string-source check labels its errors `` + // (resolver's SOURCE_LABEL). Relabel to the uniform CLI sentinel here, at the + // boundary that knows the input was stdin. let ((), warnings) = mds::check_str_collecting_warnings(&source, Some(&cwd), runtime_vars) - .map_err(miette::Error::from)?; + .map_err(|e| output::relabel_stdin_error(&e, &source))?; if !quiet { for w in &warnings { output::eprint_warning(w); } - eprintln!("OK: "); + // AD-211-3: one definition of the sentinel, shared with lint/build/fmt. + eprintln!("OK: {}", output::STDIN_DISPLAY_LABEL); } } else { let ((), warnings) = diff --git a/crates/mds-cli/src/output.rs b/crates/mds-cli/src/output.rs index 37a413a..bee6f87 100644 --- a/crates/mds-cli/src/output.rs +++ b/crates/mds-cli/src/output.rs @@ -36,10 +36,113 @@ use crate::build::{MdsConfig, OutputKind}; /// (STRING_SOURCE_MAP_LABEL) as the internal VFS entry key, which is NOT changed. /// /// Centralised here so the CLI has exactly one definition of the sentinel (AD-211-3), -/// replacing the five previous scattered literals including the hardcoded `""` -/// in `build.rs:993`. +/// replacing the previously scattered literals — including the hardcoded `""` +/// in `apply_source_map_file_label`, the `OK: ` status line in `main.rs`, and +/// the `fmt` stdin label. pub(crate) const STDIN_DISPLAY_LABEL: &str = ""; +// ── Stdin source-identity relabel (AD-211-5) ───────────────────────────────── + +/// Render-boundary wrapper that replaces the source identity embedded in an +/// [`mds::MdsError`] with [`STDIN_DISPLAY_LABEL`]. +/// +/// `resolve_source_intrinsic` sets `ctx.file_str = ""`, so every error a +/// string-source (stdin) analysis produces carries `NamedSource::new("", …)` +/// and renders as `:L:C`. Replacing it here — not in `crates/mds-core` — +/// keeps the core constant intact for the non-stdin paths that legitimately use it +/// (`resolver_tests.rs` locks `SOURCE_LABEL`) and matches the "relabel at the CLI +/// output boundary" discipline of AD-211-1. +/// +/// It also matches PF-014: the swap happens on the miette **input** (the +/// `NamedSource` handed to the renderer), never on already-rendered output. +/// +/// Delegates every `Diagnostic` method to `inner` except `source_code`, which +/// returns the pre-built replacement (or `None` when the inner error carried no +/// embedded source, so miette skips code-frame rendering rather than trying to +/// resolve spans against a source that is not there). +/// +/// # Exit codes +/// +/// This type is transparent to [`crate::build::exit_code`], which unwraps it before +/// classifying the error. A wrapped `MdsError::FileNotFound` must still exit 2, not +/// 1 — see the downcast ladder there. +pub(crate) struct StdinRelabeledError { + inner: mds::MdsError, + /// `Some(named)` when `inner` had embedded source code (so spans still render). + /// `None` when `inner` had no source code (`MdsError::Io` and similar). + source: Option>, +} + +impl StdinRelabeledError { + /// The error this wrapper renders. Used by `exit_code` so wrapping cannot + /// change a process exit status. + pub(crate) fn inner(&self) -> &mds::MdsError { + &self.inner + } +} + +impl std::fmt::Display for StdinRelabeledError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + std::fmt::Display::fmt(&self.inner, f) + } +} + +impl std::fmt::Debug for StdinRelabeledError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + std::fmt::Debug::fmt(&self.inner, f) + } +} + +impl std::error::Error for StdinRelabeledError {} + +impl miette::Diagnostic for StdinRelabeledError { + fn code<'a>(&'a self) -> Option> { + miette::Diagnostic::code(&self.inner) + } + fn severity(&self) -> Option { + miette::Diagnostic::severity(&self.inner) + } + fn help<'a>(&'a self) -> Option> { + miette::Diagnostic::help(&self.inner) + } + fn url<'a>(&'a self) -> Option> { + miette::Diagnostic::url(&self.inner) + } + fn labels<'a>(&'a self) -> Option + 'a>> { + miette::Diagnostic::labels(&self.inner) + } + fn source_code(&self) -> Option<&dyn miette::SourceCode> { + self.source.as_ref().map(|s| s as &dyn miette::SourceCode) + } + fn related<'a>(&'a self) -> Option + 'a>> { + miette::Diagnostic::related(&self.inner) + } + fn diagnostic_source(&self) -> Option<&dyn miette::Diagnostic> { + miette::Diagnostic::diagnostic_source(&self.inner) + } +} + +/// AD-211-5: build a report whose embedded source identity reads +/// [`STDIN_DISPLAY_LABEL`] instead of the core's `""`. +/// +/// This is a pure label swap — the source text used for span rendering, the +/// message, the code, the help and the labels are all untouched. `miette`'s own +/// [`miette::Report::with_source_code`] cannot do this: its `WithSourceCode` +/// wrapper returns `self.error.source_code().or(Some(&self.source_code))`, so an +/// inner diagnostic that already carries a `NamedSource` (which these do) wins and +/// the replacement is ignored. +/// +/// Call this at every CLI boundary that renders an analysis failure for stdin +/// input — `lint`, `check` and `build` all reach the same core errors, so a leg +/// that skips it renders `` and breaks the uniform-sentinel rule. +pub(crate) fn relabel_stdin_error(e: &mds::MdsError, source: &str) -> miette::Report { + miette::Report::new(StdinRelabeledError { + inner: e.clone(), + source: miette::Diagnostic::source_code(e) + .map(|_| mds::named_source_for_render(STDIN_DISPLAY_LABEL, source)), + }) +} + // ── Output base for directory mode ──────────────────────────────────────────── /// Describes where directory-mode output files are written. diff --git a/crates/mds-cli/tests/cli_lint.rs b/crates/mds-cli/tests/cli_lint.rs index e68aa1e..39f3829 100644 --- a/crates/mds-cli/tests/cli_lint.rs +++ b/crates/mds-cli/tests/cli_lint.rs @@ -1321,39 +1321,426 @@ fn stdin_json_wire_file_key_is_stdin_sentinel() { "AC-P1-01: JSON files[].file must be '' for stdin input, not '{file_key}'" ); } + // AC-P1-01/AC-P1-27, negative half: the internal VFS key must not leak anywhere + // in the document, not just in the key this test read. + assert!( + !stdout.contains("input.mds"), + "AC-P1-27: 'input.mds' must not appear anywhere in CLI stdout for a stdin \ + lint; got: {stdout}" + ); } -// ── AC-P1-14/#202: JSON wire diagnostics sorted by byte offset ─────────────── +// ── AC-P1-08/#202: JSON wire diagnostics sorted by byte offset ─────────────── // // Pins issue #202: within a file group, diagnostics must appear in ascending -// byte-offset order regardless of the order rules were applied. +// byte-offset order regardless of the order the rules were applied in. + +/// Fixture whose OFFSET order is the reverse of its RULE-EXECUTION order. +/// +/// `run_rules` (crates/mds-core/src/lint/mod.rs) dispatches `duplicate_export` +/// fifth and `legacy_interpolation` tenth — so without a sort, `duplicate-export` +/// (the LATER offset) is emitted first. `{name}` on line 2 is a legacy +/// single-brace interpolation at a low offset; the repeated `@export greet` at the +/// end is a duplicate export at a high offset. +/// +/// A fixture whose diagnostics are already ascending cannot detect the sort being +/// removed — the assertion would hold either way. +const OUT_OF_ORDER_FIXTURE: &str = + "@define greet(name):\n Hello {name}!\n@end\n\n@export greet\n@export greet\n"; + +fn stdin_json_diagnostics(source: &str) -> Vec { + let out = lint_stdin(source, &["--format", "json"]); + let stdout = String::from_utf8_lossy(&out.stdout); + let v: serde_json::Value = serde_json::from_str(&stdout) + .unwrap_or_else(|e| panic!("stdout must be JSON: {e}\n{stdout}")); + let files = v["files"].as_array().expect("JSON must have 'files' array"); + assert_eq!( + files.len(), + 1, + "stdin lint must emit exactly one file group" + ); + files[0]["diagnostics"] + .as_array() + .expect("must have 'diagnostics'") + .clone() +} #[test] fn stdin_json_diagnostics_sorted_by_offset() { - // Two diagnostics at different offsets: the one at the lower offset must - // come first. Use a source with two distinct export violations placed at - // known positions. - let source = "@define greet(name):\n Hello {{name}}!\n@end\n\n@export greet\n@export greet\n"; - let out = lint_stdin(source, &["--format", "json"]); + let diags = stdin_json_diagnostics(OUT_OF_ORDER_FIXTURE); + + let rules: Vec<&str> = diags.iter().filter_map(|d| d["rule"].as_str()).collect(); + let offsets: Vec = diags + .iter() + .filter_map(|d| d["span"]["offset"].as_i64()) + .collect(); + + // Non-vacuity guard: this assertion is meaningless on a single diagnostic, and + // meaningless if the two land on the same offset. + assert_eq!( + offsets.len(), + diags.len(), + "every diagnostic in this fixture must carry a span; got: {diags:?}" + ); + assert!( + offsets.len() >= 2 && offsets[0] != offsets[offsets.len() - 1], + "AC-P1-08 needs at least two diagnostics at DISTINCT offsets to be a real \ + check; got rules {rules:?} at offsets {offsets:?}" + ); + + let mut sorted = offsets.clone(); + sorted.sort_unstable(); + assert_eq!( + offsets, sorted, + "AC-P1-08: diagnostics must be in ascending byte-offset order; \ + got rules {rules:?} at offsets {offsets:?}" + ); + + // The positive control: `duplicate_export` runs BEFORE `legacy_interpolation` + // in run_rules but fires at the LATER offset, so it must appear LATER in the + // array. Delete the sort in LintResultBuilder::build and this flips. + let legacy = rules + .iter() + .position(|r| *r == "legacy-interpolation") + .expect("fixture must produce a legacy-interpolation diagnostic"); + let dup = rules + .iter() + .position(|r| *r == "duplicate-export") + .expect("fixture must produce a duplicate-export diagnostic"); + assert!( + legacy < dup, + "AC-P1-08: emitted order must follow byte offset, not rule-dispatch order \ + (duplicate_export is dispatched first but fires later in the file); \ + got rules {rules:?} at offsets {offsets:?}" + ); +} + +/// AC-P1-09: the human renderer must present diagnostics in the same order as the +/// JSON renderer — proving the sort lives on `LintResult.diagnostics` and not in +/// one renderer (PF-007: a per-surface assertion could not show this). +#[test] +fn stdin_human_and_json_diagnostic_order_match() { + let json_rules: Vec = stdin_json_diagnostics(OUT_OF_ORDER_FIXTURE) + .iter() + .filter_map(|d| d["rule"].as_str().map(str::to_string)) + .collect(); + // Non-vacuity guard: two empty sequences compare equal and prove nothing. + assert!( + json_rules.len() >= 2, + "AC-P1-09 needs at least two diagnostics to compare an ORDER; got {json_rules:?}" + ); + + let out = lint_stdin(OUT_OF_ORDER_FIXTURE, &[]); + let stderr = String::from_utf8_lossy(&out.stderr); + // Rule names appear in the miette `code` line of each rendered diagnostic. + let human_rules: Vec = stderr + .lines() + .filter_map(|line| { + let t = line.trim(); + json_rules + .iter() + .find(|r| t == format!("mds::lint::{r}") || t == **r) + .cloned() + }) + .collect(); + + assert_eq!( + human_rules, json_rules, + "AC-P1-09: human and JSON surfaces must agree on diagnostic order.\n\ + json: {json_rules:?}\nhuman: {human_rules:?}\nstderr:\n{stderr}" + ); +} + +/// AC-P1-11: repeated lints of identical input are byte-identical. +#[test] +fn stdin_json_output_is_byte_identical_across_runs() { + let first = lint_stdin(OUT_OF_ORDER_FIXTURE, &["--format", "json"]).stdout; + for run in 2..=5 { + let next = lint_stdin(OUT_OF_ORDER_FIXTURE, &["--format", "json"]).stdout; + assert_eq!( + first, + next, + "AC-P1-11: run {run} differed from run 1.\nrun1: {}\nrun{run}: {}", + String::from_utf8_lossy(&first), + String::from_utf8_lossy(&next) + ); + } +} + +// ── AC-P1-07 / AD-211-5: the analysis-failure envelope labels stdin `` ── + +/// Pull the source identity out of a miette code-frame header, e.g. the +/// `` in `[:1:1]`. +/// +/// Returning `Option` and requiring the caller to unwrap keeps this from passing +/// vacuously: a rendering that emitted no frame at all yields `None` and fails, +/// rather than silently satisfying a "does not contain ``" assertion +/// (PF-013). +fn frame_source_identity(rendered: &str) -> Option { + let start = rendered.find('[')?; + let rest = &rendered[start + 1..]; + let end = rest.find(']')?; + let inner = &rest[..end]; + // `