From 252b29b4feeada4e98453b55bc77f465d8453383 Mon Sep 17 00:00:00 2001 From: PathGao Date: Mon, 3 Aug 2026 18:41:19 +0800 Subject: [PATCH] test(scripts): answer scope questions by parsing, and guard 71 slice bounds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two holes in the test tooling, both demonstrated before they were fixed. 1. `enclosingFunctionName` matched `\n\t…function NAME(` and nothing else, so a call inside `const f = async () => {}` was attributed to whichever classic `function` preceded it. Planting const renderRawBypass = async (raw: string) => { return (await invoke('render_markdown', { content: raw })) as string; }; in MarkdownViewer.svelte left all 524 tests green. It is now answered from the real AST via `svelte/compiler`, which the suite already depends on, so every declaration form in `src/` — 451 classic, 74 arrow, class methods, object shorthand — and every form nobody has written yet is covered by construction. Same for `$effect` bodies: the regex needed a tab-indented `});` to terminate, so a one-line effect merged into its successor and its ungated `editor` read was masked by the next effect's `editorReady`. 2. 71 `indexOf`-derived slice and ordering bounds had no `-1` guard, the defect #432 fixed in two places. `sliceFrom`/`sliceBetween` cover the slices; `offsetOf` is added for the ordering comparisons, where `a < b` is also satisfied by `a === -1`. 110 helper call sites replace 155 raw `indexOf` bounds; 194 anchors were each corrupted in turn and 193 produced a failure naming the missing anchor. Co-Authored-By: Claude Opus 5 --- scripts/checkedReadMigration.test.ts | 33 ++-- scripts/documentWatcherSession.test.ts | 4 +- scripts/editorOptionWiring.test.ts | 16 +- scripts/editorPdfExport.test.ts | 30 ++- scripts/externalChangeReload.test.ts | 14 +- scripts/foldKeys.test.ts | 4 +- scripts/foldStatePerDocument.test.ts | 8 +- scripts/issue261EditorPdf.test.ts | 7 +- scripts/issue281MinimalMacosMenu.test.ts | 12 +- scripts/liveModeWatchedPath.test.ts | 4 +- scripts/localFileLinks.test.ts | 26 +-- scripts/lossyDecodeSaveGuard.test.ts | 23 +-- scripts/macosPdfExport.test.ts | 8 +- scripts/monacoStartupGraph.test.ts | 19 +- scripts/printFindHighlight.test.ts | 11 +- scripts/printOversizedMedia.test.ts | 7 +- scripts/renderProtocol.test.ts | 6 +- scripts/reopenDirtyDocument.test.ts | 8 +- scripts/saveFromReadingMode.test.ts | 10 +- scripts/saveImageAsAssetUrl.test.ts | 14 +- scripts/scrollSyncInput.test.ts | 23 ++- scripts/settingsPersistence.test.ts | 15 +- scripts/sourceTree.test.ts | 122 +++++++++++++ scripts/sourceTree.ts | 223 +++++++++++++++++++++-- scripts/tabTransfer.test.ts | 10 +- scripts/tabTransferHandoff.test.ts | 19 +- scripts/truncatedBufferGuard.test.ts | 24 ++- scripts/untitledTitle.test.ts | 4 +- scripts/viewModeWithoutSaving.test.ts | 14 +- scripts/windowClosePerTab.test.ts | 26 ++- scripts/windowStateRestore.test.ts | 49 ++--- scripts/windowTags.test.ts | 10 +- scripts/youtubeExternalFallback.test.ts | 15 +- 33 files changed, 571 insertions(+), 247 deletions(-) create mode 100644 scripts/sourceTree.test.ts diff --git a/scripts/checkedReadMigration.test.ts b/scripts/checkedReadMigration.test.ts index 89910055..2e2c72ab 100644 --- a/scripts/checkedReadMigration.test.ts +++ b/scripts/checkedReadMigration.test.ts @@ -2,7 +2,7 @@ import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; import test from 'node:test'; -import { readSourceFiles, sliceBetween } from './sourceTree.js'; +import { offsetOf, readSourceFiles, sliceBetween } from './sourceTree.js'; // Runes and the Tauri bridge, shimmed the way truncatedBufferGuard.test.ts // shims them: the stores are runes modules, and Node's test runner gives every @@ -174,22 +174,19 @@ test('the unchecked read command is gone, and nothing calls it', () => { }); test('entering the editor reads the fidelity and stores it', () => { - const toggle = viewer.slice(viewer.indexOf('async function toggleEdit'), viewer.indexOf('async function saveContent')); + const toggle = sliceBetween(viewer, 'async function toggleEdit', 'async function saveContent'); assert.match(toggle, /\[content, lossy\] = \(await invoke\('read_file_content_checked', \{ path: tab\.path \}\)\)/); - const read = toggle.indexOf('read_file_content_checked'); - const flag = toggle.indexOf('setTabDecodedLossy(tab.id, lossy)'); - const store = toggle.indexOf('setTabRawContent(tab.id, content)'); - assert.notEqual(flag, -1, 'the verdict must reach the tab'); + const read = offsetOf(toggle, 'read_file_content_checked'); + const flag = offsetOf(toggle, 'setTabDecodedLossy(tab.id, lossy)'); + const store = offsetOf(toggle, 'setTabRawContent(tab.id, content)'); assert.ok(read < flag && flag < store, 'flag the tab before the buffer is published'); }); test('entering split view reads the fidelity and stores it', () => { - const split = viewer.slice(viewer.indexOf('async function toggleSplitView')); - const enter = split.slice(0, split.indexOf('} else {')); + const enter = sliceBetween(viewer, 'async function toggleSplitView', '} else {'); assert.match(enter, /\[content, lossy\] = \(await invoke\('read_file_content_checked', \{ path: tab\.path \}\)\)/); - const flag = enter.indexOf('setTabDecodedLossy(tab.id, lossy)'); - const store = enter.indexOf('setTabRawContent(tab.id, content)'); - assert.notEqual(flag, -1, 'the verdict must reach the tab'); + const flag = offsetOf(enter, 'setTabDecodedLossy(tab.id, lossy)'); + const store = offsetOf(enter, 'setTabRawContent(tab.id, content)'); assert.ok(flag < store, 'flag the tab before the buffer is published'); }); @@ -212,13 +209,10 @@ test('the session can tell a refusal from a failure', () => { }); test('a refused save does not add a generic toast to its own explanation', () => { - const effect = viewer.slice(viewer.indexOf('Auto-save effect.')); - const body = effect.slice(0, effect.indexOf('for (const id of [')); - const check = body.indexOf('if (documentSession.isLossySaveRefused(s.id)) return;'); - const toast = body.indexOf("t('toast.autoSaveFailed'"); - assert.notEqual(check, -1, 'the refusal must be recognised'); - assert.notEqual(toast, -1); - assert.ok(check < toast, 'and recognised before the generic toast is raised'); + const body = sliceBetween(viewer, 'Auto-save effect.', 'for (const id of ['); + const check = offsetOf(body, 'if (documentSession.isLossySaveRefused(s.id)) return;'); + const toast = offsetOf(body, "t('toast.autoSaveFailed'"); + assert.ok(check < toast, 'the refusal must be recognised before the generic toast is raised'); }); test('a tab that can only be refused stops re-arming the timer', () => { @@ -226,8 +220,7 @@ test('a tab that can only be refused stops re-arming the timer', () => { // again every 1.5s for as long as the user kept typing — each time // producing the console warning, the wasted round trip, and (before the // test above) the toast. - const effect = viewer.slice(viewer.indexOf('Auto-save effect.')); - const body = effect.slice(0, effect.indexOf('for (const id of [')); + const body = sliceBetween(viewer, 'Auto-save effect.', 'for (const id of ['); assert.match(body, /decodedLossily: tab\.hasReplacementChars/); assert.match(body, /const eligible = [^;]*!\(s\.decodedLossily && documentSession\.isLossySaveRefused\(s\.id\)\)/); // The FIRST attempt must still happen: it is what produces the explanation. diff --git a/scripts/documentWatcherSession.test.ts b/scripts/documentWatcherSession.test.ts index f7498e10..cdbd0fb5 100644 --- a/scripts/documentWatcherSession.test.ts +++ b/scripts/documentWatcherSession.test.ts @@ -2,10 +2,12 @@ import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; import test from 'node:test'; +import { sliceFrom } from './sourceTree.js'; + const session = readFileSync('src/lib/sessions/documentSession.svelte.ts', 'utf8'); test('self writes suppress watcher reloads only during their grace period', () => { - const handler = session.slice(session.indexOf('function shouldReloadExternalChange')); + const handler = sliceFrom(session, 'function shouldReloadExternalChange'); assert.match(handler, /if \(Date\.now\(\) < until\) return false;/); assert.match(handler, /selfWriteUntilByPath\.delete\(path\);/); assert.match(handler, /return true;/); diff --git a/scripts/editorOptionWiring.test.ts b/scripts/editorOptionWiring.test.ts index ba6c508a..2b9018fe 100644 --- a/scripts/editorOptionWiring.test.ts +++ b/scripts/editorOptionWiring.test.ts @@ -2,6 +2,8 @@ import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; import test from 'node:test'; +import { sliceBetween } from './sourceTree.js'; + // Editor.svelte translates the settings store into Monaco options and // keybindings. Every regression locked here came from that translation layer // being wired to the wrong shape: a string option read as a boolean, a @@ -15,14 +17,6 @@ function count(source: string, pattern: RegExp): number { return source.match(pattern)?.length ?? 0; } -function sliceBlock(source: string, startMarker: string, endMarker: string): string { - const start = source.indexOf(startMarker); - assert.notEqual(start, -1, `expected to find ${startMarker}`); - const end = source.indexOf(endMarker, start + startMarker.length); - assert.notEqual(end, -1, `expected to find ${endMarker} after ${startMarker}`); - return source.slice(start, end); -} - test('renderLineHighlight is a Monaco string enum, not a boolean flag', () => { // The store holds 'line' / 'none'. Any non-empty string is truthy, so a // ternary on it can only ever produce "line" and silently defeats both the @@ -54,7 +48,7 @@ test('editor options are applied by a single updateOptions effect', () => { // without the zoom factor — so the winner depended on effect ordering. assert.equal(count(editor, /editor\.updateOptions\(\{/g), 1, 'exactly one updateOptions call site'); - const block = sliceBlock(editor, 'editor.updateOptions({', '});'); + const block = sliceBetween(editor, 'editor.updateOptions({', '});'); assert.match(block, /wordWrapColumn: settings\.editorMaxWidth/, 'wordWrapColumn survived the merge'); assert.match(block, /fontSize: settings\.editorFontSize \* \(zoomLevel \/ 100\)/, 'zoom-aware font size is the surviving one'); for (const option of [ @@ -110,7 +104,7 @@ test('platform detection reads settings.osType and never writes it', () => { // the keybindings are registered once, at mount, rather than re-registered // when settings.osType resolves. Full argument: the comment on // isMacPlatform() in Editor.svelte. - const helper = sliceBlock(editor, 'function isMacPlatform', '\n\t}'); + const helper = sliceBetween(editor, 'function isMacPlatform', '\n\t}'); assert.match(helper, /settings\.osType !== 'unknown'/, 'prefers the resolved Tauri os type'); assert.match(helper, /settings\.osType === 'macos'/); assert.match(helper, /navigator\.platform/, 'falls back while osType is still resolving'); @@ -139,7 +133,7 @@ test('custom copy keeps Monaco\'s whole-line copy on an empty selection', () => // (editor.emptySelectionClipboard) and Sublime Text behave the same. Since // custom-copy overrides the native copy action, bailing out on an empty // selection deleted the behaviour outright. - const copyAction = sliceBlock(editor, 'id: "custom-copy"', 'id: "toggle-minimap"'); + const copyAction = sliceBetween(editor, 'id: "custom-copy"', 'id: "toggle-minimap"'); assert.doesNotMatch( copyAction, diff --git a/scripts/editorPdfExport.test.ts b/scripts/editorPdfExport.test.ts index 37d1d09a..c02f5c92 100644 --- a/scripts/editorPdfExport.test.ts +++ b/scripts/editorPdfExport.test.ts @@ -4,6 +4,8 @@ import test from 'node:test'; import { compile } from 'svelte/compiler'; +import { offsetOf, sliceBetween, sliceFrom } from './sourceTree.js'; + /* * Export PDF from plain edit mode produced a blank page, for two independent * reasons. This file covers both. @@ -317,9 +319,8 @@ test('the reveal does not depend on which sheet the browser applies last', () => // is not guaranteed. The resolver above already places the component last, // which is the losing arrangement for the print rules; assert that the // winning declaration is `!important` so order cannot decide it either way. - const printBlock = styles.slice(styles.indexOf('@media print')); - const override = printBlock.slice(printBlock.indexOf('#app .pane.viewer-pane')); - const rule = override.slice(0, override.indexOf('}')); + const printBlock = sliceFrom(styles, '@media print'); + const rule = sliceBetween(printBlock, '#app .pane.viewer-pane', '}'); for (const property of ['width', 'flex', 'opacity']) { assert.match(rule, new RegExp(`${property}:[^;]*!important`), `${property} must be !important`); } @@ -333,14 +334,6 @@ test('the reveal does not depend on which sheet the browser applies last', () => // the resulting DOM is complete — that is what the awaited `renderRichContent` // is for, and only a real browser can confirm it. -const slice = (source: string, start: string, end: string) => { - const from = source.indexOf(start); - assert.notEqual(from, -1, `expected to find ${start}`); - const to = source.indexOf(end, from + start.length); - assert.notEqual(to, -1, `expected to find ${end} after ${start}`); - return source.slice(from, to); -}; - test('the preview is only kept live while it is on screen', () => { // The premise of the second half. If this condition ever widens to cover // plain edit mode the export-time render below becomes a no-op rather than @@ -349,17 +342,16 @@ test('the preview is only kept live while it is on screen', () => { }); test('exporting a PDF renders the buffer before the DOM is printed', () => { - const body_ = slice(viewer, 'async function exportAsPdf', 'function handleNewFile'); - const sync = body_.indexOf('await syncPreviewForPrint()'); - assert.notEqual(sync, -1, 'the export must refresh the preview first'); + const body_ = sliceBetween(viewer, 'async function exportAsPdf', 'function handleNewFile'); + const sync = offsetOf(body_, 'await syncPreviewForPrint()'); // Before the diagram pass, which reads the nodes that render produces, and // before the print itself. - assert.ok(sync < body_.indexOf('renderDiagramsForPrint'), 'refresh before re-theming the diagrams'); - assert.ok(sync < body_.indexOf('_exportPdf('), 'refresh before printing'); + assert.ok(sync < offsetOf(body_, 'renderDiagramsForPrint'), 'refresh before re-theming the diagrams'); + assert.ok(sync < offsetOf(body_, '_exportPdf('), 'refresh before printing'); }); test('the refresh is skipped when the DOM already matches the buffer', () => { - const body_ = slice(viewer, 'async function syncPreviewForPrint', 'async function exportAsPdf'); + const body_ = sliceBetween(viewer, 'async function syncPreviewForPrint', 'async function exportAsPdf'); // Reading mode is rendered by loadMarkdown from this same buffer, and // re-rendering it would discard the scroll, fold and find state on screen. assert.match(body_, /if \(!tab \|\| !\(tab\.isEditing \|\| tab\.isSplit\)\) return;/); @@ -367,7 +359,7 @@ test('the refresh is skipped when the DOM already matches the buffer', () => { }); test('the refresh actually lands before the print', () => { - const body_ = slice(viewer, 'async function syncPreviewForPrint', 'async function exportAsPdf'); + const body_ = sliceBetween(viewer, 'async function syncPreviewForPrint', 'async function exportAsPdf'); // What matters here is WHAT is rendered — this tab's buffer and this tab's // path, not the disk — and that it is awaited before the content is // swapped in. The trailing arguments are deliberately not pinned: fold @@ -382,7 +374,7 @@ test('the refresh actually lands before the print', () => { }); test('a failed refresh does not pass off a stale export as a fresh one', () => { - const body_ = slice(viewer, 'async function syncPreviewForPrint', 'async function exportAsPdf'); + const body_ = sliceBetween(viewer, 'async function syncPreviewForPrint', 'async function exportAsPdf'); assert.match(body_, /catch \(error\)/); assert.match(body_, /addToast\(/); }); diff --git a/scripts/externalChangeReload.test.ts b/scripts/externalChangeReload.test.ts index d92ee5ef..c7548be4 100644 --- a/scripts/externalChangeReload.test.ts +++ b/scripts/externalChangeReload.test.ts @@ -2,6 +2,8 @@ import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; import test from 'node:test'; +import { sliceBetween } from './sourceTree.js'; + // Live Mode watches the open file and reloads it when something else writes // it — git checkout, a cloud sync, a second Markpad window. The reload path // replaces rawContent AND originalContent, so a buffer with unsaved edits is @@ -135,8 +137,7 @@ test('our own writes still suppress the reload', async () => { // --- wiring that cannot be executed outside a Svelte runtime --- test('the watcher listener routes every event through the guarded resolution', () => { - const listener = viewer.slice(viewer.indexOf("listen('file-changed'")); - const body = listener.slice(0, listener.indexOf('}),')); + const body = sliceBetween(viewer, "listen('file-changed'", '}),'); assert.match(body, /resolveExternalChange/); // The old body called loadMarkdown(currentFile) directly. assert.doesNotMatch(body, /loadMarkdown\(currentFile\)/); @@ -152,8 +153,7 @@ test('the debounced auto-save is held back while a conflict is unanswered', () = // Otherwise the 1.5s timer fires while the bar is still asking "reload or // keep mine": the user's edits survive, but the external change is gone // from disk before either answer can be given. - const effect = viewer.slice(viewer.indexOf('Auto-save effect.')); - const body = effect.slice(0, effect.indexOf('for (const id of [')); + const body = sliceBetween(viewer, 'Auto-save effect.', 'for (const id of ['); assert.match(body, /hasPendingConflict: externalChangeConflicts\[tab\.id\] === true/); assert.match(body, /const eligible = [^;]*!s\.hasPendingConflict/); }); @@ -161,15 +161,13 @@ test('the debounced auto-save is held back while a conflict is unanswered', () = test('an explicit save answers the conflict and takes the bar down', () => { // Pressing Cmd+S is a decision. Leaving the bar up afterwards would ask a // question the user already answered. - const wrapper = viewer.slice(viewer.indexOf('async function saveContent(tabId?: string)')); - const body = wrapper.slice(0, wrapper.indexOf('async function saveContentAs')); + const body = sliceBetween(viewer, 'async function saveContent(tabId?: string)', 'async function saveContentAs'); assert.match(body, /clearExternalChangeConflict/); }); test('turning Live Mode on installs the watcher without reloading', () => { // Enabling a watcher is not a request to discard the buffer, and this was // the one loadMarkdown call in the app with no canCloseTab in front of it. - const toggle = viewer.slice(viewer.indexOf('function toggleLiveMode')); - const body = toggle.slice(0, toggle.indexOf('\n\t}') + 3); + const body = sliceBetween(viewer, 'function toggleLiveMode', '\n\t}'); assert.doesNotMatch(body, /loadMarkdown/); }); diff --git a/scripts/foldKeys.test.ts b/scripts/foldKeys.test.ts index 54df67fb..20c64c18 100644 --- a/scripts/foldKeys.test.ts +++ b/scripts/foldKeys.test.ts @@ -2,6 +2,8 @@ import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; import test from 'node:test'; +import { sliceFrom } from './sourceTree.js'; + // Fold state is keyed by `h.id || textContent`. comrak emits the // deduplicated heading id on an empty inner , not on the // heading element, so without promotion every fold consumer falls back to @@ -11,7 +13,7 @@ import test from 'node:test'; test('processMarkdownHtml promotes the anchor id onto the heading element', () => { const source = readFileSync('src/lib/utils/markdown.ts', 'utf8'); - const headingLoop = source.slice(source.indexOf('querySelectorAll("h1, h2, h3, h4, h5, h6")')); + const headingLoop = sliceFrom(source, 'querySelectorAll("h1, h2, h3, h4, h5, h6")'); assert.match(headingLoop, /querySelector\("a\.anchor"\)/, 'heading loop looks up the comrak anchor'); assert.match(headingLoop, /h\.id = \w+\.id/, 'anchor id is promoted onto the heading'); assert.match(headingLoop, /removeAttribute\("id"\)/, 'anchor id is removed so document ids stay unique'); diff --git a/scripts/foldStatePerDocument.test.ts b/scripts/foldStatePerDocument.test.ts index 094012b0..d07cb451 100644 --- a/scripts/foldStatePerDocument.test.ts +++ b/scripts/foldStatePerDocument.test.ts @@ -27,6 +27,7 @@ import test from 'node:test'; import ts from 'typescript'; import { installShimDom, parseHtml, type ShimElement } from './renderProtocolDom.ts'; +import { offsetOf } from './sourceTree.js'; // ---------------------------------------------------------------- environment @@ -98,7 +99,7 @@ function pluckFunction(source: string, name: string, required = true): string { for (const marker of [`async function ${name}(`, `function ${name}(`]) { const start = source.indexOf(marker); if (start === -1) continue; - const open = source.indexOf('{', source.indexOf(')', start)); + const open = offsetOf(source, '{', offsetOf(source, ')', start)); return source.slice(start, matchBrace(source, open) + 1); } assert.ok(!required, `expected the component to define ${name}`); @@ -243,9 +244,8 @@ function buildViewer(): Viewer { /** The real `visibleItems` out of Toc.svelte, over a caller-supplied fold set. */ function buildTocFilter() { const marker = 'let visibleItems = $derived.by(() => '; - const start = toc.indexOf(marker); - assert.notEqual(start, -1, 'expected Toc.svelte to derive visibleItems'); - const open = toc.indexOf('{', start + marker.length); + const start = offsetOf(toc, marker); + const open = offsetOf(toc, '{', start + marker.length); const body = toc.slice(open, matchBrace(toc, open) + 1); const js = ts.transpileModule(body, { compilerOptions: { target: ts.ScriptTarget.ES2022 } }).outputText; return new Function('items', 'collapsedHeaders', `"use strict";\n${js}`) as ( diff --git a/scripts/issue261EditorPdf.test.ts b/scripts/issue261EditorPdf.test.ts index 13b113cd..b4eb1ea2 100644 --- a/scripts/issue261EditorPdf.test.ts +++ b/scripts/issue261EditorPdf.test.ts @@ -2,17 +2,16 @@ import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; import test from 'node:test'; -import { sliceBetween, sliceFrom } from './sourceTree.js'; +import { offsetOf, sliceBetween, sliceFrom } from './sourceTree.js'; const viewer = readFileSync('src/lib/MarkdownViewer.svelte', 'utf8'); const styles = readFileSync('src/styles.css', 'utf8'); test('editor context menu is not intercepted by the document menu', () => { const handler = sliceBetween(viewer, 'function handleContextMenu(e: MouseEvent)', '\n\tfunction handleMouseOver'); - const editorReturn = handler.indexOf('if (isInsideEditor) return;'); - const preventDefault = handler.indexOf('e.preventDefault();'); + const editorReturn = offsetOf(handler, 'if (isInsideEditor) return;'); + const preventDefault = offsetOf(handler, 'e.preventDefault();'); - assert.ok(editorReturn !== -1, 'editor context menus must stay with Monaco'); assert.ok(editorReturn < preventDefault, 'Monaco must receive the event before the document menu prevents it'); }); diff --git a/scripts/issue281MinimalMacosMenu.test.ts b/scripts/issue281MinimalMacosMenu.test.ts index 0324fcf3..41e5be61 100644 --- a/scripts/issue281MinimalMacosMenu.test.ts +++ b/scripts/issue281MinimalMacosMenu.test.ts @@ -2,15 +2,17 @@ import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; import test from 'node:test'; +import { sliceBetween } from './sourceTree.js'; + const tauriLib = readFileSync('src-tauri/src/lib.rs', 'utf8'); const viewer = readFileSync('src/lib/MarkdownViewer.svelte', 'utf8'); test('macOS native menu keeps only application-level actions', () => { - const menuStart = tauriLib.indexOf('#[cfg(target_os = "macos")]\n {\n use tauri::menu'); - assert.notEqual(menuStart, -1, 'macOS native menu setup must exist'); - - const menuEnd = tauriLib.indexOf('\n let config_dir', menuStart); - const menuSetup = tauriLib.slice(menuStart, menuEnd); + const menuSetup = sliceBetween( + tauriLib, + '#[cfg(target_os = "macos")]\n {\n use tauri::menu', + '\n let config_dir', + ); assert.match(menuSetup, /MenuItemBuilder::with_id\("menu-app-settings", "Settings…"\)\s*\.accelerator\("CmdOrCtrl\+,"\)/); assert.match(menuSetup, /MenuItemBuilder::with_id\("check-updates", "Check for Updates…"\)/); diff --git a/scripts/liveModeWatchedPath.test.ts b/scripts/liveModeWatchedPath.test.ts index 68eac96b..edd23e43 100644 --- a/scripts/liveModeWatchedPath.test.ts +++ b/scripts/liveModeWatchedPath.test.ts @@ -2,6 +2,8 @@ import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; import test from 'node:test'; +import { sliceBetween } from './sourceTree.js'; + const runtime = readFileSync('src-tauri/src/window_runtime.rs', 'utf8'); const viewer = readFileSync('src/lib/MarkdownViewer.svelte', 'utf8'); @@ -22,6 +24,6 @@ test('Live Mode routes a watcher notification to its watched path', () => { test('Live Mode follows the active file instead of retaining a previous tab watcher', () => { assert.match(viewer, /if \(liveMode && currentFile\) \{\n\t\t\tinvoke\('watch_file', \{ path: currentFile \}\)/); assert.doesNotMatch(readFileSync('src/lib/sessions/documentSession.svelte.ts', 'utf8'), /isLiveMode\(\)\) invoke\('watch_file'/); - const toggleLiveMode = viewer.slice(viewer.indexOf('function toggleLiveMode'), viewer.indexOf('async function saveImageAs')); + const toggleLiveMode = sliceBetween(viewer, 'function toggleLiveMode', 'async function saveImageAs'); assert.doesNotMatch(toggleLiveMode, /loadMarkdown\(/); }); diff --git a/scripts/localFileLinks.test.ts b/scripts/localFileLinks.test.ts index 8afd677b..b95e9d97 100644 --- a/scripts/localFileLinks.test.ts +++ b/scripts/localFileLinks.test.ts @@ -3,6 +3,7 @@ import { readFileSync } from 'node:fs'; import test from 'node:test'; import { resolveLocalFileLinkPath } from '../src/lib/utils/localFileLinks.js'; +import { offsetOf, sliceBetween } from './sourceTree.js'; /* * `[data](./data.csv)` did nothing on macOS and Linux, and opened a dead page @@ -90,19 +91,11 @@ test('a markdown link still resolves to a path, so branch order is what keeps it // --- wiring ------------------------------------------------------------------ const viewer = readFileSync('src/lib/MarkdownViewer.svelte', 'utf8'); -const handler = (() => { - const from = viewer.indexOf('async function handleDocumentClick'); - assert.notEqual(from, -1); - const to = viewer.indexOf('let zoomLevel', from); - assert.notEqual(to, -1); - return viewer.slice(from, to); -})(); +const handler = sliceBetween(viewer, 'async function handleDocumentClick', 'let zoomLevel'); test('markdown targets are still claimed before the local-file branch', () => { - const markdown = handler.indexOf('getRelativeMarkdownTarget(rawHref)'); - const local = handler.indexOf('resolveLocalFileLinkPath(rawHref, currentFile)'); - assert.notEqual(markdown, -1); - assert.notEqual(local, -1); + const markdown = offsetOf(handler, 'getRelativeMarkdownTarget(rawHref)'); + const local = offsetOf(handler, 'resolveLocalFileLinkPath(rawHref, currentFile)'); assert.ok(markdown < local, '`./other.md` must open in a tab, not in an external editor'); }); @@ -111,9 +104,8 @@ test('a local file is handed to the OS as a path, not as a URL', () => { // The raw attribute, not `anchor.href`: the latter is the origin-resolved // URL that caused the bug. assert.match(handler, /resolveLocalFileLinkPath\(rawHref, currentFile\)/); - const local = handler.indexOf('resolveLocalFileLinkPath'); - const url = handler.indexOf('await openUrl(anchor.href)'); - assert.notEqual(url, -1, 'genuine web links must still go to the browser'); + const local = offsetOf(handler, 'resolveLocalFileLinkPath'); + const url = offsetOf(handler, 'await openUrl(anchor.href)'); assert.ok(local < url, 'a local file must be caught before the URL fallback'); }); @@ -135,13 +127,11 @@ test('neither OS call can leave an unhandled rejection behind', () => { // try/catch, that rejection was the whole visible symptom on macOS: nothing // happened, and nothing said why. for (const call of ['await openPath(localFilePath)', 'await openUrl(anchor.href)']) { - const at = handler.indexOf(call); - assert.notEqual(at, -1, call); + const at = offsetOf(handler, call); const before = handler.slice(0, at); const tryAt = before.lastIndexOf('try {'); - const catchAfter = handler.indexOf('} catch (error) {', at); assert.notEqual(tryAt, -1, `${call} must be inside a try block`); - assert.notEqual(catchAfter, -1, `${call} must have a catch`); + offsetOf(handler, '} catch (error) {', at); // must have a catch after it assert.ok(before.slice(tryAt).split('} catch').length === 1, `${call} must be inside the nearest try`); } assert.equal(handler.match(/addToast\(`Failed to open/g)?.length, 2, 'both failures are reported'); diff --git a/scripts/lossyDecodeSaveGuard.test.ts b/scripts/lossyDecodeSaveGuard.test.ts index 39132132..d3761d71 100644 --- a/scripts/lossyDecodeSaveGuard.test.ts +++ b/scripts/lossyDecodeSaveGuard.test.ts @@ -4,7 +4,7 @@ import test from 'node:test'; import type { Tab } from '../src/lib/stores/tabs.svelte.js'; import { buildTransferredTab, snapshotTab, validateTransferPayload } from '../src/lib/utils/tabTransfer.js'; -import { sliceBetween } from './sourceTree.js'; +import { offsetOf, sliceBetween } from './sourceTree.js'; // Every read path decodes leniently (#371): a file in a legacy encoding // (GBK, Big5, Shift-JIS, EUC-KR, CP1251 ...) opens as U+FFFD mojibake instead @@ -51,9 +51,8 @@ test('a preview cut inside a multi-byte character is not called lossy', () => { // is routinely cut mid-character. Reporting that as lossy would lock every // large CJK/emoji document out of saving — a guard worse than the bug. const preview = sliceBetween(rust, 'fn build_markdown_preview', '#[tauri::command]'); - const trim = preview.indexOf('utf8_truncation_boundary'); - assert.notEqual(trim, -1, 'the split tail must be dropped before decoding'); - assert.ok(trim < preview.indexOf('decode_utf8_lossy'), 'trim first, then judge fidelity'); + const trim = offsetOf(preview, 'utf8_truncation_boundary'); + assert.ok(trim < offsetOf(preview, 'decode_utf8_lossy'), 'the split tail is dropped before fidelity is judged'); }); test('the tab carries the fidelity of its buffer', () => { @@ -105,10 +104,8 @@ test('the editable-pane shortcut reports fidelity too', () => { test('the flag is set before the buffer can reach a writer', () => { const body = loadMarkdown(); - const set = body.indexOf('setTabDecodedLossy(activeId, lossy)'); - const firstAwait = body.indexOf('options.renderMarkdown(content'); - assert.notEqual(set, -1, 'the preview branch must set the flag'); - assert.notEqual(firstAwait, -1); + const set = offsetOf(body, 'setTabDecodedLossy(activeId, lossy)'); + const firstAwait = offsetOf(body, 'options.renderMarkdown(content'); assert.ok(set < firstAwait, 'the flag must be set before the first await that follows the load'); }); @@ -123,11 +120,10 @@ test('window restore cannot launder the flag away', () => { test('saveContent refuses to write a lossy buffer over its own file', () => { const body = saveContent(); - const refusal = body.indexOf('refuseIfLossilyDecoded'); - assert.notEqual(refusal, -1, 'saveContent must consult the guard'); + const refusal = offsetOf(body, 'refuseIfLossilyDecoded'); assert.match(body.slice(refusal, refusal + 120), /return false;/); assert.ok( - refusal < body.indexOf("invoke('save_file_content'"), + refusal < offsetOf(body, "invoke('save_file_content'"), 'the guard must run before save_file_content is invoked', ); }); @@ -165,10 +161,9 @@ test('Save As onto the same file is still a destructive overwrite', () => { // same bytes. The guard is therefore handed the target's resolved identity // as well as its path. See pathIdentityCaseFolding.test.ts, which drives // that refusal for real instead of reading for it. - const refusal = body.indexOf('refuseIfLossilyDecoded(tab, selected'); - assert.notEqual(refusal, -1, 'picking the source file again must be refused'); + const refusal = offsetOf(body, 'refuseIfLossilyDecoded(tab, selected'); assert.ok( - refusal < body.indexOf("invoke('save_file_content'"), + refusal < offsetOf(body, "invoke('save_file_content'"), 'the guard must run before save_file_content is invoked', ); }); diff --git a/scripts/macosPdfExport.test.ts b/scripts/macosPdfExport.test.ts index 8d33f08b..a3a475e7 100644 --- a/scripts/macosPdfExport.test.ts +++ b/scripts/macosPdfExport.test.ts @@ -2,6 +2,8 @@ import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; import test from 'node:test'; +import { sliceBetween } from './sourceTree.js'; + test('all Markpad webviews may invoke Tauri native printing', () => { const capability = JSON.parse(readFileSync('src-tauri/capabilities/default.json', 'utf8')) as { windows: string[]; @@ -26,12 +28,8 @@ test('both PDF commands the exporter invokes are registered with Tauri', () => { const exporter = readFileSync('src/lib/utils/export.ts', 'utf8'); const tauriLib = readFileSync('src-tauri/src/lib.rs', 'utf8'); - const handlerStart = tauriLib.indexOf('tauri::generate_handler!['); - assert.notEqual(handlerStart, -1, 'lib.rs must keep a generate_handler! list'); - const handlerEnd = tauriLib.indexOf(']', handlerStart); const registered = new Set( - tauriLib - .slice(handlerStart, handlerEnd) + sliceBetween(tauriLib, 'tauri::generate_handler![', ']') .split(',') .map((entry) => entry.trim().replace(/^.*::/, '')), ); diff --git a/scripts/monacoStartupGraph.test.ts b/scripts/monacoStartupGraph.test.ts index 37330e2e..5b1dfe39 100644 --- a/scripts/monacoStartupGraph.test.ts +++ b/scripts/monacoStartupGraph.test.ts @@ -3,6 +3,8 @@ import { existsSync, readFileSync, readdirSync } from 'node:fs'; import { dirname, relative, resolve } from 'node:path'; import test from 'node:test'; +import { callbackBodies } from './sourceTree.js'; + // Monaco is ~86% of Markpad's startup JavaScript (measured: a 4.4 MB chunk out // of 4.7 MB on first paint, ~360ms of parse+eval, paid once per window because // every window is its own webview) and a reader who only ever views Markdown @@ -189,7 +191,22 @@ test('every effect that drives the editor waits for editorReady', () => { // alone runs once against nothing and is never re-triggered, which silently // drops scroll sync, the zoom-aware font size, the theme and Vim mode. const editor = readFileSync('src/lib/components/Editor.svelte', 'utf8'); - const effects = [...editor.matchAll(/\$effect\(\(\) => \{([\s\S]*?)\n\t\}\);/g)].map((m) => m[1]); + + // The bodies come from the parsed component, not from + // `/\$effect\(\(\) => \{([\s\S]*?)\n\t\}\);/`. That pattern needed a + // tab-indented `});` to end a body, so an effect written on one line had no + // terminator of its own and the lazy match ran on to the *next* effect's — + // returning the two as a single body whose text contains the `editorReady` + // the first half was missing. Measured, by planting + // `$effect(() => { if (editor) editor.layout(); });` in Editor.svelte: the + // ungated read passed, and `effects.length >= 5` did not notice because the + // merge kept the count at six. scripts/sourceTree.test.ts pins that form. + const effects = callbackBodies(editor, '$effect'); + + // Vacuity guard, restated so it cannot be satisfied by a body going missing: + // the parse must account for every `$effect` the file spells. + const spelled = (editor.match(/\$effect\s*\(/g) ?? []).length; + assert.equal(effects.length, spelled, `every $effect(…) yielded a body (spelled ${spelled})`); assert.ok(effects.length >= 5, `found the effects (got ${effects.length})`); for (const body of effects) { diff --git a/scripts/printFindHighlight.test.ts b/scripts/printFindHighlight.test.ts index 58b0c945..05debb57 100644 --- a/scripts/printFindHighlight.test.ts +++ b/scripts/printFindHighlight.test.ts @@ -2,15 +2,16 @@ import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; import test from 'node:test'; +import { offsetOf, sliceBetween, sliceFrom } from './sourceTree.js'; + const styles = readFileSync('src/styles.css', 'utf8'); const findBar = readFileSync('src/lib/components/FindBar.svelte', 'utf8'); /** The `@media print` block only — not everything that follows it in the file. */ function extractPrintBlock(source: string): string { - const start = source.indexOf('@media print {'); - assert.notEqual(start, -1, 'src/styles.css must keep an @media print block'); + const start = offsetOf(source, '@media print {'); let depth = 0; - for (let i = source.indexOf('{', start); i < source.length; i += 1) { + for (let i = offsetOf(source, '{', start); i < source.length; i += 1) { if (source[i] === '{') depth += 1; else if (source[i] === '}') { depth -= 1; @@ -70,9 +71,9 @@ test('the neutralising rule outranks the component styles it overrides', () => { // specificity, and stylesheet order between a component style and // styles.css is not guaranteed, so `!important` is what decides. const body = printRuleBody('.markdown-body mark.markpad-find-match'); - const findBarRule = findBar.slice(findBar.indexOf(':global(.markdown-body mark.markpad-find-match)')); + const findBarRule = sliceBetween(findBar, ':global(.markdown-body mark.markpad-find-match)', ''); - assert.doesNotMatch(findBarRule.slice(0, findBarRule.indexOf('')), /!important/); + assert.doesNotMatch(findBarRule, /!important/); for (const property of ['background-color', 'box-shadow']) { assert.match(body, new RegExp(`(? { diff --git a/scripts/reopenDirtyDocument.test.ts b/scripts/reopenDirtyDocument.test.ts index 0088b45e..8c57374f 100644 --- a/scripts/reopenDirtyDocument.test.ts +++ b/scripts/reopenDirtyDocument.test.ts @@ -2,6 +2,8 @@ import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; import test from 'node:test'; +import { sliceBetween } from './sourceTree.js'; + // Opening a file that is ALREADY open in a tab with unsaved edits must not // re-read it from disk. `setTabRawContent` replaces rawContent AND // originalContent, so the edits would not merely be overwritten — the tab @@ -144,8 +146,7 @@ test('following a link into a new tab shows the tab that already holds the file' }); test('the call shape this guards is still the one MarkdownViewer uses', () => { - const body = viewer.slice(viewer.indexOf('async function openMarkdownTargetInNewTab')); - const fn = body.slice(0, body.indexOf('\n\tasync function', 1)); + const fn = sliceBetween(viewer, 'async function openMarkdownTargetInNewTab', '\n\tasync function'); assert.match(fn, /tabManager\.addTab\(/); assert.match(fn, /loadMarkdown\([^)]*\{[^}]*skipTabManagement: true/); }); @@ -226,8 +227,7 @@ test('an edited tab following a link to a DIFFERENT file still loads it', async // passing this flag is a caller claiming the user chose to lose work. for (const name of ['resolveExternalChangeByReloading', 'reloadFromDisk']) { test(`${name} declares that it is discarding the buffer`, () => { - const body = viewer.slice(viewer.indexOf(`async function ${name}`)); - const fn = body.slice(0, body.indexOf('\n\t}') + 3); + const fn = sliceBetween(viewer, `async function ${name}`, '\n\t}'); assert.match(fn, /loadMarkdown\(/); assert.match(fn, /discardUnsavedBuffer: true/); }); diff --git a/scripts/saveFromReadingMode.test.ts b/scripts/saveFromReadingMode.test.ts index 06c79b73..1cf15149 100644 --- a/scripts/saveFromReadingMode.test.ts +++ b/scripts/saveFromReadingMode.test.ts @@ -2,14 +2,12 @@ import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; import test from 'node:test'; +import { offsetOf, sliceBetween } from './sourceTree.js'; + const viewer = readFileSync(new URL('../src/lib/MarkdownViewer.svelte', import.meta.url), 'utf8'); function keydownSaveBranch(): string { - const start = viewer.indexOf("if (cmdOrCtrl && key === 's') {"); - assert.notEqual(start, -1, 'the Ctrl/Cmd+S keydown branch should exist'); - const end = viewer.indexOf("if (cmdOrCtrl && e.shiftKey && key === 't')", start); - assert.notEqual(end, -1, 'expected a following branch to bound the slice'); - return viewer.slice(start, end); + return sliceBetween(viewer, "if (cmdOrCtrl && key === 's') {", "if (cmdOrCtrl && e.shiftKey && key === 't')"); } // Reported in #168 by @dayeggpi: with a document that was never saved, @@ -42,7 +40,7 @@ test('the browser save dialog is always suppressed', () => { // preventDefault has to run for every mode, including the no-op case, // or reading mode would surface the webview's own Save Page dialog. const branch = keydownSaveBranch(); - const beforeGuard = branch.slice(0, branch.indexOf('const saveTarget')); + const beforeGuard = branch.slice(0, offsetOf(branch, 'const saveTarget')); assert.match(beforeGuard, /e\.preventDefault\(\);/); }); diff --git a/scripts/saveImageAsAssetUrl.test.ts b/scripts/saveImageAsAssetUrl.test.ts index feb9834d..ec83dc2c 100644 --- a/scripts/saveImageAsAssetUrl.test.ts +++ b/scripts/saveImageAsAssetUrl.test.ts @@ -3,6 +3,7 @@ import { readFileSync } from 'node:fs'; import test from 'node:test'; import { normalizeAssetPath } from '../src/lib/utils/exportHtml.js'; +import { offsetOf, sliceBetween } from './sourceTree.js'; /* * "Save image as" could never save a remote image, and on Windows it could not @@ -28,13 +29,7 @@ import { normalizeAssetPath } from '../src/lib/utils/exportHtml.js'; */ const viewer = readFileSync('src/lib/MarkdownViewer.svelte', 'utf8'); -const body = (() => { - const from = viewer.indexOf('async function saveImageAs'); - assert.notEqual(from, -1); - const to = viewer.indexOf('async function saveDiagramAs', from); - assert.notEqual(to, -1); - return viewer.slice(from, to); -})(); +const body = sliceBetween(viewer, 'async function saveImageAs', 'async function saveDiagramAs'); test('a Windows asset URL names the same file as the asset: form', () => { // The property the old `startsWith('asset:')` test lacked. Running it here @@ -80,9 +75,8 @@ test('a local image is copied on the Rust side', () => { assert.match(body, /invoke\('copy_file', \{ src: realPath, dest \}\)/); // And the save dialog is only offered for a file that can actually be // written; the remote case bails out before it. - const bail = body.indexOf("addToast('Saving a remote image is not supported yet'"); - const dialog = body.indexOf('await save('); - assert.notEqual(bail, -1, 'the unsupported case must say so'); + const bail = offsetOf(body, "addToast('Saving a remote image is not supported yet'"); + const dialog = offsetOf(body, 'await save('); assert.ok(bail < dialog, 'do not ask for a destination that cannot be written'); }); diff --git a/scripts/scrollSyncInput.test.ts b/scripts/scrollSyncInput.test.ts index f908d364..99168379 100644 --- a/scripts/scrollSyncInput.test.ts +++ b/scripts/scrollSyncInput.test.ts @@ -2,13 +2,24 @@ import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; import test from 'node:test'; +import { callbackBodies } from './sourceTree.js'; + +// The effect is identified by what it does — it is the one that reads +// `onscrollsync` — not by where its text starts and stops. +// +// It used to be sliced between `'&& onscrollsync)'` and `'\n\t$effect(() => {'`, +// and both halves of that were the same kind of fragile. The start anchor had +// already drifted once, silently: the original marker spelled the whole guard +// condition, the effect gained an `editorReady &&` term when Monaco moved behind +// a dynamic import, and the slice became empty rather than failing — which is +// what the shortened anchor and its `notEqual(-1)` were added for. The end +// anchor never got that guard, and it depends on the *next* effect being +// tab-indented and spelled `$effect(() => {`; write it any other way and this +// file silently starts asserting about the rest of the component. const editor = readFileSync('src/lib/components/Editor.svelte', 'utf8'); -// Anchor on the tail of the guard, not the whole condition: the effect gained an -// `editorReady &&` term when Monaco moved behind a dynamic import, and an -// exact-match marker silently sliced an empty string instead of failing loudly. -const syncStart = editor.indexOf('&& onscrollsync)'); -assert.notEqual(syncStart, -1, 'expected to find the scroll-sync effect guard'); -const syncEffect = editor.slice(syncStart, editor.indexOf('\n\t$effect(() => {', syncStart + 1)); +const syncEffects = callbackBodies(editor, '$effect').filter((body) => body.includes('onscrollsync')); +assert.equal(syncEffects.length, 1, `expected exactly one $effect reading onscrollsync (got ${syncEffects.length})`); +const syncEffect = syncEffects[0]; test('typing does not initiate split scroll synchronization', () => { assert.match(syncEffect, /editor\.onDidScrollChange/); diff --git a/scripts/settingsPersistence.test.ts b/scripts/settingsPersistence.test.ts index 35213d3e..50b67d75 100644 --- a/scripts/settingsPersistence.test.ts +++ b/scripts/settingsPersistence.test.ts @@ -2,6 +2,8 @@ import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; import test from 'node:test'; +import { callbackBodies, sliceBetween } from './sourceTree.js'; + /* * `settings.svelte.ts` is a runes module, so it cannot be imported the way the * other suites import plain utils. Node's test runner gives every file its own @@ -399,8 +401,7 @@ test('spin buttons clamp with the same rules as typing', () => { test('numeric setting inputs are one-way bound and commit through the clamp', () => { const numericInputIds = ['editor-font-size', 'editor-max-width', 'preview-font-size', 'code-font-size']; for (const id of numericInputIds) { - const block = componentSource.slice(componentSource.indexOf(`id="${id}"`)); - const input = block.slice(0, block.indexOf('/>')); + const input = sliceBetween(componentSource, `id="${id}"`, '/>'); assert.doesNotMatch(input, /bind:value/, `${id} still uses a two-way number binding`); assert.match(input, /oninput=\{\(e\) => handleNumberInput\(/, `${id} does not validate input`); assert.match(input, /onchange=\{\(e\) => commitNumberInput\(/, `${id} does not clamp on change`); @@ -481,9 +482,13 @@ test('a stored language is validated against the catalogue', () => { */ test('the open effect neither reads the app version nor re-enters itself', () => { - const effectStart = componentSource.indexOf('$effect(() => {\n\t\tif (show) {'); - assert.ok(effectStart > 0, 'settings open effect not found'); - const effectBody = componentSource.slice(effectStart, componentSource.indexOf('\n\t});', effectStart)); + // The open effect is identified by what it reads, not by its indentation and + // closing brace: the previous anchors were `'$effect(() => {\n\t\tif (show) {'` + // and `'\n\t});'`, so reformatting the component silently moved this + // assertion onto a different span of source. + const openEffects = callbackBodies(componentSource, '$effect').filter((body) => /\bshow\b/.test(body)); + assert.equal(openEffects.length, 1, `expected exactly one $effect gated on show (got ${openEffects.length})`); + const effectBody = openEffects[0]; // Reading a state it also writes is what made the effect re-run and // re-capture the focus target from inside the dialog. diff --git a/scripts/sourceTree.test.ts b/scripts/sourceTree.test.ts new file mode 100644 index 00000000..81503683 --- /dev/null +++ b/scripts/sourceTree.test.ts @@ -0,0 +1,122 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { callbackBodies, enclosingFunctionName, offsetOf, sliceBetween, sliceFrom } from './sourceTree.js'; + +// scripts/sourceTree.ts is the plumbing four convention tests state their claims +// through, so a hole in it is a hole in all of them at once, and it is invisible +// from those files: they keep passing. Both holes below were real. +// +// - `enclosingFunctionName` matched `\n\t…function NAME(` and nothing else. A +// raw `invoke('render_markdown')` inside `const f = async () => {}` was +// therefore attributed to the classic `function` above it, which in +// MarkdownViewer.svelte is the wrapper renderPipelineConvention.test.ts +// demands. Measured: the bypass planted, the whole suite green. +// - The `$effect` bodies monacoStartupGraph.test.ts checks were sliced with +// `/\$effect\(\(\) => \{([\s\S]*?)\n\t\}\);/`, which needs a tab-indented +// closing brace. Measured: a one-line effect merged into its successor and +// the ungated `editor` read inside it went unreported. +// +// So the forms are pinned here, on the helpers, where a regression is one +// failure with a name rather than four tests quietly asserting nothing. These +// run against string literals, not against `src/` — refactoring the app cannot +// make them fail, and changing the helper is the only thing that can. + +/** `enclosingFunctionName` over one `\n`; + return enclosingFunctionName(source, offsetOf(source, needle)); +} + +test('every declaration form in src/ names its enclosing function', () => { + // The four shapes `src/` actually uses: 451 classic declarations, 74 + // `const f = (…) =>` / `const f = async (…) =>`, the class methods in + // stores/, and object-literal shorthand (MarkdownViewer's `acceptNode`). + assert.equal(scopeOf('function classic() {\n\tHERE;\n}'), 'classic'); + assert.equal(scopeOf('export async function exported() {\n\tHERE;\n}'), 'exported'); + assert.equal(scopeOf('const arrow = () => {\n\tHERE;\n};'), 'arrow'); + assert.equal(scopeOf('const asyncArrow = async (raw: string) => {\n\tHERE;\n};'), 'asyncArrow'); + assert.equal(scopeOf('const typed: () => void = () => {\n\tHERE;\n};'), 'typed'); + assert.equal(scopeOf('const expression = function () {\n\tHERE;\n};'), 'expression'); + assert.equal(scopeOf('const named = function inner() {\n\tHERE;\n};'), 'inner'); + assert.equal(scopeOf('const obj = {\n\tshorthand() {\n\t\tHERE;\n\t},\n};'), 'shorthand'); + assert.equal(scopeOf('const obj = {\n\tprop: () => {\n\t\tHERE;\n\t},\n};'), 'prop'); + assert.equal(scopeOf('class C {\n\tmethod() {\n\t\tHERE;\n\t}\n}'), 'method'); + assert.equal(scopeOf('class C {\n\tfield = () => {\n\t\tHERE;\n\t};\n}'), 'field'); + assert.equal(scopeOf('obj.assigned = () => {\n\tHERE;\n};'), 'assigned'); + + // The regression that motivated the rewrite: an arrow function declared + // after a classic one used to inherit the classic one's name. + assert.equal( + scopeOf('function wrapper() {\n\treturn 1;\n}\n\nconst bypass = async () => {\n\tHERE;\n};'), + 'bypass', + ); +}); + +test('an offset outside every named function has no enclosing name', () => { + assert.equal(scopeOf('const x = HERE;'), null); + assert.equal(scopeOf('function f() {\n\treturn 1;\n}\nconst x = HERE;'), null); + + // An inline handler in the markup is not inside any named function either, + // so a caller comparing against a wrapper name fails on it rather than + // silently inheriting the last function declared above. + const markup = '\n\n\n'; + assert.equal(enclosingFunctionName(markup, offsetOf(markup, 'HERE')), null); +}); + +test('the innermost named function wins, and anonymous ones are transparent', () => { + assert.equal(scopeOf('function outer() {\n\tfunction inner() {\n\t\tHERE;\n\t}\n}'), 'inner'); + assert.equal(scopeOf('function outer() {\n\tconst inner = () => {\n\t\tHERE;\n\t};\n}'), 'inner'); + + // Lexically still inside `outer`, which is what the callers ask about. + assert.equal(scopeOf('function outer() {\n\tqueue.then(() => {\n\t\tHERE;\n\t});\n}'), 'outer'); +}); + +test('a callback body is its own body, whatever the closing brace looks like', () => { + // The exact hole: with no `\n\t});` of its own, the one-line effect used to + // merge into the next one — and the merged text carried the `editorReady` + // the first effect was missing. + const source = [ + '', + ].join('\n'); + + assert.deepEqual(callbackBodies(source, '$effect'), [ + '{ if (editor) editor.layout(); }', + '{\n\t\tif (editorReady) sync();\n\t}', + ]); + assert.deepEqual(callbackBodies(source, '$effect.pre'), ['{\n\t\tpre();\n\t}']); + assert.deepEqual(callbackBodies(source, 'onMount'), []); +}); + +test('the parsers read .ts files as well as components', () => { + const module = 'export function fromModule() {\n\tHERE;\n}\n'; + assert.equal(enclosingFunctionName(module, offsetOf(module, 'HERE')), 'fromModule'); + assert.deepEqual(callbackBodies('queueMicrotask(() => {\n\trun();\n});\n', 'queueMicrotask'), ['{\n\trun();\n}']); +}); + +test('a missing anchor fails loudly instead of slicing from -1', () => { + // `'abc'.slice(-1)` is `'c'`, not `''` — which is why an unguarded + // `slice(indexOf(...))` produces a one-character subject that no + // `doesNotMatch` can fail against. + assert.equal('abc'.slice('abc'.indexOf('zzz')), 'c'); + + assert.throws(() => sliceFrom('abc', 'zzz'), /expected to find "zzz"/); + assert.throws(() => sliceBetween('abc', 'zzz', 'b'), /expected to find "zzz"/); + assert.throws(() => sliceBetween('abc', 'a', 'zzz'), /expected to find "zzz" after "a"/); + assert.throws(() => offsetOf('abc', 'zzz'), /expected to find "zzz"/); + assert.throws(() => offsetOf('abcb', 'b', 4), /expected to find "b"/); + + assert.equal(sliceFrom('abc', 'b'), 'bc'); + assert.equal(sliceBetween('abcd', 'b', 'd'), 'bc'); + assert.equal(offsetOf('abcb', 'b', 2), 3); + + // `end` is searched from the end of `start`, so an earlier occurrence of + // `end` is a failure rather than an empty subject. + assert.throws(() => sliceBetween('xabc', 'b', 'x'), /expected to find "x" after "b"/); +}); diff --git a/scripts/sourceTree.ts b/scripts/sourceTree.ts index 473f5740..ec5ec0d8 100644 --- a/scripts/sourceTree.ts +++ b/scripts/sourceTree.ts @@ -2,6 +2,8 @@ import assert from 'node:assert/strict'; import { readFileSync, readdirSync, statSync } from 'node:fs'; import { join } from 'node:path'; +import { parse } from 'svelte/compiler'; + // Shared plumbing for the few tests that have to read `src/` as text. // // They exist because some contracts are invisible to the compiler: a Tauri @@ -50,6 +52,22 @@ export function sliceBetween(source: string, start: string, end: string): string return source.slice(from, to); } +/** + * The offset of `marker` in `source`, which must be present. + * + * The third shape, for the assertions that compare two offsets rather than + * slice with them: `assert.ok(a < b)` reads as "a comes first" but is also + * satisfied by `a === -1`, so a marker that stopped existing turns the + * ordering claim into a claim about nothing — the same failure `sliceFrom` + * exists for, one operator away. `from` mirrors `String#indexOf`'s second + * argument for the "…after this point" cases. + */ +export function offsetOf(source: string, marker: string, from = 0): number { + const at = source.indexOf(marker, from); + assert.notEqual(at, -1, `expected to find ${JSON.stringify(marker)}`); + return at; +} + /** Every compilable source file under `dir`, with forward-slash paths. */ export function walkSourceFiles(dir: string): string[] { const out: string[] = []; @@ -85,25 +103,206 @@ export function filesMatching(sources: SourceFile[], marker: RegExp): string[] { */ export const SANITIZER_FILES = ['src/lib/utils/richContent.ts', 'src/lib/utils/sanitize.ts']; +// ------------------------------------------------------------ syntax, parsed +// +// The three helpers below answer questions about *scope* — "which function is +// this offset inside", "what is the body of this callback". Those were regexes +// until they were shown to be answerable by writing the code differently: +// +// const renderRawBypass = async (raw: string) => { +// return (await invoke('render_markdown', { content: raw })) as string; +// }; +// +// dropped into MarkdownViewer.svelte left the whole suite green, because +// `enclosingFunctionName` matched `\n\t…function NAME(` and nothing else, so the +// call was attributed to whichever `function` happened to precede it — which was +// the wrapper the convention test demands. Fixing that by adding arrow functions +// to the pattern fixes one spelling. The forms actually in `src/` are 451 classic +// declarations, 74 `const f = (…) =>` / `const f = async (…) =>`, the class +// methods in `stores/`, and object-literal shorthand (`acceptNode(node) {`) — and +// the next contributor is free to invent a 5th. +// +// So the scope questions are answered from the real AST instead. `svelte/compiler` +// is already a dependency and `homeTabRender.test.ts` already parses a component +// with it; a function node is a function node whatever the spelling, and there is +// no pattern left to write around. The cost is ~170ms for the largest component, +// paid once per file thanks to the cache below. + +type FunctionScope = { start: number; end: number; name: string | null }; + +/** Node kinds that introduce a function scope. */ +const FUNCTION_TYPES = new Set(['FunctionDeclaration', 'FunctionExpression', 'ArrowFunctionExpression']); + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null; +} + +function offsetsOf(node: Record): { start: number; end: number } | null { + return typeof node.start === 'number' && typeof node.end === 'number' + ? { start: node.start, end: node.end } + : null; +} + +/** + * The parsed component, plus the shift from AST offsets back to `text` offsets. + * + * A `.ts` file is not a component, so it is wrapped in a ``, { modern: true }) as unknown, + shift: isComponent ? 0 : -prefix.length, + }; + parseCache.set(text, parsed); + return parsed; +} + +/** Depth-first walk of every node in the tree, with the binding name in scope. */ +function walkAst( + node: unknown, + visit: (node: Record, boundName: string | null) => void, + boundName: string | null = null, + seen: Set = new Set(), +): void { + if (Array.isArray(node)) { + for (const child of node) walkAst(child, visit, boundName, seen); + return; + } + if (!isRecord(node) || seen.has(node)) return; + seen.add(node); + + if (typeof node.type === 'string') visit(node, boundName); + + for (const [key, value] of Object.entries(node)) { + if (key === 'loc' || key === 'range' || key === 'parent' || key === 'metadata') continue; + walkAst(value, visit, nameBoundTo(node, key), seen); + } +} + +/** `foo` for `const foo = …`, `{ foo: … }`, `foo() {}`, `x.foo = …`. */ +function nameBoundTo(parent: Record, key: string): string | null { + switch (parent.type) { + case 'VariableDeclarator': + return key === 'init' ? identifierName(parent.id) : null; + case 'Property': + case 'PropertyDefinition': + case 'MethodDefinition': + return key === 'value' ? identifierName(parent.key) : null; + case 'AssignmentExpression': + return key === 'right' ? identifierName(parent.left) : null; + default: + return null; + } +} + +function identifierName(node: unknown): string | null { + if (!isRecord(node)) return null; + switch (node.type) { + case 'Identifier': + return typeof node.name === 'string' ? node.name : null; + case 'PrivateIdentifier': + return typeof node.name === 'string' ? `#${node.name}` : null; + case 'Literal': + return typeof node.value === 'string' ? node.value : null; + case 'MemberExpression': + return identifierName(node.property); + default: + return null; + } +} + +const scopeCache = new Map(); + +/** Every function scope in `text`, outermost first, with the name it is bound to. */ +function functionScopes(text: string): FunctionScope[] { + const cached = scopeCache.get(text); + if (cached) return cached; + + const { root, shift } = parsedSource(text); + const scopes: FunctionScope[] = []; + walkAst(root, (node, boundName) => { + if (typeof node.type !== 'string' || !FUNCTION_TYPES.has(node.type)) return; + const span = offsetsOf(node); + if (!span) return; + scopes.push({ + start: span.start + shift, + end: span.end + shift, + name: identifierName(node.id) ?? boundName, + }); + }); + scopes.sort((a, b) => a.start - b.start || b.end - a.end); + + scopeCache.set(text, scopes); + return scopes; +} + /** - * The name of the top-level `