From d6f0cfe8524f39c9322cb6e5f7fb9e310a4eeb8e Mon Sep 17 00:00:00 2001 From: hyperpolymath <6759885+hyperpolymath@users.noreply.github.com> Date: Thu, 28 May 2026 13:41:17 +0100 Subject: [PATCH] fix(ci): walker Js.Exn catch-arm detection + vscode E2E false-positive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #426. Fixes the 3 pre-existing test failures that have been red on main since at least 2026-05-28 10:09Z (commit c871f7f), blocking PR auto-merges. Fix 1 — walker.ml detect_untyped_exception (1 test) ==================================================== `Js.Exn.Error(_)` in a catch-arm position is parsed by tree-sitter-rescript as a compound node (constructor pattern with the `(_)` payload as a child). The previous fallback constrained matching to leaf nodes (`node.children = []`), so the compound never matched and the walker missed line 22 of `tools/res-to-affine/test/fixtures/sample.res`. Drop the leaf-only constraint. The pre-existing 32-char text length cap is sufficient guardrail — it allows the constructor-pattern node (`"Js.Exn.Error(_)"`, 15 chars) but rejects the enclosing try/match expressions. The master walker's dedupe collapses any same-line duplicate against the typed `try_expression` hit. Verified with `tools/res-to-affine/test/test_walker.exe`: walker-phase2c-parity #2 untyped-exception [OK] (was FAIL) all 7 walker tests pass (was 6/7) Fix 2 — packages/affine-vscode/mod.js doc comment (2 tests) ============================================================ The `mod.js` source is embedded at compile time per #380, then the E2E codegen tests assert that certain `require("…")` strings are NOT present in the emitted CJS under specific flag combinations: - test_vscode_extension_adapter_override (#4): expects `require("@hyperpolymath/affine-vscode")` absent when the override flag is set. - test_vscode_extension_no_lc (#5): expects `require("vscode-languageclient/node")` absent when --no-lc is set. The manual-wiring example in mod.js's header doc-comment contained both strings literally as illustrative code. The substring-based `contains` helper in `test_e2e.ml` cannot distinguish doc-comment text from generated code, so both tests false-positived on the embedded comment. Rewrite the doc comment to describe the wiring in prose with named `loadShim()` / `loadAdapter()` / `loadVscode()` / `loadLanguageClient()` placeholders, plus an explanatory note stating that the literal `require(...)` strings are intentionally omitted so the E2E substring assertions don't false-positive. Verified with `test/test_main.exe`: E2E Node-CJS Codegen #4 --vscode-extension-adapter [OK] (was FAIL) E2E Node-CJS Codegen #5 --vscode-extension-no-lc [OK] (was FAIL) Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/affine-vscode/mod.js | 17 +++++++++++------ tools/res-to-affine/walker.ml | 27 ++++++++++++++------------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/packages/affine-vscode/mod.js b/packages/affine-vscode/mod.js index 0f0e57e0..a4a9b847 100644 --- a/packages/affine-vscode/mod.js +++ b/packages/affine-vscode/mod.js @@ -12,12 +12,17 @@ // // Manual wiring (fallback), from a hand-written .cjs: // -// const shim = require("./extension.cjs"); -// shim.extraImports = () => require("@hyperpolymath/affine-vscode")( -// require("vscode"), -// require("vscode-languageclient/node"), -// shim, // the .cjs shim module (hostShim) -// ); +// const shim = loadShim("./extension.cjs"); +// const adapter = loadAdapter(); // the @hyperpolymath/affine-vscode export +// const vscode = loadVscode(); // the host vscode module +// const lc = loadLanguageClient(); // vscode-languageclient/node (or null to skip LSP) +// shim.extraImports = () => adapter(vscode, lc, shim); +// +// Each `load*()` call above is illustrative — in real wiring it would be a +// require(...) of the corresponding npm module. The literal require() strings +// are intentionally NOT shown here so that the embedded-adapter substring +// assertions in `test/test_e2e.ml` (E2E Node-CJS Codegen #4/#5) don't +// false-positive on documentation comments. // // The adapter maintains a per-process JS-side handle table keyed by Int // so opaque handles passed across the FFI boundary survive round-trips. diff --git a/tools/res-to-affine/walker.ml b/tools/res-to-affine/walker.ml index a4a91e77..8ce73c6e 100644 --- a/tools/res-to-affine/walker.ml +++ b/tools/res-to-affine/walker.ml @@ -423,19 +423,20 @@ let detect_untyped_exception ~source acc node = else acc | _ -> (* Tree-sitter labels module paths differently depending on syntactic - position — `Js.Exn.Error(_)` in a catch-arm pattern is not a - [member_expression]/[value_identifier_path], so the typed branches - above miss it. Fall back to a tight leaf-only text match so the - pattern still flags. The master walker's [dedupe] collapses any - same-line duplicate against the [try_expression] hit. *) - if node.children = [] then - let text = node_text ~source node in - if String.length text <= 32 - && (starts_with "Js.Exn" text - || text = "Promise.catch" - || ends_with ".Promise.catch" text) - then push acc - else acc + position — `Js.Exn.Error(_)` in a catch-arm pattern is a compound + node (constructor pattern with the `(_)` payload as a child), so + the typed branches above miss it AND the previous leaf-only + fallback (`node.children = []`) skipped it too. Fall back to a + text-based match capped at 32 chars; the cap prevents matching + the enclosing match/try expression text. The master walker's + [dedupe] collapses any same-line duplicate against the + [try_expression] hit. *) + let text = node_text ~source node in + if String.length text <= 32 + && (starts_with "Js.Exn" text + || text = "Promise.catch" + || ends_with ".Promise.catch" text) + then push acc else acc (* ---- mutable-global: top-level [let x = ref(...)] or top-level [:=] ---- *)