Let F# keep per-document data in Roslyn's persistent storage

[xperiandri]

Navigate To keeps C# and Visual Basic declarations in TopLevelSyntaxTreeIndex, persisted through
IChecksummedPersistentStorage under a checksum of each document's text, so a new session reads the index back
instead of parsing, and the search that runs while a solution loads answers from what the last session wrote. F# has
had a place in that search since #85213 — IFSharpAdvancedNavigateToSearchService.SearchCachedDocumentsAsync, whose
contract asks for results "from a previously computed cache (even if that cache is out of date)" — but nothing to keep
such a cache in. Its declarations cannot be a TopLevelSyntaxTreeIndex (F# documents have no SyntaxTree, and
IDeclaredSymbolInfoFactoryService exists only for C# and Visual Basic), and the storage that would hold its own
index is internal.

This exposes the storage, not an index: F# writes its own per-document data where Roslyn keeps its indices, under the
same solution key, checksums and lifetime.

• IFSharpChecksummedPersistentStorageService.GetStorageAsync(Solution, CancellationToken) returns the solution's
  storage, as IChecksummedPersistentStorageService.GetStorageAsync does for a SolutionKey.
• IFSharpChecksummedPersistentStorage carries the per-document members of IChecksummedPersistentStorage —
  ChecksumMatchesAsync, ReadStreamAsync, WriteStreamAsync — with their semantics: a read given a checksum
  returns null unless the data was written with it.
• A checksum crosses as the bytes of a Checksum, an ImmutableArray<byte> passed to Checksum.From, since
  Checksum is internal; where the storage takes an optional one, default stands for none. Data crosses as a
  Stream, so ObjectWriter and ObjectReader stay internal.
• FSharpChecksummedPersistentStorageService implements it over solution.Services.GetPersistentStorageService()
  and SolutionKey.ToSolutionKey, and is exported for F# to import, as FSharpGlobalOptions is.

The type forwards and the InternalAPI.Unshipped.txt entries of both external access assemblies are added as #85213
added its own. Neither file is checked by an analyzer outside src/Razor locally, so the entries follow their
neighbours rather than a generated baseline.

The consumer

The F# editor keeps its Navigate To items on disk between sessions today in a file cache of its own, behind an
interface shaped after this one: a document's items are stored under a checksum of its text — and of the defines for
a file with #if — and read back before parsing, which also answers the loading search before a project has its
options. Once this has flowed, that cache becomes an adapter over IFSharpChecksummedPersistentStorageService, and
F# stops writing a second store next to Roslyn's.

No test is added here, matching the other external access contracts. Both external access projects build with
-p:RunAnalyzersDuringBuild=true without warnings.

🤖 Generated with Claude Code

Microsoft Reviewers: Open in CodeFlow

[azure-pipelines]

Azure Pipelines:
Successfully started running 2 pipeline(s).
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

🔵 Needs a closer look

Add coverage for normal, default, and invalid-length checksum conversion paths.

Pull request overview

Adds a Roslyn persistent-storage bridge for F# per-document Navigate To data.

Changes:

• Adds F# storage interfaces and service access.
• Implements the Roslyn storage adapter and checksum conversion.
• Updates type forwards and API baselines.

File summaries

File	Summary
src/VisualStudio/ExternalAccess/FSharp/TypeForwards.cs	Forwards the new F# storage contracts.
src/VisualStudio/ExternalAccess/FSharp/InternalAPI.Unshipped.txt	Records F# API additions.
src/VisualStudio/ExternalAccess/Core/InternalAPI.Unshipped.txt	Records Core API additions.
src/VisualStudio/ExternalAccess/Core/FSharp/Storage/IFSharpChecksummedPersistentStorageService.cs	Defines solution storage access.
src/VisualStudio/ExternalAccess/Core/FSharp/Storage/IFSharpChecksummedPersistentStorage.cs	Defines document storage operations.
src/VisualStudio/ExternalAccess/Core/FSharp/Internal/Storage/FSharpChecksummedPersistentStorageService.cs	Adapts Roslyn storage; checksum conversion paths need focused coverage.

Review details

Suppressed comments (1)

src/VisualStudio/ExternalAccess/Core/FSharp/Internal/Storage/FSharpChecksummedPersistentStorageService.cs:42

• This adapter contains the only conversion from F# checksum bytes to Roslyn Checksum/null, including the default sentinel used to read stale data or write without a checksum, but no test exercises those paths. A regression here would either make persisted F# Navigate To data unreadable or cause stale-cache reads to throw; add focused coverage for normal, default, and invalid-length checksum inputs (through the service/integration boundary if the private adapter remains inaccessible).

        public Task<Stream?> ReadStreamAsync(Document document, string name, ImmutableArray<byte> checksum, CancellationToken cancellationToken)
            => storage.ReadStreamAsync(document, name, ToOptionalChecksum(checksum), cancellationToken);

        public Task<bool> WriteStreamAsync(Document document, string name, Stream stream, ImmutableArray<byte> checksum, CancellationToken cancellationToken)
            => storage.WriteStreamAsync(document, name, stream, ToOptionalChecksum(checksum), cancellationToken);

        private static Checksum? ToOptionalChecksum(ImmutableArray<byte> checksum)
            => checksum.IsDefault ? null : Checksum.From(checksum);

• Files reviewed: 6/6 changed files
• Comments generated: 0
• Review effort level: Lite

[CyrusNajmabadi]

Why not use Roslyn s storage? It's a simple stream/byte[] oriented api

[xperiandri]

@CyrusNajmabadi Could you specify which interface you mean? I know only one and it is marked as deprecated

[Copilot]

🔵 Needs a closer look

Add unit coverage for the adapter’s checksum boundary behavior.

Review details

Suppressed comments (1)

src/VisualStudio/ExternalAccess/Core/FSharp/Internal/Storage/FSharpChecksummedPersistentStorageService.cs:42

• Please add unit coverage for this adapter's checksum boundary, including a valid checksum, a longer checksum, the default array used for an omitted optional checksum, and invalid short/empty arrays. ToOptionalChecksum intentionally maps only default to no checksum while Checksum.From rejects short inputs and truncates long ones; without exercising this bridge, a future contract change can make F# cache reads miss or throw even though the underlying storage tests still pass.

        private static Checksum? ToOptionalChecksum(ImmutableArray<byte> checksum)
            => checksum.IsDefault ? null : Checksum.From(checksum);

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[xperiandri]

Added checksum boundary coverage per the review: a 16-byte checksum, a longer one (truncated, not rejected), a default array on each method, and shorter non-default arrays (0/1/15 bytes) on all three. ChecksumMatchesAsync has no "omitted" case — the storage method it calls takes a non-optional Checksum, so a default array is rejected there like a too-short one, unlike ReadStreamAsync/WriteStreamAsync. Verified the read/write test actually guards ToOptionalChecksum: reverting it to call Checksum.From directly fails exactly that test and no other.

23b4f3b

[xperiandri]

@T-Gro could you take a look at this when you have a moment?

[Copilot]

🔵 Needs a closer look

Tests use NoOpPersistentStorage and do not verify storage forwarding or round-trip behavior.

Review details

Suppressed comments (2)

src/VisualStudio/ExternalAccess/Test/FSharp/FSharpChecksummedPersistentStorageServiceTests.cs:29

• The PR description still says “No test is added here,” but this revision adds a dedicated test file with five checksum-boundary tests. Please update the description so the stated scope and validation accurately reflect the current diff.

public sealed class FSharpChecksummedPersistentStorageServiceTests

src/VisualStudio/ExternalAccess/Test/FSharp/FSharpChecksummedPersistentStorageServiceTests.cs:35

• All test cases create an AdhocWorkspace with no solution file path, so GetPersistentStorageService() returns NoOpPersistentStorage. They therefore only validate checksum conversion; they would still pass if GetStorageAsync used the wrong SolutionKey or any of the document/name/stream/cancellation arguments were forwarded incorrectly. Add a file-backed or custom-storage round-trip test that exercises ChecksumMatchesAsync, ReadStreamAsync, and WriteStreamAsync through this adapter.

        using var workspace = new AdhocWorkspace();
        var projectId = workspace.AddProject("Project", LanguageNames.CSharp).Id;
        var document = workspace.AddDocument(projectId, "Test.cs", SourceText.From(""));

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Lite