From 74b795a989e8df5228eeed0e40ded582bb52d982 Mon Sep 17 00:00:00 2001 From: Thomas Dickson Date: Fri, 17 Jul 2026 09:39:41 +0100 Subject: [PATCH 1/4] fix(engine): resolve .js-extension imports against sibling .ts/.tsx files (CD-104) TypeScript's NodeNext/ESM convention writes `.js` in an import specifier (`import "./foo.js"`) even when only `foo.ts` exists on disk. The shared oxc_resolver instance had no extension_alias mapping for this, so every such import failed to resolve and Design.OrphanExport treated the target as unimported. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_012sAEyzKZLM1Z4YUcDZxZsh --- .../tests/orphan_js_extension_resolution.rs | 161 ++++++++++++++++++ crates/cofferdam-engine/src/graph.rs | 17 ++ 2 files changed, 178 insertions(+) create mode 100644 crates/cofferdam-cli/tests/orphan_js_extension_resolution.rs diff --git a/crates/cofferdam-cli/tests/orphan_js_extension_resolution.rs b/crates/cofferdam-cli/tests/orphan_js_extension_resolution.rs new file mode 100644 index 00000000..685a7510 --- /dev/null +++ b/crates/cofferdam-cli/tests/orphan_js_extension_resolution.rs @@ -0,0 +1,161 @@ +//! Regression test for `Design.OrphanExport` false-positives on TypeScript's +//! NodeNext/ESM `.js`-extension import convention (CD-104). +//! +//! Under `"module": "NodeNext"` / `"moduleResolution": "NodeNext"`, Node's ESM +//! loader requires import specifiers to carry the *compiled output* +//! extension (`.js`) even though the source file on disk is `.ts` — e.g. +//! `import { x } from "./foo.js"` where only `foo.ts` exists. Without +//! resolving `.js` specifiers against sibling `.ts`/`.tsx` files, every such +//! import fails to resolve and the target's export is reported orphan. + +use std::path::PathBuf; +use std::process::Command; + +fn cofferdam_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_cofferdam")) +} + +fn run_check(root: &std::path::Path) -> (serde_json::Value, String) { + let out = Command::new(cofferdam_bin()) + .args(["check", "--no-baseline", "--format=json", "."]) + .current_dir(root) + .output() + .expect("spawn cofferdam"); + let stdout = String::from_utf8_lossy(&out.stdout).into_owned(); + let v: serde_json::Value = serde_json::from_str(&stdout).unwrap_or_else(|e| { + panic!( + "cofferdam stdout not valid JSON: {e}\nstdout={stdout}\nstderr={}", + String::from_utf8_lossy(&out.stderr) + ) + }); + (v, stdout) +} + +fn orphan_files(v: &serde_json::Value) -> Vec { + v["findings"] + .as_array() + .map(|arr| { + arr.iter() + .filter(|f| f["id"].as_str() == Some("Design.OrphanExport")) + .filter_map(|f| f["file"].as_str().map(str::to_string)) + .collect() + }) + .unwrap_or_default() +} + +fn write_genuine_orphan(root: &std::path::Path) { + std::fs::create_dir_all(root.join("lib")).ok(); + std::fs::write( + root.join("lib").join("dead.ts"), + "export function dead() { return 0; }\n", + ) + .expect("write dead"); +} + +/// The exact CD-104 repro: a same-directory relative import written with a +/// `.js` specifier against a `.ts` file on disk, under NodeNext resolution. +#[test] +fn orphan_export_not_flagged_for_js_extension_relative_import() { + let dir = tempfile::TempDir::new().expect("tempdir"); + let root = dir.path(); + + std::fs::write( + root.join("package.json"), + r#"{"name": "repro", "type": "module"}"#, + ) + .expect("write package.json"); + + std::fs::write( + root.join("tsconfig.json"), + r#"{ + "compilerOptions": { + "module": "NodeNext", + "moduleResolution": "NodeNext" + } +}"#, + ) + .expect("write tsconfig"); + + std::fs::write( + root.join("difficulty.ts"), + "export function deriveDifficultyProfile() { return 1; }\n", + ) + .expect("write difficulty.ts"); + + std::fs::write( + root.join("aiTactics.ts"), + "import { deriveDifficultyProfile } from \"./difficulty.js\";\nconsole.log(deriveDifficultyProfile());\n", + ) + .expect("write aiTactics.ts"); + + write_genuine_orphan(root); + let (v, stdout) = run_check(root); + let orphans = orphan_files(&v); + + assert!( + orphans.iter().any(|f| f.contains("dead")), + "expected lib/dead.ts to be orphan — confirms OrphanExport is running.\n\ + orphans={orphans:?}" + ); + + assert!( + !orphans.iter().any(|f| f.contains("difficulty")), + "deriveDifficultyProfile is imported via \"./difficulty.js\" from aiTactics.ts \ + (NodeNext .js-extension convention against a .ts file on disk) — must not be orphan.\n\ + Got OrphanExport on: {orphans:?}\nstdout={stdout}" + ); +} + +/// Same convention one directory level down, and with a `.tsx` target, to +/// confirm the extension-alias fallback isn't limited to same-dir `.ts`. +#[test] +fn orphan_export_not_flagged_for_js_extension_nested_tsx_import() { + let dir = tempfile::TempDir::new().expect("tempdir"); + let root = dir.path(); + + std::fs::write( + root.join("package.json"), + r#"{"name": "repro", "type": "module"}"#, + ) + .expect("write package.json"); + + std::fs::write( + root.join("tsconfig.json"), + r#"{ + "compilerOptions": { + "module": "NodeNext", + "moduleResolution": "NodeNext", + "jsx": "react-jsx" + } +}"#, + ) + .expect("write tsconfig"); + + std::fs::create_dir_all(root.join("components")).expect("mkdir components"); + std::fs::write( + root.join("components").join("Widget.tsx"), + "export function Widget() { return null; }\n", + ) + .expect("write Widget.tsx"); + + std::fs::write( + root.join("main.ts"), + "import { Widget } from \"./components/Widget.js\";\nconsole.log(Widget);\n", + ) + .expect("write main.ts"); + + write_genuine_orphan(root); + let (v, stdout) = run_check(root); + let orphans = orphan_files(&v); + + assert!( + orphans.iter().any(|f| f.contains("dead")), + "expected lib/dead.ts to be orphan — sanity check. orphans={orphans:?}" + ); + + assert!( + !orphans.iter().any(|f| f.contains("Widget")), + "Widget is imported via \"./components/Widget.js\" (NodeNext convention against \ + a .tsx file on disk) — must not be orphan.\nGot OrphanExport on: {orphans:?}\nstdout={stdout}" + ); +} diff --git a/crates/cofferdam-engine/src/graph.rs b/crates/cofferdam-engine/src/graph.rs index 6cd8e47e..c6e443b2 100644 --- a/crates/cofferdam-engine/src/graph.rs +++ b/crates/cofferdam-engine/src/graph.rs @@ -82,6 +82,23 @@ impl GraphBuilder { ".cjs".into(), ".json".into(), ], + // TS NodeNext/ESM convention (CD-104): source writes a `.js` + // specifier (`import "./foo.js"`) that must resolve against + // the sibling `.ts`/`.tsx` file actually on disk, since Node's + // ESM loader requires the extension the *compiled* output will + // have, not the source extension. Without this alias map, + // `resolve_file` only ever looks for a literal `foo.js`, so + // every such import fails to resolve and `Design.OrphanExport` + // treats the target as unimported. + extension_alias: vec![ + ( + ".js".into(), + vec![".ts".into(), ".tsx".into(), ".js".into()], + ), + (".jsx".into(), vec![".tsx".into(), ".jsx".into()]), + (".mjs".into(), vec![".mts".into(), ".mjs".into()]), + (".cjs".into(), vec![".cts".into(), ".cjs".into()]), + ], tsconfig: Some(TsconfigDiscovery::Auto), ..ResolveOptions::default() }; From d6f3c81a954e14e3ecb1a59728ce68a32a695e8b Mon Sep 17 00:00:00 2001 From: Thomas Dickson Date: Fri, 17 Jul 2026 09:40:02 +0100 Subject: [PATCH 2/4] fix(engine): canonicalize paths before baseline path comparison (CD-105) normalize_path compared the freshly-discovered issue path against the project root with a plain Path::strip_prefix on whatever raw form each side happened to be in. On Windows, Path::strip_prefix compares path components byte-for-byte, so a root path with different directory-name casing than the file's own absolutized path (both naming the identical on-disk file, since Windows filesystems are case-preserving but case-insensitive) silently failed to strip. That fell back to the full absolute form, which never matched the stored (relative) baseline entry, so every finding looked new immediately after baseline write. Canonicalizing both sides first makes the comparison form-independent; falls back to the prior raw-strip behavior when either path doesn't exist on disk (synthetic paths in unit tests). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_012sAEyzKZLM1Z4YUcDZxZsh --- .../tests/baseline_workspace_subdir.rs | 81 +++++++++++++++++++ crates/cofferdam-engine/src/baseline.rs | 48 ++++++++++- 2 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 crates/cofferdam-cli/tests/baseline_workspace_subdir.rs diff --git a/crates/cofferdam-cli/tests/baseline_workspace_subdir.rs b/crates/cofferdam-cli/tests/baseline_workspace_subdir.rs new file mode 100644 index 00000000..8e007e2c --- /dev/null +++ b/crates/cofferdam-cli/tests/baseline_workspace_subdir.rs @@ -0,0 +1,81 @@ +//! Regression test for CD-105: `cofferdam check` reported every baselined +//! finding as new immediately after `baseline write`, with no code changes +//! in between. +//! +//! Root cause: `baseline::normalize_path` compared the freshly-discovered +//! `issue.file` against the project root using a plain `Path::strip_prefix` +//! on whatever raw form each side happened to be in. In an npm-workspaces +//! monorepo, `baseline write` is commonly run from the repo root (`roots = +//! ["."]`, so `issue.file` comes back relative, e.g. `pkg/src/a.ts`) while +//! `check` is run per-workspace from a package subdirectory (`roots = +//! ["."]` relative to that subdir, so `issue.file` comes back as `src/a.ts`) +//! with the shared root baseline auto-discovered by walking up from cwd. +//! The stored `pkg/src/a.ts` and the freshly computed `src/a.ts` never +//! string-match, so every finding looks new even though nothing changed. + +use std::path::PathBuf; +use std::process::Command; + +fn cofferdam_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_cofferdam")) +} + +#[test] +fn check_from_workspace_subdir_matches_baseline_written_from_repo_root() { + let dir = tempfile::TempDir::new().expect("temp dir"); + let root = dir.path(); + + std::fs::create_dir_all(root.join("pkg/src")).expect("mkdir pkg/src"); + std::fs::write( + root.join("pkg/src/a.ts"), + "export function f() { if (x == 1) { return 1; } }\n", + ) + .expect("write a.ts"); + + // Baseline is written once from the repo root, covering the whole tree. + let write_out = Command::new(cofferdam_bin()) + .args(["baseline", "write"]) + .current_dir(root) + .output() + .expect("spawn baseline write"); + assert!( + write_out.status.success(), + "baseline write should succeed; stderr={}", + String::from_utf8_lossy(&write_out.stderr) + ); + + let baseline_path = root.join(".cofferdam/baseline.json"); + assert!(baseline_path.is_file(), "baseline.json must be written"); + let baseline_contents = std::fs::read_to_string(&baseline_path).expect("read baseline"); + assert!( + baseline_contents.contains("Warning.TripleEquals"), + "sanity check: expected a TripleEquals finding in the baseline; got:\n{baseline_contents}" + ); + + // `check` runs from the package subdirectory (the npm-workspaces + // per-package script pattern), with no explicit --baseline flag — it + // must auto-discover the repo-root baseline by walking up. + let check_out = Command::new(cofferdam_bin()) + .args(["check", "--format=json", "."]) + .current_dir(root.join("pkg")) + .output() + .expect("spawn check"); + + let stdout = String::from_utf8_lossy(&check_out.stdout); + let v: serde_json::Value = serde_json::from_str(&stdout).unwrap_or_else(|e| { + panic!( + "check stdout not valid JSON: {e}\nstdout={stdout}\nstderr={}", + String::from_utf8_lossy(&check_out.stderr) + ) + }); + + let baselined_count = v["summary"]["baselined"].as_u64().unwrap_or(0); + let new_count = v["summary"]["new"].as_u64().unwrap_or(0); + + assert!( + baselined_count > 0 && new_count == 0, + "findings present at baseline-write time must be suppressed as baselined when \ + `check` runs from a workspace subdirectory with the baseline auto-discovered \ + from the repo root; got new={new_count} baselined={baselined_count}\nstdout={stdout}" + ); +} diff --git a/crates/cofferdam-engine/src/baseline.rs b/crates/cofferdam-engine/src/baseline.rs index e43e7297..0cc73918 100644 --- a/crates/cofferdam-engine/src/baseline.rs +++ b/crates/cofferdam-engine/src/baseline.rs @@ -324,8 +324,24 @@ pub fn entry_for(issue: &Issue, signature: String, root: Option<&Path>) -> Basel /// forward-slash form when the path can't be made relative. pub fn normalize_path(path: &Path, root: Option<&Path>) -> String { let rel = match root { - Some(r) => path.strip_prefix(r).unwrap_or(path), - None => path, + // CD-105: `path` and `root` frequently arrive in different forms + // between `baseline write` and `check` — one relative-to-cwd, the + // other absolute, or (on Windows) differing only in drive-letter + // casing — even though they name the same files. A plain + // `strip_prefix` on the raw forms then silently fails (falling + // back to the untouched absolute `path`), so the stored baseline + // entry and the freshly computed one never string-match and every + // finding looks "new". Canonicalizing both sides first (when the + // paths actually exist on disk) makes the comparison form- + // independent; synthetic/nonexistent paths (unit tests) fall back + // to the prior raw-strip behavior unchanged. + Some(r) => match (path.canonicalize(), r.canonicalize()) { + (Ok(cp), Ok(cr)) => cp + .strip_prefix(&cr) + .map_or_else(|_| path.to_path_buf(), Path::to_path_buf), + _ => path.strip_prefix(r).unwrap_or(path).to_path_buf(), + }, + None => path.to_path_buf(), }; // Filter `.` components introduced by `Path::new(".").join(...)` and // similar paths so the stored form is the cleanest representation. @@ -750,6 +766,34 @@ mod tests { assert_eq!(normalize_path(&p, None), "src/foo.ts"); } + /// CD-105: `Path::strip_prefix` compares path components byte-for-byte. + /// On Windows the filesystem is case-preserving but case-*insensitive*, + /// so the same directory can be reported with different casing by + /// different tools/shells (e.g. `cwd()` returning `...\start-line` in + /// one invocation and `...\Start-Line` in another) while still naming + /// the identical on-disk path. A root path with different casing than + /// the file's own absolutized path then silently failed to strip + /// (`unwrap_or(path)` swallows the error), falling back to the full + /// absolute form — the stored baseline entry (relative, from the write + /// side) never matched the freshly computed one (still absolute, from + /// the check side), so every finding looked new. + #[test] + #[cfg(windows)] + fn normalize_path_ignores_windows_directory_casing_mismatch() { + let dir = tempdir().expect("tempdir"); + let sub = dir.path().join("Src"); + std::fs::create_dir_all(&sub).expect("mkdir Src"); + let file = sub.join("a.ts"); + std::fs::write(&file, "").expect("write a.ts"); + + // Same directory, deliberately different case than what's on disk — + // simulates a root path sourced from a differently-cased cwd/shell. + let root_str = dir.path().to_string_lossy().to_string(); + let cased_root = PathBuf::from(root_str.to_uppercase()); + + assert_eq!(normalize_path(&file, Some(&cased_root)), "Src/a.ts"); + } + #[test] fn write_then_read_roundtrips() { let dir = tempdir().expect("tempdir"); From 654940e8675650cb89cac011c8eddad0c4408778 Mon Sep 17 00:00:00 2001 From: Thomas Dickson Date: Fri, 17 Jul 2026 09:40:43 +0100 Subject: [PATCH 3/4] fix(html): downgrade recovered-parse Warning.ParseError to Low severity (CD-101) Real-repo validation against ERB/EJS/Jinja template directories (Flask templates/, Rails app/views/) found the HTML adapter's recovered-parse path fires routinely on legitimate template idioms tree-sitter-html has no grammar for: an ERB/EJS <% %> scriptlet, or a Jinja {{ url_for("x") }} nested inside a double-quoted attribute. Neither is malformed HTML in its rendered form, only in unrendered template source. Downgraded from Critical to Low so it stays visible without failing CI at the default --fail-on=medium. html_load_error_issue (tree-sitter produced no tree at all) is unaffected and stays Critical, since that's a genuine hard failure regardless of source language. Documented the caveat and the disabled-override workaround in docs/languages.md. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_012sAEyzKZLM1Z4YUcDZxZsh --- .../html_template_parse_error_severity.rs | 106 ++++++++++++++++++ crates/cofferdam-engine/src/lib.rs | 16 ++- docs/languages.md | 14 +++ 3 files changed, 134 insertions(+), 2 deletions(-) create mode 100644 crates/cofferdam-cli/tests/html_template_parse_error_severity.rs diff --git a/crates/cofferdam-cli/tests/html_template_parse_error_severity.rs b/crates/cofferdam-cli/tests/html_template_parse_error_severity.rs new file mode 100644 index 00000000..c197b2e4 --- /dev/null +++ b/crates/cofferdam-cli/tests/html_template_parse_error_severity.rs @@ -0,0 +1,106 @@ +//! Regression test for CD-101: `Warning.ParseError` from the HTML adapter's +//! recovered-parse path (tree-sitter-html hit ERROR/MISSING nodes) fired at +//! `Critical` severity on legitimate ERB/EJS `<% %>` scriptlets and Jinja +//! nested-double-quote attribute idioms — template-source constructs that +//! are only invalid as raw, unrendered HTML. Downgraded to `Low` so it +//! surfaces for visibility without failing CI at the default +//! `--fail-on=medium`. + +use std::path::PathBuf; +use std::process::Command; + +fn cofferdam_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_cofferdam")) +} + +fn parse_error_findings(root: &std::path::Path, filename: &str) -> serde_json::Value { + let out = Command::new(cofferdam_bin()) + .args(["check", "--no-baseline", "--format=json", filename]) + .current_dir(root) + .output() + .expect("spawn cofferdam"); + let stdout = String::from_utf8_lossy(&out.stdout).into_owned(); + let v: serde_json::Value = serde_json::from_str(&stdout).unwrap_or_else(|e| { + panic!( + "cofferdam stdout not valid JSON: {e}\nstdout={stdout}\nstderr={}", + String::from_utf8_lossy(&out.stderr) + ) + }); + v +} + +fn assert_low_severity_parse_error(v: &serde_json::Value, label: &str) { + let findings = v["findings"].as_array().expect("findings array"); + let parse_errors: Vec<&serde_json::Value> = findings + .iter() + .filter(|f| f["id"].as_str() == Some("Warning.ParseError")) + .collect(); + assert!( + !parse_errors.is_empty(), + "{label}: expected a Warning.ParseError finding (tree-sitter-html has no grammar \ + for this template idiom); findings={findings:?}" + ); + for f in &parse_errors { + assert_eq!( + f["severity"].as_str(), + Some("low"), + "{label}: HTML adapter recovered-parse Warning.ParseError must be Low severity \ + (template-source false alarm, not a real bug); got {f:?}" + ); + } +} + +/// Root cause 1 (CD-101): ERB/EJS `<% %>` scriptlet — the leading `<` in +/// `<%` is indistinguishable from a malformed tag-open to an HTML-only +/// grammar. +#[test] +fn erb_style_scriptlet_reports_low_severity_parse_error() { + let dir = tempfile::TempDir::new().expect("temp dir"); + std::fs::write( + dir.path().join("index.html"), + "<% if (user) { %>

Hi <%= user.name %>

<% } %>\n", + ) + .expect("write html"); + + let v = parse_error_findings(dir.path(), "index.html"); + assert_low_severity_parse_error(&v, "ERB/EJS scriptlet"); +} + +/// Root cause 2 (CD-101): Jinja nested-double-quote attribute idiom, e.g. +/// `action="{{ url_for("tasks.add") }}"` — genuinely invalid standalone +/// HTML (unescaped `"` inside a double-quoted attribute value), only valid +/// after Jinja renders it away. +#[test] +fn jinja_nested_quote_attribute_reports_low_severity_parse_error() { + let dir = tempfile::TempDir::new().expect("temp dir"); + std::fs::write( + dir.path().join("index.html"), + "
\n", + ) + .expect("write html"); + + let v = parse_error_findings(dir.path(), "index.html"); + assert_low_severity_parse_error(&v, "Jinja nested-quote attribute"); +} + +/// A clean, fully-rendered HTML file must never trip `Warning.ParseError` +/// at all — sanity check that the adapter isn't just downgrading severity +/// blindly on every file. +#[test] +fn clean_html_reports_no_parse_error() { + let dir = tempfile::TempDir::new().expect("temp dir"); + std::fs::write( + dir.path().join("index.html"), + "

Hello

\n", + ) + .expect("write html"); + + let v = parse_error_findings(dir.path(), "index.html"); + let findings = v["findings"].as_array().expect("findings array"); + assert!( + !findings + .iter() + .any(|f| f["id"].as_str() == Some("Warning.ParseError")), + "clean HTML must not report a parse error; findings={findings:?}" + ); +} diff --git a/crates/cofferdam-engine/src/lib.rs b/crates/cofferdam-engine/src/lib.rs index ce542c54..7f31713e 100644 --- a/crates/cofferdam-engine/src/lib.rs +++ b/crates/cofferdam-engine/src/lib.rs @@ -1721,6 +1721,16 @@ fn rust_load_error_issue(file: &SourceFile, err: &cofferdam_rust::RustParseError /// parse recovered with ERROR / MISSING nodes. Mirrors /// `rust_parse_error_issue` — see its docs for the "first span only" /// rationale. +/// +/// Severity is `Low`, not `Critical` (CD-101): real-repo validation against +/// ERB/EJS/Jinja template directories (Flask `templates/`, Rails +/// `app/views/`) found this fires routinely on template idioms tree-sitter- +/// html has no grammar for — a `<% %>` scriptlet, or a Jinja +/// `{{ url_for("x") }}` nested inside a double-quoted attribute — that are +/// only invalid as *unrendered* HTML source, not bugs. Unlike a genuine +/// unparseable file (`html_load_error_issue`, still `Critical`), a +/// recovered parse is usually a template-source false alarm rather than +/// "almost always a real bug". fn html_parse_error_issue(file: &SourceFile, tree: &HtmlParseTree) -> Issue { let span = tree.error_spans().first().copied().unwrap_or(Span { start_byte: 0, @@ -1730,12 +1740,14 @@ fn html_parse_error_issue(file: &SourceFile, tree: &HtmlParseTree) -> Issue { }); Issue { check_id: "Warning.ParseError".to_string(), - message: "parse error: tree-sitter recovered with ERROR / MISSING nodes (HTML adapter)" + message: "parse error: tree-sitter recovered with ERROR / MISSING nodes (HTML adapter) \ + — often a template idiom (ERB/EJS `<% %>`, Jinja nested-quote attributes) \ + rather than malformed HTML; see docs/languages.md#html-build-output--plugin-surface" .to_string(), file: file.path.clone(), location: Location::from_span(&file.path, span), priority: Priority(20), - severity: Severity::Critical, + severity: Severity::Low, related: Vec::new(), } } diff --git a/docs/languages.md b/docs/languages.md index ffcec8f3..43ddc06f 100644 --- a/docs/languages.md +++ b/docs/languages.md @@ -16,6 +16,20 @@ plugin `AstView`/`LineView` surface is the same shape used for TypeScript — [Author guide](/plugin-sdk-guide#html-findall-—-flag-img-with-no-alt-cd-84) and demonstrated end-to-end in [SEO-grade checking](/seo-checking). +**Caveat — template source directories (CD-101):** tree-sitter-html has no +grammar for template-engine delimiters. Scanning *unrendered* template +source (Flask `templates/`, Rails `app/views/`, Express `views/`) with the +default `.html`/`.htm` extension globs routinely trips `Warning.ParseError` +on legitimate idioms — an ERB/EJS `<% %>` scriptlet, or a Jinja +`{{ url_for("x") }}` nested inside a double-quoted attribute — that are +only invalid as raw HTML, not bugs. `Warning.ParseError` from the HTML +adapter's recovered-parse path is `Low` severity for exactly this reason, +so it won't fail CI at the default `--fail-on=medium`. If you'd rather not +see it at all, exclude the template directory (`[[overrides]] disabled = +true` scoped to that path, or a `.cofferdamignore` entry) until it's +rendered/compiled — [`cofferdam verify --dist`](/verify-dist) already +covers the rendered-output case correctly. + ## Rust (second language, dogfood) A Rust adapter ships under `crates/cofferdam-rust`, exercising the engine's per-language dispatch (cd-91zc). Three checks today: From eebaf9c422b5651c8411209de05fd3cce9fac463 Mon Sep 17 00:00:00 2001 From: Thomas Dickson Date: Fri, 17 Jul 2026 09:41:01 +0100 Subject: [PATCH 4/4] fix(plugin-sdk): give .rs files real line views on the plugin wire (CD-93) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The plugin wire's per-language dispatch gave .rs files (Vec::new(), None) for lineViews/ast while file.text remained populated, so a Pattern-A line-scan plugin check (file.lines()) scoped to .rs silently iterated zero lines instead of erroring or being skipped. Astro already got Lines::plain-built (unclassified) line views for the identical reason; Rust gets the same treatment here. No plugin-facing AST wire builder exists for Rust yet, so ast stays None — only lineViews changes. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_012sAEyzKZLM1Z4YUcDZxZsh --- crates/cofferdam-cli/src/plugins.rs | 30 +++++- .../tests/plugin_rust_line_views.rs | 91 +++++++++++++++++++ 2 files changed, 116 insertions(+), 5 deletions(-) create mode 100644 crates/cofferdam-cli/tests/plugin_rust_line_views.rs diff --git a/crates/cofferdam-cli/src/plugins.rs b/crates/cofferdam-cli/src/plugins.rs index f09b151b..21cfe44c 100644 --- a/crates/cofferdam-cli/src/plugins.rs +++ b/crates/cofferdam-cli/src/plugins.rs @@ -748,11 +748,31 @@ pub fn run_plugins_with_sources( .collect(); (line_views, None) } - // Rust, unrecognised: no whole-file parse for - // plugins today (matches the engine's catch-all — - // no built-in check declares Rust for the plugin - // surface yet). - Language::Rust => (Vec::new(), None), + // Rust: no plugin-facing AST wire builder yet (matches + // the engine's catch-all — no built-in check declares + // Rust for the plugin surface yet), but a Pattern-A + // line-scan check scoped to `.rs` needs real line + // text/spans the same way Astro does (CD-93) — + // `Lines::plain` gives unclassified (all flags false) + // line views rather than silently iterating zero lines. + Language::Rust => { + let line_views = cofferdam_core::Lines::plain(text) + .map(|lv| ManifestLineView { + line_no: lv.line_no, + text: lv.text.to_string(), + is_comment: false, + is_doc_comment: false, + is_string_literal: false, + string_literal_ranges: Vec::new(), + is_jsx_text: false, + is_pragma: false, + is_tag: false, + is_text: false, + line_start: lv.line_start, + }) + .collect(); + (line_views, None) + } }; // Resolve layer membership for this file. `None` → JSON `null`. let layer: Option = layers_cfg diff --git a/crates/cofferdam-cli/tests/plugin_rust_line_views.rs b/crates/cofferdam-cli/tests/plugin_rust_line_views.rs new file mode 100644 index 00000000..05fd53de --- /dev/null +++ b/crates/cofferdam-cli/tests/plugin_rust_line_views.rs @@ -0,0 +1,91 @@ +//! Regression test for CD-93: the plugin wire's per-language dispatch +//! (`plugins.rs`) gave `.rs` files `(Vec::new(), None)` for `lineViews`/ +//! `ast` while `file.text` remained populated — a Pattern-A line-scan +//! plugin check (`file.lines()`) scoped to `.rs` would silently iterate +//! zero lines instead of erroring or being skipped. `.astro` already got +//! `Lines::plain`-built (unclassified) line views for the same reason; +//! Rust gets the identical treatment here. + +use std::path::PathBuf; +use std::process::Command; + +fn cofferdam_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_cofferdam")) +} + +fn node_present() -> bool { + Command::new("node") + .arg("--version") + .output() + .map(|o| o.status.success()) + .unwrap_or(false) +} + +/// Reports one finding per line seen via `file.lines()`, scoped to `.rs` +/// only. Plain ESM, no SDK dependency (matches the style of other +/// plain-ESM plugin fixtures in this test suite). +const RUST_LINE_COUNTER_PLUGIN: &str = r#" +export default { + id: "Test.RustLineCount", + category: "warning", + basePriority: 5, + defaultSeverity: "medium", + explanation: "reports one finding per line seen via file.lines(), for CD-93 regression cover", + requiresTypes: false, + options: {}, + files: { extensions: ["rs"] }, + run(file, ctx) { + for (const ln of file.lines()) { + ctx.report({ + message: `line ${ln.lineNo}: ${ln.text}`, + span: { start_byte: 0, end_byte: 1 }, + }); + } + }, +}; +"#; + +#[test] +fn rust_files_get_real_line_views_not_empty() { + if !node_present() { + return; + } + let dir = tempfile::TempDir::new().expect("temp dir"); + let plugin_dir = dir.path().join("plugin"); + std::fs::create_dir_all(&plugin_dir).expect("mkdir plugin"); + std::fs::write(plugin_dir.join("index.mjs"), RUST_LINE_COUNTER_PLUGIN).expect("write plugin"); + std::fs::write( + dir.path().join("cofferdam.toml"), + "plugins = [\"./plugin\"]\n", + ) + .expect("write toml"); + std::fs::write( + dir.path().join("a.rs"), + "fn main() {\n println!(\"hi\");\n}\n", + ) + .expect("write rs"); + + let out = Command::new(cofferdam_bin()) + .args(["check", "--no-baseline", "--format=json", "."]) + .current_dir(dir.path()) + .output() + .expect("spawn cofferdam"); + + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + let v: serde_json::Value = serde_json::from_str(&stdout).unwrap_or_else(|e| { + panic!("cofferdam stdout not valid JSON: {e}\nstdout={stdout}\nstderr={stderr}") + }); + + let findings = v["findings"].as_array().expect("findings array"); + let rust_line_findings: Vec<&serde_json::Value> = findings + .iter() + .filter(|f| f["id"].as_str() == Some("Test.RustLineCount")) + .collect(); + + assert!( + !rust_line_findings.is_empty(), + "file.lines() must yield real line views for a .rs file, not silently \ + iterate zero; findings={findings:?}\nstderr={stderr}" + ); +}