From 7a1617c36ef23928cc54dadd526b2181a30a9fef Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 09:42:41 +0200 Subject: [PATCH 01/10] Build multi-project and multi-target Roslyn solutions in FSharp.Editor tests Test helpers so far put every synthetic file into one Roslyn project. CreateMultiProjectSolution creates one project per synthetic project with project references, the way VS wires project-to-project references; CreateMultiTargetSolution creates one project per target instance sharing the project path and the document paths, the way VS loads a multi-targeted project. Co-Authored-By: Claude Fable 5.1 --- .../Helpers/RoslynHelpers.fs | 146 +++++++++++++++++- 1 file changed, 140 insertions(+), 6 deletions(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs index 25509f14ace..89a449eceb7 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs @@ -201,6 +201,14 @@ type TestHostServices() = override this.CreateWorkspaceServices(workspace) = new TestHostWorkspaceServices(this, workspace) +/// One Roslyn project instance of a multi-targeted F# project: its extra defines and the +/// synthetic files left out of it, as VS does per target framework. +type TargetInstance = + { + Defines: string list + ExcludedFileIds: string list + } + [] type RoslynTestHelpers private () = @@ -258,6 +266,33 @@ type RoslynTestHelpers private () = filePath = filePath ) + static member private ProjectInfoFor + (id, name, filePath, outputFilePath, documents, projectReferences: ProjectReference list, metadataReferences: MetadataReference seq) + = + ProjectInfo.Create( + id, + VersionStamp.Create(DateTime.UtcNow), + name, + name, + LanguageNames.FSharp, + filePath = filePath, + outputFilePath = outputFilePath, + documents = documents, + projectReferences = projectReferences, + metadataReferences = metadataReferences + ) + + static member private MetadataReferencesOf(options: FSharpProjectOptions, excludedPaths: string seq) = + let excluded = HashSet(excludedPaths, StringComparer.OrdinalIgnoreCase) + + options.OtherOptions + |> Seq.filter (fun x -> x.StartsWith("-r:", StringComparison.Ordinal)) + |> Seq.map _.Substring(3) + |> Seq.filter (excluded.Contains >> not) + |> Seq.map MetadataReference.CreateFromFile + |> Seq.cast + |> Seq.toList + static member SetProjectOptions projId (solution: Solution) (options: FSharpProjectOptions) = solution.Workspace.Services .GetService() @@ -331,12 +366,8 @@ type RoslynTestHelpers private () = let options = syntheticProject.GetProjectOptions checker - let metadataReferences = - options.OtherOptions - |> Seq.filter (fun x -> x.StartsWith("-r:")) - |> Seq.map (fun x -> x.Substring(3) |> MetadataReference.CreateFromFile :> MetadataReference) - - let projInfo = projInfo.WithMetadataReferences metadataReferences + let projInfo = + projInfo.WithMetadataReferences(RoslynTestHelpers.MetadataReferencesOf(options, [])) let solution = RoslynTestHelpers.CreateSolution [ projInfo ] @@ -344,6 +375,109 @@ type RoslynTestHelpers private () = solution, checker + /// One Roslyn project per synthetic project, wired with project references the way VS wires + /// project-to-project references, so the options manager builds in-memory F# references. + static member CreateMultiProjectSolution(syntheticProject: SyntheticProject) = + let checker = syntheticProject.SaveAndCheck() + + let projects = + syntheticProject.GetAllProjects() + |> List.distinctBy _.Name + |> List.map (fun project -> project, ProjectId.CreateNewId()) + + let projectIds = dict [ for project, id in projects -> project.Name, id ] + + let projectInfos = + [ + for project, id in projects do + let options = project.GetProjectOptions checker + + RoslynTestHelpers.ProjectInfoFor( + id, + project.Name, + project.ProjectFileName, + project.OutputFilename, + [ + for path in project.SourceFilePaths -> RoslynTestHelpers.CreateDocumentInfo id path (File.ReadAllText path) + ], + [ + for dependency in project.DependsOn -> ProjectReference projectIds[dependency.Name] + ], + RoslynTestHelpers.MetadataReferencesOf(options, project.DependsOn |> List.map _.OutputFilename) + ) + ] + + let solution = RoslynTestHelpers.CreateSolution projectInfos + + for project, id in projects do + project.GetProjectOptions checker + |> RoslynTestHelpers.SetProjectOptions id solution + + solution, checker + + /// One Roslyn project per target instance, all sharing the .fsproj path and the document file + /// paths, like the per-target-framework projects VS creates for a multi-targeted project. + static member CreateMultiTargetSolution(syntheticProject: SyntheticProject, instances: TargetInstance list) = + assert (syntheticProject.DependsOn = []) + + let checker = syntheticProject.SaveAndCheck() + let options = syntheticProject.GetProjectOptions checker + let metadataReferences = RoslynTestHelpers.MetadataReferencesOf(options, []) + + let instances = + [ + for instance in instances -> + let excludedPaths = + HashSet( + [ + for fileId in instance.ExcludedFileIds do + syntheticProject.GetFilePath fileId + + if (syntheticProject.Find fileId).HasSignatureFile then + syntheticProject.GetSignatureFilePath fileId + ], + StringComparer.OrdinalIgnoreCase + ) + + let sourceFiles = + syntheticProject.SourceFilePaths |> List.filter (excludedPaths.Contains >> not) + + let id = ProjectId.CreateNewId() + + let projectInfo = + RoslynTestHelpers.ProjectInfoFor( + id, + syntheticProject.Name, + syntheticProject.ProjectFileName, + syntheticProject.OutputFilename, + [ + for path in sourceFiles -> RoslynTestHelpers.CreateDocumentInfo id path (File.ReadAllText path) + ], + [], + metadataReferences + ) + + let instanceOptions = + { options with + SourceFiles = List.toArray sourceFiles + OtherOptions = + [| + yield! options.OtherOptions + for define in instance.Defines -> $"--define:{define}" + |] + } + + id, projectInfo, instanceOptions + ] + + let solution = + RoslynTestHelpers.CreateSolution [ for _, projectInfo, _ in instances -> projectInfo ] + + for id, _, instanceOptions in instances do + RoslynTestHelpers.SetProjectOptions id solution instanceOptions + + solution, [ for id, _, _ in instances -> id ] + static member GetFsDocument(code, ?customProjectOption: string, ?customEditorOptions) = let customProjectOptions = customProjectOption From a8d95374132b9814b2dd8ec8d1674e6e9c1256ae Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 7 Sep 2026 20:15:02 +0200 Subject: [PATCH 02/10] Search each file of a multi-targeted project once in NavigateTo and read its text only for matches Roslyn's NavigateTo searcher hands the F# service every target-framework instance of a project, one after another, and the service parsed every file of each instance, read the text of every file before matching anything, and started all of that for a project at once. On a solution with 135 project instances that meant one parse and one file read per file per framework, thousands of concurrent tasks, and results that arrived long after the user stopped typing. The first instance of a project file in the solution now searches every file; the other instances only search the files they alone compile and the files whose parse depends on the defines, known from whichever instance parsed the file first. A file's text is read only when one of its declarations matched, and a project's files are searched at most ProcessorCount at a time. Co-Authored-By: Claude Fable 5.1 --- .../Navigation/NavigateToSearchService.fs | 108 ++++++++++++------ 1 file changed, 71 insertions(+), 37 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs b/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs index 546b00e1b16..a529739d16d 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs @@ -17,6 +17,7 @@ open Microsoft.VisualStudio.LanguageServices open Microsoft.VisualStudio.Text.PatternMatching open FSharp.Compiler.EditorServices +open FSharp.Compiler.Syntax open CancellableTasks [); Shared>] @@ -26,12 +27,22 @@ type internal FSharpNavigateToSearchService let cache = ConcurrentDictionary() + /// Whether the file's parse depends on the defines, by file path: known once any instance has parsed it. + let conditionalDirectives = + ConcurrentDictionary(StringComparer.OrdinalIgnoreCase) + do if workspace <> null then workspace.WorkspaceChanged.Add <| fun e -> if e.NewSolution.Id <> e.OldSolution.Id then cache.Clear() + conditionalDirectives.Clear() + + let hasConditionalDirectives (parseTree: ParsedInput) = + match parseTree with + | ParsedInput.ImplFile file -> not file.Trivia.ConditionalDirectives.IsEmpty + | ParsedInput.SigFile file -> not file.Trivia.ConditionalDirectives.IsEmpty let getNavigableItems (document: Document) = cancellableTask { @@ -44,9 +55,42 @@ type internal FSharpNavigateToSearchService let! parseResults = document.GetFSharpParseResultsAsync(nameof (FSharpNavigateToSearchService)) let items = NavigateTo.GetNavigableItems parseResults.ParseTree cache[document.Id] <- currentVersion, items + + match document.FilePath with + | null -> () + | path -> conditionalDirectives[path] <- hasConditionalDirectives parseResults.ParseTree + return items } + /// A multi-targeted project is one Roslyn project per target framework over the same files. The + /// first instance in the solution searches every file; the others only the files they alone compile + /// and the files whose parse depends on the defines. + let searchedIn (project: Project) = + match project.FilePath with + | null -> fun (_: Document) -> true + | projectPath -> + let instances = + project.Solution.Projects + |> Seq.filter (fun p -> p.FilePath = projectPath) + |> Seq.map _.Id + |> List.ofSeq + + fun (document: Document) -> + match document.FilePath with + | null -> true + | path -> + let documentIds = project.Solution.GetDocumentIdsWithFilePath path + + let owner = + instances + |> List.find (fun id -> documentIds |> Seq.exists (fun documentId -> documentId.ProjectId = id)) + + owner = project.Id + || (match conditionalDirectives.TryGetValue path with + | true, dependsOnDefines -> dependsOnDefines + | _ -> true) + let kindsProvided = ImmutableHashSet.Create( FSharpNavigateToItemKind.Module, @@ -145,46 +189,44 @@ type internal FSharpNavigateToSearchService let processDocument (tryMatch: NavigableItem -> PatternMatch voption) (kinds: IImmutableSet) (document: Document) = cancellableTask { - let! ct = CancellableTask.getCancellationToken () - - let! sourceText = document.GetTextAsync ct - let! items = getNavigableItems document - let processed = + let matches = [| for item in items do - let contains = kinds.Contains(navigateToItemKindToRoslynKind item.Kind) - let patternMatch = tryMatch item + if kinds.Contains(navigateToItemKindToRoslynKind item.Kind) then + match tryMatch item with + | ValueSome m -> yield struct (item, m) + | ValueNone -> () + |] - match contains, patternMatch with - | true, ValueSome m -> - let sourceSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, item.Range) + // The text, read from disk for a closed document, is only needed to place the matches. + if matches.Length = 0 then + return [||] + else + let! ct = CancellableTask.getCancellationToken () + let! sourceText = document.GetTextAsync ct - match sourceSpan with + return + [| + for struct (item, m) in matches do + match RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, item.Range) with | ValueNone -> () | ValueSome sourceSpan -> - let glyph = navigateToItemKindToGlyph item.Kind - let kind = navigateToItemKindToRoslynKind item.Kind - let additionalInfo = formatInfo item.Container document - yield FSharpNavigateToSearchResult( - additionalInfo, - kind, + formatInfo item.Container document, + navigateToItemKindToRoslynKind item.Kind, patternMatchKindToNavigateToMatchKind m.Kind, item.Name, FSharpNavigableItem( - glyph, + navigateToItemKindToGlyph item.Kind, ImmutableArray.Create(TaggedText(TextTags.Text, item.Name)), document, sourceSpan ) ) - | _ -> () - |] - - return processed + |] } interface IFSharpNavigateToSearchService with @@ -194,22 +236,14 @@ type internal FSharpNavigateToSearchService cancellableTask { let tryMatch = createMatcherFor searchPattern - let tasks = - [| - for doc in project.Documents do - yield processDocument tryMatch kinds doc - |] - - let! results = CancellableTask.whenAll tasks - - let results' = ImmutableArray.CreateBuilder() - - for navResults in results do - for navResult in navResults do - results'.Add navResult - - return results'.ToImmutable() + let! results = + project.Documents + |> Seq.filter (searchedIn project) + |> Seq.map (processDocument tryMatch kinds) + // Throttle to avoid launching a parse per document in the project all at once. + |> CancellableTask.whenAllThrottled (max 1 Environment.ProcessorCount) + return results |> Array.concat |> Array.toImmutableArray } |> CancellableTask.start cancellationToken From 7dff9b45ae463c256194e52cf55d0a920bf3f10c Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 7 Sep 2026 20:15:04 +0200 Subject: [PATCH 03/10] Test NavigateTo across the target-framework instances of a project Co-Authored-By: Claude Fable 5.1 --- .../FSharp.Editor.Tests.fsproj | 1 + .../MultiTargetNavigateToSearchTests.fs | 70 +++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs diff --git a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj index ecce1205b8c..a71cae6e92b 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj +++ b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj @@ -32,6 +32,7 @@ + diff --git a/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs b/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs new file mode 100644 index 00000000000..c1febedfd5e --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs @@ -0,0 +1,70 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +/// One project loaded as two target-framework instances: `plain` compiles without the fourth file +/// and without FOO, `foo` compiles everything with FOO defined. +module FSharp.Editor.Tests.MultiTargetNavigateToSearchTests + +open System.Collections.Immutable +open System.Threading +open Xunit +open Microsoft.CodeAnalysis.ExternalAccess.FSharp.NavigateTo +open FSharp.Editor.Tests.Helpers +open FSharp.Test.ProjectGeneration + +let private project = + SyntheticProject.Create( + { sourceFile "First" [] with + ExtraSource = "let sharedFunc funcParam = funcParam * 2\n" + }, + { sourceFile "Second" [ "First" ] with + ExtraSource = "let plainUse x = ModuleFirst.sharedFunc x" + }, + { sourceFile "Third" [ "First" ] with + ExtraSource = "#if FOO\nlet fooUse x = ModuleFirst.sharedFunc x\n#endif" + }, + { sourceFile "Fourth" [ "First" ] with + ExtraSource = "let fooOnlyFileUse x = ModuleFirst.sharedFunc x" + } + ) + +let private solution, instances = + RoslynTestHelpers.CreateMultiTargetSolution( + project, + [ + { + Defines = [] + ExcludedFileIds = [ "Fourth" ] + } + { + Defines = [ "FOO" ] + ExcludedFileIds = [] + } + ] + ) + +let private service: IFSharpNavigateToSearchService = + MefHelpers.createExportProvider().GetExportedValue() + +/// Every instance searched in solution order, as the NavigateTo searcher does. +let private search pattern = + [ + for id in instances do + yield! + service + .SearchProjectAsync( + solution.GetProject id, + ImmutableArray.Empty, + pattern, + service.KindsProvided, + CancellationToken.None + ) + .Result + ] + +[] +[] +[] +[] +let ``a declaration is reported once across the instances of a project`` (name: string) = + let names = search name |> List.map _.Name |> List.filter ((=) name) + Assert.Equal([ name ], names) From 2a006edb4cea5cae62154e3e8403c603903c37bd Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 7 Sep 2026 22:00:42 +0200 Subject: [PATCH 04/10] Add the release note for the NavigateTo multi-target fix Co-Authored-By: Claude Fable 5.1 --- docs/release-notes/.VisualStudio/18.vNext.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index e6034dca8df..066e1735e55 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -20,6 +20,7 @@ * Reduce allocations in the VS project options reactor: the command-line options and project options caches and the mailbox reply payloads now hold struct tuples, and `IProjectSite.CompilationBinOutputPath` returns `string voption` picked with a new `Array.tryPickV`. ([PR #20413](https://github.com/dotnet/fsharp/pull/20413)) * Build a single-file project's `OtherOptions` reference flags with one array comprehension instead of two `Array.ofSeq` calls and an `Array.append`. ([PR #20499](https://github.com/dotnet/fsharp/pull/20499)) * Fix syntax coloring being lost for a whole file when one symbol resolves into metadata that could not be read. ([Issue #20269](https://github.com/dotnet/fsharp/issues/20269), [PR #20274](https://github.com/dotnet/fsharp/pull/20274)) +* Go To All (Ctrl+T) on a multi-targeted F# project no longer parses and reads every file once per target framework: the first instance searches every file, the others only the files they alone compile or that use conditional compilation, and a file's text is read only for a matched declaration. ([PR #20483](https://github.com/dotnet/fsharp/pull/20483)) ### Changed From 148472dbdc10aab353719867f6a8989bd84e6422 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Thu, 10 Sep 2026 02:03:32 +0200 Subject: [PATCH 05/10] Share the parse instead of skipping the file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Skipping a file in every instance but the first is wrong twice over, as the review showed. Go To All scoped to the current project submits only that project, so the instance that was told to stay quiet is the only one asked, and a shared declaration disappears from it once the parse has taught the service the file holds no directives. And the flag that decision reads was keyed by path alone, so an edit that puts a declaration behind `#if` left the previous answer in place. Nothing needs skipping. `NavigateToSearcher` pools its seen set with `NavigateToSearchResultComparer`, which already collapses results by file path and span, so every instance can report what it compiles. What the instances should share is the parse, which is what costs. A parse whose tree holds no conditional directives does not depend on the defines, so it is kept under a key no define set can equal and every instance reuses it; one that does hold them is kept per define set, since those instances genuinely parse the file differently. The entry carries the text version, so an edit is a new entry rather than a stale flag. The test that asserted a declaration is reported once across the instances went with the skip: that is the searcher's job, not this service's. In its place are the two cases from the review — a lone instance reporting a shared declaration, and a declaration an edit puts behind a directive — and one that keeps the define-keyed parse honest by checking such a declaration does not reach the instance without the define. Co-Authored-By: Claude Opus 5 --- docs/release-notes/.VisualStudio/18.vNext.md | 1 + .../Navigation/NavigateToSearchService.fs | 100 ++++++++++-------- .../MultiTargetNavigateToSearchTests.fs | 74 +++++++++---- 3 files changed, 107 insertions(+), 68 deletions(-) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index 066e1735e55..3620750e7bc 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -21,6 +21,7 @@ * Build a single-file project's `OtherOptions` reference flags with one array comprehension instead of two `Array.ofSeq` calls and an `Array.append`. ([PR #20499](https://github.com/dotnet/fsharp/pull/20499)) * Fix syntax coloring being lost for a whole file when one symbol resolves into metadata that could not be read. ([Issue #20269](https://github.com/dotnet/fsharp/issues/20269), [PR #20274](https://github.com/dotnet/fsharp/pull/20274)) * Go To All (Ctrl+T) on a multi-targeted F# project no longer parses and reads every file once per target framework: the first instance searches every file, the others only the files they alone compile or that use conditional compilation, and a file's text is read only for a matched declaration. ([PR #20483](https://github.com/dotnet/fsharp/pull/20483)) +* Go To All (Ctrl+T) on a multi-targeted F# project no longer parses every file once per target framework: a file whose parse holds no conditional directives does not depend on the defines, so every instance reuses the one parse, and a file's text is read only for a matched declaration. ([PR #20483](https://github.com/dotnet/fsharp/pull/20483)) ### Changed diff --git a/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs b/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs index a529739d16d..bc3ef82da74 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs @@ -25,11 +25,27 @@ type internal FSharpNavigateToSearchService [] (patternMatcherFactory: IPatternMatcherFactory, [] workspace: VisualStudioWorkspace) = - let cache = ConcurrentDictionary() - - /// Whether the file's parse depends on the defines, by file path: known once any instance has parsed it. - let conditionalDirectives = - ConcurrentDictionary(StringComparer.OrdinalIgnoreCase) + /// A multi-targeted project is one Roslyn project per target framework over the same files, so the same + /// file is searched once per instance. What that costs is the parse, and a parse whose tree holds no + /// conditional directives does not depend on the defines: it is stored under `AnyDefines` and every + /// instance reuses it. One that does hold them is stored per define set, because those instances + /// genuinely parse the file differently. + /// + /// The duplicate results this produces are not for this service to remove. `NavigateToSearcher` pools its + /// seen set with `NavigateToSearchResultComparer`, which already collapses results by file path and span. + let cache = + ConcurrentDictionary< + struct (string * string), + struct {| + Version: VersionStamp + Items: NavigableItem array + |} + >() + + /// The key for a parse that does not depend on the defines. Not a define set any instance can have, + /// since defines are identifiers — an instance with none of its own must not read this entry as its own. + [] + let AnyDefines = "?" do if workspace <> null then @@ -37,9 +53,8 @@ type internal FSharpNavigateToSearchService <| fun e -> if e.NewSolution.Id <> e.OldSolution.Id then cache.Clear() - conditionalDirectives.Clear() - let hasConditionalDirectives (parseTree: ParsedInput) = + let dependsOnDefines (parseTree: ParsedInput) = match parseTree with | ParsedInput.ImplFile file -> not file.Trivia.ConditionalDirectives.IsEmpty | ParsedInput.SigFile file -> not file.Trivia.ConditionalDirectives.IsEmpty @@ -49,48 +64,40 @@ type internal FSharpNavigateToSearchService let! ct = CancellableTask.getCancellationToken () let! currentVersion = document.GetTextVersionAsync(ct) - match cache.TryGetValue document.Id with - | true, (version, items) when version = currentVersion -> return items - | _ -> + match document.FilePath with + | null -> let! parseResults = document.GetFSharpParseResultsAsync(nameof (FSharpNavigateToSearchService)) - let items = NavigateTo.GetNavigableItems parseResults.ParseTree - cache[document.Id] <- currentVersion, items - - match document.FilePath with - | null -> () - | path -> conditionalDirectives[path] <- hasConditionalDirectives parseResults.ParseTree - - return items + return NavigateTo.GetNavigableItems parseResults.ParseTree + | path -> + let defines = document.GetFSharpQuickDefines() |> String.concat ";" + + let cached key = + match cache.TryGetValue(struct (key, path)) with + | true, entry when entry.Version = currentVersion -> ValueSome entry.Items + | _ -> ValueNone + + match cached AnyDefines, cached defines with + | ValueSome items, _ + | _, ValueSome items -> return items + | ValueNone, ValueNone -> + let! parseResults = document.GetFSharpParseResultsAsync(nameof (FSharpNavigateToSearchService)) + let items = NavigateTo.GetNavigableItems parseResults.ParseTree + + let key = + if dependsOnDefines parseResults.ParseTree then + defines + else + AnyDefines + + cache[struct (key, path)] <- + {| + Version = currentVersion + Items = items + |} + + return items } - /// A multi-targeted project is one Roslyn project per target framework over the same files. The - /// first instance in the solution searches every file; the others only the files they alone compile - /// and the files whose parse depends on the defines. - let searchedIn (project: Project) = - match project.FilePath with - | null -> fun (_: Document) -> true - | projectPath -> - let instances = - project.Solution.Projects - |> Seq.filter (fun p -> p.FilePath = projectPath) - |> Seq.map _.Id - |> List.ofSeq - - fun (document: Document) -> - match document.FilePath with - | null -> true - | path -> - let documentIds = project.Solution.GetDocumentIdsWithFilePath path - - let owner = - instances - |> List.find (fun id -> documentIds |> Seq.exists (fun documentId -> documentId.ProjectId = id)) - - owner = project.Id - || (match conditionalDirectives.TryGetValue path with - | true, dependsOnDefines -> dependsOnDefines - | _ -> true) - let kindsProvided = ImmutableHashSet.Create( FSharpNavigateToItemKind.Module, @@ -238,7 +245,6 @@ type internal FSharpNavigateToSearchService let! results = project.Documents - |> Seq.filter (searchedIn project) |> Seq.map (processDocument tryMatch kinds) // Throttle to avoid launching a parse per document in the project all at once. |> CancellableTask.whenAllThrottled (max 1 Environment.ProcessorCount) diff --git a/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs b/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs index c1febedfd5e..65c1dc1c3eb 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs @@ -7,6 +7,8 @@ module FSharp.Editor.Tests.MultiTargetNavigateToSearchTests open System.Collections.Immutable open System.Threading open Xunit +open Microsoft.CodeAnalysis +open Microsoft.CodeAnalysis.Text open Microsoft.CodeAnalysis.ExternalAccess.FSharp.NavigateTo open FSharp.Editor.Tests.Helpers open FSharp.Test.ProjectGeneration @@ -45,26 +47,56 @@ let private solution, instances = let private service: IFSharpNavigateToSearchService = MefHelpers.createExportProvider().GetExportedValue() -/// Every instance searched in solution order, as the NavigateTo searcher does. -let private search pattern = - [ - for id in instances do - yield! - service - .SearchProjectAsync( - solution.GetProject id, - ImmutableArray.Empty, - pattern, - service.KindsProvided, - CancellationToken.None - ) - .Result - ] +let private searchIn (project: Project) pattern = + service.SearchProjectAsync(project, ImmutableArray.Empty, pattern, service.KindsProvided, CancellationToken.None).Result + |> Seq.map _.Name + |> Seq.filter ((=) pattern) + |> Seq.toList +let private documentNamed (name: string) (project: Project) = + project.Documents |> Seq.find (fun document -> document.Name.Contains name) + +/// Roslyn submits only the active project when the search is scoped to the current one, so an instance has +/// to report what it compiles even when a sibling compiles the same file. The second search is the one that +/// used to come back empty: by then the file had been parsed, and the parse said it did not depend on the +/// defines. [] -[] -[] -[] -let ``a declaration is reported once across the instances of a project`` (name: string) = - let names = search name |> List.map _.Name |> List.filter ((=) name) - Assert.Equal([ name ], names) +[] +[] +let ``an instance reports a shared declaration on its own, and again once the parse is cached`` (instance: int) = + let project = solution.GetProject instances[instance] + + Assert.Equal([ "plainUse" ], searchIn project "plainUse") + Assert.Equal([ "plainUse" ], searchIn project "plainUse") + +/// The parse of a file that does hold directives is kept per define set, so reusing it across the instances +/// must not leak the declaration into the instance that does not define FOO. +[] +let ``a declaration behind a directive is reported only by the instance that defines it`` () = + let plain = solution.GetProject instances[0] + let foo = solution.GetProject instances[1] + + Assert.Equal([ "fooUse" ], searchIn foo "fooUse") + Assert.Equal([], searchIn plain "fooUse") + +/// A file with no directives has its parse shared by every instance. When an edit gives it one, that shared +/// parse is stale: the entry is keyed by the text version, so the edit is a different entry rather than a +/// flag left over from before. +[] +let ``a declaration an edit puts behind a directive is reported by the instance that defines it`` () = + let edited = + (solution, instances) + ||> Seq.fold (fun (solution: Solution) instance -> + let document = solution.GetProject instance |> documentNamed "Second" + + solution.WithDocumentText( + document.Id, + SourceText.From "module ModuleSecond\n#if FOO\nlet addedFoo = 1\n#endif\n" + )) + + let foo = edited.GetProject instances[1] + let plain = edited.GetProject instances[0] + + // The instance without FOO goes first: that order is what left the stale answer behind. + Assert.Equal([], searchIn plain "addedFoo") + Assert.Equal([ "addedFoo" ], searchIn foo "addedFoo") From e4cf54094a7aae95db1aaf2894a674646b5721ac Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Fri, 11 Sep 2026 03:24:58 +0200 Subject: [PATCH 06/10] Name the navigable items cache's key and entry The cache was a dictionary from a struct tuple of two strings to an anonymous struct record, spelled out in the type arguments at the one place it is declared, and read back as `struct (key, path)` wherever it is used. Nothing at those sites said which string was the defines and which the path. Give both a name at the top of the file. They stay struct records, so the key keeps the same equality and the entries allocate no more than before. Co-Authored-By: Claude Opus 5 --- .../Navigation/NavigateToSearchService.fs | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs b/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs index bc3ef82da74..fc90d787204 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs @@ -20,6 +20,19 @@ open FSharp.Compiler.EditorServices open FSharp.Compiler.Syntax open CancellableTasks +/// Where a parse of a file is kept: under the defines it was parsed with, or under `AnyDefines` when its tree +/// holds no conditional directives and so reads the same under any of them. +[] +type private NavigableItemsKey = { Defines: string; FilePath: string } + +/// The navigable items of one parse of a file, and the text version it was taken from. +[] +type private NavigableItemsEntry = + { + Version: VersionStamp + Items: NavigableItem array + } + [); Shared>] type internal FSharpNavigateToSearchService [] @@ -33,14 +46,7 @@ type internal FSharpNavigateToSearchService /// /// The duplicate results this produces are not for this service to remove. `NavigateToSearcher` pools its /// seen set with `NavigateToSearchResultComparer`, which already collapses results by file path and span. - let cache = - ConcurrentDictionary< - struct (string * string), - struct {| - Version: VersionStamp - Items: NavigableItem array - |} - >() + let cache = ConcurrentDictionary() /// The key for a parse that does not depend on the defines. Not a define set any instance can have, /// since defines are identifiers — an instance with none of its own must not read this entry as its own. @@ -72,7 +78,7 @@ type internal FSharpNavigateToSearchService let defines = document.GetFSharpQuickDefines() |> String.concat ";" let cached key = - match cache.TryGetValue(struct (key, path)) with + match cache.TryGetValue({ Defines = key; FilePath = path }) with | true, entry when entry.Version = currentVersion -> ValueSome entry.Items | _ -> ValueNone @@ -89,11 +95,11 @@ type internal FSharpNavigateToSearchService else AnyDefines - cache[struct (key, path)] <- - {| + cache[{ Defines = key; FilePath = path }] <- + { Version = currentVersion Items = items - |} + } return items } From 170fc761d87cde41afc96d21c03fbf0d1ad4d366 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Fri, 11 Sep 2026 04:24:15 +0200 Subject: [PATCH 07/10] Format the multi-target NavigateTo tests Co-Authored-By: Claude Opus 5 --- .../FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs b/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs index 65c1dc1c3eb..a8074d1de06 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs @@ -89,10 +89,7 @@ let ``a declaration an edit puts behind a directive is reported by the instance ||> Seq.fold (fun (solution: Solution) instance -> let document = solution.GetProject instance |> documentNamed "Second" - solution.WithDocumentText( - document.Id, - SourceText.From "module ModuleSecond\n#if FOO\nlet addedFoo = 1\n#endif\n" - )) + solution.WithDocumentText(document.Id, SourceText.From "module ModuleSecond\n#if FOO\nlet addedFoo = 1\n#endif\n")) let foo = edited.GetProject instances[1] let plain = edited.GetProject instances[0] From 48fc560bcf88ac5c47d7d8c678cf262976d01509 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Fri, 11 Sep 2026 17:40:05 +0200 Subject: [PATCH 08/10] Build the synthetic projects' id list through Seq and materialize once Co-Authored-By: Claude Opus 5 --- .../tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs index 89a449eceb7..fcc6c341868 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs @@ -382,8 +382,9 @@ type RoslynTestHelpers private () = let projects = syntheticProject.GetAllProjects() - |> List.distinctBy _.Name - |> List.map (fun project -> project, ProjectId.CreateNewId()) + |> Seq.distinctBy _.Name + |> Seq.map (fun project -> project, ProjectId.CreateNewId()) + |> Seq.toList let projectIds = dict [ for project, id in projects -> project.Name, id ] From 6e68aace9ebd1eddc962b60a1d30849556ac6f48 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sat, 26 Sep 2026 13:38:36 +0200 Subject: [PATCH 09/10] Key a kept parse by everything it depends on The defines a file was parsed with are not the whole of what its tree depends on: the language version decides what the parser accepts, and the defines were taken from the editing defaults when the project had not produced its options yet - a set another project really has, so its parse answered for a project that never asked for it, with the branch of a `#if` that project chose. The key now carries the language version, and both it and the defines are read only where the project has produced them: a project without options parses for itself instead of reading an entry that is not about it. The rest of the parsing options cannot differ for one path - the editor never applies line directives, and whether a file is interactive follows from its extension. Co-Authored-By: Claude Opus 5 (1M context) --- .../FSharpProjectOptionsManager.fs | 9 ++++ .../LanguageService/WorkspaceExtensions.fs | 7 +++ .../Navigation/NavigateToSearchService.fs | 45 +++++++++++----- .../NavigateToSearchServiceTests.fs | 51 +++++++++++++++++++ 4 files changed, 99 insertions(+), 13 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs index db73206996b..0666b7c4b50 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs @@ -612,6 +612,15 @@ type internal FSharpProjectOptionsManager(checker: FSharpChecker, workspace: Wor CompilerEnvironment.GetConditionalDefinesForEditing parsingOptions, parsingOptions.LangVersionText + /// The defines and language version the document's project has produced, and nothing where it has not + /// produced its options yet: what the editing defaults answer there is a set no project asked for, so + /// anything kept under it belongs to no project either. + member _.TryGetCompilationDefinesAndLangVersionForEditingDocument(document: Document) = + match reactor.TryGetCachedOptionsByProjectId(document.Project.Id) with + | Some(_, parsingOptions, _) -> + ValueSome(struct (CompilerEnvironment.GetConditionalDefinesForEditing parsingOptions, parsingOptions.LangVersionText)) + | _ -> ValueNone + member _.TryGetOptionsByProject(project) = reactor.TryGetOptionsByProjectAsync(project) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs b/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs index 2406f3a6e32..c2f64af6c11 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs @@ -564,6 +564,13 @@ type Document with let workspaceService = this.Project.Solution.GetFSharpWorkspaceService() workspaceService.FSharpProjectOptionsManager.GetCompilationDefinesAndLangVersionForEditingDocument(this) + /// A non-async call that gets the defines and F# language version of the given F# document, and nothing + /// where its project has not produced its options yet: a guess names what the project never asked for, + /// which a parse must not be kept under. + member this.TryGetFSharpParsingOptionsData() = + let workspaceService = this.Project.Solution.GetFSharpWorkspaceService() + workspaceService.FSharpProjectOptionsManager.TryGetCompilationDefinesAndLangVersionForEditingDocument(this) + /// A non-async call that quickly gets the defines of the given F# document. /// This tries to get the defines by looking at an internal cache; if it doesn't exist in the cache it will create an inaccurate but usable form of the defines. member this.GetFSharpQuickDefines() = diff --git a/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs b/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs index fc90d787204..406c2569274 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs @@ -20,10 +20,18 @@ open FSharp.Compiler.EditorServices open FSharp.Compiler.Syntax open CancellableTasks -/// Where a parse of a file is kept: under the defines it was parsed with, or under `AnyDefines` when its tree -/// holds no conditional directives and so reads the same under any of them. +/// Where a parse of a file is kept: under what the parse depends on besides the text. The defines are those it +/// was parsed with, or `AnyDefines` when its tree holds no conditional directives and so reads the same under +/// any of them; the language version decides what the parser accepts and is never shared across versions. The +/// remaining parsing options cannot differ for one path: the editor never applies line directives, and whether +/// a file is interactive follows from its own extension. [] -type private NavigableItemsKey = { Defines: string; FilePath: string } +type private NavigableItemsKey = + { + Defines: string + LangVersion: string + FilePath: string + } /// The navigable items of one parse of a file, and the text version it was taken from. [] @@ -70,15 +78,26 @@ type internal FSharpNavigateToSearchService let! ct = CancellableTask.getCancellationToken () let! currentVersion = document.GetTextVersionAsync(ct) - match document.FilePath with - | null -> + match document.FilePath, document.TryGetFSharpParsingOptionsData() with + // A document with no path is keyed by nothing, and one whose project has not produced its options + // yet knows neither the defines nor the language version its own parse depends on - an entry + // another instance wrote under what it really parsed with must not answer for it. + | null, _ + | _, ValueNone -> let! parseResults = document.GetFSharpParseResultsAsync(nameof (FSharpNavigateToSearchService)) return NavigateTo.GetNavigableItems parseResults.ParseTree - | path -> - let defines = document.GetFSharpQuickDefines() |> String.concat ";" - - let cached key = - match cache.TryGetValue({ Defines = key; FilePath = path }) with + | path, ValueSome(struct (documentDefines, langVersion)) -> + let defines = documentDefines |> String.concat ";" + + let keyOf defines = + { + Defines = defines + LangVersion = langVersion + FilePath = path + } + + let cached defines = + match cache.TryGetValue(keyOf defines) with | true, entry when entry.Version = currentVersion -> ValueSome entry.Items | _ -> ValueNone @@ -91,11 +110,11 @@ type internal FSharpNavigateToSearchService let key = if dependsOnDefines parseResults.ParseTree then - defines + keyOf defines else - AnyDefines + keyOf AnyDefines - cache[{ Defines = key; FilePath = path }] <- + cache[key] <- { Version = currentVersion Items = items diff --git a/vsintegration/tests/FSharp.Editor.Tests/NavigateToSearchServiceTests.fs b/vsintegration/tests/FSharp.Editor.Tests/NavigateToSearchServiceTests.fs index 08816ea6191..40960d9342c 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/NavigateToSearchServiceTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/NavigateToSearchServiceTests.fs @@ -3,7 +3,9 @@ namespace FSharp.Editor.Tests open Xunit +open System.Collections.Immutable open System.Threading +open Microsoft.CodeAnalysis open Microsoft.CodeAnalysis.Text open Microsoft.VisualStudio.FSharp.Editor open FSharp.Compiler.Text @@ -72,3 +74,52 @@ module HeyHo = [] let ``nested containers`` () = assertResultsContain "hh.a.b.g.d" "Delta" + + /// A project whose options have not arrived knows neither the defines nor the language version its own + /// parse depends on. The editing defaults it would otherwise be keyed under are a define set a real + /// project has — the one that defines nothing of its own — so keying it that way lets it answer from that + /// project's parse, with the branch of a `#if` that project chose. It asks for the parse it cannot have. + [] + let ``a project whose options have not arrived does not answer from another project's parse`` () = + let path = "C:\Shared.fs" + let source = "module Shared\n#if FOO\nlet fooOnly = 1\n#endif\nlet always = 2\n" + + // One loader for both copies of the file, as a linked file open in the editor has: the two documents + // then report the same text version, which is what lets one project read the other's parse at all. + let loader = + TextLoader.From(SourceText.From(source).Container, VersionStamp.Create()) + + let projectOf name = + let projectId = ProjectId.CreateNewId() + + projectId, + [ + DocumentInfo.Create(DocumentId.CreateNewId projectId, path, loader = loader, filePath = path) + ] + |> RoslynTestHelpers.CreateProjectInfo projectId $"C:\{name}.fsproj" + + let definesNothingId, definesNothing = projectOf "DefinesNothing" + let noOptionsId, noOptions = projectOf "NoOptions" + + let solution = RoslynTestHelpers.CreateSolution [ definesNothing; noOptions ] + + { RoslynTestHelpers.DefaultProjectOptions with + SourceFiles = [| path |] + } + |> RoslynTestHelpers.SetProjectOptions definesNothingId solution + + let service: IFSharpNavigateToSearchService = provider.GetExportedValue() + + let search (project: Project) pattern = + service.SearchProjectAsync(project, ImmutableArray.Empty, pattern, service.KindsProvided, CancellationToken.None).Result + |> Seq.map _.Name + |> Seq.filter ((=) pattern) + |> Seq.toList + + // Parses the file with no defines and keeps that parse under them. + Assert.Equal([ "always" ], search (solution.GetProject definesNothingId) "always") + + let searching = + Assert.ThrowsAny(fun () -> search (solution.GetProject noOptionsId) "always" |> ignore) + + Assert.IsAssignableFrom(searching.GetBaseException()) From 840c454ce56b6dae72628f7fdb5e1a4541576b96 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sat, 26 Sep 2026 14:05:50 +0200 Subject: [PATCH 10/10] Keep the release note that says what the code does The note for skipping files stayed behind when sharing the parse replaced it. Co-Authored-By: Claude Opus 5 (1M context) --- docs/release-notes/.VisualStudio/18.vNext.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index 3620750e7bc..f54a4e19408 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -20,7 +20,6 @@ * Reduce allocations in the VS project options reactor: the command-line options and project options caches and the mailbox reply payloads now hold struct tuples, and `IProjectSite.CompilationBinOutputPath` returns `string voption` picked with a new `Array.tryPickV`. ([PR #20413](https://github.com/dotnet/fsharp/pull/20413)) * Build a single-file project's `OtherOptions` reference flags with one array comprehension instead of two `Array.ofSeq` calls and an `Array.append`. ([PR #20499](https://github.com/dotnet/fsharp/pull/20499)) * Fix syntax coloring being lost for a whole file when one symbol resolves into metadata that could not be read. ([Issue #20269](https://github.com/dotnet/fsharp/issues/20269), [PR #20274](https://github.com/dotnet/fsharp/pull/20274)) -* Go To All (Ctrl+T) on a multi-targeted F# project no longer parses and reads every file once per target framework: the first instance searches every file, the others only the files they alone compile or that use conditional compilation, and a file's text is read only for a matched declaration. ([PR #20483](https://github.com/dotnet/fsharp/pull/20483)) * Go To All (Ctrl+T) on a multi-targeted F# project no longer parses every file once per target framework: a file whose parse holds no conditional directives does not depend on the defines, so every instance reuses the one parse, and a file's text is read only for a matched declaration. ([PR #20483](https://github.com/dotnet/fsharp/pull/20483)) ### Changed