diff --git a/.devflow/features/mds-lint/KNOWLEDGE.md b/.devflow/features/mds-lint/KNOWLEDGE.md index 2c7bc283..c0f53e17 100644 --- a/.devflow/features/mds-lint/KNOWLEDGE.md +++ b/.devflow/features/mds-lint/KNOWLEDGE.md @@ -107,7 +107,12 @@ The `fixable` flag in the canonical JSON output is computed as `(fix_removals.is { "lint": { "rules": { "unused-variable": "off", "shadow-variable": "warn" } } } ``` -**Unknown rule NAMEs** → warn-and-ignore at CLI (forward compat). +**Unknown rule NAMEs** → warned about on every surface, then ignored; the config still +loads and lint continues, and the exit code does not move (forward compat: a config naming +a rule from a newer release must not break an older binary). The CLI writes the warning to +stderr (suppressed by `--quiet`); napi/WASM/Python return it in `lint_warnings`. +The registry is `mds::KNOWN_LINT_RULES`, derived from each rule module's own `RULE` const; +detection is `mds::find_unknown_rule_names`. (#224) **Unknown severity VALUES** → hard parse error → exit 2 (closed enum, no sensible fallback). `LintConfig` lives in `mds-core` (not mds-cli). The CLI `LintCliConfig` from `build.rs` converts to it via `into_core_config()`. @@ -497,7 +502,9 @@ LintDiagnostic.fix_removals (FixLineSpan) OR .fix_edits (TextEdit) **shadow-variable is info AND default-off**: Only fires when explicitly configured. `Info` findings never contribute to the exit code. -**Unknown rule NAMES vs unknown severity VALUES behave differently**: An unknown rule name in `mds.json` is warned about and ignored. An unknown severity value fails loudly with a serde deserialization error (exits 2). +**Unknown rule NAMES vs unknown severity VALUES behave differently**: An unknown rule name is warned about and then ignored — on every surface, not just the CLI — and the run continues with an unchanged exit code and an unchanged JSON envelope. An unknown severity value fails loudly with a serde deserialization error (exits 2). The asymmetry is deliberate: severities are a closed set, rule names grow every release. + +**`.devflow/features/*/KNOWLEDGE.md` is TRACKED, not gitignored**: `.gitignore` ignores `.devflow/*` but re-includes `!.devflow/features/*/KNOWLEDGE.md` (lines 64-70). A doc sweep that excludes `.devflow` wholesale will miss this file, and the source-hygiene gate does scan it. **D2 mechanical ripple in resolver.rs**: The `..` in the three `ExportDirective` match arms in `resolver.rs` is intentional — it acknowledges the new `offset` field without reading it. @@ -546,7 +553,7 @@ LintDiagnostic.fix_removals (FixLineSpan) OR .fix_edits (TextEdit) - `crates/mds-core/src/lint/config.rs` — `LintConfig` (lives in mds-core; CLI converts to it) - `crates/mds-core/src/ast.rs` — `ElseifBranch { offset }`, `IfBlock { else_offset, end_offset }`, `ForBlock/DefineBlock { end_offset }` - `crates/mds-core/src/lint/rules/` — 10 rule modules + `structural_eq.rs` -- `crates/mds-cli/src/lint.rs` — CLI subcommand; `render_diag_human` (HUMAN for message/help; filename+source via `named_source_for_render`; all status lines via `safe_path`); `set_diag_display_path`, `LintDirCtx`, `KNOWN_RULES` +- `crates/mds-cli/src/lint.rs` — CLI subcommand; `render_diag_human` (HUMAN for message/help; filename+source via `named_source_for_render`; all status lines via `safe_path`); `set_diag_display_path`, `LintDirCtx`; the rule-name list lives in `mds::KNOWN_LINT_RULES`, not in this crate (#224) - `crates/mds-cli/src/output.rs` — `atomic_write_file`; `eprint_error` (single CLI stderr choke-point, wraps in `SanitizedReport`); `SanitizedReport` / `SanitizedNode` / `MAX_AUX_DEPTH`; `render_error_sanitized` (private, plain `format!("{report:?}")` on sanitized wrapper); `eprint_warning` (HUMAN, new); `safe_path` / `safe_file_display` / `safe_inline` (all WIRE, new); `preview_text_for` (TTY-gated source neutralization for `--diff`); `render_unified_diff` / `colorize_unified_diff` - `crates/mds-cli/src/build.rs` — `LintCliConfig` struct, `into_core_config()`, `MdsConfig.lint` field - `crates/mds-cli/src/watch.rs` — all 11 error prints route through `eprint_error`; lifecycle status lines route through `safe_path` / `safe_inline` / `eprint_warning` diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d0570d5c..90b7bd7a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -115,6 +115,14 @@ jobs: # after removing redundant to_canonical_json re-sort in 56424f7). # Guard NOT raised: 16,498 bytes (1.94%) headroom. CI uses # Binaryen v129 (distinct toolchain from local). (AC-P1-23) + # ticket/pr2-unknown-rule-names (2026-08-14, #224): unknown-rule-name warning + # engine (find_unknown_rule_names, format_unknown_rule_names_warning, + # attach_lint_warnings; three duplicated binding copies deleted) plus CLI + # warning body unification (ba117a6) added net +10,670 bytes from PR1 + # baseline; PR1 baseline 833,502, post-change 844,172 (wasm-pack 0.15.0 + # bundled wasm-opt, nodejs target, measured locally at HEAD ba117a6). + # Guard NOT raised: 5,828 bytes (0.69%) headroom. CI uses Binaryen v129 + # (distinct toolchain from local). Three more wave PRs still to land. (AC-224-18) # Follow-up: pin the wasm build toolchain to make the size deterministic # and re-tighten this guard. if [ "$raw" -gt 850000 ]; then diff --git a/CHANGELOG.md b/CHANGELOG.md index f680ca88..494ac8c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -529,6 +529,32 @@ diagnostic messages must update to check for the `\uXXXX` literal form instead. ### Added +- **Lint rule-name registry, exposed on every surface (#224).** The recognised rule + names now have one source of truth, derived from each rule module's own name constant. + - `mds-core`: `KNOWN_LINT_RULES: &[&str]` (the canonical slice), + `find_unknown_rule_names(&HashMap) -> Option` + (`None` when every name is recognised), `UnknownRuleNames` (a `#[non_exhaustive]` + report with a `names() -> &[String]` accessor, always non-empty and sorted), and + `format_unknown_rule_names_warning(&UnknownRuleNames) -> String`. The formatter takes + the report type rather than a slice so its non-empty precondition is structural — it + has no panic path — and it WIRE-escapes each name before interpolating it. + - `mds-core`: `LintConfig::from_rules_checked(HashMap) -> (LintConfig, + Option)` — the preferred constructor. It returns the config and the + unknowns report in one `#[must_use]` call so a caller cannot silently skip detection. + `LintConfig::from_rules` is retained but **deprecated since 0.4.0** in its favour; it + still behaves exactly as before (it never fails on an unknown name) and is not removed. + - `mds-core`: `attach_lint_warnings(&mut serde_json::Map, Option)` + — the single definition of the `lint_warnings` wire contract (key name, `string[]` + shape, absent-when-empty) shared by the napi, WASM, and Python bindings. It takes a + `&mut Map` rather than a `&mut Value` so the "target is a JSON object" precondition is + structural rather than a silent no-op on a non-object. + - `@mdscript/mds` (Node entry point): `LINT_RULE_NAMES: readonly LintRuleName[]` and + the `LintRuleName` string-union type. The browser entry point does not export them + yet — it has no lint API to configure. + - TypeScript `LintResult` gains `lint_warnings?: string[]`. + - Python `LintResult` gains a `.lint_warnings` property returning `list[str]` (empty + when there is nothing to report); the type stub is updated to match. + - **`--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`). @@ -560,9 +586,11 @@ diagnostic messages must update to check for the `\uXXXX` literal form instead. Cross-platform wheel matrix and PyPI publishing are a tracked follow-up (#132) — for now, install from source: `pip install ./crates/mds-python`. (#59) -- **`mds lint`** — 9-rule static analyzer for `.mds` templates (#61). Available - across all surfaces (CLI, Rust, napi, WASM, Python) with byte-identical canonical - JSON output. +- **`mds lint`** — 10-rule static analyzer for `.mds` templates (#61). Available + across all surfaces (CLI, Rust, napi, WASM, Python). The per-file and + per-diagnostic canonical JSON payload is byte-identical across all surfaces; + binding surfaces (napi, WASM, Python) additionally expose a `lint_warnings` + channel absent from the CLI surface (see #224 in this block). **Rules** (individually configurable via `mds.json` `lint.rules` or the `rules` API option; severities differ per rule): @@ -755,6 +783,48 @@ diagnostic messages must update to check for the `\uXXXX` literal form instead. ### Changed +- **Unknown lint rule names now emit a warning instead of being silently ignored + (#224).** Previously an unrecognised rule name in `mds.json`'s `lint.rules` object + (or in the `rules` option on a binding surface) was silently accepted: the rule had + no effect and nothing signalled that the key was misconfigured. Now the unknown name + is reported and linting continues — **exit codes are unchanged**, the JSON envelope on + stdout is unchanged, and the rule is still not enforced (it does not exist). This + surfaces typos without hard-failing a config that names a rule added in a newer + release. + - **CLI**: the warning goes to **stderr**, never stdout, so `mds lint --format json` + still writes a single valid JSON document. `--quiet` suppresses it. Singular and + plural formats (offenders sorted lexicographically): + - `warning: in mds.json: unknown lint rule 'NAME'; recognised rules are: …; ignoring` + - `warning: in mds.json: unknown lint rules: 'A', 'B'; recognised rules are: …; ignoring` + - **napi / WASM / Python**: the warning is surfaced as `lint_warnings: string[]` on + the lint result. In the JSON wire form and in `to_dict()` / `to_json()` output, the + key is absent (not `null`, not `[]`) when no warnings occurred. On the Python + live-object surface, `LintResult.lint_warnings` is a property that always exists + and returns an empty list when no warnings occurred. The message body is shared with + the CLI via `mds::format_unknown_rule_names_warning` (AC-224-3 met); the only + per-surface difference is that the CLI prefixes `"warning: in mds.json: "` to carry + the source-file provenance on stderr, while the bindings use the body as-is: + - Singular: `unknown lint rule 'NAME'; recognised rules are: …; ignoring` + - Plural: `unknown lint rules: 'A', 'B'; recognised rules are: …; ignoring` + The recognised-rules list, sort order, and name wire-escaping are all shared. + Per-surface parity (PF-007): each surface's format is asserted by its own tests. + - Only `mds lint` reads `lint.rules`, so only `mds lint` warns. `mds build`, + `mds fmt `, and `watch` read `mds.json` via `load_config` but deserialize + the `lint` field without calling `load_lint_config` — an accepted D2(a) + asymmetry, not an oversight (see build.rs:49-51). `mds check` and `mds fmt + ` do not call `load_config` at all. The D2(a) invariant is held in CI by + the `build_unknown_lint_rule_in_mds_json_emits_no_warning` and + `fmt_unknown_lint_rule_in_mds_json_emits_no_warning` tests in `cli_build.rs`, + each with a positive-control arm (unknown severity causes non-zero exit, proving + `load_config` was reached). Those tests mechanically hold the AC-224-14 + watch-path invariant: `watch.rs:822` calls `load_config(...).unwrap_or(None)`; + because `build` and `mds fmt ` share the same `load_config` implementation, + a passing build or dir-fmt proves `load_config` returns `Ok` for configs with + unknown rule names, so `unwrap_or(None)` cannot collapse `output_dir` to `None` + on account of an unknown lint rule name alone. + - Unknown **severity values** continue to hard-fail with `mds::invalid_options`. The + asymmetry is deliberate: severities are a closed set, rule names grow every release. + - **napi and Python `compileFile` / `compile_file` now emit root-relative `sources[]`** in Source Map v3 output. Previously these surfaces emitted the absolute filesystem path as `sources[0]` (e.g. `/home/user/project/src/foo.mds`); diff --git a/README.md b/README.md index 923b6382..19c4fb0e 100644 --- a/README.md +++ b/README.md @@ -80,7 +80,7 @@ mds build [FILE|DIR] [OPTIONS] Compile an MDS template or directory to Markdown mds watch [FILE|DIR] [OPTIONS] Watch and auto-recompile on save mds check [FILE|DIR] [OPTIONS] Validate without rendering mds fmt [FILE|DIR] [OPTIONS] Reformat MDS file(s) in place (opinionated, safety-gated) -mds lint [FILE|DIR] [OPTIONS] Static-analysis lint (9 rules; --fix, --format json) +mds lint [FILE|DIR] [OPTIONS] Static-analysis lint (10 rules; --fix, --format json) mds init [FILENAME] Create a starter MDS file Global options: @@ -212,7 +212,7 @@ any formatting behavior yet; frontmatter key sorting is deferred to a future ver ### Static analysis with `mds lint` -A 9-rule static analyzer that catches common template authoring issues: +A 10-rule static analyzer that catches common template authoring issues: ```bash mds lint template.mds # lint a single file @@ -231,6 +231,7 @@ Rules (configure via `mds.json` `lint.rules`; severities differ per rule): | `unused-function` | warn | `@define` function that is never called (Tier B: auto-fixed only for standalone files) | | `shadow-variable` | off/info | Inner-scope variable shadows an outer-scope variable (must be enabled via `mds.json`) | | `empty-block` | warn | `@if`/`@elseif`/`@else`/`@for`/`@define`/`@message` body is empty or whitespace-only (auto-fixable) | +| `legacy-interpolation` | warn | Single-brace `{x}` syntax from MDS v0.x; migrates to `{{x}}` automatically (auto-fixable) | | `redundant-else` | warn | `@else` body is structurally identical to the `@if`/`@elseif` then-body | | `unreachable-branch` | **error** | Branch condition is always-true or always-false (auto-fixable) | | `duplicate-import` | **error** | Same file imported more than once (auto-fixable) | diff --git a/crates/mds-cli/src/build.rs b/crates/mds-cli/src/build.rs index 17e6cdb6..051014a8 100644 --- a/crates/mds-cli/src/build.rs +++ b/crates/mds-cli/src/build.rs @@ -32,7 +32,10 @@ pub(crate) struct MdsConfig { /// Per-rule severity overrides for `mds lint` (AC-F-17). /// /// Unknown severity VALUES fail config loading loudly (closed enum). - /// Unknown rule NAMES are preserved for forward compat (CLI warns and ignores). + /// Unknown rule NAMES: only `mds lint` warns on stderr and continues — single-file + /// mode via `load_lint_config`, directory mode via `LintDirCtx::config_for`. + /// `mds build`, `check`, `fmt`, and `watch` load this field but do not emit the + /// warning — an accepted asymmetry, not an oversight (see CHANGELOG). #[serde(default)] pub(crate) lint: LintCliConfig, } @@ -44,7 +47,10 @@ pub(crate) struct MdsConfig { /// /// Unknown severity VALUES (e.g. `"banana"`) cause a hard parse error (exit 2) /// because `Severity` is a closed enum with no sensible fallback. Unknown rule -/// NAMES are warn-and-ignored at the CLI layer (forward compat). +/// NAMES: only `mds lint` warns on stderr and continues — single-file mode via +/// `load_lint_config`, directory mode via `LintDirCtx::config_for`. `mds build`, +/// `check`, `fmt`, and `watch` deserialize this struct but do not emit the +/// warning — an accepted asymmetry, not an oversight (see CHANGELOG). #[derive(Debug, Default, Deserialize)] pub(crate) struct LintCliConfig { #[serde(default)] @@ -52,9 +58,21 @@ pub(crate) struct LintCliConfig { } impl LintCliConfig { - /// Convert to the core `LintConfig` consumed by `mds::lint_*` functions. - pub(crate) fn into_core_config(self) -> mds::LintConfig { - mds::LintConfig::from_rules(self.rules) + /// Convert to the core `LintConfig` consumed by `mds::lint_*` functions, + /// returning any unknown rule names alongside it. + /// + /// Uses [`mds::LintConfig::from_rules_checked`] so the caller receives both + /// the config and the unknowns report in one step, rather than building the + /// config and optionally invoking a separate check. This closes the gap + /// identified in the review finding for config.rs:104 — a consumer could + /// previously skip detection silently by not invoking the separate check. + /// Note: `#[must_use]` on `from_rules_checked` fires only when the entire + /// return value is dropped; `let (config, _) = …` silently discards the + /// `Option` and is the caller's own choice (as + /// config.rs:249 states: the `#[must_use]` only warns "if the return value + /// is discarded entirely"). + pub(crate) fn into_core_config(self) -> (mds::LintConfig, Option) { + mds::LintConfig::from_rules_checked(self.rules) } } diff --git a/crates/mds-cli/src/lint.rs b/crates/mds-cli/src/lint.rs index dafc09bf..773b96ed 100644 --- a/crates/mds-cli/src/lint.rs +++ b/crates/mds-cli/src/lint.rs @@ -43,19 +43,9 @@ use crate::output::{ STDIN_DISPLAY_LABEL, }; -/// Known lint rule names — used to warn about unknown names in mds.json config. -const KNOWN_RULES: &[&str] = &[ - "unused-variable", - "unused-import", - "unused-function", - "shadow-variable", - "empty-block", - "redundant-else", - "unreachable-branch", - "duplicate-import", - "duplicate-export", - "legacy-interpolation", -]; +// AC-224-15: No local rule-name list. The single source of truth is +// mds::KNOWN_LINT_RULES (composed from each rule module's own RULE const). +// No rule-name string literals from the registry appear in this directory. pub(crate) struct LintArgs { pub(crate) input: Option, @@ -187,37 +177,72 @@ fn do_lint(args: LintArgs) -> Result<()> { // ── Config helpers ──────────────────────────────────────────────────────────── /// Load mds.json and extract the core `LintConfig`, warning about unknown rule names. -fn load_lint_config(dir: &Path) -> Result { +/// +/// AD-224-1 (2026-08-12 ruling): an unknown rule name WARNS and lint CONTINUES. +/// The domain is asymmetric — severities are a closed set but rule names grow +/// every release — so hard-failing would break configs naming rules from a newer +/// mds when run with an older binary. +/// +/// AD-224-5 (AC-224-21, AC-224-22): the warning goes to STDERR only (never +/// stdout — `--format json` stdout must remain valid parseable JSON), and is +/// SUPPRESSED under `--quiet` (AC-224-22, coordination point with PR4 D4): the +/// unknown-rule warning is never the causal reason for a non-zero exit — an +/// unknown rule has no enforcement and does not affect `result_exit_code`, which +/// counts actual lint findings, not config anomalies. A `--quiet` consumer who +/// receives a non-zero exit will always have a visible, causal lint finding. +/// `crates/mds-cli/src/main.rs:30` documents `--quiet` as suppressing +/// *"status and diagnostic output"*; this warning is status, not an error. +fn load_lint_config(dir: &Path, quiet: bool) -> Result { let config_opt = load_config(dir)?; match config_opt { None => Ok(mds::LintConfig::default()), Some((mds_config, _config_dir)) => { - // Warn on unknown rule names — unknown NAMES are ignored for forward compat. - // - // `name` is an arbitrary attacker-supplied JSON object key: `mds.json` is - // read from the working tree, and JSON `\uXXXX` escapes decode to real - // control bytes. - // - // Two escapes, deliberately different (spec §7.5 per-field rule): the warning - // PROSE goes through `eprint_warning` (HUMAN — it is the message body), while - // the rule NAME goes through `safe_inline` (WIRE). Routing the whole line - // through `eprint_warning` alone was NOT sufficient: HUMAN mode preserves - // `\n`, so a rule name of `x\nClean: totally-real.mds\n0 problems found\n` - // still emitted three standalone lines byte-identical to genuine status - // output (CWE-117). A JSON object key is never legitimately multi-line. - for name in mds_config.lint.rules.keys() { - if !KNOWN_RULES.contains(&name.as_str()) { - eprint_warning(&format!( - "warning: unknown lint rule '{}' in mds.json; ignoring", - safe_inline(name) - )); - } + // into_core_config returns (config, Option) in one step — + // structurally forcing the caller to handle the unknowns report so detection + // cannot be accidentally skipped (review finding at config.rs:104). + // The safe_inline / print-discipline contract (AD-224-3, AC-224-6) and the + // --quiet gate (AD-224-5, AC-224-22) are documented at `emit_unknown_rule_warning`. + let (lint_config, unknown) = mds_config.lint.into_core_config(); + if let Some(unknown) = unknown { + emit_unknown_rule_warning(&unknown, quiet); } - Ok(mds_config.lint.into_core_config()) + Ok(lint_config) } } } +/// Emit the unknown-rule warning for one config load, suppressed under `--quiet`. +/// +/// Both `load_lint_config` (stdin/single-file path) and `LintDirCtx::config_for` +/// (directory path) call this function. One definition prevents the duplication +/// that already drifted once inside this PR on the CWE-117 escape path (PF-009: +/// the same work set represented twice drifts; ADR-008: the escape contract is +/// per-file, so every call site is equally security-relevant). +/// +/// AC-224-3: delegates the message body to [`mds::format_unknown_rule_names_warning`] +/// so the CLI and all binding surfaces share one canonical phrasing. The CLI adds +/// `"warning: in mds.json: "` as a prefix to carry the source-file provenance — +/// the ONLY structural difference from the binding surface format. +/// +/// AD-224-3 (AC-224-6): every value interpolated inside `eprint_warning`'s +/// `format!` must be a WHOLE-EXPRESSION `safe_inline` call — the one shape that +/// `print_discipline.rs`'s trace accepts without an allowlist entry. Keeping the +/// `eprint_warning` call here (in a named function inside `crates/mds-cli/src/`) +/// preserves that machine-checked coverage: the scanner enumerates every `.rs` +/// file under `src/` and checks every `eprint_warning(…)` call site it finds. +/// `safe_inline(&core_msg)` satisfies that constraint; the call is idempotent +/// because names are already wire-escaped inside `format_unknown_rule_names_warning`. +/// +/// AD-224-5 (AC-224-22): no-op when `quiet` is true. +fn emit_unknown_rule_warning(unknown: &mds::UnknownRuleNames, quiet: bool) { + if quiet { + return; + } + // AC-224-3: shared message body; "in mds.json:" prefix carries source provenance. + let core_msg = mds::format_unknown_rule_names_warning(unknown); + eprint_warning(&format!("warning: in mds.json: {}", safe_inline(&core_msg))); +} + // ── Display-path remap ──────────────────────────────────────────────────────── /// Remap the `file` field in every diagnostic in `result` to `display`. @@ -686,7 +711,7 @@ fn run_lint_stdin( let (source, cwd) = read_stdin()?; // mds.json load/parse failure → JSON envelope in --format json mode (AC-F-14). - let config = match load_lint_config(&cwd) { + let config = match load_lint_config(&cwd, quiet) { Ok(c) => c, Err(e) => { let mds_err = MdsError::Io { @@ -831,7 +856,7 @@ fn run_lint_file( // effective_parent maps "" (bare filename) to "." — avoids PF-006. let base_dir = effective_parent(path); // mds.json load/parse failure → JSON envelope in --format json mode (AC-F-14). - let config = match load_lint_config(base_dir) { + let config = match load_lint_config(base_dir, quiet) { Ok(c) => c, Err(e) => { let mds_err = MdsError::Io { @@ -984,41 +1009,120 @@ fn run_lint_file( /// (issue #6 / zero-warnings policy). Pattern mirrors `FileCompileCtx` / `DirWatchCtx` /// in watch.rs. /// -/// `config_cache` maps a directory path to the resolved `LintConfig` for that directory. -/// The cache avoids re-loading the config for every file in the same directory; the -/// `RefCell` provides interior mutability so per-file helpers can populate it through -/// a shared `&LintDirCtx` reference (A6 / D20 decision). +/// Two caches serve two independent fast paths into the config resolution logic. +/// They are kept SEPARATE so the key namespaces never collide: a directory that +/// happens to contain an `mds.json` would appear as a `base_dir` key in one run +/// and as a `config_dir` key in another, and a single map would conflate them. +/// +/// - **Fast path 1** (`base_dir_cache`): if a file's parent directory has already +/// been resolved, return the cached config without re-walking the ancestor chain. +/// - **Fast path 2** (`config_dir_cache`): if a DIFFERENT `base_dir` resolves to +/// the SAME `mds.json`, return the cached config and skip emitting a duplicate +/// warning (AC-224-19: "at most once per distinct config directory"). +/// +/// NOTE: `load_config(base_dir)` is called BEFORE consulting `config_dir_cache`, +/// so the file I/O and ancestor walk still happen for each new `base_dir` — fast +/// path 2 prevents the DUPLICATE WARNING, not the re-parse. For a tree of N files +/// under one directory, fast path 1 amortises the cost to a single read (the common +/// case). The AC-224-19 threshold (200 files, one directory) is met by fast path 1. +/// +/// The `RefCell` provides interior mutability so per-file helpers can populate the +/// caches through a shared `&LintDirCtx` reference. struct LintDirCtx<'a> { lint_root: &'a Path, flags: LintFlags, runtime_vars: &'a Option>, - config_cache: RefCell>>, + /// Fast-path-1 cache: `base_dir → config`. Every directory whose files have + /// been linted at least once is recorded here; a second file in the same + /// directory returns immediately without calling `load_config`. + base_dir_cache: RefCell>>, + /// Fast-path-2 cache: `config_dir → config`. Keyed by the RESOLVED directory + /// that contains `mds.json`, not the file's parent. Prevents a duplicate + /// unknown-rule warning when multiple `base_dir`s share one root `mds.json`. + config_dir_cache: RefCell>>, } impl<'a> LintDirCtx<'a> { /// Return the `LintConfig` for the directory `base_dir`. /// - /// On the first call for a given `base_dir`, the config is loaded by walking - /// up the directory tree (via `load_lint_config`). Subsequent calls return the - /// cached value; files in the same directory always share a config. - /// - /// On config-load failure, returns `Err(MdsError::Io{..})` so the caller can - /// record a per-file error and continue linting the rest of the tree. + /// Consults `base_dir_cache` (fast path 1) and `config_dir_cache` (fast path 2) + /// before loading from disk; see the struct doc for the two-cache design and the + /// AC-224-19 rationale. On config-load failure returns `Err(MdsError::Io{..})` so + /// the caller can record a per-file error and continue linting the rest of the tree. fn config_for(&self, base_dir: &Path) -> Result, MdsError> { + // Fast path 1: base_dir was already resolved in a previous call (common case + // for multiple files in the same directory — avoids the ancestor walk). { - let cache = self.config_cache.borrow(); + let cache = self.base_dir_cache.borrow(); if let Some(cfg) = cache.get(base_dir) { return Ok(Rc::clone(cfg)); } } - let config = load_lint_config(base_dir).map_err(|e| MdsError::Io { + + // Walk the ancestor chain to find the mds.json governing this directory. + // We call `load_config` directly (not `load_lint_config`) so we can inspect + // the RESOLVED config directory before deciding whether to emit the warning: + // multiple subdirectories can resolve to the same root mds.json, and + // AC-224-19 requires the warning fires at most once per distinct config dir. + let raw = load_config(base_dir).map_err(|e| MdsError::Io { message: format!("{e}"), })?; - let rc = Rc::new(config); - self.config_cache - .borrow_mut() - .insert(base_dir.to_path_buf(), Rc::clone(&rc)); - Ok(rc) + + match raw { + None => { + // No mds.json found: use the default config, keyed by base_dir only. + let rc = Rc::new(mds::LintConfig::default()); + self.base_dir_cache + .borrow_mut() + .insert(base_dir.to_path_buf(), Rc::clone(&rc)); + Ok(rc) + } + Some((mds_config, config_dir)) => { + // Fast path 2: a different base_dir already resolved to this same + // config directory (e.g. a/file.mds and b/file.mds both governed by + // root/mds.json). Return the cached config without emitting a + // duplicate warning (AC-224-19). Note: load_config above still ran + // — fast path 2 suppresses the duplicate WARNING, not the re-parse. + // + // `config_dir_cache` and `base_dir_cache` are different RefCells, so + // holding the shared borrow on `config_dir_cache` through the body is + // safe — the body only mutably borrows `base_dir_cache`. + if let Some(rc) = self + .config_dir_cache + .borrow() + .get(&config_dir) + .map(Rc::clone) + { + // Alias base_dir → cached config so fast path 1 fires on the + // next call for a file in this same directory. + self.base_dir_cache + .borrow_mut() + .insert(base_dir.to_path_buf(), Rc::clone(&rc)); + return Ok(rc); + } + + // First load for this config_dir: build the config and emit the warning. + // Contract documentation (safe_inline, --quiet, AC-224-3) lives at + // `emit_unknown_rule_warning` — the single emitter shared with + // load_lint_config (PF-009). + let (lint_config, unknown) = mds_config.lint.into_core_config(); + if let Some(unknown) = unknown { + emit_unknown_rule_warning(&unknown, self.flags.quiet); + } + + // Populate BOTH caches. `config_dir_cache` enables fast path 2 for + // future base_dirs that resolve to this same mds.json; `base_dir_cache` + // enables fast path 1 for future files in this same directory. + let rc = Rc::new(lint_config); + self.base_dir_cache + .borrow_mut() + .insert(base_dir.to_path_buf(), Rc::clone(&rc)); + self.config_dir_cache + .borrow_mut() + .insert(config_dir, Rc::clone(&rc)); + Ok(rc) + } + } } } @@ -1058,8 +1162,9 @@ fn run_lint_directory( let LintFlags { quiet, format, .. } = flags; // A6/D20: config is now discovered per-file (each file walks up to its nearest - // mds.json). The single root config load is removed; config_cache in LintDirCtx - // amortises repeated loads for files in the same directory. + // mds.json). The single root config load is removed; base_dir_cache and + // config_dir_cache in LintDirCtx amortise repeated loads for files in the same + // directory and across subdirectories sharing one root mds.json. let walk = collect_mds_files_detailed(dir, MAX_DEPTH, None); let mut files = walk.files; @@ -1126,7 +1231,8 @@ fn run_lint_directory( lint_root: dir, flags, runtime_vars: &runtime_vars, - config_cache: RefCell::new(HashMap::new()), + base_dir_cache: RefCell::new(HashMap::new()), + config_dir_cache: RefCell::new(HashMap::new()), }; for file in &files { diff --git a/crates/mds-cli/tests/cli_build.rs b/crates/mds-cli/tests/cli_build.rs index 99c16e12..e682983f 100644 --- a/crates/mds-cli/tests/cli_build.rs +++ b/crates/mds-cli/tests/cli_build.rs @@ -1287,3 +1287,233 @@ fn build_esc_byte_in_syntax_error_is_sanitized_on_stderr() { &out.stderr[..out.stderr.len().min(512)] ); } + +// ── AC-224-14 / D2(a): build and fmt (dir) do not emit unknown-rule warnings ── +// +// Only `mds lint` reads `lint.rules` and emits an unknown-rule warning — single-file +// mode via `load_lint_config`, directory mode via `LintDirCtx::config_for`. +// `mds build` and `mds fmt ` read `mds.json` via `load_config` but deserialize +// the `lint` field without calling `load_lint_config`, so they never warn — an +// accepted D2(a) asymmetry (see build.rs:49-51 and CHANGELOG). `mds check` and +// `mds fmt ` do not call `load_config` at all and never reach any lint-config +// path; their absence assertions are structural, not D2(a). +// +// The build and fmt (dir) tests mechanically hold the AC-224-14 watch-path invariant: +// `watch.rs:822` calls `load_config(...).unwrap_or(None)`. Because `build` and +// `mds fmt ` share the same `load_config` implementation, a passing build or +// dir-fmt proves `load_config` returns `Ok` for configs with unknown rule names — +// so `unwrap_or(None)` cannot collapse `output_dir` to `None` on account of an +// unknown lint rule name alone. Non-vacuity is secured by the positive-control +// arms in each test (ADR-009 / PF-013). + +/// AC-224-14 / D2(a): `mds build` must NOT emit an unknown-rule warning even +/// when `mds.json` names a lint rule that does not exist. +/// +/// Non-vacuity (ADR-009 / PF-013): a paired positive-control arm runs the same +/// config with an unknown SEVERITY value — `mds build` exits non-zero on that +/// config, proving `load_config` was reached and the JSON was parsed. The main +/// assertion (unknown rule name → exits 0, no warning) is therefore non-vacuous: +/// `load_config` was called; only the warning was declined. The complementary +/// positive control showing the warning IS emitted lives in `cli_lint.rs`. +#[test] +fn build_unknown_lint_rule_in_mds_json_emits_no_warning() { + let dir = tempfile::tempdir().unwrap(); + let src = dir.path().join("tpl.mds"); + std::fs::write(&src, "Hello!\n").unwrap(); + std::fs::write( + dir.path().join("mds.json"), + r#"{"lint":{"rules":{"no-such-rule-xyzzy":"warn"}}}"#, + ) + .unwrap(); + + let out = mds_bin() + .arg("build") + .arg(&src) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + + assert!( + out.status.success(), + "D2(a): mds build must succeed even with an unknown lint rule in mds.json; \ + stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + !stderr.contains("unknown lint rule"), + "D2(a): mds build must NOT emit an unknown-rule warning; stderr: {stderr}" + ); + assert!( + !stderr.contains("no-such-rule-xyzzy"), + "D2(a): mds build must NOT name the unknown rule in stderr; stderr: {stderr}" + ); + + // Positive-control arm (ADR-009 / PF-013): an unknown SEVERITY value is a hard + // serde parse error in load_config (Severity is a closed enum; "banana" is not a + // recognised variant). build must exit non-zero, proving load_config was called + // and the JSON was actually parsed in the no-warning arm above. If build ever + // stops reading mds.json this arm will fail, surfacing the regression. + std::fs::write( + dir.path().join("mds.json"), + r#"{"lint":{"rules":{"no-such-rule-xyzzy":"banana"}}}"#, + ) + .unwrap(); + let out_bad = mds_bin() + .arg("build") + .arg(&src) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + assert!( + !out_bad.status.success(), + "D2(a) positive-control: mds build must exit non-zero on an unknown severity \ + value ('banana') in mds.json — this proves load_config was reached in the \ + no-warning arm; stderr: {}", + String::from_utf8_lossy(&out_bad.stderr) + ); +} + +/// AC-224-14: `mds check` must NOT emit an unknown-rule warning even +/// when `mds.json` names a lint rule that does not exist. +/// +/// `mds check ` does not call `load_config` at all (see main.rs:288-290); +/// it never reads `mds.json`. The absence is structural, not a D2(a) choice. +/// Non-vacuity (ADR-009 / PF-013): a positive-control arm confirms that even an +/// unknown SEVERITY value in `mds.json` leaves `mds check`'s exit code unchanged, +/// proving the file is not read rather than merely that a warning was declined. +#[test] +fn check_unknown_lint_rule_in_mds_json_emits_no_warning() { + let dir = tempfile::tempdir().unwrap(); + let src = dir.path().join("tpl.mds"); + std::fs::write(&src, "Hello!\n").unwrap(); + std::fs::write( + dir.path().join("mds.json"), + r#"{"lint":{"rules":{"no-such-rule-xyzzy":"warn"}}}"#, + ) + .unwrap(); + + let out = mds_bin() + .arg("check") + .arg(&src) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + + assert!( + out.status.success(), + "AC-224-14: mds check must succeed even with an unknown lint rule in mds.json; \ + stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + !stderr.contains("unknown lint rule"), + "AC-224-14: mds check must NOT emit an unknown-rule warning; stderr: {stderr}" + ); + assert!( + !stderr.contains("no-such-rule-xyzzy"), + "AC-224-14: mds check must NOT name the unknown rule in stderr; stderr: {stderr}" + ); + + // Positive-control arm (ADR-009 / PF-013): write mds.json with an unknown + // SEVERITY value ("banana" is not a recognised Severity variant) — a config + // that would cause `mds build` to exit non-zero (hard serde parse error). + // `mds check ` must still exit 0, proving that check never reads + // mds.json at all. If check ever starts reading the config, this arm will + // fail, surfacing the regression. + std::fs::write( + dir.path().join("mds.json"), + r#"{"lint":{"rules":{"no-such-rule-xyzzy":"banana"}}}"#, + ) + .unwrap(); + let out_bad = mds_bin() + .arg("check") + .arg(&src) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + assert!( + out_bad.status.success(), + "AC-224-14 positive-control: mds check must still exit 0 with an unknown \ + severity in mds.json — check does not read the config; \ + stderr: {}", + String::from_utf8_lossy(&out_bad.stderr) + ); +} + +/// AC-224-14 / D2(a): `mds fmt ` must NOT emit an unknown-rule warning even +/// when `mds.json` names a lint rule that does not exist. +/// +/// `mds fmt ` calls `load_config` (fmt.rs:308) and thus follows the same +/// code path as `mds build` and `watch.rs:822`. A passing test proves +/// `load_config` returns `Ok` for configs with unknown rule names. +/// Note: `mds fmt ` does NOT call `load_config`; only the directory target +/// exercises this code path. +/// Non-vacuity (ADR-009 / PF-013): a positive-control arm confirms that an +/// unknown SEVERITY value causes `mds fmt ` to exit non-zero, proving +/// `load_config` was reached and the JSON was parsed in the no-warning arm. +#[test] +fn fmt_unknown_lint_rule_in_mds_json_emits_no_warning() { + let dir = tempfile::tempdir().unwrap(); + let src = dir.path().join("tpl.mds"); + std::fs::write(&src, "Hello!\n").unwrap(); + std::fs::write( + dir.path().join("mds.json"), + r#"{"lint":{"rules":{"no-such-rule-xyzzy":"warn"}}}"#, + ) + .unwrap(); + + // Target the DIRECTORY so load_config (fmt.rs:308) is actually called. + // Targeting a single file bypasses load_config entirely. + let out = mds_bin() + .arg("fmt") + .arg(dir.path()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + + assert!( + out.status.success(), + "D2(a): mds fmt must succeed even with an unknown lint rule in mds.json; \ + stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + !stderr.contains("unknown lint rule"), + "D2(a): mds fmt must NOT emit an unknown-rule warning; stderr: {stderr}" + ); + assert!( + !stderr.contains("no-such-rule-xyzzy"), + "D2(a): mds fmt must NOT name the unknown rule in stderr; stderr: {stderr}" + ); + + // Positive-control arm (ADR-009 / PF-013): an unknown SEVERITY value is a hard + // serde parse error in load_config (Severity is a closed enum). mds fmt + // must exit non-zero, proving load_config was called in the no-warning arm above. + std::fs::write( + dir.path().join("mds.json"), + r#"{"lint":{"rules":{"no-such-rule-xyzzy":"banana"}}}"#, + ) + .unwrap(); + let out_bad = mds_bin() + .arg("fmt") + .arg(dir.path()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + assert!( + !out_bad.status.success(), + "D2(a) positive-control: mds fmt must exit non-zero on an unknown \ + severity value ('banana') in mds.json — this proves load_config was reached \ + in the no-warning arm; stderr: {}", + String::from_utf8_lossy(&out_bad.stderr) + ); +} diff --git a/crates/mds-cli/tests/cli_lint.rs b/crates/mds-cli/tests/cli_lint.rs index aaae668d..e33171b6 100644 --- a/crates/mds-cli/tests/cli_lint.rs +++ b/crates/mds-cli/tests/cli_lint.rs @@ -22,6 +22,12 @@ //! - L-CLI-DIR2: directory --format json files[] order is deterministic (TEST-6) //! - I-24: unreachable-branch (always-true @if) --fix applied; file changed, exit 0 //! - I-26: shadow-variable Info severity emits diagnostic and exits 0 (Info never affects exit) +//! - AC-224-10: unknown rule name → JSON wire shape unchanged (lint continues) +//! - AC-224-11: unknown rule warning goes to stderr, not stdout +//! - AC-224-14: D2(a) asymmetry — build/check/fmt do NOT warn (see cli_build.rs) +//! - AC-224-19: directory with N files emits exactly ONE unknown-rule warning +//! - AC-224-21: stdout JSON remains valid when unknown rule name is present +//! - AC-224-22: --quiet suppresses the unknown-rule warning mod common; use common::{assert_no_control_chars, fixture, mds_bin}; @@ -3265,3 +3271,1250 @@ fn lint_dir_nested_malformed_config_per_file_error() { "output must reference the malformed config source; got: {combined}" ); } + +// ── AC-224-10/11/21/22/19: unknown rule-name warn-and-continue ─────────────── +// +// An unknown rule name in `mds.json` must: +// AC-224-10: not alter the JSON wire shape on stdout (lint continues as normal) +// AC-224-11: emit the warning on stderr (never stdout) +// AC-224-21: preserve valid JSON on stdout in --format json mode +// AC-224-22: be suppressed by --quiet +// AC-224-19: emit exactly ONE warning per mds lint invocation, not one per file + +/// Write a `mds.json` with an unknown rule name into `dir`. +fn write_unknown_rule_config(dir: &std::path::Path) { + let config = serde_json::json!({ + "lint": { + "rules": { + "no-such-rule-xyzzy": "warn" + } + } + }); + std::fs::write( + dir.join("mds.json"), + serde_json::to_string(&config).unwrap(), + ) + .unwrap(); +} + +/// AC-224-21 / AC-224-11: warning goes to stderr; stdout JSON is unaffected. +/// +/// In `--format json` mode the stdout JSON must be valid and not contain the +/// warning text; the warning must appear on stderr instead. +#[test] +fn unknown_rule_warning_to_stderr_not_stdout_in_json_mode() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("clean.mds"), "Hello!\n").unwrap(); + write_unknown_rule_config(dir.path()); + + let out = mds_bin() + .arg("lint") + .arg(dir.path()) + .arg("--format") + .arg("json") + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + + // AC-224-21: stdout must still be valid JSON (the warning must NOT land there). + let parsed: serde_json::Value = serde_json::from_str(stdout.trim()).unwrap_or_else(|e| { + panic!( + "AC-224-21: stdout must be valid JSON even when unknown rules are present; \ + parse error: {e}; stdout: {stdout}" + ) + }); + assert_eq!( + parsed["version"].as_u64(), + Some(1), + "AC-224-21: JSON must have version:1; got: {parsed}" + ); + assert!( + !stdout.contains("unknown"), + "AC-224-21: 'unknown' warning text must not appear in stdout JSON; got: {stdout}" + ); + + // AC-224-11: the warning must appear on stderr and name the rule. + // Both substrings required (&&, not ||) so neither half alone can satisfy the + // positive control — aligns with the stronger form in + // `unknown_rule_json_stdout_contains_no_warning_text` (avoids PF-013 asymmetry). + assert!( + stderr.contains("unknown lint rule") && stderr.contains("no-such-rule-xyzzy"), + "AC-224-11: the unknown-rule warning must go to stderr and name the rule; \ + got stderr: {stderr}" + ); +} + +/// AC-224-10: the JSON wire shape is unchanged when lint continues past an unknown rule. +/// +/// The `files` array and `truncated` / `version` fields must be present with their +/// normal shape. No extra top-level error key must appear. +#[test] +fn unknown_rule_json_wire_shape_unchanged() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("a.mds"), "Hello!\n").unwrap(); + write_unknown_rule_config(dir.path()); + + let out = mds_bin() + .arg("lint") + .arg(dir.path()) + .arg("--format") + .arg("json") + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + + let stdout = String::from_utf8_lossy(&out.stdout); + let parsed: serde_json::Value = + serde_json::from_str(stdout.trim()).expect("stdout must be valid JSON"); + + // AC-224-10: standard fields are present; no error envelope. + assert!( + parsed.get("files").is_some(), + "AC-224-10: 'files' key must be present in the JSON output; got: {parsed}" + ); + assert!( + parsed.get("truncated").is_some(), + "AC-224-10: 'truncated' key must be present in the JSON output; got: {parsed}" + ); + assert!( + parsed.get("error").is_none(), + "AC-224-10: no 'error' key must appear for a lint-continues result; got: {parsed}" + ); + + // Exit code must be 0 (clean file, no real diagnostics). + assert_eq!( + out.status.code(), + Some(0), + "AC-224-10: exit code must be 0 when the only unknown element is a rule name; \ + got stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); +} + +/// AC-224-22: `--quiet` suppresses the unknown-rule warning on stderr. +#[test] +fn unknown_rule_warning_suppressed_by_quiet() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("a.mds"), "Hello!\n").unwrap(); + write_unknown_rule_config(dir.path()); + + let out = mds_bin() + .arg("lint") + .arg(dir.path()) + .arg("--quiet") + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + + let stderr = String::from_utf8_lossy(&out.stderr); + + // AC-224-22: --quiet must suppress the unknown-rule warning. + assert!( + !stderr.contains("unknown lint rule") && !stderr.contains("no-such-rule-xyzzy"), + "AC-224-22: --quiet must suppress the unknown-rule warning; got stderr: {stderr}" + ); + + // Exit code is still 0 (clean file). + assert_eq!( + out.status.code(), + Some(0), + "AC-224-22: exit code must be 0; got stderr: {stderr}" + ); +} + +/// AC-224-19: a directory tree whose files span multiple subdirectories emits exactly +/// ONE unknown-rule warning, not one per file or one per subdirectory. +/// +/// A flat-directory test (all files in one dir) would be vacuous because the +/// base_dir cache already coalesces them — the caching claim under review is about +/// SUBDIRECTORIES that all resolve to the same root `mds.json`. This test uses a +/// nested tree (root + subdirs a/, b/, c/d/) so that each unique base_dir produces a +/// distinct cache lookup. Without the config_dir-keyed deduplication in +/// `LintDirCtx::config_for`, each distinct base_dir would re-load the config and +/// re-emit the warning, yielding N warnings for N subdirs (the AC-224-19 bug). +#[test] +fn unknown_rule_one_warning_per_invocation_not_per_file() { + let dir = tempfile::tempdir().unwrap(); + // Files in different subdirectories — each has a distinct base_dir. All are + // governed by the single root mds.json, exercising the config_dir deduplication. + for subdir in ["a", "b", "c", "c/d"] { + let sub = dir.path().join(subdir); + std::fs::create_dir_all(&sub).unwrap(); + std::fs::write(sub.join("file.mds"), "Hello!\n").unwrap(); + } + // One file at the root level so the base_dir == config_dir case is also covered. + std::fs::write(dir.path().join("root.mds"), "Hello!\n").unwrap(); + // Single mds.json at the root governs every file in the tree. + write_unknown_rule_config(dir.path()); + + let out = mds_bin() + .arg("lint") + .arg(dir.path()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + + let stderr = String::from_utf8_lossy(&out.stderr); + + // AC-224-19: the warning must appear at least once (positive control, PF-013). + assert!( + stderr.contains("unknown lint rule") && stderr.contains("no-such-rule-xyzzy"), + "AC-224-19: warning must appear on stderr; got stderr: {stderr}" + ); + + // AC-224-19: exactly one warning per invocation, regardless of how many + // subdirectories contributed distinct base_dir lookups. A warning_count > 1 here + // means config_for is keying on base_dir rather than config_dir and emitting a + // duplicate for each subdir that resolves to the same root mds.json. + let warning_count = stderr + .lines() + .filter(|l| l.contains("unknown lint rule")) + .count(); + assert_eq!( + warning_count, 1, + "AC-224-19: warning must appear exactly once per distinct config directory \ + (got {warning_count} — one per subdir instead of one per config); \ + got stderr:\n{stderr}" + ); +} + +/// AC-224-2 (plural branch): when two or more rule names are unknown the plural form +/// of the warning is emitted and the names appear in lexicographic sorted order. +/// +/// Test-plan entry 2: CLI config `{"zzz-bad":"warn","aaa-bad":"error"}` yields +/// exactly one warning line naming both unknown rules with `aaa-bad` before `zzz-bad`. +/// +/// Non-vacuity (PF-013/ADR-009): the test also confirms the positive control — +/// `recognised rules are` appears — and the negative control — a known rule in the +/// same config does NOT trigger the plural path for itself. +#[test] +fn unknown_rule_plural_sorted_warning() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("a.mds"), "Hello!\n").unwrap(); + // zzz-bad and aaa-bad are both unknown; unused-variable is real so it does NOT + // count toward the unknown list. The inserted order is zzz first to confirm + // lexicographic sorting, not insertion order. + write_rules_config( + dir.path(), + serde_json::json!({ + "zzz-bad": "warn", + "aaa-bad": "error", + "unused-variable": "off" + }), + ); + + let out = mds_bin() + .arg("lint") + .arg(dir.path()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + + let stderr = String::from_utf8_lossy(&out.stderr); + + // AC-224-2: both unknown names are reported. + assert!( + stderr.contains("aaa-bad"), + "AC-224-2: 'aaa-bad' must appear in the plural warning; got stderr: {stderr}" + ); + assert!( + stderr.contains("zzz-bad"), + "AC-224-2: 'zzz-bad' must appear in the plural warning; got stderr: {stderr}" + ); + // AC-224-2: plural form is used (not singular). + assert!( + stderr.contains("unknown lint rules"), + "AC-224-2: plural form must be used for two unknown names; got stderr: {stderr}" + ); + // AC-224-2: aaa-bad sorts before zzz-bad. + let aaa_pos = stderr + .find("aaa-bad") + .expect("aaa-bad must appear in stderr"); + let zzz_pos = stderr + .find("zzz-bad") + .expect("zzz-bad must appear in stderr"); + assert!( + aaa_pos < zzz_pos, + "AC-224-2: 'aaa-bad' must appear before 'zzz-bad' (lexicographic order); \ + got stderr: {stderr}" + ); + // Positive control: recognised-rules list is present (AC-224-2 completeness). + assert!( + stderr.contains("recognised rules are"), + "recognised-rules list must be included in the plural warning; got stderr: {stderr}" + ); + // Negative control: unused-variable is a known rule and must NOT appear + // QUOTED (as an unknown name) in the warning — it does appear unquoted in + // the "recognised rules are: …" part, which is expected. The unknown-names + // list uses single-quote delimiters ('NAME'), so checking for the quoted + // form distinguishes the two positions. + let warning_line = stderr + .lines() + .find(|l| l.contains("unknown lint rules")) + .unwrap_or_else(|| panic!("no plural unknown-rule warning line; got stderr: {stderr}")); + assert!( + !warning_line.contains("'unused-variable'"), + "a recognised rule name must not appear quoted in the unknown-names list; \ + got warning line: {warning_line}" + ); +} + +/// AC-224-2 / AC-224-3 golden (singular and plural): pins the EXACT rendered warning +/// text for both the one-offender and two-offender paths on the CLI. +/// +/// Test-plan entries 2 and 3 require exact rendering, not just `contains` checks, so +/// that future edits to the message format fail loudly here rather than silently +/// diverging from the CHANGELOG and spec.md documentation. +/// +/// PF-007: these goldens cover the CLI surface only; the binding surfaces are pinned +/// by their own per-surface tests. +#[test] +fn unknown_rule_cli_exact_golden() { + // We cannot import mds:: directly from an integration test; hard-code the + // current ten names in alphabetical order (the same value KNOWN_LINT_RULES + // holds, verified by AC-224-7). If the registry grows this golden must be + // updated alongside the registry — the test will fail loudly. + #[rustfmt::skip] + let recognised = concat!( + "duplicate-export, duplicate-import, empty-block, legacy-interpolation, ", + "redundant-else, shadow-variable, unreachable-branch, unused-function, ", + "unused-import, unused-variable" + ); + + // ── Singular golden ──────────────────────────────────────────────────────── + { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("a.mds"), "Hello!\n").unwrap(); + write_rules_config( + dir.path(), + serde_json::json!({ "no-such-rule-xyzzy": "warn" }), + ); + + let out = mds_bin() + .arg("lint") + .arg(dir.path()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + + let stderr = String::from_utf8_lossy(&out.stderr); + let expected_singular = format!( + "warning: in mds.json: unknown lint rule 'no-such-rule-xyzzy'; \ + recognised rules are: {recognised}; ignoring" + ); + let warning_line = stderr + .lines() + .find(|l| l.contains("unknown lint rule")) + .unwrap_or_else(|| { + panic!("no singular unknown-rule warning line; got stderr: {stderr}") + }); + assert_eq!( + warning_line.trim(), + expected_singular, + "AC-224-2/AC-224-3 singular golden mismatch" + ); + } + + // ── Plural golden ────────────────────────────────────────────────────────── + { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("a.mds"), "Hello!\n").unwrap(); + // Insert zzz-bad before aaa-bad to confirm lexicographic sorting, not + // insertion order. + write_rules_config( + dir.path(), + serde_json::json!({ "zzz-bad": "warn", "aaa-bad": "error" }), + ); + + let out = mds_bin() + .arg("lint") + .arg(dir.path()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + + let stderr = String::from_utf8_lossy(&out.stderr); + let expected_plural = format!( + "warning: in mds.json: unknown lint rules: 'aaa-bad', 'zzz-bad'; \ + recognised rules are: {recognised}; ignoring" + ); + let warning_line = stderr + .lines() + .find(|l| l.contains("unknown lint rules")) + .unwrap_or_else(|| panic!("no plural unknown-rule warning line; got stderr: {stderr}")); + assert_eq!( + warning_line.trim(), + expected_plural, + "AC-224-2/AC-224-3 plural golden mismatch" + ); + } +} + +/// AC-224-2 / AC-224-3 golden for the SINGLE-FILE target path. +/// +/// `unknown_rule_cli_exact_golden` exercises `mds lint `, which goes through +/// `LintDirCtx::config_for`. This test exercises `mds lint `, which goes +/// through `load_lint_config` — a distinct code path. By pinning the EXACT warning +/// string for both paths we ensure that a future edit to either emitter fails +/// loudly here rather than diverging silently (the duplication that existed before +/// the `emit_unknown_rule_warning` extraction was introduced in this PR). +/// +/// PF-007: covers the CLI single-file surface only. +#[test] +fn unknown_rule_cli_exact_golden_single_file() { + #[rustfmt::skip] + let recognised = concat!( + "duplicate-export, duplicate-import, empty-block, legacy-interpolation, ", + "redundant-else, shadow-variable, unreachable-branch, unused-function, ", + "unused-import, unused-variable" + ); + + // ── Singular golden (single-file path) ──────────────────────────────────── + { + let dir = tempfile::tempdir().unwrap(); + let file = dir.path().join("a.mds"); + std::fs::write(&file, "Hello!\n").unwrap(); + write_rules_config( + dir.path(), + serde_json::json!({ "no-such-rule-xyzzy": "warn" }), + ); + + let out = mds_bin() + .arg("lint") + .arg(&file) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + + let stderr = String::from_utf8_lossy(&out.stderr); + let expected_singular = format!( + "warning: in mds.json: unknown lint rule 'no-such-rule-xyzzy'; \ + recognised rules are: {recognised}; ignoring" + ); + let warning_line = stderr + .lines() + .find(|l| l.contains("unknown lint rule")) + .unwrap_or_else(|| { + panic!( + "AC-224-2/AC-224-3 single-file: no singular unknown-rule warning; \ + got stderr: {stderr}" + ) + }); + assert_eq!( + warning_line.trim(), + expected_singular, + "AC-224-2/AC-224-3 single-file singular golden mismatch" + ); + } + + // ── Plural golden (single-file path) ────────────────────────────────────── + { + let dir = tempfile::tempdir().unwrap(); + let file = dir.path().join("a.mds"); + std::fs::write(&file, "Hello!\n").unwrap(); + write_rules_config( + dir.path(), + serde_json::json!({ "zzz-bad": "warn", "aaa-bad": "error" }), + ); + + let out = mds_bin() + .arg("lint") + .arg(&file) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + + let stderr = String::from_utf8_lossy(&out.stderr); + let expected_plural = format!( + "warning: in mds.json: unknown lint rules: 'aaa-bad', 'zzz-bad'; \ + recognised rules are: {recognised}; ignoring" + ); + let warning_line = stderr + .lines() + .find(|l| l.contains("unknown lint rules")) + .unwrap_or_else(|| { + panic!( + "AC-224-2/AC-224-3 single-file: no plural unknown-rule warning; \ + got stderr: {stderr}" + ) + }); + assert_eq!( + warning_line.trim(), + expected_plural, + "AC-224-2/AC-224-3 single-file plural golden mismatch" + ); + } +} + +/// Populate `dir` with a fixed three-file tree — one clean, two with real findings — +/// so the JSON envelope under test carries actual `files[]` entries rather than an +/// empty array (an all-clean tree would make the comparison below near-vacuous). +fn write_mixed_lint_tree(dir: &std::path::Path) { + std::fs::write(dir.join("clean.mds"), "Hello!\n").unwrap(); + std::fs::write( + dir.join("unused.mds"), + "---\nunused_key: value\n---\nHello!\n", + ) + .unwrap(); + std::fs::write( + dir.join("dup.mds"), + "---\nanother_unused: v\n---\nHi there!\n", + ) + .unwrap(); +} + +/// Write an `mds.json` naming `rules` verbatim. +fn write_rules_config(dir: &std::path::Path, rules: serde_json::Value) { + let config = serde_json::json!({ "lint": { "rules": rules } }); + std::fs::write( + dir.join("mds.json"), + serde_json::to_string(&config).unwrap(), + ) + .unwrap(); +} + +/// AC-224-10 (NEGATIVE, envelope frozen): an unknown rule name changes the `--format json` +/// stdout by **zero bytes**. +/// +/// The two runs differ only by the presence of `no-such-rule-xyzzy` in `mds.json`. Their +/// stdout buffers are compared byte-for-byte, and the top-level key set is compared as an +/// EXACT set (not `contains`), so a new top-level key or a `files[]` entry lacking +/// `diagnostics` fails. Exit codes must match too. +/// +/// Non-vacuity (PF-013 / ADR-009): the same run asserts the warning IS on stderr and that +/// the envelope actually carries `files[]` entries — a byte-identical comparison of two +/// empty or two error envelopes would prove nothing. +#[test] +fn unknown_rule_json_stdout_is_byte_identical_to_run_without_it() { + let with_dir = tempfile::tempdir().unwrap(); + write_mixed_lint_tree(with_dir.path()); + write_rules_config( + with_dir.path(), + serde_json::json!({ "unused-variable": "warn", "no-such-rule-xyzzy": "warn" }), + ); + + let without_dir = tempfile::tempdir().unwrap(); + write_mixed_lint_tree(without_dir.path()); + write_rules_config( + without_dir.path(), + serde_json::json!({ "unused-variable": "warn" }), + ); + + let run = |dir: &std::path::Path| { + mds_bin() + .arg("lint") + .arg(dir) + .arg("--format") + .arg("json") + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap() + }; + let with = run(with_dir.path()); + let without = run(without_dir.path()); + + // Non-vacuity: the warning fired in the first run and not in the second. + let with_stderr = String::from_utf8_lossy(&with.stderr); + assert!( + with_stderr.contains("unknown lint rule") && with_stderr.contains("no-such-rule-xyzzy"), + "non-vacuity: the unknown-rule warning must fire on stderr; got: {with_stderr}" + ); + let without_stderr = String::from_utf8_lossy(&without.stderr); + assert!( + !without_stderr.contains("unknown lint rule"), + "control run must not warn; got: {without_stderr}" + ); + + // AC-224-10: stdout is byte-identical, and the exit code does not move. + assert_eq!( + with.stdout, + without.stdout, + "AC-224-10: stdout must be byte-identical with and without the unknown rule name;\n\ + with: {}\nwithout: {}", + String::from_utf8_lossy(&with.stdout), + String::from_utf8_lossy(&without.stdout) + ); + assert_eq!( + with.status.code(), + without.status.code(), + "AC-224-13: the exit code must not move because of an unknown rule name" + ); + + // AC-224-10: exact top-level key set — not `contains`. + let parsed: serde_json::Value = + serde_json::from_str(String::from_utf8_lossy(&with.stdout).trim()) + .expect("stdout must be valid JSON"); + let obj = parsed.as_object().expect("envelope must be a JSON object"); + let mut top_keys: Vec<&str> = obj.keys().map(String::as_str).collect(); + top_keys.sort_unstable(); + assert_eq!( + top_keys, + ["files", "truncated", "version"], + "AC-224-10: the top-level key set must be exactly {{files, truncated, version}}; \ + got: {parsed}" + ); + assert_eq!(parsed["version"].as_u64(), Some(1)); + + // Non-vacuity: the envelope really does carry file entries, and each has the + // `diagnostics` key with no `error` key. + let files = parsed["files"].as_array().expect("files must be an array"); + assert!( + !files.is_empty(), + "non-vacuity: the fixture tree must produce at least one files[] entry; got: {parsed}" + ); + for entry in files { + let e = entry.as_object().expect("files[] entry must be an object"); + assert!( + e.contains_key("diagnostics"), + "AC-224-10: every files[] entry must carry a diagnostics key; got: {entry}" + ); + assert!( + !e.contains_key("error"), + "AC-224-10: no files[] entry may carry an error key; got: {entry}" + ); + } +} + +/// AC-224-21: `--format json` stdout carries no warning text at all — including the +/// literal substring `warning`, which the bare `!stdout.contains("unknown")` check above +/// would not catch. Paired positive control: the warning IS on stderr in the same run. +#[test] +fn unknown_rule_json_stdout_contains_no_warning_text() { + let dir = tempfile::tempdir().unwrap(); + write_mixed_lint_tree(dir.path()); + write_rules_config( + dir.path(), + serde_json::json!({ "no-such-rule-xyzzy": "warn" }), + ); + + let out = mds_bin() + .arg("lint") + .arg(dir.path()) + .arg("--format") + .arg("json") + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + + // Positive control first: without this, every negative below is vacuous. + assert!( + stderr.contains("no-such-rule-xyzzy") && stderr.contains("recognised rules are"), + "positive control: the warning must be on stderr; got: {stderr}" + ); + + for needle in [ + "no-such-rule-xyzzy", + "recognised rules are", + "warning", + "ignoring", + ] { + assert!( + !stdout.contains(needle), + "AC-224-21: stdout must not contain {needle:?}; got: {stdout}" + ); + } + serde_json::from_str::(stdout.trim()) + .expect("AC-224-21: stdout must parse as a single JSON document"); +} + +/// AC-224-21 (single-file target): `--format json` stdout is clean when the unknown- +/// rule warning fires. +/// +/// `run_lint_file` is a separate emitter from `run_lint_directory`; this test covers +/// it independently (AC-224-21 gap identified in the code review). Paired positive +/// control (PF-013 / ADR-009): the warning IS on stderr, so a clean-stdout assertion +/// cannot pass vacuously on a run where the warning never fired. +#[test] +fn unknown_rule_json_stdout_clean_single_file() { + let dir = tempfile::tempdir().unwrap(); + let file = dir.path().join("clean.mds"); + std::fs::write(&file, "Hello!\n").unwrap(); + write_unknown_rule_config(dir.path()); + + let out = mds_bin() + .arg("lint") + .arg(&file) + .arg("--format") + .arg("json") + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + + // Positive control: warning fired on stderr. + assert!( + stderr.contains("unknown lint rule") && stderr.contains("no-such-rule-xyzzy"), + "AC-224-21 (file): warning must fire on stderr; got: {stderr}" + ); + + // AC-224-21: stdout must be valid JSON and contain no warning text. + serde_json::from_str::(stdout.trim()).unwrap_or_else(|e| { + panic!("AC-224-21 (file): stdout must be valid JSON; error: {e}; got: {stdout}") + }); + for needle in [ + "no-such-rule-xyzzy", + "recognised rules are", + "warning", + "ignoring", + ] { + assert!( + !stdout.contains(needle), + "AC-224-21 (file): stdout must not contain {needle:?}; got: {stdout}" + ); + } +} + +/// AC-224-21 (stdin target): `--format json` stdout is clean when the unknown-rule +/// warning fires. +/// +/// `run_lint_stdin` is a separate emitter from `run_lint_directory` and +/// `run_lint_file`; this test covers it independently (AC-224-21 gap). `current_dir` +/// is set to a tempdir containing `mds.json` so `run_lint_stdin` picks up the unknown +/// rule. Paired positive control (PF-013 / ADR-009). +#[test] +fn unknown_rule_json_stdout_clean_stdin() { + use std::io::Write; + let dir = tempfile::tempdir().unwrap(); + write_unknown_rule_config(dir.path()); + + let mut child = mds_bin() + .arg("lint") + .arg("-") + .arg("--format") + .arg("json") + .current_dir(dir.path()) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .unwrap(); + // Ignore BrokenPipe — child may exit before reading all of stdin. + let _ = child.stdin.take().unwrap().write_all(b"Hello!\n"); + let out = child.wait_with_output().unwrap(); + + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + + // Positive control: warning fired on stderr. + assert!( + stderr.contains("unknown lint rule") && stderr.contains("no-such-rule-xyzzy"), + "AC-224-21 (stdin): warning must fire on stderr; got: {stderr}" + ); + + // AC-224-21: stdout must be valid JSON and contain no warning text. + serde_json::from_str::(stdout.trim()).unwrap_or_else(|e| { + panic!("AC-224-21 (stdin): stdout must be valid JSON; error: {e}; got: {stdout}") + }); + for needle in [ + "no-such-rule-xyzzy", + "recognised rules are", + "warning", + "ignoring", + ] { + assert!( + !stdout.contains(needle), + "AC-224-21 (stdin): stdout must not contain {needle:?}; got: {stdout}" + ); + } +} + +/// AC-224-22: the pre-subcommand global form `mds --quiet lint ` suppresses the +/// warning exactly like `mds lint --quiet `, and neither moves the exit code. +/// +/// Paired positive control (PF-013): the same tree without `--quiet` DOES warn. +#[test] +fn unknown_rule_warning_suppressed_by_global_quiet_form() { + let dir = tempfile::tempdir().unwrap(); + write_mixed_lint_tree(dir.path()); + write_rules_config( + dir.path(), + serde_json::json!({ "no-such-rule-xyzzy": "warn" }), + ); + + let loud = mds_bin() + .arg("lint") + .arg(dir.path()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + assert!( + String::from_utf8_lossy(&loud.stderr).contains("unknown lint rule"), + "positive control: the non-quiet run must warn" + ); + + for args in [vec!["lint", "--quiet"], vec!["--quiet", "lint"]] { + let mut cmd = mds_bin(); + for a in &args { + cmd.arg(a); + } + let out = cmd + .arg(dir.path()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + !stderr.contains("unknown lint rule") && !stderr.contains("no-such-rule-xyzzy"), + "AC-224-22: `mds {}` must suppress the unknown-rule warning; got: {stderr}", + args.join(" ") + ); + assert_eq!( + out.status.code(), + loud.status.code(), + "AC-224-22: --quiet must not move the exit code (form: {})", + args.join(" ") + ); + } +} + +/// AC-224-22 (single-file target): `--quiet` suppresses the unknown-rule warning when +/// targeting a single file. +/// +/// `load_lint_config` (the `run_lint_file` / `run_lint_stdin` code path) has a quiet gate +/// inside `emit_unknown_rule_warning`; this test covers the single-file path independently +/// from the directory target covered by `unknown_rule_warning_suppressed_by_quiet`, which +/// exercises the `config_for` emitter instead. +/// +/// Paired positive control (PF-013 / ADR-009): the same invocation WITHOUT `--quiet` DOES +/// emit the warning, so the absence assertion cannot pass vacuously. +#[test] +fn unknown_rule_warning_suppressed_by_quiet_single_file() { + let dir = tempfile::tempdir().unwrap(); + let file = dir.path().join("clean.mds"); + std::fs::write(&file, "Hello!\n").unwrap(); + write_unknown_rule_config(dir.path()); + + // Positive control: without --quiet the warning fires on the single-file path. + let loud = mds_bin() + .arg("lint") + .arg(&file) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + assert!( + String::from_utf8_lossy(&loud.stderr).contains("unknown lint rule"), + "positive control: single-file mode without --quiet must warn; got: {}", + String::from_utf8_lossy(&loud.stderr) + ); + + // AC-224-22: --quiet must suppress the warning. + let out = mds_bin() + .arg("lint") + .arg(&file) + .arg("--quiet") + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + !stderr.contains("unknown lint rule") && !stderr.contains("no-such-rule-xyzzy"), + "AC-224-22 (single-file): --quiet must suppress the unknown-rule warning; got: {stderr}" + ); + assert_eq!( + out.status.code(), + loud.status.code(), + "AC-224-22 (single-file): --quiet must not move the exit code; got stderr: {stderr}" + ); +} + +/// AC-224-22 (stdin target): `--quiet` suppresses the unknown-rule warning when reading +/// from stdin. +/// +/// `load_lint_config` (the `run_lint_stdin` code path) has a quiet gate inside +/// `emit_unknown_rule_warning`; this test covers the stdin path independently from the +/// directory target covered by `unknown_rule_warning_suppressed_by_quiet`, which exercises +/// the `config_for` emitter instead. +/// +/// `current_dir` is set to a tempdir containing `mds.json` so `run_lint_stdin` picks up +/// the unknown rule. Paired positive control (PF-013 / ADR-009): the same invocation +/// WITHOUT `--quiet` DOES emit the warning. +#[test] +fn unknown_rule_warning_suppressed_by_quiet_stdin() { + use std::io::Write; + let dir = tempfile::tempdir().unwrap(); + write_unknown_rule_config(dir.path()); + + // Positive control: without --quiet the warning fires on the stdin path. + let mut loud_child = mds_bin() + .arg("lint") + .arg("-") + .current_dir(dir.path()) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .unwrap(); + // Ignore BrokenPipe -- child may exit before reading all of stdin. + let _ = loud_child.stdin.take().unwrap().write_all(b"Hello!\n"); + let loud = loud_child.wait_with_output().unwrap(); + assert!( + String::from_utf8_lossy(&loud.stderr).contains("unknown lint rule"), + "positive control: stdin mode without --quiet must warn; got: {}", + String::from_utf8_lossy(&loud.stderr) + ); + + // AC-224-22: --quiet must suppress the warning on the stdin path. + let mut child = mds_bin() + .arg("lint") + .arg("-") + .arg("--quiet") + .current_dir(dir.path()) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .unwrap(); + // Ignore BrokenPipe -- child may exit before reading all of stdin. + let _ = child.stdin.take().unwrap().write_all(b"Hello!\n"); + let out = child.wait_with_output().unwrap(); + + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + !stderr.contains("unknown lint rule") && !stderr.contains("no-such-rule-xyzzy"), + "AC-224-22 (stdin): --quiet must suppress the unknown-rule warning; got: {stderr}" + ); + assert_eq!( + out.status.code(), + loud.status.code(), + "AC-224-22 (stdin): --quiet must not move the exit code; got stderr: {stderr}" + ); +} + +/// AC-224-12 (NEGATIVE / fix behaviour unchanged): an unknown rule name changes nothing +/// about `--fix`, `--fix --check` or `--fix --diff`. +/// +/// Two identical trees are built; only one names `no-such-rule-xyzzy` alongside the same +/// valid rules. For each of the three invocations the exit code, the post-run file bytes +/// and the stdout are compared between the trees. +/// +/// Non-vacuity (PF-013): the run asserts a fix was ACTUALLY applied (the file bytes moved) +/// and that the warning fired in the unknown-rule tree — a parity assertion between two +/// trees where nothing happened would prove nothing. +#[test] +fn unknown_rule_does_not_change_fix_behaviour() { + /// Build a tree with one auto-fixable (Tier A) duplicate-export violation. + fn build_tree(rules: serde_json::Value) -> (tempfile::TempDir, std::path::PathBuf) { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("lint_error.mds"); + fs::copy(fixture("lint_error.mds"), &target).unwrap(); + write_rules_config(dir.path(), rules); + (dir, target) + } + + let valid_rules = serde_json::json!({ "duplicate-export": "error" }); + let mut with_rules = valid_rules.clone(); + with_rules["no-such-rule-xyzzy"] = serde_json::json!("warn"); + + // ── --fix --check: must not write, must report pending fixes ──────────── + let (with_dir, with_target) = build_tree(with_rules.clone()); + let (without_dir, without_target) = build_tree(valid_rules.clone()); + let with_before = fs::read(&with_target).unwrap(); + + let with_check = lint_path(with_dir.path(), &["--fix", "--check"]); + let without_check = lint_path(without_dir.path(), &["--fix", "--check"]); + assert!( + String::from_utf8_lossy(&with_check.stderr).contains("unknown lint rule"), + "non-vacuity: the unknown-rule tree must warn" + ); + assert_eq!( + with_check.status.code(), + without_check.status.code(), + "AC-224-12: --fix --check exit code must not move" + ); + assert_eq!( + fs::read(&with_target).unwrap(), + with_before, + "AC-224-12: --fix --check must not write" + ); + + // ── --fix --diff: identical diff output, still no write ───────────────── + // + // The `---`/`+++` headers name the absolute file path, which necessarily differs + // between the two tempdirs. Normalise each tree's own root to a placeholder so the + // comparison is about the diff CONTENT — the only thing an unknown rule name could + // plausibly change — rather than about tempdir naming. + let with_diff = lint_path(with_dir.path(), &["--fix", "--diff"]); + let without_diff = lint_path(without_dir.path(), &["--fix", "--diff"]); + let normalize = |bytes: &[u8], root: &std::path::Path| { + String::from_utf8_lossy(bytes).replace(root.to_string_lossy().as_ref(), "") + }; + let with_diff_text = normalize(&with_diff.stdout, with_dir.path()); + let without_diff_text = normalize(&without_diff.stdout, without_dir.path()); + assert!( + with_diff_text.contains("@export greet"), + "non-vacuity: --fix --diff must actually render a hunk; got: {with_diff_text}" + ); + assert_eq!( + with_diff_text, without_diff_text, + "AC-224-12: --fix --diff output must be identical once the tempdir root is normalised" + ); + assert_eq!( + with_diff.status.code(), + without_diff.status.code(), + "AC-224-12: --fix --diff exit code must not move" + ); + assert_eq!( + fs::read(&with_target).unwrap(), + with_before, + "AC-224-12: --fix --diff must not write" + ); + + // ── --fix: identical result bytes and exit code, and a fix really happened ── + let with_fix = lint_path(with_dir.path(), &["--fix"]); + let without_fix = lint_path(without_dir.path(), &["--fix"]); + assert_eq!( + with_fix.status.code(), + without_fix.status.code(), + "AC-224-12: --fix exit code must not move" + ); + let with_after = fs::read(&with_target).unwrap(); + let without_after = fs::read(&without_target).unwrap(); + assert_eq!( + with_after, without_after, + "AC-224-12: the fixed file bytes must be identical with and without the unknown rule" + ); + assert_ne!( + with_after, with_before, + "non-vacuity: a fix must actually have been applied, or the parity above is empty" + ); + + // No stray artefacts left behind in either tree. + for dir in [with_dir.path(), without_dir.path()] { + for entry in fs::read_dir(dir).unwrap() { + let name = entry.unwrap().file_name(); + let name = name.to_string_lossy(); + assert!( + name == "lint_error.mds" || name == "mds.json", + "AC-224-12: --fix must leave no temp or backup artefact; found {name}" + ); + } + } +} + +/// AC-224-11 (HUMAN format): stdout stays empty, and the warning is the ONLY delta on +/// stderr — every file is still linted and reported exactly as it was. +/// +/// The JSON arm is pinned by `unknown_rule_json_stdout_is_byte_identical_to_run_without_it`. +/// Human format routes diagnostics through a completely different renderer +/// (`render_result_human` → `eprint_error` → miette, on stderr) and writes nothing to +/// stdout, so a JSON-only assertion proves nothing here — this arm has to be asserted +/// separately (PF-007 reasoning applied within one surface: two output formats are two +/// renderers). +/// +/// The SAME tempdir is reused for both runs, with only `mds.json` rewritten between +/// them, so the display paths are byte-identical and the stderr comparison is about +/// content rather than tempdir naming. +/// +/// Non-vacuity (PF-013 / ADR-009): the run asserts the warning IS present in the first +/// run and ABSENT in the second, and that the control run's stderr actually carries +/// per-file diagnostics — comparing two empty buffers would prove nothing. +#[test] +fn unknown_rule_human_stdout_empty_and_per_file_output_unchanged() { + let dir = tempfile::tempdir().unwrap(); + write_mixed_lint_tree(dir.path()); + + let run = || { + mds_bin() + .arg("lint") + .arg(dir.path()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap() + }; + + // Run 1: config names an unknown rule alongside a valid one. + write_rules_config( + dir.path(), + serde_json::json!({ "unused-variable": "warn", "no-such-rule-xyzzy": "warn" }), + ); + let with = run(); + // Run 2: identical tree and identical paths, unknown entry deleted. + write_rules_config(dir.path(), serde_json::json!({ "unused-variable": "warn" })); + let without = run(); + + let with_stderr = String::from_utf8_lossy(&with.stderr); + let without_stderr = String::from_utf8_lossy(&without.stderr); + + // Positive control: the warning fired in run 1 and not in run 2. + assert!( + with_stderr.contains("unknown lint rule") && with_stderr.contains("no-such-rule-xyzzy"), + "non-vacuity: the unknown-rule warning must fire on stderr in human mode; \ + got: {with_stderr}" + ); + assert!( + !without_stderr.contains("unknown lint rule"), + "control run must not warn; got: {without_stderr}" + ); + // Non-vacuity: the control run really does carry per-file lint output, so the + // equality assertion below is comparing real diagnostics, not two empty buffers. + assert!( + without_stderr.contains("unused-variable"), + "non-vacuity: the fixture tree must produce per-file diagnostics on stderr; \ + got: {without_stderr}" + ); + + // AC-224-11: human format writes NOTHING to stdout, with or without the unknown rule. + assert!( + with.stdout.is_empty(), + "AC-224-11: human-format stdout must be empty on the unknown-rule path; got: {}", + String::from_utf8_lossy(&with.stdout) + ); + assert!( + without.stdout.is_empty(), + "AC-224-11: human-format stdout must be empty in the control run; got: {}", + String::from_utf8_lossy(&without.stdout) + ); + + // AC-224-11: the added warning line is the ONLY difference on stderr — no file is + // skipped and no per-file result changes. + let with_minus_warning: String = with_stderr + .lines() + .filter(|l| !l.contains("unknown lint rule")) + .map(|l| format!("{l}\n")) + .collect(); + let without_normalized: String = without_stderr.lines().map(|l| format!("{l}\n")).collect(); + assert_eq!( + with_minus_warning, without_normalized, + "AC-224-11: apart from the added warning line, human-mode stderr must be \ + identical with and without the unknown rule name;\nwith:\n{with_stderr}\n\ + without:\n{without_stderr}" + ); + + // AC-224-13: the exit code does not move because of the unknown rule name. + assert_eq!( + with.status.code(), + without.status.code(), + "AC-224-13: the exit code must not move because of an unknown rule name in human mode" + ); +} + +/// AC-224-10 / AC-224-19 (nested config): the `mds.json` lives in a SUBDIRECTORY and the +/// lint root itself has none. +/// +/// This is a distinct `LintDirCtx::config_for` branch from the other unknown-rule tests: +/// the root's `base_dir` resolves to `None` (default config, cached under `base_dir` only) +/// while the subdirectory resolves to `Some((config, config_dir))`. Both branches are +/// exercised in one invocation, so a regression that emitted the warning from the `None` +/// arm, or that failed to reach the `Some` arm at all, is caught here. +/// +/// Non-vacuity (PF-013 / ADR-009): the run asserts the warning fires exactly once AND +/// that both files were actually linted (the root file is reported too), so neither the +/// count nor the JSON comparison can pass because nothing ran. +#[test] +fn unknown_rule_nested_config_warns_once_and_envelope_unchanged() { + let dir = tempfile::tempdir().unwrap(); + let sub = dir.path().join("sub"); + std::fs::create_dir_all(&sub).unwrap(); + // Root file: governed by NO config (the lint root has no mds.json). + std::fs::write( + dir.path().join("root.mds"), + "---\nroot_unused: v\n---\nHi!\n", + ) + .unwrap(); + // Subdirectory file: governed by sub/mds.json, which names the unknown rule. + std::fs::write(sub.join("nested.mds"), "---\nsub_unused: v\n---\nHo!\n").unwrap(); + + let run = || { + mds_bin() + .arg("lint") + .arg(dir.path()) + .arg("--format") + .arg("json") + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap() + }; + + write_rules_config( + &sub, + serde_json::json!({ "unused-variable": "warn", "no-such-rule-xyzzy": "warn" }), + ); + let with = run(); + write_rules_config(&sub, serde_json::json!({ "unused-variable": "warn" })); + let without = run(); + + let with_stderr = String::from_utf8_lossy(&with.stderr); + + // AC-224-19: exactly one warning, emitted from the subdirectory's config only. + let warning_count = with_stderr + .lines() + .filter(|l| l.contains("unknown lint rule")) + .count(); + assert_eq!( + warning_count, 1, + "AC-224-19 (nested): the subdirectory config must warn exactly once; got stderr:\n\ + {with_stderr}" + ); + assert!( + with_stderr.contains("no-such-rule-xyzzy"), + "non-vacuity: the warning must name the unknown rule; got: {with_stderr}" + ); + assert!( + !String::from_utf8_lossy(&without.stderr).contains("unknown lint rule"), + "control run must not warn; got: {}", + String::from_utf8_lossy(&without.stderr) + ); + + // AC-224-10: the stdout envelope is byte-identical across the two runs. + assert_eq!( + with.stdout, + without.stdout, + "AC-224-10 (nested): stdout must be byte-identical with and without the unknown \ + rule name;\nwith: {}\nwithout: {}", + String::from_utf8_lossy(&with.stdout), + String::from_utf8_lossy(&without.stdout) + ); + assert_eq!( + with.status.code(), + without.status.code(), + "AC-224-13 (nested): the exit code must not move" + ); + + // Non-vacuity: BOTH files were linted — the root file (default config, no warning) + // and the nested file (subdir config). A byte-identical comparison of two envelopes + // that never reached the nested directory would prove nothing. + let parsed: serde_json::Value = + serde_json::from_str(String::from_utf8_lossy(&with.stdout).trim()) + .expect("stdout must be valid JSON"); + let files: Vec = parsed["files"] + .as_array() + .expect("files must be an array") + .iter() + .map(|f| f["file"].as_str().unwrap_or_default().to_string()) + .collect(); + assert!( + files.iter().any(|f| f == "root.mds"), + "non-vacuity: the root file must still be linted; got files: {files:?}" + ); + assert!( + files.iter().any(|f| f == "sub/nested.mds"), + "non-vacuity: the nested file must still be linted; got files: {files:?}" + ); +} diff --git a/crates/mds-cli/tests/security.rs b/crates/mds-cli/tests/security.rs index fb3004b3..ba9fda7a 100644 --- a/crates/mds-cli/tests/security.rs +++ b/crates/mds-cli/tests/security.rs @@ -546,8 +546,9 @@ fn build_cli_authored_error_message_escapes_control_bytes() { // Both are reproduced-first vectors: on the pre-fix binary each command below emits // the raw hostile bytes (verified with `od -c`). -/// T-ESC-RULE-1 [security-11 / CWE-150 / PF-004 / PF-013 / #176]: an unknown lint rule -/// NAME from `mds.json` reaches stderr escaped. +/// T-ESC-RULE-1 [security-11 / CWE-150 / PF-004 / PF-013 / #176 / AC-224-4 / AC-224-5]: +/// an unknown lint rule NAME from `mds.json` reaches stderr escaped; lint warns and +/// continues rather than hard-failing. /// /// Vector: `mds.json` is read from the working tree and its rule names are arbitrary /// JSON object keys. A JSON `\uXXXX` escape decodes to a real byte, so a repository can @@ -569,6 +570,11 @@ fn build_cli_authored_error_message_escapes_control_bytes() { /// `assert_no_control_chars`, which permits `\n` — so it certified the fix while the /// forgery still worked. A rule name is a JSON object key: never legitimately /// multi-line, so it is WIRE per the spec §7.5 per-field rule. +/// +/// **AC-224-4**: run across multiple terminal widths to prove the warning never wraps +/// (eprint_warning → bare eprintln!, independent of COLUMNS). The multi-width loop +/// produces byte-identical output at all seven column values and empirically confirms +/// the single-line invariant, consistent with the sibling T-ESC-RULE-2 test. #[test] fn lint_unknown_rule_name_escapes_control_bytes() { let dir = tempfile::tempdir().unwrap(); @@ -590,73 +596,214 @@ fn lint_unknown_rule_name_escapes_control_bytes() { ) .unwrap(); - let out = mds_bin() - .arg("lint") - .arg(dir.path()) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .output() - .unwrap(); - - let stderr = String::from_utf8_lossy(&out.stderr); + // AC-224-4: run across multiple terminal widths to prove the singular warning + // never wraps (eprint_warning → bare eprintln!, independent of COLUMNS). + for columns in [40u32, 60, 80, 100, 120, 160, 200] { + let out = mds_bin() + .arg("lint") + .arg(dir.path()) + .env("COLUMNS", columns.to_string()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); - // ── Non-vacuity: the warning actually fired, naming the rule ───────────── - assert!( - stderr.contains("unknown lint rule"), - "non-vacuity: the unknown-rule warning must be the one rendered; got: {stderr}" - ); - assert!( - stderr.contains("EVIL"), - "non-vacuity: the rule name itself must be printed; got: {stderr}" - ); + let stderr = String::from_utf8_lossy(&out.stderr); - // ── Negative: no raw hostile byte survives ─────────────────────────────── - assert!( - !out.stderr.contains(&0x1Bu8), - "raw ESC byte (0x1B) must not reach stderr from an mds.json rule name; got: {stderr}" - ); - assert_no_control_chars(&stderr, "mds lint unknown-rule warning"); + // ── Non-vacuity: the warning fired, naming the rule and the recognised list ── + assert!( + stderr.contains("unknown lint rule"), + "COLUMNS={columns}: the unknown-rule warning must be rendered; got: {stderr}" + ); + assert!( + stderr.contains("EVIL"), + "COLUMNS={columns}: the rule name itself must be printed; got: {stderr}" + ); + assert!( + stderr.contains("recognised rules are"), + "COLUMNS={columns}: the recognised-rules list must appear; got: {stderr}" + ); - // ── Negative: neither forged line appears on a line of its own ─────────── - // - // `assert_no_control_chars` deliberately permits `\n` so it can be used on HUMAN-mode - // prose, so it cannot see this. Mirrors T-ESC-FNAME-1's standalone-line assertion. - for forged in ["Clean: totally-real.mds", "OK: all-fine.mds"] { + // ── Negative: no raw hostile byte survives ─────────────────────────────── assert!( - !stderr.lines().any(|l| l.trim() == forged), - "an mds.json rule name must not be able to forge the standalone status line \ - {forged:?}; got: {stderr}" + !out.stderr.contains(&0x1Bu8), + "COLUMNS={columns}: raw ESC byte (0x1B) must not reach stderr from an mds.json rule name; got: {stderr}" ); - } + assert_no_control_chars(&stderr, "mds lint unknown-rule warning"); + + // ── Negative: neither forged line appears on a line of its own ─────────── + // + // `assert_no_control_chars` deliberately permits `\n` so it can be used on HUMAN-mode + // prose, so it cannot see this. Mirrors T-ESC-FNAME-1's standalone-line assertion. + // The rule name carries TWO newlines so "Clean: totally-real.mds" lands as a + // middle segment that would appear standalone if safe_inline were removed — live. + for forged in ["Clean: totally-real.mds", "OK: all-fine.mds"] { + assert!( + !stderr.lines().any(|l| l.trim() == forged), + "COLUMNS={columns}: an mds.json rule name must not be able to forge the standalone \ + status line {forged:?}; got: {stderr}" + ); + } - // ── Positive: the escaped literals are present ─────────────────────────── - for escaped in ["\\u001B", "\\u202E", "\\u061C"] { + // ── Positive: the escaped literals are present ─────────────────────────── + for escaped in ["\\u001B", "\\u202E", "\\u061C"] { + assert!( + stderr.contains(escaped), + "COLUMNS={columns}: {escaped} must appear in the unknown-rule warning; got: {stderr}" + ); + } + assert_eq!( + stderr.matches("\\u000A").count(), + 2, + "COLUMNS={columns}: both embedded newlines must be escaped to their WIRE literal; got: {stderr}" + ); + + // ── Non-vacuity: the whole rule name landed on ONE line ────────────────── + // + // AC-224-4: the multi-width loop proves the warning occupies exactly one line + // at every terminal width — eprint_warning never wraps. + let warning_lines: Vec<&str> = stderr + .lines() + .filter(|l| l.contains("unknown lint rule")) + .collect(); + assert_eq!( + warning_lines.len(), + 1, + "COLUMNS={columns}: the warning must occupy exactly one line; got: {stderr}" + ); assert!( - stderr.contains(escaped), - "{escaped} must appear in the unknown-rule warning; got: {stderr}" + warning_lines[0].contains("EVIL") + && warning_lines[0].contains("recognised rules are") + && warning_lines[0].ends_with("; ignoring"), + "COLUMNS={columns}: the single warning line must carry the rule name, the \ + recognised-rules list, and the trailing '; ignoring'; got: {stderr}" ); } - assert_eq!( - stderr.matches("\\u000A").count(), - 2, - "both embedded newlines must be escaped to their WIRE literal; got: {stderr}" - ); +} - // ── Non-vacuity: the whole rule name landed on ONE line ────────────────── - let warning_lines: Vec<&str> = stderr - .lines() - .filter(|l| l.contains("unknown lint rule")) - .collect(); - assert_eq!( - warning_lines.len(), - 1, - "the warning must occupy exactly one line; got: {stderr}" - ); - assert!( - warning_lines[0].contains("EVIL") && warning_lines[0].ends_with("; ignoring"), - "the single warning line must carry the whole rule name and the trailing prose; \ - got: {stderr}" +/// T-ESC-RULE-2 [security-11-plural / CWE-150 / PF-004 / PF-013 / AC-224-4 / AC-224-5]: +/// two unknown lint rule names containing control bytes — the plural branch of the +/// warning emitter — reach stderr fully escaped; no raw hostile byte survives. +/// +/// Companion to T-ESC-RULE-1 which covers the singular path (one unknown name). +/// This test exercises the plural path (two or more unknown names) so that +/// AD-224-3's per-name safe_inline shape is verified for the multi-name loop. +/// +/// Both rule names contain control bytes from different escape sub-classes to +/// confirm that each is independently escaped before it is assembled into the +/// comma-separated list. The multi-width COLUMNS loop proves the plural warning +/// also lands on exactly one line at every terminal width. +#[test] +fn lint_plural_unknown_rule_names_escape_control_bytes() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("a.mds"), "Hello!\n").unwrap(); + + // Two distinct hostile rule names — ESC + colour code in the first, + // RTL-override + Arabic-letter-mark in the second — plus embedded newlines + // carrying forged status lines in each. + // Two newlines in rule_a: "Clean: real-a.mds" is the middle segment that would + // land standalone if safe_inline were removed — makes the forged-line assertion live. + let rule_a = format!( + "{}[31mAAA{}RULE\nClean: real-a.mds\nOK: fake-a.mds", + '\u{1b}', '\u{202e}' ); + let rule_b = format!("BBB{}RULE\nOK: real-b.mds", '\u{061c}'); + let mut rules = serde_json::Map::new(); + rules.insert(rule_a, serde_json::Value::String("warn".to_string())); + rules.insert(rule_b, serde_json::Value::String("warn".to_string())); + let config = serde_json::json!({ "lint": { "rules": rules } }); + std::fs::write( + dir.path().join("mds.json"), + serde_json::to_string(&config).unwrap(), + ) + .unwrap(); + + // AC-224-4: run across multiple terminal widths to prove the plural warning + // never wraps (eprint_warning → bare eprintln!, independent of COLUMNS). + for columns in [40u32, 60, 80, 100, 120, 160, 200] { + let out = mds_bin() + .arg("lint") + .arg(dir.path()) + .env("COLUMNS", columns.to_string()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + + let stderr = String::from_utf8_lossy(&out.stderr); + + // ── Non-vacuity: the plural warning fired ──────────────────────────── + assert!( + stderr.contains("unknown lint rules"), + "COLUMNS={columns}: plural unknown-rule warning must be rendered; got: {stderr}" + ); + assert!( + stderr.contains("AAA"), + "COLUMNS={columns}: first hostile rule prefix must appear; got: {stderr}" + ); + assert!( + stderr.contains("BBB"), + "COLUMNS={columns}: second hostile rule prefix must appear; got: {stderr}" + ); + assert!( + stderr.contains("recognised rules are"), + "COLUMNS={columns}: recognised-rules list must appear; got: {stderr}" + ); + + // ── Negative: no raw hostile byte survives ─────────────────────────── + assert!( + !out.stderr.contains(&0x1Bu8), + "COLUMNS={columns}: raw ESC byte (0x1B) must not reach stderr; got: {stderr}" + ); + assert_no_control_chars(&stderr, "mds lint plural unknown-rule warning"); + + // ── Negative: no forged standalone line ────────────────────────────── + for forged in ["Clean: real-a.mds", "OK: real-b.mds"] { + assert!( + !stderr.lines().any(|l| l.trim() == forged), + "COLUMNS={columns}: rule name must not forge standalone line {forged:?}; \ + got: {stderr}" + ); + } + + // ── Positive: escaped literals are present for both names ──────────── + assert!( + stderr.contains("\\u001B"), + "COLUMNS={columns}: ESC in first name must be escaped to \\u001B; got: {stderr}" + ); + assert!( + stderr.contains("\\u202E"), + "COLUMNS={columns}: RTL-override in first name must be escaped to \\u202E; \ + got: {stderr}" + ); + assert!( + stderr.contains("\\u061C"), + "COLUMNS={columns}: Arabic-letter-mark in second name must be escaped to \ + \\u061C; got: {stderr}" + ); + // Three embedded newlines total: two from rule_a, one from rule_b. + assert_eq!( + stderr.matches("\\u000A").count(), + 3, + "COLUMNS={columns}: all embedded newlines (2 from first name, 1 from second) \ + must be escaped to \\u000A; got: {stderr}" + ); + + // ── Non-vacuity: the plural warning occupies exactly ONE line ──────── + let warning_lines: Vec<&str> = stderr + .lines() + .filter(|l| l.contains("unknown lint rules")) + .collect(); + assert_eq!( + warning_lines.len(), + 1, + "COLUMNS={columns}: plural warning must occupy exactly one line; got: {stderr}" + ); + assert!( + warning_lines[0].ends_with("; ignoring"), + "COLUMNS={columns}: warning line must end with '; ignoring'; got: {stderr}" + ); + } } /// T-ESC-FNAME-1 [S14 / CWE-117 / PF-013 / #176]: a filename containing newlines diff --git a/crates/mds-core/src/lib.rs b/crates/mds-core/src/lib.rs index 39304a61..c5c30c9d 100644 --- a/crates/mds-core/src/lib.rs +++ b/crates/mds-core/src/lib.rs @@ -61,12 +61,13 @@ pub(crate) mod value; pub use formatter::{format_str, format_str_named, format_str_with}; pub use fs::{effective_parent, FileSystem, NativeFs, VirtualFs}; pub use lint::{ - fix, named_source_for_render, neutralize_source_for_render, sanitize_control_chars, - sanitize_control_chars_wire, FixLineSpan, LintConfig, LintDiagnostic, LintResult, Severity, - TextEdit, + find_unknown_rule_names, fix, format_unknown_rule_names_warning, named_source_for_render, + neutralize_source_for_render, sanitize_control_chars, sanitize_control_chars_wire, FixLineSpan, + LintConfig, LintDiagnostic, LintResult, Severity, TextEdit, UnknownRuleNames, KNOWN_LINT_RULES, }; pub use options::{ - format_unknown_keys_error, json_type_name, parse_json_vars, reject_unknown_json_keys, VarsError, + attach_lint_warnings, format_unknown_keys_error, json_type_name, parse_json_vars, + reject_unknown_json_keys, VarsError, }; pub use resolver::ModuleCache; pub use source_path::relativize_source; diff --git a/crates/mds-core/src/lint/config.rs b/crates/mds-core/src/lint/config.rs index 4fb79ba7..5ed5f350 100644 --- a/crates/mds-core/src/lint/config.rs +++ b/crates/mds-core/src/lint/config.rs @@ -3,12 +3,174 @@ //! `LintConfig` is public (lives in mds-core) — the CLI converts the `mds.json` //! `lint.rules` section into it, and all `lint_*` entry points accept a `&LintConfig`. //! -//! **Unknown rule NAMEs** are preserved in the map (warn-and-ignore at the CLI layer). +//! **Unknown rule NAMEs** do not cause construction failures — the rule simply has no +//! effect and lint continues. The preferred construction path for surfaces that must +//! surface unknowns is [`LintConfig::from_rules_checked`]: it returns the config and +//! an unknowns report in one atomic call, making it structurally impossible to miss +//! the detection step. [`find_unknown_rule_names`] remains available for callers that +//! perform detection separately. mds-core itself emits no warning. The CLI's +//! `mds lint` warns on stderr; napi/WASM/Python return `lint_warnings`; `mds build`, +//! `check`, `fmt`, and `watch` do not warn — an accepted asymmetry, not an oversight. +//! This is deliberate forward-compatibility: severities are a closed set, but rule +//! names grow every release; hard-failing an unknown name would break a config naming +//! a newer rule when run with an older binary. //! **Unknown severity VALUES** fail loudly via serde deserialization error (closed enum). use std::collections::HashMap; use super::diagnostic::Severity; +use super::rules; + +/// All known lint rule names, sorted lexicographically. +/// +/// AD-224-2: assembled from each rule module's own `RULE` const (the single +/// source of truth for the string). The omission risk — a new module whose `RULE` +/// is never listed — is closed by the bidirectional tier table in `tier.rs`. +/// +/// PF-015: this list is accurate as of this release. Future releases may add +/// entries; code that gates on this list should be prepared for it to grow. +pub const KNOWN_LINT_RULES: &[&str] = rules::ALL_RULE_NAMES; + +/// The set of rule names in a `rules` map that are not registered with the lint engine. +/// +/// AD-224-1: this is NOT an error. Under the 2026-08-12 ruling, an unknown rule name +/// warns and lint continues — the rule simply has no effect. See [`find_unknown_rule_names`]. +/// +/// The names are sorted lexicographically. This type is `#[non_exhaustive]`: +/// use the [`UnknownRuleNames::names`] accessor, not a struct literal. +/// +/// This type is `#[non_exhaustive]` per ADR-010. It is constructible only through +/// the library — external crates MUST NOT build it via a struct literal. +#[non_exhaustive] +#[derive(Debug, Clone)] +pub struct UnknownRuleNames { + /// Sorted, non-empty list of unknown rule names. + names: Vec, +} + +impl UnknownRuleNames { + fn new(mut names: Vec) -> Self { + // `sort_unstable` rather than `sort`: the names come from `HashMap` keys and are + // therefore distinct, so stability is unobservable — and the stable merge sort + // monomorphised for `String` is measurably larger in the WASM binary, which runs + // against a hard size guard (AC-224-18). + names.sort_unstable(); + UnknownRuleNames { names } + } + + /// The unknown rule names, sorted lexicographically. + /// + /// Always non-empty — `UnknownRuleNames` is only constructed when at least + /// one unknown name is present. + pub fn names(&self) -> &[String] { + &self.names + } +} + +/// Detect rule names in `rules` that are not registered in [`KNOWN_LINT_RULES`]. +/// +/// Returns `None` when every name in `rules` is known. Returns `Some(UnknownRuleNames)` +/// when at least one unknown name is found; names inside are sorted lexicographically. +/// +/// # Examples +/// +/// ``` +/// use std::collections::HashMap; +/// use mds::{Severity, find_unknown_rule_names, KNOWN_LINT_RULES}; +/// +/// // All-known map → no unknowns. +/// let known = HashMap::from([("unused-variable".to_string(), Severity::Off)]); +/// assert!(find_unknown_rule_names(&known).is_none()); +/// +/// // Map with an unknown name → Some with that name. +/// let mixed = HashMap::from([ +/// ("unused-variable".to_string(), Severity::Off), +/// ("no-such-rule".to_string(), Severity::Warn), +/// ]); +/// let u = find_unknown_rule_names(&mixed).expect("should have unknown"); +/// assert_eq!(u.names(), &["no-such-rule".to_string()]); +/// assert_eq!(KNOWN_LINT_RULES.len(), 10); +/// ``` +pub fn find_unknown_rule_names(rules: &HashMap) -> Option { + let unknowns: Vec = rules + .keys() + .filter(|k| !KNOWN_LINT_RULES.contains(&k.as_str())) + .cloned() + .collect(); + if unknowns.is_empty() { + None + } else { + Some(UnknownRuleNames::new(unknowns)) + } +} + +/// Format the warning message for the unknown lint rule names in `unknown`. +/// +/// AD-224-4: both the offending-name list and the recognised-rules list are +/// sorted lexicographically in the output, so the message is byte-identical +/// across runs and surfaces regardless of `HashMap` iteration order. +/// +/// The empty-input precondition is **structural, not asserted**: the only way to +/// obtain an [`UnknownRuleNames`] is [`find_unknown_rule_names`], which returns +/// `None` rather than an empty report. This function therefore cannot panic and +/// has no failure mode (PF-005 — a `debug_assert!` here would be a no-op in +/// release, and a release `assert!` would be a panic in a library). +/// +/// Each name is WIRE-escaped with [`sanitize_control_chars_wire`] before it is +/// interpolated (spec §7.5 per-field rule: a rule name is a single-line +/// identifier, never prose). A rule name is an arbitrary caller-supplied map key +/// and JSON `\uXXXX` escapes decode to real control bytes, so the escape is what +/// makes the returned string safe for a consumer to render (applies ADR-008, +/// avoids PF-014 — the input is sanitized, not the rendered output). The +/// recognised-rules list needs no escaping: it is a slice of compile-time string +/// literals. +/// +/// Called by the CLI and by the bindings (napi/WASM/Python) to build their +/// warning strings. The CLI wraps the returned string with a `"warning: in +/// mds.json: "` prefix so the source-file provenance appears on stderr; the +/// bindings use the returned string as-is for the `lint_warnings` field. +/// The print-discipline guard is satisfied on the CLI surface by passing the +/// return value through `safe_inline` inside `eprint_warning`'s `format!` +/// (`safe_inline` is `sanitize_control_chars_wire` by another name; the call +/// is idempotent because names are already wire-escaped here). +/// +/// Produces one of: +/// - `"unknown lint rule 'NAME'; recognised rules are: ...; ignoring"` +/// - `"unknown lint rules: 'A', 'B'; recognised rules are: ...; ignoring"` +#[must_use] +pub fn format_unknown_rule_names_warning(unknown: &UnknownRuleNames) -> String { + // Assembled with `push_str` rather than `format!` + `join`. This function is + // reachable from the WASM binary, which runs against a hard 850,000-byte guard + // (AC-224-18); `[&str]::join` and `Vec::join` each monomorphise into + // kilobytes there, and nothing else in the WASM surface pulls them in. The output + // is byte-identical to the `format!` form. + let names = unknown.names(); + let mut out = String::with_capacity(256); + if let [only] = names { + out.push_str("unknown lint rule '"); + out.push_str(&super::sanitize_control_chars_wire(only)); + out.push('\''); + } else { + out.push_str("unknown lint rules: "); + for (i, name) in names.iter().enumerate() { + if i > 0 { + out.push_str(", "); + } + out.push('\''); + out.push_str(&super::sanitize_control_chars_wire(name)); + out.push('\''); + } + } + out.push_str("; recognised rules are: "); + for (i, rule) in KNOWN_LINT_RULES.iter().enumerate() { + if i > 0 { + out.push_str(", "); + } + out.push_str(rule); + } + out.push_str("; ignoring"); + out +} /// Per-rule severity override configuration. /// @@ -18,17 +180,20 @@ use super::diagnostic::Severity; /// ``` /// /// Absent rules default to the engine's built-in severity (defined per rule in the -/// rule catalog). Unknown rule names in the map are preserved and may emit a -/// warn-at-CLI-layer diagnostic (so forward-compat: a new rule name from a newer -/// version of mds does not break older configs). +/// rule catalog). Unknown rule names in the map do not cause construction to fail +/// — the rule simply has no effect, and lint continues. Detection is opt-in via +/// [`find_unknown_rule_names`]: mds-core itself emits no warning; callers are +/// responsible for surfacing unknowns. This is deliberate forward-compatibility: +/// a config naming a rule from a newer mds version does not break when run with +/// an older binary. /// /// Unknown severity *values* (e.g. `"verbose"`) cause a hard parse error (`exit 2`) /// because the closed enum has no sensible fallback. /// /// This type is `#[non_exhaustive]`: new fields may be added in minor releases. /// Use `LintConfig::default()` for a config with all rules at engine defaults, or -/// [`LintConfig::from_rules`] to supply per-rule overrides; do not construct via -/// struct literal. +/// [`LintConfig::from_rules_checked`] (preferred) to supply per-rule overrides and +/// receive an unknowns report in a single step. Do not construct via struct literal. #[non_exhaustive] #[derive(Debug, Default, Clone)] pub struct LintConfig { @@ -38,6 +203,59 @@ pub struct LintConfig { } impl LintConfig { + /// Construct a `LintConfig` with the given per-rule overrides AND detect any + /// unknown rule names in one atomic step. + /// + /// This is the **preferred construction path** for any surface that must warn + /// about unknown rule names (the 2026-08-12 ruling: unknown names warn and lint + /// continues — the silence is the only thing that changes). The return type + /// carries the unknowns report alongside the config so the caller cannot + /// accidentally omit the detection step — compare with [`LintConfig::from_rules`], + /// where forgetting a follow-up [`find_unknown_rule_names`] call silently reverts + /// to the pre-warning behaviour on that surface (R12 in the PR plan). + /// + /// Marked `#[must_use]` — the compiler emits a warning if the return value is + /// discarded entirely, making it structurally harder to drop the unknowns report. + /// Callers that deliberately do not need the report should write: + /// `let (config, _) = LintConfig::from_rules_checked(map);` + /// + /// The asymmetry with unknown severities is deliberate and unchanged: severity + /// values are a closed enum and hard-fail at the serde layer; rule names grow + /// every release, so they warn rather than fail (AD-224-1). + /// + /// # Examples + /// + /// ``` + /// use std::collections::HashMap; + /// use mds::{LintConfig, Severity}; + /// + /// // All-known map: unknowns report is None. + /// let (config, unknown) = LintConfig::from_rules_checked(HashMap::from([ + /// ("unused-variable".to_string(), Severity::Off), + /// ])); + /// assert!(unknown.is_none()); + /// assert_eq!(config.severity_for("unused-variable"), Some(&Severity::Off)); + /// + /// // Map with an unknown name alongside a known one: report fires, config loads. + /// let (config2, unknown2) = LintConfig::from_rules_checked(HashMap::from([ + /// ("no-such-rule".to_string(), Severity::Warn), + /// ("unused-variable".to_string(), Severity::Error), + /// ])); + /// let u = unknown2.expect("should detect unknown"); + /// assert_eq!(u.names(), &["no-such-rule".to_string()]); + /// // The config still loads — the known sibling is queryable at its configured severity. + /// // The unknown name is stored verbatim in the map, but the lint engine never queries + /// // it (no rule implementation matches "no-such-rule"), so it has no effect on linting. + /// assert_eq!(config2.severity_for("unused-variable"), Some(&Severity::Error)); + /// ``` + #[must_use] + pub fn from_rules_checked( + rules: HashMap, + ) -> (Self, Option) { + let unknown = find_unknown_rule_names(&rules); + (LintConfig { rules }, unknown) + } + /// Construct a `LintConfig` with the given per-rule severity overrides. /// /// This is the supported construction path for external crates — struct literals @@ -47,16 +265,33 @@ impl LintConfig { /// or `with_*` only when taking `self`. This function does not take `self`, /// so it is named `from_rules`. /// + /// **Prefer [`LintConfig::from_rules_checked`]** — it returns the unknowns report + /// in a single call, making it structurally impossible to miss the detection step. + /// `from_rules` remains available for external crates that have already performed + /// the unknowns check separately; it will not be removed without a major-version bump. + /// + /// Unknown rule names in the map do not cause this constructor to fail — the rule + /// simply has no effect, and lint continues. mds-core itself emits no warning; + /// call [`find_unknown_rule_names`] on the same map before passing it here if you + /// need to inspect or surface those names. + /// /// # Examples /// /// ``` /// use std::collections::HashMap; + /// #[allow(deprecated)] /// use mds::{LintConfig, Severity}; + /// #[allow(deprecated)] /// let config = LintConfig::from_rules(HashMap::from([ /// ("unused-variable".to_string(), Severity::Off), /// ])); /// assert_eq!(config.severity_for("unused-variable"), Some(&Severity::Off)); /// ``` + #[deprecated( + since = "0.4.0", + note = "prefer `LintConfig::from_rules_checked`, which returns the unknowns report \ + in one atomic call — all built-in callers have migrated" + )] #[must_use] pub fn from_rules(rules: HashMap) -> Self { LintConfig { rules } @@ -70,3 +305,75 @@ impl LintConfig { self.rules.get(rule) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn unknowns(names: &[&str]) -> UnknownRuleNames { + let map: HashMap = names + .iter() + .map(|n| ((*n).to_string(), Severity::Warn)) + .collect(); + find_unknown_rule_names(&map).expect("names must be unknown") + } + + /// AC-224-2 / AC-224-3: singular form names the rule and every recognised rule. + #[test] + fn warning_singular_names_rule_and_full_registry() { + let msg = format_unknown_rule_names_warning(&unknowns(&["no-such-rule"])); + assert_eq!( + msg, + format!( + "unknown lint rule 'no-such-rule'; recognised rules are: {}; ignoring", + KNOWN_LINT_RULES.join(", ") + ) + ); + for rule in KNOWN_LINT_RULES { + assert!(msg.contains(rule), "message must list {rule}; got: {msg}"); + } + } + + /// AC-224-3: multiple offenders are listed in sorted order, not map order. + #[test] + fn warning_plural_sorts_offenders() { + // Inserted zzz-first; the output must still be aaa-first. + let msg = format_unknown_rule_names_warning(&unknowns(&["zzz-bad", "aaa-bad", "mmm-bad"])); + assert!( + msg.starts_with("unknown lint rules: 'aaa-bad', 'mmm-bad', 'zzz-bad'; "), + "offenders must be sorted lexicographically; got: {msg}" + ); + } + + /// AC-224-4 / ADR-008: a hostile rule name is WIRE-escaped by the formatter, so + /// the string handed to a binding consumer carries no raw control byte. + /// + /// PF-018: the hostile characters are built from Rust `\u{..}` escapes at + /// runtime — never authored as literal bytes in this file. + #[test] + fn warning_wire_escapes_hostile_rule_name() { + let hostile = format!( + "{}[31mEVIL{}X\nClean: totally-real.mds", + '\u{1b}', '\u{202e}' + ); + let msg = format_unknown_rule_names_warning(&unknowns(&[&hostile])); + + // Negative: no raw control byte survives. + assert!( + !msg.chars().any(|c| c.is_control()), + "no raw control character may survive escaping; got: {msg:?}" + ); + // Positive control (PF-013): the escaped literals are actually present, so the + // negative assertion above cannot pass because the name never reached the message. + for escaped in ["\\u001B", "\\u202E", "\\u000A"] { + assert!( + msg.contains(escaped), + "{escaped} must appear in the escaped warning; got: {msg}" + ); + } + assert!( + msg.contains("EVIL"), + "non-vacuity: the rule name must reach the message; got: {msg}" + ); + } +} diff --git a/crates/mds-core/src/lint/diagnostic.rs b/crates/mds-core/src/lint/diagnostic.rs index b55e4106..2b784a3a 100644 --- a/crates/mds-core/src/lint/diagnostic.rs +++ b/crates/mds-core/src/lint/diagnostic.rs @@ -193,8 +193,8 @@ use crate::limits::MAX_DIAGNOSTICS; /// `Error` renders as an error; produces exit code 2. /// /// Serialization: `"off"` / `"info"` / `"warn"` / `"error"` (closed enum — unknown -/// severity VALUE strings fail loudly via serde deserialization error, not -/// warn-and-ignore; only unknown rule NAMES get the lenient treatment). +/// severity VALUE strings fail loudly via serde deserialization error. Unknown rule +/// NAMES emit a warning and lint continues; see `find_unknown_rule_names`). #[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)] #[serde(rename_all = "lowercase")] pub enum Severity { diff --git a/crates/mds-core/src/lint/mod.rs b/crates/mds-core/src/lint/mod.rs index afb5fa8c..566a01b8 100644 --- a/crates/mds-core/src/lint/mod.rs +++ b/crates/mds-core/src/lint/mod.rs @@ -2,7 +2,7 @@ //! //! The engine runs AFTER the check gate (resolve+validate) passes, confirming the //! template compiles correctly. It then independently tokenizes and parses the entry -//! source for a single-pass facts walk, applies the 9 lint rules as plain +//! source for a single-pass facts walk, applies the 10 lint rules as plain //! functions, and returns a `LintResult`. //! //! ## Pipeline (per file) @@ -32,7 +32,10 @@ pub mod fix; pub(crate) mod rules; pub(crate) mod tier; -pub use config::LintConfig; +pub use config::{ + find_unknown_rule_names, format_unknown_rule_names_warning, LintConfig, UnknownRuleNames, + KNOWN_LINT_RULES, +}; pub use diagnostic::{ named_source_for_render, neutralize_source_for_render, sanitize_control_chars, sanitize_control_chars_wire, FixLineSpan, LintDiagnostic, LintResult, Severity, TextEdit, diff --git a/crates/mds-core/src/lint/rules/legacy_interpolation.rs b/crates/mds-core/src/lint/rules/legacy_interpolation.rs index 54c87d8f..42724bd8 100644 --- a/crates/mds-core/src/lint/rules/legacy_interpolation.rs +++ b/crates/mds-core/src/lint/rules/legacy_interpolation.rs @@ -21,7 +21,7 @@ use crate::lint::config::LintConfig; use crate::lint::diagnostic::{LintDiagnostic, LintResultBuilder, Severity}; use crate::lint::TextEdit; -const RULE: &str = "legacy-interpolation"; +pub(crate) const RULE: &str = "legacy-interpolation"; /// Check for legacy single-brace interpolation syntax in the token stream. pub(crate) fn check( diff --git a/crates/mds-core/src/lint/rules/mod.rs b/crates/mds-core/src/lint/rules/mod.rs index bbbaead0..f0002ff4 100644 --- a/crates/mds-core/src/lint/rules/mod.rs +++ b/crates/mds-core/src/lint/rules/mod.rs @@ -30,3 +30,23 @@ pub(crate) mod unused_import; pub(crate) mod unused_variable; pub(crate) mod structural_eq; + +/// All registered lint rule names, sorted lexicographically. +/// +/// AD-224-2: composed from each rule module's own `RULE` const so the string +/// is the single source of truth. This does NOT close the *omission* risk — a +/// new module whose `RULE` is never listed here compiles fine. The mechanical +/// closure is the bidirectional tier table in `tier.rs` plus an explicit +/// `ALL_RULE_NAMES.len() == 10` pin in that table's test. +pub(crate) const ALL_RULE_NAMES: &[&str] = &[ + duplicate_export::RULE, + duplicate_import::RULE, + empty_block::RULE, + legacy_interpolation::RULE, + redundant_else::RULE, + shadow_variable::RULE, + unreachable_branch::RULE, + unused_function::RULE, + unused_import::RULE, + unused_variable::RULE, +]; diff --git a/crates/mds-core/src/lint/tier.rs b/crates/mds-core/src/lint/tier.rs index 304563a9..afb792ab 100644 --- a/crates/mds-core/src/lint/tier.rs +++ b/crates/mds-core/src/lint/tier.rs @@ -97,48 +97,84 @@ pub(crate) fn first_occurrence( mod tests { use super::*; - /// ARC-3: Every registered rule name must map to its documented FixTier. + /// AC-224-9: Every registered rule name must map to its documented FixTier, + /// and the expected-tier table must agree with the registry in BOTH directions. /// - /// Enumerating all 10 rules explicitly means that a newly-added rule which - /// falls silently into the `_ => FixTier::C` catch-all arm will cause this - /// test to fail (once its expected tier is added here), preventing silent - /// misclassification in the JSON `fixable` field and the fix planner. + /// AD-224-2 (amended): `ALL_RULE_NAMES` removes string duplication but not + /// omission risk — an 11th rule module forgotten here compiles fine. The + /// mechanical closure requires: + /// 1. A separate `EXPECTED` table with explicit (name, tier) pairs. + /// 2. `assert_eq!(EXPECTED.len(), ALL_RULE_NAMES.len())` — length pin. + /// 3. Every registry entry appears in `EXPECTED` (registry → table). + /// 4. Every `EXPECTED` entry appears in the registry (table → registry). /// - /// Tier A: duplicate-import, duplicate-export, unreachable-branch, empty-block, - /// legacy-interpolation - /// Tier B: unused-import, unused-function - /// Tier C: unused-variable, redundant-else, shadow-variable + /// A rule present in the registry but absent from `EXPECTED`, or vice versa, + /// fails this test. The `_ => FixTier::C` catch-all arm is now unreachable + /// for valid registered rules. #[test] - fn all_ten_rules_map_to_expected_tier() { - // Tier A — auto-fixable with reverify gate - assert_eq!( - rule_tier("duplicate-import"), - FixTier::A, - "duplicate-import" - ); + fn registry_and_tier_table_are_bidirectionally_consistent() { + use super::super::config::KNOWN_LINT_RULES; + use super::super::rules::ALL_RULE_NAMES; + + /// Canonical expected-tier table: (rule_name, expected_FixTier). + /// Must be updated whenever a rule is added, removed, or re-tiered. + /// Pin: len must equal ALL_RULE_NAMES.len() (10 as of this release). + const EXPECTED: &[(&str, FixTier)] = &[ + // Tier A — auto-fixable with reverify gate + ("duplicate-export", FixTier::A), + ("duplicate-import", FixTier::A), + ("empty-block", FixTier::A), + ("legacy-interpolation", FixTier::A), + ("unreachable-branch", FixTier::A), + // Tier B — standalone-only fixable + ("unused-function", FixTier::B), + ("unused-import", FixTier::B), + // Tier C — report-only, never fixed + ("redundant-else", FixTier::C), + ("shadow-variable", FixTier::C), + ("unused-variable", FixTier::C), + ]; + + // Length pin: bump this when a new rule ships. assert_eq!( - rule_tier("duplicate-export"), - FixTier::A, - "duplicate-export" + EXPECTED.len(), + 10, + "EXPECTED table length must equal the number of registered rules (10)" ); assert_eq!( - rule_tier("unreachable-branch"), - FixTier::A, - "unreachable-branch" + ALL_RULE_NAMES.len(), + 10, + "ALL_RULE_NAMES length must equal the number of registered rules (10)" ); - assert_eq!(rule_tier("empty-block"), FixTier::A, "empty-block"); assert_eq!( - rule_tier("legacy-interpolation"), - FixTier::A, - "legacy-interpolation" + KNOWN_LINT_RULES.len(), + 10, + "KNOWN_LINT_RULES length must equal the number of registered rules (10)" ); - // Tier B — standalone-only fixable - assert_eq!(rule_tier("unused-import"), FixTier::B, "unused-import"); - assert_eq!(rule_tier("unused-function"), FixTier::B, "unused-function"); - // Tier C — report-only, never fixed - assert_eq!(rule_tier("unused-variable"), FixTier::C, "unused-variable"); - assert_eq!(rule_tier("redundant-else"), FixTier::C, "redundant-else"); - assert_eq!(rule_tier("shadow-variable"), FixTier::C, "shadow-variable"); + + // Direction 1: every registry entry has a correct entry in EXPECTED. + for &name in ALL_RULE_NAMES { + let got = rule_tier(name); + let entry = EXPECTED.iter().find(|(n, _)| *n == name); + let expected_tier = entry + .unwrap_or_else(|| { + panic!("rule {name:?} is in the registry but missing from EXPECTED") + }) + .1 + .clone(); + assert_eq!( + got, expected_tier, + "rule_tier({name:?}) should be {expected_tier:?} per EXPECTED" + ); + } + + // Direction 2: every EXPECTED entry exists in the registry. + for &(name, ref _tier) in EXPECTED { + assert!( + ALL_RULE_NAMES.contains(&name), + "rule {name:?} is in EXPECTED but missing from ALL_RULE_NAMES (registry)" + ); + } } /// is_output_neutral: legacy-interpolation is NOT output-neutral; all other diff --git a/crates/mds-core/src/options.rs b/crates/mds-core/src/options.rs index b1b9ba05..b5f9f59e 100644 --- a/crates/mds-core/src/options.rs +++ b/crates/mds-core/src/options.rs @@ -1,12 +1,15 @@ -//! Shared options-parsing utilities for WASM and napi binding layers. +//! Shared options-parsing and wire-format utilities for WASM and napi binding layers. //! //! Both binding layers accept a user-supplied `vars` object and need to: //! 1. Determine the runtime type-name of an arbitrary JSON value. //! 2. Validate and convert a JSON vars object into a `HashMap`. //! 3. Reject unknown option keys with a uniform error message. +//! 4. Inject `lint_warnings` into the canonical-JSON result for unknown rule names. //! -//! Centralising these three functions here eliminates identical copies that -//! previously lived in `mds-wasm/src/lib.rs` and `mds-napi/src/lib.rs`. +//! Centralising these functions here eliminates identical copies that previously +//! lived in `mds-wasm/src/lib.rs` and `mds-napi/src/lib.rs`, and ensures the +//! D8 wire contract (`lint_warnings` key name, shape, absent-when-empty semantics) +//! has a single authoritative definition. use std::collections::HashMap; @@ -193,6 +196,47 @@ pub fn reject_unknown_json_keys( Err(format_unknown_keys_error(&unknowns, known)) } +// ── attach_lint_warnings ────────────────────────────────────────────────────── + +/// Inject `lint_warnings` into a canonical JSON result object when a warning is present. +/// +/// D8 (AC-224-1): the napi, WASM, and Python bindings surface unknown-rule warnings +/// by adding a `lint_warnings: string[]` field to the returned JSON object. This +/// function is the single implementation of that D8 wire contract — the key name +/// `"lint_warnings"`, the array-of-one shape, and the absent-when-empty semantics +/// — so the contract cannot diverge across surfaces. +/// +/// `Option` rather than `Vec`: there is exactly one warning message +/// today (unknown rule names are reported as a single sentence), so a vector would be +/// over-general plumbing. The JSON shape is still `string[]` — the array is built +/// here — so adding a second warning kind later is a change to this function, not to +/// the wire contract. +/// +/// Deliberately kept out of `LintResult::to_canonical_json` so the CLI serializer +/// path (`--format json`) remains byte-frozen: the CLI writes the warning to stderr +/// via `eprint_warning` and never touches the JSON. +/// +/// The precondition (the target value is a JSON object produced by +/// `LintResult::to_canonical_json`) is **structural**: the argument type +/// `&mut serde_json::Map` is unrepresentable for non-object +/// values, so the caller must extract the map explicitly before calling. This eliminates +/// the silent-discard failure mode that would exist with a `serde_json::Value` parameter +/// (PF-005: make preconditions structural rather than asserted at runtime). Callers +/// obtain a map reference via `value.as_object_mut().expect(…)` — the `expect` is the +/// correct tool because `LintResult::to_canonical_json` is contractually guaranteed to +/// return a JSON object. +pub fn attach_lint_warnings( + json: &mut serde_json::Map, + warning: Option, +) { + if let Some(w) = warning { + json.insert( + "lint_warnings".to_string(), + serde_json::Value::Array(vec![serde_json::Value::String(w)]), + ); + } +} + // ── Tests ───────────────────────────────────────────────────────────────────── #[cfg(test)] @@ -373,4 +417,37 @@ mod tests { "Conversion should have a source" ); } + + // ── attach_lint_warnings ────────────────────────────────────────────────── + + /// D8: a present warning is injected as `lint_warnings: [string]`. + /// + /// PF-013 / ADR-009: both directions are tested — present warning inserts + /// the field; absent warning leaves the object unchanged. + #[test] + fn attach_lint_warnings_injects_field_when_warning_present() { + let mut json = json!({ "version": 1 }); + let obj = json.as_object_mut().expect("json! produces an object"); + attach_lint_warnings(obj, Some("unknown lint rule 'foo'; ignoring".into())); + let arr = json["lint_warnings"] + .as_array() + .expect("lint_warnings must be an array"); + assert_eq!(arr.len(), 1, "exactly one element"); + assert_eq!( + arr[0].as_str().unwrap(), + "unknown lint rule 'foo'; ignoring" + ); + } + + /// D8: no `lint_warnings` key is added when warning is absent (absent-when-empty semantics). + #[test] + fn attach_lint_warnings_leaves_object_unchanged_when_no_warning() { + let mut json = json!({ "version": 1 }); + let obj = json.as_object_mut().expect("json! produces an object"); + attach_lint_warnings(obj, None); + assert!( + json.get("lint_warnings").is_none(), + "lint_warnings must be absent when no warning; got: {json:?}" + ); + } } diff --git a/crates/mds-core/tests/api_surface.rs b/crates/mds-core/tests/api_surface.rs index 0e7f456f..c1888e83 100644 --- a/crates/mds-core/tests/api_surface.rs +++ b/crates/mds-core/tests/api_surface.rs @@ -1086,6 +1086,7 @@ fn lint_api_signatures_exist() { /// L-API-2: pub types LintDiagnostic, Severity, LintConfig, LintResult are accessible /// and have the expected fields/variants. #[test] +#[allow(deprecated)] // exercises LintConfig::from_rules for surface coverage fn lint_types_exist() { // Severity has four variants with lowercase serde names. let _off = Severity::Off; @@ -1130,6 +1131,161 @@ fn lint_types_exist() { assert!(!result.truncated); } +/// AC-224-7 / AC-224-8: KNOWN_LINT_RULES and UnknownRuleNames honour ADR-010. +/// +/// - `KNOWN_LINT_RULES` is publicly reachable from an external crate. +/// - It contains exactly the ten registered rule names. +/// - Every entry in the registry is accepted by `LintConfig::from_rules` without +/// error or warning (exercises the deprecated constructor for surface coverage). +/// - `find_unknown_rule_names` returns `None` for all-known maps and `Some` for +/// maps containing unknown names. +/// - `UnknownRuleNames` exposes names via accessor, not a public field, and is +/// only constructible through the library. +#[test] +#[allow(deprecated)] // exercises LintConfig::from_rules for surface coverage +fn known_lint_rules_and_unknown_detection() { + use mds::{find_unknown_rule_names, KNOWN_LINT_RULES}; + + // AC-224-7: exactly 10 rules. + assert_eq!( + KNOWN_LINT_RULES.len(), + 10, + "KNOWN_LINT_RULES must have exactly 10 entries" + ); + + // AC-224-7: exact sorted contents. + let expected = [ + "duplicate-export", + "duplicate-import", + "empty-block", + "legacy-interpolation", + "redundant-else", + "shadow-variable", + "unreachable-branch", + "unused-function", + "unused-import", + "unused-variable", + ]; + assert_eq!( + KNOWN_LINT_RULES, &expected, + "KNOWN_LINT_RULES must match the expected sorted list" + ); + + // AC-224-8: every known rule is accepted by from_rules without unknowns. + let all_known: HashMap = KNOWN_LINT_RULES + .iter() + .map(|&n| (n.to_string(), Severity::Warn)) + .collect(); + let _config = LintConfig::from_rules(all_known.clone()); + assert!( + find_unknown_rule_names(&all_known).is_none(), + "all-known rules map must produce no unknowns" + ); + + // AC-224-8: empty rules map produces no unknowns. + let empty: HashMap = HashMap::new(); + assert!( + find_unknown_rule_names(&empty).is_none(), + "empty rules map must produce no unknowns" + ); + + // AC-224-7: UnknownRuleNames is only obtainable via the library API. + let mixed: HashMap = HashMap::from([ + ("unused-variable".to_string(), Severity::Off), + ("no-such-rule".to_string(), Severity::Warn), + ("another-bad".to_string(), Severity::Error), + ]); + let unknown = find_unknown_rule_names(&mixed).expect("should detect two unknown rules"); + // Accessor returns names; struct literal construction is impossible (#[non_exhaustive]). + let names = unknown.names(); + assert_eq!( + names, + &["another-bad".to_string(), "no-such-rule".to_string()], + "names must be sorted lexicographically" + ); + assert_eq!(names.len(), 2); + + // Positive control (PF-013 / ADR-009): find_unknown_rule_names does NOT return None + // for a single-unknown map. + let one_bad: HashMap = + HashMap::from([("no-such-rule".to_string(), Severity::Warn)]); + assert!( + find_unknown_rule_names(&one_bad).is_some(), + "single-unknown map must produce Some" + ); +} + +/// Review finding (config.rs:104) — `from_rules_checked` is the structurally-safe +/// construction path. +/// +/// - An all-known map returns `(config, None)`. +/// - A map with unknowns returns `(config, Some(UnknownRuleNames))`. +/// - Both arms return a usable `LintConfig` (lint always continues). +/// - `from_rules_checked` is `#[must_use]`: the compiler warns if the caller +/// discards the return value entirely, making it structurally harder to miss +/// the detection step compared with calling `from_rules` and `find_unknown_rule_names` +/// separately. +#[test] +fn from_rules_checked_structurally_returns_unknowns() { + use mds::{LintConfig, KNOWN_LINT_RULES}; + + // All-known map: config is usable, unknowns is None. + let all_known: HashMap = KNOWN_LINT_RULES + .iter() + .map(|&n| (n.to_string(), Severity::Warn)) + .collect(); + let (config, unknown) = LintConfig::from_rules_checked(all_known); + assert!( + unknown.is_none(), + "all-known map must return None for unknowns" + ); + assert_eq!( + config.severity_for("unused-variable"), + Some(&Severity::Warn), + "config must still be usable after from_rules_checked" + ); + + // Empty map: config is usable, unknowns is None. + let (empty_config, empty_unknown) = LintConfig::from_rules_checked(HashMap::new()); + assert!( + empty_unknown.is_none(), + "empty map must return None for unknowns" + ); + assert!( + empty_config.severity_for("unused-variable").is_none(), + "empty config must have no overrides" + ); + + // Map with unknown names: config still loads, unknowns is Some. + let mixed: HashMap = HashMap::from([ + ("unused-variable".to_string(), Severity::Off), + ("no-such-rule".to_string(), Severity::Warn), + ("another-bad".to_string(), Severity::Error), + ]); + let (config2, unknown2) = LintConfig::from_rules_checked(mixed); + let u = unknown2.expect("from_rules_checked must detect two unknown rules"); + // Accessor returns sorted names. + assert_eq!( + u.names(), + &["another-bad".to_string(), "no-such-rule".to_string()], + "names must be sorted lexicographically" + ); + // The config still loads — the unknown rules have no effect but the valid one does. + assert_eq!( + config2.severity_for("unused-variable"), + Some(&Severity::Off), + "valid rule must still be present in config after detection" + ); + + // Positive control (PF-013): a single-unknown map produces Some, not None. + let (_, one_bad_unknown) = + LintConfig::from_rules_checked(HashMap::from([("bad-rule".to_string(), Severity::Warn)])); + assert!( + one_bad_unknown.is_some(), + "single-unknown map must produce Some from from_rules_checked" + ); +} + /// L-API-4: MdsError enum is unchanged — lint findings are LintDiagnostic, not MdsError variants. #[test] fn mds_error_variants_unchanged_by_lint() { diff --git a/crates/mds-napi/README.md b/crates/mds-napi/README.md index 679f1e16..43fac915 100644 --- a/crates/mds-napi/README.md +++ b/crates/mds-napi/README.md @@ -63,10 +63,10 @@ Source-map options are **not accepted** — check does not generate output. ### `lint(source, opts?)` / `lintFile(path, opts?)` / `lintVirtual(modules, entry, opts?)` Static analysis. Returns the canonical lint JSON: -`{ version: 1, files: [{file, diagnostics: [{rule, severity, message, help, fixable, fix_edits, span?},...]},...], truncated: bool }` +`{ version: 1, files: [{file, diagnostics: [{rule, severity, message, help, fixable, fix_edits, span?},...]},...], truncated: bool, lint_warnings?: string[] }` Options: `basePath` (lint only — lintFile derives the base from the file path; lintVirtual resolves against the module map), `vars`, `rules` (`Record`). -Unknown rule names in `rules` are silently accepted (a typo has no effect); unknown severity values throw `mds::invalid_options`. +Unknown rule names in `rules` emit a warning and lint continues — the unknown name has no effect but `result.lint_warnings` (a `string[]` field, absent when empty) is populated so callers can surface the issue; unknown severity values throw `mds::invalid_options`. See `index.d.ts` for the full typed surface. diff --git a/crates/mds-napi/__test__/index.spec.mjs b/crates/mds-napi/__test__/index.spec.mjs index f0066e06..c7110265 100644 --- a/crates/mds-napi/__test__/index.spec.mjs +++ b/crates/mds-napi/__test__/index.spec.mjs @@ -1585,3 +1585,112 @@ describe('ESC-injection hardening (issue #176 / CWE-150)', () => { ); }); }); + +// ── AC-224 D8: unknown rule name warning channel on napi ───────────────────── +// +// An unknown rule name in the `rules` option produces a `lint_warnings` array +// in the result rather than hard-failing (D8 / AC-224-1). Known rule names +// produce no `lint_warnings` field (common-case cleanliness). + +describe('unknown rule name warning (AC-224 D8)', () => { + // L-N-WARN-1: lint() with unknown rule name returns lint_warnings array + test('L-N-WARN-1: lint with unknown rule name returns lint_warnings', () => { + const result = lint('Hello!\n', { rules: { 'no-such-rule-xyzzy': 'warn' } }); + assert.equal(result.version, 1, 'version must be 1'); + assert.ok(Array.isArray(result.lint_warnings), 'lint_warnings must be an array'); + assert.ok(result.lint_warnings.length > 0, 'lint_warnings must be non-empty'); + assert.ok( + result.lint_warnings[0].includes('no-such-rule-xyzzy'), + `lint_warnings[0] must name the unknown rule; got: ${result.lint_warnings[0]}`, + ); + assert.ok( + result.lint_warnings[0].includes('recognised rules are') || + result.lint_warnings[0].includes('recognized rules are'), + `lint_warnings[0] must list recognised rules; got: ${result.lint_warnings[0]}`, + ); + }); + + // L-N-WARN-2: lint() with only known rules has no lint_warnings + test('L-N-WARN-2: lint with only known rules has no lint_warnings', () => { + const result = lint('Hello!\n', { rules: { 'unused-variable': 'off' } }); + assert.equal(result.version, 1); + assert.ok( + result.lint_warnings === undefined || result.lint_warnings.length === 0, + `lint_warnings must be absent or empty for known rules; got: ${JSON.stringify(result.lint_warnings)}`, + ); + }); + + // L-N-WARN-3: lintFile() with unknown rule name returns lint_warnings + test('L-N-WARN-3: lintFile with unknown rule name returns lint_warnings', () => { + const result = lintFile(SIMPLE_MDS, { rules: { 'no-such-rule-xyzzy': 'warn' } }); + assert.equal(result.version, 1); + assert.ok(Array.isArray(result.lint_warnings), 'lint_warnings must be an array'); + assert.ok(result.lint_warnings.length > 0, 'lint_warnings must be non-empty'); + assert.ok( + result.lint_warnings[0].includes('no-such-rule-xyzzy'), + `lint_warnings[0] must name the unknown rule; got: ${result.lint_warnings[0]}`, + ); + }); + + // L-N-WARN-4: lintVirtual() with unknown rule name returns lint_warnings + test('L-N-WARN-4: lintVirtual with unknown rule name returns lint_warnings', () => { + const modules = { 'main.mds': 'Hello!\n' }; + const result = lintVirtual(modules, 'main.mds', { rules: { 'no-such-rule-xyzzy': 'error' } }); + assert.equal(result.version, 1); + assert.ok(Array.isArray(result.lint_warnings), 'lint_warnings must be an array'); + assert.ok(result.lint_warnings.length > 0, 'lint_warnings must be non-empty'); + assert.ok( + result.lint_warnings[0].includes('no-such-rule-xyzzy'), + `lint_warnings[0] must name the unknown rule; got: ${result.lint_warnings[0]}`, + ); + }); + + // L-N-WARN-5: lint continues — files[] is still present and correctly shaped + test('L-N-WARN-5: lint continues when rule name is unknown (files[] present)', () => { + const result = lint('Hello!\n', { rules: { 'no-such-rule-xyzzy': 'warn' } }); + assert.ok(Array.isArray(result.files), 'files must be an array'); + assert.strictEqual(result.truncated, false, 'truncated must be false'); + }); + + // L-N-WARN-ESC: ADR-008 per-surface — lint_warnings must not deliver raw control bytes. + // + // The mds-core unit test `warning_wire_escapes_hostile_rule_name` proves the formatter + // escapes hostile bytes before building the string. This test proves the string that + // actually reaches a JS consumer through the napi binding carries no raw control byte + // (negative) and DOES carry the sanitized \\u001B literal (positive control, PF-013). + // + // PF-018: the ESC byte is constructed at runtime — never authored as a literal. + test('L-N-WARN-ESC: hostile rule name (ESC byte) — lint_warnings delivers escaped literal, not raw byte', () => { + const esc = String.fromCharCode(0x1b); // U+001B — runtime construction only (PF-018) + const hostileRule = esc + '[31mhostile-rule' + esc + '[0m'; + const result = lint('Hello!\n', { rules: { [hostileRule]: 'warn' } }); + + // The call must succeed and expose the warning through lint_warnings (D8 / AC-224-1). + assert.ok(Array.isArray(result.lint_warnings), 'L-N-WARN-ESC: lint_warnings must be an array'); + assert.ok(result.lint_warnings.length > 0, 'L-N-WARN-ESC: lint_warnings must be non-empty'); + + const w0 = result.lint_warnings[0]; + assert.equal(typeof w0, 'string', 'L-N-WARN-ESC: lint_warnings[0] must be a string'); + + // Negative: no raw C0 (excl. \\t \\n), DEL, or C1 byte may survive (ADR-008). + for (let i = 0; i < w0.length; i++) { + const code = w0.charCodeAt(i); + const isC0 = code < 0x20 && code !== 0x09 && code !== 0x0a; + const isDel = code === 0x7f; + const isC1 = code >= 0x80 && code <= 0x9f; + assert.ok( + !isC0 && !isDel && !isC1, + 'L-N-WARN-ESC: raw hostile char U+' + code.toString(16).toUpperCase().padStart(4, '0') + + ' at index ' + i + ' must not appear in lint_warnings[0]; got: ' + JSON.stringify(w0), + ); + } + + // Positive control (PF-013/ADR-009): the sanitized literal must be present so + // the negative above cannot pass merely because the name never reached the message. + assert.ok( + w0.includes('\\u001B'), + 'L-N-WARN-ESC: sanitized \\u001B literal must appear in lint_warnings[0]; got: ' + + JSON.stringify(w0), + ); + }); +}); diff --git a/crates/mds-napi/src/lib.rs b/crates/mds-napi/src/lib.rs index e7574375..b0f9e9f1 100644 --- a/crates/mds-napi/src/lib.rs +++ b/crates/mds-napi/src/lib.rs @@ -783,18 +783,26 @@ pub fn check_file(env: Env, path: String, opts: Option) -> napi::Result< // ── Lint options parsing ────────────────────────────────────────────────────── -/// Extract and validate the `rules` option: `Record` → `mds::LintConfig`. +/// Extract and validate the `rules` option: `Record` → `(mds::LintConfig, Vec)`. /// /// Returns the default config (all rules at built-in defaults) when `rules` is absent, /// `null`, or `undefined`. Validates each severity value against the closed enum. -fn extract_rules_direct(env: &Env, obj: &Object) -> napi::Result { +/// +/// D8 (AC-224-1): also detects unknown rule names and returns them as warning strings +/// so callers can surface them in the `lint_warnings` field of the returned JSON object. +/// This is the binding warning channel: napi `lint`/`lintFile`/`lintVirtual` add +/// `lint_warnings: string[]` to their return value when unknown rule names are present. +fn extract_rules_direct( + env: &Env, + obj: &Object, +) -> napi::Result<(mds::LintConfig, Option)> { if !obj.has_named_property("rules")? { - return Ok(mds::LintConfig::default()); + return Ok((mds::LintConfig::default(), None)); } let val: Unknown = obj.get_named_property_unchecked("rules")?; let vt = val.get_type()?; match vt { - ValueType::Undefined | ValueType::Null => Ok(mds::LintConfig::default()), + ValueType::Undefined | ValueType::Null => Ok((mds::LintConfig::default(), None)), ValueType::Object => { // Deserialize the rules sub-object; js arrays also satisfy Object so // we guard against that in the JSON shape check below. @@ -832,7 +840,13 @@ fn extract_rules_direct(env: &Env, obj: &Object) -> napi::Result Err(throw_options_error( env, @@ -847,34 +861,43 @@ fn extract_rules_direct(env: &Env, obj: &Object) -> napi::Result, Option>, mds::LintConfig, + Option, ); fn parse_lint_opts(env: &Env, opts: Option) -> napi::Result { let Some(opts_obj) = opts else { - return Ok((None, None, mds::LintConfig::default())); + return Ok((None, None, mds::LintConfig::default(), None)); }; reject_unknown_napi_keys(env, &opts_obj, &["basePath", "vars", "rules"])?; let base_path = extract_base_path_direct(env, &opts_obj)?; let vars = extract_vars_direct(env, &opts_obj)?; - let lint_config = extract_rules_direct(env, &opts_obj)?; + let (lint_config, lint_warnings) = extract_rules_direct(env, &opts_obj)?; - Ok((base_path, vars, lint_config)) + Ok((base_path, vars, lint_config, lint_warnings)) } /// Parse options for `lintFile` (file-path variant). /// /// Valid keys: `vars`, `rules`. `basePath` is not accepted (derived from file path). -type LintFileOpts = (Option>, mds::LintConfig); +/// Returns `(vars, lint_config, lint_warnings)`. +type LintFileOpts = ( + Option>, + mds::LintConfig, + Option, +); fn parse_lint_file_opts(env: &Env, opts: Option) -> napi::Result { let Some(opts_obj) = opts else { - return Ok((None, mds::LintConfig::default())); + return Ok((None, mds::LintConfig::default(), None)); }; if opts_obj.has_named_property("basePath")? { @@ -887,9 +910,9 @@ fn parse_lint_file_opts(env: &Env, opts: Option) -> napi::Result) -> napi::Result) -> napi::Result { let Some(opts_obj) = opts else { - return Ok((None, mds::LintConfig::default())); + return Ok((None, mds::LintConfig::default(), None)); }; if opts_obj.has_named_property("basePath")? { @@ -912,9 +935,9 @@ fn parse_lint_virtual_opts(env: &Env, opts: Option) -> napi::Result) -> napi::Result) -> napi::Result { check_source_size(&env, &source)?; - let (base_path, vars, lint_config) = parse_lint_opts(&env, opts)?; + let (base_path, vars, lint_config, lint_warnings) = parse_lint_opts(&env, opts)?; let result = run_catching( &env, @@ -957,7 +985,14 @@ pub fn lint(env: Env, source: String, opts: Option) -> napi::Result) -> napi::Result) -> napi::Result { - let (vars, lint_config) = parse_lint_file_opts(&env, opts)?; + let (vars, lint_config, lint_warnings) = parse_lint_file_opts(&env, opts)?; let path_buf = PathBuf::from(path); let result = run_catching( @@ -985,7 +1020,14 @@ pub fn lint_file(env: Env, path: String, opts: Option) -> napi::Result bool: ... @property def files(self) -> list[LintFileReport]: ... + @property + def lint_warnings(self) -> list[str]: + """Non-fatal warnings raised while configuring the lint run. + + An unknown rule name in the ``rules`` mapping produces one entry here: + the rule is not enforced (it does not exist) but the call still + succeeds. Empty in the common case. + """ def __new__(cls, canonical: Mapping[str, Any]) -> LintResult: ... def to_dict(self) -> dict[str, Any]: ... def to_json(self) -> str: ... diff --git a/crates/mds-python/src/lib.rs b/crates/mds-python/src/lib.rs index 990cd4fe..cee5b0e3 100644 --- a/crates/mds-python/src/lib.rs +++ b/crates/mds-python/src/lib.rs @@ -844,6 +844,34 @@ impl LintResult { .unwrap_or(false) } + /// Warnings produced during linting — for example, unknown rule names + /// passed in the `rules` mapping (D8 / AC-224-1). + /// + /// Returns an empty list when no warnings occurred (the common case). + /// Each element is a human-readable warning string. + /// + /// **Absent-when-empty convention:** the backing JSON key `lint_warnings` is + /// omitted when no warnings occurred. As a result `"lint_warnings" not in + /// r.to_dict()` and `r.lint_warnings == []` both describe the same "no warnings" + /// state — the Python attribute returns `[]` (idiomatic Python empty default) + /// while `to_dict()` omits the key entirely. This is consistent with the + /// TypeScript surface (`lint_warnings?: string[]`) and intentionally asymmetric + /// with per-diagnostic optional fields (`help`, `span`, `fix_edits`) which follow + /// the always-present-key-with-JSON-null convention inside each diagnostic object. + #[getter] + fn lint_warnings(&self) -> Vec { + self.value + .get("lint_warnings") + .and_then(serde_json::Value::as_array) + .map(|arr| { + arr.iter() + .filter_map(serde_json::Value::as_str) + .map(str::to_owned) + .collect() + }) + .unwrap_or_default() + } + fn __repr__(&self) -> String { let n = self .value @@ -902,8 +930,8 @@ fn sanitize_json_str_field(obj: &mut serde_json::Map, } } -/// Sanitize all message, help, and file string fields in a canonical lint result value -/// in-place. +/// Sanitize all string fields in a canonical lint result value in-place: per-file +/// `file`, per-diagnostic `message` and `help`, and every string in `lint_warnings`. /// /// Called in [`LintResult::new`] so any data arriving through the /// `LintResult(canonical)` / pickle path is sanitized before the typed getters or @@ -911,28 +939,48 @@ fn sanitize_json_str_field(obj: &mut serde_json::Map, /// `LintResult::to_canonical_json()` does on the live lint path, closing the /// parallel-path gap (PF-004). /// +/// The live lint path is safe because `format_unknown_rule_names_warning` +/// WIRE-escapes each string before `mds::attach_lint_warnings` injects it. The +/// `LintResult(canonical)` / pickle path must also sanitize `lint_warnings` because +/// callers may supply untrusted canonical data containing hostile control bytes. +/// /// **No re-sort (AD-202-1b):** `to_canonical_json()` preserves caller-supplied /// order for `LintResult::new` callers; `sort_diagnostics` is the single ordering /// choke point, called only from `LintResultBuilder::build`. This function /// intentionally mirrors that contract — it sanitizes but does not reorder. fn sanitize_lint_value(value: &mut serde_json::Value) { - let Some(files) = value.get_mut("files").and_then(|v| v.as_array_mut()) else { - return; - }; - for file_val in files.iter_mut() { - let Some(obj) = file_val.as_object_mut() else { - continue; - }; - sanitize_json_str_field(obj, "file"); - let Some(diags) = obj.get_mut("diagnostics").and_then(|v| v.as_array_mut()) else { - continue; - }; - for d in diags.iter_mut() { - let Some(d_obj) = d.as_object_mut() else { + // Sanitize per-file string fields. + if let Some(files) = value.get_mut("files").and_then(|v| v.as_array_mut()) { + for file_val in files.iter_mut() { + let Some(obj) = file_val.as_object_mut() else { continue; }; - sanitize_json_str_field(d_obj, "message"); - sanitize_json_str_field(d_obj, "help"); + sanitize_json_str_field(obj, "file"); + let Some(diags) = obj.get_mut("diagnostics").and_then(|v| v.as_array_mut()) else { + continue; + }; + for d in diags.iter_mut() { + let Some(d_obj) = d.as_object_mut() else { + continue; + }; + sanitize_json_str_field(d_obj, "message"); + sanitize_json_str_field(d_obj, "help"); + } + } + } + // Sanitize lint_warnings strings (PF-004: the live path escapes via + // format_unknown_rule_names_warning before injection, but LintResult(canonical) / + // pickle callers may supply untrusted data directly). + if let Some(arr) = value + .get_mut("lint_warnings") + .and_then(|v| v.as_array_mut()) + { + for item in arr.iter_mut() { + if let serde_json::Value::String(s) = item { + if let std::borrow::Cow::Owned(sanitized) = mds::sanitize_control_chars_wire(s) { + *s = sanitized; + } + } } } } @@ -1245,17 +1293,24 @@ fn parse_modules(py: Python<'_>, modules: &Bound<'_, PyAny>) -> PyResult, rules: Option<&Bound<'_, PyAny>>) -> PyResult { +/// +/// D8 (AC-224-1): unknown rule NAMES are detected and returned as warning strings +/// in the second element of the tuple. Callers add them to the `LintResult` JSON +/// as `lint_warnings: list[str]` — the Python binding warning channel. +fn extract_rules( + py: Python<'_>, + rules: Option<&Bound<'_, PyAny>>, +) -> PyResult<(mds::LintConfig, Option)> { let Some(obj) = rules else { - return Ok(mds::LintConfig::default()); + return Ok((mds::LintConfig::default(), None)); }; if obj.is_none() { - return Ok(mds::LintConfig::default()); + return Ok((mds::LintConfig::default(), None)); } let json: serde_json::Value = depythonize(obj).map_err(|e| options_error(py, &format!("invalid rules: {e}")))?; @@ -1291,7 +1346,13 @@ fn extract_rules(py: Python<'_>, rules: Option<&Bound<'_, PyAny>>) -> PyResult>, ) -> PyResult { let vars = extract_vars(py, vars.as_ref())?; - let lint_config = extract_rules(py, rules.as_ref())?; + let (lint_config, lint_warnings) = extract_rules(py, rules.as_ref())?; let result = run_catching(py, move || mds::lint(&path, vars, &lint_config))?; - Ok(LintResult { - value: result.to_canonical_json(), - }) + let mut json = result.to_canonical_json(); + // LintResult::to_canonical_json is contractually guaranteed to return a JSON object. + mds::attach_lint_warnings( + json.as_object_mut() + .expect("LintResult::to_canonical_json always returns a JSON object"), + lint_warnings, + ); + Ok(LintResult { value: json }) } /// Lint a module from an in-memory virtual filesystem. @@ -1537,13 +1608,18 @@ fn lint_virtual( ) -> PyResult { let modules = parse_modules(py, &modules)?; let vars = extract_vars(py, vars.as_ref())?; - let lint_config = extract_rules(py, rules.as_ref())?; + let (lint_config, lint_warnings) = extract_rules(py, rules.as_ref())?; let result = run_catching(py, move || { mds::lint_virtual(modules, &entry, vars, &lint_config) })?; - Ok(LintResult { - value: result.to_canonical_json(), - }) + let mut json = result.to_canonical_json(); + // LintResult::to_canonical_json is contractually guaranteed to return a JSON object. + mds::attach_lint_warnings( + json.as_object_mut() + .expect("LintResult::to_canonical_json always returns a JSON object"), + lint_warnings, + ); + Ok(LintResult { value: json }) } // ── Module ────────────────────────────────────────────────────────────────────── diff --git a/crates/mds-python/tests/test_lint.py b/crates/mds-python/tests/test_lint.py index 333b44d3..f7bb97ef 100644 --- a/crates/mds-python/tests/test_lint.py +++ b/crates/mds-python/tests/test_lint.py @@ -357,3 +357,202 @@ def test_compile_to_dict_sourcemap_present_when_requested() -> None: d = r.to_dict() assert "sourceMap" in d assert isinstance(d["sourceMap"], dict), "sourceMap must be a dict when generated" + + +# ── AC-224 D8: unknown rule name warning channel (Python) ──────────────────── +# +# An unknown rule name in the `rules` kwarg produces `lint_warnings` in the +# LintResult (D8 / AC-224-1). Known rule names produce no lint_warnings. +# Python surface: lint_warnings() is a property getter that returns list[str]. + + +def test_py_warn_l1_unknown_rule_name_returns_lint_warnings() -> None: + """lint() with unknown rule name → lint_warnings non-empty (AC-224-1/D8).""" + r = m.lint(CLEAN_SOURCE, rules={"no-such-rule-xyzzy": "warn"}) + assert r.version == 1 + warnings = r.lint_warnings + assert isinstance(warnings, list), f"lint_warnings must be a list; got: {type(warnings)}" + assert len(warnings) > 0, "lint_warnings must be non-empty for an unknown rule name" + assert "no-such-rule-xyzzy" in warnings[0], ( + f"lint_warnings[0] must name the unknown rule; got: {warnings[0]}" + ) + assert "recognised rules are" in warnings[0] or "recognized rules are" in warnings[0], ( + f"lint_warnings[0] must list recognised rules; got: {warnings[0]}" + ) + + +def test_py_warn_l2_known_rule_names_no_lint_warnings() -> None: + """lint() with only known rule names → lint_warnings empty (AC-224-1/D8).""" + r = m.lint(CLEAN_SOURCE, rules={"unused-variable": "off"}) + assert r.version == 1 + assert r.lint_warnings == [], ( + f"lint_warnings must be empty for known rule names; got: {r.lint_warnings}" + ) + + +def test_py_warn_lf1_lint_file_unknown_rule_returns_warnings(fixtures: pathlib.Path) -> None: + """lint_file() with unknown rule name → lint_warnings non-empty (AC-224-1/D8).""" + r = m.lint_file(fixtures / "simple.mds", rules={"no-such-rule-xyzzy": "error"}) + assert r.version == 1 + warnings = r.lint_warnings + assert len(warnings) > 0, "lint_warnings must be non-empty for an unknown rule name" + assert "no-such-rule-xyzzy" in warnings[0], ( + f"lint_warnings[0] must name the unknown rule; got: {warnings[0]}" + ) + + +def test_py_warn_lv1_lint_virtual_unknown_rule_returns_warnings() -> None: + """lint_virtual() with unknown rule name → lint_warnings non-empty (AC-224-1/D8).""" + modules = {"main.mds": CLEAN_SOURCE} + r = m.lint_virtual(modules, "main.mds", rules={"no-such-rule-xyzzy": "warn"}) + assert r.version == 1 + warnings = r.lint_warnings + assert len(warnings) > 0, "lint_warnings must be non-empty for an unknown rule name" + assert "no-such-rule-xyzzy" in warnings[0], ( + f"lint_warnings[0] must name the unknown rule; got: {warnings[0]}" + ) + + +def test_py_warn_l3_lint_continues_with_unknown_rule() -> None: + """lint() with unknown rule name still returns files[] and truncated (AC-224-1/D8).""" + r = m.lint(CLEAN_SOURCE, rules={"no-such-rule-xyzzy": "warn"}) + d = r.to_dict() + assert "files" in d, "files key must be present even when unknown rule name is given" + assert "truncated" in d, "truncated key must be present even when unknown rule name is given" + assert d["truncated"] is False + + +def test_py_warn_l4_to_dict_includes_lint_warnings() -> None: + """lint() with unknown rule name → to_dict() includes 'lint_warnings' key (AC-224-1/D8).""" + r = m.lint(CLEAN_SOURCE, rules={"no-such-rule-xyzzy": "warn"}) + d = r.to_dict() + assert "lint_warnings" in d, ( + f"to_dict() must include 'lint_warnings' when unknown rule names are present; got: {d}" + ) + assert isinstance(d["lint_warnings"], list) + assert len(d["lint_warnings"]) > 0 + + +def test_py_warn_l5_multiple_unknown_rules() -> None: + """lint() with multiple unknown rule names → both appear sorted lexicographically (AC-224-3). + + Names are deliberately supplied in reverse lexicographic order ('zzz-bad' before + 'aaa-bad') to verify that UnknownRuleNames::new sorts the offender list rather + than emitting it in HashMap iteration order (AC-224-3). + """ + r = m.lint( + CLEAN_SOURCE, + rules={"zzz-bad": "warn", "aaa-bad": "error"}, + ) + warnings = r.lint_warnings + assert len(warnings) > 0, "lint_warnings must be non-empty" + combined = " ".join(warnings) + assert "zzz-bad" in combined, ( + f"all unknown rule names must appear in lint_warnings; got: {warnings}" + ) + assert "aaa-bad" in combined, ( + f"all unknown rule names must appear in lint_warnings; got: {warnings}" + ) + # AC-224-3: offenders MUST be sorted lexicographically (not HashMap iteration order). + # 'aaa-bad' < 'zzz-bad', so 'aaa-bad' must appear first in the combined warning text. + idx_aaa = combined.index("aaa-bad") + idx_zzz = combined.index("zzz-bad") + assert idx_aaa < idx_zzz, ( + f"offenders must be sorted lexicographically: 'aaa-bad' must precede 'zzz-bad' " + f"(AC-224-3); got: {warnings}" + ) + + +def test_py_warn_l2b_known_rule_to_dict_omits_lint_warnings_key() -> None: + """lint() with only known rules → to_dict() omits 'lint_warnings' key entirely. + + Pins the absent-when-empty convention: r.lint_warnings == [] and + 'lint_warnings' not in r.to_dict() both describe the same "no warnings" state. + Positive control: test_py_warn_l4 asserts the key IS present when warnings fired, + so this absence assertion is not vacuous (ADR-009 / PF-013). + """ + r = m.lint(CLEAN_SOURCE, rules={"unused-variable": "off"}) + assert r.lint_warnings == [], "lint_warnings attribute must be empty for known rules" + d = r.to_dict() + assert "lint_warnings" not in d, ( + "to_dict() must omit 'lint_warnings' when no warnings occurred " + f"(absent-when-empty convention); got keys: {list(d.keys())}" + ) + + +def test_py_warn_canonical_sanitizes_lint_warnings() -> None: + """LintResult(canonical) sanitizes hostile control bytes in lint_warnings (PF-004). + + The live lint path is safe because format_unknown_rule_names_warning WIRE-escapes + strings before injection. The LintResult(canonical) / pickle path must also sanitize + lint_warnings because callers may supply untrusted canonical data. + + Positive control: construct an ESC byte at runtime (not as a literal, per PF-018) + and confirm the getter does NOT return the raw byte after construction. + """ + esc = chr(0x1B) # ESC — constructed at runtime, not written as a literal (PF-018) + raw_warning = f"unknown rule {esc}[31mhostile{esc}[0m" + canonical = { + "version": 1, + "files": [], + "truncated": False, + "lint_warnings": [raw_warning], + } + result = m.LintResult(canonical) + warnings = result.lint_warnings + assert len(warnings) == 1, f"expected one warning entry; got: {warnings}" + assert esc not in warnings[0], ( + f"ESC byte must be sanitized in lint_warnings via LintResult(canonical); " + f"got: {warnings[0]!r}" + ) + # The sanitized form must be non-empty: hostile bytes are replaced, not dropped. + assert len(warnings[0]) > 0, "sanitized warning must be non-empty" + # WIRE-escaping must replace raw ESC with its JSON escape sequence (6 ASCII chars). + # Construct the expected string at runtime to avoid authoring a literal control byte + # inside a string constant (PF-018: the tooling decodes backslash-u + 4 hex to real bytes). + expected_escape = "\\u001B" # backslash + u + 0 + 0 + 1 + B — six ASCII characters + assert expected_escape in warnings[0], ( + f"ESC must appear as the escaped literal \\u001B; got: {warnings[0]!r}" + ) + + +def test_py_warn_live_lint_escapes_hostile_rule_name() -> None: + """live lint() path: a hostile rule name (ESC byte) must not reach lint_warnings raw. + + ADR-008 per-surface: the mds-core unit test warning_wire_escapes_hostile_rule_name + proves the formatter escapes before building the string. This test proves the string + that actually reaches Python through the PyO3 binding carries no raw control byte + (negative) and DOES carry the sanitized literal (positive control, PF-013/ADR-009). + + PF-018: the ESC byte is constructed at runtime via chr() — never authored as a literal. + """ + esc = chr(0x1B) # U+001B — runtime construction only (PF-018) + hostile_rule = esc + "[31mhostile-rule" + esc + "[0m" + r = m.lint(CLEAN_SOURCE, rules={hostile_rule: "warn"}) + + # The call must succeed and expose the warning through lint_warnings (D8 / AC-224-1). + warnings = r.lint_warnings + assert isinstance(warnings, list), f"lint_warnings must be a list; got: {type(warnings)}" + assert len(warnings) > 0, "lint_warnings must be non-empty for a hostile unknown rule name" + + w0 = warnings[0] + assert isinstance(w0, str), f"lint_warnings[0] must be a str; got: {type(w0)}" + + # Negative: no raw C0 (excl. \t \n), DEL, or C1 byte may survive (ADR-008). + for i, ch in enumerate(w0): + code = ord(ch) + is_c0 = code < 0x20 and code not in (0x09, 0x0A) + is_del = code == 0x7F + is_c1 = 0x80 <= code <= 0x9F + assert not (is_c0 or is_del or is_c1), ( + f"raw hostile char U+{code:04X} at index {i} must not appear in " + f"lint_warnings[0]; got: {w0!r}" + ) + + # Positive control (PF-013/ADR-009): the sanitized literal must be present so + # the negative above cannot pass merely because the name never reached the message. + # Construct the expected 6-char string at runtime (PF-018). + expected_escape = "\\u001B" # backslash + u + 0 + 0 + 1 + B + assert expected_escape in w0, ( + f"sanitized \\u001B literal must appear in lint_warnings[0]; got: {w0!r}" + ) diff --git a/crates/mds-python/tests/typecheck_sample.py b/crates/mds-python/tests/typecheck_sample.py index b05d3371..21c26971 100644 --- a/crates/mds-python/tests/typecheck_sample.py +++ b/crates/mds-python/tests/typecheck_sample.py @@ -96,6 +96,10 @@ def lint_source(source: str) -> int: def lint_typed_access(source: str) -> list[str]: """Demonstrate fully-typed attribute access on lint results (B6/F10).""" result: LintResult = mdscript.lint(source) + # AC-224-1 / D8: the unknown-rule warning channel is typed as list[str], so a + # consumer can read it without a cast or a `type: ignore`. + lint_warnings: list[str] = result.lint_warnings + _ = lint_warnings rules: list[str] = [] report: LintFileReport for report in result.files: diff --git a/crates/mds-wasm/src/lib.rs b/crates/mds-wasm/src/lib.rs index dcd34420..3551fa45 100644 --- a/crates/mds-wasm/src/lib.rs +++ b/crates/mds-wasm/src/lib.rs @@ -448,11 +448,16 @@ fn parse_options(options: JsValue) -> Result { /// Parsed options for the `lint` and `lint_virtual` functions. /// /// Extends the standard options with a `rules` field — absent in `compile`/`check`. +/// +/// `lint_warnings` is populated (non-empty) when unknown rule names are found; +/// callers add it to the returned JSON as `lint_warnings: string[]` (D8 binding channel). struct ParsedLintOptions { /// Standard options (filename, extra_modules, vars). opts: ParsedOptions, /// Per-rule severity overrides parsed from `options.rules`. lint_config: mds::LintConfig, + /// Warning message for unknown rule names, if any (AC-224-1 D8 channel). + lint_warnings: Option, } /// Extract and validate the `rules` field from a lint options object. @@ -461,10 +466,13 @@ struct ParsedLintOptions { /// names and values are severity strings (`"off"` | `"info"` | `"warn"` | `"error"`). /// An absent or null/undefined `rules` key returns the default config (all rules at /// built-in defaults). An unknown severity VALUE is a hard error (closed enum). -fn extract_rules(obj: &js_sys::Object) -> Result { +/// +/// D8 (AC-224-1): unknown rule NAMES are detected and returned as warning strings. +/// Callers surface them via `lint_warnings` in the returned JSON object. +fn extract_rules(obj: &js_sys::Object) -> Result<(mds::LintConfig, Option), JsValue> { let val = get_prop_js(obj, "rules"); if val.is_undefined() || val.is_null() { - return Ok(mds::LintConfig::default()); + return Ok((mds::LintConfig::default(), None)); } // Deserialize the rules sub-object via serde_wasm_bindgen. let rules_json: serde_json::Value = serde_wasm_bindgen::from_value(val) @@ -493,7 +501,13 @@ fn extract_rules(obj: &js_sys::Object) -> Result { })?; rules.insert(key, severity); } - Ok(mds::LintConfig::from_rules(rules)) + // D8: detect unknown rule names and build config in one step via + // from_rules_checked. The return type structurally forces the caller + // to handle the unknowns report — a fifth caller cannot accidentally + // omit the detection step (review finding: config.rs:104). + let (lint_config, unknown) = mds::LintConfig::from_rules_checked(rules); + let lint_warnings = unknown.map(|u| mds::format_unknown_rule_names_warning(&u)); + Ok((lint_config, lint_warnings)) } /// Parse the JS options for `lint` and `lint_virtual`. @@ -507,6 +521,7 @@ fn parse_lint_options(options: JsValue) -> Result { return Ok(ParsedLintOptions { opts: ParsedOptions::default(), lint_config: mds::LintConfig::default(), + lint_warnings: None, }); } @@ -523,7 +538,7 @@ fn parse_lint_options(options: JsValue) -> Result { let filename = extract_filename(&obj)?; let extra_modules = extract_modules(&obj)?; let vars = extract_vars(&obj)?; - let lint_config = extract_rules(&obj)?; + let (lint_config, lint_warnings) = extract_rules(&obj)?; Ok(ParsedLintOptions { opts: ParsedOptions { @@ -534,6 +549,7 @@ fn parse_lint_options(options: JsValue) -> Result { include_sources_content: false, }, lint_config, + lint_warnings, }) } @@ -583,6 +599,7 @@ fn parse_lint_virtual_options(options: JsValue) -> Result Result Result Result { ) .map_err(mds_error_to_js)?; - to_js(&result.to_canonical_json()) + let mut json = result.to_canonical_json(); + // LintResult::to_canonical_json is contractually guaranteed to return a JSON object. + mds::attach_lint_warnings( + json.as_object_mut() + .expect("LintResult::to_canonical_json always returns a JSON object"), + lint_opts.lint_warnings, + ); + to_js(&json) })) } @@ -897,7 +922,14 @@ pub fn lint_virtual(modules: JsValue, entry: &str, options: JsValue) -> Result 0, + "W-WARN-1: lint_warnings must be non-empty for an unknown rule name" + ); + let w0 = warnings_arr + .get(0) + .as_string() + .expect("W-WARN-1: lint_warnings[0] must be a string"); + assert!( + w0.contains("no-such-rule-xyzzy"), + "W-WARN-1: lint_warnings[0] must name the unknown rule; got: {w0}" + ); + assert!( + w0.contains("recognised rules are") || w0.contains("recognized rules are"), + "W-WARN-1: lint_warnings[0] must list recognised rules; got: {w0}" + ); +} + +#[wasm_bindgen_test] +fn wasm_lint_known_rule_names_no_lint_warnings() { + // W-WARN-2 (AC-224-1/D8): lint() with only known rule names has no lint_warnings. + let opts = to_js_object(&serde_json::json!({ + "rules": { "unused-variable": "off" } + })); + let result = mds_wasm::lint("Hello!\n", opts).expect("W-WARN-2: lint must succeed"); + let lint_warnings = get_prop(&result, "lint_warnings"); + assert!( + lint_warnings.is_undefined(), + "W-WARN-2: lint_warnings must be absent for known rule names; got: {lint_warnings:?}" + ); +} + +#[wasm_bindgen_test] +fn wasm_lint_virtual_unknown_rule_name_returns_lint_warnings() { + // W-WARN-3 (AC-224-1/D8): lintVirtual() with an unknown rule name returns lint_warnings. + let modules_val = to_js_object(&serde_json::json!({ "main.mds": "Hello!\n" })); + let opts = to_js_object(&serde_json::json!({ + "rules": { "no-such-rule-xyzzy": "error" } + })); + let result = mds_wasm::lint_virtual(modules_val, "main.mds", opts) + .expect("W-WARN-3: lintVirtual must succeed"); + + let lint_warnings = get_prop(&result, "lint_warnings"); + assert!( + !lint_warnings.is_undefined() && !lint_warnings.is_null(), + "W-WARN-3: lint_warnings must be present for an unknown rule name" + ); + let warnings_arr = js_sys::Array::from(&lint_warnings); + assert!( + warnings_arr.length() > 0, + "W-WARN-3: lint_warnings must be non-empty for an unknown rule name" + ); + let w0 = warnings_arr + .get(0) + .as_string() + .expect("W-WARN-3: lint_warnings[0] must be a string"); + assert!( + w0.contains("no-such-rule-xyzzy"), + "W-WARN-3: lint_warnings[0] must name the unknown rule; got: {w0}" + ); +} + +#[wasm_bindgen_test] +fn wasm_lint_continues_with_unknown_rule_name() { + // W-WARN-4 (AC-224-1/D8): lint continues — files[] and truncated are present. + let opts = to_js_object(&serde_json::json!({ + "rules": { "no-such-rule-xyzzy": "warn" } + })); + let result = mds_wasm::lint("Hello!\n", opts).expect("W-WARN-4: lint must succeed"); + + // files[] must be present and be a genuine JS array. `js_sys::Array::from` coerces + // almost anything, so asserting on its length would pass vacuously — check + // `Array::is_array` on the raw value instead. + let files = get_prop(&result, "files"); + assert!( + js_sys::Array::is_array(&files), + "W-WARN-4: files must be a JS array even when the rule name is unknown" + ); + assert_eq!( + js_sys::Array::from(&files).length(), + 0, + "W-WARN-4: a clean source yields no file entries; the unknown rule name must not \ + add one" + ); + + // truncated must be present and false. + let truncated = get_prop(&result, "truncated").as_bool().unwrap_or(true); + assert!( + !truncated, + "W-WARN-4: truncated must be false for a clean source" + ); +} + +#[wasm_bindgen_test] +fn wasm_lint_unknown_rule_no_lint_warnings_absent_on_clean() { + // W-WARN-2b (AC-224-1/D8): no options → no lint_warnings field at all. + let result = + mds_wasm::lint("Hello!\n", JsValue::NULL).expect("W-WARN-2b: lint(NULL) must succeed"); + let lint_warnings = get_prop(&result, "lint_warnings"); + assert!( + lint_warnings.is_undefined(), + "W-WARN-2b: lint_warnings must be absent when no rules are passed; got: {lint_warnings:?}" + ); +} + +/// W-WARN-ESC (ADR-008 per-surface): a hostile rule name containing U+001B must not +/// deliver a raw control byte through `lint_warnings` to the JS consumer. +/// +/// The mds-core unit test `warning_wire_escapes_hostile_rule_name` proves the +/// formatter escapes before building the string. This test proves the string that +/// actually crosses the WASM FFI boundary carries no raw control byte (negative) and +/// DOES carry the six-character ASCII sequence backslash-u-0-0-1-B (positive control, +/// PF-013/ADR-009). +/// +/// PF-018: hostile bytes are built from Rust `\u{..}` escapes — never authored as +/// literal bytes in this source file. +#[wasm_bindgen_test] +fn wasm_lint_hostile_rule_name_escapes_control_bytes_in_lint_warnings() { + // Construct the hostile rule name at runtime using Rust char escapes (PF-018). + let hostile_rule = "\u{1b}[31mhostile-rule\u{1b}[0m".to_string(); + let opts = to_js_object(&serde_json::json!({ + "rules": { hostile_rule: "warn" } + })); + let result = mds_wasm::lint("Hello!\n", opts) + .expect("W-WARN-ESC: lint must succeed with hostile rule name"); + + // The warning must be present (D8 / AC-224-1) — call succeeds, lint_warnings is non-empty. + let lint_warnings = get_prop(&result, "lint_warnings"); + assert!( + !lint_warnings.is_undefined() && !lint_warnings.is_null(), + "W-WARN-ESC: lint_warnings must be present for a hostile unknown rule name" + ); + let warnings_arr = js_sys::Array::from(&lint_warnings); + assert!( + warnings_arr.length() > 0, + "W-WARN-ESC: lint_warnings must be non-empty" + ); + let w0 = warnings_arr + .get(0) + .as_string() + .expect("W-WARN-ESC: lint_warnings[0] must be a string"); + + // Negative: no raw control byte (C0 excl. \t \n, DEL, C1) may survive (ADR-008). + for (i, ch) in w0.char_indices() { + let code = ch as u32; + let is_c0 = code < 0x20 && code != 0x09 && code != 0x0a; + let is_del = code == 0x7f; + let is_c1 = (0x80..=0x9f).contains(&code); + assert!( + !is_c0 && !is_del && !is_c1, + "W-WARN-ESC: raw hostile char U+{code:04X} at byte {i} must not appear \ + in lint_warnings[0]; got: {w0:?}" + ); + } + + // Positive control (PF-013/ADR-009): the sanitized literal must be present so + // the negative above cannot pass merely because the name never reached the message. + assert!( + w0.contains("\\u001B"), + "W-WARN-ESC: sanitized \\u001B literal must appear in lint_warnings[0]; got: {w0:?}" + ); +} + +#[wasm_bindgen_test] +fn wasm_lint_unknown_severity_value_throws_invalid_options() { + // W-SEVER-1 (AC-224-1 — paired throw arm, lint path): unknown severity + // VALUES remain a hard error on the WASM surface. napi pins this in L-N-6; + // Python in test_l5_lint_rules_unknown_severity_raises. Per PF-007 those + // prove nothing about WASM — this test pins the throw arm here. + // + // Severities are a closed enum; "verbose" is not a valid severity string. + let opts = to_js_object(&serde_json::json!({ + "rules": { "unused-variable": "verbose" } + })); + let err = mds_wasm::lint("Hello!\n", opts).unwrap_err(); + let code = get_str(&err, "code"); + assert_eq!( + code, "mds::invalid_options", + "W-SEVER-1: unknown severity value must throw mds::invalid_options; got: {code}" + ); +} + +#[wasm_bindgen_test] +fn wasm_lint_virtual_unknown_severity_value_throws_invalid_options() { + // W-SEVER-2 (AC-224-1 — paired throw arm, lint_virtual path): mirrors + // W-SEVER-1 for lintVirtual(). Both entry points route through + // extract_rules(), so both paths are pinned (PF-007). + let modules_val = to_js_object(&serde_json::json!({ "main.mds": "Hello!\n" })); + let opts = to_js_object(&serde_json::json!({ + "rules": { "unused-variable": "verbose" } + })); + let err = mds_wasm::lint_virtual(modules_val, "main.mds", opts).unwrap_err(); + let code = get_str(&err, "code"); + assert_eq!( + code, "mds::invalid_options", + "W-SEVER-2: lint_virtual unknown severity value must throw mds::invalid_options; got: {code}" + ); +} diff --git a/examples/README.md b/examples/README.md index 4c781157..8ddfa7d2 100644 --- a/examples/README.md +++ b/examples/README.md @@ -15,7 +15,7 @@ Four capabilities shipped with v0.4.0 — each has a dedicated example: # Safety-gated formatter — rewrites directive lines only, never body text mds fmt --check examples/ -# Static analysis — 9 rules, human and JSON output, --fix --diff preview +# Static analysis — 10 rules, human and JSON output, --fix --diff preview mds lint examples/linting/ # Source Map v3 — sidecar .map file, --inline data-URI, or --embed-sources diff --git a/examples/linting/README.md b/examples/linting/README.md index 907e77af..b91bb6c7 100644 --- a/examples/linting/README.md +++ b/examples/linting/README.md @@ -186,8 +186,13 @@ surfaces `shadow-variable` as **info** (☞) and `unused-variable` as an **error > Files outside `config-demo/` find no `mds.json` and use built-in defaults. Config errors are strict: an unknown severity value or malformed JSON fails the run -with exit `2`; an unknown *rule name* prints a `warning: unknown lint rule …` and is -ignored (forward-compatible). +with exit `2`. An unknown *rule name* is handled more leniently by `mds lint`: a +`warning: unknown lint rule …` is printed to stderr, the config still loads, lint +continues, and the unknown rule is not enforced — it is skipped +(forward-compatible: a config naming a rule from a newer release warns instead of +failing on an older binary). Pass `--quiet` to suppress it. Other commands +(`mds build`, `mds check`, `mds fmt`, `mds watch`) also read `mds.json` but do +not emit the unknown-rule warning. ## Exit codes diff --git a/packages/mds-wasm/README.md b/packages/mds-wasm/README.md index 91c33ce9..0ec3c2b1 100644 --- a/packages/mds-wasm/README.md +++ b/packages/mds-wasm/README.md @@ -47,9 +47,11 @@ const checked = check(source, { vars: { name: 'World' } }); // lint(source, options) // Accepted keys: filename, modules, vars, rules. // options.rules — { [ruleName: string]: 'off' | 'info' | 'warn' | 'error' } -// Unknown rule names are silently accepted; unknown severity values throw. +// Unknown rule names emit a warning and lint continues — the unknown name has no effect +// (the rule is not enforced); unknown severity values throw. +// When unknown rule names are present, lintResult.lint_warnings is a non-empty string[]. const lintResult = lint(source, { rules: { 'shadow-variable': 'warn' } }); -// lintResult: { version: 1, files: [...], truncated: boolean } +// lintResult: { version: 1, files: [...], truncated: boolean, lint_warnings?: string[] } // lintVirtual(modules, entry, options) // modules: { [key: string]: string } — the full virtual module map. diff --git a/packages/mds/README.md b/packages/mds/README.md index 5bd913c2..072e7e05 100644 --- a/packages/mds/README.md +++ b/packages/mds/README.md @@ -147,11 +147,14 @@ and `lintVirtual`. **Source maps:** for string-source compiles (`compile`) `sources[0]` in the generated map is `"input.mds"`. For stdin builds via the CLI it is `""`. -**Lint `rules` map:** unknown rule names are silently accepted (a typo has no effect); -unknown severity values throw `mds::invalid_options`. +**Lint `rules` map:** unknown rule names emit a warning and lint continues — the unknown +name has no effect but `result.lint_warnings` (a `string[]` field, absent when empty) +is populated so callers can surface the issue. Unknown severity values throw +`mds::invalid_options`. On the CLI surface the warning goes to stderr; in binding +surfaces it appears in `lint_warnings`. **Lint result shape:** ```ts -{ version: 1, files: [{ file: string, diagnostics: LintDiagnostic[] }], truncated: boolean } +{ version: 1, files: [{ file: string, diagnostics: LintDiagnostic[] }], truncated: boolean, lint_warnings?: string[] } // LintDiagnostic: { rule, severity, message, help?, fixable, fix_edits, span? } ``` diff --git a/packages/mds/__test__/lint.spec.mjs b/packages/mds/__test__/lint.spec.mjs index ab173a10..24a3ac8a 100644 --- a/packages/mds/__test__/lint.spec.mjs +++ b/packages/mds/__test__/lint.spec.mjs @@ -15,7 +15,7 @@ import path from 'node:path'; import { spawnSync } from 'node:child_process'; import { existsSync, statSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; -import { lint, lintFile, lintVirtual, isMdsError, init } from '../dist/node.js'; +import { lint, lintFile, lintVirtual, isMdsError, init, LINT_RULE_NAMES } from '../dist/node.js'; import { assertResultShape } from '../dist/backend/contract.js'; import { initWasmNode, createWasmBackend } from '../dist/backend/wasm.js'; @@ -617,4 +617,159 @@ describe('lint canonical JSON goldens', () => { `lintVirtual silenced golden mismatch: got ${JSON.stringify(result)}`, ); }); + + // spec.md normative claim: in alphabetical key order `lint_warnings` sorts between + // `files` and `truncated`. This test pins that position in the actual JSON output. + // (ADR-009 / PF-013: verifying key ORDER requires a result that actually contains the + // key; clean-source goldens above cannot exercise this.) + test('U-LG4: lint_warnings key sorts between files and truncated in the JSON wire form', () => { + const result = lintVirtual( + { 'main.mds': CLEAN_SOURCE }, + 'main.mds', + { rules: { 'no-such-rule-xyzzy': 'warn' } }, + ); + assert.ok( + Array.isArray(result.lint_warnings) && result.lint_warnings.length > 0, + `U-LG4: lint_warnings must be a non-empty array; got ${JSON.stringify(result.lint_warnings)}`, + ); + const json = JSON.stringify(result); + const filesIdx = json.indexOf('"files"'); + const warnIdx = json.indexOf('"lint_warnings"'); + const truncIdx = json.indexOf('"truncated"'); + assert.ok( + filesIdx !== -1 && warnIdx !== -1 && truncIdx !== -1, + `U-LG4: expected files, lint_warnings, and truncated keys in serialized result; got: ${json}`, + ); + assert.ok( + filesIdx < warnIdx && warnIdx < truncIdx, + `U-LG4: lint_warnings must sort between files and truncated (alphabetical BTreeMap order); ` + + `filesIdx=${filesIdx} warnIdx=${warnIdx} truncIdx=${truncIdx} in: ${json}`, + ); + }); +}); + +// --------------------------------------------------------------------------- +// AC-224-1 / AC-224-8 (D8): unknown lint rule names on the universal surface. +// +// The universal package is the fifth surface AC-224-1 enumerates, and PF-007 says +// a per-surface assertion elsewhere proves nothing here: napi's spec proves napi, +// web.rs proves WASM. These tests assert the contract as `@mdscript/mds` exposes +// it, on the default backend AND on a directly-constructed WASM backend, so the +// claim is not silently limited to whichever backend init() happens to pick. +// +// Contract: an unknown rule name never throws. It surfaces in +// `result.lint_warnings`, and the field is ABSENT when every name is recognised. +// --------------------------------------------------------------------------- + +describe('unknown lint rule names (AC-224-1 / D8)', () => { + before(() => init()); + + /** Assert one lint result carries a warning naming `ruleName`. */ + function assertWarnsAbout(result, ruleName, label) { + assert.equal(result.version, 1, `${label}: version must be 1`); + assert.ok( + Array.isArray(result.lint_warnings), + `${label}: lint_warnings must be an array; got ${JSON.stringify(result.lint_warnings)}`, + ); + assert.ok(result.lint_warnings.length > 0, `${label}: lint_warnings must be non-empty`); + const joined = result.lint_warnings.join(' '); + assert.ok( + joined.includes(ruleName), + `${label}: lint_warnings must name ${ruleName}; got: ${joined}`, + ); + assert.ok( + joined.includes('recognised rules are'), + `${label}: lint_warnings must list the recognised rules; got: ${joined}`, + ); + } + + test('U-L-WARN-1: lint() warns and continues on an unknown rule name', () => { + const result = lint(UNUSED_SOURCE, { rules: { 'no-such-rule-xyzzy': 'warn' } }); + assertWarnsAbout(result, 'no-such-rule-xyzzy', 'U-L-WARN-1'); + // Lint CONTINUES: the real finding is still reported, unchanged. + assert.doesNotThrow(() => assertResultShape(result, 'lint')); + const rules = result.files.flatMap((f) => f.diagnostics).map((d) => d.rule); + assert.ok( + rules.includes('unused-variable'), + `U-L-WARN-1: linting must continue and still report findings; got: ${JSON.stringify(rules)}`, + ); + }); + + test('U-L-WARN-2: lintVirtual() warns on an unknown rule name', () => { + const result = lintVirtual( + { 'main.mds': CLEAN_SOURCE }, + 'main.mds', + { rules: { 'no-such-rule-xyzzy': 'error' } }, + ); + assertWarnsAbout(result, 'no-such-rule-xyzzy', 'U-L-WARN-2'); + }); + + test('U-L-WARN-3: lintFile() warns on an unknown rule name', async () => { + const result = await lintFile(LINT_WARN_MDS, { rules: { 'no-such-rule-xyzzy': 'warn' } }); + assertWarnsAbout(result, 'no-such-rule-xyzzy', 'U-L-WARN-3'); + }); + + // Paired negative arm (PF-013 / ADR-009): the positive arms above only prove the + // field can appear. Without this, an implementation that always populated + // lint_warnings would pass every assertion above. + test('U-L-WARN-4: every recognised rule name is accepted with no lint_warnings', () => { + assert.equal(LINT_RULE_NAMES.length, 10, 'LINT_RULE_NAMES must have exactly 10 entries'); + for (const name of LINT_RULE_NAMES) { + const result = lint(CLEAN_SOURCE, { rules: { [name]: 'off' } }); + assert.equal( + result.lint_warnings, + undefined, + `U-L-WARN-4: ${name} is a recognised rule — lint_warnings must be absent; ` + + `got: ${JSON.stringify(result.lint_warnings)}`, + ); + } + // Empty and absent rules maps behave identically to a recognised-only map. + assert.equal(lint(CLEAN_SOURCE, { rules: {} }).lint_warnings, undefined); + assert.equal(lint(CLEAN_SOURCE).lint_warnings, undefined); + }); + + test('U-L-WARN-5: an unknown rule name never throws (contrast: unknown severity does)', () => { + assert.doesNotThrow( + () => lint(CLEAN_SOURCE, { rules: { 'no-such-rule-xyzzy': 'warn' } }), + 'U-L-WARN-5: an unknown rule NAME must not throw', + ); + // The asymmetry is deliberate and must stay: unknown severity VALUES still throw. + assert.throws( + () => lint(CLEAN_SOURCE, { rules: { 'unused-variable': 'verbose' } }), + (err) => { + assert.ok(isMdsError(err), `expected MdsError, got: ${err}`); + assert.equal(err.code, 'mds::invalid_options', `got code: ${err.code}`); + return true; + }, + 'U-L-WARN-5: an unknown severity VALUE must still throw mds::invalid_options', + ); + }); + + // PF-007: init() prefers native wherever the napi addon resolves, so every test + // above may have exercised only one backend. Force the WASM backend directly so + // the universal contract is proven on both, mirroring U-L12's approach. + test('U-L-WARN-6: the same contract holds on a directly-constructed WASM backend', async () => { + let wasmBackend; + try { + const wasmMod = await initWasmNode(); + wasmBackend = createWasmBackend(wasmMod); + } catch (initErr) { + if (process.env.CI) { + throw new Error( + 'U-L-WARN-6: WASM backend is required in CI for AC-224-1 universal/WASM coverage. ' + + `Caused by: ${initErr.message}`, + ); + } + console.warn('U-L-WARN-6: skipping WASM surface -- mds-wasm module not built'); + return; + } + const warned = wasmBackend.lint(CLEAN_SOURCE, { rules: { 'no-such-rule-xyzzy': 'warn' } }); + assertWarnsAbout(warned, 'no-such-rule-xyzzy', 'U-L-WARN-6'); + const clean = wasmBackend.lint(CLEAN_SOURCE, { rules: { 'unused-variable': 'off' } }); + assert.equal( + clean.lint_warnings, + undefined, + `U-L-WARN-6: recognised rules must produce no lint_warnings; got: ${JSON.stringify(clean.lint_warnings)}`, + ); + }); }); diff --git a/packages/mds/src/index.ts b/packages/mds/src/index.ts index 54d3ac15..579d7a3f 100644 --- a/packages/mds/src/index.ts +++ b/packages/mds/src/index.ts @@ -11,6 +11,7 @@ export type { LintFileReport, LintOptions, LintResult, + LintRuleName, LintSpan, RuleSeverity, MdsErrorSpan, @@ -21,6 +22,6 @@ export type { MdsBaseBackend, MdsNodeBackend, } from './types.js'; -export { isMdsError } from './types.js'; +export { isMdsError, LINT_RULE_NAMES } from './types.js'; export type { WasmModule } from './backend/wasm.js'; export { initWasmNode, initWasmBrowser, createWasmBackend } from './backend/wasm.js'; diff --git a/packages/mds/src/node.ts b/packages/mds/src/node.ts index 1f941f0c..388f521f 100644 --- a/packages/mds/src/node.ts +++ b/packages/mds/src/node.ts @@ -295,7 +295,12 @@ export function getBackend(): BackendType { return assertReady().getBackend(); } -export { isMdsError } from './types.js'; +// `LINT_RULE_NAMES` is exported here, not only from `index.ts`: the package +// `exports` map resolves `@mdscript/mds` to `dist/node.js` (Node) or +// `dist/browser.js`, and never to `dist/index.js` — a value re-exported only +// from `index.ts` is unreachable for consumers. The browser entry gains it with +// the browser lint surface; today it has no lint API to configure. +export { isMdsError, LINT_RULE_NAMES } from './types.js'; export type { BackendType, CheckOptions, @@ -309,6 +314,7 @@ export type { LintFileReport, LintOptions, LintResult, + LintRuleName, LintSpan, RuleSeverity, MarkdownResult, diff --git a/packages/mds/src/types.ts b/packages/mds/src/types.ts index 4b483e87..9536ed98 100644 --- a/packages/mds/src/types.ts +++ b/packages/mds/src/types.ts @@ -166,6 +166,52 @@ export interface LintDiagnostic { */ export type RuleSeverity = 'error' | 'warn' | 'info' | 'off'; +/** + * Union of all recognised lint rule names. + * + * D-224-1 (2026-08-12 ruling): passing a key outside this set is NOT an error. + * The engine emits a warning (see {@link LintResult.lint_warnings}) and lint + * continues — the unknown rule is not enforced, but the caller is told. The + * asymmetry with unknown severity values (which are hard errors) is deliberate: + * severity is a closed set, but rule names grow every release, so hard-failing + * would break configs that name a rule added in a newer binary. + */ +export type LintRuleName = + | 'duplicate-export' + | 'duplicate-import' + | 'empty-block' + | 'legacy-interpolation' + | 'redundant-else' + | 'shadow-variable' + | 'unreachable-branch' + | 'unused-function' + | 'unused-import' + | 'unused-variable'; + +/** + * All recognised lint rule names, sorted alphabetically. + * + * D-224-2: this array is a manual mirror of the Rust `KNOWN_LINT_RULES` + * registry (composed from each rule module's own `RULE` const). The TS mirror + * is guarded in one direction only — a new rule must be added here manually + * after landing in `mds-core`. Drift is a named residual (avoids PF-015). + * + * `rules` keys not found here emit a warning (see {@link LintResult.lint_warnings}) + * and lint continues — unknown names are ignored by the engine after the warning. + */ +export const LINT_RULE_NAMES: readonly LintRuleName[] = [ + 'duplicate-export', + 'duplicate-import', + 'empty-block', + 'legacy-interpolation', + 'redundant-else', + 'shadow-variable', + 'unreachable-branch', + 'unused-function', + 'unused-import', + 'unused-variable', +] as const; + /** All diagnostics for a single file in a lint result. */ export interface LintFileReport { /** Path or name of the linted file. */ @@ -175,9 +221,12 @@ export interface LintFileReport { } /** - * Canonical lint result returned by all lint surfaces (CLI `--format json`, - * napi, WASM, Python). All surfaces produce byte-identical JSON for the same - * input and rules configuration. + * Lint result returned by the napi, WASM, and Python binding surfaces, and + * by the CLI `--format json` surface. The `files`, `truncated`, and `version` + * fields are present on every surface. The optional `lint_warnings` field is + * included in the binding-surface JSON when non-fatal warnings occurred; the + * CLI writes those warnings to stderr so its JSON stdout remains parseable + * without modification. */ export interface LintResult { /** Schema version; always 1 in this release. */ @@ -189,13 +238,26 @@ export interface LintResult { * earlier diagnostics were dropped. Re-run after fixing to surface the rest. */ truncated: boolean; + /** + * Non-fatal warnings produced during linting — for example, unknown rule + * names passed in the `rules` option. Absent (not `[]`) when no warnings + * occurred. On the CLI surface, these warnings go to stderr instead. + */ + lint_warnings?: string[]; } /** Options for source-string lint operations. */ export interface LintOptions { /** Runtime variables injected into the check gate (not the lint rules). */ vars?: Record; - /** Per-rule severity overrides, e.g. `{ 'shadow-variable': 'warn' }`. */ + /** + * Per-rule severity overrides, e.g. `{ 'shadow-variable': 'warn' }`. + * + * Keys should be {@link LintRuleName} values. An unrecognised key emits a + * warning in {@link LintResult.lint_warnings} and lint continues — the + * unknown rule is not enforced by the engine. `Record` is + * accepted for forward compatibility with future rule names. + */ rules?: Record; /** * Base directory for resolving `@import` directives in the source string. @@ -209,7 +271,14 @@ export interface LintOptions { export interface LintFileOptions { /** Runtime variables injected into the check gate (not the lint rules). */ vars?: Record; - /** Per-rule severity overrides, e.g. `{ 'shadow-variable': 'warn' }`. */ + /** + * Per-rule severity overrides, e.g. `{ 'shadow-variable': 'warn' }`. + * + * Keys should be {@link LintRuleName} values. An unrecognised key emits a + * warning in {@link LintResult.lint_warnings} and lint continues — the + * unknown rule is not enforced by the engine. `Record` is + * accepted for forward compatibility with future rule names. + */ rules?: Record; } diff --git a/spec.md b/spec.md index 523c69c7..44e87da1 100644 --- a/spec.md +++ b/spec.md @@ -995,6 +995,8 @@ mode, which uses a single config located from the directory argument. Keys are in alphabetical order (BTreeMap serialization). Within each `files[].diagnostics` array, diagnostics are ordered by ascending `span.offset`; span-less diagnostics sort last; equal-offset ties preserve rule-execution order (stable sort). (The CLI and binding surfaces always produce results through `LintResultBuilder`; a `LintResult` assembled directly via `LintResult::new` preserves caller-supplied order instead.) In directory mode, `files[].file` is the forward-slash-separated path relative to the lint root (e.g. `src/template.mds`), and the `files[]` array is ordered by the byte-wise string comparison of that relative display path (e.g. `api-utils.mds` sorts before `api/x.mds` because `'-'` (0x2D) < `'/'` (0x2F)). `"truncated": true` when the result set was capped by the per-file diagnostic cap of 1,000. `"span"` is JSON `null` for diagnostics that lack a source location. When linting from stdin (`mds lint -`), `files[].file` is `""`. +**`lint_warnings` field (binding surfaces only):** The napi, WASM, and Python binding surfaces include an optional top-level `"lint_warnings"` key in the returned result object when non-fatal warnings were produced during linting (for example, unknown rule names in `mds.json`). In the JSON wire form (napi, WASM, and Python `to_dict()` / `to_json()`) the key is absent (not `null`, not `[]`) when no warnings occurred; on the Python live-object surface, `LintResult.lint_warnings` is a property that always exists and returns an empty list when no warnings occurred. In alphabetical key order `"lint_warnings"` sorts between `"files"` and `"truncated"`. The CLI does **not** include `"lint_warnings"` in its `--format json` stdout envelope — it writes warnings to stderr so the JSON stdout remains valid and parseable without modification. + A file that produces a per-file analysis failure in directory mode (malformed config, I/O error) emits a `{"file":"…","error":{"code":"…","message":"…","help":"…","span":…}}` entry without a `"diagnostics"` key and contributes to exit code 2. When a stdin source fails the check gate before linting begins, the CLI emits an analysis-failure envelope to stdout: `{"version":1,"error":{"code":"…","message":"…","help":"…","span":…}}`. This envelope carries no `"files"` or `"truncated"` key, and no `"file"` key (unlike the success envelope above). A JSON consumer MUST handle both the success envelope and the analysis-failure envelope and MUST NOT assume a `"file"` key is present in error results. #### Sanitization invariant (v1) @@ -1023,12 +1025,13 @@ escaped, in either mode. | `message`, `help` | Every codepoint in the escaped class above is replaced with its six-character `\uXXXX` literal before serialization. | | `file` | Sanitized on the same pass as `message`/`help`. Hostile filenames cannot inject control, bidi, or separator characters into this JSON output. A filename occupying one of the **diagnostic** `file` fields — this JSON key, a CLI status line, or a `[file:line:col]` frame header — is escaped with the **full** class including `\n` on each of those, human surfaces included, because it is always rendered on a single line and POSIX permits a newline inside a filename. Two path positions are outside that rule and are **not** escaped: a path interpolated into a diagnostic *message body*, which is prose (see "Residual" below), and a path in a source map or in `CompileResult.dependencies`, which is a functional reference (see "Carve-out" below). | | `rule` | Fixed ASCII identifier; never contains control bytes by construction. Not sanitized. | +| `lint_warnings` | Binding-surface-only field (absent from the CLI's `--format json` stdout). Each element is a human-readable warning string whose interpolated user-supplied values (rule names from the caller's `rules` option) are WIRE-escaped via the full escaped class during construction, before the string is formed. The surrounding template text is static ASCII and contains no codepoints in the escaped class. | | `span`, `fix_edits` | **Raw byte offsets** into the unmodified source — deliberately not sanitized. These are numeric position values and must reflect the original source exactly. | This invariant applies across all surfaces that emit `"version": 1` JSON: CLI (`mds lint --format json`), napi (`lintVirtual` / `lint` / `lintFile`), WASM (`lintVirtual` / `lint`), and Python (`lint_virtual` / `lint` / `lint_file`). -All four surfaces emit byte-identical values, with one exception: the `"file"` key when the source is piped via stdin. `mds lint -` (CLI) relabels the internal virtual-FS key `"input.mds"` to `""` at the output boundary; the binding surfaces (`lintVirtual` / `lint` on napi, WASM, and Python) retain `"input.mds"`. All other fields — `message`, `help`, `rule`, `severity`, `span`, `fix_edits` — are byte-identical across all four surfaces. +All four surfaces emit byte-identical values on the fields they share, with two exceptions. First, `"lint_warnings"` is a binding-surface-only key: it is absent from the CLI's `--format json` output (the CLI writes unknown-rule warnings to stderr instead). Second, the `"file"` key differs when the source is piped via stdin: `mds lint -` (CLI) relabels the internal virtual-FS key `"input.mds"` to `""` at the output boundary; the binding surfaces (`lintVirtual` / `lint` on napi, WASM, and Python) retain `"input.mds"`. All other fields — `message`, `help`, `rule`, `severity`, `span`, `fix_edits` — are byte-identical across all four surfaces. ##### Mode is chosen per field, not per surface @@ -1067,9 +1070,9 @@ Applied, that means: |-------|------|---------| | `message`, `help`, warning bodies, `LabeledSpan` text | HUMAN on terminal surfaces, WIRE on the JSON wire | Prose; legitimately multi-line in a rendered frame | | A filename or path in a diagnostic `file` **field**: the JSON `file` key, a CLI status line, a `[file:line:col]` frame header | WIRE on every surface that renders one | Single-line by construction; POSIX permits `\n` in a filename and the user never types it | -| `mds.json` rule names and config values, `--format` arguments | WIRE on every surface that renders one | Single-line identifiers read from the working tree or the command line | +| `mds.json` rule names and config values, `--format` arguments | WIRE on every surface that renders one | Single-line identifiers. WIRE applies in all rendering contexts — including when a rule name appears inside a warning body (e.g., the unknown-rule-names warning), the name is WIRE-escaped, not HUMAN. This row takes precedence over the residual row below for `mds.json`-sourced values. | | `io::Error` / `MdsError` causes interpolated into a CLI status or warning line | WIRE on every surface that renders one | Single-line, and they embed paths of their own | -| A path, identifier or cause interpolated into a diagnostic **message body** | HUMAN on terminal surfaces, WIRE on the JSON wire | Follows the message row above — it is part of prose. This is the **residual** below: it is not covered by the WIRE rows | +| A path, identifier or cause interpolated into a diagnostic **message body** | HUMAN on terminal surfaces, WIRE on the JSON wire | Follows the message row above — it is part of prose. This is the **residual** below: it is not covered by the WIRE rows. Exception: `mds.json` rule names and config values that appear inside a warning body are governed by the WIRE row above, not this residual — the more specific row takes precedence. | | Compiled template output (`mds build -o -`) | not escaped | It is the command's product, not a diagnostic; redirects must stay byte-faithful | | Source-map `file` / `sources` / `sourcesContent`, and `CompileResult.dependencies` | not escaped | Functional references, not display text; escaping would break resolution. This is the **carve-out** below | @@ -1215,7 +1218,7 @@ Place `mds.json` in the project root (or any ancestor directory). The compiler w | Field | Type | Description | |-------|------|-------------| | `build.output_dir` | string | Relative path to output directory. Must not contain `..` components. | -| `lint.rules` | object | Per-rule severity overrides for `mds lint`. Keys are rule names; values are `"warn"`, `"error"`, or `"off"`. Unknown severity values cause a hard config-load error. Unknown rule names are warn-and-ignored (forward compat). | +| `lint.rules` | object | Per-rule severity overrides for `mds lint`. Keys are rule names; values are `"warn"`, `"error"`, or `"off"`. Unknown severity values cause a hard config-load error. An unknown rule name emits a warning naming it and listing the rules this build recognises, the config still loads, and lint continues — the unknown rule is not enforced (forward compat: a config naming a rule added in a newer release warns instead of failing on an older binary). Under `mds lint`, the warning goes to stderr and is suppressed by `--quiet`; `mds build`, `mds check`, `mds fmt`, and `mds watch` also read this file but do not emit the unknown-rule warning. On the `lint` API surfaces it is returned in `lint_warnings`. | Maximum config file size: 1 MB.