Match a path-mapped file name against the solution, not the current directory

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, 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))

vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs

@@ -3,7 +3,89 @@ 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
+
+/// 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 segments = ResizeArray<string>()
+    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.
+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) =
+    match path, fileName with
+    | null, _
+    | _, null -> false
+    | path, rooted when isRootedPath rooted -> String.Equals(Path.GetFullPathSafe rooted, path, StringComparison.OrdinalIgnoreCase)
+    | path, relative ->
+        match mappedNameTail relative with
+        | ValueSome tail -> endsWithMappedName path tail
+        | ValueNone -> false
+
+/// 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<Solution, ConcurrentDictionary<string, DocumentId list>>()
+
+/// 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
 
@@ -80,12 +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 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) =
-
-        let filePath = System.IO.Path.GetFullPathSafe range.FileName
+    /// 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
@@ -107,13 +221,12 @@ type Solution with
                         matchingDoc tail
                 | None -> Some docId
 
-        self.GetDocumentIdsWithFilePath filePath |> List.ofSeq |> 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

vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs

@@ -63,11 +63,17 @@ type FSharpSymbolUse with
                     // should be treated as an individual project
                     Some(SymbolScope.Projects([ currentDocument.Project ], isSymbolLocalForProject))
                 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 =
-                        currentDocument.Project.Solution.GetDocumentIdsWithFilePath(filePath)
+                        solution.GetDocumentIdsWithFSharpFileName(loc.FileName, this.Symbol.Assembly.SimpleName)
                         |> Seq.map (fun x -> x.ProjectId)
                         |> Seq.distinct
-                        |> Seq.map currentDocument.Project.Solution.GetProject
+                        |> Seq.map solution.GetProject
                         |> Seq.toList
 
                     match projects with

vsintegration/src/FSharp.Editor/Navigation/FindUsagesService.fs

@@ -59,12 +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 {
-                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, in the
+                // projects that compile the assembly.
+                let documentIds =
+                    solution.GetDocumentIdsWithFSharpFileName(range.FileName, declaringAssembly)
 
                 let! spans =
                     seq {
@@ -119,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 =