From 0070388463851352bc1f831eec3bba4c89a8ba54 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 09:42:41 +0200 Subject: [PATCH 01/12] 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 847064f1230d166a7e9c7c0216524d205b00bc42 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 7 Sep 2026 03:58:48 +0200 Subject: [PATCH 02/12] Test navigation into a project built with a path map Co-Authored-By: Claude Fable 5.1 --- .../tests/FSharp.Editor.Tests/PathMapNavigationTests.fs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs b/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs index 241faa94488..0fc8c7ffe77 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs @@ -32,9 +32,7 @@ let private app = OtherOptions = pathMap app } -[] -module private Fixture = - let struct (solution, _) = RoslynTestHelpers.CreateMultiProjectSolution app +let private solution, _ = RoslynTestHelpers.CreateMultiProjectSolution app let private documentOf (project: SyntheticProject) fileId = solution.GetDocumentIdsWithFilePath(project.GetFilePath fileId) From c6f31a3eb268de749a9c0564916c08094f756b9d Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Fri, 11 Sep 2026 04:16:38 +0200 Subject: [PATCH 03/12] Match a range's file against the solution, not the current directory An assembly built with a path map names its source files relative to a root it never records. Resolving such a name with Path.GetFullPath resolved it against the process's current directory, which is not that root and is not even the solution's - it is wherever the last component to set it left it - so the answer differed between sessions and named a file that does not exist. Navigation then took the symbol for an external one and opened generated metadata instead of its source. A name that arrives relative is now matched by its tail against the paths the solution already holds, anchored on a separator so that it matches whole directories rather than the tail of one. A rooted name still goes through the workspace's index, so nothing changes for a build without a map. Co-Authored-By: Claude Opus 5 --- docs/release-notes/.VisualStudio/18.vNext.md | 1 + .../Common/CodeAnalysisExtensions.fs | 43 +++++++++++++++++-- .../FSharp.Editor/LanguageService/Symbols.fs | 2 +- .../PathMapNavigationTests.fs | 41 ++++++++++++++++++ 4 files changed, 83 insertions(+), 4 deletions(-) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index 2d032095814..a1a34f0c436 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -9,6 +9,7 @@ * Go To Definition no longer blocks the UI thread with a bare `Task.Wait`: the synchronous `IFSharpGoToDefinitionService` call now waits through the cancellable threaded-wait dialog, and the editor's `TaskCompletionSource` bridges run their continuations on the thread pool instead of inline on whichever thread finished the check, so repeated F12 on a large solution no longer starves semantic classification and other main-thread work. ([PR #20482](https://github.com/dotnet/fsharp/pull/20482)) * Peek Definition on an F# symbol whose definition lives in metadata no longer deadlocks Visual Studio. Peek holds the main thread in `JoinableTaskFactory.Run` without pumping messages while it asks the language service for the definition, and generating the metadata document needs that same thread; Peek now stops at definitions that already have a document, and Go To Definition, which owns the wait it makes, still opens the generated one. ([PR #20503](https://github.com/dotnet/fsharp/pull/20503)) +* Go To Definition, Find All References and Rename reach the source of a symbol whose assembly was built with `--pathmap` (as `DeterministicSourcePaths` sets it). Such an assembly names its files relative to a root it does not record, and that name was resolved against the process's current directory — which is not the root, and belongs to whatever last set it — so navigation landed on a file that does not exist and fell back to generated metadata. Such a name is now matched against the paths the solution already knows. * Improve Find All References performance by throttling parallel typechecks. ([PR #20128](https://github.com/dotnet/fsharp/pull/20128)) * Fixed Rename incorrectly renaming `get` and `set` keywords for properties with explicit accessors. ([Issue #18270](https://github.com/dotnet/fsharp/issues/18270), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) * Fixed Find All References crash when F# project contains non-F# files like `.cshtml`. ([Issue #16394](https://github.com/dotnet/fsharp/issues/16394), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) diff --git a/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs b/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs index e0b29c8f9f1..bf8d36793d9 100644 --- a/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs +++ b/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs @@ -3,8 +3,32 @@ module internal Microsoft.VisualStudio.FSharp.Editor.CodeAnalysisExtensions open Microsoft.CodeAnalysis open FSharp.Compiler.Text +open System open System.IO +/// Whether the file name a compiler range carries is the file at this path. A build that maps its source +/// paths (`DeterministicSourcePaths`) leaves that name relative to a root the assembly never records, so a +/// relative one is matched by its tail rather than resolved against the process's current directory — +/// which is not that root, and belongs to whatever last set it. +let isTheFileAt (path: string) (fileName: string) = + // Paths, not identifiers: the file systems this runs on do not case them. + let comparison = StringComparison.OrdinalIgnoreCase + + match path, fileName with + | null, _ + | _, null -> false + | path, rooted when Path.IsPathRooted rooted -> String.Equals(Path.GetFullPathSafe rooted, path, comparison) + | path, relative -> + let separator = string Path.DirectorySeparatorChar + + let fromTheRoot = + relative.Split([| '/'; '\\' |], StringSplitOptions.RemoveEmptyEntries) + |> Array.filter (fun segment -> segment <> ".") + |> String.concat separator + + // Anchored on a separator so that a name matches whole directories, never the tail of one. + path.EndsWith($"{separator}{fromTheRoot}", comparison) + type Project with /// Returns the projectIds of all projects within the same solution that directly reference this project @@ -80,13 +104,26 @@ type Solution with member self.GetAllProjectsThisProjectDependsOn(projectId: ProjectId) = self.GetProjectIdsOfAllProjectReferences projectId |> Seq.map self.GetProject + /// The documents whose file is the one a compiler range names. A name a path map left relative + /// reaches no document through the workspace's index, which is keyed by the paths on disk, so it + /// is matched against those paths one by one instead. + member self.GetDocumentIdsWithFSharpFileName(fileName: string) = + match fileName with + | null -> [] + | rooted when Path.IsPathRooted rooted -> self.GetDocumentIdsWithFilePath(Path.GetFullPathSafe rooted) |> List.ofSeq + | relative -> + [ + for project in self.Projects do + for document in project.Documents do + if relative |> isTheFileAt document.FilePath then + document.Id + ] + /// Try to retrieve the corresponding DocumentId for the range's file in the solution /// and if a projectId is provided, only try to find the document within that project /// or a project referenced by that project member self.TryGetDocumentIdFromFSharpRange(range: range, ?projectId: ProjectId) = - let filePath = System.IO.Path.GetFullPathSafe range.FileName - let checkProjectId (docId: DocumentId) = if projectId.IsSome then docId.ProjectId = projectId.Value @@ -107,7 +144,7 @@ type Solution with matchingDoc tail | None -> Some docId - self.GetDocumentIdsWithFilePath filePath |> List.ofSeq |> matchingDoc + self.GetDocumentIdsWithFSharpFileName range.FileName |> matchingDoc /// Try to retrieve the corresponding Document for the range's file in the solution /// and if a projectId is provided, only try to find the document within that project diff --git a/vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs b/vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs index 19e446f2d08..83e7854803a 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs @@ -64,7 +64,7 @@ type FSharpSymbolUse with Some(SymbolScope.Projects([ currentDocument.Project ], isSymbolLocalForProject)) else let projects = - currentDocument.Project.Solution.GetDocumentIdsWithFilePath(filePath) + currentDocument.Project.Solution.GetDocumentIdsWithFSharpFileName loc.FileName |> Seq.map (fun x -> x.ProjectId) |> Seq.distinct |> Seq.map currentDocument.Project.Solution.GetProject diff --git a/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs b/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs index 0fc8c7ffe77..c71a3b4c517 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs @@ -10,6 +10,7 @@ open System.Threading open Xunit open Microsoft.VisualStudio.FSharp.Editor open Microsoft.VisualStudio.FSharp.Editor.CancellableTasks +open FSharp.Compiler.Text open FSharp.Editor.Tests.Helpers open FSharp.Test.ProjectGeneration @@ -64,3 +65,43 @@ let ``goto definition into a project built with a path map reaches its source`` match result with | ValueSome(FSharpGoToDefinitionResult.NavigableItem item, _) -> Assert.Equal(library.GetFilePath "Library", item.Document.FilePath) | result -> failwith $"expected a navigable item, got %A{result}" + +/// A mapped name arrives with the separator its replacement doubled (`.\` + `\rest`), which is what the +/// compiler writes and what a build on a path map hands back. +[] +[] +[] +[] +[] +[] +let ``a relative name denotes the file whose path ends with it`` (path: string) (fileName: string) (expected: bool) = + Assert.Equal(expected, fileName |> isTheFileAt path) + +/// An assembly built with a path map records no root for the names it maps, so a name that arrives +/// relative cannot be resolved against the current directory: that belongs to the process, not to the +/// solution, and points wherever the last component to set it left it. +[] +let ``a range a path map left relative still names its document`` () = + let real = library.GetFilePath "Library" + let root = Path.GetDirectoryName library.ProjectDir + + let relative = + $".\\{real.Substring(root.Length).TrimStart(Path.DirectorySeparatorChar)}" + + let range = Range.mkRange relative (Position.mkPos 1 0) (Position.mkPos 1 0) + + match solution.TryGetDocumentIdFromFSharpRange range with + | Some documentId -> Assert.Equal(real, solution.GetDocument(documentId).FilePath) + | None -> failwith $"no document is named by {relative}" + +[] +let ``a relative name is matched by whole directories, not by the tail of one`` () = + let real = library.GetFilePath "Library" + let root = Path.GetDirectoryName library.ProjectDir + let insideASegment = real.Substring(root.Length + 2) + + let range = Range.mkRange insideASegment (Position.mkPos 1 0) (Position.mkPos 1 0) + + match solution.TryGetDocumentIdFromFSharpRange range with + | Some documentId -> failwith $"{insideASegment} must not name {solution.GetDocument(documentId).FilePath}" + | None -> () From 31d0b76f45daac2aae93259c6a2b450026935980 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Fri, 11 Sep 2026 04:31:08 +0200 Subject: [PATCH 04/12] Link the release note to PR #20519 Co-Authored-By: Claude Opus 5 --- 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 a1a34f0c436..d91ebdfc134 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -10,6 +10,7 @@ * Go To Definition no longer blocks the UI thread with a bare `Task.Wait`: the synchronous `IFSharpGoToDefinitionService` call now waits through the cancellable threaded-wait dialog, and the editor's `TaskCompletionSource` bridges run their continuations on the thread pool instead of inline on whichever thread finished the check, so repeated F12 on a large solution no longer starves semantic classification and other main-thread work. ([PR #20482](https://github.com/dotnet/fsharp/pull/20482)) * Peek Definition on an F# symbol whose definition lives in metadata no longer deadlocks Visual Studio. Peek holds the main thread in `JoinableTaskFactory.Run` without pumping messages while it asks the language service for the definition, and generating the metadata document needs that same thread; Peek now stops at definitions that already have a document, and Go To Definition, which owns the wait it makes, still opens the generated one. ([PR #20503](https://github.com/dotnet/fsharp/pull/20503)) * Go To Definition, Find All References and Rename reach the source of a symbol whose assembly was built with `--pathmap` (as `DeterministicSourcePaths` sets it). Such an assembly names its files relative to a root it does not record, and that name was resolved against the process's current directory — which is not the root, and belongs to whatever last set it — so navigation landed on a file that does not exist and fell back to generated metadata. Such a name is now matched against the paths the solution already knows. +* Go To Definition, Find All References and Rename reach the source of a symbol whose assembly was built with `--pathmap` (as `DeterministicSourcePaths` sets it). Such an assembly names its files relative to a root it does not record, and that name was resolved against the process's current directory — which is not the root, and belongs to whatever last set it — so navigation landed on a file that does not exist and fell back to generated metadata. Such a name is now matched against the paths the solution already knows. ([PR #20519](https://github.com/dotnet/fsharp/pull/20519)) * Improve Find All References performance by throttling parallel typechecks. ([PR #20128](https://github.com/dotnet/fsharp/pull/20128)) * Fixed Rename incorrectly renaming `get` and `set` keywords for properties with explicit accessors. ([Issue #18270](https://github.com/dotnet/fsharp/issues/18270), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) * Fixed Find All References crash when F# project contains non-F# files like `.cshtml`. ([Issue #16394](https://github.com/dotnet/fsharp/issues/16394), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) From 3426050560b96e6c9789145f826340caaff886bf Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Thu, 10 Sep 2026 22:36:16 +0200 Subject: [PATCH 05/12] Recognise a mapped name as the file a document holds Finding the document a range names got the caret to the right file, and then the search for the declaration inside it compared the range's file name to the document's path with `=`. Under a path map the first is relative to a root the assembly never records and the second is absolute, so they never match: the search fell through to a full check of the file, looking for uses of a symbol that belongs to another compilation, and came back with nothing. Both places now go through one rule, `isTheFileAt`, rather than two spellings of it, so a name a path map left relative is matched by its tail wherever a file is identified. Co-Authored-By: Claude Opus 5 --- vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs | 2 +- .../tests/FSharp.Editor.Tests/PathMapNavigationTests.fs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs index 05da71ef97e..d5df0262f3f 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs @@ -355,7 +355,7 @@ type internal GoToDefinition(metadataAsSource: FSharpMetadataAsSourceService) = let! ct = Async.CancellationToken |> liftAsync match targetSymbolUse.Symbol.DeclarationLocation with - | Some decl when decl.FileName = filePath -> return decl + | Some decl when decl.FileName |> isTheFileAt filePath -> return decl | _ -> let! _, checkFileResults = document.GetFSharpParseAndCheckResultsAsync("FindSymbolDeclarationInDocument") diff --git a/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs b/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs index c71a3b4c517..3e127a02140 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs @@ -66,6 +66,7 @@ let ``goto definition into a project built with a path map reaches its source`` | ValueSome(FSharpGoToDefinitionResult.NavigableItem item, _) -> Assert.Equal(library.GetFilePath "Library", item.Document.FilePath) | result -> failwith $"expected a navigable item, got %A{result}" +/// The one rule the document lookup and the search for a declaration inside a document both go through. /// A mapped name arrives with the separator its replacement doubled (`.\` + `\rest`), which is what the /// compiler writes and what a build on a path map hands back. [] From f637065d022198bd978366577b3d81c3128e7bcb Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sat, 26 Sep 2026 03:52:57 +0200 Subject: [PATCH 06/12] Keep a mapped name from narrowing a search to a file it only resembles A name a path map left relative is matched by its tail, so a file of an unrelated project that ends the same way answers to it. The search then restricted itself to that project and never looked at the one that references the assembly, so a use went missing while an unrelated declaration's uses were reported instead. Only a project that compiles the symbol's own assembly may narrow the search now; a rooted name, which resolves to exactly its document, is unaffected, and so is a file linked into several projects. Two more things the same lookup got wrong. A declaration imported from an assembly can carry a name written on another operating system, and `Path.IsPathRooted` throws on one where .NET Framework runs, out of a lookup whose callers expect "no document" for a name it cannot place. And the scan it falls back to walks every document of the solution while Find All References asks for the same name once per project it searches, so the answer is now kept for the solution - a miss as much as a hit, since a name outside the solution is asked for just as often. Co-Authored-By: Claude Opus 5 (1M context) --- .../Common/CodeAnalysisExtensions.fs | 41 +++++++++++++++---- .../FSharp.Editor/LanguageService/Symbols.fs | 21 +++++++++- .../PathMapNavigationTests.fs | 29 +++++++++++++ 3 files changed, 81 insertions(+), 10 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs b/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs index bf8d36793d9..945dbfd6d16 100644 --- a/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs +++ b/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs @@ -4,7 +4,18 @@ module internal Microsoft.VisualStudio.FSharp.Editor.CodeAnalysisExtensions open Microsoft.CodeAnalysis open FSharp.Compiler.Text open System +open System.Collections.Concurrent open System.IO +open System.Runtime.CompilerServices + +/// Whether the name is rooted, for a name that need not be a path this platform can spell: an imported +/// declaration can carry one written on another operating system, and `Path.IsPathRooted` throws on it +/// where `.NET Framework` runs - which is where the editor runs. +let isRootedPath (path: string) = + try + Path.IsPathRooted path + with :? ArgumentException -> + false /// Whether the file name a compiler range carries is the file at this path. A build that maps its source /// paths (`DeterministicSourcePaths`) leaves that name relative to a root the assembly never records, so a @@ -17,7 +28,7 @@ let isTheFileAt (path: string) (fileName: string) = match path, fileName with | null, _ | _, null -> false - | path, rooted when Path.IsPathRooted rooted -> String.Equals(Path.GetFullPathSafe rooted, path, comparison) + | path, rooted when isRootedPath rooted -> String.Equals(Path.GetFullPathSafe rooted, path, comparison) | path, relative -> let separator = string Path.DirectorySeparatorChar @@ -29,6 +40,11 @@ let isTheFileAt (path: string) (fileName: string) = // Anchored on a separator so that a name matches whole directories, never the tail of one. path.EndsWith($"{separator}{fromTheRoot}", comparison) +/// The documents of a solution whose file a mapped name reaches, kept per solution instance: a search asks +/// for the same name once per project, and a solution is replaced rather than mutated. +let private documentsByMappedName = + ConditionalWeakTable>() + type Project with /// Returns the projectIds of all projects within the same solution that directly reference this project @@ -110,14 +126,23 @@ type Solution with member self.GetDocumentIdsWithFSharpFileName(fileName: string) = match fileName with | null -> [] - | rooted when Path.IsPathRooted rooted -> self.GetDocumentIdsWithFilePath(Path.GetFullPathSafe rooted) |> List.ofSeq + | rooted when isRootedPath rooted -> self.GetDocumentIdsWithFilePath(Path.GetFullPathSafe rooted) |> List.ofSeq | relative -> - [ - for project in self.Projects do - for document in project.Documents do - if relative |> isTheFileAt document.FilePath then - document.Id - ] + // The scan walks every document of the solution, and Find All References asks for the same name + // once per project it searches, so the answer - including the empty one - is kept for the solution. + let byName = + documentsByMappedName.GetValue(self, fun _ -> ConcurrentDictionary(StringComparer.OrdinalIgnoreCase)) + + byName.GetOrAdd( + relative, + fun relative -> + [ + for project in self.Projects do + for document in project.Documents do + if relative |> isTheFileAt document.FilePath then + document.Id + ] + ) /// Try to retrieve the corresponding DocumentId for the range's file in the solution /// and if a projectId is provided, only try to find the document within that project diff --git a/vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs b/vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs index 83e7854803a..186b5a1e148 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs @@ -1,6 +1,7 @@ [] module internal Microsoft.VisualStudio.FSharp.Editor.Symbols +open System open System.IO open Microsoft.CodeAnalysis open FSharp.Compiler.CodeAnalysis @@ -63,13 +64,29 @@ type FSharpSymbolUse with // should be treated as an individual project Some(SymbolScope.Projects([ currentDocument.Project ], isSymbolLocalForProject)) else + let solution = currentDocument.Project.Solution + let projects = - currentDocument.Project.Solution.GetDocumentIdsWithFSharpFileName loc.FileName + solution.GetDocumentIdsWithFSharpFileName loc.FileName |> Seq.map (fun x -> x.ProjectId) |> Seq.distinct - |> Seq.map currentDocument.Project.Solution.GetProject + |> Seq.map solution.GetProject |> Seq.toList + let projects = + if isRootedPath loc.FileName then + projects + else + // A name a path map left relative is matched by its tail, which a file of an + // unrelated project can share. Only a project that compiles the symbol's own + // assembly may narrow the search; where none does, the search keeps the scope it + // would have had before the name reached a document at all. + let declaringAssembly = this.Symbol.Assembly.SimpleName + + projects + |> List.filter (fun project -> + String.Equals(project.AssemblyName, declaringAssembly, StringComparison.OrdinalIgnoreCase)) + match projects with | [] -> None | projects -> Some(SymbolScope.Projects(projects, isSymbolLocalForProject)) diff --git a/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs b/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs index 3e127a02140..be5f511b619 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs @@ -106,3 +106,32 @@ let ``a relative name is matched by whole directories, not by the tail of one`` match solution.TryGetDocumentIdFromFSharpRange range with | Some documentId -> failwith $"{insideASegment} must not name {solution.GetDocument(documentId).FilePath}" | None -> () + +[] +let ``a name this platform cannot spell names no document`` () = + // A declaration imported from an assembly can carry a name written on another operating system. + let foreign = "/home/build/a|b/Library.fs" + let range = Range.mkRange foreign (Position.mkPos 1 0) (Position.mkPos 1 0) + + match solution.TryGetDocumentIdFromFSharpRange range with + | Some documentId -> failwith $"{foreign} must not name {solution.GetDocument(documentId).FilePath}" + | None -> () + +[] +let ``the documents a mapped name reaches are the same on every search`` () = + let real = library.GetFilePath "Library" + let root = Path.GetDirectoryName library.ProjectDir + + let relative = + $".\{real.Substring(root.Length).TrimStart(Path.DirectorySeparatorChar)}" + + let reached name = + solution.GetDocumentIdsWithFSharpFileName name + |> List.map (fun documentId -> solution.GetDocument(documentId).FilePath) + + Assert.Equal([ real ], reached relative) + Assert.Equal(reached relative, reached relative) + + // A search asks for the same name once per project, and a name that reaches nothing is asked just as often. + Assert.Empty(reached ".\Nowhere\Nothing.fs") + Assert.Empty(reached ".\Nowhere\Nothing.fs") From 680f381c8c54ac26cd6a5fbe8284c6340096d99d Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sat, 26 Sep 2026 12:32:42 +0200 Subject: [PATCH 07/12] Normalise a mapped name once, not once per document The scan compares every document of the solution against the name, and the comparison rebuilt the name each time: split, filtered, joined and interpolated per document, for a name that does not change between them. The tail a path must end with is built once for the lookup now, and each path is read where it lies. Co-Authored-By: Claude Opus 5 (1M context) --- .../Common/CodeAnalysisExtensions.fs | 40 ++++++++++++------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs b/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs index 945dbfd6d16..dcfef6b6cad 100644 --- a/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs +++ b/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs @@ -17,28 +17,36 @@ let isRootedPath (path: string) = with :? ArgumentException -> false +/// The tail a document's path must end with to be the file a relative name denotes: the name normalised to +/// this platform's separator and anchored on one, so that it matches whole directories, never the tail of one. +let mappedNameTail (fileName: string) = + let separator = string Path.DirectorySeparatorChar + + let fromTheRoot = + fileName.Split([| '/'; '\\' |], StringSplitOptions.RemoveEmptyEntries) + |> Array.filter (fun segment -> segment <> ".") + |> String.concat separator + + $"{separator}{fromTheRoot}" + +/// Whether the path ends with such a tail. The tail is built once for the name; each document of the +/// solution is then compared against it where its own path lies. +let endsWithMappedName (path: string) (tail: string) = + match path with + | null -> false + // Paths, not identifiers: the file systems this runs on do not case them. + | path -> path.AsSpan().EndsWith(tail.AsSpan(), StringComparison.OrdinalIgnoreCase) + /// Whether the file name a compiler range carries is the file at this path. A build that maps its source /// paths (`DeterministicSourcePaths`) leaves that name relative to a root the assembly never records, so a /// relative one is matched by its tail rather than resolved against the process's current directory — /// which is not that root, and belongs to whatever last set it. let isTheFileAt (path: string) (fileName: string) = - // Paths, not identifiers: the file systems this runs on do not case them. - let comparison = StringComparison.OrdinalIgnoreCase - match path, fileName with | null, _ | _, null -> false - | path, rooted when isRootedPath rooted -> String.Equals(Path.GetFullPathSafe rooted, path, comparison) - | path, relative -> - let separator = string Path.DirectorySeparatorChar - - let fromTheRoot = - relative.Split([| '/'; '\\' |], StringSplitOptions.RemoveEmptyEntries) - |> Array.filter (fun segment -> segment <> ".") - |> String.concat separator - - // Anchored on a separator so that a name matches whole directories, never the tail of one. - path.EndsWith($"{separator}{fromTheRoot}", comparison) + | path, rooted when isRootedPath rooted -> String.Equals(Path.GetFullPathSafe rooted, path, StringComparison.OrdinalIgnoreCase) + | path, relative -> endsWithMappedName path (mappedNameTail relative) /// The documents of a solution whose file a mapped name reaches, kept per solution instance: a search asks /// for the same name once per project, and a solution is replaced rather than mutated. @@ -136,10 +144,12 @@ type Solution with byName.GetOrAdd( relative, fun relative -> + let tail = mappedNameTail relative + [ for project in self.Projects do for document in project.Documents do - if relative |> isTheFileAt document.FilePath then + if endsWithMappedName document.FilePath tail then document.Id ] ) From 1c6f3429c549b0e5ef0253d4cf4c4c9a2c09f852 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sat, 26 Sep 2026 13:11:48 +0200 Subject: [PATCH 08/12] Resolve every range's file the way a mapped name needs Go To Definition's `DeclFound` branch and the declaration spans Find All References reports still asked the workspace's index of the paths on disk for the file a range names, so a name a path map left relative reached no document there: the definition was called external and the reference lost its declaration. Both now go through the resolver that matches such a name against the solution's own paths. `rangeToNavigableItem` resolved the file again although its caller had already found the document for that range, so it uses that document. Co-Authored-By: Claude Opus 5 (1M context) --- .../Navigation/FindUsagesService.fs | 4 +- .../Navigation/GoToDefinition.fs | 49 ++++++++----------- 2 files changed, 24 insertions(+), 29 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Navigation/FindUsagesService.fs b/vsintegration/src/FSharp.Editor/Navigation/FindUsagesService.fs index a81d2ec3fed..25d709b4fe1 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/FindUsagesService.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/FindUsagesService.fs @@ -64,7 +64,9 @@ module FSharpFindUsagesService = CancellableTask.singleton [||] else cancellableTask { - let documentIds = solution.GetDocumentIdsWithFilePath(range.FileName) + // A name a path map left relative reaches no document through the workspace's index of the + // paths on disk, so a declaration imported from such an assembly is matched by its tail. + let documentIds = solution.GetDocumentIdsWithFSharpFileName range.FileName let! spans = seq { diff --git a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs index d5df0262f3f..26af402c860 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs @@ -174,29 +174,17 @@ type internal GoToDefinition(metadataAsSource: FSharpMetadataAsSourceService) = && symbol1.DeclaringEntity.CompiledName = symbol2.DeclaringEntity.CompiledName | _ -> false - /// Use an origin document to provide the solution & workspace used to - /// find the corresponding textSpan and INavigableItem for the range + /// Find the textSpan and INavigableItem for the range inside the document holding its file: the caller + /// has already found that document for the range, which a name a path map left relative needs - it + /// reaches no document through the workspace's index of the paths on disk. let rangeToNavigableItem (range: range, document: Document) = cancellableTask { - let fileName = - try - System.IO.Path.GetFullPath range.FileName - with _ -> - range.FileName + let! cancellationToken = Async.CancellationToken + let! sourceText = document.GetTextAsync(cancellationToken) |> Async.AwaitTask - let refDocumentIds = document.Project.Solution.GetDocumentIdsWithFilePath fileName - - if not refDocumentIds.IsEmpty then - let refDocumentId = refDocumentIds.First() - let refDocument = document.Project.Solution.GetDocument refDocumentId - let! cancellationToken = Async.CancellationToken - let! refSourceText = refDocument.GetTextAsync(cancellationToken) |> Async.AwaitTask - - match RoslynHelpers.TryFSharpRangeToTextSpan(refSourceText, range) with - | ValueNone -> return None - | ValueSome refTextSpan -> return Some(FSharpGoToDefinitionNavigableItem(refDocument, refTextSpan)) - else - return None + match RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, range) with + | ValueNone -> return None + | ValueSome textSpan -> return Some(FSharpGoToDefinitionNavigableItem(document, textSpan)) } member _.TryGetExternalDeclarationAsync(targetSymbolUse: FSharpSymbolUse, metadataReferences: seq) = @@ -456,8 +444,13 @@ type internal GoToDefinition(metadataAsSource: FSharpMetadataAsSourceService) = return ValueSome(FSharpGoToDefinitionResult.ExternalAssembly(targetSymbolUse, metadataReferences), idRange) | FindDeclResult.DeclFound targetRange -> - // If the file is not associated with a document, it's considered external. - if not (originDocument.Project.Solution.ContainsDocumentWithFilePath(targetRange.FileName)) then + // If the file is not associated with a document, it's considered external. A build that + // maps its source paths leaves that name relative, so it is the solution's own paths it + // is matched against, not the workspace's index of them. + if + originDocument.Project.Solution.GetDocumentIdsWithFSharpFileName targetRange.FileName + |> List.isEmpty + then let metadataReferences = originDocument.Project.MetadataReferences return ValueSome(FSharpGoToDefinitionResult.ExternalAssembly(targetSymbolUse, metadataReferences), idRange) else if @@ -507,11 +500,11 @@ type internal GoToDefinition(metadataAsSource: FSharpMetadataAsSourceService) = match declarations with | FindDeclResult.DeclFound targetRange -> let sigDocument = - originDocument.Project.Solution.TryGetDocumentFromPath targetRange.FileName + originDocument.Project.Solution.TryGetDocumentFromFSharpRange targetRange match sigDocument with - | ValueNone -> return ValueNone - | ValueSome sigDocument -> + | None -> return ValueNone + | Some sigDocument -> let! sigSourceText = sigDocument.GetTextAsync(cancellationToken) let sigTextSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sigSourceText, targetRange) @@ -527,11 +520,11 @@ type internal GoToDefinition(metadataAsSource: FSharpMetadataAsSourceService) = // - gotoDefn origin = implementation, gotoDefn destination = implementation else let sigDocument = - originDocument.Project.Solution.TryGetDocumentFromPath targetRange.FileName + originDocument.Project.Solution.TryGetDocumentFromFSharpRange targetRange match sigDocument with - | ValueNone -> return ValueNone - | ValueSome sigDocument -> + | None -> return ValueNone + | Some sigDocument -> let! sigSourceText = sigDocument.GetTextAsync(cancellationToken) let sigTextSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sigSourceText, targetRange) From ba1ba88977d2e5b7dcdc119f79c0185a91855181 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sat, 26 Sep 2026 13:11:49 +0200 Subject: [PATCH 09/12] Keep one release note for the path map fix Linking the note to the PR added a second copy of it instead of editing the first. Co-Authored-By: Claude Opus 5 (1M context) --- docs/release-notes/.VisualStudio/18.vNext.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index d91ebdfc134..d0ef40290b7 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -9,7 +9,6 @@ * Go To Definition no longer blocks the UI thread with a bare `Task.Wait`: the synchronous `IFSharpGoToDefinitionService` call now waits through the cancellable threaded-wait dialog, and the editor's `TaskCompletionSource` bridges run their continuations on the thread pool instead of inline on whichever thread finished the check, so repeated F12 on a large solution no longer starves semantic classification and other main-thread work. ([PR #20482](https://github.com/dotnet/fsharp/pull/20482)) * Peek Definition on an F# symbol whose definition lives in metadata no longer deadlocks Visual Studio. Peek holds the main thread in `JoinableTaskFactory.Run` without pumping messages while it asks the language service for the definition, and generating the metadata document needs that same thread; Peek now stops at definitions that already have a document, and Go To Definition, which owns the wait it makes, still opens the generated one. ([PR #20503](https://github.com/dotnet/fsharp/pull/20503)) -* Go To Definition, Find All References and Rename reach the source of a symbol whose assembly was built with `--pathmap` (as `DeterministicSourcePaths` sets it). Such an assembly names its files relative to a root it does not record, and that name was resolved against the process's current directory — which is not the root, and belongs to whatever last set it — so navigation landed on a file that does not exist and fell back to generated metadata. Such a name is now matched against the paths the solution already knows. * Go To Definition, Find All References and Rename reach the source of a symbol whose assembly was built with `--pathmap` (as `DeterministicSourcePaths` sets it). Such an assembly names its files relative to a root it does not record, and that name was resolved against the process's current directory — which is not the root, and belongs to whatever last set it — so navigation landed on a file that does not exist and fell back to generated metadata. Such a name is now matched against the paths the solution already knows. ([PR #20519](https://github.com/dotnet/fsharp/pull/20519)) * Improve Find All References performance by throttling parallel typechecks. ([PR #20128](https://github.com/dotnet/fsharp/pull/20128)) * Fixed Rename incorrectly renaming `get` and `set` keywords for properties with explicit accessors. ([Issue #18270](https://github.com/dotnet/fsharp/issues/18270), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) From 5c4f63dc320585039eeb05a10e7148c2ff5f05b0 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Thu, 1 Oct 2026 19:16:22 +0200 Subject: [PATCH 10/12] 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 | 21 ++++++++++++------- .../PathMapNavigationTests.fs | 2 +- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs index 89a449eceb7..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 @@ -382,8 +383,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 -> struct (project, ProjectId.CreateNewId())) + |> Seq.toArray let projectIds = dict [ for project, id in projects -> project.Name, id ] @@ -413,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 = []) @@ -429,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 ) @@ -467,7 +474,7 @@ type RoslynTestHelpers private () = |] } - id, projectInfo, instanceOptions + struct (id, projectInfo, instanceOptions) ] let solution = @@ -476,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/PathMapNavigationTests.fs b/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs index be5f511b619..9811c760e8f 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs @@ -33,7 +33,7 @@ let private app = OtherOptions = pathMap app } -let private solution, _ = RoslynTestHelpers.CreateMultiProjectSolution app +let struct (solution, _) = RoslynTestHelpers.CreateMultiProjectSolution app let private documentOf (project: SyntheticProject) fileId = solution.GetDocumentIdsWithFilePath(project.GetFilePath fileId) From df78b14581fb094eb7c95df804a1f196d3dbec8b Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Thu, 1 Oct 2026 22:22:26 +0200 Subject: [PATCH 11/12] 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 --- .../tests/FSharp.Editor.Tests/PathMapNavigationTests.fs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs b/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs index 9811c760e8f..7119adc1cc3 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs @@ -33,7 +33,9 @@ let private app = OtherOptions = pathMap app } -let struct (solution, _) = RoslynTestHelpers.CreateMultiProjectSolution app +[] +module private Fixture = + let struct (solution, _) = RoslynTestHelpers.CreateMultiProjectSolution app let private documentOf (project: SyntheticProject) fileId = solution.GetDocumentIdsWithFilePath(project.GetFilePath fileId) From e0fa3eaa5a3b6a0b7cac63e4bf81cdf129d6d9d5 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 11 Oct 2026 11:46:51 +0200 Subject: [PATCH 12/12] Take a mapped name for a file only in the source of its own assembly A name a path map left relative only ends the way a file's path does, and the path of an unrelated file can end the same way. Such a name now reaches a document only for a declaration, in the projects that compile the declaring assembly, and only when they hold exactly one file ending that way. Otherwise the declaration stays external, for Go To Definition and for the declaration Find All References reports alike. The . and .. segments of a name are resolved in the name itself before it is matched; a name that climbs above where it starts matches nothing, since what it denotes depends on the root the assembly never records. For a mapped assembly built with a signature, Go To Definition from an implementation file takes the symbol's implementation location instead of looking only at where it declares itself, and where the mapped source gives no location the definition is the external one it was before. Co-Authored-By: Claude Opus 5.5 --- docs/release-notes/.VisualStudio/18.vNext.md | 2 +- .../Common/CodeAnalysisExtensions.fs | 131 +++++--- .../FSharp.Editor/LanguageService/Symbols.fs | 21 +- .../Navigation/FindUsagesService.fs | 16 +- .../Navigation/GoToDefinition.fs | 61 +++- .../PathMapNavigationTests.fs | 283 +++++++++++++++++- 6 files changed, 428 insertions(+), 86 deletions(-) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index d0ef40290b7..3c8d3bdf00f 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -9,7 +9,7 @@ * Go To Definition no longer blocks the UI thread with a bare `Task.Wait`: the synchronous `IFSharpGoToDefinitionService` call now waits through the cancellable threaded-wait dialog, and the editor's `TaskCompletionSource` bridges run their continuations on the thread pool instead of inline on whichever thread finished the check, so repeated F12 on a large solution no longer starves semantic classification and other main-thread work. ([PR #20482](https://github.com/dotnet/fsharp/pull/20482)) * Peek Definition on an F# symbol whose definition lives in metadata no longer deadlocks Visual Studio. Peek holds the main thread in `JoinableTaskFactory.Run` without pumping messages while it asks the language service for the definition, and generating the metadata document needs that same thread; Peek now stops at definitions that already have a document, and Go To Definition, which owns the wait it makes, still opens the generated one. ([PR #20503](https://github.com/dotnet/fsharp/pull/20503)) -* Go To Definition, Find All References and Rename reach the source of a symbol whose assembly was built with `--pathmap` (as `DeterministicSourcePaths` sets it). Such an assembly names its files relative to a root it does not record, and that name was resolved against the process's current directory — which is not the root, and belongs to whatever last set it — so navigation landed on a file that does not exist and fell back to generated metadata. Such a name is now matched against the paths the solution already knows. ([PR #20519](https://github.com/dotnet/fsharp/pull/20519)) +* Go To Definition, Find All References and Rename reach the source of a symbol whose assembly was built with `--pathmap` (as `DeterministicSourcePaths` sets it). Such an assembly names its files relative to a root it does not record, and that name was resolved against the process's current directory — which is not the root, and belongs to whatever last set it — so navigation landed on a file that does not exist and fell back to generated metadata. Such a name is now matched against the paths the solution already knows, in the projects that compile the symbol's assembly; where they do not hold exactly one such file, the symbol stays external. ([PR #20519](https://github.com/dotnet/fsharp/pull/20519)) * Improve Find All References performance by throttling parallel typechecks. ([PR #20128](https://github.com/dotnet/fsharp/pull/20128)) * Fixed Rename incorrectly renaming `get` and `set` keywords for properties with explicit accessors. ([Issue #18270](https://github.com/dotnet/fsharp/issues/18270), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) * Fixed Find All References crash when F# project contains non-F# files like `.cshtml`. ([Issue #16394](https://github.com/dotnet/fsharp/issues/16394), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) diff --git a/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs b/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs index dcfef6b6cad..5b4026361e3 100644 --- a/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs +++ b/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs @@ -17,17 +17,26 @@ let isRootedPath (path: string) = with :? ArgumentException -> false -/// The tail a document's path must end with to be the file a relative name denotes: the name normalised to -/// this platform's separator and anchored on one, so that it matches whole directories, never the tail of one. +/// The tail a document's path must end with to be the file a relative name denotes: the name with its `.` and +/// `..` segments resolved, in this platform's separator and anchored on one, so that it matches whole +/// directories, never the tail of one. A name that climbs above where it starts has no such tail: what it +/// denotes depends on the root, which the assembly never records. let mappedNameTail (fileName: string) = - let separator = string Path.DirectorySeparatorChar - - let fromTheRoot = - fileName.Split([| '/'; '\\' |], StringSplitOptions.RemoveEmptyEntries) - |> Array.filter (fun segment -> segment <> ".") - |> String.concat separator - - $"{separator}{fromTheRoot}" + let segments = ResizeArray() + let mutable climbsAboveItsStart = false + + for segment in fileName.Split([| '/'; '\\' |], StringSplitOptions.RemoveEmptyEntries) do + match segment with + | "." -> () + | ".." when segments.Count > 0 -> segments.RemoveAt(segments.Count - 1) + | ".." -> climbsAboveItsStart <- true + | segment -> segments.Add segment + + if climbsAboveItsStart || segments.Count = 0 then + ValueNone + else + let separator = string Path.DirectorySeparatorChar + ValueSome $"{separator}{String.Join(separator, segments)}" /// Whether the path ends with such a tail. The tail is built once for the name; each document of the /// solution is then compared against it where its own path lies. @@ -46,13 +55,38 @@ let isTheFileAt (path: string) (fileName: string) = | null, _ | _, null -> false | path, rooted when isRootedPath rooted -> String.Equals(Path.GetFullPathSafe rooted, path, StringComparison.OrdinalIgnoreCase) - | path, relative -> endsWithMappedName path (mappedNameTail relative) + | path, relative -> + match mappedNameTail relative with + | ValueSome tail -> endsWithMappedName path tail + | ValueNone -> false -/// The documents of a solution whose file a mapped name reaches, kept per solution instance: a search asks -/// for the same name once per project, and a solution is replaced rather than mutated. +/// The documents of a solution whose path ends the way a mapped name does, kept per solution instance: a search +/// asks for the same name once per project, and a solution is replaced rather than mutated. let private documentsByMappedName = ConditionalWeakTable>() +/// The documents whose path ends the way a name a path map left relative does. They are candidates only: the +/// path of an unrelated file can end the same way. +let private candidatesForMappedName (solution: Solution) (relative: string) = + // The scan walks every document of the solution, and Find All References asks for the same name + // once per project it searches, so the answer - including the empty one - is kept for the solution. + let byName = + documentsByMappedName.GetValue(solution, fun _ -> ConcurrentDictionary(StringComparer.OrdinalIgnoreCase)) + + byName.GetOrAdd( + relative, + fun relative -> + match mappedNameTail relative with + | ValueNone -> [] + | ValueSome tail -> + [ + for project in solution.Projects do + for document in project.Documents do + if endsWithMappedName document.FilePath tail then + document.Id + ] + ) + type Project with /// Returns the projectIds of all projects within the same solution that directly reference this project @@ -128,36 +162,44 @@ type Solution with member self.GetAllProjectsThisProjectDependsOn(projectId: ProjectId) = self.GetProjectIdsOfAllProjectReferences projectId |> Seq.map self.GetProject - /// The documents whose file is the one a compiler range names. A name a path map left relative - /// reaches no document through the workspace's index, which is keyed by the paths on disk, so it - /// is matched against those paths one by one instead. - member self.GetDocumentIdsWithFSharpFileName(fileName: string) = - match fileName with - | null -> [] - | rooted when isRootedPath rooted -> self.GetDocumentIdsWithFilePath(Path.GetFullPathSafe rooted) |> List.ofSeq - | relative -> - // The scan walks every document of the solution, and Find All References asks for the same name - // once per project it searches, so the answer - including the empty one - is kept for the solution. - let byName = - documentsByMappedName.GetValue(self, fun _ -> ConcurrentDictionary(StringComparer.OrdinalIgnoreCase)) - - byName.GetOrAdd( - relative, - fun relative -> - let tail = mappedNameTail relative - - [ - for project in self.Projects do - for document in project.Documents do - if endsWithMappedName document.FilePath tail then - document.Id - ] - ) + /// The documents whose file is the one a compiler range names. A rooted name is that file's path. A name a + /// path map left relative only ends the way the path does, as the path of an unrelated file can, so it + /// names a file only for a declaration whose assembly is given, and only where the projects compiling that + /// assembly hold exactly one file ending that way. Otherwise it names none, and the declaration stays + /// external. + member self.GetDocumentIdsWithFSharpFileName(fileName: string, ?declaringAssembly: string) = + match fileName, declaringAssembly with + | null, _ -> [] + | rooted, _ when isRootedPath rooted -> self.GetDocumentIdsWithFilePath(Path.GetFullPathSafe rooted) |> List.ofSeq + | _, None -> [] + | relative, Some declaringAssembly -> + let ofTheAssembly = + candidatesForMappedName self relative + |> List.filter (fun documentId -> + String.Equals( + self.GetProject(documentId.ProjectId).AssemblyName, + declaringAssembly, + StringComparison.OrdinalIgnoreCase + )) + + match ofTheAssembly with + | [] -> [] + | first :: others -> + let path = self.GetDocument(first).FilePath + + let isTheSameFile (documentId: DocumentId) = + String.Equals(self.GetDocument(documentId).FilePath, path, StringComparison.OrdinalIgnoreCase) + + if List.forall isTheSameFile others then + ofTheAssembly + else + [] /// Try to retrieve the corresponding DocumentId for the range's file in the solution /// and if a projectId is provided, only try to find the document within that project - /// or a project referenced by that project - member self.TryGetDocumentIdFromFSharpRange(range: range, ?projectId: ProjectId) = + /// or a project referenced by that project. The range of a declaration in another assembly needs + /// that assembly's name: see GetDocumentIdsWithFSharpFileName. + member self.TryGetDocumentIdFromFSharpRange(range: range, ?projectId: ProjectId, ?declaringAssembly: string) = let checkProjectId (docId: DocumentId) = if projectId.IsSome then @@ -179,13 +221,12 @@ type Solution with matchingDoc tail | None -> Some docId - self.GetDocumentIdsWithFSharpFileName range.FileName |> matchingDoc + self.GetDocumentIdsWithFSharpFileName(range.FileName, ?declaringAssembly = declaringAssembly) + |> matchingDoc /// Try to retrieve the corresponding Document for the range's file in the solution /// and if a projectId is provided, only try to find the document within that project /// or a project referenced by that project - member self.TryGetDocumentFromFSharpRange(range: range, ?projectId: ProjectId) = - match projectId with - | Some projectId -> self.TryGetDocumentIdFromFSharpRange(range, projectId) - | None -> self.TryGetDocumentIdFromFSharpRange range + member self.TryGetDocumentFromFSharpRange(range: range, ?projectId: ProjectId, ?declaringAssembly: string) = + self.TryGetDocumentIdFromFSharpRange(range, ?projectId = projectId, ?declaringAssembly = declaringAssembly) |> Option.map self.GetDocument diff --git a/vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs b/vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs index 186b5a1e148..5494213074d 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs @@ -1,7 +1,6 @@ [] module internal Microsoft.VisualStudio.FSharp.Editor.Symbols -open System open System.IO open Microsoft.CodeAnalysis open FSharp.Compiler.CodeAnalysis @@ -66,27 +65,17 @@ type FSharpSymbolUse with else let solution = currentDocument.Project.Solution + // A name a path map left relative is matched by its tail, which a file of an unrelated + // project can share. Only a project that compiles the symbol's own assembly may narrow the + // search; where none does, the search keeps the scope it would have had before the name + // reached a document at all. let projects = - solution.GetDocumentIdsWithFSharpFileName loc.FileName + solution.GetDocumentIdsWithFSharpFileName(loc.FileName, this.Symbol.Assembly.SimpleName) |> Seq.map (fun x -> x.ProjectId) |> Seq.distinct |> Seq.map solution.GetProject |> Seq.toList - let projects = - if isRootedPath loc.FileName then - projects - else - // A name a path map left relative is matched by its tail, which a file of an - // unrelated project can share. Only a project that compiles the symbol's own - // assembly may narrow the search; where none does, the search keeps the scope it - // would have had before the name reached a document at all. - let declaringAssembly = this.Symbol.Assembly.SimpleName - - projects - |> List.filter (fun project -> - String.Equals(project.AssemblyName, declaringAssembly, StringComparison.OrdinalIgnoreCase)) - match projects with | [] -> None | projects -> Some(SymbolScope.Projects(projects, isSymbolLocalForProject)) diff --git a/vsintegration/src/FSharp.Editor/Navigation/FindUsagesService.fs b/vsintegration/src/FSharp.Editor/Navigation/FindUsagesService.fs index 25d709b4fe1..a33a78e0cbc 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/FindUsagesService.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/FindUsagesService.fs @@ -59,14 +59,16 @@ module FSharpFindUsagesService = } // File can be included in more than one project, hence single `range` may results with multiple `Document`s. - let rangeToDocumentSpans (solution: Solution, range: range, symbolName: string) = + let rangeToDocumentSpans (solution: Solution, range: range, symbolName: string, declaringAssembly: string) = if range.Start = range.End then CancellableTask.singleton [||] else cancellableTask { // A name a path map left relative reaches no document through the workspace's index of the - // paths on disk, so a declaration imported from such an assembly is matched by its tail. - let documentIds = solution.GetDocumentIdsWithFSharpFileName range.FileName + // paths on disk, so a declaration imported from such an assembly is matched by its tail, in the + // projects that compile the assembly. + let documentIds = + solution.GetDocumentIdsWithFSharpFileName(range.FileName, declaringAssembly) let! spans = seq { @@ -121,7 +123,13 @@ module FSharpFindUsagesService = let! declarationSpans = match declarationRange with - | Some range -> rangeToDocumentSpans (document.Project.Solution, range, symbol.Ident.idText) + | Some range -> + rangeToDocumentSpans ( + document.Project.Solution, + range, + symbol.Ident.idText, + symbolUse.Symbol.Assembly.SimpleName + ) | None -> CancellableTask.singleton [||] let declarationSpans = diff --git a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs index 26af402c860..8641ec001d6 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs @@ -341,10 +341,21 @@ type internal GoToDefinition(metadataAsSource: FSharpMetadataAsSourceService) = asyncMaybe { let filePath = document.FilePath let! ct = Async.CancellationToken |> liftAsync - - match targetSymbolUse.Symbol.DeclarationLocation with - | Some decl when decl.FileName |> isTheFileAt filePath -> return decl - | _ -> + let symbol = targetSymbolUse.Symbol + + // A symbol with a signature declares itself there, so in its implementation file it is the + // implementation's location that lies in the document. + let locationInDocument = + [ + symbol.DeclarationLocation + symbol.ImplementationLocation + symbol.SignatureLocation + ] + |> List.tryPick (Option.filter (fun location -> location.FileName |> isTheFileAt filePath)) + + match locationInDocument with + | Some location -> return location + | None -> let! _, checkFileResults = document.GetFSharpParseAndCheckResultsAsync("FindSymbolDeclarationInDocument") |> CancellableTask.start ct @@ -444,15 +455,29 @@ type internal GoToDefinition(metadataAsSource: FSharpMetadataAsSourceService) = return ValueSome(FSharpGoToDefinitionResult.ExternalAssembly(targetSymbolUse, metadataReferences), idRange) | FindDeclResult.DeclFound targetRange -> + let declaringAssembly = targetSymbolUse.Symbol.Assembly.SimpleName + + let externalDeclaration = + let metadataReferences = originDocument.Project.MetadataReferences + ValueSome(FSharpGoToDefinitionResult.ExternalAssembly(targetSymbolUse, metadataReferences), idRange) + + // A declaration with a mapped name was external before such a name reached a document, + // and stays so wherever the source it reaches gives no location for it. + let withoutLocation = + if isRootedPath targetRange.FileName then + ValueNone + else + externalDeclaration + // If the file is not associated with a document, it's considered external. A build that // maps its source paths leaves that name relative, so it is the solution's own paths it - // is matched against, not the workspace's index of them. + // is matched against, not the workspace's index of them - and only those of the projects + // compiling the symbol's assembly: an unrelated file can end the same way. if - originDocument.Project.Solution.GetDocumentIdsWithFSharpFileName targetRange.FileName + originDocument.Project.Solution.GetDocumentIdsWithFSharpFileName(targetRange.FileName, declaringAssembly) |> List.isEmpty then - let metadataReferences = originDocument.Project.MetadataReferences - return ValueSome(FSharpGoToDefinitionResult.ExternalAssembly(targetSymbolUse, metadataReferences), idRange) + return externalDeclaration else if // if goto definition is called as we are already at the declaration location of a symbol in // either a signature or an implementation file then we jump to its respective position in the document @@ -500,7 +525,10 @@ type internal GoToDefinition(metadataAsSource: FSharpMetadataAsSourceService) = match declarations with | FindDeclResult.DeclFound targetRange -> let sigDocument = - originDocument.Project.Solution.TryGetDocumentFromFSharpRange targetRange + originDocument.Project.Solution.TryGetDocumentFromFSharpRange( + targetRange, + declaringAssembly = declaringAssembly + ) match sigDocument with | None -> return ValueNone @@ -520,16 +548,19 @@ type internal GoToDefinition(metadataAsSource: FSharpMetadataAsSourceService) = // - gotoDefn origin = implementation, gotoDefn destination = implementation else let sigDocument = - originDocument.Project.Solution.TryGetDocumentFromFSharpRange targetRange + originDocument.Project.Solution.TryGetDocumentFromFSharpRange( + targetRange, + declaringAssembly = declaringAssembly + ) match sigDocument with - | None -> return ValueNone + | None -> return withoutLocation | Some sigDocument -> let! sigSourceText = sigDocument.GetTextAsync(cancellationToken) let sigTextSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sigSourceText, targetRange) match sigTextSpan with - | ValueNone -> return ValueNone + | ValueNone -> return withoutLocation | ValueSome sigTextSpan -> // if the gotodef call originated from a signature and the returned target is a signature, navigate there if isSignatureFile targetRange.FileName && preferSignature then @@ -548,12 +579,12 @@ type internal GoToDefinition(metadataAsSource: FSharpMetadataAsSourceService) = originDocument.Project.Solution.TryGetDocumentFromPath implFilePath match implDocument with - | ValueNone -> return ValueNone + | ValueNone -> return withoutLocation | ValueSome implDocument -> let! targetRange = this.FindSymbolDeclarationInDocument(targetSymbolUse, implDocument) match targetRange with - | None -> return ValueNone + | None -> return withoutLocation | Some targetRange -> let! implSourceText = implDocument.GetTextAsync(cancellationToken) @@ -561,7 +592,7 @@ type internal GoToDefinition(metadataAsSource: FSharpMetadataAsSourceService) = RoslynHelpers.TryFSharpRangeToTextSpan(implSourceText, targetRange) match implTextSpan with - | ValueNone -> return ValueNone + | ValueNone -> return withoutLocation | ValueSome implTextSpan -> let navItem = FSharpGoToDefinitionNavigableItem(implDocument, implTextSpan) return ValueSome(FSharpGoToDefinitionResult.NavigableItem(navItem), idRange) diff --git a/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs b/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs index 7119adc1cc3..e5a0e39fe97 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/PathMapNavigationTests.fs @@ -5,11 +5,17 @@ module FSharp.Editor.Tests.PathMapNavigationTests open System +open System.Collections.Concurrent open System.IO open System.Threading +open System.Threading.Tasks open Xunit +open Microsoft.CodeAnalysis +open Microsoft.CodeAnalysis.ExternalAccess.FSharp.Editor.FindUsages +open Microsoft.CodeAnalysis.ExternalAccess.FSharp.FindUsages open Microsoft.VisualStudio.FSharp.Editor open Microsoft.VisualStudio.FSharp.Editor.CancellableTasks +open FSharp.Compiler.EditorServices open FSharp.Compiler.Text open FSharp.Editor.Tests.Helpers open FSharp.Test.ProjectGeneration @@ -70,13 +76,19 @@ let ``goto definition into a project built with a path map reaches its source`` /// The one rule the document lookup and the search for a declaration inside a document both go through. /// A mapped name arrives with the separator its replacement doubled (`.\` + `\rest`), which is what the -/// compiler writes and what a build on a path map hands back. +/// compiler writes and what a build on a path map hands back. Its `.` and `..` segments are resolved in the +/// name itself; one that climbs above where it starts denotes a file only from a root, which is not known. [] [] [] [] [] [] +[] +[] +[] +[] +[] let ``a relative name denotes the file whose path ends with it`` (path: string) (fileName: string) (expected: bool) = Assert.Equal(expected, fileName |> isTheFileAt path) @@ -93,10 +105,13 @@ let ``a range a path map left relative still names its document`` () = let range = Range.mkRange relative (Position.mkPos 1 0) (Position.mkPos 1 0) - match solution.TryGetDocumentIdFromFSharpRange range with + match solution.TryGetDocumentIdFromFSharpRange(range, declaringAssembly = library.Name) with | Some documentId -> Assert.Equal(real, solution.GetDocument(documentId).FilePath) | None -> failwith $"no document is named by {relative}" + // Without the assembly that declares what the range locates, nothing says the file is the one it names. + Assert.Equal(None, solution.TryGetDocumentIdFromFSharpRange range) + [] let ``a relative name is matched by whole directories, not by the tail of one`` () = let real = library.GetFilePath "Library" @@ -105,7 +120,7 @@ let ``a relative name is matched by whole directories, not by the tail of one`` let range = Range.mkRange insideASegment (Position.mkPos 1 0) (Position.mkPos 1 0) - match solution.TryGetDocumentIdFromFSharpRange range with + match solution.TryGetDocumentIdFromFSharpRange(range, declaringAssembly = library.Name) with | Some documentId -> failwith $"{insideASegment} must not name {solution.GetDocument(documentId).FilePath}" | None -> () @@ -115,7 +130,7 @@ let ``a name this platform cannot spell names no document`` () = let foreign = "/home/build/a|b/Library.fs" let range = Range.mkRange foreign (Position.mkPos 1 0) (Position.mkPos 1 0) - match solution.TryGetDocumentIdFromFSharpRange range with + match solution.TryGetDocumentIdFromFSharpRange(range, declaringAssembly = library.Name) with | Some documentId -> failwith $"{foreign} must not name {solution.GetDocument(documentId).FilePath}" | None -> () @@ -128,7 +143,7 @@ let ``the documents a mapped name reaches are the same on every search`` () = $".\{real.Substring(root.Length).TrimStart(Path.DirectorySeparatorChar)}" let reached name = - solution.GetDocumentIdsWithFSharpFileName name + solution.GetDocumentIdsWithFSharpFileName(name, library.Name) |> List.map (fun documentId -> solution.GetDocument(documentId).FilePath) Assert.Equal([ real ], reached relative) @@ -137,3 +152,261 @@ let ``the documents a mapped name reaches are the same on every search`` () = // A search asks for the same name once per project, and a name that reaches nothing is asked just as often. Assert.Empty(reached ".\Nowhere\Nothing.fs") Assert.Empty(reached ".\Nowhere\Nothing.fs") + +/// A solution of bare projects, written `Assembly=path;path|Assembly=path`. +let private solutionOf (projects: string) = + RoslynTestHelpers.CreateSolution + [ + for project in projects.Split '|' do + match project.Split '=' with + | [| assembly; paths |] -> + let id = ProjectId.CreateNewId() + + ProjectInfo.Create( + id, + VersionStamp.Default, + assembly, + assembly, + LanguageNames.FSharp, + documents = + [ + for path in paths.Split ';' -> RoslynTestHelpers.CreateDocumentInfo id path "" + ] + ) + | _ -> failwith $"not a project: {project}" + ] + +/// The path of an unrelated file can end the way a mapped name does, so such a name is the file of a +/// declaration only where the projects compiling the declaring assembly, `Lib` here, hold exactly one file +/// ending that way. The rows are: that file alone; the source absent and an unrelated file in its place; an +/// unrelated file beside it; two files of the assembly; the one file in two projects of the assembly, as +/// multi-targeting has it; an equivalent name; and a name that needs the root. +[] +[] +[] +[] +[] +[] +[] +[] +let ``a mapped name reaches the one file of the declaring assembly that ends with it`` + (projects: string) + (name: string) + (expected: string) + = + let solution = solutionOf projects + + let reached = + solution.GetDocumentIdsWithFSharpFileName(name, "Lib") + |> List.map (fun documentId -> solution.GetDocument(documentId).FilePath) + + Assert.Equal(expected.Split([| ';' |], StringSplitOptions.RemoveEmptyEntries) |> List.ofArray, reached) + +/// A library the solutions hold as an assembly on disk, compiled from its own directory with a path map as a +/// build under DeterministicSourcePaths compiles it: it names its files `.\FileLibrary.fsi` and `.\FileLibrary.fs`. +type private MappedBinary = + { + Library: SyntheticProject + /// References the assembly, never the project it was built from. + Consumer: SyntheticProject + /// The consumer and an unrelated project with a file of the library's name: the library's source is absent. + WithoutSource: Solution + /// The consumer and the project the assembly was built from. + WithSource: Solution + } + +let private mappedBinary = + lazy + (let declarations = "type Kind =\n | A\n | B\n" + + let library = + SyntheticProject.Create( + "External", + { sourceFile "Library" [] with + SignatureFile = Custom $"{declarations}\nval value: int" + Source = $"{declarations}\nlet value = 42" + } + ) + + let options = library.GetProjectOptions(library.SaveAndCheck()) + let assembly = Path.Combine(library.ProjectDir, "bin", $"{library.Name}.dll") + Directory.CreateDirectory(Path.GetDirectoryName assembly) |> ignore + + File.WriteAllLines( + Path.Combine(library.ProjectDir, "build.rsp"), + [ + "--target:library" + $"--pathmap:{library.ProjectDir}=." + $"-o:{assembly}" + yield! options.OtherOptions + yield! options.SourceFiles + ] + ) + + let exitCode, output, errors = + TestFramework.Commands.executeProcess TestFramework.initialConfig.FSC "@build.rsp" library.ProjectDir + + Assert.True((exitCode = 0), $"fsc exited with {exitCode}:\n{output}\n{errors}") + + let consumer = + { SyntheticProject.Create( + "Consumer", + { sourceFile "App" [] with + SignatureFile = Custom $"val result: int\nval kind: {library.Name}.ModuleLibrary.Kind" + Source = $"let result = {library.Name}.ModuleLibrary.value\nlet kind = {library.Name}.ModuleLibrary.Kind.A" + } + ) with + OtherOptions = [ $"-r:{assembly}" ] + } + + // Declares a value of the same name on the same line as the library does. + let unrelated = + SyntheticProject.Create( + "Unrelated", + { sourceFile "Library" [] with + Source = $"{declarations}\nlet value = 99" + } + ) + + let solutionWith (projects: SyntheticProject list) = + let root = + { SyntheticProject.Create("Root", sourceFile "Root" []) with + DependsOn = projects + } + + let struct (solution, _) = RoslynTestHelpers.CreateMultiProjectSolution root + solution + + { + Library = library + Consumer = consumer + WithoutSource = solutionWith [ consumer; unrelated ] + WithSource = solutionWith [ consumer; library ] + }) + +let private documentAt (solution: Solution) (path: string) = + solution.GetDocumentIdsWithFilePath path + |> Seq.exactlyOne + |> solution.GetDocument + +/// The position of the last character of the name's first occurrence. +let private positionOf (document: Document) (name: string) = + let text = document.GetTextAsync(CancellationToken.None).Result.ToString() + text.IndexOf(name, StringComparison.Ordinal) + name.Length - 1 + +let private definitionAt (document: Document) (name: string) = + GoToDefinition(FSharpMetadataAsSourceService()).FindDefinitionAtPosition(document, positionOf document name) + |> CancellableTask.runSynchronouslyWithoutCancellation + +[] +let ``goto definition stays external when only an unrelated file ends as the mapped name does`` () = + let binary = mappedBinary.Value + + let consumer = documentAt binary.WithoutSource (binary.Consumer.GetFilePath "App") + + match definitionAt consumer "ModuleLibrary.value" with + | ValueSome(FSharpGoToDefinitionResult.ExternalAssembly _, _) -> () + | result -> failwith $"expected the external assembly, got %A{result}" + +/// The assembly was built from a signature and an implementation. From an implementation file the definition +/// is the one in the implementation, though the symbol declares itself in the signature; from a signature +/// file it is the one in the signature. +[] +[] +[] +let ``goto definition into a mapped assembly with a signature reaches the file of the origin's kind`` + (fromSignature: bool) + (name: string) + (expected: string) + = + let binary = mappedBinary.Value + + let origin, destination = + if fromSignature then + binary.Consumer.GetSignatureFilePath "App", binary.Library.GetSignatureFilePath "Library" + else + binary.Consumer.GetFilePath "App", binary.Library.GetFilePath "Library" + + match definitionAt (documentAt binary.WithSource origin) name with + | ValueSome(FSharpGoToDefinitionResult.NavigableItem item, _) -> + Assert.Equal(destination, item.Document.FilePath) + Assert.Equal(expected, item.Document.GetTextAsync(CancellationToken.None).Result.ToString(item.SourceSpan)) + | result -> failwith $"expected a navigable item, got %A{result}" + +/// The assembly records where a type is declared in the signature only, so from an implementation file its +/// source gives no location: the definition is then the external one it was before a mapped name reached a +/// document, not none at all. +[] +let ``goto definition falls back to the assembly when the mapped source gives no location`` () = + let binary = mappedBinary.Value + let consumer = documentAt binary.WithSource (binary.Consumer.GetFilePath "App") + + match definitionAt consumer "ModuleLibrary.Kind" with + | ValueSome(FSharpGoToDefinitionResult.ExternalAssembly _, _) -> () + | result -> failwith $"expected the external assembly, got %A{result}" + +/// The spans Find All References reports the declaration of the library's value at. +let private declarationSpans (solution: Solution) (binary: MappedBinary) = + cancellableTask { + let consumer = documentAt solution (binary.Consumer.GetFilePath "App") + let! cancellationToken = CancellableTask.getCancellationToken () + let! text = consumer.GetTextAsync cancellationToken + let position = positionOf consumer "ModuleLibrary.value" + let line = text.Lines.GetLineFromPosition(position).ToString() + let linePosition = text.Lines.GetLinePosition position + let names = [ binary.Library.Name; "ModuleLibrary"; "value" ] + let! _, checkResults = consumer.GetFSharpParseAndCheckResultsAsync "test" + + let symbolUse = + checkResults.GetSymbolUseAtLocation(linePosition.Line + 1, linePosition.Character + 1, line, names) + |> Option.defaultWith (fun () -> failwith "the consumer does not use the library's value") + + match checkResults.GetDeclarationLocation(linePosition.Line + 1, linePosition.Character + 1, line, names, false) with + | FindDeclResult.DeclFound range -> + return! FSharpFindUsagesService.rangeToDocumentSpans (solution, range, "value", symbolUse.Symbol.Assembly.SimpleName) + | result -> return failwith $"expected the declaration of the library's value, got %A{result}" + } + |> CancellableTask.runSynchronouslyWithoutCancellation + +[] +let ``find all references reports a mapped declaration only in the source of its own assembly`` () = + let binary = mappedBinary.Value + + Assert.Empty(declarationSpans binary.WithoutSource binary) + + let span = declarationSpans binary.WithSource binary |> Array.exactlyOne + Assert.Equal(binary.Library.GetFilePath "Library", span.Document.FilePath) + +[] +let ``find all references keeps the use of a mapped symbol when only an unrelated file ends as its name does`` () = + let binary = mappedBinary.Value + + let consumer = documentAt binary.WithoutSource (binary.Consumer.GetFilePath "App") + + let definitions = ConcurrentBag() + let references = ConcurrentBag() + + let context = + { new IFSharpFindUsagesContext with + member _.OnDefinitionFoundAsync(definition: FSharpDefinitionItem) = + definitions.Add definition + Task.CompletedTask + + member _.OnReferenceFoundAsync(reference: FSharpSourceReferenceItem) = + references.Add reference + Task.CompletedTask + + member _.ReportMessageAsync _ = Task.CompletedTask + member _.ReportProgressAsync(_, _) = Task.CompletedTask + member _.SetSearchTitleAsync _ = Task.CompletedTask + member _.CancellationToken = CancellationToken.None + } + + FSharpFindUsagesService.findReferencedSymbolsAsync (consumer, positionOf consumer "ModuleLibrary.value", context, true, "test") + |> CancellableTask.runSynchronouslyWithoutCancellation + + // What an item holds cannot be read from here: the declaration is counted, and so is the consumer's one use. + Assert.Equal(1, definitions.Count) + Assert.Equal(1, references.Count)