test(scripts): answer scope questions by parsing, and guard 71 slice bounds

[PathGao]

Two holes in the test tooling. They are independent; they share a PR because
scripts/sourceTree.ts is where both are fixed. Both were reproduced before
they were touched.

1. A convention test was bypassed by writing an arrow function

enclosingFunctionName decides which function an offset sits inside. It matched
one spelling:

\n\t(?:export\s+)?(?:async\s+)?function\s+NAME\s*\(

So a call site inside const f = async () => {} was attributed to whichever
classic function happened to precede it — and in MarkdownViewer.svelte the
function preceding the obvious place to add one is renderMarkdownPreview, the
exact wrapper the test demands.

Measured, both ways

Planted immediately after renderMarkdownPreview:

const renderRawBypass = async (raw: string) => {
	return (await invoke('render_markdown', { content: raw })) as string;  // no front-matter strip
};

	npm test
before	tests 524 pass 524 fail 0
after	fail 1 — actual: [ 'renderMarkdownPreview', 'renderRawBypass' ]

renderPipelineConvention and singleImplementationConvention were both green
with the bypass in place. Nothing else in the suite saw it either.

The same weakness in monacoStartupGraph, confirmed

"every effect that drives the editor waits for editorReady" sliced effect
bodies with /\$effect\(\(\) => \{([\s\S]*?)\n\t\}\);/. That needs a
tab-indented }); to end a body, so an effect written on one line has no
terminator of its own and the lazy match runs on to the next effect's:

$effect(() => { if (editor) editor.layout(); });   // planted in Editor.svelte

	result
before	passes — the merged body carries the next effect's editorReady
after	fails: an effect touching the editor is not gated on editorReady: { if (editor) editor.layout(); }

The effects.length >= 5 vacuity guard did not notice, because merging two
effects into one body kept the count at six.

Parsing, not a longer pattern

svelte/compiler is already a dependency and homeTabRender.test.ts already
parses a component with it. enclosingFunctionName and the new callbackBodies
now walk the real AST: parse(source, { modern: true }), collect every
FunctionDeclaration / FunctionExpression / ArrowFunctionExpression span
with the name it is bound to, and answer containment against that. Parses are
cached per source string; the largest component costs ~170ms once.

A survey of src/ first, because the alternative was to widen the regex to
whatever is there today:

form	count in src/
function f() / export async function f()	451
const f = () => {} / const f = async () => {}	74
class methods (stores/)	all of TabManager
object shorthand (acceptNode(node) {)	2

A regex covering those four fails the next time someone writes a fifth. A
function node is a function node whatever the spelling, so there is no longer a
pattern to write around. The cost is one dependency the suite already had and
~170ms; the alternative saves that and keeps a hole whose size is "whatever
nobody thought of".

Two decisions inside it, both deliberate:

• Anonymous functions are transparent. A call inside .then(() => …) inside
  renderMarkdownPreview is still lexically inside renderMarkdownPreview,
  which is the question the callers ask. An offset with no named function above
  it at all — an inline onclick={() => …} in the markup — is null, and a
  caller comparing against a wrapper name fails loudly on it.
• .ts files are supported by wrapping them in a <script> before parsing
  and subtracting the wrapper length, so a caller that passes a store instead of
  a component gets an answer rather than a silent null everywhere.

scripts/sourceTree.test.ts (new) pins all twelve declaration forms, the
nesting rules, and the one-line-effect case against string literals rather than
against src/ — refactoring the app cannot make it fail, and changing the
helper is the only thing that can.

No violation was hiding behind the old regex

Both convention tests are green on the real tree after the tightening. The
bypass had to be planted to make either fail.

2. 71 unguarded slice and ordering bounds

slice(x.indexOf(m)) with an absent m slices from -1, which yields the last
character of the file — so assert.doesNotMatch against it can never fail.
#432 fixed two instances and added sliceFrom/sliceBetween; the rest were
left.

Re-derived, because #433 deleted 14 files since

At e196547 (merge base), counting indexOf calls on source text whose result
becomes a slice bound or an ordering operand, with nothing asserting they are
not -1:

	bound-forming indexOf	unguarded
4251f0a (#432 landed)	113	74
e196547 (after #433)	108	71
this branch	3	1

The remaining one is assert.equal('abc'.slice('abc'.indexOf('zzz')), 'c') in
the new unit test, which exists to demonstrate the defect.

155 raw indexOf bounds are replaced by 110 helper call sites across 31 files.
Seven files carried a private copy of sliceBetween under another name
(sliceBlock, slice, pluck, an IIFE) — those are deleted and the shared one
used, which is also how three of them acquired a guard on their end anchor.

One new shape, offsetOf

The migration exposed a second form the two existing helpers do not cover:

const a = body.indexOf('foo');
const b = body.indexOf('bar');
assert.ok(a < b, 'foo comes first');

a < b reads as an ordering claim but is also satisfied by a === -1, so an
anchor that stopped existing turns it into a claim about nothing. offsetOf
asserts and returns; 56 of the migrated sites are this shape, which is what
justifies a third helper rather than a fourth copy of assert.notEqual(…, -1).

Two sites had bespoke bounds that no helper covers and were rewritten in place:

• truncatedBufferGuard.test.ts ended a handler at
  indexOf('\n\tfunction ') + 1 || indexOf('\n\tasync function ') + 1. That
  reads as "or else" but is really "or else, if the first is absent or at
  offset 0", and when the first form appeared later in the file than the second
  it won anyway — widening the body past the handler, which for an
  assert.match is a false green. It now takes the earlier of the two that
  exist, and asserts at least one does.
• renderProtocol.test.ts sliced between indexOf('$$') and
  lastIndexOf('$$'); those coincide for a fixture with one $$, which is now
  a failure.

Two anchors replaced by a parse instead of a guard

scrollSyncInput.test.ts — the file #432 named as the prior fix, and flagged
as still half-guarded — was sliced between '&& onscrollsync)' and
'\n\t$effect(() => {'. Its start anchor had already drifted once, silently,
when the effect gained an editorReady && term; its end anchor never got a
guard and depended on the next effect's indentation. It now selects the one
$effect whose body reads onscrollsync and asserts there is exactly one.
settingsPersistence.test.ts's open-effect anchor
('$effect(() => {\n\t\tif (show) {') is replaced the same way.

Falsifiability, checked by breaking every anchor

Each string anchor passed to sliceFrom / sliceBetween / offsetOf was
corrupted in turn — ZZNOPE inserted before the closing quote — and its test
file run:

	count
anchors mutated	194
failed naming the missing anchor	193
failed for another reason	0
did not fail	1

The one that did not fail is the second argument of
assert.throws(() => sliceBetween('abc', 'zzz', 'b'), /expected to find "zzz"/)
in the new unit test — unreachable by construction, since the first anchor
throws first.

Not covered

• 24 indexOf calls remain in scripts/, deliberately. CSS and comment
  tokenizers that already branch on -1 (editorPdfExport, exportFoldParity,
  i18nCoverage); Array#indexOf on DOM shims and event logs, where -1 means
  "not a child" rather than "anchor missing"; and exportSanitize.test.ts,
  already guarded with messages more specific than offsetOf's.
• Three comment-skipping loops (foldStatePerDocument,
  viewModeWithoutSaving) do i = source.indexOf('\n', i) with no guard. A
  -1 there hangs the loop rather than passing a degenerate assertion, so it is
  a different failure mode; it cannot trigger while every source file ends with
  a newline. Left as is.
• The AST helpers answer scope, not dataflow. "The raw command is invoked
  from renderMarkdownPreview" is still lexical containment. That the value
  reaching it had its front matter stripped is not established here, and was not
  established by the regex either.
• ~25 sites still slice one function out by naming the next one
  (sliceBetween(viewer, 'async function a', 'async function b')). They are
  guarded now, so a rename fails loudly instead of silently, but a
  functionSource(text, name) built on the same parse would remove the second
  anchor entirely. Not done here — it changes what those slices contain.
• No behaviour change. src/ and src-tauri/ are untouched;
  npm test (530), npm run check (631 files, 0 errors) and npm run build
  are clean.

🤖 Generated with Claude Code