From 6d5f4cc232eefb36aa88c993bf31320aa1565add Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 09:42:41 +0200 Subject: [PATCH 1/5] 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 f5467d90a2fa9bb0b8292037279a4f88fc1b5705 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 7 Sep 2026 20:15:02 +0200 Subject: [PATCH 2/5] 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 51d45190b5cb98bd934d2ed39208857f79795f77 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 7 Sep 2026 20:15:04 +0200 Subject: [PATCH 3/5] 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 a7e670e4904969c6defa8ce01a2669971b9bb5f3 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 7 Sep 2026 22:00:42 +0200 Subject: [PATCH 4/5] 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 ba03f663967..01793b5dc3f 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -15,6 +15,7 @@ * Fix doubled F# diagnostics in tooltips. ([Issue #16360](https://github.com/dotnet/fsharp/issues/16360)) * Fix `NotSupportedException` in the memory-mapped-file optimization when copying `ReadOnlyMemory` into `MemoryMappedFileViewStream`. ([Issue #20263](https://github.com/dotnet/fsharp/issues/20263)) * 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)) +* 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 6daf31f730f6358af0a2920131aee71d856bbec8 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Thu, 10 Sep 2026 02:03:32 +0200 Subject: [PATCH 5/5] 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 | 2 +- .../Navigation/NavigateToSearchService.fs | 100 ++++++++++-------- .../MultiTargetNavigateToSearchTests.fs | 74 +++++++++---- 3 files changed, 107 insertions(+), 69 deletions(-) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index 01793b5dc3f..66cfd8a81a7 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -15,7 +15,7 @@ * Fix doubled F# diagnostics in tooltips. ([Issue #16360](https://github.com/dotnet/fsharp/issues/16360)) * Fix `NotSupportedException` in the memory-mapped-file optimization when copying `ReadOnlyMemory` into `MemoryMappedFileViewStream`. ([Issue #20263](https://github.com/dotnet/fsharp/issues/20263)) * 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)) -* 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")