From 4192fa4bfda985c9ecf2ae2c17fc59882e4820ca Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 04:32:28 +0000 Subject: [PATCH 1/2] feat(diagnostics): canonical rendering and emission order (#255 PR 2/3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR 1 modelled a diagnostic's identity and deliberately left two things open: it treated `message` as an opaque string, and asserted ordering only as a PROPERTY (total, antisymmetric, reproducible) rather than claiming to know the sequence a surface emits. This slice supplies both. Rendering: `Diagnostic::render` / `render_pretty` / `title` / the ` [resource: ...]` suffix / the `note:` evidence lines, each compared byte-for-byte against text the reference actually produced. Ordering: the sequence `ownlang.__main__.check_module` emits — and it is NOT PR 1's `DiagIdentity` order. The reference ends with diags.sort(key=lambda d: (d.line, d.code)) and Python's `list.sort` is STABLE, so ties keep their emission order (policies -> lifetimes -> per function: CFG, then analysis). Two plausible ports reproduce the right set in the wrong sequence: sorting by the full identity (which consults subject/message/evidence — none of which the reference looks at), or using `sort_unstable_by`. `sort_emission_order` is therefore a stable sort on exactly `(line, code)`. Note what the key omits: the PATH. Within one module every diagnostic shares a file, so the core never orders by it; cross-file ordering belongs to the finding layers (`ownlang.di`, `ownlang.effects`) and inventing it here would be a behaviour the reference does not have. The load-bearing hazard is the caret column. `render_pretty` places it via `_caret_col`, a renderer heuristic — explicitly not the #317 source column PR 1 modelled. Python's `str.find` and `re.Match.start()` return CHARACTER offsets; Rust's `str::find` returns a BYTE offset. On any non-ASCII line the two disagree silently and the caret lands mid-glyph. Measured on the Cyrillic control: the reference says column 16, a forwarded byte index says 26. Every offset in this port is a `char` count, and the `unicode_caret_*` cases fail loudly if that regresses — verified by mutation (forwarding the byte index fails the replay with the case named). No regex crate was added: the two patterns are a first-quoted-group scan and a word-boundary search, both small enough to write directly, and a new production dependency would need its own review against the crate DAG. The fixture is a NEW file, not an extension of `diag_model.json`. PR 1 declared that schema final and promised later slices would add cases, never reshape a record; bolting expected strings onto it would have broken that promise on the very first follow-up. Completed checkpoint: #255 PR 2/3 — canonical rendering + total ordering Remaining #255 acceptance: full OWN/DI/EFF corpus ledger with stale/missing/orphan fixture failures and the final counters (PR 3) Python source of truth: ownlang/diagnostics.py (render, render_pretty, _caret_col, _SUBJECT_RE, _kind_suffix, Evidence.render), ownlang/__main__.py::check_module (the sort) Fixture subset: 19 render cases (10 with a caret rendering), 5 ordering cases Regeneration command: python tests/test_diag_render_fixtures.py --write Zero-Python replay command: cd rust && cargo test -p own-diagnostics Acceptance changed: no Behavior changed: no Unexplained differences: 0 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CJF7MBi1ijU5m9cJVWgQsM --- rust/crates/own-diagnostics/src/lib.rs | 13 + rust/crates/own-diagnostics/src/render.rs | 332 +++++++++ .../own-diagnostics/tests/render_replay.rs | 224 ++++++ tests/fixtures/diag_render.json | 645 ++++++++++++++++++ tests/test_diag_render_fixtures.py | 408 +++++++++++ 5 files changed, 1622 insertions(+) create mode 100644 rust/crates/own-diagnostics/src/render.rs create mode 100644 rust/crates/own-diagnostics/tests/render_replay.rs create mode 100644 tests/fixtures/diag_render.json create mode 100644 tests/test_diag_render_fixtures.py diff --git a/rust/crates/own-diagnostics/src/lib.rs b/rust/crates/own-diagnostics/src/lib.rs index 46c06dd1..04225f18 100644 --- a/rust/crates/own-diagnostics/src/lib.rs +++ b/rust/crates/own-diagnostics/src/lib.rs @@ -47,8 +47,21 @@ //! This crate still owns **no rendering**: canonical message text and the //! ordering contract are the next slice of #255, and report/SARIF is #256. +//! ## Step 5a, PR 2: canonical rendering and emission order +//! +//! [`Diagnostic::render`] / [`Diagnostic::render_pretty`] reproduce the +//! reference's text, and [`sort_emission_order`] reproduces the sequence +//! `check_module` emits — a **stable** sort on `(line, code)`, which is +//! deliberately *not* [`DiagIdentity`]'s total order (that one exists for set +//! operations and would reorder ties the reference leaves alone). Pinned by +//! `tests/fixtures/diag_render.json` (regenerate: `python +//! tests/test_diag_render_fixtures.py --write`), replayed with zero Python by +//! `tests/render_replay.rs`. + mod diagnostic; mod located; +mod render; pub use diagnostic::{title, DiagKey, Diagnostic, Evidence, Severity, UnknownCode, TITLES}; pub use located::{DiagIdentity, EvidenceIdentity, LocatedDiagnostic}; +pub use render::sort_emission_order; diff --git a/rust/crates/own-diagnostics/src/render.rs b/rust/crates/own-diagnostics/src/render.rs new file mode 100644 index 00000000..1102b4be --- /dev/null +++ b/rust/crates/own-diagnostics/src/render.rs @@ -0,0 +1,332 @@ +//! Canonical rendering and the emission ordering contract +//! (P-022 step 5a, issue #255 — PR 2 of 3). +//! +//! PR 1 modelled a diagnostic's *identity* and treated `message` as opaque. This +//! module is the text: the exact strings `ownlang.diagnostics.Diagnostic.render` +//! and `render_pretty` produce, plus the sequence +//! `ownlang.__main__.check_module` emits them in. +//! +//! ## The ordering contract is a STABLE two-key sort, not a total order +//! +//! The reference ends its pipeline with: +//! +//! ```text +//! diags.sort(key=lambda d: (d.line, d.code)) +//! ``` +//! +//! Python's `list.sort` is stable, so equal `(line, code)` pairs keep their +//! **emission** order. Two ways to get this wrong produce the right *set* in the +//! wrong *sequence*: +//! +//! * sorting by [`DiagIdentity`](crate::DiagIdentity) — that key is total by +//! design (PR 1 needed it for set operations) and would reorder ties by +//! subject, message or evidence, none of which the reference consults; +//! * using an unstable sort — `sort_unstable_by` may permute ties freely. +//! +//! [`sort_emission_order`] is therefore a stable sort on exactly `(line, code)`. +//! Note what the key omits: the **path**. Within one module every diagnostic +//! shares a file, so the core never orders by it; cross-file ordering belongs to +//! the finding layers (`ownlang.di`, `ownlang.effects`), and inventing it here +//! would be a behaviour the reference does not have. +//! +//! ## The caret column counts CHARACTERS +//! +//! `render_pretty` places a caret via the reference's `_caret_col`, a renderer +//! heuristic — explicitly *not* the #317 source column PR 1 modelled. It is +//! reproduced here in full, and the load-bearing detail is that Python's +//! `str.find` and `re.Match.start()` return **character** offsets while Rust's +//! `str::find` returns a **byte** offset. Forwarding a byte index would put the +//! caret mid-glyph on any non-ASCII line, silently and plausibly. Every offset +//! below is a `char` count. +//! +//! No regex crate is pulled in for this: the two patterns involved are a +//! first-single-quoted-group scan and a word-boundary search, both small enough +//! to write directly, and a new production dependency would need its own review +//! against the crate DAG. + +use crate::diagnostic::{Diagnostic, Evidence, Severity}; + +impl Severity { + /// The severity word the reference renders (`Severity.value`). + #[must_use] + pub const fn as_str(self) -> &'static str { + match self { + Self::Error => "error", + Self::Warning => "warning", + } + } +} + +impl Evidence { + /// The one-line `note:` rendering of this step. + /// + /// `anchor_file` is the diagnostic's own file, used when the step shares it + /// (`file` is `None`) — the port of `Evidence.render`. + #[must_use] + pub fn render(&self, anchor_file: &str) -> String { + let where_ = self.file.as_deref().unwrap_or(anchor_file); + format!(" note: {} at {}:{}", self.label, where_, self.line) + } +} + +/// True for a character Python's `\w` matches on a `str` pattern: ASCII +/// alphanumerics and `_`, widened to Unicode alphanumerics. +fn is_word_char(ch: char) -> bool { + ch.is_alphanumeric() || ch == '_' +} + +/// The first single-quoted, non-empty group in `message` — the port of +/// `_SUBJECT_RE = re.compile(r"'([^']+)'")`. +/// +/// Returns `None` for an unpaired quote or an empty `''`, matching the regex: +/// `[^']+` requires at least one character. +fn first_quoted(message: &str) -> Option<&str> { + let open = message.find('\'')?; + let rest = message.get(open.checked_add(1)?..)?; + let close = rest.find('\'')?; + let inner = rest.get(..close)?; + if inner.is_empty() { + None + } else { + Some(inner) + } +} + +/// The CHARACTER index at which `needle` occurs in `haystack` as a whole word — +/// the port of `re.search(rf"\b{escape(name)}\b", line)`. +/// +/// A `\b` holds where a word character abuts a non-word character or a string +/// edge; the check is applied to both ends of the candidate match. +fn find_word_boundary(haystack: &str, needle: &str) -> Option { + let need: Vec = needle.chars().collect(); + // `first`/`last` double as the non-empty guard `windows` needs: a zero-length + // window panics, and an empty needle has no boundary to speak of anyway. + let (&first, &last) = (need.first()?, need.last()?); + let hay: Vec = haystack.chars().collect(); + // A `\b` holds where a word character abuts a non-word one (or a string + // edge), checked at both ends of the candidate. + hay.windows(need.len()) + .enumerate() + .find(|(start, window)| { + if *window != need.as_slice() { + return false; + } + let left_ok = match start.checked_sub(1).and_then(|i| hay.get(i)) { + Some(&prev) => is_word_char(prev) != is_word_char(first), + None => true, + }; + let right_ok = match start.checked_add(need.len()).and_then(|i| hay.get(i)) { + Some(&next) => is_word_char(next) != is_word_char(last), + None => true, + }; + left_ok && right_ok + }) + .map(|(start, _)| start) +} + +/// The CHARACTER index of `needle` in `haystack`, or `None` — the port of +/// `str.find`, which returns a character offset in Python. +fn find_char_index(haystack: &str, needle: &str) -> Option { + haystack + .find(needle) + .and_then(|byte_idx| haystack.get(..byte_idx)) + .map(|prefix| prefix.chars().count()) +} + +impl Diagnostic { + /// The ` [resource: ]` tail, or empty when the finding carries no kind. + /// + /// A verbatim passthrough, not a lookup: a profile's new tag needs no change + /// here. + #[must_use] + pub fn kind_suffix(&self) -> String { + self.resource_kind + .as_ref() + .map_or_else(String::new, |kind| format!(" [resource: {kind}]")) + } + + /// The 1-based **character** column this diagnostic points at within + /// `src_line`, or `None` when it cannot be located. + /// + /// The port of `_caret_col`: prefer the first single-quoted name matched at a + /// word boundary, fall back to a plain substring occurrence, and finally to + /// the line's indent — `None` only for a blank line. A renderer heuristic, + /// never a source column (see the module docs). + #[must_use] + pub fn caret_col(&self, src_line: &str) -> Option { + if let Some(name) = first_quoted(&self.message) { + // Whole-word first, so 'a' lands on the argument in `Hash(a)` rather + // than the 'a' inside `Hash`. + if let Some(idx) = find_word_boundary(src_line, name) { + return idx.checked_add(1); + } + if let Some(idx) = find_char_index(src_line, name) { + return idx.checked_add(1); + } + } + if src_line.trim().is_empty() { + return None; + } + src_line + .chars() + .count() + .checked_sub(src_line.trim_start().chars().count()) + .and_then(|indent| indent.checked_add(1)) + } + + /// The `note:` lines for this diagnostic's reachability slice, in order. + #[must_use] + pub fn evidence_lines(&self, filename: &str) -> Vec { + self.evidence.iter().map(|e| e.render(filename)).collect() + } + + /// Plain rendering: `file:line: severity: [code] message`, then one `note:` + /// line per evidence step. + #[must_use] + pub fn render(&self, filename: &str) -> String { + let head = format!( + "{}:{}: {}: [{}] {}{}", + filename, + self.line, + self.severity.as_str(), + self.code, + self.message, + self.kind_suffix() + ); + let mut out = vec![head]; + out.extend(self.evidence_lines(filename)); + out.join("\n") + } + + /// A rustc-style rendering: a `file:line:col` header, the offending source + /// line, a caret under the named identifier, then the `note:` lines. + /// + /// Degrades to the plain header when the line or column cannot be resolved, + /// exactly as the reference does. + #[must_use] + pub fn render_pretty(&self, filename: &str, source: &str) -> String { + let lines: Vec<&str> = source.split('\n').collect(); + // Python's `str.splitlines()` drops a single trailing newline's empty + // tail; `split('\n')` keeps it, so trim that one element back off. + let lines = match lines.split_last() { + Some((last, head)) if last.is_empty() && !head.is_empty() => head, + _ => lines.as_slice(), + }; + let src_line = usize::try_from(self.line) + .ok() + .and_then(|n| n.checked_sub(1)) + .and_then(|idx| lines.get(idx)) + .copied() + .unwrap_or(""); + let col = self.caret_col(src_line); + let loc = col.map_or_else( + || format!("{}:{}", filename, self.line), + |c| format!("{}:{}:{}", filename, self.line, c), + ); + let mut out = vec![format!( + "{}: {}: [{}] {}{}", + loc, + self.severity.as_str(), + self.code, + self.message, + self.kind_suffix() + )]; + if !src_line.trim().is_empty() { + let gutter = format!(" {} | ", self.line); + out.push(format!("{gutter}{src_line}")); + if let Some(pad) = col.and_then(|c| { + gutter + .chars() + .count() + .checked_add(c) + .and_then(|n| n.checked_sub(1)) + }) { + out.push(format!("{}^", " ".repeat(pad))); + } + } + out.extend(self.evidence_lines(filename)); + out.join("\n") + } +} + +/// Order diagnostics the way the core emits them: a **stable** sort on +/// `(line, code)`. +/// +/// Stability is the contract, not an implementation detail — ties keep their +/// emission order (policies → lifetimes → per function: CFG, then analysis). Do +/// not "improve" this into a total order or an unstable sort; see the module +/// docs for why either reproduces the right set in the wrong sequence. +pub fn sort_emission_order(diagnostics: &mut [Diagnostic]) { + diagnostics.sort_by(|a, b| (a.line, &a.code).cmp(&(b.line, &b.code))); +} + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] +mod tests { + use super::*; + + fn diag(code: &str, message: &str, line: u32) -> Diagnostic { + Diagnostic::new(code, message, line).expect("known code") + } + + #[test] + fn first_quoted_needs_a_matched_non_empty_pair() { + assert_eq!(first_quoted("use 'b' after release"), Some("b")); + assert_eq!(first_quoted("move 'a' while 'b' is borrowed"), Some("a")); + assert_eq!(first_quoted("it's complicated"), None); + assert_eq!(first_quoted("empty '' group"), None); + assert_eq!(first_quoted("no quotes at all"), None); + } + + #[test] + fn word_boundary_beats_a_substring_hit() { + // 'a' occurs inside `Hash` first, but the boundary match is the argument. + assert_eq!(find_word_boundary(" Hash(a);", "a"), Some(9)); + assert_eq!(find_char_index(" Hash(a);", "a"), Some(5)); + } + + #[test] + fn caret_column_is_counted_in_characters() { + // Cyrillic is 2 bytes per char: a byte offset would overshoot badly. + let line = " освободить поток;"; + let d = diag("OWN003", "'поток' освобождён дважды", 1); + assert_eq!(d.caret_col(line), Some(16)); + assert!( + line.find("поток").expect("present") > 16, + "byte index differs" + ); + } + + #[test] + fn caret_falls_back_to_indent_then_to_nothing() { + let d = diag("OWN020", "unsupported construct", 1); + assert_eq!(d.caret_col(" deeply(indented);"), Some(9)); + assert_eq!(d.caret_col(" "), None); + assert_eq!(d.caret_col(""), None); + } + + #[test] + fn stable_sort_keeps_emission_order_on_ties() { + let mut diags = vec![ + diag("OWN001", "zulu", 5), + diag("OWN001", "alpha", 5), + diag("OWN001", "mike", 5), + ]; + sort_emission_order(&mut diags); + let messages: Vec<&str> = diags.iter().map(|d| d.message.as_str()).collect(); + assert_eq!( + messages, + ["zulu", "alpha", "mike"], + "ties must NOT be reordered — the reference sort is stable and does \ + not consult the message" + ); + } + + #[test] + fn codes_compare_as_strings() { + let mut diags = vec![diag("OWN010", "ten", 4), diag("OWN009", "nine", 4)]; + sort_emission_order(&mut diags); + let codes: Vec<&str> = diags.iter().map(|d| d.code.as_str()).collect(); + assert_eq!(codes, ["OWN009", "OWN010"]); + } +} diff --git a/rust/crates/own-diagnostics/tests/render_replay.rs b/rust/crates/own-diagnostics/tests/render_replay.rs new file mode 100644 index 00000000..eae7eaae --- /dev/null +++ b/rust/crates/own-diagnostics/tests/render_replay.rs @@ -0,0 +1,224 @@ +//! Zero-Python replay of the rendering + ordering fixture +//! (`tests/fixtures/diag_render.json`, authoritative via +//! `python tests/test_diag_render_fixtures.py --write`) — P-022 step 5a, #255, +//! PR 2 of 3. +//! +//! Two contracts, both compared against text Python actually produced rather +//! than against a restatement of the intent: +//! +//! * **rendering** — `Diagnostic.render` byte-for-byte, and `render_pretty` +//! (header, source line, caret) wherever the case carries a source; +//! * **ordering** — the sequence `check_module` emits. Each ordering case ships +//! its emission list with labels and the label sequence the reference's own +//! `sorted(key=(line, code))` produced, so a port that reorders ties or sorts +//! on the wrong key fails on the labels, not on a hand-written expectation. +//! +//! Out of scope here, and left to PR 3: the full OWN/DI/EFF corpus ledger with +//! stale/orphan fixture detection. + +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use own_diagnostics::{sort_emission_order, Diagnostic}; +use serde_json::Value; + +const FIXTURE: &str = concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../../tests/fixtures/diag_render.json" +); + +/// Independent of `diag_model.json`'s version: a different contract, free to +/// move separately. +const SCHEMA_VERSION: u64 = 1; + +fn load() -> Value { + let raw = std::fs::read_to_string(FIXTURE) + .expect("fixture missing — regenerate: python tests/test_diag_render_fixtures.py --write"); + let root: Value = serde_json::from_str(&raw).expect("diag_render.json parses"); + assert_eq!( + root.get("schema_version").and_then(Value::as_u64), + Some(SCHEMA_VERSION), + "fixture schema_version changed — a reviewed contract, not a passing reshape" + ); + root +} + +fn diagnostic_of(value: &Value, what: &str) -> Diagnostic { + serde_json::from_value(value.clone()) + .unwrap_or_else(|e| panic!("{what}: diagnostic does not load: {e}")) +} + +fn render_cases(root: &Value) -> &Vec { + root.get("render_cases") + .and_then(Value::as_array) + .expect("'render_cases' array") +} + +fn order_cases(root: &Value) -> &Vec { + root.get("order_cases") + .and_then(Value::as_array) + .expect("'order_cases' array") +} + +#[test] +fn fixture_is_non_empty_on_both_contracts() { + let root = load(); + assert!(!render_cases(&root).is_empty(), "no rendering cases"); + assert!(!order_cases(&root).is_empty(), "no ordering cases"); + let with_source = render_cases(&root) + .iter() + .filter(|c| !c.get("source").is_some_and(Value::is_null)) + .count(); + assert!( + with_source >= 5, + "expected several caret renderings; found {with_source}" + ); +} + +#[test] +fn plain_rendering_matches_the_reference_byte_for_byte() { + let root = load(); + for case in render_cases(&root) { + let name = case.get("name").and_then(Value::as_str).unwrap_or(""); + let path = case.get("path").and_then(Value::as_str).expect("'path'"); + let diagnostic = diagnostic_of(case.get("diagnostic").expect("'diagnostic'"), name); + let expected = case + .get("rendered") + .and_then(Value::as_str) + .expect("'rendered'"); + assert_eq!( + diagnostic.render(path), + expected, + "case {name:?}: render() text diverged from the reference" + ); + } +} + +#[test] +fn caret_rendering_matches_the_reference_byte_for_byte() { + let root = load(); + let mut checked = 0_usize; + for case in render_cases(&root) { + let name = case.get("name").and_then(Value::as_str).unwrap_or(""); + let Some(source) = case.get("source").and_then(Value::as_str) else { + continue; + }; + let path = case.get("path").and_then(Value::as_str).expect("'path'"); + let diagnostic = diagnostic_of(case.get("diagnostic").expect("'diagnostic'"), name); + let expected = case + .get("rendered_pretty") + .and_then(Value::as_str) + .expect("a case with a source must pin its pretty rendering"); + assert_eq!( + diagnostic.render_pretty(path, source), + expected, + "case {name:?}: render_pretty() text diverged — on a non-ASCII line this \ + is usually a byte offset forwarded where the reference counts characters" + ); + checked += 1; + } + assert!(checked > 0, "no pretty renderings were exercised"); +} + +#[test] +fn every_case_agrees_on_the_title() { + let root = load(); + for case in render_cases(&root) { + let name = case.get("name").and_then(Value::as_str).unwrap_or(""); + let diagnostic = diagnostic_of(case.get("diagnostic").expect("'diagnostic'"), name); + let expected = case.get("title").and_then(Value::as_str).expect("'title'"); + assert_eq!( + diagnostic.title(), + Some(expected), + "case {name:?}: TITLES entry diverged from the reference" + ); + } +} + +#[test] +fn emission_order_matches_the_reference_sequence() { + let root = load(); + for case in order_cases(&root) { + let name = case.get("name").and_then(Value::as_str).unwrap_or(""); + let emitted = case + .get("emitted") + .and_then(Value::as_array) + .expect("'emitted' array"); + + let mut labels: Vec = Vec::with_capacity(emitted.len()); + let mut diagnostics: Vec = Vec::with_capacity(emitted.len()); + for item in emitted { + labels.push( + item.get("label") + .and_then(Value::as_str) + .expect("'label'") + .to_owned(), + ); + diagnostics.push(diagnostic_of( + item.get("diagnostic").expect("'diagnostic'"), + name, + )); + } + + // Sort label-carrying pairs so a reordering is observable by name, which + // is the only way a TIE reordering can be detected at all: tied records + // are equal under the reference key by construction. + let mut pairs: Vec<(String, Diagnostic)> = labels.into_iter().zip(diagnostics).collect(); + let mut only_diags: Vec = pairs.iter().map(|(_, d)| d.clone()).collect(); + sort_emission_order(&mut only_diags); + pairs.sort_by(|a, b| (a.1.line, &a.1.code).cmp(&(b.1.line, &b.1.code))); + + let produced: Vec<&str> = pairs.iter().map(|(l, _)| l.as_str()).collect(); + let expected: Vec<&str> = case + .get("ordered_labels") + .and_then(Value::as_array) + .expect("'ordered_labels' array") + .iter() + .map(|v| v.as_str().expect("label is a string")) + .collect(); + + assert_eq!( + produced, expected, + "case {name:?}: emission order diverged. Fewer clues than it looks: if \ + only TIED entries moved, the port either used an unstable sort or sorted \ + on more than (line, code)" + ); + + // And the public helper must agree with that same key on the diagnostics + // themselves, so the helper is what callers can rely on. + let helper_keys: Vec<(u32, &str)> = only_diags + .iter() + .map(|d| (d.line, d.code.as_str())) + .collect(); + let pair_keys: Vec<(u32, &str)> = pairs + .iter() + .map(|(_, d)| (d.line, d.code.as_str())) + .collect(); + assert_eq!(helper_keys, pair_keys, "case {name:?}: helper key mismatch"); + } +} + +#[test] +fn sorting_is_idempotent_and_preserves_length() { + let root = load(); + for case in order_cases(&root) { + let name = case.get("name").and_then(Value::as_str).unwrap_or(""); + let emitted = case + .get("emitted") + .and_then(Value::as_array) + .expect("'emitted' array"); + let mut diags: Vec = emitted + .iter() + .map(|item| diagnostic_of(item.get("diagnostic").expect("'diagnostic'"), name)) + .collect(); + let before = diags.len(); + sort_emission_order(&mut diags); + let once = diags.clone(); + sort_emission_order(&mut diags); + assert_eq!( + diags.len(), + before, + "case {name:?}: sorting changed the count" + ); + assert_eq!(diags, once, "case {name:?}: sorting is not idempotent"); + } +} diff --git a/tests/fixtures/diag_render.json b/tests/fixtures/diag_render.json new file mode 100644 index 00000000..f3844812 --- /dev/null +++ b/tests/fixtures/diag_render.json @@ -0,0 +1,645 @@ +{ + "comment": "GENERATED by tests/test_diag_render_fixtures.py --write; do not edit. Python (ownlang) is authoritative. Canonical rendering (Diagnostic.render / render_pretty / title / resource-kind suffix / note lines) and the emission ordering contract from ownlang.__main__.check_module -- a STABLE sort on (line, code), so ties keep emission order (P-022 step 5a, issue #255, PR 2 of 3). rust/crates/own-diagnostics replays every case with zero Python.", + "schema_version": 1, + "render_cases": [ + { + "name": "plain_minimal", + "why": "the base line: file:line: severity: [code] message, nothing else", + "path": "src/A.cs", + "diagnostic": { + "code": "OWN003", + "message": "double release", + "line": 7, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + }, + "title": "double release", + "source": null, + "rendered": "src/A.cs:7: error: [OWN003] double release", + "rendered_pretty": null + }, + { + "name": "resource_kind_suffix", + "why": "the domain-neutral ` [resource: ...]` tail a profile keys off", + "path": "src/A.cs", + "diagnostic": { + "code": "OWN001", + "message": "not released", + "line": 12, + "severity": "error", + "subject": null, + "resource_kind": "subscription token", + "evidence": [] + }, + "title": "owned resource not released on all paths (possible leak)", + "source": null, + "rendered": "src/A.cs:12: error: [OWN001] not released [resource: subscription token]", + "rendered_pretty": null + }, + { + "name": "resource_kind_unknown_value", + "why": "an unrecognised kind is rendered verbatim -- the suffix is a passthrough, not a lookup, so a new profile's tag needs no renderer change", + "path": "src/A.cs", + "diagnostic": { + "code": "OWN001", + "message": "not released", + "line": 12, + "severity": "error", + "subject": null, + "resource_kind": "quantum flux capacitor", + "evidence": [] + }, + "title": "owned resource not released on all paths (possible leak)", + "source": null, + "rendered": "src/A.cs:12: error: [OWN001] not released [resource: quantum flux capacitor]", + "rendered_pretty": null + }, + { + "name": "warning_severity", + "why": "the P-004 warning tier renders its own severity word", + "path": "src/A.cs", + "diagnostic": { + "code": "OWN001", + "message": "may not be released", + "line": 12, + "severity": "warning", + "subject": null, + "resource_kind": null, + "evidence": [] + }, + "title": "owned resource not released on all paths (possible leak)", + "source": null, + "rendered": "src/A.cs:12: warning: [OWN001] may not be released", + "rendered_pretty": null + }, + { + "name": "evidence_same_file_uses_the_anchor", + "why": "`file=None` means 'the anchor's own file', and the note resolves it", + "path": "src/A.cs", + "diagnostic": { + "code": "OWN001", + "message": "not released", + "line": 12, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [ + { + "line": 4, + "label": "acquired here", + "file": null, + "role": "acquired" + } + ] + }, + "title": "owned resource not released on all paths (possible leak)", + "source": null, + "rendered": "src/A.cs:12: error: [OWN001] not released\n note: acquired here at src/A.cs:4", + "rendered_pretty": null + }, + { + "name": "evidence_cross_file_keeps_its_own", + "why": "an explicit step file wins over the anchor -- the DI captive shape", + "path": "src/Vm.cs", + "diagnostic": { + "code": "DI001", + "message": "captive dependency", + "line": 9, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [ + { + "line": 3, + "label": "registered here", + "file": "src/Startup.cs", + "role": "related" + } + ] + }, + "title": "captive dependency: a shorter-lived service is captured by a longer-lived one", + "source": null, + "rendered": "src/Vm.cs:9: error: [DI001] captive dependency\n note: registered here at src/Startup.cs:3", + "rendered_pretty": null + }, + { + "name": "evidence_order_is_rendered_in_order", + "why": "note lines follow the slice order; the slice IS the reachability path", + "path": "src/Flow.cs", + "diagnostic": { + "code": "OWN014", + "message": "value escapes", + "line": 30, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [ + { + "line": 10, + "label": "acquired here", + "file": null, + "role": "acquired" + }, + { + "line": 20, + "label": "escapes here", + "file": null, + "role": "escaped" + }, + { + "line": 25, + "label": "promoted here", + "file": null, + "role": "step" + } + ] + }, + "title": "value escapes to a longer-lived region (lifetime promotion)", + "source": null, + "rendered": "src/Flow.cs:30: error: [OWN014] value escapes\n note: acquired here at src/Flow.cs:10\n note: escapes here at src/Flow.cs:20\n note: promoted here at src/Flow.cs:25", + "rendered_pretty": null + }, + { + "name": "di004_root_provider_anchor", + "why": "a real DI004 with its subject and consuming-context note", + "path": "src/Svc.cs", + "diagnostic": { + "code": "DI004", + "message": "scoped 'Repo' resolved from the root provider", + "line": 41, + "severity": "error", + "subject": "Repo#41", + "resource_kind": null, + "evidence": [ + { + "line": 12, + "label": "singleton 'Cache' (captor)", + "file": null, + "role": "related" + } + ] + }, + "title": "scoped service resolved from the root provider (captured for the app lifetime)", + "source": null, + "rendered": "src/Svc.cs:41: error: [DI004] scoped 'Repo' resolved from the root provider\n note: singleton 'Cache' (captor) at src/Svc.cs:12", + "rendered_pretty": null + }, + { + "name": "di005_cache_site_anchor", + "why": "DI005 anchors at the field store, not the scope creation", + "path": "src/Svc.cs", + "diagnostic": { + "code": "DI005", + "message": "scope-resolved 'Repo' cached into a field", + "line": 55, + "severity": "error", + "subject": "_repo#55", + "resource_kind": null, + "evidence": [ + { + "line": 50, + "label": "scope created here", + "file": null, + "role": "acquired" + }, + { + "line": 55, + "label": "cached here", + "file": null, + "role": "escaped" + } + ] + }, + "title": "disposable transient resolved from a long-lived scope (delayed disposal)", + "source": null, + "rendered": "src/Svc.cs:55: error: [DI005] scope-resolved 'Repo' cached into a field\n note: scope created here at src/Svc.cs:50\n note: cached here at src/Svc.cs:55", + "rendered_pretty": null + }, + { + "name": "caret_on_quoted_identifier", + "why": "the caret lands on the first single-quoted name in the message", + "path": "src/A.cs", + "diagnostic": { + "code": "OWN002", + "message": "use 'b' after it was released", + "line": 3, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + }, + "title": "use after release", + "source": "fn f() {\n let a = 1;\n Hash(b);\n}\n", + "rendered": "src/A.cs:3: error: [OWN002] use 'b' after it was released", + "rendered_pretty": "src/A.cs:3:10: error: [OWN002] use 'b' after it was released\n 3 | Hash(b);\n ^" + }, + { + "name": "caret_prefers_a_word_boundary", + "why": "'a' must land on the ARGUMENT in `Hash(a)`, not on the 'a' inside 'Hash' -- the word-boundary attempt is tried before a plain substring search", + "path": "src/A.cs", + "diagnostic": { + "code": "OWN002", + "message": "use 'a' after it was released", + "line": 3, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + }, + "title": "use after release", + "source": "fn f() {\n let a = 1;\n Hash(a);\n}\n", + "rendered": "src/A.cs:3: error: [OWN002] use 'a' after it was released", + "rendered_pretty": "src/A.cs:3:10: error: [OWN002] use 'a' after it was released\n 3 | Hash(a);\n ^" + }, + { + "name": "caret_falls_back_to_substring", + "why": "no word-boundary match exists, so the plain substring position is used", + "path": "src/A.cs", + "diagnostic": { + "code": "OWN002", + "message": "use 'ash' after it was released", + "line": 3, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + }, + "title": "use after release", + "source": "fn f() {\n let a = 1;\n Hash(a);\n}\n", + "rendered": "src/A.cs:3: error: [OWN002] use 'ash' after it was released", + "rendered_pretty": "src/A.cs:3:6: error: [OWN002] use 'ash' after it was released\n 3 | Hash(a);\n ^" + }, + { + "name": "caret_falls_back_to_indent_without_a_quote", + "why": "an unquoted message has no name to point at, so the caret goes to the first non-blank column", + "path": "src/A.cs", + "diagnostic": { + "code": "OWN020", + "message": "unsupported construct", + "line": 3, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + }, + "title": "unsupported construct (out of scope for the MVP)", + "source": "fn f() {\n let a = 1;\n deeply(indented);\n}\n", + "rendered": "src/A.cs:3: error: [OWN020] unsupported construct", + "rendered_pretty": "src/A.cs:3:9: error: [OWN020] unsupported construct\n 3 | deeply(indented);\n ^" + }, + { + "name": "caret_is_absent_on_a_blank_line", + "why": "a blank source line yields no column at all -- the header degrades to file:line and the source/caret block is omitted entirely", + "path": "src/A.cs", + "diagnostic": { + "code": "OWN020", + "message": "unsupported construct", + "line": 2, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + }, + "title": "unsupported construct (out of scope for the MVP)", + "source": "fn f() {\n\n}\n", + "rendered": "src/A.cs:2: error: [OWN020] unsupported construct", + "rendered_pretty": "src/A.cs:2: error: [OWN020] unsupported construct" + }, + { + "name": "caret_when_the_line_is_out_of_range", + "why": "a line past the end of the source falls back to the plain header", + "path": "src/A.cs", + "diagnostic": { + "code": "OWN020", + "message": "unsupported construct", + "line": 99, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + }, + "title": "unsupported construct (out of scope for the MVP)", + "source": "fn f() {\n}\n", + "rendered": "src/A.cs:99: error: [OWN020] unsupported construct", + "rendered_pretty": "src/A.cs:99: error: [OWN020] unsupported construct" + }, + { + "name": "unicode_caret_counts_characters_not_bytes", + "why": "THE byte-vs-char trap: Python reports a CHARACTER offset, Rust's str::find reports a BYTE offset. On this Cyrillic line the two differ, so a port that forwards a byte index puts the caret mid-glyph", + "path": "src/Отчёт.cs", + "diagnostic": { + "code": "OWN003", + "message": "'поток' освобождён дважды", + "line": 1, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + }, + "title": "double release", + "source": " освободить поток;\n", + "rendered": "src/Отчёт.cs:1: error: [OWN003] 'поток' освобождён дважды", + "rendered_pretty": "src/Отчёт.cs:1:16: error: [OWN003] 'поток' освобождён дважды\n 1 | освободить поток;\n ^" + }, + { + "name": "unicode_caret_wide_glyphs", + "why": "the same trap with 3-byte characters, where a byte offset overshoots by more", + "path": "src/報告.cs", + "diagnostic": { + "code": "OWN001", + "message": "'リソース' が解放されていません", + "line": 1, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + }, + "title": "owned resource not released on all paths (possible leak)", + "source": " リソース を 解放 する;\n", + "rendered": "src/報告.cs:1: error: [OWN001] 'リソース' が解放されていません", + "rendered_pretty": "src/報告.cs:1:5: error: [OWN001] 'リソース' が解放されていません\n 1 | リソース を 解放 する;\n ^" + }, + { + "name": "message_with_several_quoted_names", + "why": "only the FIRST quoted group is the subject -- a later name must not steal the caret", + "path": "src/A.cs", + "diagnostic": { + "code": "OWN007", + "message": "move 'a' while 'b' is borrowed", + "line": 3, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + }, + "title": "move while borrowed", + "source": "fn f() {\n let b = 1;\n let a = move b;\n}\n", + "rendered": "src/A.cs:3: error: [OWN007] move 'a' while 'b' is borrowed", + "rendered_pretty": "src/A.cs:3:9: error: [OWN007] move 'a' while 'b' is borrowed\n 3 | let a = move b;\n ^" + }, + { + "name": "message_with_an_unmatched_quote", + "why": "a single unpaired quote matches no group, so the indent fallback applies", + "path": "src/A.cs", + "diagnostic": { + "code": "OWN020", + "message": "it's complicated", + "line": 3, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + }, + "title": "unsupported construct (out of scope for the MVP)", + "source": "fn f() {\n let a = 1;\n whatever();\n}\n", + "rendered": "src/A.cs:3: error: [OWN020] it's complicated", + "rendered_pretty": "src/A.cs:3:5: error: [OWN020] it's complicated\n 3 | whatever();\n ^" + } + ], + "order_cases": [ + { + "name": "sorts_by_line_then_code", + "why": "the plain case: the key is (line, code), in that precedence", + "emitted": [ + { + "label": "c", + "diagnostic": { + "code": "OWN003", + "message": "third", + "line": 9, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + } + }, + { + "label": "a", + "diagnostic": { + "code": "OWN001", + "message": "first", + "line": 2, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + } + }, + { + "label": "b", + "diagnostic": { + "code": "OWN002", + "message": "second", + "line": 9, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + } + } + ], + "ordered_labels": [ + "a", + "b", + "c" + ] + }, + { + "name": "ties_keep_emission_order", + "why": "THE stability control: equal (line, code) must preserve INPUT order. An unstable sort, or a sort over the full record, reproduces the same set in the wrong sequence", + "emitted": [ + { + "label": "emitted-1", + "diagnostic": { + "code": "OWN001", + "message": "zulu", + "line": 5, + "severity": "error", + "subject": "z#5", + "resource_kind": null, + "evidence": [] + } + }, + { + "label": "emitted-2", + "diagnostic": { + "code": "OWN001", + "message": "alpha", + "line": 5, + "severity": "error", + "subject": "a#5", + "resource_kind": null, + "evidence": [] + } + }, + { + "label": "emitted-3", + "diagnostic": { + "code": "OWN001", + "message": "mike", + "line": 5, + "severity": "error", + "subject": "m#5", + "resource_kind": null, + "evidence": [] + } + } + ], + "ordered_labels": [ + "emitted-1", + "emitted-2", + "emitted-3" + ] + }, + { + "name": "ties_ignore_severity_and_evidence", + "why": "severity and the evidence slice are NOT in the key -- they must not perturb a tie either", + "emitted": [ + { + "label": "warn-first", + "diagnostic": { + "code": "OWN001", + "message": "x", + "line": 5, + "severity": "warning", + "subject": null, + "resource_kind": null, + "evidence": [] + } + }, + { + "label": "error-second", + "diagnostic": { + "code": "OWN001", + "message": "x", + "line": 5, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [ + { + "line": 1, + "label": "a", + "file": null, + "role": "related" + } + ] + } + } + ], + "ordered_labels": [ + "warn-first", + "error-second" + ] + }, + { + "name": "path_is_not_part_of_the_core_key", + "why": "within one module every diagnostic shares a file, so the core key has no path component; equal-line records from different notional files order by code alone and otherwise keep emission order. Cross-file ordering belongs to the finding layers (ownlang.di / ownlang.effects), not here", + "emitted": [ + { + "label": "from-z-file", + "diagnostic": { + "code": "OWN002", + "message": "z", + "line": 7, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + } + }, + { + "label": "from-a-file", + "diagnostic": { + "code": "OWN002", + "message": "a", + "line": 7, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + } + }, + { + "label": "from-m-file", + "diagnostic": { + "code": "OWN001", + "message": "m", + "line": 7, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + } + } + ], + "ordered_labels": [ + "from-m-file", + "from-z-file", + "from-a-file" + ] + }, + { + "name": "code_order_is_lexicographic_not_numeric", + "why": "codes are compared as STRINGS: 'OWN010' sorts before 'OWN009' would if numbers were parsed, and before 'OWN2' regardless -- pinning this stops a port from 'helpfully' comparing the numeric tail", + "emitted": [ + { + "label": "own9", + "diagnostic": { + "code": "OWN009", + "message": "nine", + "line": 4, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + } + }, + { + "label": "own10", + "diagnostic": { + "code": "OWN010", + "message": "ten", + "line": 4, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + } + }, + { + "label": "di1", + "diagnostic": { + "code": "DI001", + "message": "di", + "line": 4, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + } + }, + { + "label": "eff1", + "diagnostic": { + "code": "EFF001", + "message": "eff", + "line": 4, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + } + } + ], + "ordered_labels": [ + "di1", + "eff1", + "own9", + "own10" + ] + } + ] +} diff --git a/tests/test_diag_render_fixtures.py b/tests/test_diag_render_fixtures.py new file mode 100644 index 00000000..36f1f2f8 --- /dev/null +++ b/tests/test_diag_render_fixtures.py @@ -0,0 +1,408 @@ +#!/usr/bin/env python3 +"""Canonical rendering + ordering parity fixtures (P-022 step 5a, issue #255) — Python side. + +PR 2 of three. PR 1 froze the *structural identity* of a diagnostic +(`tests/fixtures/diag_model.json`); it deliberately treated `message` as an +opaque string and asserted ordering only as a *property* (total, antisymmetric, +reproducible). This fixture supplies the two things it left open: + +1. the **canonical rendered text** — `Diagnostic.render`, `render_pretty`, + `title`, the ` [resource: ...]` suffix and the `note:` evidence lines; +2. the **emission ordering contract** — what sequence the core actually emits. + +A separate file on purpose: PR 1's schema was declared final, and later slices +were promised they would *add cases, never reshape a record*. Bolting expected +strings onto `diag_model.json` would have broken that promise on the first +follow-up, so the rendering contract gets its own schema and its own generator. + +## The ordering contract is narrower than PR 1's key — and that matters + +`ownlang.__main__.check_module` ends with: + + diags.sort(key=lambda d: (d.line, d.code)) + +Python's `list.sort` is **stable**, so this is *not* a total order over the +record: two diagnostics with equal `(line, code)` keep their **emission** order +(policies -> lifetimes -> per function: CFG diagnostics, then analysis +diagnostics). A port that sorts by the full identity, or that uses an unstable +sort, reproduces the same *set* in the wrong *sequence*. + +Note also what the key does **not** contain: the path. Within one module every +diagnostic shares a file, so the core never orders by it. Cross-file ordering is +a different layer's contract (`ownlang.di`/`ownlang.effects` sort their own +finding types by `(file, line, ...)`), and inventing it here would be a +behaviour the reference does not have. + +## Why the caret column is a parity hazard + +`render_pretty` places a caret with `_caret_col`, which is a *renderer +heuristic* — not the #317 source column PR 1 modelled. It searches the first +single-quoted identifier in the message, prefers a word-boundary match, falls +back to a plain substring, and finally to the line's indent. + +Python's `str.find` and `re.match.start()` return **character** offsets. Rust's +`str::find` returns a **byte** offset. On any non-ASCII source line the two +disagree, silently, and the caret lands mid-glyph. The `unicode_*` cases below +exist to make that divergence fail loudly rather than look plausible. + +Run: python tests/test_diag_render_fixtures.py (verify) + python tests/test_diag_render_fixtures.py --write (regenerate) + python tests/run_tests.py (runs it as part of the suite) +""" + +from __future__ import annotations + +import json +import os +import sys + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) + +from ownlang.diagnostics import Diagnostic, Evidence, Severity + +FIXTURE = os.path.join(os.path.dirname(__file__), "fixtures", "diag_render.json") + +# Independent of diag_model.json's version: this is a different contract with a +# different shape, and the two must be free to move apart. +SCHEMA_VERSION = 1 + +# Non-ASCII rendering inputs, one literal per line so a confusable-character lint +# stays reviewable at the string it fires on. +_RU_LINE = " освободить поток;" +_RU_NAME = "поток" +_RU_MESSAGE = "'поток' освобождён дважды" +_JA_LINE = " リソース を 解放 する;" +_JA_NAME = "リソース" + + +def _d(code: str, message: str, line: int, **kw: object) -> Diagnostic: + return Diagnostic(code=code, message=message, line=line, **kw) # type: ignore[arg-type] + + +# --------------------------------------------------------------------------- +# Rendering cases: one record + the exact text the reference produces for it. +# `source` is optional; when present, render_pretty is pinned too. +# --------------------------------------------------------------------------- +_RENDER: list[tuple[str, str, str, Diagnostic, str | None]] = [ + ( + "plain_minimal", + "the base line: file:line: severity: [code] message, nothing else", + "src/A.cs", + _d("OWN003", "double release", 7), + None, + ), + ( + "resource_kind_suffix", + "the domain-neutral ` [resource: ...]` tail a profile keys off", + "src/A.cs", + _d("OWN001", "not released", 12, resource_kind="subscription token"), + None, + ), + ( + "resource_kind_unknown_value", + "an unrecognised kind is rendered verbatim -- the suffix is a passthrough, " + "not a lookup, so a new profile's tag needs no renderer change", + "src/A.cs", + _d("OWN001", "not released", 12, resource_kind="quantum flux capacitor"), + None, + ), + ( + "warning_severity", + "the P-004 warning tier renders its own severity word", + "src/A.cs", + _d("OWN001", "may not be released", 12, severity=Severity.WARNING), + None, + ), + ( + "evidence_same_file_uses_the_anchor", + "`file=None` means 'the anchor's own file', and the note resolves it", + "src/A.cs", + _d("OWN001", "not released", 12, evidence=( + Evidence(line=4, label="acquired here", role="acquired"), + )), + None, + ), + ( + "evidence_cross_file_keeps_its_own", + "an explicit step file wins over the anchor -- the DI captive shape", + "src/Vm.cs", + _d("DI001", "captive dependency", 9, evidence=( + Evidence(line=3, label="registered here", file="src/Startup.cs"), + )), + None, + ), + ( + "evidence_order_is_rendered_in_order", + "note lines follow the slice order; the slice IS the reachability path", + "src/Flow.cs", + _d("OWN014", "value escapes", 30, evidence=( + Evidence(line=10, label="acquired here", role="acquired"), + Evidence(line=20, label="escapes here", role="escaped"), + Evidence(line=25, label="promoted here", role="step"), + )), + None, + ), + ( + "di004_root_provider_anchor", + "a real DI004 with its subject and consuming-context note", + "src/Svc.cs", + _d("DI004", "scoped 'Repo' resolved from the root provider", 41, + subject="Repo#41", evidence=( + Evidence(line=12, label="singleton 'Cache' (captor)", role="related"), + )), + None, + ), + ( + "di005_cache_site_anchor", + "DI005 anchors at the field store, not the scope creation", + "src/Svc.cs", + _d("DI005", "scope-resolved 'Repo' cached into a field", 55, + subject="_repo#55", evidence=( + Evidence(line=50, label="scope created here", role="acquired"), + Evidence(line=55, label="cached here", role="escaped"), + )), + None, + ), + # ---- render_pretty / caret placement ---- + ( + "caret_on_quoted_identifier", + "the caret lands on the first single-quoted name in the message", + "src/A.cs", + _d("OWN002", "use 'b' after it was released", 3), + "fn f() {\n let a = 1;\n Hash(b);\n}\n", + ), + ( + "caret_prefers_a_word_boundary", + "'a' must land on the ARGUMENT in `Hash(a)`, not on the 'a' inside 'Hash' " + "-- the word-boundary attempt is tried before a plain substring search", + "src/A.cs", + _d("OWN002", "use 'a' after it was released", 3), + "fn f() {\n let a = 1;\n Hash(a);\n}\n", + ), + ( + "caret_falls_back_to_substring", + "no word-boundary match exists, so the plain substring position is used", + "src/A.cs", + _d("OWN002", "use 'ash' after it was released", 3), + "fn f() {\n let a = 1;\n Hash(a);\n}\n", + ), + ( + "caret_falls_back_to_indent_without_a_quote", + "an unquoted message has no name to point at, so the caret goes to the " + "first non-blank column", + "src/A.cs", + _d("OWN020", "unsupported construct", 3), + "fn f() {\n let a = 1;\n deeply(indented);\n}\n", + ), + ( + "caret_is_absent_on_a_blank_line", + "a blank source line yields no column at all -- the header degrades to " + "file:line and the source/caret block is omitted entirely", + "src/A.cs", + _d("OWN020", "unsupported construct", 2), + "fn f() {\n\n}\n", + ), + ( + "caret_when_the_line_is_out_of_range", + "a line past the end of the source falls back to the plain header", + "src/A.cs", + _d("OWN020", "unsupported construct", 99), + "fn f() {\n}\n", + ), + ( + "unicode_caret_counts_characters_not_bytes", + "THE byte-vs-char trap: Python reports a CHARACTER offset, Rust's str::find " + "reports a BYTE offset. On this Cyrillic line the two differ, so a port that " + "forwards a byte index puts the caret mid-glyph", + "src/Отчёт.cs", + _d("OWN003", _RU_MESSAGE, 1), + _RU_LINE + "\n", + ), + ( + "unicode_caret_wide_glyphs", + "the same trap with 3-byte characters, where a byte offset overshoots by more", + "src/報告.cs", + _d("OWN001", f"'{_JA_NAME}' が解放されていません", 1), + _JA_LINE + "\n", + ), + ( + "message_with_several_quoted_names", + "only the FIRST quoted group is the subject -- a later name must not steal " + "the caret", + "src/A.cs", + _d("OWN007", "move 'a' while 'b' is borrowed", 3), + "fn f() {\n let b = 1;\n let a = move b;\n}\n", + ), + ( + "message_with_an_unmatched_quote", + "a single unpaired quote matches no group, so the indent fallback applies", + "src/A.cs", + _d("OWN020", "it's complicated", 3), + "fn f() {\n let a = 1;\n whatever();\n}\n", + ), +] + + +# --------------------------------------------------------------------------- +# Ordering cases: an EMISSION sequence and the sequence `check_module` produces +# from it. Records are labelled so a reordering is visible in the golden. +# --------------------------------------------------------------------------- +_ORDER: list[tuple[str, str, list[tuple[str, Diagnostic]]]] = [ + ( + "sorts_by_line_then_code", + "the plain case: the key is (line, code), in that precedence", + [ + ("c", _d("OWN003", "third", 9)), + ("a", _d("OWN001", "first", 2)), + ("b", _d("OWN002", "second", 9)), + ], + ), + ( + "ties_keep_emission_order", + "THE stability control: equal (line, code) must preserve INPUT order. An " + "unstable sort, or a sort over the full record, reproduces the same set in " + "the wrong sequence", + [ + ("emitted-1", _d("OWN001", "zulu", 5, subject="z#5")), + ("emitted-2", _d("OWN001", "alpha", 5, subject="a#5")), + ("emitted-3", _d("OWN001", "mike", 5, subject="m#5")), + ], + ), + ( + "ties_ignore_severity_and_evidence", + "severity and the evidence slice are NOT in the key -- they must not " + "perturb a tie either", + [ + ("warn-first", _d("OWN001", "x", 5, severity=Severity.WARNING)), + ("error-second", _d("OWN001", "x", 5, severity=Severity.ERROR, + evidence=(Evidence(line=1, label="a"),))), + ], + ), + ( + "path_is_not_part_of_the_core_key", + "within one module every diagnostic shares a file, so the core key has no " + "path component; equal-line records from different notional files order by " + "code alone and otherwise keep emission order. Cross-file ordering belongs " + "to the finding layers (ownlang.di / ownlang.effects), not here", + [ + ("from-z-file", _d("OWN002", "z", 7)), + ("from-a-file", _d("OWN002", "a", 7)), + ("from-m-file", _d("OWN001", "m", 7)), + ], + ), + ( + "code_order_is_lexicographic_not_numeric", + "codes are compared as STRINGS: 'OWN010' sorts before 'OWN009' would if " + "numbers were parsed, and before 'OWN2' regardless -- pinning this stops a " + "port from 'helpfully' comparing the numeric tail", + [ + ("own9", _d("OWN009", "nine", 4)), + ("own10", _d("OWN010", "ten", 4)), + ("di1", _d("DI001", "di", 4)), + ("eff1", _d("EFF001", "eff", 4)), + ], + ), +] + + +def _evidence_json(ev: Evidence) -> dict[str, object]: + return {"line": ev.line, "label": ev.label, "file": ev.file, "role": ev.role} + + +def _diagnostic_json(d: Diagnostic) -> dict[str, object]: + return { + "code": d.code, + "message": d.message, + "line": d.line, + "severity": d.severity.value, + "subject": d.subject, + "resource_kind": d.resource_kind, + "evidence": [_evidence_json(e) for e in d.evidence], + } + + +def build() -> dict[str, object]: + render_cases: list[dict[str, object]] = [] + for name, why, path, diag, source in _RENDER: + case: dict[str, object] = { + "name": name, + "why": why, + "path": path, + "diagnostic": _diagnostic_json(diag), + "title": diag.title, + "source": source, + "rendered": diag.render(path), + } + # render_pretty is pinned only where a source text exists to render against. + case["rendered_pretty"] = ( + diag.render_pretty(path, source) if source is not None else None + ) + render_cases.append(case) + + order_cases: list[dict[str, object]] = [] + for name, why, labelled in _ORDER: + # Reproduce `check_module`'s final step EXACTLY -- same key, same stable + # sort -- rather than restating the intended order by hand. + ordered = sorted(labelled, key=lambda pair: (pair[1].line, pair[1].code)) + order_cases.append({ + "name": name, + "why": why, + "emitted": [ + {"label": label, "diagnostic": _diagnostic_json(d)} + for label, d in labelled + ], + "ordered_labels": [label for label, _ in ordered], + }) + + return { + "comment": ( + "GENERATED by tests/test_diag_render_fixtures.py --write; do not edit. " + "Python (ownlang) is authoritative. Canonical rendering (Diagnostic.render / " + "render_pretty / title / resource-kind suffix / note lines) and the emission " + "ordering contract from ownlang.__main__.check_module -- a STABLE sort on " + "(line, code), so ties keep emission order (P-022 step 5a, issue #255, PR 2 of 3). " + "rust/crates/own-diagnostics replays every case with zero Python." + ), + "schema_version": SCHEMA_VERSION, + "render_cases": render_cases, + "order_cases": order_cases, + } + + +def _render_json(data: dict[str, object]) -> str: + return json.dumps(data, indent=2, ensure_ascii=False) + "\n" + + +def run() -> int: + expected = _render_json(build()) + if not os.path.exists(FIXTURE): + print(f"FAIL: {FIXTURE} missing; regenerate with " + f"'python tests/test_diag_render_fixtures.py --write'") + return 1 + with open(FIXTURE, encoding="utf-8") as f: + actual = f.read() + if actual != expected: + print(f"FAIL: {FIXTURE} is stale (the renderer or the cases changed); " + f"regenerate with 'python tests/test_diag_render_fixtures.py --write' " + f"and re-run the Rust side (cd rust && cargo test -p own-diagnostics)") + return 1 + data = json.loads(actual) + if data.get("schema_version") != SCHEMA_VERSION: + print(f"FAIL: {FIXTURE} schema_version " + f"{data.get('schema_version')!r} != {SCHEMA_VERSION}") + return 1 + n_pretty = sum(1 for c in data["render_cases"] if c["rendered_pretty"] is not None) + print(f"diagnostic render fixtures OK: {len(data['render_cases'])} render cases " + f"({n_pretty} with a caret rendering), " + f"{len(data['order_cases'])} ordering cases verified in sync") + return 0 + + +if __name__ == "__main__": + if "--write" in sys.argv[1:]: + os.makedirs(os.path.dirname(FIXTURE), exist_ok=True) + with open(FIXTURE, "w", encoding="utf-8") as f: + f.write(_render_json(build())) + print(f"wrote {FIXTURE}") + raise SystemExit(0) + raise SystemExit(run()) From 2e497cbdabadc98babefed9c6d5c90cd765c3dfc Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 04:43:47 +0000 Subject: [PATCH 2/2] fix(diagnostics): five Python-truthiness and boundary divergences (#255 PR 2/3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bot review on 4192fa4 raised six items. Five were real divergences from the reference, each confirmed by probing `ownlang` directly before touching anything; one was a test that could not fail. All are fixed with a fixture case apiece, so a regression is loud rather than plausible. Three of the five are the same root cause: Python's `or` and `if` are TRUTHINESS tests, not presence tests, and the model accepts the empty string everywhere it accepts a value. 1. `_kind_suffix` guards with `if self.resource_kind`, so an empty kind emits NOTHING. The port tested `Option` presence and rendered a bare ` [resource: ]`. Probed: `Diagnostic(..., resource_kind="")` renders `f.cs:1: error: [OWN001] m`. 2. `Evidence.render` resolves `self.file or anchor_file`, so an empty file falls back to the anchor exactly as `None` does. The port rendered `note: L at :3`. Probed: it renders `note: L at anchor.cs:3`. 3. `first_quoted` stopped at the first empty `''` pair. `[^']+` needs one character, so an empty pair is not a match — but the engine RETRIES from the next position and a later pair still wins. Probed on `"empty '' group 'x'"`: the reference captures `" group "`, the port returned `None` and silently fell through to substring/indent placement. 4. The word-boundary edge rule was wrong. The reference pattern is BOTH-ended (`\b…\b`), and at a string edge a boundary holds only when the adjacent NEEDLE character is a word character. Treating a missing neighbour as an unconditional match made `-foo`, `foo-` and `(a)` match where the reference finds nothing. Probed all three: `None` each. Worth noting the reviewer's own probe used single-ended patterns (`\b-foo`), which gives a different answer for the right edge; re-probing with the real both-ended shape is what settled the rule. 5. `render_pretty` split on `'\n'`, which is not `str.splitlines()`. CRLF left a trailing `\r` inside the rendered source gutter (shifting the caret), and a lone `\r`, `\x0b`, `\x0c`, `\x1c`–`\x1e`, `\x85`, `
` or `
` was not a boundary at all, putting every later line out of range. Replaced with a `splitlines`-equivalent split, CRLF counted as one boundary and no empty tail element. Sixth item — the ordering replay could not fail. It sorted a parallel vector with a COPY of the key and then compared only `(line, code)`, which tied records share by construction, so `ties_keep_emission_order` passed even against `sort_unstable_by`. The label sequence now comes from the public `sort_emission_order`, with each label recovered by matching the sorted record back to an emitted one (consuming the match, so a future duplicate cannot alias). Verified by mutation, with `--no-fail-fast` so every catcher is visible: sorting on `(line, code, message)` now fails BOTH the unit test and the replay (previously only the unit test), and reverting the empty-kind fix fails the rendering replay. Completed checkpoint: unchanged — #255 PR 2/3 Remaining #255 acceptance: unchanged — full corpus ledger (PR 3) Python source of truth: ownlang/diagnostics.py (_kind_suffix, _caret_col, _SUBJECT_RE, Evidence.render, render_pretty) Fixture subset: 26 render cases (15 with a caret rendering), 5 ordering cases Regeneration command: python tests/test_diag_render_fixtures.py --write Zero-Python replay command: cd rust && cargo test -p own-diagnostics Acceptance changed: no Behavior changed: no Unexplained differences: 0 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CJF7MBi1ijU5m9cJVWgQsM --- rust/crates/own-diagnostics/src/render.rs | 118 +++++++++++++--- .../own-diagnostics/tests/render_replay.rs | 45 +++--- tests/fixtures/diag_render.json | 133 ++++++++++++++++++ tests/test_diag_render_fixtures.py | 63 +++++++++ 4 files changed, 319 insertions(+), 40 deletions(-) diff --git a/rust/crates/own-diagnostics/src/render.rs b/rust/crates/own-diagnostics/src/render.rs index 1102b4be..1224d6e5 100644 --- a/rust/crates/own-diagnostics/src/render.rs +++ b/rust/crates/own-diagnostics/src/render.rs @@ -61,10 +61,20 @@ impl Evidence { /// The one-line `note:` rendering of this step. /// /// `anchor_file` is the diagnostic's own file, used when the step shares it - /// (`file` is `None`) — the port of `Evidence.render`. + /// — the port of `Evidence.render`. + /// + /// The reference writes `self.file or anchor_file`, and Python's `or` is a + /// **truthiness** test: an empty string falls through to the anchor exactly + /// as `None` does. The model accepts `Some("")` (nothing rejects it on + /// deserialisation), so treating it as merely "present" would render + /// `note: … at :3`. #[must_use] pub fn render(&self, anchor_file: &str) -> String { - let where_ = self.file.as_deref().unwrap_or(anchor_file); + let where_ = self + .file + .as_deref() + .filter(|f| !f.is_empty()) + .unwrap_or(anchor_file); format!(" note: {} at {}:{}", self.label, where_, self.line) } } @@ -78,17 +88,25 @@ fn is_word_char(ch: char) -> bool { /// The first single-quoted, non-empty group in `message` — the port of /// `_SUBJECT_RE = re.compile(r"'([^']+)'")`. /// -/// Returns `None` for an unpaired quote or an empty `''`, matching the regex: -/// `[^']+` requires at least one character. +/// `[^']+` requires at least one character, so an empty `''` pair is not a +/// match — but the engine does **not** give up there: it retries from the next +/// position, and a later non-empty pair still wins. On `"empty '' group 'x'"` +/// the reference returns `" group "` (the quote at index 7 opens it), not +/// `None`. Returning `None` at the first empty pair would silently drop the +/// quoted-name lookup and fall through to substring/indent placement. fn first_quoted(message: &str) -> Option<&str> { - let open = message.find('\'')?; - let rest = message.get(open.checked_add(1)?..)?; - let close = rest.find('\'')?; - let inner = rest.get(..close)?; - if inner.is_empty() { - None - } else { - Some(inner) + // A `'` is one byte, so every offset derived from one is a char boundary. + let mut from = 0_usize; + loop { + let rel = message.get(from..)?.find('\'')?; + let open = from.checked_add(rel)?; + let after_open = open.checked_add(1)?; + let rest = message.get(after_open..)?; + let close = rest.find('\'')?; + if close > 0 { + return rest.get(..close); + } + from = after_open; } } @@ -113,17 +131,74 @@ fn find_word_boundary(haystack: &str, needle: &str) -> Option { } let left_ok = match start.checked_sub(1).and_then(|i| hay.get(i)) { Some(&prev) => is_word_char(prev) != is_word_char(first), - None => true, + // At a string edge `\b` holds only when the adjacent NEEDLE + // character is a word character — a boundary is a transition + // involving `\w`, and a string edge supplies the `\W` side. The + // reference pattern is `\b…\b`, both-ended, so this applies to + // each edge independently: `-foo` in `-foo` does not match, and + // neither does `foo-` in `foo-`. + None => is_word_char(first), }; let right_ok = match start.checked_add(need.len()).and_then(|i| hay.get(i)) { Some(&next) => is_word_char(next) != is_word_char(last), - None => true, + None => is_word_char(last), }; left_ok && right_ok }) .map(|(start, _)| start) } +/// Every character Python's `str.splitlines` treats as a line boundary. +/// +/// Deliberately the full set, not just `\n`: `split('\n')` leaves a trailing +/// `\r` on every CRLF line (which would then be rendered *inside* the source +/// gutter and shift the caret), and treats a lone `\r` — or any of the Unicode +/// separators — as ordinary text, putting every later line out of range. +const LINE_BOUNDARIES: [char; 10] = [ + '\n', // Line Feed + '\r', // Carriage Return (and, with a following \n, CRLF as ONE break) + '\u{000b}', // Line Tabulation + '\u{000c}', // Form Feed + '\u{001c}', // File Separator + '\u{001d}', // Group Separator + '\u{001e}', // Record Separator + '\u{0085}', // Next Line + '\u{2028}', // Line Separator + '\u{2029}', // Paragraph Separator +]; + +/// Split `source` the way Python's `str.splitlines` does. +/// +/// Notably it does **not** leave a trailing empty element after a final +/// boundary, which is why the reference's `lines[self.line - 1]` indexes the way +/// it does. +fn split_lines_python(source: &str) -> Vec<&str> { + let mut out: Vec<&str> = Vec::new(); + let mut start = 0_usize; + let mut chars = source.char_indices().peekable(); + while let Some((idx, ch)) = chars.next() { + if !LINE_BOUNDARIES.contains(&ch) { + continue; + } + if let Some(line) = source.get(start..idx) { + out.push(line); + } + let mut next = idx.saturating_add(ch.len_utf8()); + if ch == '\r' && matches!(chars.peek(), Some(&(_, '\n'))) { + // CRLF is a single boundary, not two. + chars.next(); + next = next.saturating_add(1); + } + start = next; + } + if let Some(tail) = source.get(start..) { + if !tail.is_empty() { + out.push(tail); + } + } + out +} + /// The CHARACTER index of `needle` in `haystack`, or `None` — the port of /// `str.find`, which returns a character offset in Python. fn find_char_index(haystack: &str, needle: &str) -> Option { @@ -138,10 +213,15 @@ impl Diagnostic { /// /// A verbatim passthrough, not a lookup: a profile's new tag needs no change /// here. + /// + /// The reference guards with `if self.resource_kind`, a **truthiness** test, + /// so an empty kind emits no suffix at all. The model accepts `Some("")`, so + /// testing mere presence would render a bare ` [resource: ]`. #[must_use] pub fn kind_suffix(&self) -> String { self.resource_kind - .as_ref() + .as_deref() + .filter(|kind| !kind.is_empty()) .map_or_else(String::new, |kind| format!(" [resource: {kind}]")) } @@ -205,13 +285,7 @@ impl Diagnostic { /// exactly as the reference does. #[must_use] pub fn render_pretty(&self, filename: &str, source: &str) -> String { - let lines: Vec<&str> = source.split('\n').collect(); - // Python's `str.splitlines()` drops a single trailing newline's empty - // tail; `split('\n')` keeps it, so trim that one element back off. - let lines = match lines.split_last() { - Some((last, head)) if last.is_empty() && !head.is_empty() => head, - _ => lines.as_slice(), - }; + let lines = split_lines_python(source); let src_line = usize::try_from(self.line) .ok() .and_then(|n| n.checked_sub(1)) diff --git a/rust/crates/own-diagnostics/tests/render_replay.rs b/rust/crates/own-diagnostics/tests/render_replay.rs index eae7eaae..ceada2d3 100644 --- a/rust/crates/own-diagnostics/tests/render_replay.rs +++ b/rust/crates/own-diagnostics/tests/render_replay.rs @@ -159,15 +159,32 @@ fn emission_order_matches_the_reference_sequence() { )); } - // Sort label-carrying pairs so a reordering is observable by name, which - // is the only way a TIE reordering can be detected at all: tied records - // are equal under the reference key by construction. - let mut pairs: Vec<(String, Diagnostic)> = labels.into_iter().zip(diagnostics).collect(); + // The label sequence must come from the PUBLIC helper, not from a + // restatement of its key. A tie reordering is observable only by name, + // and only if the helper is the thing that moved the records: sorting a + // parallel vector with a copy of the key would pass even against + // `sort_unstable_by`, which is exactly the failure this case exists for. + let pairs: Vec<(String, Diagnostic)> = labels.into_iter().zip(diagnostics).collect(); let mut only_diags: Vec = pairs.iter().map(|(_, d)| d.clone()).collect(); sort_emission_order(&mut only_diags); - pairs.sort_by(|a, b| (a.1.line, &a.1.code).cmp(&(b.1.line, &b.1.code))); - let produced: Vec<&str> = pairs.iter().map(|(l, _)| l.as_str()).collect(); + // Recover each label by matching the sorted record back to an emitted + // one. `remove` consumes the match, so duplicates (should a case ever add + // one) cannot alias onto the same label twice. + let mut remaining = pairs; + let produced_owned: Vec = only_diags + .iter() + .map(|d| { + let at = remaining + .iter() + .position(|(_, candidate)| candidate == d) + .unwrap_or_else(|| { + panic!("case {name:?}: a sorted record is not one of the emitted records") + }); + remaining.remove(at).0 + }) + .collect(); + let produced: Vec<&str> = produced_owned.iter().map(String::as_str).collect(); let expected: Vec<&str> = case .get("ordered_labels") .and_then(Value::as_array) @@ -182,18 +199,10 @@ fn emission_order_matches_the_reference_sequence() { only TIED entries moved, the port either used an unstable sort or sorted \ on more than (line, code)" ); - - // And the public helper must agree with that same key on the diagnostics - // themselves, so the helper is what callers can rely on. - let helper_keys: Vec<(u32, &str)> = only_diags - .iter() - .map(|d| (d.line, d.code.as_str())) - .collect(); - let pair_keys: Vec<(u32, &str)> = pairs - .iter() - .map(|(_, d)| (d.line, d.code.as_str())) - .collect(); - assert_eq!(helper_keys, pair_keys, "case {name:?}: helper key mismatch"); + assert!( + remaining.is_empty(), + "case {name:?}: sorting dropped or duplicated records" + ); } } diff --git a/tests/fixtures/diag_render.json b/tests/fixtures/diag_render.json index f3844812..3715c924 100644 --- a/tests/fixtures/diag_render.json +++ b/tests/fixtures/diag_render.json @@ -56,6 +56,49 @@ "rendered": "src/A.cs:12: error: [OWN001] not released [resource: quantum flux capacitor]", "rendered_pretty": null }, + { + "name": "resource_kind_empty_emits_no_suffix", + "why": "the reference guards with `if self.resource_kind` -- a TRUTHINESS test, so an empty kind emits nothing. The model accepts Some(\"\"), and a port that tests mere presence renders a bare ` [resource: ]`", + "path": "src/A.cs", + "diagnostic": { + "code": "OWN001", + "message": "not released", + "line": 12, + "severity": "error", + "subject": null, + "resource_kind": "", + "evidence": [] + }, + "title": "owned resource not released on all paths (possible leak)", + "source": null, + "rendered": "src/A.cs:12: error: [OWN001] not released", + "rendered_pretty": null + }, + { + "name": "evidence_empty_file_falls_back_to_the_anchor", + "why": "`self.file or anchor_file` is truthiness too: an empty string resolves to the anchor exactly as None does, where a presence test renders `at :4`", + "path": "src/A.cs", + "diagnostic": { + "code": "OWN001", + "message": "not released", + "line": 12, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [ + { + "line": 4, + "label": "acquired here", + "file": "", + "role": "related" + } + ] + }, + "title": "owned resource not released on all paths (possible leak)", + "source": null, + "rendered": "src/A.cs:12: error: [OWN001] not released\n note: acquired here at src/A.cs:4", + "rendered_pretty": null + }, { "name": "warning_severity", "why": "the P-004 warning tier renders its own severity word", @@ -396,6 +439,96 @@ "source": "fn f() {\n let a = 1;\n whatever();\n}\n", "rendered": "src/A.cs:3: error: [OWN020] it's complicated", "rendered_pretty": "src/A.cs:3:5: error: [OWN020] it's complicated\n 3 | whatever();\n ^" + }, + { + "name": "empty_quote_pair_does_not_end_the_scan", + "why": "`[^']+` needs one character, so an empty '' pair is not a match -- but the engine RETRIES from the next position and a later pair still wins. Here the reference captures ' group ' (opened by the quote at index 7); stopping at the first empty pair would silently drop the quoted-name lookup", + "path": "src/A.cs", + "diagnostic": { + "code": "OWN020", + "message": "empty '' group 'x'", + "line": 3, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + }, + "title": "unsupported construct (out of scope for the MVP)", + "source": "fn f() {\n let a = 1;\n call( group );\n}\n", + "rendered": "src/A.cs:3: error: [OWN020] empty '' group 'x'", + "rendered_pretty": "src/A.cs:3:10: error: [OWN020] empty '' group 'x'\n 3 | call( group );\n ^" + }, + { + "name": "quoted_name_with_non_word_edges_has_no_boundary", + "why": "the reference pattern is BOTH-ended (`\\b...\\b`), and at a string edge a boundary needs the adjacent needle character to be a word character. '(a)' therefore has no whole-word match anywhere and falls to the substring position", + "path": "src/A.cs", + "diagnostic": { + "code": "OWN002", + "message": "use '(a)' after it was released", + "line": 3, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + }, + "title": "use after release", + "source": "fn f() {\n let a = 1;\n Hash((a));\n}\n", + "rendered": "src/A.cs:3: error: [OWN002] use '(a)' after it was released", + "rendered_pretty": "src/A.cs:3:10: error: [OWN002] use '(a)' after it was released\n 3 | Hash((a));\n ^" + }, + { + "name": "crlf_source_drops_the_carriage_return", + "why": "`str.splitlines()` treats CRLF as ONE boundary and keeps no `\\r`; a plain split on '\\n' would render the carriage return inside the source gutter and shift the caret", + "path": "src/A.cs", + "diagnostic": { + "code": "OWN002", + "message": "use 'b' after it was released", + "line": 3, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + }, + "title": "use after release", + "source": "fn f() {\r\n let a = 1;\r\n Hash(b);\r\n}\r\n", + "rendered": "src/A.cs:3: error: [OWN002] use 'b' after it was released", + "rendered_pretty": "src/A.cs:3:10: error: [OWN002] use 'b' after it was released\n 3 | Hash(b);\n ^" + }, + { + "name": "lone_cr_source_still_splits", + "why": "a lone carriage return is a boundary for splitlines but invisible to a '\\n' split, which would put every later line out of range", + "path": "src/A.cs", + "diagnostic": { + "code": "OWN002", + "message": "use 'b' after it was released", + "line": 3, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + }, + "title": "use after release", + "source": "fn f() {\r let a = 1;\r Hash(b);\r}\r", + "rendered": "src/A.cs:3: error: [OWN002] use 'b' after it was released", + "rendered_pretty": "src/A.cs:3:10: error: [OWN002] use 'b' after it was released\n 3 | Hash(b);\n ^" + }, + { + "name": "unicode_line_separator_is_a_boundary", + "why": "U+2028 is a splitlines boundary too -- the exotic end of the same contract", + "path": "src/A.cs", + "diagnostic": { + "code": "OWN002", + "message": "use 'b' after it was released", + "line": 3, + "severity": "error", + "subject": null, + "resource_kind": null, + "evidence": [] + }, + "title": "use after release", + "source": "fn f() {
 let a = 1;
 Hash(b);
}
", + "rendered": "src/A.cs:3: error: [OWN002] use 'b' after it was released", + "rendered_pretty": "src/A.cs:3:10: error: [OWN002] use 'b' after it was released\n 3 | Hash(b);\n ^" } ], "order_cases": [ diff --git a/tests/test_diag_render_fixtures.py b/tests/test_diag_render_fixtures.py index 36f1f2f8..59621e91 100644 --- a/tests/test_diag_render_fixtures.py +++ b/tests/test_diag_render_fixtures.py @@ -106,6 +106,25 @@ def _d(code: str, message: str, line: int, **kw: object) -> Diagnostic: _d("OWN001", "not released", 12, resource_kind="quantum flux capacitor"), None, ), + ( + "resource_kind_empty_emits_no_suffix", + "the reference guards with `if self.resource_kind` -- a TRUTHINESS test, so " + "an empty kind emits nothing. The model accepts Some(\"\"), and a port that " + "tests mere presence renders a bare ` [resource: ]`", + "src/A.cs", + _d("OWN001", "not released", 12, resource_kind=""), + None, + ), + ( + "evidence_empty_file_falls_back_to_the_anchor", + "`self.file or anchor_file` is truthiness too: an empty string resolves to " + "the anchor exactly as None does, where a presence test renders `at :4`", + "src/A.cs", + _d("OWN001", "not released", 12, evidence=( + Evidence(line=4, label="acquired here", file=""), + )), + None, + ), ( "warning_severity", "the P-004 warning tier renders its own severity word", @@ -240,6 +259,50 @@ def _d(code: str, message: str, line: int, **kw: object) -> Diagnostic: _d("OWN020", "it's complicated", 3), "fn f() {\n let a = 1;\n whatever();\n}\n", ), + ( + "empty_quote_pair_does_not_end_the_scan", + "`[^']+` needs one character, so an empty '' pair is not a match -- but the " + "engine RETRIES from the next position and a later pair still wins. Here the " + "reference captures ' group ' (opened by the quote at index 7); stopping at " + "the first empty pair would silently drop the quoted-name lookup", + "src/A.cs", + _d("OWN020", "empty '' group 'x'", 3), + "fn f() {\n let a = 1;\n call( group );\n}\n", + ), + ( + "quoted_name_with_non_word_edges_has_no_boundary", + "the reference pattern is BOTH-ended (`\\b...\\b`), and at a string edge a " + "boundary needs the adjacent needle character to be a word character. " + "'(a)' therefore has no whole-word match anywhere and falls to the substring " + "position", + "src/A.cs", + _d("OWN002", "use '(a)' after it was released", 3), + "fn f() {\n let a = 1;\n Hash((a));\n}\n", + ), + ( + "crlf_source_drops_the_carriage_return", + "`str.splitlines()` treats CRLF as ONE boundary and keeps no `\\r`; a plain " + "split on '\\n' would render the carriage return inside the source gutter and " + "shift the caret", + "src/A.cs", + _d("OWN002", "use 'b' after it was released", 3), + "fn f() {\r\n let a = 1;\r\n Hash(b);\r\n}\r\n", + ), + ( + "lone_cr_source_still_splits", + "a lone carriage return is a boundary for splitlines but invisible to a " + "'\\n' split, which would put every later line out of range", + "src/A.cs", + _d("OWN002", "use 'b' after it was released", 3), + "fn f() {\r let a = 1;\r Hash(b);\r}\r", + ), + ( + "unicode_line_separator_is_a_boundary", + "U+2028 is a splitlines boundary too -- the exotic end of the same contract", + "src/A.cs", + _d("OWN002", "use 'b' after it was released", 3), + "fn f() {\u2028 let a = 1;\u2028 Hash(b);\u2028}\u2028", + ), ]