diff --git a/claude-notes/plans/2026-07-02-strict-mode-warnings-as-errors.md b/claude-notes/plans/2026-07-02-strict-mode-warnings-as-errors.md new file mode 100644 index 000000000..40f80e5bf --- /dev/null +++ b/claude-notes/plans/2026-07-02-strict-mode-warnings-as-errors.md @@ -0,0 +1,245 @@ +# Strict mode: promote warning diagnostics to errors (GH #220) + +- **Braid strand:** bd-yjs54ptg +- **GitHub issue:** https://github.com/quarto-dev/q2/issues/220 +- **Status:** feasibility study + implementation plan (no code yet) + +## Overview + +GH #220 asks for a strict mode where warnings become errors, primarily for +CI. The design goal stated for this study: find the (hopefully few, central) +locations where warnings flow, and upgrade warning diagnostics to errors +there, so the command *naturally* fails with what are now errors — and so +**future warning diagnostics automatically participate in strict mode with +zero per-call-site work**. + +**Verdict: feasible, and cheap.** The architecture already has the seam we +need. The recommended design touches ~4 places, all policy-side (CLI + +summary type); the diagnostic engine stays policy-free, consistent with +Decision D1 (bd-creo) and the config-error-handling decision of 2025-12-07 +("the caller decides"). + +## How diagnostics flow today (verified 2026-07-02) + +1. **One diagnostic type.** `DiagnosticMessage` in `quarto-error-reporting` + (source: `external-sources/quarto-error-reporting/src/diagnostic.rs:215`) + with `kind: DiagnosticKind` (`Error | Warning | Info | Note`, + `diagnostic.rs:9`). A warning differs from an error **only** by this + field. Promotion is a one-field mutation. The crate has no built-in + severity-promotion affordance — by design, policy is consumer-side. + +2. **Many emitters, no single emission chokepoint.** Warnings are pushed + into `StageContext.diagnostics` (`crates/quarto-core/src/stage/context.rs:94`), + `RenderContext.diagnostics` (`crates/quarto-core/src/render.rs:233`), + pampa's `DiagnosticCollector` + (`crates/pampa/src/utils/diagnostic_collector.rs`), and ~15 transforms + that take a bare `&mut Vec`. Lua `quarto.warn()` is + harvested into real `DiagnosticMessage`s + (`crates/pampa/src/lua/diagnostics.rs:354`), so filter warnings ride the + same rails. Emission-point promotion would therefore **not** be "few, + central" — but it doesn't need to be, because: + +3. **Everything converges on `ProjectRenderSummary`** + (`crates/quarto-core/src/project/orchestrator.rs:481`), via exactly four + fields: `pass1_failures[].diagnostics`, `pass2_failures[].diagnostics`, + `project_diagnostics`, and per-output `outputs[].render_output.diagnostics` + (exposed by the `OutputDiagnostics` trait, `orchestrator.rs:546`). + Both the printing (`print_render_diagnostics`, text and `--json-errors` + paths, `crates/quarto/src/commands/render.rs:840`) and the error/warning + tally (`ProjectRenderSummary::diagnostic_counts`, `orchestrator.rs:583`) + operate **post-run on the summary** — nothing structured is streamed to + the user mid-render. This is the central seam. + +4. **The exit-code boundary is one function**: `should_exit_nonzero` + (`crates/quarto/src/commands/render.rs:830`), called from both + `execute_single_doc` (:728) and `execute_project` (:811). Today it + returns true iff pass1/pass2 failures exist or a *project-level* + diagnostic has `kind == Error`. + +5. **Precedent already in-tree:** `quarto-doctemplate`'s `EvalContext` has a + working `strict_mode` flag with `warn_or_error_at` / + `warn_or_error_with_code` (`crates/quarto-doctemplate/src/eval_context.rs:125-232`). + That's the emission-side pattern; we deliberately do **not** generalize + it (see "Alternatives rejected"). + +### Two latent facts that shaped the design + +- **Per-document diagnostics on *successful* outputs never affect the exit + code today — even `Error`-kind ones.** `should_exit_nonzero` only looks + at failures + `project_diagnostics`. So "promote kind and the command + naturally fails" is *almost* true: promotion must be paired with a small + exit-gate extension, otherwise a promoted warning on a successful render + would print as `error` and still exit 0. (This is arguably a latent + inconsistency even without strict mode — see Open Questions.) +- **~552 `eprintln!` and ~63 `tracing::warn!` call sites bypass the + structured system entirely.** Strict mode structurally cannot see these. + That is acceptable (they are logging, not user-facing diagnostics), but + it makes the convention "user-visible warnings must be + `DiagnosticMessage`s" load-bearing. A follow-up xtask lint could police + new `eprintln!("warning: ...")`-style output in quarto-core. + +## Recommended design: promote at the summary boundary + +Add a `--strict` flag to `quarto render`. When set, after `pipeline.run()` +returns and **before** printing: + +```rust +if args.strict { + summary.promote_warnings_to_errors(); +} +``` + +where `ProjectRenderSummary::promote_warnings_to_errors()` (new, in +`orchestrator.rs`) walks the four diagnostic sources and flips +`DiagnosticKind::Warning` → `DiagnosticKind::Error`. `Info`/`Note` are left +alone. Then extend the exit gate: + +```rust +fn should_exit_nonzero(summary: &ProjectRenderSummary, strict: bool) -> bool { + if !summary.pass1_failures.is_empty() || !summary.pass2_failures.is_empty() { + return true; + } + if strict && summary.diagnostic_counts().errors > 0 { + return true; // post-promotion, this is exactly "any former warning or real error anywhere" + } + summary.project_diagnostics.iter().any(|d| d.kind == DiagnosticKind::Error) +} +``` + +### Why this satisfies the sustainability requirement + +Every structured diagnostic — from pipeline stages, transforms, pampa, +Lua filters, project post-render — already drains into the summary; both +existing consumers of severity (printer, counter) read the summary +post-promotion. A **new warning added anywhere in the codebase tomorrow +automatically**: prints as `error` under `--strict` (text and +`--json-errors` alike), counts as an error in the summary line, and fails +the command. No per-call-site opt-in, no new emission API to remember. + +### What the user sees + +- Without `--strict`: unchanged. +- With `--strict`: diagnostics render with error severity (ariadne styling, + JSON `"kind": "error"`), the counts clause reports them as errors, and + the process exits 1. Consistent labeling everywhere because promotion + happens *before* any output is produced. + +### Touched locations (the "few, central" list) + +1. `crates/quarto/src/commands/render.rs` — `RenderArgs.strict` (clap flag, + modeled on `fail_fast`, :89), the two `promote` call sites (or one, if + the shared tail of `execute_single_doc`/`execute_project` is factored), + and the `should_exit_nonzero` extension. +2. `crates/quarto-core/src/project/orchestrator.rs` — + `promote_warnings_to_errors()`; add `diagnostics_mut()` to the + `OutputDiagnostics` trait (with the same `#[cfg(target_arch = "wasm32")]` + empty-impl shape as the existing `diagnostics()`), or implement the + method directly on `ProjectRenderSummary`. + +Nothing in `quarto-error-reporting` (external crate) changes. + +### Scope boundaries + +- **`q2 preview` / hub-client / WASM: intentionally out.** Decision D1 + makes preview/hub deliberately lenient (partial progress over strictness); + the WASM response even carries `warnings` as a separate field + (`crates/wasm-quarto-hub-client/src/lib.rs:616-621`). Strict mode is a + CLI-render policy. If browser strictness is ever wanted, the promotion + point would be where the WASM response assembles its + `warnings`/`diagnostics` fields. +- **`quarto-doctemplate`'s internal `strict_mode`: untouched.** Its + warnings surface upward as ordinary `DiagnosticMessage` warnings and get + promoted at the boundary like everything else. +- **`eprintln!`/`tracing` output: out of scope** (not structurally + reachable; see above). +- **Render control flow: unchanged.** Strict mode does not abort rendering + mid-flight or change what gets rendered — it renders everything, then + reports and fails. (`--fail-fast` keeps its current meaning: stop at + first *failure*; it does not learn to stop at first warning in v1.) + +## Alternatives rejected + +- **Exit-gate-only** (`strict && warnings > 0` → exit 1, no kind + promotion): smallest diff, but the command fails while the output still + says "warning" — confusing in CI logs, and the issue/user intent is + explicitly "what are now errors". +- **Emission-point promotion** (generalize doctemplate's + `warn_or_error_*` everywhere): no central emission chokepoint exists + (~15 bare-`Vec` pushes, several sink types, plus Lua harvesting); every + future warning author would need to remember the strict-aware API — + exactly the unsustainable shape we're avoiding. It would also let strict + mode alter mid-render control flow (`has_errors()` checks, fail-fast + aborts), changing *what renders* rather than just the outcome. +- **Promotion inside `quarto-error-reporting`**: violates the + engine-stays-policy-free design (D1; 2025-12-07 config plan) and would + require a release cycle of the external crate for no benefit. + +## Work items (TDD order) + +### Phase 1 — tests first + +- [x] Pick/build a fixture with a stable, successful-render warning: + an unresolved crossref (`@fig-nonexistent`) — confirmed empirically + to render successfully with one warning, exit 0. +- [x] CLI integration tests: + `crates/quarto/tests/integration/strict_mode.rs` (7 tests). + Verified failing first: 6 failed with clap's + `unexpected argument '--strict'`, baseline test passed. +- [x] Unit tests for `promote_warnings_to_errors` covering all four + summary sources; `Info`/`Note` untouched (orchestrator.rs tests). +- [x] Unit tests for the `should_exit_nonzero` strict arm (render.rs + tests: warning-only, promoted, clean, info-only). +- [x] `--json-errors --strict` test: emitted JSON `kind` is `"error"`. +- [x] Project-render variant (multi-file, warning in one file). + +### Phase 2 — implementation + +- [x] `RenderArgs.strict` + clap wiring (`--strict`, global to the render + subcommand, modeled on `--fail-fast`) + `--help` text. +- [x] `OutputDiagnostics::diagnostics_mut` (native + wasm impls) + + `ProjectRenderSummary::promote_warnings_to_errors`. +- [x] Promotion call + `should_exit_nonzero(summary, strict)` in both + execute paths. The strict arm defensively checks `warnings > 0` too, + so the gate stays correct even if a caller forgets to promote. +- [x] `cargo build --workspace` clean; `cargo nextest run --workspace`: + 9884 passed. Full `cargo xtask verify` (WASM leg) run as well. + +### Phase 3 — end-to-end + docs + +- [x] End-to-end verification (2026-07-02, real binary, output inspected): + + ``` + $ q2 render warn.qmd --strict # warn.qmd contains @fig-nonexistent + Error: unresolved crossref `@fig-nonexistent`: no target with this identifier was found. + 1 error + EXIT: 1 # warn.html still written (905 bytes) + + $ q2 render warn.qmd # without --strict: unchanged + Warning: unresolved crossref `@fig-nonexistent`: ... + 1 warning + EXIT: 0 + + $ q2 render warn.qmd --strict --json-errors + {"$schema":".../json-diagnostic.json","kind":"error","title":"unresolved crossref ..."} + ``` + +- [x] User-facing docs: "Rendering in CI" section in + `docs/guides/publishing/index.qmd`; page verified to render via + `q2 render docs/guides/publishing/index.qmd`. +- [x] Close the loop on GH #220: PR + https://github.com/quarto-dev/q2/pull/362 closes it on merge and + links this plan + strand bd-yjs54ptg. + +## Decisions (Carlos, 2026-07-02) + +1. **Latent inconsistency** (Error-kind diagnostic on a successful output + exits 0): **separate strand**, likely handled right after this one. + Filed as a discovered-from strand of bd-yjs54ptg. +2. **Config surface**: **CLI-flag-only** for v1; no `_quarto.yml` key. +3. **Flag name**: **`--strict`**, with help text spelling out the + warnings-become-errors semantics. +4. **`--strict --fail-fast` interplay**: **keep orthogonal**. Under + `--strict --fail-fast`, a promoted warning does not stop the render + (fail-fast still means "stop at first *failure*"). Accepted as + not-ideal; not worth reconsidering the parallelism / diagnostic-merge + design until there is strong evidence of shortcomings. diff --git a/crates/quarto-core/src/project/orchestrator.rs b/crates/quarto-core/src/project/orchestrator.rs index 204b5cf7f..b64e01436 100644 --- a/crates/quarto-core/src/project/orchestrator.rs +++ b/crates/quarto-core/src/project/orchestrator.rs @@ -545,6 +545,10 @@ impl DiagnosticCounts { /// (native [`RenderToFileResult`] vs the WASM `WasmPassTwoOutput`). pub trait OutputDiagnostics { fn diagnostics(&self) -> &[DiagnosticMessage]; + + /// Mutable access to the same diagnostics, so severity-promotion + /// policies (`--strict`, bd-yjs54ptg) can rewrite `kind` in place. + fn diagnostics_mut(&mut self) -> &mut [DiagnosticMessage]; } #[cfg(not(target_arch = "wasm32"))] @@ -552,6 +556,10 @@ impl OutputDiagnostics for RenderToFileResult { fn diagnostics(&self) -> &[DiagnosticMessage] { &self.render_output.diagnostics } + + fn diagnostics_mut(&mut self) -> &mut [DiagnosticMessage] { + &mut self.render_output.diagnostics + } } #[cfg(target_arch = "wasm32")] @@ -561,6 +569,10 @@ impl OutputDiagnostics for RenderToFileResult { // `cfg(target_arch = "wasm32")` definition above). &[] } + + fn diagnostics_mut(&mut self) -> &mut [DiagnosticMessage] { + &mut [] + } } impl ProjectRenderSummary { @@ -599,6 +611,41 @@ impl ProjectRenderSummary { counts } + + /// Promote every `Warning` diagnostic to `Error`, across all four + /// diagnostic sources (strict mode, bd-yjs54ptg / GH #220). + /// + /// This is the single seam through which every structured + /// diagnostic flows before anything is printed or counted, so a + /// warning emitted anywhere in the pipeline — stages, transforms, + /// Lua filters, project post-render — participates in strict mode + /// with no per-call-site opt-in. `Info` / `Note` are left alone. + /// + /// The orchestrator itself stays policy-free: this method is only + /// invoked by consumers that opt into strictness (the `--strict` + /// CLI flag); it does not run during the render and cannot change + /// what gets rendered. + pub fn promote_warnings_to_errors(&mut self) { + fn promote(diagnostics: &mut [DiagnosticMessage]) { + for diagnostic in diagnostics { + if diagnostic.kind == DiagnosticKind::Warning { + diagnostic.kind = DiagnosticKind::Error; + } + } + } + + for failure in self + .pass1_failures + .iter_mut() + .chain(&mut self.pass2_failures) + { + promote(&mut failure.diagnostics); + } + promote(&mut self.project_diagnostics); + for output in &mut self.outputs { + promote(output.diagnostics_mut()); + } + } } /// Two-pass project render driver (native only for Phase 1). @@ -1820,6 +1867,10 @@ mod tests { fn diagnostics(&self) -> &[DiagnosticMessage] { &self.diagnostics } + + fn diagnostics_mut(&mut self) -> &mut [DiagnosticMessage] { + &mut self.diagnostics + } } fn failure(diagnostics: Vec) -> FileFailure { @@ -1948,6 +1999,74 @@ mod tests { assert!(!counts.is_empty()); } + // === bd-yjs54ptg (GH #220): strict-mode warning promotion === + + #[test] + fn promote_warnings_covers_all_four_sources() { + let mut summary: ProjectRenderSummary = ProjectRenderSummary { + pass1_failures: vec![failure(vec![DiagnosticMessage::warning("p1w")])], + pass2_failures: vec![failure(vec![ + DiagnosticMessage::error("p2e"), + DiagnosticMessage::warning("p2w"), + ])], + project_diagnostics: vec![DiagnosticMessage::warning("pw")], + outputs: vec![output_with(vec![DiagnosticMessage::warning("ow")])], + stopped_early: false, + }; + summary.promote_warnings_to_errors(); + + let counts = summary.diagnostic_counts(); + assert_eq!(counts.warnings, 0, "no warnings may survive promotion"); + assert_eq!(counts.errors, 5, "every former warning counts as an error"); + + // Kind is rewritten on the diagnostics themselves (printing + // reads `kind` directly, not the counts). + assert!( + summary + .pass1_failures + .iter() + .chain(&summary.pass2_failures) + .flat_map(|f| &f.diagnostics) + .chain(&summary.project_diagnostics) + .chain(summary.outputs.iter().flat_map(|o| o.diagnostics())) + .all(|d| d.kind == DiagnosticKind::Error) + ); + } + + #[test] + fn promote_warnings_leaves_info_and_note_alone() { + let mut summary: ProjectRenderSummary = ProjectRenderSummary { + outputs: vec![output_with(vec![ + DiagnosticMessage::info("i"), + DiagnosticMessage::new(DiagnosticKind::Note, "n"), + DiagnosticMessage::warning("w"), + ])], + ..Default::default() + }; + summary.promote_warnings_to_errors(); + + let kinds: Vec = summary.outputs[0] + .diagnostics() + .iter() + .map(|d| d.kind) + .collect(); + assert_eq!( + kinds, + vec![ + DiagnosticKind::Info, + DiagnosticKind::Note, + DiagnosticKind::Error + ] + ); + } + + #[test] + fn promote_warnings_on_clean_summary_is_a_no_op() { + let mut summary: ProjectRenderSummary = ProjectRenderSummary::default(); + summary.promote_warnings_to_errors(); + assert!(summary.diagnostic_counts().is_empty()); + } + #[test] fn factory_dispatches_by_kind() { let make = |kind: ProjectKind| ProjectContext { diff --git a/crates/quarto/src/commands/render.rs b/crates/quarto/src/commands/render.rs index 922b50663..94b2f0927 100644 --- a/crates/quarto/src/commands/render.rs +++ b/crates/quarto/src/commands/render.rs @@ -87,6 +87,16 @@ pub struct RenderArgs { /// [`ProjectPipeline::with_fail_fast`]. Useful for an iterative /// fix loop. Exit code is unchanged (any failure → non-zero). pub fail_fast: bool, + /// Treat warnings as errors (bd-yjs54ptg, GH #220). Warning + /// diagnostics are promoted to error severity on the completed + /// [`ProjectRenderSummary`](quarto_core::project::orchestrator::ProjectRenderSummary) + /// before anything is printed, so the text path, `--json-errors`, + /// the counts clause, and the exit code all see errors. The render + /// itself is unaffected: everything still renders, then the + /// command fails. Orthogonal to `fail_fast`, which keeps meaning + /// "stop at first *failure*" — a promoted warning does not stop + /// the render. + pub strict: bool, } /// What to render after argument classification. @@ -700,7 +710,7 @@ fn execute_single_doc( .with_format_override(args.to.clone()) .with_fail_fast(args.fail_fast); - let summary = match pollster::block_on(pipeline.run()) { + let mut summary = match pollster::block_on(pipeline.run()) { Ok(s) => s, Err(QuartoError::Parse(parse_error)) => { if args.json_errors { @@ -713,6 +723,10 @@ fn execute_single_doc( Err(e) => return Err(anyhow::anyhow!("{}", e)), }; + if args.strict { + summary.promote_warnings_to_errors(); + } + print_render_diagnostics(&summary, args); // bd-ooleh: a single-file render has no "Rendered N of M" line to @@ -725,7 +739,7 @@ fn execute_single_doc( quarto_util::user_status!(args.quiet, "{}", clause); } - if should_exit_nonzero(&summary) { + if should_exit_nonzero(&summary, args.strict) { std::process::exit(1); } Ok(()) @@ -778,7 +792,7 @@ fn execute_project( pipeline = pipeline.with_mode(RenderMode::Subset(set)); } - let summary = match pollster::block_on(pipeline.run()) { + let mut summary = match pollster::block_on(pipeline.run()) { Ok(s) => s, Err(QuartoError::Parse(parse_error)) => { if args.json_errors { @@ -791,6 +805,10 @@ fn execute_project( Err(e) => return Err(anyhow::anyhow!("{}", e)), }; + if args.strict { + summary.promote_warnings_to_errors(); + } + print_render_diagnostics(&summary, args); let rendered = summary.outputs.len(); @@ -808,7 +826,7 @@ fn execute_project( quarto_util::user_status!(args.quiet, "{}", line); } - if should_exit_nonzero(&summary) { + if should_exit_nonzero(&summary, args.strict) { std::process::exit(1); } Ok(()) @@ -827,10 +845,28 @@ fn execute_project( /// previous behavior of "render the named file" rather than a silent /// zero-byte success. Warning / Info severity diagnostics are /// printed but do not affect the exit code. -fn should_exit_nonzero(summary: &quarto_core::project::orchestrator::ProjectRenderSummary) -> bool { +/// +/// Under `--strict` (bd-yjs54ptg), warnings have already been +/// promoted to errors on the summary, and the gate additionally +/// fails on *any* error diagnostic anywhere in the summary — +/// including per-document diagnostics on successful outputs, which +/// the non-strict gate does not consider. (Whether the non-strict +/// gate *should* consider them is bd-zcjtaz78.) +fn should_exit_nonzero( + summary: &quarto_core::project::orchestrator::ProjectRenderSummary, + strict: bool, +) -> bool { if !summary.pass1_failures.is_empty() || !summary.pass2_failures.is_empty() { return true; } + if strict { + let counts = summary.diagnostic_counts(); + // Post-promotion `warnings` is always 0; checking it anyway + // keeps the gate correct even if a caller forgets to promote. + if counts.errors > 0 || counts.warnings > 0 { + return true; + } + } summary .project_diagnostics .iter() @@ -1936,4 +1972,56 @@ mod tests { let clause = format_counts_clause(&counts(2, 1), false).unwrap(); assert!(!clause.contains('\x1b'), "got: {clause:?}"); } + + // === bd-yjs54ptg (GH #220): --strict exit gate === + + use quarto_core::project::orchestrator::ProjectRenderSummary; + use quarto_error_reporting::DiagnosticMessage; + + /// Build a summary carrying only project-level diagnostics. + /// `RenderToFileResult` (the default `O`) has no `Default`, so + /// the fields are spelled out; per-output coverage lives in the + /// orchestrator's own `promote_warnings_*` tests and the + /// strict_mode integration tests. + fn summary_with(project_diagnostics: Vec) -> ProjectRenderSummary { + ProjectRenderSummary { + outputs: Vec::new(), + pass1_failures: Vec::new(), + pass2_failures: Vec::new(), + project_diagnostics, + stopped_early: false, + } + } + + /// A warning-carrying summary: exit 0 without strict, exit + /// non-zero with it. + #[test] + fn exit_gate_warning_only_respects_strict() { + let summary = summary_with(vec![DiagnosticMessage::warning("w")]); + assert!(!should_exit_nonzero(&summary, false)); + // The defensive warnings-arm bites even without promotion. + assert!(should_exit_nonzero(&summary, true)); + } + + #[test] + fn exit_gate_promoted_summary_fails_under_strict() { + let mut summary = summary_with(vec![DiagnosticMessage::warning("w")]); + summary.promote_warnings_to_errors(); + assert!(should_exit_nonzero(&summary, true)); + // Promotion produced a real error, so even the non-strict + // gate fails on it (project-level errors already gate). + assert!(should_exit_nonzero(&summary, false)); + } + + #[test] + fn exit_gate_clean_summary_passes_under_strict() { + let summary = summary_with(Vec::new()); + assert!(!should_exit_nonzero(&summary, true)); + } + + #[test] + fn exit_gate_info_only_passes_under_strict() { + let summary = summary_with(vec![DiagnosticMessage::info("i")]); + assert!(!should_exit_nonzero(&summary, true)); + } } diff --git a/crates/quarto/src/main.rs b/crates/quarto/src/main.rs index a08f9f912..e57197120 100644 --- a/crates/quarto/src/main.rs +++ b/crates/quarto/src/main.rs @@ -164,6 +164,12 @@ enum Commands { /// When executed with many threads, many errors might still be reported. #[arg(long = "fail-fast")] fail_fast: bool, + + /// Treat warnings as errors: warning diagnostics are reported + /// with error severity and any of them makes the command exit + /// non-zero. Useful in CI. Does not stop the render early. + #[arg(long)] + strict: bool, }, /// Start a live preview of a Quarto document or project. @@ -642,6 +648,7 @@ fn main() -> Result<()> { attribution, json_errors, fail_fast, + strict, .. } => commands::render::execute(commands::render::RenderArgs { inputs, @@ -655,6 +662,7 @@ fn main() -> Result<()> { attribution, json_errors, fail_fast, + strict, }), Commands::Preview { path, diff --git a/crates/quarto/tests/integration/main.rs b/crates/quarto/tests/integration/main.rs index 1371c8cbb..97f8897c9 100644 --- a/crates/quarto/tests/integration/main.rs +++ b/crates/quarto/tests/integration/main.rs @@ -10,6 +10,7 @@ pub mod render_cli_e2e; pub mod render_integration; pub mod revealjs_cli; pub mod smoke_all; +pub mod strict_mode; pub mod trace_cli; pub mod version_cli; diff --git a/crates/quarto/tests/integration/strict_mode.rs b/crates/quarto/tests/integration/strict_mode.rs new file mode 100644 index 000000000..40328f78c --- /dev/null +++ b/crates/quarto/tests/integration/strict_mode.rs @@ -0,0 +1,300 @@ +//! End-to-end CLI tests for `q2 render --strict` (bd-yjs54ptg, GH #220). +//! +//! Each test spawns the real `q2` binary as a subprocess, runs +//! `q2 render` against a fixture that produces a warning on an +//! otherwise-successful render (an unresolved crossref), and asserts +//! on severity labeling and exit codes. +//! +//! Contract under verification (per +//! `claude-notes/plans/2026-07-02-strict-mode-warnings-as-errors.md`): +//! - Without `--strict`, warnings print with warning severity and the +//! command exits 0 (unchanged behavior). +//! - With `--strict`, warning diagnostics are promoted to errors +//! *before* any output is produced: the text path labels them as +//! errors, the `--json-errors` path emits `"kind": "error"`, the +//! counts clause tallies them as errors, and the command exits +//! non-zero. +//! - A clean render under `--strict` still exits 0. +//! +//! TDD note: these tests are written *before* the implementation and +//! must fail in expected ways before Phase 2 starts (clap rejects the +//! unknown `--strict` flag). + +use std::path::{Path, PathBuf}; +use std::process::Command; + +use serde_json::Value; +use tempfile::TempDir; + +const Q2_BIN: &str = env!("CARGO_BIN_EXE_q2"); +const JSON_DIAGNOSTIC_SCHEMA_URL: &str = "https://quarto.org/schemas/v1/json-diagnostic.json"; + +/// Front matter + an unresolved crossref: renders successfully but +/// emits exactly one warning diagnostic ("unresolved crossref"). +const WARNING_DOC: &str = "---\ntitle: Warn\n---\n\nSee @fig-nonexistent for details.\n"; + +/// A document that renders with no diagnostics at all. +const CLEAN_DOC: &str = "---\ntitle: Clean\n---\n\nNothing to report.\n"; + +fn write_file(path: &Path, contents: &str) { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(path, contents).unwrap(); +} + +fn canonical(p: &Path) -> PathBuf { + p.canonicalize().unwrap_or_else(|_| p.to_path_buf()) +} + +/// Run `q2 render ` from `cwd`. Returns the exit status and +/// captured stdout / stderr. +fn run_q2_render(cwd: &Path, args: &[&str]) -> std::process::Output { + let mut cmd = Command::new(Q2_BIN); + cmd.current_dir(cwd); + cmd.arg("render"); + for a in args { + cmd.arg(a); + } + cmd.output().expect("spawn q2 binary") +} + +/// Parse stderr as NDJSON, keeping only lines that parse as JSON +/// objects (tracing lines may share the channel). +fn parse_ndjson_lines(stderr: &str) -> Vec { + stderr + .lines() + .filter_map(|line| { + let trimmed = line.trim_start(); + if !trimmed.starts_with('{') { + return None; + } + serde_json::from_str::(trimmed).ok() + }) + .collect() +} + +fn is_diagnostic_shape(value: &Value) -> bool { + value.get("$schema").and_then(|v| v.as_str()) == Some(JSON_DIAGNOSTIC_SCHEMA_URL) +} + +// ==================================================================== +// Tests +// ==================================================================== + +/// The flag exists and `--help` advertises it. Guards against silent +/// removal / rename. +#[test] +fn strict_flag_exists() { + let output = Command::new(Q2_BIN) + .args(["render", "--help"]) + .output() + .expect("spawn q2 --help"); + assert!(output.status.success(), "q2 render --help should succeed"); + let help = String::from_utf8_lossy(&output.stdout); + assert!( + help.contains("--strict"), + "q2 render --help should advertise --strict; got:\n{help}" + ); +} + +/// Baseline (unchanged behavior): a warning-producing document +/// without `--strict` renders successfully, exits 0, and labels the +/// diagnostic with warning severity. +#[test] +fn warning_without_strict_exits_zero() { + let temp = TempDir::new().unwrap(); + let dir = canonical(temp.path()); + write_file(&dir.join("warn.qmd"), WARNING_DOC); + + let out_path = dir.join("warn.html"); + let output = run_q2_render(&dir, &["-o", out_path.to_str().unwrap(), "warn.qmd"]); + + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + output.status.success(), + "expected exit 0 for a warning without --strict; stderr:\n{stderr}" + ); + assert!( + stderr.contains("unresolved crossref"), + "expected the crossref warning on stderr; got:\n{stderr}" + ); + assert!( + stderr.contains("Warning"), + "expected warning severity labeling without --strict; got:\n{stderr}" + ); + assert!( + out_path.exists(), + "the render itself should have succeeded and written output" + ); +} + +/// The core strict-mode contract: the same document with `--strict` +/// exits non-zero, and the diagnostic is labeled as an error +/// everywhere — no warning-severity output remains. +#[test] +fn strict_promotes_warning_and_fails() { + let temp = TempDir::new().unwrap(); + let dir = canonical(temp.path()); + write_file(&dir.join("warn.qmd"), WARNING_DOC); + + let out_path = dir.join("warn.html"); + let output = run_q2_render( + &dir, + &["--strict", "-o", out_path.to_str().unwrap(), "warn.qmd"], + ); + + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + !output.status.success(), + "expected non-zero exit under --strict with a warning; stderr:\n{stderr}" + ); + assert!( + stderr.contains("unresolved crossref"), + "the promoted diagnostic should still be reported; got:\n{stderr}" + ); + assert!( + stderr.contains("Error"), + "expected error severity labeling under --strict; got:\n{stderr}" + ); + assert!( + !stderr.contains("Warning"), + "no warning-severity labeling should remain under --strict; got:\n{stderr}" + ); + assert!( + stderr.contains("1 error"), + "the counts clause should tally the promoted warning as an error; got:\n{stderr}" + ); + assert!( + !stderr.contains("1 warning"), + "the counts clause should not report warnings under --strict; got:\n{stderr}" + ); +} + +/// Strict mode does not abort the render: even though the command +/// fails, the output file is still produced (render-everything-then- +/// fail semantics; strict changes the outcome, not the control flow). +#[test] +fn strict_still_writes_output() { + let temp = TempDir::new().unwrap(); + let dir = canonical(temp.path()); + write_file(&dir.join("warn.qmd"), WARNING_DOC); + + let out_path = dir.join("warn.html"); + let output = run_q2_render( + &dir, + &["--strict", "-o", out_path.to_str().unwrap(), "warn.qmd"], + ); + + assert!(!output.status.success(), "expected non-zero exit"); + assert!( + out_path.exists(), + "strict mode must not suppress the rendered output" + ); +} + +/// A clean document under `--strict` exits 0 — strict mode only +/// bites when there is something to promote. +#[test] +fn strict_clean_run_exits_zero() { + let temp = TempDir::new().unwrap(); + let dir = canonical(temp.path()); + write_file(&dir.join("clean.qmd"), CLEAN_DOC); + + let out_path = dir.join("clean.html"); + let output = run_q2_render( + &dir, + &["--strict", "-o", out_path.to_str().unwrap(), "clean.qmd"], + ); + + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + output.status.success(), + "expected exit 0 for a clean render under --strict; stderr:\n{stderr}" + ); +} + +/// `--strict --json-errors`: the promoted diagnostic crosses the wire +/// as `"kind": "error"` and the command exits non-zero. +#[test] +fn strict_json_errors_kind_is_error() { + let temp = TempDir::new().unwrap(); + let dir = canonical(temp.path()); + write_file(&dir.join("warn.qmd"), WARNING_DOC); + + let out_path = dir.join("warn.html"); + let output = run_q2_render( + &dir, + &[ + "--strict", + "--json-errors", + "-o", + out_path.to_str().unwrap(), + "warn.qmd", + ], + ); + + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + !output.status.success(), + "expected non-zero exit under --strict; stderr:\n{stderr}" + ); + + let lines = parse_ndjson_lines(&stderr); + let crossref_diags: Vec<&Value> = lines + .iter() + .filter(|v| { + is_diagnostic_shape(v) + && v.get("title") + .and_then(|t| t.as_str()) + .is_some_and(|t| t.contains("unresolved crossref")) + }) + .collect(); + assert!( + !crossref_diags.is_empty(), + "expected the crossref diagnostic as a JsonDiagnostic; stderr:\n{stderr}" + ); + for diag in &crossref_diags { + assert_eq!( + diag.get("kind").and_then(|k| k.as_str()), + Some("error"), + "promoted diagnostic must cross the wire as kind=error; got:\n{diag:#?}" + ); + } +} + +/// Project render: a warning in one page of an otherwise-clean +/// project exits 0 without `--strict` and non-zero with it. +#[test] +fn strict_project_render_fails_on_warning() { + let temp = TempDir::new().unwrap(); + let dir = canonical(temp.path()); + write_file( + &dir.join("_quarto.yml"), + "project:\n type: website\n output-dir: _site\n", + ); + write_file(&dir.join("index.qmd"), CLEAN_DOC); + write_file(&dir.join("warn.qmd"), WARNING_DOC); + + let lenient = run_q2_render(&dir, &[]); + let lenient_stderr = String::from_utf8_lossy(&lenient.stderr); + assert!( + lenient.status.success(), + "expected exit 0 for the project render without --strict; stderr:\n{lenient_stderr}" + ); + + let strict = run_q2_render(&dir, &["--strict"]); + let strict_stderr = String::from_utf8_lossy(&strict.stderr); + assert!( + !strict.status.success(), + "expected non-zero exit for the project render under --strict; stderr:\n{strict_stderr}" + ); + assert!( + strict_stderr.contains("unresolved crossref"), + "the promoted diagnostic should be reported; got:\n{strict_stderr}" + ); + assert!( + strict_stderr.contains("1 error"), + "the summary line should tally the promoted warning as an error; got:\n{strict_stderr}" + ); +} diff --git a/docs/guides/publishing/index.qmd b/docs/guides/publishing/index.qmd index bd01cfeb3..7d4c58f20 100644 --- a/docs/guides/publishing/index.qmd +++ b/docs/guides/publishing/index.qmd @@ -2,4 +2,25 @@ title: Publishing --- +## Rendering in CI + +When rendering as part of an automated pipeline, pass `--strict` to +treat warnings as errors: + +```bash +quarto render --strict +``` + +Under `--strict`, any diagnostic that would normally be reported as a +warning — an unresolved cross-reference, an undefined template +variable, a warning raised by a Lua filter — is reported as an error +instead, and the command exits with a non-zero status. The render +itself still runs to completion, so a single invocation reports every +problem in the project rather than stopping at the first one. + +A render that produces no warnings behaves identically with and +without the flag. + +## More + TBD.