brink-analyzer 0.0.17

//! Inline-markup vocabulary validation against the host capability manifest
//! (`docs/prose-dialect-spec.md` §4.2, issue #1733; required attributes,
//! issue #1780/#1997).
//!
//! §4.2's ruling has two halves. The first — **freeform by default** — landed
//! with the markup grammar itself (PR #1732): an unrecognized `<tag>` is
//! never a parse error, and nothing here changes that. This module is the
//! second half: *when the host manifest declares a markup vocabulary*, span
//! names and attributes are checked against it.
//!
//! # Freeform stays the default
//!
//! [`check`] returns immediately — before touching the HIR at all — unless
//! the registered manifest declares at least one span kind. That is not an
//! optimization: it is the ruling. A project with no manifest, *and* a
//! project whose manifest declares only `externals`/`types`, must behave
//! exactly as they did before this module existed. `markup` is opt-in
//! tightening, never a tax on hosts that never asked for it.
//!
//! # Where the vocabulary lives
//!
//! In the **host capability manifest**, alongside `externals` — §3.4's
//! authorship test ("co-locate declarations only if hand-authored or
//! generated by the same source"): markup vocabulary is host-authored and can
//! be generated from engine code, the way bindings generate externals.
//! Element conventions are project-authored and live in the
//! `brink.toml`-referenced conventions module instead; the two are not the
//! same surface.
//!
//! # Required attributes (issue #1997)
//!
//! Until #1997, `attrs` was an *allow*-list only: an attribute outside the
//! declared set reported `E165`, but a declared attribute simply absent from
//! a span was never diagnosed — there was no way to say "this attribute is
//! mandatory". [`brink_ir::ManifestSpanAttr::required`] adds that: a span of
//! a declared kind missing one of its kind's `required` attributes reports
//! `E173`, gated the same way `E164`/`E165` are (only for a span whose name
//! *is* declared, one report per missing attribute rather than one combined
//! message). Attribute *values* stay unchecked and untyped — this is the
//! required/optional distinction only, not a type system.
//!
//! # Severity
//!
//! `E164`/`E165`/`E173` default to `Warning`, which is exactly what makes
//! them configurable: `[lints] E164 = "deny"` promotes a declared vocabulary
//! to binding, `[lints] E164 = "allow"`/`@[allow(E164)]`/
//! `// brink-disable E164` turn a check off for a project, a declaration, or
//! a line. Hard-error codes are neither overridable nor suppressible
//! (`brink_ir::suppressions`), so `Warning` is the only base that satisfies
//! §4.2's "configurable severity".

use std::collections::{BTreeMap, BTreeSet};

use rowan::TextRange;

use brink_ir::hir::{Content, ContentContext, ContentPart, HirFile, HirVisitor, SpanPart};
use brink_ir::{Diagnostic, DiagnosticCode, FileId, HostManifest, Provenance};

/// Check every inline markup span in `files` against the manifest's declared
/// markup vocabulary.
///
/// Returns an empty vector when no vocabulary is declared (the freeform
/// default) — see the module doc.
#[must_use]
pub fn check(files: &[(FileId, &HirFile)], manifest: Option<&HostManifest>) -> Vec<Diagnostic> {
    let Some(manifest) = manifest else {
        return Vec::new();
    };
    if manifest.markup.is_empty() {
        return Vec::new();
    }

    // `BTreeMap`/`BTreeSet` (not `HashMap`): a duplicate declaration of the
    // same kind merges deterministically, and diagnostic *messages* quote
    // nothing iteration-ordered, so this is order-insensitive by
    // construction rather than by luck.
    let mut vocab: BTreeMap<&str, KindVocab<'_>> = BTreeMap::new();
    for kind in &manifest.markup {
        let entry = vocab.entry(kind.name.as_str()).or_default();
        for attr in &kind.attrs {
            entry.allowed.insert(attr.name.as_str());
            // A duplicate kind declaration's `required` is unioned in, not
            // overwritten: once any declaration of this kind marks an
            // attribute required, it stays required — the same
            // never-loosens-on-merge posture `allowed`'s `extend` already
            // has (a later declaration can only add, never take away).
            if attr.required {
                entry.required.insert(attr.name.as_str());
            }
        }
    }

    let mut out = Vec::new();
    for &(file, hir) in files {
        let mut walker = SpanWalker {
            file,
            vocab: &vocab,
            out: &mut out,
        };
        brink_ir::hir::visit::visit(hir, &mut walker);
    }
    out
}

/// One declared span kind's vocabulary: every attribute name it allows, and
/// the subset of those that are `required` (issue #1997).
///
/// `required` is always a subset of `allowed` by construction — the vocab
/// builder only ever inserts into `required` alongside the matching
/// `allowed` insert, never on its own — so [`SpanWalker::check_span`] never
/// has to guard against a "required but not allowed" attribute name.
#[derive(Default)]
struct KindVocab<'a> {
    allowed: BTreeSet<&'a str>,
    required: BTreeSet<&'a str>,
}

/// Collects markup diagnostics for one file.
///
/// Descends into spans by hand rather than relying on the shared walker:
/// [`HirVisitor`]'s content hook hands over the whole [`Content`], and
/// `walk_content_part` recurses *through* a span into its children without
/// exposing the [`SpanPart`] itself (a span is presentational, so structural
/// walkers see past it). This is the one pass that needs the span node.
struct SpanWalker<'a> {
    file: FileId,
    vocab: &'a BTreeMap<&'a str, KindVocab<'a>>,
    out: &'a mut Vec<Diagnostic>,
}

impl SpanWalker<'_> {
    /// Report against a caller-chosen range — the span's own (issue #1782:
    /// `SpanPart::ptr`) or, for `E165`, an individual attribute's own
    /// (issue #1829: `SpanAttr::ptr`).
    ///
    /// Every `SpanPart`/`SpanAttr` is lowered from a real syntax node
    /// (native is the only frontend that can spell markup), so a caller
    /// always has somewhere precise to point — no enclosing-line or
    /// enclosing-choice fallback needed. This is what makes several spans
    /// on one line, or several undeclared attributes on one span, even
    /// repeats of the same name, distinguishable: each gets its own range
    /// instead of sharing its container's.
    fn report(&mut self, range: TextRange, code: DiagnosticCode, message: String) {
        self.out.push(Diagnostic {
            file: self.file,
            range,
            message,
            code,
        });
    }

    fn check_span(&mut self, span: &SpanPart) {
        match self.vocab.get(span.name.as_str()) {
            None => self.report(
                Provenance::text_range(&span.ptr),
                DiagnosticCode::E164,
                format!(
                    "unknown markup tag `<{}>`: the host manifest's markup vocabulary does not declare it",
                    span.name
                ),
            ),
            Some(kind_vocab) => {
                for attr in &span.attrs {
                    if !kind_vocab.allowed.contains(attr.name.as_str()) {
                        // Issue #1829: ranged against the attribute's own
                        // provenance (`SpanAttr::ptr`), not the whole
                        // enclosing span — two undeclared attributes on one
                        // span used to collapse into two diagnostics with
                        // identical range *and* identical message.
                        self.report(
                            Provenance::text_range(&attr.ptr),
                            DiagnosticCode::E165,
                            format!(
                                "unknown attribute `{}` on markup tag `<{}>`: the host manifest does not declare it for this span kind",
                                attr.name, span.name
                            ),
                        );
                    }
                }
                // Gated the same way E164/E165 already are: this only ever
                // runs for a span whose *name* the manifest declares (an
                // undeclared tag reports E164 alone), and only for the
                // subset of a declared kind's attributes actually marked
                // `required` — a kind with none declared required never
                // fires this for any span of that kind. One diagnostic per
                // missing attribute, in sorted (`BTreeSet`) order, so a span
                // missing several required attributes gets one report per
                // name rather than a single combined message.
                let present: BTreeSet<&str> =
                    span.attrs.iter().map(|attr| attr.name.as_str()).collect();
                for &required_attr in &kind_vocab.required {
                    if !present.contains(required_attr) {
                        // Stays span-ranged, unlike E165 above: a *missing*
                        // attribute has no `SpanAttr` node of its own to
                        // point at — the span is the narrowest real range.
                        self.report(
                            Provenance::text_range(&span.ptr),
                            DiagnosticCode::E173,
                            format!(
                                "markup tag `<{}>` is missing required attribute `{required_attr}`: the host manifest declares it required for this span kind",
                                span.name
                            ),
                        );
                    }
                }
            }
        }
        for child in &span.children {
            self.check_part(child);
        }
    }

    fn check_part(&mut self, part: &ContentPart) {
        match part {
            ContentPart::Span(span) => self.check_span(span),
            // Logic nests freely inside markup and vice versa (§4.3), so a
            // span can hide inside an inline conditional's or sequence's
            // branch content. Those branches are `Content` nodes of their
            // own and the shared walker delivers them through
            // `enter_content` in turn — nothing to recurse into here.
            ContentPart::Text(_)
            | ContentPart::Glue
            | ContentPart::Spring
            | ContentPart::Interpolation(_)
            | ContentPart::InlineConditional(_)
            | ContentPart::InlineSequence(_) => {}
        }
    }
}

impl HirVisitor for SpanWalker<'_> {
    fn enter_content(&mut self, content: &Content, _ctx: ContentContext) {
        // `content.parts` only — deliberately *not* `content.tags`, even
        // though a `Tag` structurally owns a `Vec<ContentPart>` too. The
        // native frontend's `lower_tag` concatenates a tag's raw token text
        // into exactly one `ContentPart::Text`, so no `Span` can ever appear
        // under a tag; descending there would be unreachable code. (A `#`
        // tag written with markup in it keeps the angle brackets as literal
        // tag text — tags are metadata, not prose.) If tag lowering ever
        // gains real content parts, this is the line that has to change.
        for part in &content.parts {
            self.check_part(part);
        }
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    use brink_ir::{ManifestExternal, ManifestSpanAttr, ManifestSpanKind, SemanticTypeDef};

    /// A plain optional attribute (`required` defaults to `false`).
    fn attr(name: &str) -> ManifestSpanAttr {
        ManifestSpanAttr {
            name: name.to_string(),
            required: false,
            ty: None,
        }
    }

    /// A `required` attribute (issue #1997).
    fn required_attr(name: &str) -> ManifestSpanAttr {
        ManifestSpanAttr {
            name: name.to_string(),
            required: true,
            ty: None,
        }
    }

    fn lower(src: &str) -> HirFile {
        let parse = brink_syntax_native::parse(src);
        assert!(
            parse.errors().is_empty(),
            "parse errors: {:?}",
            parse.errors()
        );
        let (hir, _manifest, diags) = brink_ir::hir::lower_native::lower(FileId(0), &parse.tree());
        assert!(diags.is_empty(), "lowering diagnostics: {diags:?}");
        hir
    }

    fn run(src: &str, manifest: Option<&HostManifest>) -> Vec<Diagnostic> {
        let hir = lower(src);
        check(&[(FileId(0), &hir)], manifest)
    }

    fn codes(diags: &[Diagnostic]) -> Vec<&'static str> {
        diags.iter().map(|d| d.code.as_str()).collect()
    }

    /// A vocabulary declaring `<wave amount="…">` and a bare `<b>`. Neither
    /// attribute is `required` — the pre-#1997 allow-list-only shape.
    fn wave_manifest() -> HostManifest {
        HostManifest {
            markup: vec![
                ManifestSpanKind {
                    name: "wave".to_string(),
                    attrs: vec![attr("amount")],
                },
                ManifestSpanKind {
                    name: "b".to_string(),
                    attrs: Vec::new(),
                },
            ],
            ..HostManifest::default()
        }
    }

    // ── Freeform by default (§4.2, the half that must not regress) ──────

    #[test]
    fn no_manifest_at_all_never_diagnoses_a_span() {
        let diags = run(
            "flow a() {\n  He hands you <item id=\"lantern\">the lantern</item>.\n}\n",
            None,
        );
        assert!(
            diags.is_empty(),
            "freeform default must stay silent: {diags:?}"
        );
    }

    #[test]
    fn a_manifest_with_externals_but_no_markup_key_never_diagnoses_a_span() {
        // The load-bearing case: registering a manifest for the *externals*
        // vocabulary must not silently opt a project into markup checking.
        // `markup` is what tightens; nothing else does.
        let manifest = HostManifest {
            externals: vec![ManifestExternal {
                name: "play_sfx".to_string(),
                params: Vec::new(),
                returns: brink_ir::TypeRef::default(),
                kind: brink_ir::ExternalKind::default(),
                doc: None,
                widgets: Vec::new(),
                path: Vec::new(),
            }],
            types: vec![SemanticTypeDef {
                name: "actor_id".to_string(),
                base: brink_ir::BaseType::Int,
                constraint: None,
                values: None,
                widget: None,
            }],
            markup: Vec::new(),
        };
        let diags = run(
            "flow a() {\n  <totally undeclared=\"yes\">hi</totally>\n}\n",
            Some(&manifest),
        );
        assert!(
            diags.is_empty(),
            "an externals-only manifest must not enable markup checking: {diags:?}"
        );
    }

    #[test]
    fn a_declared_vocabulary_still_accepts_every_declared_tag() {
        let diags = run(
            "flow a() {\n  <wave amount=\"3\">shimmer</wave> and <b>bold</b>\n}\n",
            Some(&wave_manifest()),
        );
        assert!(diags.is_empty(), "declared tags must pass: {diags:?}");
    }

    // ── Manifest-validated (the half this module adds) ──────────────────

    #[test]
    fn an_undeclared_tag_reports_e164_once() {
        let diags = run(
            "flow a() {\n  <shake power=\"9\">whoa</shake>\n}\n",
            Some(&wave_manifest()),
        );
        assert_eq!(codes(&diags), ["E164"], "{diags:?}");
        assert!(
            diags[0].message.contains("<shake>"),
            "message must name the tag: {}",
            diags[0].message
        );
    }

    #[test]
    fn an_undeclared_attribute_on_a_declared_tag_reports_e165() {
        let diags = run(
            "flow a() {\n  <wave speed=\"2\">shimmer</wave>\n}\n",
            Some(&wave_manifest()),
        );
        assert_eq!(codes(&diags), ["E165"], "{diags:?}");
        assert!(
            diags[0].message.contains("speed") && diags[0].message.contains("<wave>"),
            "message must name attribute and tag: {}",
            diags[0].message
        );
    }

    #[test]
    fn an_undeclared_tag_does_not_cascade_one_e165_per_attribute() {
        let diags = run(
            "flow a() {\n  <shake power=\"9\" decay=\"1\">whoa</shake>\n}\n",
            Some(&wave_manifest()),
        );
        assert_eq!(codes(&diags), ["E164"], "{diags:?}");
    }

    #[test]
    fn a_self_closing_undeclared_tag_reports_e164() {
        let diags = run(
            "flow a() {\n  Bell tolls. <pause/> Door slams.\n}\n",
            Some(&wave_manifest()),
        );
        assert_eq!(codes(&diags), ["E164"], "{diags:?}");
    }

    #[test]
    fn a_nested_span_is_checked_not_just_the_outermost() {
        let diags = run(
            "flow a() {\n  <b><glitch>hi</glitch></b>\n}\n",
            Some(&wave_manifest()),
        );
        assert_eq!(codes(&diags), ["E164"], "{diags:?}");
        assert!(
            diags[0].message.contains("<glitch>"),
            "{}",
            diags[0].message
        );
    }

    #[test]
    fn a_span_inside_a_conditional_branch_is_checked() {
        // §4.3 lets logic and markup nest freely; the branch body is its own
        // `Content` node, so this proves the walker reaches it.
        let diags = run(
            "flow a(hp) {\n  {if hp > 0: <glitch>yawn</glitch> else: Ready.}\n}\n",
            Some(&wave_manifest()),
        );
        assert_eq!(codes(&diags), ["E164"], "{diags:?}");
    }

    #[test]
    fn a_span_in_a_real_choice_point_is_checked_in_every_display_region() {
        // A real `{? … }` choice point, not a bare bullet — native has no
        // bare knot-level `*`/`+` (`brink-syntax-native/src/parser/choice.rs`:
        // "All choices live inside a point"). `lower_choice_region` now
        // stamps a real per-region `Content::ptr` from each region's own
        // syntax node (start/bracket/inner — issue #3181/#3202), so this no
        // longer pins an *absent*-provenance case; it pins that a span in
        // each of the three still gets diagnosed via its own `SpanPart::ptr`
        // (issue #1782), independent of whichever provenance the enclosing
        // `Content` itself now carries.
        let diags = run(
            "flow a() {\n  {?\n    * <glitch>start</glitch>[<shake>bracket</shake>]<wobble>inner</wobble>\n  }\n}\n",
            Some(&wave_manifest()),
        );
        assert_eq!(codes(&diags), ["E164", "E164", "E164"], "{diags:?}");
        assert!(
            diags[0].message.contains("<glitch>"),
            "{}",
            diags[0].message
        );
        assert!(diags[1].message.contains("<shake>"), "{}", diags[1].message);
        assert!(
            diags[2].message.contains("<wobble>"),
            "{}",
            diags[2].message
        );
    }

    #[test]
    fn markup_written_inside_a_tag_is_tag_text_not_a_span() {
        // Pins `enter_content`'s reason for skipping `content.tags`: native
        // `lower_tag` flattens a tag's raw tokens into one `Text` part, so
        // there is no `Span` under a tag to check. Guards the skip — if tag
        // lowering ever produces real content parts, this goes red and the
        // walker has to grow a tag descent.
        let hir = lower("flow a() {\n  Hello. # <glitch>loud</glitch>\n}\n");
        let tags: Vec<_> = hir
            .knots
            .iter()
            .flat_map(|k| &k.body.stmts)
            .filter_map(|s| match s {
                brink_ir::hir::Stmt::Content(c) => Some(&c.tags),
                _ => None,
            })
            .flatten()
            .collect();
        assert!(!tags.is_empty(), "fixture must produce a tag: {hir:?}");
        for tag in tags {
            // Asserts the content, not just that every part happens to be
            // `Text` — that predicate is vacuously true on an empty `parts`,
            // exactly the state a future grammar change (a `Span` nested
            // inside `TAG`) would produce silently through this guard.
            assert_eq!(
                tag.parts,
                vec![ContentPart::Text("<glitch>loud</glitch>".to_string())]
            );
        }
    }

    #[test]
    fn every_undeclared_tag_on_one_line_is_reported() {
        let diags = run(
            "flow a() {\n  <glitch>a</glitch> and <shake>b</shake>\n}\n",
            Some(&wave_manifest()),
        );
        assert_eq!(codes(&diags), ["E164", "E164"], "{diags:?}");
    }

    // ── Per-span provenance (issue #1782) ────────────────────────────────

    #[test]
    fn two_different_undeclared_tags_on_one_line_get_distinct_squiggle_ranges() {
        // Before #1782, `SpanPart` carried no `Provenance` of its own, so
        // both diagnostics pointed at the *whole content line* — a consumer
        // saw two identical-range squiggles instead of one per tag.
        let src = "flow a() {\n  <glitch>a</glitch> and <shake>b</shake>\n}\n";
        let diags = run(src, Some(&wave_manifest()));
        assert_eq!(codes(&diags), ["E164", "E164"], "{diags:?}");
        assert_ne!(
            diags[0].range, diags[1].range,
            "two spans on one line must not share a range: {diags:?}"
        );
        assert_eq!(
            &src[usize::from(diags[0].range.start())..usize::from(diags[0].range.end())],
            "<glitch>a</glitch>",
            "range must cover only the offending span, not the whole line"
        );
        assert_eq!(
            &src[usize::from(diags[1].range.start())..usize::from(diags[1].range.end())],
            "<shake>b</shake>",
            "range must cover only the offending span, not the whole line"
        );
    }

    #[test]
    fn repeated_undeclared_tag_on_one_line_still_gets_per_occurrence_ranges() {
        // The sharper case from #1782: the *same* undeclared tag twice on
        // one line used to produce two byte-identical diagnostics (same
        // code, same line-wide range, same message) — indistinguishable to
        // a consumer. Message text is still identical (both name `<pulse>`),
        // but the range must now single out each occurrence.
        let src = "flow a() {\n  <pulse>a</pulse> <pulse>b</pulse>\n}\n";
        let diags = run(src, Some(&wave_manifest()));
        assert_eq!(codes(&diags), ["E164", "E164"], "{diags:?}");
        assert_eq!(
            diags[0].message, diags[1].message,
            "same undeclared tag name -> same message text: {diags:?}"
        );
        assert_ne!(
            diags[0].range, diags[1].range,
            "repeated undeclared tag must still get per-occurrence ranges: {diags:?}"
        );
        assert_eq!(
            &src[usize::from(diags[0].range.start())..usize::from(diags[0].range.end())],
            "<pulse>a</pulse>"
        );
        assert_eq!(
            &src[usize::from(diags[1].range.start())..usize::from(diags[1].range.end())],
            "<pulse>b</pulse>"
        );
    }

    // ── Per-attribute provenance (issue #1829) ───────────────────────────

    #[test]
    fn two_undeclared_attributes_on_one_span_get_distinct_squiggle_ranges() {
        // Before #1829, `SpanPart::attrs` had no per-attribute provenance,
        // so every `E165` for a span pointed at the *whole span* — a
        // consumer saw two identical-range squiggles instead of one per
        // undeclared attribute. `wave_manifest()` allows only `amount` on
        // `wave`, so both `speed` and `decay` are undeclared.
        let src = "flow a() {\n  <wave amount=\"1\" speed=\"2\" decay=\"3\">shimmer</wave>\n}\n";
        let diags = run(src, Some(&wave_manifest()));
        assert_eq!(codes(&diags), ["E165", "E165"], "{diags:?}");
        // Distinguishability, not merely "a snapshot changed": the two
        // ranges must actually differ, AND each must cover only its own
        // attribute's text — asserting equal-but-wrong ranges would pass a
        // snapshot-only test just as happily.
        assert_ne!(
            diags[0].range, diags[1].range,
            "two undeclared attributes on one span must not share a range: {diags:?}"
        );
        assert_eq!(
            &src[usize::from(diags[0].range.start())..usize::from(diags[0].range.end())],
            "speed=\"2\"",
            "range must cover only the offending attribute, not the whole span"
        );
        assert_eq!(
            &src[usize::from(diags[1].range.start())..usize::from(diags[1].range.end())],
            "decay=\"3\"",
            "range must cover only the offending attribute, not the whole span"
        );
    }

    #[test]
    fn repeated_undeclared_attribute_name_on_one_span_still_gets_per_occurrence_ranges() {
        // The sharper case, mirroring `repeated_undeclared_tag_on_one_line_
        // still_gets_per_occurrence_ranges` above: the *same* undeclared
        // attribute name twice on one span used to produce two
        // byte-identical diagnostics (same code, same whole-span range,
        // same message) — indistinguishable to a consumer. §1829's fence:
        // this does NOT diagnose the duplicate attribute *name* itself
        // (that is a separate, unfiled diagnostic) — it only proves each
        // occurrence's `E165` now gets its own range.
        let src = "flow a() {\n  <wave speed=\"1\" speed=\"2\">shimmer</wave>\n}\n";
        let diags = run(src, Some(&wave_manifest()));
        assert_eq!(codes(&diags), ["E165", "E165"], "{diags:?}");
        assert_eq!(
            diags[0].message, diags[1].message,
            "same undeclared attribute name -> same message text: {diags:?}"
        );
        assert_ne!(
            diags[0].range, diags[1].range,
            "repeated undeclared attribute name must still get per-occurrence ranges: {diags:?}"
        );
        assert_eq!(
            &src[usize::from(diags[0].range.start())..usize::from(diags[0].range.end())],
            "speed=\"1\""
        );
        assert_eq!(
            &src[usize::from(diags[1].range.start())..usize::from(diags[1].range.end())],
            "speed=\"2\""
        );
    }

    #[test]
    fn a_single_undeclared_attribute_still_reports_e165_narrowed_to_that_attribute() {
        // Positive control alongside the #1820 tests above: the
        // single-undeclared-attribute case (already pinned by
        // `an_undeclared_attribute_on_a_declared_tag_reports_e165`, message
        // only) keeps working — and now also gets a range narrower than
        // the whole span, which is strictly more precise, not a
        // regression.
        let src = "flow a() {\n  <wave speed=\"2\">shimmer</wave>\n}\n";
        let diags = run(src, Some(&wave_manifest()));
        assert_eq!(codes(&diags), ["E165"], "{diags:?}");
        assert_eq!(
            &src[usize::from(diags[0].range.start())..usize::from(diags[0].range.end())],
            "speed=\"2\""
        );
    }

    #[test]
    fn a_missing_required_attribute_stays_span_ranged_not_attribute_ranged() {
        // E173 (issue #1997) reports a *missing* attribute, which has no
        // `SpanAttr` syntax node in source to point at — unlike E165, it
        // must keep pointing at the whole span. Guards against #1829's fix
        // accidentally narrowing E173 too.
        let manifest = HostManifest {
            markup: vec![ManifestSpanKind {
                name: "sfx".to_string(),
                attrs: vec![required_attr("volume")],
            }],
            ..HostManifest::default()
        };
        let src = "flow a() {\n  <sfx>clank</sfx>\n}\n";
        let diags = run(src, Some(&manifest));
        assert_eq!(codes(&diags), ["E173"], "{diags:?}");
        assert_eq!(
            &src[usize::from(diags[0].range.start())..usize::from(diags[0].range.end())],
            "<sfx>clank</sfx>",
            "a missing attribute has no node of its own; E173 must stay span-ranged"
        );
    }

    // ── Severity is configurable, which needs a `Warning` base ──────────

    #[test]
    fn both_codes_default_to_warning_so_they_stay_overridable_and_suppressible() {
        // `[lints]` overrides and `@[allow(…)]` both refuse `Error`-base
        // codes outright (`brink_ir::suppressions`), so this *is* the
        // "configurable severity" requirement, not a cosmetic choice.
        assert_eq!(DiagnosticCode::E164.severity(), brink_ir::Severity::Warning);
        assert_eq!(DiagnosticCode::E165.severity(), brink_ir::Severity::Warning);
        assert_eq!(DiagnosticCode::E173.severity(), brink_ir::Severity::Warning);
    }

    // ── Determinism ─────────────────────────────────────────────────────

    #[test]
    fn duplicate_kind_declarations_merge_their_attribute_sets() {
        let manifest = HostManifest {
            markup: vec![
                ManifestSpanKind {
                    name: "wave".to_string(),
                    attrs: vec![attr("amount")],
                },
                ManifestSpanKind {
                    name: "wave".to_string(),
                    attrs: vec![attr("speed")],
                },
            ],
            ..HostManifest::default()
        };
        let diags = run(
            "flow a() {\n  <wave amount=\"3\">x</wave> <wave speed=\"2\">y</wave>\n}\n",
            Some(&manifest),
        );
        assert!(diags.is_empty(), "both attrs must be accepted: {diags:?}");
    }

    // ── Required attributes (issue #1780/#1997) ─────────────────────────

    /// A vocabulary declaring `<sfx name="…" volume="…">` where `volume` is
    /// `required` and `name` is not.
    fn sfx_manifest() -> HostManifest {
        HostManifest {
            markup: vec![ManifestSpanKind {
                name: "sfx".to_string(),
                attrs: vec![attr("name"), required_attr("volume")],
            }],
            ..HostManifest::default()
        }
    }

    #[test]
    fn a_span_carrying_its_required_attribute_is_accepted() {
        let diags = run(
            "flow a() {\n  <sfx name=\"door\" volume=\"3\">clank</sfx>\n}\n",
            Some(&sfx_manifest()),
        );
        assert!(diags.is_empty(), "required attribute present: {diags:?}");
    }

    #[test]
    fn a_span_missing_its_required_attribute_reports_e173() {
        let diags = run(
            "flow a() {\n  <sfx name=\"door\">clank</sfx>\n}\n",
            Some(&sfx_manifest()),
        );
        assert_eq!(codes(&diags), ["E173"], "{diags:?}");
        assert!(
            diags[0].message.contains("volume") && diags[0].message.contains("<sfx>"),
            "message must name the missing attribute and the tag: {}",
            diags[0].message
        );
    }

    #[test]
    fn a_span_missing_every_optional_attribute_but_none_required_is_not_diagnosed() {
        // `wave_manifest`'s `amount` is not required, so a bare `<wave>` with
        // no attributes at all must not fire E173 (or E165 — it carries no
        // undeclared attribute either).
        let diags = run(
            "flow a() {\n  <wave>shimmer</wave>\n}\n",
            Some(&wave_manifest()),
        );
        assert!(diags.is_empty(), "no required attrs declared: {diags:?}");
    }

    #[test]
    fn a_span_missing_several_required_attributes_reports_one_e173_per_attribute() {
        let manifest = HostManifest {
            markup: vec![ManifestSpanKind {
                name: "sfx".to_string(),
                attrs: vec![required_attr("name"), required_attr("volume")],
            }],
            ..HostManifest::default()
        };
        let diags = run("flow a() {\n  <sfx>clank</sfx>\n}\n", Some(&manifest));
        // Sorted (`BTreeSet`-driven) order: "name" < "volume".
        assert_eq!(codes(&diags), ["E173", "E173"], "{diags:?}");
        assert!(diags[0].message.contains("name"), "{}", diags[0].message);
        assert!(diags[1].message.contains("volume"), "{}", diags[1].message);
    }

    #[test]
    fn an_undeclared_tag_does_not_also_report_e173() {
        // The same gating E165 already has: an undeclared tag reports E164
        // alone. There is no declared kind to have a required-attribute set
        // in the first place, so this is really pinning that `check_span`'s
        // `None` arm never falls through into the `Some` arm's E173 logic.
        let diags = run(
            "flow a() {\n  <shake power=\"9\">whoa</shake>\n}\n",
            Some(&sfx_manifest()),
        );
        assert_eq!(codes(&diags), ["E164"], "{diags:?}");
    }

    #[test]
    fn duplicate_kind_declarations_union_required_rather_than_overwrite() {
        // The first declaration marks `volume` required; the second
        // (same-named) declaration only adds `name` as an allowed attribute
        // and says nothing about `volume`. The merge must not let the
        // second declaration silently un-require `volume`.
        let manifest = HostManifest {
            markup: vec![
                ManifestSpanKind {
                    name: "sfx".to_string(),
                    attrs: vec![required_attr("volume")],
                },
                ManifestSpanKind {
                    name: "sfx".to_string(),
                    attrs: vec![attr("name")],
                },
            ],
            ..HostManifest::default()
        };
        let diags = run(
            "flow a() {\n  <sfx name=\"door\">clank</sfx>\n}\n",
            Some(&manifest),
        );
        assert_eq!(
            codes(&diags),
            ["E173"],
            "volume must still be required after the merge: {diags:?}"
        );
    }
}