From bf970ade3553c7f33ab225cb056eb0153604c4a6 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 09:42:41 +0200 Subject: [PATCH 01/16] 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 | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs index 72e5ac9a27d..89a449eceb7 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs @@ -203,7 +203,6 @@ type TestHostServices() = /// 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 @@ -383,9 +382,8 @@ type RoslynTestHelpers private () = let projects = syntheticProject.GetAllProjects() - |> Seq.distinctBy _.Name - |> Seq.map (fun project -> struct (project, ProjectId.CreateNewId())) - |> Seq.toArray + |> List.distinctBy _.Name + |> List.map (fun project -> project, ProjectId.CreateNewId()) let projectIds = dict [ for project, id in projects -> project.Name, id ] @@ -415,15 +413,10 @@ type RoslynTestHelpers private () = project.GetProjectOptions checker |> RoslynTestHelpers.SetProjectOptions id solution - struct (solution, checker) + 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. - /// - /// The project every instance is made from; it must not depend on other projects. - /// One Roslyn project per entry: its extra defines and the synthetic files left out of it. - /// The solution and the id of each instance's project, in the order of . static member CreateMultiTargetSolution(syntheticProject: SyntheticProject, instances: TargetInstance list) = assert (syntheticProject.DependsOn = []) @@ -436,13 +429,13 @@ type RoslynTestHelpers private () = for instance in instances -> let excludedPaths = HashSet( - seq { + [ for fileId in instance.ExcludedFileIds do syntheticProject.GetFilePath fileId if (syntheticProject.Find fileId).HasSignatureFile then syntheticProject.GetSignatureFilePath fileId - }, + ], StringComparer.OrdinalIgnoreCase ) @@ -474,7 +467,7 @@ type RoslynTestHelpers private () = |] } - struct (id, projectInfo, instanceOptions) + id, projectInfo, instanceOptions ] let solution = @@ -483,7 +476,7 @@ type RoslynTestHelpers private () = for id, _, instanceOptions in instances do RoslynTestHelpers.SetProjectOptions id solution instanceOptions - struct (solution, [ for id, _, _ in instances -> id ]) + solution, [ for id, _, _ in instances -> id ] static member GetFsDocument(code, ?customProjectOption: string, ?customEditorOptions) = let customProjectOptions = From 2e0607102ce8adbf0da4f8544c3bb13b3c6d4b6d Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 7 Sep 2026 20:15:02 +0200 Subject: [PATCH 02/16] 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 55e6038c2fc8e5d0b02af1668c5eff1bb3d3da20 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 7 Sep 2026 20:15:04 +0200 Subject: [PATCH 03/16] 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 a62b25af1cf..8dd66408076 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj +++ b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj @@ -33,6 +33,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 cc25922624a43c67741a4f593e1c66eeced6d919 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 7 Sep 2026 22:00:42 +0200 Subject: [PATCH 04/16] 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 8f47e2e0dbe..9d4b03afe21 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -23,6 +23,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 Definition into an F# project built with a path map (`DeterministicSourcePaths` or `PathMap`) opens its source instead of a generated signature: the IDE no longer applies `--pathmap` to the project options it checks with. ([PR #20470](https://github.com/dotnet/fsharp/pull/20470)) +* 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 b4ef42f4a7381662325fd154f38371b9c0bd58bd Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Thu, 10 Sep 2026 02:03:32 +0200 Subject: [PATCH 05/16] 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 9d4b03afe21..fb329f65858 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -24,6 +24,7 @@ * 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 Definition into an F# project built with a path map (`DeterministicSourcePaths` or `PathMap`) opens its source instead of a generated signature: the IDE no longer applies `--pathmap` to the project options it checks with. ([PR #20470](https://github.com/dotnet/fsharp/pull/20470)) * 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 73fd328cc6869beb95515aece8cbbf234b123b8d Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Fri, 11 Sep 2026 03:24:58 +0200 Subject: [PATCH 06/16] 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 8f83b179e14212c228683d38c1124016c3520e91 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Fri, 11 Sep 2026 04:24:15 +0200 Subject: [PATCH 07/16] 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 9856ebf5e410983b1a1103ca839ae712c976ea7a Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Fri, 11 Sep 2026 17:40:05 +0200 Subject: [PATCH 08/16] 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 5cd23d7d5e72fd46a8d450cbc9f9baeb52898583 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sat, 26 Sep 2026 13:38:36 +0200 Subject: [PATCH 09/16] 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 f5ea528a9e6..2fba06c0f5d 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs @@ -646,6 +646,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 f381a3d59fbd0f9340afdeeb6748944c691603ef Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Thu, 1 Oct 2026 19:16:33 +0200 Subject: [PATCH 10/16] Take the shared test fixture as #20462 lands it The multi-project and multi-target helpers now match the ones #20462 adds, struct results included, so this branch rebases onto it without reconciling two copies. A struct tuple cannot be bound by a `let private` pattern, so the module-level fixture bindings lose the modifier. Co-Authored-By: Claude Opus 5.5 --- .../Helpers/RoslynHelpers.fs | 20 ++++++++++++------- .../MultiTargetNavigateToSearchTests.fs | 2 +- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs index fcc6c341868..72e5ac9a27d 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs @@ -203,6 +203,7 @@ type TestHostServices() = /// 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 @@ -383,8 +384,8 @@ type RoslynTestHelpers private () = let projects = syntheticProject.GetAllProjects() |> Seq.distinctBy _.Name - |> Seq.map (fun project -> project, ProjectId.CreateNewId()) - |> Seq.toList + |> Seq.map (fun project -> struct (project, ProjectId.CreateNewId())) + |> Seq.toArray let projectIds = dict [ for project, id in projects -> project.Name, id ] @@ -414,10 +415,15 @@ type RoslynTestHelpers private () = project.GetProjectOptions checker |> RoslynTestHelpers.SetProjectOptions id solution - solution, checker + struct (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. + /// + /// The project every instance is made from; it must not depend on other projects. + /// One Roslyn project per entry: its extra defines and the synthetic files left out of it. + /// The solution and the id of each instance's project, in the order of . static member CreateMultiTargetSolution(syntheticProject: SyntheticProject, instances: TargetInstance list) = assert (syntheticProject.DependsOn = []) @@ -430,13 +436,13 @@ type RoslynTestHelpers private () = for instance in instances -> let excludedPaths = HashSet( - [ + seq { for fileId in instance.ExcludedFileIds do syntheticProject.GetFilePath fileId if (syntheticProject.Find fileId).HasSignatureFile then syntheticProject.GetSignatureFilePath fileId - ], + }, StringComparer.OrdinalIgnoreCase ) @@ -468,7 +474,7 @@ type RoslynTestHelpers private () = |] } - id, projectInfo, instanceOptions + struct (id, projectInfo, instanceOptions) ] let solution = @@ -477,7 +483,7 @@ type RoslynTestHelpers private () = for id, _, instanceOptions in instances do RoslynTestHelpers.SetProjectOptions id solution instanceOptions - solution, [ for id, _, _ in instances -> id ] + struct (solution, [ for id, _, _ in instances -> id ]) static member GetFsDocument(code, ?customProjectOption: string, ?customEditorOptions) = let customProjectOptions = diff --git a/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs b/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs index a8074d1de06..f659ca0d77b 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs @@ -29,7 +29,7 @@ let private project = } ) -let private solution, instances = +let struct (solution, instances) = RoslynTestHelpers.CreateMultiTargetSolution( project, [ From e643efe2a9c17b483abb20fd0326bcbcb27dda04 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Thu, 1 Oct 2026 22:22:29 +0200 Subject: [PATCH 11/16] Keep the shared fixture's results private A struct tuple cannot be bound with `let private` (FS0010: the grammar takes an access modifier only before a name), so the binding sits in a private AutoOpen module: the tests here see both names, nothing outside does. Co-Authored-By: Claude Opus 5.5 --- .../MultiTargetNavigateToSearchTests.fs | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs b/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs index f659ca0d77b..715c75dc5ea 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs @@ -29,20 +29,22 @@ let private project = } ) -let struct (solution, instances) = - RoslynTestHelpers.CreateMultiTargetSolution( - project, - [ - { - Defines = [] - ExcludedFileIds = [ "Fourth" ] - } - { - Defines = [ "FOO" ] - ExcludedFileIds = [] - } - ] - ) +[] +module private Fixture = + let struct (solution, instances) = + RoslynTestHelpers.CreateMultiTargetSolution( + project, + [ + { + Defines = [] + ExcludedFileIds = [ "Fourth" ] + } + { + Defines = [ "FOO" ] + ExcludedFileIds = [] + } + ] + ) let private service: IFSharpNavigateToSearchService = MefHelpers.createExportProvider().GetExportedValue() From 462822d4a17fd6b4c912118c062ec5abf711d14e Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 11 Oct 2026 02:07:23 +0200 Subject: [PATCH 12/16] Keep a document's own parse where its project has no options A standalone script has its options kept per document, so its project never produces the defines and language version a parse is keyed by, and every search of the unchanged script asked for the parse again. Such a document's navigable items are now kept under its DocumentId with the text version. Nothing is shared across projects, and a project whose first parse makes it produce its options is still keyed by them from the next search on. Co-Authored-By: Claude Opus 5.5 --- .../Navigation/NavigateToSearchService.fs | 24 +++++++++- .../NavigateToSearchServiceTests.fs | 47 +++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs b/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs index 406c2569274..473bf5d5b8c 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs @@ -56,6 +56,11 @@ type internal FSharpNavigateToSearchService /// seen set with `NavigateToSearchResultComparer`, which already collapses results by file path and span. let cache = ConcurrentDictionary() + /// The parses of documents whose options are kept per document rather than per project - a standalone + /// script, a file outside any project - and so name no key in `cache`. Each is kept under its own + /// document and answers for nothing else. + let documentCache = 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. [] @@ -67,6 +72,7 @@ type internal FSharpNavigateToSearchService <| fun e -> if e.NewSolution.Id <> e.OldSolution.Id then cache.Clear() + documentCache.Clear() let dependsOnDefines (parseTree: ParsedInput) = match parseTree with @@ -84,8 +90,22 @@ type internal FSharpNavigateToSearchService // 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 + match documentCache.TryGetValue document.Id with + | true, entry when entry.Version = currentVersion -> return entry.Items + | _ -> + let! parseResults = document.GetFSharpParseResultsAsync(nameof (FSharpNavigateToSearchService)) + let items = NavigateTo.GetNavigableItems parseResults.ParseTree + + // A project's first parse is what makes it produce its options, and the next search keys the + // document by them: only a document that still has none is kept under its own id. + if document.TryGetFSharpParsingOptionsData().IsNone then + documentCache[document.Id] <- + { + Version = currentVersion + Items = items + } + + return items | path, ValueSome(struct (documentDefines, langVersion)) -> let defines = documentDefines |> String.concat ";" diff --git a/vsintegration/tests/FSharp.Editor.Tests/NavigateToSearchServiceTests.fs b/vsintegration/tests/FSharp.Editor.Tests/NavigateToSearchServiceTests.fs index 40960d9342c..c6bcacf716c 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/NavigateToSearchServiceTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/NavigateToSearchServiceTests.fs @@ -4,6 +4,7 @@ namespace FSharp.Editor.Tests open Xunit open System.Collections.Immutable +open System.Diagnostics open System.Threading open Microsoft.CodeAnalysis open Microsoft.CodeAnalysis.Text @@ -123,3 +124,49 @@ module HeyHo = Assert.ThrowsAny(fun () -> search (solution.GetProject noOptionsId) "always" |> ignore) Assert.IsAssignableFrom(searching.GetBaseException()) + + /// A standalone script has its options kept per document, so its project never has any to key the script's + /// parse by. The parse is kept under the document itself instead. + [] + let ``a repeated search of an unchanged standalone script does not parse it again`` () = + let path = "C:\\Standalone.fsx" + let projectId = ProjectId.CreateNewId() + + let solution = + [ + RoslynTestHelpers.CreateDocumentInfo projectId path "let standaloneValue = 1\n" + ] + |> RoslynTestHelpers.CreateProjectInfo projectId FSharpConstants.FSharpMiscellaneousFilesName + |> List.singleton + |> RoslynTestHelpers.CreateSolution + + let parseRequests = ref 0 + + use listener = + new ActivityListener( + ShouldListenTo = (fun source -> source.Name = FSharp.Compiler.Diagnostics.ActivityNames.FscSourceName), + Sample = (fun _ -> ActivitySamplingResult.AllData), + ActivityStarted = + (fun activity -> + if + activity.OperationName = "BackgroundCompiler.ParseFile" + && activity.Tags |> Seq.exists (fun tag -> tag.Value = path) + then + Interlocked.Increment(&parseRequests.contents) |> ignore) + ) + + ActivitySource.AddActivityListener listener + + let service: IFSharpNavigateToSearchService = provider.GetExportedValue() + let project = solution.GetProject projectId + + let search () = + service + .SearchProjectAsync(project, ImmutableArray.Empty, "standaloneValue", service.KindsProvided, CancellationToken.None) + .Result + |> Seq.map _.Name + |> Seq.toList + + Assert.Equal([ "standaloneValue" ], search ()) + Assert.Equal([ "standaloneValue" ], search ()) + Assert.Equal(1, parseRequests.Value) From a118991c21b83ad725691c6cba0353c96805c1c6 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 11 Oct 2026 02:07:24 +0200 Subject: [PATCH 13/16] Drop the release note of the approach the parse sharing replaced The rebase onto main brought the bullet back beside the one that describes what the PR does now. Co-Authored-By: Claude Opus 5.5 --- 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 fb329f65858..dc83e2d9107 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -23,7 +23,6 @@ * 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 Definition into an F# project built with a path map (`DeterministicSourcePaths` or `PathMap`) opens its source instead of a generated signature: the IDE no longer applies `--pathmap` to the project options it checks with. ([PR #20470](https://github.com/dotnet/fsharp/pull/20470)) -* 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 From b868c623927fadd8eb0d32989627d3ea76d6cab3 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Thu, 15 Oct 2026 06:35:59 +0100 Subject: [PATCH 14/16] Key Navigate To's parse cache by the options the parse uses The cache key is built from the parsing options the document's parse is made with: the ones the options reactor already holds, else the awaited ones. That removes the per-document cache, the cold-project branch, the sentinel define set and the forwarding members for the options peek, and lets the key say in its type whether a parse depends on the defines. Co-Authored-By: Claude Sonnet 5.5 --- docs/release-notes/.VisualStudio/18.vNext.md | 2 +- .../FSharpProjectOptionsManager.fs | 10 +- .../LanguageService/WorkspaceExtensions.fs | 7 - .../Navigation/NavigateToSearchService.fs | 170 +++++++----------- 4 files changed, 63 insertions(+), 126 deletions(-) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index dc83e2d9107..f89056b8fe0 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -23,7 +23,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 Definition into an F# project built with a path map (`DeterministicSourcePaths` or `PathMap`) opens its source instead of a generated signature: the IDE no longer applies `--pathmap` to the project options it checks with. ([PR #20470](https://github.com/dotnet/fsharp/pull/20470)) -* 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)) +* Go To All (Ctrl+T) on a multi-targeted F# project no longer parses every file once per target framework. ([PR #20483](https://github.com/dotnet/fsharp/pull/20483)) ### Changed diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs index 2fba06c0f5d..a2a2865f15c 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs @@ -646,14 +646,8 @@ 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 _.TryGetCachedOptionsByProjectId(projectId) = + reactor.TryGetCachedOptionsByProjectId(projectId) 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 c2f64af6c11..2406f3a6e32 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs @@ -564,13 +564,6 @@ 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 473bf5d5b8c..3022635ed9f 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs @@ -16,55 +16,22 @@ open Microsoft.CodeAnalysis.ExternalAccess.FSharp.NavigateTo open Microsoft.VisualStudio.LanguageServices open Microsoft.VisualStudio.Text.PatternMatching +open FSharp.Compiler open FSharp.Compiler.EditorServices open FSharp.Compiler.Syntax open CancellableTasks -/// 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 - LangVersion: 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 - } +type private ParseKey = + | AnyDefines of filePath: string * langVersion: string + | DefinedAs of filePath: string * langVersion: string * defines: string list [); Shared>] type internal FSharpNavigateToSearchService [] (patternMatcherFactory: IPatternMatcherFactory, [] workspace: VisualStudioWorkspace) = - /// 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() - - /// The parses of documents whose options are kept per document rather than per project - a standalone - /// script, a file outside any project - and so name no key in `cache`. Each is kept under its own - /// document and answers for nothing else. - let documentCache = 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. - [] - let AnyDefines = "?" + let cache = ConcurrentDictionary() do if workspace <> null then @@ -72,75 +39,50 @@ type internal FSharpNavigateToSearchService <| fun e -> if e.NewSolution.Id <> e.OldSolution.Id then cache.Clear() - documentCache.Clear() - let dependsOnDefines (parseTree: ParsedInput) = - match parseTree with - | ParsedInput.ImplFile file -> not file.Trivia.ConditionalDirectives.IsEmpty - | ParsedInput.SigFile file -> not file.Trivia.ConditionalDirectives.IsEmpty + // Awaiting a project's options recomputes them after an edit to a C# project it references. + let parsingOptionsOf (document: Document) = + let optionsManager = + document.Project.Solution.GetFSharpWorkspaceService().FSharpProjectOptionsManager + + match optionsManager.TryGetCachedOptionsByProjectId document.Project.Id with + | Some(_, parsingOptions, _) -> CancellableTask.singleton parsingOptions + | None -> + document.GetFSharpCompilationOptionsAsync(nameof (FSharpNavigateToSearchService)) + |> CancellableTask.map (fun (_, _, parsingOptions, _) -> parsingOptions) + + let storedUnder anyDefinesKey definedAsKey = + function + | ParsedInput.ImplFile(ParsedImplFileInput(trivia = trivia)) + | ParsedInput.SigFile(ParsedSigFileInput(trivia = trivia)) when trivia.ConditionalDirectives.IsEmpty -> anyDefinesKey + | _ -> definedAsKey let getNavigableItems (document: Document) = cancellableTask { let! ct = CancellableTask.getCancellationToken () let! currentVersion = document.GetTextVersionAsync(ct) + let! parsingOptions = parsingOptionsOf document + let anyDefinesKey = AnyDefines(document.FilePath, parsingOptions.LangVersionText) + + let definedAsKey = + DefinedAs( + document.FilePath, + parsingOptions.LangVersionText, + CompilerEnvironment.GetConditionalDefinesForEditing parsingOptions + ) + + let cached key = + match cache.TryGetValue key with + | true, (version, items) when version = currentVersion -> ValueSome items + | _ -> ValueNone - 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 -> - match documentCache.TryGetValue document.Id with - | true, entry when entry.Version = currentVersion -> return entry.Items - | _ -> - let! parseResults = document.GetFSharpParseResultsAsync(nameof (FSharpNavigateToSearchService)) - let items = NavigateTo.GetNavigableItems parseResults.ParseTree - - // A project's first parse is what makes it produce its options, and the next search keys the - // document by them: only a document that still has none is kept under its own id. - if document.TryGetFSharpParsingOptionsData().IsNone then - documentCache[document.Id] <- - { - Version = currentVersion - Items = items - } - - return items - | 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 - - 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 - keyOf defines - else - keyOf AnyDefines - - cache[key] <- - { - Version = currentVersion - Items = items - } - - return items + match cached anyDefinesKey |> ValueOption.orElseWith (fun () -> cached definedAsKey) with + | ValueSome items -> return items + | ValueNone -> + let! parseResults = document.GetFSharpParseResultsAsync(nameof (FSharpNavigateToSearchService)) + let items = NavigateTo.GetNavigableItems parseResults.ParseTree + cache[storedUnder anyDefinesKey definedAsKey parseResults.ParseTree] <- currentVersion, items + return items } let kindsProvided = @@ -252,7 +194,6 @@ type internal FSharpNavigateToSearchService | ValueNone -> () |] - // The text, read from disk for a closed document, is only needed to place the matches. if matches.Length = 0 then return [||] else @@ -262,17 +203,23 @@ type internal FSharpNavigateToSearchService return [| for struct (item, m) in matches do - match RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, item.Range) with + let sourceSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, item.Range) + + match sourceSpan with | ValueNone -> () | ValueSome sourceSpan -> + let glyph = navigateToItemKindToGlyph item.Kind + let kind = navigateToItemKindToRoslynKind item.Kind + let additionalInfo = formatInfo item.Container document + yield FSharpNavigateToSearchResult( - formatInfo item.Container document, - navigateToItemKindToRoslynKind item.Kind, + additionalInfo, + kind, patternMatchKindToNavigateToMatchKind m.Kind, item.Name, FSharpNavigableItem( - navigateToItemKindToGlyph item.Kind, + glyph, ImmutableArray.Create(TaggedText(TextTags.Text, item.Name)), document, sourceSpan @@ -288,13 +235,16 @@ type internal FSharpNavigateToSearchService cancellableTask { let tryMatch = createMatcherFor searchPattern - let! results = - project.Documents - |> 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) + let tasks = + [| + for doc in project.Documents do + yield processDocument tryMatch kinds doc + |] + + let! results = CancellableTask.whenAllThrottled (max 1 Environment.ProcessorCount) tasks return results |> Array.concat |> Array.toImmutableArray + } |> CancellableTask.start cancellationToken From f3c95e4f6535fe22248391c4f7b0cbb00b7a1c8e Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Thu, 15 Oct 2026 06:36:08 +0100 Subject: [PATCH 15/16] Compact the Navigate To tests One search helper for the three that repeated it, the unused synthetic files and excluded-file data gone, the edit test run in both search orders, and the comments that restated the test names replaced by those names. Co-Authored-By: Claude Sonnet 5.5 --- .../MultiTargetNavigateToSearchTests.fs | 92 ++++++------------- .../NavigateToSearchServiceTests.fs | 82 ++++++----------- 2 files changed, 55 insertions(+), 119 deletions(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs b/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs index 715c75dc5ea..4b5a6571e9a 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs @@ -1,31 +1,21 @@ // 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 open Microsoft.CodeAnalysis.Text -open Microsoft.CodeAnalysis.ExternalAccess.FSharp.NavigateTo open FSharp.Editor.Tests.Helpers open FSharp.Test.ProjectGeneration -let private project = +let private exactNamesIn = NavigateToSearchServiceTests.exactNamesIn + +let private syntheticProject = 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 "Plain" [] with + ExtraSource = "let plainUse = 1" }, - { sourceFile "Fourth" [ "First" ] with - ExtraSource = "let fooOnlyFileUse x = ModuleFirst.sharedFunc x" + { sourceFile "Conditional" [] with + ExtraSource = "#if FOO\nlet fooUse = 1\n#endif" } ) @@ -33,69 +23,39 @@ let private project = module private Fixture = let struct (solution, instances) = RoslynTestHelpers.CreateMultiTargetSolution( - project, - [ - { - Defines = [] - ExcludedFileIds = [ "Fourth" ] - } - { - Defines = [ "FOO" ] - ExcludedFileIds = [] - } - ] + syntheticProject, + [ for defs in [ []; [ "FOO" ] ] -> { Defines = defs; ExcludedFileIds = [] } ] ) -let private service: IFSharpNavigateToSearchService = - MefHelpers.createExportProvider().GetExportedValue() - -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) + let withoutFoo, withFoo = instances[0], instances[1] -/// 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 ``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") + Assert.Equal([ "plainUse" ], exactNamesIn project "plainUse") + Assert.Equal([ "plainUse" ], exactNamesIn 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" ], exactNamesIn (solution.GetProject withFoo) "fooUse") + Assert.Equal([], exactNamesIn (solution.GetProject withoutFoo) "fooUse") - Assert.Equal([ "fooUse" ], searchIn foo "fooUse") - Assert.Equal([], searchIn plain "fooUse") +[] +[] +[] +let ``a declaration an edit puts behind a directive is reported by the instance that defines it`` (fooFirst: bool) = + Assert.Equal([ "plainUse" ], exactNamesIn (solution.GetProject withoutFoo) "plainUse") -/// 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")) + solution.WithDocumentText( + solution.GetDocumentIdsWithFilePath(syntheticProject.GetFilePath "Plain"), + SourceText.From "module ModulePlain\n#if FOO\nlet addedFoo = 1\n#endif\n" + ) - let foo = edited.GetProject instances[1] - let plain = edited.GetProject instances[0] + let searches = [ withoutFoo, []; withFoo, [ "addedFoo" ] ] - // 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") + for instance, expected in (if fooFirst then List.rev searches else searches) do + Assert.Equal(expected, exactNamesIn (edited.GetProject instance) "addedFoo") diff --git a/vsintegration/tests/FSharp.Editor.Tests/NavigateToSearchServiceTests.fs b/vsintegration/tests/FSharp.Editor.Tests/NavigateToSearchServiceTests.fs index c6bcacf716c..901d4841e84 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/NavigateToSearchServiceTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/NavigateToSearchServiceTests.fs @@ -3,7 +3,6 @@ namespace FSharp.Editor.Tests open Xunit -open System.Collections.Immutable open System.Diagnostics open System.Threading open Microsoft.CodeAnalysis @@ -41,13 +40,19 @@ module HeyHo = let project = solution.Projects |> Seq.exactlyOne let provider = MefHelpers.createExportProvider () - let navigateToSearch pattern = + let navigateToSearch (project: Project) pattern = let service: IFSharpNavigateToSearchService = provider.GetExportedValue() service.SearchProjectAsync(project, [] |> Seq.toImmutableArray, pattern, service.KindsProvided, CancellationToken.None).Result + let exactNamesIn project pattern = + navigateToSearch project pattern + |> Seq.map _.Name + |> Seq.filter ((=) pattern) + |> Seq.toList + let assertResultsContain pattern expected = - navigateToSearch pattern + navigateToSearch project pattern |> Seq.exists (fun i -> i.Name = expected) |> Assert.True @@ -76,57 +81,35 @@ 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() + let path = "C:\\Shared.fs" + let definesNothingId, noOptionsId = ProjectId.CreateNewId(), ProjectId.CreateNewId() - projectId, - [ - DocumentInfo.Create(DocumentId.CreateNewId projectId, path, loader = loader, filePath = path) - ] - |> RoslynTestHelpers.CreateProjectInfo projectId $"C:\{name}.fsproj" + let file = + RoslynTestHelpers.CreateDocumentInfo definesNothingId path "module Shared\n#if FOO\nlet fooOnly = 1\n#endif\nlet always = 2\n" - let definesNothingId, definesNothing = projectOf "DefinesNothing" - let noOptionsId, noOptions = projectOf "NoOptions" + let sameVersionFile = file.WithId(DocumentId.CreateNewId noOptionsId) - let solution = RoslynTestHelpers.CreateSolution [ definesNothing; noOptions ] + let solution = + RoslynTestHelpers.CreateSolution + [ + RoslynTestHelpers.CreateProjectInfo definesNothingId "C:\\DefinesNothing.fsproj" [ file ] + RoslynTestHelpers.CreateProjectInfo noOptionsId "C:\\NoOptions.fsproj" [ sameVersionFile ] + ] { 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") + Assert.Equal([ "always" ], exactNamesIn (solution.GetProject definesNothingId) "always") let searching = - Assert.ThrowsAny(fun () -> search (solution.GetProject noOptionsId) "always" |> ignore) + Assert.ThrowsAny(fun () -> exactNamesIn (solution.GetProject noOptionsId) "always" |> ignore) Assert.IsAssignableFrom(searching.GetBaseException()) - /// A standalone script has its options kept per document, so its project never has any to key the script's - /// parse by. The parse is kept under the document itself instead. [] let ``a repeated search of an unchanged standalone script does not parse it again`` () = let path = "C:\\Standalone.fsx" @@ -140,6 +123,10 @@ module HeyHo = |> List.singleton |> RoslynTestHelpers.CreateSolution + let isParseOfScript (activity: Activity) = + activity.OperationName = "BackgroundCompiler.ParseFile" + && activity.Tags |> Seq.exists (fun tag -> tag.Value = path) + let parseRequests = ref 0 use listener = @@ -148,25 +135,14 @@ module HeyHo = Sample = (fun _ -> ActivitySamplingResult.AllData), ActivityStarted = (fun activity -> - if - activity.OperationName = "BackgroundCompiler.ParseFile" - && activity.Tags |> Seq.exists (fun tag -> tag.Value = path) - then + if isParseOfScript activity then Interlocked.Increment(&parseRequests.contents) |> ignore) ) ActivitySource.AddActivityListener listener - let service: IFSharpNavigateToSearchService = provider.GetExportedValue() - let project = solution.GetProject projectId - - let search () = - service - .SearchProjectAsync(project, ImmutableArray.Empty, "standaloneValue", service.KindsProvided, CancellationToken.None) - .Result - |> Seq.map _.Name - |> Seq.toList + let script = solution.GetProject projectId - Assert.Equal([ "standaloneValue" ], search ()) - Assert.Equal([ "standaloneValue" ], search ()) + Assert.Equal([ "standaloneValue" ], exactNamesIn script "standaloneValue") + Assert.Equal([ "standaloneValue" ], exactNamesIn script "standaloneValue") Assert.Equal(1, parseRequests.Value) From 502ab345763b57eeae203ebd597020270a905826 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Thu, 15 Oct 2026 06:36:09 +0100 Subject: [PATCH 16/16] Give the instances of a file one text version in the multi-target tests Documents that AdhocWorkspace.AddSolution loads each carry their own version, so no instance could read another's parse and the tests passed on main. They now share a version per file, as the projects of a loaded solution do, and a test counts the parse requests across the instances. Co-Authored-By: Claude Sonnet 5.5 --- .../MultiTargetNavigateToSearchTests.fs | 25 +++++++++- .../NavigateToSearchServiceTests.fs | 47 ++++++++++--------- 2 files changed, 50 insertions(+), 22 deletions(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs b/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs index 4b5a6571e9a..c26e50c5c29 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs @@ -2,7 +2,9 @@ module FSharp.Editor.Tests.MultiTargetNavigateToSearchTests +open System.IO open Xunit +open Microsoft.CodeAnalysis open Microsoft.CodeAnalysis.Text open FSharp.Editor.Tests.Helpers open FSharp.Test.ProjectGeneration @@ -19,14 +21,24 @@ let private syntheticProject = } ) +let private linkedText path (text: string) (solution: Solution) = + let linked = TextAndVersion.Create(SourceText.From text, VersionStamp.Create()) + + (solution, solution.GetDocumentIdsWithFilePath path) + ||> Seq.fold (fun (solution: Solution) id -> solution.WithDocumentText(id, linked)) + [] module private Fixture = - let struct (solution, instances) = + let struct (unlinked, instances) = RoslynTestHelpers.CreateMultiTargetSolution( syntheticProject, [ for defs in [ []; [ "FOO" ] ] -> { Defines = defs; ExcludedFileIds = [] } ] ) + let solution = + (unlinked, syntheticProject.SourceFilePaths) + ||> List.fold (fun solution path -> linkedText path (File.ReadAllText path) solution) + let withoutFoo, withFoo = instances[0], instances[1] [] @@ -38,6 +50,17 @@ let ``an instance reports a shared declaration on its own, and again once the pa Assert.Equal([ "plainUse" ], exactNamesIn project "plainUse") Assert.Equal([ "plainUse" ], exactNamesIn project "plainUse") +[] +let ``an instance reuses the parse another instance made of the same text`` () = + let path = syntheticProject.GetFilePath "Plain" + let linked = linkedText path "module ModulePlain\nlet reusedUse = 1\n" solution + + let searchEveryInstance () = + for instance in instances do + Assert.Equal([ "reusedUse" ], exactNamesIn (linked.GetProject instance) "reusedUse") + + Assert.Equal(1, NavigateToSearchServiceTests.countParseRequests path searchEveryInstance) + [] let ``a declaration behind a directive is reported only by the instance that defines it`` () = Assert.Equal([ "fooUse" ], exactNamesIn (solution.GetProject withFoo) "fooUse") diff --git a/vsintegration/tests/FSharp.Editor.Tests/NavigateToSearchServiceTests.fs b/vsintegration/tests/FSharp.Editor.Tests/NavigateToSearchServiceTests.fs index 901d4841e84..bc3e2214975 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/NavigateToSearchServiceTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/NavigateToSearchServiceTests.fs @@ -51,6 +51,27 @@ module HeyHo = |> Seq.filter ((=) pattern) |> Seq.toList + let countParseRequests (path: string) searches = + let isParseOfPath (activity: Activity) = + activity.OperationName = "BackgroundCompiler.ParseFile" + && activity.Tags |> Seq.exists (fun tag -> tag.Value = path) + + let parseRequests = ref 0 + + use listener = + new ActivityListener( + ShouldListenTo = (fun source -> source.Name = FSharp.Compiler.Diagnostics.ActivityNames.FscSourceName), + Sample = (fun _ -> ActivitySamplingResult.AllData), + ActivityStarted = + (fun activity -> + if isParseOfPath activity then + Interlocked.Increment(&parseRequests.contents) |> ignore) + ) + + ActivitySource.AddActivityListener listener + searches () + parseRequests.Value + let assertResultsContain pattern expected = navigateToSearch project pattern |> Seq.exists (fun i -> i.Name = expected) @@ -123,26 +144,10 @@ module HeyHo = |> List.singleton |> RoslynTestHelpers.CreateSolution - let isParseOfScript (activity: Activity) = - activity.OperationName = "BackgroundCompiler.ParseFile" - && activity.Tags |> Seq.exists (fun tag -> tag.Value = path) - - let parseRequests = ref 0 - - use listener = - new ActivityListener( - ShouldListenTo = (fun source -> source.Name = FSharp.Compiler.Diagnostics.ActivityNames.FscSourceName), - Sample = (fun _ -> ActivitySamplingResult.AllData), - ActivityStarted = - (fun activity -> - if isParseOfScript activity then - Interlocked.Increment(&parseRequests.contents) |> ignore) - ) - - ActivitySource.AddActivityListener listener - let script = solution.GetProject projectId - Assert.Equal([ "standaloneValue" ], exactNamesIn script "standaloneValue") - Assert.Equal([ "standaloneValue" ], exactNamesIn script "standaloneValue") - Assert.Equal(1, parseRequests.Value) + let searchTwice () = + for _ in 1..2 do + Assert.Equal([ "standaloneValue" ], exactNamesIn script "standaloneValue") + + Assert.Equal(1, countParseRequests path searchTwice)