The shared source-walker blanks interpolated-string holes, so a call inside an interpolation is invisible to every scan built on it

[erikdarlingdata]

Summary

StripCommentsAndStrings — copied into ViewerCommandTimeoutTests, ViewerFleetTimerGuardTests and the other source-walking pins — blanks the entire span of an interpolated string, including the {expr} holes. So a call written inside an interpolation is invisible to every scan built on it.

Raised by the review bot on #2910 and worth keeping: it produces no false negative today (nothing on the fleet-timer fan-out calls through an interpolation), so this is a latent gap, not a live defect.

// visible to the scan
var x = Foo();
// invisible: SkipRegularString consumes to the closing quote, holes included
Log($"count={Foo()}");

Why it is worth fixing rather than noting

It is the same category as the defect that bit #2910's pin during review, one shape further out. That pin claimed to reach a guard one level down; it did not, because RefreshAgAsync is => LoadAsync(); — a plain call in an expression body, matching neither call shape the walk looked for. The walk ran out of edges and reported the path safe without ever reaching the guard that makes it safe.

A pass for the wrong reason is worth less than a failure, and these pins exist precisely to convert an open question into a checked one. An interpolation hole is another way for an edge to go missing, and the failure direction is the dangerous one: a reachability scan under-reports, and an assertion that a count is zero (FetchStoreConnectionBorrowTests) is satisfied vacuously by an edge it cannot see.

Where it applies

StripCommentsAndStrings / SkipRegularString / SkipVerbatimString are duplicated across several test files rather than shared. The near-identical IlCallSiteScanner consolidation (#2898) is the precedent: five copies of an IL walk, two soundness problems, neither yet producing a wrong answer — fixed once, centrally, with the scanner carrying its own witnesses.

Worth checking whether the same treatment applies here, i.e. one shared source-walker with:

• interpolation holes preserved as code, not blanked ($"...", $@"..." / @$"...", and raw string literals """...""", which the current skipper also does not know about);
• nested braces inside a hole handled ($"{d[k]}", $"{(c ? a() : b())}");
• {{ / }} escapes not mistaken for holes.

Proving it

Same rule as every other pin here: prove it red before green. A fixture whose only call to a guarded method sits inside an interpolation should make the walker report that call before the fix and after — and must be shown to report nothing with the current skipper, or the fixture is not testing what it claims. Do not rely on a real source file happening to have the shape; arrange it.

[erikdarlingdata]

Fixed by #2925, merged to dev. One shared CSharpSourceWalker with its own witnesses, replacing five duplicated copies.

Five, not three — and this issue's framing is why the count looked smaller

This issue described the duplication as copies of StripCommentsAndStrings. The actual shape is two families sharing two skippers: StripCommentsAndStrings ×3, StatementSpanFrom ×3, and SkipVerbatimString/SkipRegularString ×5. So a grep for StripCommentsAndStrings alone under-counts, which is how AlertPassCommandTimeoutTests and StorageCommandTimeoutTests — both carrying StatementSpanFrom plus both skippers but no stripper — went unlisted.

One copy had diverged, which is the second argument for consolidating: ViewerFleetTimerGuardTests' SkipRegularString had grown a newline stop and return text.Length that the other four lacked. One of five carried hardening nobody else got.

This was NOT latent — the old walker mis-classifies shipped source today

This issue said it "produces no false negative today." That is true of the pins' verdicts, and not true of the walker. Two of the 269 files the pins scan come out bracket-unbalanced under the old walk: ViewerServerTab.ChartContextMenu.cs at braces −1 / parens +2, from a value.Contains('"') — a char literal holding a quote — and StoreConnectionSelfTest.cs at braces +3 from a raw string.

That matters beyond tidiness, because the fleet-timer pin extracts method bodies by brace-matching that same output. A walker that miscounts braces on shipped source is one edit away from slicing the wrong body.

No pin's verdict changed — and the inputs demonstrably moved, so that isn't vacuous

The claim "nothing changed" would be hollow if the walker saw identically. It doesn't:

• +25 real call edges the old walk could not see (NewGuid, Split, TrimEnd, property reads like FailedCollectorCount) — calls written inside interpolations.
• −235 phantom edges that were SQL keywords (AND, ANY, COALESCE, date_trunc, CAST) leaking out of desynchronised raw strings and being read as code.

Against that, all 362 command-construction sites across three timeout pins return a byte-identical statement span and the same timed/untimed verdict (span-text-changed=0, VERDICT-CHANGED=0). The hand-off sweeps stay 0→0 with their own positive-control fixtures still matching, so the zero is not a quiet zero. The fleet-timer pin's four assertions pass with identical internals both ways — 883 method names, 1,020 declarations, 815,347 body chars, fan-out 5, guarded 5. None of the 25 newly-visible edges resolves to a viewer-declared method reaching a store read, which is why the verdict holds.

And no assertion was touched to achieve that: Assert. counts are identical dev→branch across all five files (8/10/9/15/16), test-attribute counts identical (6/15/7/17/4), and no assertion appears among the 597 deleted lines.

Fifteen witnesses, living with the walker

Interpolation holes · raw strings · $"""/$$""" holes with the brace-run rule · nested brackets and a deep : that is not a format specifier · {{/}} escapes · format specifier and alignment · verbatim doubled-quote escape plus $@"/@$" holes · @ followed by a quote run · a char literal holding a quote · the span walker sharing the mask · three comment forms · a 269-file corpus balance check · character-alignment and newline preservation.

Red-first per construct, against this issue's own stated bar — the fixture must report nothing under the old skipper, not merely less. Interpolated call: legacy strip contains it once (the control, written outside an interpolation), new contains it twice. $""" holes: Probe() absent from legacy, present in new — while {notAHole} (a single brace under two $) is correctly absent from both, which is the compiler's rule rather than a guess. Raw strings invert the direction: legacy over-reports, classifying a raw body as code and leaving paren balance +1; new reads it as text at 0. Plus seven mutations, each failing a distinct witness set, with zero consolidated-pin failures under any of them — the latency claim itself asserted as a test.

The corpus check found a bug in the new walker

Widening the balance check from the 269 files the pins read to all 2,088 .cs files in the repo surfaced a real defect in the replacement: a raw string literal cannot carry the @ prefix, so a quote run after @ is an escaped quote and not a delimiter. Exactly two of 2,088 came out unbalanced and both were that shape. Fixed with an arranged witness.

Also worth recording: $"""{{""" is not answerable from the specification — the compiler rejects it (CS9006), so the doubled-brace escape is only legal on the non-raw forms. Measured, not assumed.

A git trap found on the way

The new test file first committed as a CRLF blob, because a single stray lone \r flipped git's text=auto detection to binary and silently skipped normalisation — and --renormalize was a no-op until the CR was removed. Confirmed with a controlled two-file experiment. Both files now match every other .cs blob: no BOM, LF in the blob, CRLF in the working tree.

Follow-on

#2923 is now unblocked. That gap — ViewerFleetTimerGuardTests passing if the fan-out relocates below the tab early-return — needed comment-and-string stripping to write soundly, and was deliberately queued behind this rather than becoming a sixth copy. It can now be written on the shared walker.

Not verified

Darling.Tests never ran on the working machine (net10.0-windows); the fifteen witnesses ran in a throwaway net10.0 host compiling the real source, 28 → 43 tests (+15) green both sides, and CI's build job is the arbiter — green at 65cfe7d5. ViewerCommandTimeoutTests cannot run at all without the WPF viewer; its walker inputs and all ten InlineData fixtures were compared old-vs-new outside xunit instead, and its three deadline-band tests are pure constants, untouched. No CI-runner performance measurement was taken; StatementSpanFrom recomputes the mask per call, 0.27 s for all 43 locally.