diff --git a/.github/workflows/latest-blend-gate.yml b/.github/workflows/latest-blend-gate.yml new file mode 100644 index 0000000..b02a276 --- /dev/null +++ b/.github/workflows/latest-blend-gate.yml @@ -0,0 +1,51 @@ +# Latest-blend pre-release gate (#203) — resolve the LATEST `@juspay/blend-design-system@beta` +# dist-tag (NOT a pinned version), generate with this checkout, and assert it exits 0 with +# non-empty output that compiles on ReScript 12. +# +# Why this exists (see #203): the benchmark pins specific blend versions, but blend is the primary +# downstream target and moves faster than our pins. Twice a regression compiled fine on the pins but +# broke on a newer real one (#110, #198 — the latter a crash-BEFORE-emit that compile gates can't +# see, because there is no output to compile). Only running the LATEST beta catches that class. +# +# Runs: weekly (catch a new blend beta soon after it ships), on demand, and — most importantly — +# manually BEFORE cutting a release. It is intentionally NOT a required PR check: the version floats, +# so it belongs on a cadence + pre-release, not on every PR. +name: Latest-blend gate + +on: + schedule: + - cron: '17 6 * * 1' # Mondays 06:17 UTC — before the weekly stable release cadence + workflow_dispatch: # run this before cutting any release + +concurrency: + group: latest-blend-gate-${{ github.ref }} + cancel-in-progress: true + +jobs: + latest-blend: + name: Generate + compile the latest blend beta + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - uses: actions/checkout@v7 + + - name: Setup Node.js + uses: actions/setup-node@v7 + with: + node-version: 20 + cache: npm + + - run: npm ci + + # Resolves + installs `@juspay/blend-design-system@beta` from the DEFAULT public registry with + # no auth — the same access the benchmark's sandbox `npm ci` already relies on (blend is + # published public). If blend ever becomes access-restricted, add an `npm config set + # //registry.npmjs.org/:_authToken` step with a scoped-read token; until then no secret is needed, + # and an unresolvable `@beta` correctly fails the gate closed rather than passing silently. + - name: Run latest-blend gate + run: node benchmark/run.mjs --latest-blend + + - name: Job summary + if: always() + run: cat benchmark/.work/latest-blend.md >> "$GITHUB_STEP_SUMMARY" || echo "no latest-blend.md produced" >> "$GITHUB_STEP_SUMMARY" diff --git a/CHANGELOG.md b/CHANGELOG.md index 896b15d..ee3aa1e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,30 @@ This project adheres to [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Added + +- **Post-emit dangling-reference guard** (#202) — the reachability sweep's root/child allowlist fell + behind the emitter four times (#191, #195, #197 ×2), each shipping output where a `Module.type` or + `JsFn.t` reference pointed at a declaration the sweep had dropped — a ReScript **compile error** + caught only by manual review or downstream validation. A new allowlist-INDEPENDENT check + (`src/validate.mjs`) scans every written `.res` after generation and asserts each reference into one + of our own file-modules resolves to a real declaration (plus bare `JsFn.t` requires a written + `JsFn.res`). It **hard-fails** the golden suite (controlled output must be clean) and **warns** + (non-fatal) at generation time for real packages, so a text-parse edge case can never break a + user's build. Validated for zero false positives across every golden + benchmark baseline, and it + trips on a synthetic dangling ref (`test/dangling-refs.mjs`). It would have caught all four past + bugs mechanically — "the next reviewer finds it" becomes "CI finds it." + +- **Latest-blend pre-release gate** (#203) — the benchmark pins specific blend versions, but blend is + the primary downstream target and moves faster than the pins; twice a regression compiled on the + pins yet broke on a newer real blend (#110, and #198's crash-before-emit, invisible to compile + gates because there is no output to compile). `npm run bench:latest-blend` (and + `.github/workflows/latest-blend-gate.yml`, weekly + on demand) resolves the live + `@juspay/blend-design-system@beta` dist-tag, generates with the shipped checkout, and fails on a + crash, empty output, or a compile break — surfacing a diff vs the newest pinned baseline. Run it + before cutting any release. Complements pinning the exact failing version as a permanent regression + lock: the pin catches *that* case forever; this gate catches the *next* one. + ## [1.4.0-beta.2] — 2026-08-20 > **Upgrading: regenerate your bindings, and commit `.bindgen-manifest.json` alongside them.** #190 diff --git a/benchmark/README.md b/benchmark/README.md index 47c830c..70edd79 100644 --- a/benchmark/README.md +++ b/benchmark/README.md @@ -41,6 +41,21 @@ The job exits 1 (red check) only on FAIL. | `npm run bench` | verify all packages against baselines (what CI runs) | | `npm run bench:update` | regenerate `baselines//{bindings/,metrics.json,package-lock.json}` | | `node benchmark/run.mjs --only react-markdown` | debug one package (slug or name) | +| `npm run bench:latest-blend` | **pre-release gate (#203):** resolve the LATEST `@juspay/blend-design-system@beta` and prove it still generates + compiles | + +## The latest-blend gate (#203) + +The pins above are frozen, but blend — the primary downstream target — moves faster than them. +Twice a regression compiled on the pins yet broke on a newer real blend (#110, #198; the latter a +**crash-before-emit**, invisible to any compile gate because there is no output to compile). The +latest-blend gate closes that gap: it resolves the live `@juspay/blend-design-system@beta` dist-tag +(not a pin), generates with this checkout, and **fails** on a crash, empty output, or a compile +break. It also prints an informational diff vs the newest pinned blend baseline. + +Run it **before cutting any release** (`npm run bench:latest-blend`). CI runs it weekly and on +demand via `.github/workflows/latest-blend-gate.yml` — deliberately *not* a per-PR required check, +since the version floats. Complements pinning the exact failing version as a permanent regression +lock (as `0.0.38-beta.1` is): the pin catches *that* case forever; this gate catches the *next* one. ## Accepting an intentional output change (WARN → PASS) diff --git a/benchmark/run.mjs b/benchmark/run.mjs index b544a37..2fa4cfa 100644 --- a/benchmark/run.mjs +++ b/benchmark/run.mjs @@ -38,6 +38,13 @@ const WORK = join(HERE, '.work') const UPDATE = process.argv.includes('--update') const onlyIx = process.argv.indexOf('--only') const ONLY = onlyIx === -1 ? null : process.argv[onlyIx + 1] +// #203: the pre-release gate — resolve the LATEST blend beta dist-tag (not a pinned version) and +// prove the shipped checkout still generates + compiles it. blend is the primary downstream target +// and moves faster than our pins; twice a regression compiled on the pins but broke on the newer +// real one (#110, #198 — the latter a crash-before-emit that compile gates can't see, since there +// is no output to compile). Only running the LATEST catches that class before it ships. +const LATEST_BLEND = process.argv.includes('--latest-blend') +const BLEND_PKG = '@juspay/blend-design-system' const YELLOW = (s) => `\x1b[33m${s}\x1b[0m` const slugOf = (name) => name.replace(/[@/]/g, '_') // "@s/p" -> "_s_p" (a leading @ already becomes _) @@ -54,7 +61,7 @@ function sh(cmd, cwd) { * the committed lockfile. node_modules is kept across runs and only reinstalled * when the lockfile stamp changes (so CI can cache it). */ -function setupSandbox(pkg, slug, sandbox) { +function setupSandbox(pkg, slug, sandbox, { freshInstall = false } = {}) { mkdirSync(sandbox, { recursive: true }) const tpl = JSON.parse(readFileSync(join(TEMPLATE, 'package.json'), 'utf-8')) tpl.dependencies[pkg.name] = pkg.version // exact pin, no range @@ -62,8 +69,9 @@ function setupSandbox(pkg, slug, sandbox) { cpSync(join(TEMPLATE, 'rescript.json'), join(sandbox, 'rescript.json')) const committedLock = join(BASELINES, slug, 'package-lock.json') - if (UPDATE) { - // Fresh resolve; the resulting lockfile becomes the committed baseline. + if (UPDATE || freshInstall) { + // Fresh resolve. Under --update the resulting lockfile becomes the committed baseline; under + // --latest-blend (fresh) the version floats and has no committed lockfile to reuse anyway. rmSync(join(sandbox, 'package-lock.json'), { force: true }) const r = sh('npm install --silent --no-audit --no-fund', sandbox) if (!r.ok) throw new Error(`npm install failed:\n${r.out.slice(0, 2000)}`) @@ -141,14 +149,125 @@ function verdictFor(metrics, baseline, diffProblems) { return { verdict: 'PASS', reason: '' } } +/** Semver precedence for the version strings we pin. Returns <0 / 0 / >0. Correct on the subtleties + * that can arise: build metadata (`+sha`) is ignored (spec: not part of precedence); all numeric + * release segments compare numerically (not just the first three); a prerelease is OLDER than its + * release (`0.0.38-beta.1` < `0.0.38`); and numeric prerelease identifiers compare numerically + * (`beta.2` < `beta.10`, `alpha` < `beta` < `rc`). */ +function cmpVersion(a, b) { + const split = (v) => { const s = v.split('+')[0]; const i = s.indexOf('-'); return i < 0 ? [s, ''] : [s.slice(0, i), s.slice(i + 1)] } + const [ma, pa] = split(a), [mb, pb] = split(b) + const na = ma.split('.').map(Number), nb = mb.split('.').map(Number) + for (let i = 0; i < Math.max(na.length, nb.length); i++) if ((na[i] || 0) !== (nb[i] || 0)) return (na[i] || 0) - (nb[i] || 0) + if (!pa && !pb) return 0 + if (!pa) return 1 // a is the release -> newer than any prerelease of it + if (!pb) return -1 + const ia = pa.split('.'), ib = pb.split('.') + for (let i = 0; i < Math.max(ia.length, ib.length); i++) { + const x = ia[i], y = ib[i] + if (x === undefined) return -1 + if (y === undefined) return 1 + if (/^\d+$/.test(x) && /^\d+$/.test(y)) { const dv = Number(x) - Number(y); if (dv) return dv } + else if (x !== y) return x < y ? -1 : 1 + } + return 0 +} + +/** Slug for a package's baseline dir, shared by the main loop AND the latest-blend gate so the + * scheme lives in ONE place: a name pinned at multiple versions gets a version-qualified slug; a + * single-version name keeps its bare slug. */ +function slugForPkg(pkg, allPackages) { + const multiVersion = allPackages.filter((p) => p.name === pkg.name).length > 1 + return slugOf(multiVersion ? `${pkg.name}@${pkg.version}` : pkg.name) +} + +/** Pick the newest pinned blend baseline to diff a floating latest build against — informational + * only; a newer version legitimately differs from every pin. */ +function newestPinnedBlendSlug(allPackages) { + const blends = allPackages.filter((p) => p.name === BLEND_PKG) + if (!blends.length) return null + blends.sort((a, b) => cmpVersion(a.version, b.version)) + return slugForPkg(blends[blends.length - 1], allPackages) +} + +/** #203 pre-release gate: resolve the LATEST blend beta and prove the shipped checkout generates + * non-empty output that compiles on ReScript 12. Fails on crash / empty output / compile break — + * the three ways a "compiles on pinned, breaks on latest" regression shows up. Diff vs the newest + * pinned blend baseline is surfaced for review but never fails the gate (a newer version differs). */ +function runLatestBlendGate(allPackages) { + const ver = sh(`npm view ${BLEND_PKG}@beta version`) + // sh() merges stderr, so an `npm notice`/warning line can precede the value — take the LAST line + // that looks like a version, not blindly the last line. Still fails closed (empty -> exit 1). + const version = (ver.ok ? ver.out.trim().split('\n').map((s) => s.trim()) : []).reverse().find((l) => /^\d+\.\d+\.\d+/.test(l)) || '' + if (!/^\d+\.\d+\.\d+/.test(version)) { + console.error(RED(`✗ could not resolve ${BLEND_PKG}@beta version:\n${ver.out.slice(0, 500)}`)) + process.exit(1) + } + const pkg = { name: BLEND_PKG, version, flags: ['--webapi'] } + const label = `${pkg.name}@${version} (latest beta)` + const slug = slugForPkg(pkg, allPackages) + const workDir = join(WORK, 'latest-blend') + const sandbox = join(workDir, 'sandbox') + mkdirSync(workDir, { recursive: true }) + console.error(DIM(`── ${label} ──`)) + + const pinnedSlug = newestPinnedBlendSlug(allPackages) + const alreadyPinned = allPackages.some((p) => p.name === BLEND_PKG && p.version === version) + if (alreadyPinned) console.error(DIM(` (latest beta ${version} is already pinned in packages.json — the benchmark covers it; this gate re-verifies the LATEST regardless)`)) + + let res + try { + setupSandbox(pkg, slug, sandbox, { freshInstall: true }) + const gen = generate(pkg, sandbox, workDir) + const generated = readDir(join(sandbox, 'src')) + const hasRes = [...generated.keys()].some((f) => f.endsWith('.res')) + const cmp = compile(sandbox, workDir, hasRes) + + const failures = [] + if (gen.exit !== 0) failures.push(`generator exited ${gen.exit} (crash-before-emit — invisible to compile gates)`) + if (!hasRes || generated.size === 0) failures.push('no output produced') + if (hasRes && !cmp.ok) failures.push('generated bindings do not compile on ReScript 12') + + // Informational diff vs the newest pinned blend baseline. + let diff = [] + if (pinnedSlug && existsSync(join(BASELINES, pinnedSlug, 'bindings'))) { + diff = diffDirs(readDir(join(BASELINES, pinnedSlug, 'bindings')), generated).map(stripAnsi) + } + const metrics = { + resolvedVersion: version, generatorExit: gen.exit, compileOk: cmp.ok, warnings: cmp.warnings, + files: generated.size, buckets: gen.summary?.components ?? null, + } + const verdict = failures.length ? 'FAIL' : 'PASS' + res = { pkg: label, slug, verdict, reason: failures.join('; '), metrics, diffVsPinned: pinnedSlug, diff, compileErrors: cmp.errors } + const color = verdict === 'PASS' ? GREEN : RED + console.error(color(`${verdict === 'PASS' ? '✓' : '✗'} ${label}${res.reason ? ' — ' + res.reason : ''}`)) + if (verdict === 'PASS') console.error(DIM(` generated ${generated.size} file(s), compiled clean${diff.length ? `; ${diff.filter((d) => !d.startsWith(' ')).length} diff(s) vs pinned ${pinnedSlug}` : ''}`)) + } catch (e) { + console.error(RED(`✗ ${label} — ${e.message.split('\n')[0]}`)) + res = { pkg: label, slug, verdict: 'FAIL', reason: e.message.split('\n')[0], metrics: null, diff: [] } + } + + mkdirSync(WORK, { recursive: true }) + writeFileSync(join(WORK, 'latest-blend.json'), JSON.stringify(res, null, 2) + '\n') + const md = [`## Latest-blend gate: ${res.verdict === 'PASS' ? '✅ PASS' : '❌ FAIL'}`, '', `- Resolved \`${BLEND_PKG}@beta\` → \`${res.metrics?.resolvedVersion || '?'}\``, + `- Generator exit: ${res.metrics?.generatorExit ?? '—'} · Compiles: ${res.metrics ? (res.metrics.compileOk ? '✅' : '❌') : '—'} · Files: ${res.metrics?.files ?? '—'}`, + res.reason ? `- **Failure:** ${res.reason}` : '', res.diff?.length ? `- Diff vs pinned \`${res.diffVsPinned}\`: ${res.diff.filter((d) => !d.startsWith(' ')).length} file(s) changed (informational)` : ''] + if (res.compileErrors) md.push('', '```', res.compileErrors, '```') + writeFileSync(join(WORK, 'latest-blend.md'), md.filter(Boolean).join('\n') + '\n') + + console.error('') + if (res.verdict === 'FAIL') { console.error(RED(`❌ latest-blend gate FAILED — ${res.reason}`)); process.exit(1) } + console.error(GREEN(`✅ latest-blend gate PASS — ${BLEND_PKG}@${version} generates + compiles`)) + process.exit(0) +} + // ── main ──────────────────────────────────────────────────────────────────── const allPackages = JSON.parse(readFileSync(join(HERE, 'packages.json'), 'utf-8')) +if (LATEST_BLEND) runLatestBlendGate(allPackages) // Two pinned versions of the SAME package (blend 0.0.36 stable + the 0.0.37 beta line that // blend-rescript actually regenerates from) need separate baselines — a duplicated name gets a // version-qualified slug; single-version packages keep their existing baseline dirs. -const nameCount = new Map() -for (const p of allPackages) nameCount.set(p.name, (nameCount.get(p.name) || 0) + 1) -const slugFor = (p) => slugOf(nameCount.get(p.name) > 1 ? `${p.name}@${p.version}` : p.name) +const slugFor = (p) => slugForPkg(p, allPackages) const packages = allPackages.filter((p) => !ONLY || slugFor(p) === ONLY || slugOf(p.name) === ONLY || p.name === ONLY) if (!packages.length) { console.error(RED(`no package matches --only ${ONLY}`)) diff --git a/package.json b/package.json index bf4139d..2eac1c2 100644 --- a/package.json +++ b/package.json @@ -16,16 +16,18 @@ }, "scripts": { "gen": "node src/cli.mjs", - "test": "node test/smoke.mjs && node test/public-name-stability.mjs && node test/representation-flip.mjs && node test/module-move-compat.mjs && node test/golden.mjs && node test/html-attrs.mjs", + "test": "node test/smoke.mjs && node test/public-name-stability.mjs && node test/representation-flip.mjs && node test/module-move-compat.mjs && node test/dangling-refs.mjs && node test/golden.mjs && node test/html-attrs.mjs", "test:smoke": "node test/smoke.mjs", "test:golden": "node test/golden.mjs", "test:names": "node test/public-name-stability.mjs", "test:flip": "node test/representation-flip.mjs", "test:module-move": "node test/module-move-compat.mjs", + "test:dangling": "node test/dangling-refs.mjs", "test:golden:update": "node test/golden.mjs --update", "test:compile": "node test/golden-compile.mjs", "bench": "node benchmark/run.mjs", "bench:update": "node benchmark/run.mjs --update", + "bench:latest-blend": "node benchmark/run.mjs --latest-blend", "gen:attrs": "node scripts/gen-html-attrs.mjs" }, "files": [ diff --git a/src/cli.mjs b/src/cli.mjs index c1c13d3..66bf116 100755 --- a/src/cli.mjs +++ b/src/cli.mjs @@ -19,6 +19,7 @@ import { extractComponent, extractModule } from './extract.mjs' import { emit, emitFunction, emitClass, emitNamespace, report, planSharedModules, emitSharedModule, makeResolveRef } from './emit.mjs' import { resolveInput } from './resolve.mjs' import { writeReport } from './report.mjs' +import { findDanglingRefs } from './validate.mjs' import { planHtmlAttrs, HTML_ATTRS_PIN } from './html-attrs.mjs' import { RESCRIPT_RESERVED } from './stdlib-types.mjs' import { writeFileSync, mkdirSync, existsSync, readFileSync, readdirSync, unlinkSync, renameSync } from 'fs' @@ -665,6 +666,27 @@ async function main() { } } + // #202: post-emit dangling-reference guard. Read back every .res we just wrote and assert each + // `Module.type` / `JsFn.t` reference into one of OUR OWN file-modules resolves to a real + // declaration. This is allowlist-INDEPENDENT — it catches the recurring failure (an emitter + // change outruns the reachability sweep's roots and strands a reference whose declaration was + // dropped → a ReScript compile error) without compiling. WARN, never fail: a real user's build + // must never be blocked by a text-parse edge case (the golden suite hard-fails on controlled + // output instead). Skipped for --stdout (nothing on disk to read back). + if (!opts.stdout) { + const emitted = [] + for (const rel of written) { + if (!rel.endsWith('.res')) continue + try { emitted.push([rel, readFileSync(join(outDir, rel), 'utf-8')]) } catch { /* vanished — skip */ } + } + const dangling = findDanglingRefs(emitted) + if (dangling.length) { + console.error(`[bindgen] ⚠️ ${dangling.length} dangling reference(s) in generated output (please report — #202):`) + for (const p of dangling.slice(0, 20)) console.error(`[bindgen] - ${p}`) + if (dangling.length > 20) console.error(`[bindgen] … and ${dangling.length - 20} more`) + } + } + // The registry is append-only. A removed/moved/renamed upstream declaration becomes inactive, // but its names stay reserved forever; if the same source identity reappears, it gets them back. // Single-file mode has no shared registry, so it preserves any module-mode rows untouched. diff --git a/src/validate.mjs b/src/validate.mjs new file mode 100644 index 0000000..54a36f2 --- /dev/null +++ b/src/validate.mjs @@ -0,0 +1,156 @@ +// ============================================================================ +// validate.mjs — post-emit dangling-reference guard (#202). +// +// The reachability sweep (#191) keeps a root/child allowlist of where a live type +// can be reached from; four times (#191, #195, #197, #197-review) an emitter change +// outran that allowlist and shipped output where a `Module.type` reference pointed +// at a declaration the sweep had dropped — a dangling ref, which is a ReScript +// COMPILE ERROR. Each fix widened the allowlist; the next change outran it again. +// +// This guard is allowlist-INDEPENDENT: after all `.res` files are written, it scans +// each for references to one of OUR OWN file-modules and asserts the referenced name +// is actually declared in that module — plus the bare `JsFn.t` handle must have a +// written `JsFn.res`. It never compiles anything, so it is fast and offline; it is a +// structural mirror of "does this compile?" for the one failure mode that recurs. +// +// Rollout (see #202): HARD-fail in the golden suite (controlled output — must be +// clean); WARN (non-fatal) at generation time for real packages, so a text-parse +// edge case can never break a user's build. +// +// Soundness bias: it is tuned for ZERO false positives on known-good output (verified +// against every golden + benchmark baseline). It only flags a reference into a +// FILE-module (a `.res` whose whole declaration was dropped) — the exact historical +// failure. References into nested `module X = {…}` (resolved intra-file by ReScript's +// own scoping) are deliberately not checked: they never produced the recurring bug and +// checking them risks false positives on legitimate intra-file scoping. +// ============================================================================ + +/** Blank out comments and string literals (to spaces, newlines preserved) so a module-looking + * token inside a `@module(...)`/`@as(...)` string arg, a line or block comment, or a backtick + * template is never mistaken for a real cross-module reference. A single left-to-right state + * machine — NOT ordered regex replaces — so the contexts interact correctly: + * - a line-comment marker inside a string does not start a comment, and vice versa; + * - block comments nest; backslash escapes inside strings/chars are respected; + * - a ReScript CHAR literal (`'x'`, `'\n'`, and critically `'"'`) is masked as a unit, so the + * quote inside it can NEVER open a phantom string that eats real code to EOF — while a type + * VARIABLE (`'a`, `'b`: a quote with no closing quote right after) is left as code; + * - inside a `` `…` `` template, the literal text is masked but a `${ … }` interpolation is + * scanned as CODE, so a real `Module.ref` there is still seen (no false negative). */ +function stripNoise(content) { + const n = content.length + let out = '' + let block = 0 // block-comment nesting depth (0 = not in a block comment) + let tmpl = false // inside a `…` template literal, masking its literal text + const interp = [] // brace depth of each open `${ … }` interpolation being scanned as code + for (let i = 0; i < n; ) { + const c = content[i], d = i + 1 < n ? content[i + 1] : '' + if (block > 0) { + if (c === '/' && d === '*') { block++; out += ' '; i += 2; continue } + if (c === '*' && d === '/') { block--; out += ' '; i += 2; continue } + out += c === '\n' ? '\n' : ' '; i++; continue + } + if (tmpl) { // template literal text: mask, but hand a `${` back to code scanning + if (c === '\\') { out += ' '; i += 2; continue } + if (c === '`') { tmpl = false; out += ' '; i++; continue } + if (c === '$' && d === '{') { interp.push(1); tmpl = false; out += ' '; i += 2; continue } + out += c === '\n' ? '\n' : ' '; i++; continue + } + // --- CODE context (top level, or inside a `${ … }` interpolation) --- + if (c === '/' && d === '*') { block = 1; out += ' '; i += 2; continue } + if (c === '/' && d === '/') { while (i < n && content[i] !== '\n') { out += ' '; i++ } continue } + if (c === '"') { // regular string literal + out += ' '; i++ + while (i < n) { + const e = content[i] + if (e === '\\') { out += ' '; i += 2; continue } + if (e === '"') { out += ' '; i++; break } + out += e === '\n' ? '\n' : ' '; i++ + } + continue + } + if (c === '`') { tmpl = true; out += ' '; i++; continue } // enter a template literal + if (c === "'") { // char literal vs type variable + if (d === '\\' && content[i + 3] === "'") { out += ' '; i += 4; continue } // '\n' '\'' etc. + if (d !== '\\' && d !== '' && content[i + 2] === "'") { out += ' '; i += 3; continue } // 'x' + out += c; i++; continue // type variable ('a, 'b) — leave as code + } + if (interp.length) { // track braces so we know when the interpolation closes back to text + if (c === '{') { interp[interp.length - 1]++; out += c; i++; continue } + if (c === '}') { + if (--interp[interp.length - 1] === 0) { interp.pop(); tmpl = true; out += ' '; i++; continue } + out += c; i++; continue + } + } + out += c; i++ + } + return out +} + +/** Every identifier a module `.res` file DECLARES, collected generously. Over-collection + * is safe here — it can only hide a real dangling ref (a miss), never invent one (a false + * positive) — so we err toward breadth: type/and, let/external, nested module names, + * variant constructors, and record field labels. */ +export function collectDeclaredNames(content) { + const src = stripNoise(content) + const names = new Set() + const add = (re, group = 1) => { + for (const m of src.matchAll(re)) names.add(m[group]) + } + // `\b` (not line-start) so decorators are transparent — `@unboxed type stringOrNumber`, + // `@tag(...) type …`, `@module(...) external …` all still register the declared name; + // optional `rec` covers `type rec styledBlockProps` / `let rec …`. + add(/\b(?:type|and)[ \t]+(?:rec[ \t]+)?([a-z_][A-Za-z0-9_]*)/g) // type / and [rec] + add(/\b(?:let|external)[ \t]+(?:rec[ \t]+)?([a-zA-Z_][A-Za-z0-9_]*)/g) // let / external [rec] + add(/\bmodule[ \t]+([A-Z][A-Za-z0-9_]*)/g) // nested module + add(/[=|][ \t]*([A-Z][A-Za-z0-9_]*)/g) // = FirstArm / | Constructor (the first @unboxed/variant + // arm has no leading `|`; the extra `= Upper` matches are + // harmless over-collection) + add(/[({,\n][ \t]*([a-z_][A-Za-z0-9_]*)[ \t]*\??:/g) // record field label: (over-collects; safe) + return names +} + +/** Find dangling references across a whole emitted output tree. + * @param {Iterable<[string, string]>} files [relativeName, content] for every written file. + * @returns {string[]} human-readable problems (empty = clean). + * + * A reference `M.member` is dangling when `M` is one of our written FILE-modules but + * `member` is not declared anywhere in M's file. Bare `JsFn.` is dangling when no + * `JsFn.res` was written (the #197 failure: usesJsFn recomputed false, yet a + * `@set_index` still emitted a `JsFn.t`). References whose module is external (React, + * JSON, Dict, Dom, ReactEvent, …) are skipped automatically — they are not in the + * written set — and so are nested-module references (M not a file). */ +export function findDanglingRefs(files) { + const entries = [...files].filter(([name]) => name.endsWith('.res')) + // moduleName (file basename, sans dir + `.res`) -> Set of declared names. + const decls = new Map() + const baseName = (rel) => rel.replace(/^.*[/\\]/, '').replace(/\.res$/, '') + for (const [name, content] of entries) decls.set(baseName(name), collectDeclaredNames(content)) + const hasJsFn = decls.has('JsFn') + + const problems = [] + const refRe = /\b([A-Z][A-Za-z0-9_]*)\.([A-Za-z_][A-Za-z0-9_]*)/g + for (const [name, content] of entries) { + const src = stripNoise(content) + const seen = new Set() // de-dupe identical (module, member) misses within one file + for (const m of src.matchAll(refRe)) { + const [, mod, member] = m + if (mod === 'JsFn' && !hasJsFn) { + // No JsFn.res at all: any `JsFn.` is orphaned (the #197 failure). When JsFn.res IS + // present we fall through to the general check below, so a `JsFn.bogus` that JsFn.res + // does not declare is still caught (it would be a compile error just the same). + if (!seen.has('JsFn.' + member)) { + seen.add('JsFn.' + member) + problems.push(`${name} — references \`JsFn.${member}\` but no JsFn.res was emitted (orphaned JsFn handle, #197 class)`) + } + continue + } + const target = decls.get(mod) + if (!target) continue // external module (React/JSON/…) or a nested module — not ours to check + if (!target.has(member) && !seen.has(mod + '.' + member)) { + seen.add(mod + '.' + member) + problems.push(`${name} — dangling reference \`${mod}.${member}\`: ${mod}.res declares no \`${member}\` (declaration dropped by the reachability sweep — #202 class)`) + } + } + } + return problems +} diff --git a/test/dangling-refs.mjs b/test/dangling-refs.mjs new file mode 100644 index 0000000..4159483 --- /dev/null +++ b/test/dangling-refs.mjs @@ -0,0 +1,201 @@ +// ============================================================================ +// dangling-refs.mjs — unit test for the post-emit dangling-reference guard (#202). +// +// The guard is validated for ZERO false positives against every golden + benchmark +// baseline by the golden/benchmark suites themselves (they hard-fail on any flag). +// This file pins the other half of the contract: it TRIPS on the exact failure modes +// that shipped four times (a dropped file-module type; an orphaned `JsFn.t`), and it +// stays silent on the legitimate shapes that look superficially similar (external +// modules, intra-file nested modules, decorated / `rec` declarations). +// +// Run: node test/dangling-refs.mjs +// ============================================================================ +import { findDanglingRefs } from '../src/validate.mjs' +import { GREEN, RED } from './lib/diff.mjs' + +let failed = 0 +function check(label, files, expect /* fn(problems)->bool */, describe) { + const problems = findDanglingRefs(files) + if (expect(problems)) { + console.log(GREEN(`✓ ${label}`)) + } else { + failed++ + console.log(RED(`✗ ${label} — ${describe}`)) + problems.forEach((p) => console.log(' ' + p)) + if (!problems.length) console.log(' (no problems reported)') + } +} +const none = (p) => p.length === 0 +const some = (re) => (p) => p.some((x) => re.test(x)) + +// --- TRIPS: a dropped file-module type (the #195 / #197-review class) --------- +check( + 'dangling type ref into a file-module', + [ + ['Foo.res', 'type props = {size: FooTypes.gone}\n'], + ['FooTypes.res', 'type kept = string\n'], + ], + (p) => p.length === 1 && /FooTypes\.gone/.test(p[0]), + 'should flag FooTypes.gone (FooTypes declares only `kept`)', +) + +// --- TRIPS: orphaned JsFn.t with no JsFn.res (the #197 class) ------------------ +check( + 'orphaned JsFn.t with no JsFn.res', + [['Rec.res', '@set_index external set: (t, string, JsFn.t) => unit = ""\n']], + (p) => p.length === 1 && /JsFn\.t/.test(p[0]), + 'should flag JsFn.t when no JsFn.res emitted', +) + +// --- SILENT: JsFn.t is fine once JsFn.res is emitted -------------------------- +check( + 'JsFn.t resolves when JsFn.res is present', + [ + ['Rec.res', '@set_index external set: (t, string, JsFn.t) => unit = ""\n'], + ['JsFn.res', 'type t\n'], + ], + none, + 'JsFn.res present -> no flag', +) + +// --- SILENT: external modules are never ours to check ------------------------- +check( + 'external modules (React/JSON/Dict/Dom/ReactEvent) never flag', + [['C.res', 'type p = {a: React.element, b: JSON.t, c: Dict.t, d: Dom.element, e: ReactEvent.Mouse.t}\n']], + none, + 'externals are not in the written set', +) + +// --- SILENT: intra-file nested module is resolved by ReScript scoping ---------- +check( + 'intra-file nested-module ref is not checked', + [['HighchartsSharedTypes.res', 'module ColorType = {\n type t = string\n}\ntype series = {color: ColorType.t}\n']], + none, + 'ColorType is a nested module (not a file) -> skipped', +) + +// --- SILENT: decorated + rec declarations register their names ---------------- +check( + 'decorated (@unboxed/@tag) and `rec` declarations resolve', + [ + ['A.res', 'type p = {x: BTypes.stringOrNumber, y: BTypes.node, z: BTypes.tagged}\n'], + ['BTypes.res', '@unboxed type stringOrNumber = Str(string) | Num(float)\ntype rec node = {next?: node}\n@tag("kind") type tagged = | A | B\n'], + ], + none, + 'decorator/rec forms must be collected as declared names', +) + +// --- SILENT: the FIRST @unboxed/variant arm (no leading `|`) is collected ------ +// (review P2: an under-collected first constructor would invent a false positive on +// a cross-module `Mod.FirstArm` reference.) +check( + 'first constructor arm (no leading pipe) is a declared name', + [ + ['M.res', '@unboxed type u = Str(string) | Num(float)\n'], + ['U.res', 'let z = M.Str("a")\nlet w = M.Num(1.0)\n'], + ], + none, + 'both Str (first arm) and Num must be collected', +) + +// --- SILENT: a `//` inside a string must not eat the rest of the line ---------- +// (review P2: line-comment stripping ordered before string stripping was a false positive.) +check( + 'line-comment marker inside a string does not swallow a declaration', + [ + ['A.res', '@module("http://cdn/x") external realThing: int = "realThing"\n'], + ['User.res', 'let x = A.realThing\n'], + ], + none, + 'A.realThing is a real external; the // in the URL must not hide it', +) + +// --- SILENT: nested block comments are fully masked ---------------------------- +// (review P3: a non-greedy block-comment strip leaked the tail after an inner close.) +check( + 'nested block comments do not leak a ref', + [['A.res', '/* outer /* inner */ Gone.here is still comment */\ntype t = int\n']], + none, + 'Gone.here lives inside a nested comment -> not a reference', +) + +// --- SILENT: a char literal `'"'` must not open a phantom string --------------- +// (external-review P2: ReScript HAS char literals — `'"'` is the double-quote char. If `'` were +// not handled, the inner `"` would open a phantom string that eats a later declaration -> FP.) +check( + 'a char literal containing a quote does not swallow a later declaration', + [ + ['A.res', `let sep = '"'\n@module("pkg") external thing: int = "thing"\n`], + ['User.res', 'let x = A.thing\n'], + ], + none, + "the \" inside '\"' must not start a string; A.thing stays visible", +) + +// --- SILENT: type variables ('a, 'b) are still code, not char literals ---------- +check( + "type variables are not mistaken for char literals", + [ + ['R.res', 'let g: JsFn.t = JsFn.fromFn2(h)\n'], + ['JsFn.res', "type t\nexternal fromFn2: (('a, 'b) => 'c) => t = \"%identity\"\n"], + ], + none, + "JsFn.fromFn2 and the 'a/'b/'c type vars must resolve normally", +) + +// --- TRIPS: a real ref inside a `${…}` template interpolation is scanned -------- +// (external-review: masking the whole template was a false NEGATIVE. Interpolations are code.) +check( + 'a dangling ref inside a template interpolation is caught', + [ + ['W.res', 'let s = `hi ${Gone.member} there`\n'], + ['Gone.res', 'type kept = int\n'], + ], + (p) => p.length === 1 && /Gone\.member/.test(p[0]), + 'Gone.member inside ${…} must be checked (Gone declares only kept)', +) + +// --- SILENT: template LITERAL text is still masked ----------------------------- +check( + 'a ref in template literal text (not an interpolation) is ignored', + [['W.res', 'let s = `see Gone.member here`\ntype t = int\n'], ['Gone.res', 'type kept = int\n']], + none, + 'literal template text is not code', +) + +// --- TRIPS: an orphaned JsFn member is caught even when JsFn.res exists --------- +// (external-review: the old blanket `continue` on any JsFn ref skipped member checking.) +check( + 'JsFn. is caught even when JsFn.res is present', + [ + ['R.res', 'let x: JsFn.bogus = y\n'], + ['JsFn.res', 'type t\nexternal fromFn0: (unit => \'a) => t = "%identity"\n'], + ], + (p) => p.length === 1 && /JsFn\.bogus/.test(p[0]), + 'JsFn.res declares no `bogus` -> must flag (it would compile-error)', +) + +// --- SILENT: a valid cross-file type ref resolves ----------------------------- +check( + 'valid cross-file type ref resolves', + [ + ['Widget.res', 'type props = {h: CommonTypes.stringOrNumber}\n'], + ['CommonTypes.res', '@unboxed type stringOrNumber = Str(string) | Num(float)\n'], + ], + none, + 'CommonTypes.stringOrNumber is declared', +) + +// --- SILENT: reference living only inside a string/comment is ignored ---------- +check( + 'refs inside strings/comments are ignored', + [['A.res', '// see FooTypes.gone for history\n@module("Pkg.Sub") external x: int = "x"\ntype t = int\n']], + none, + 'stripNoise removes comment + string module-looking tokens', +) + +if (failed) { + console.log(RED(`\n${failed} dangling-ref test(s) failed.`)) + process.exit(1) +} +console.log(GREEN('\n✅ all dangling-ref guard tests pass')) diff --git a/test/golden.mjs b/test/golden.mjs index b131c26..32fd8a8 100644 --- a/test/golden.mjs +++ b/test/golden.mjs @@ -24,6 +24,7 @@ import { join, dirname } from 'path' import { tmpdir } from 'os' import { fileURLToPath } from 'url' import { GREEN, RED, DIM, readDir, diffDirs } from './lib/diff.mjs' +import { findDanglingRefs } from '../src/validate.mjs' const HERE = dirname(fileURLToPath(import.meta.url)) const ROOT = dirname(HERE) @@ -96,6 +97,10 @@ for (const name of names) { // Drop nothing — every emitted file is part of the snapshot. const problems = [] checkNoStrayIdentity(actual, name, problems) + // #202: post-emit dangling-reference guard — hard-fail here (controlled output must + // be clean). Catches a `Module.type` / `JsFn.t` reference whose declaration an + // incomplete reachability sweep dropped, without needing a ReScript compile. + for (const p of findDanglingRefs(actual)) problems.push(`${name}/${p}`) if (UPDATE) { rmSync(expectedDir, { recursive: true, force: true })