Name a declaration from a path-mapped assembly from its root once

docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

@@ -1,6 +1,8 @@
 ### Fixed
 
 * Fix internal error FS0192 "Iterate2D" when a `[<ReflectedDefinition(true)>]` parameter auto-quotes an argument that captures a not-yet-generalized use of an inferred generically-recursive function. The auto-quoted (`Expr.WithValue`) copy now keeps a fresh link to the recursive-value use so it receives the same inferred type arguments as the executable expression at the letrec point. ([Issue #20379](https://github.com/dotnet/fsharp/issues/20379))
+* `GetDeclarationLocation` no longer names the compile-time directory twice for a declaration in an assembly built with `--pathmap` (as `DeterministicSourcePaths` sets it). Such a build maps the directory it compiled in as well as the file names of its ranges, so both already reach the same root and joining them repeated the directory, leaving a path that matches no file.
+* `GetDeclarationLocation` no longer names the compile-time directory twice for a declaration in an assembly built with `--pathmap` (as `DeterministicSourcePaths` sets it). Such a build maps the directory it compiled in as well as the file names of its ranges, so both already reach the same root and joining them repeated the directory, leaving a path that matches no file. ([PR #20518](https://github.com/dotnet/fsharp/pull/20518))
 * Fix `NativePtr.stackalloc` nested in a larger expression (e.g. a call argument or the right of an assignment) producing an assembly that throws `InvalidProgramException` at load. ([Issue #8083](https://github.com/dotnet/fsharp/issues/8083), [PR #20302](https://github.com/dotnet/fsharp/pull/20302))
 * Fix internal error "Unexpected generalized type variables when compiling an active pattern" when an active pattern is used in a `let` binding whose right-hand side is a generic value, e.g. `let (T) = id`. Such a binding is now checked like the equivalent `match` and is not generalized. ([Issue #16856](https://github.com/dotnet/fsharp/issues/16856), [PR #20383](https://github.com/dotnet/fsharp/pull/20383))
 * Fix Release-only (`--optimize+`) `System.InvalidProgramException` from `Seq.collect` / `yield!` over a value-type (struct) collection implementing `seq<'T>` (e.g. `ImmutableArray<_>`) when materialised with `List.ofSeq` / `Seq.toList` / `Seq.toArray` or a list/array comprehension. The collector lowering now boxes a struct sub-collection to `seq<'T>` before calling `AddMany`/`AddManyAndClose` (matching the coercion the type checker already inserts for `yield!`), and uses `unit` as the try/finally result type instead of the body type (removing a spurious `ldnull` store). ([Issue #20203](https://github.com/dotnet/fsharp/issues/20203))
@@ -148,6 +150,7 @@
 * Import: Don't walk non-F# assemblies when labelling trait constraint sources (PR [#20090](https://github.com/dotnet/fsharp/pull/20090))
 * Avoid per-instance lock object in InterruptibleLazy and DelayInitArrayMap (PR [#20088](https://github.com/dotnet/fsharp/pull/20088))
 * IL: fix leaking binary view ([PR #20250](https://github.com/dotnet/fsharp/pull/20250))
+* A project reusing cached framework imports is checked with its own `--pathmap` instead of the map of the project that filled the cache, so the ranges of its in-memory reference data name its real files. ([Issue #20474](https://github.com/dotnet/fsharp/issues/20474), [PR #20476](https://github.com/dotnet/fsharp/pull/20476))
 
 ### Added
 

src/Compiler/Service/IncrementalBuild.fs

@@ -557,12 +557,13 @@ type FrameworkImportsCache(size) =
         let node = this.GetNode(tcConfig, frameworkDLLs, nonFrameworkResolutions)
         let! tcGlobals, frameworkTcImports = node.GetOrComputeValue()
 
-        // If the tcGlobals was loaded from a different project, langVersion and realsig may be different
-        // for each cached project.  So here we create a new tcGlobals, with the existing framework values
-        // and updated realsig and langversion
+        // If the tcGlobals was loaded from a different project, langVersion, realsig and pathMap may be
+        // different for each cached project.  So here we create a new tcGlobals, with the existing framework
+        // values and the updated realsig, langversion and pathMap
         let tcGlobals =
             if tcGlobals.langVersion <> tcConfig.langVersion
-                || tcGlobals.realsig <> tcConfig.realsig then
+                || tcGlobals.realsig <> tcConfig.realsig
+                || tcGlobals.pathMap <> tcConfig.pathMap then
                     TcGlobals(
                         tcGlobals.compilingFSharpCore,
                         tcGlobals.ilg,
@@ -574,7 +575,7 @@ type FrameworkImportsCache(size) =
                         tcGlobals.tryFindSysTypeCcuHelper,
                         tcGlobals.emitDebugInfoInQuotations,
                         tcGlobals.noDebugAttributes,
-                        tcGlobals.pathMap,
+                        tcConfig.pathMap,
                         tcConfig.langVersion,
                         tcConfig.realsig,
                         tcConfig.compilationMode

src/Compiler/Service/TransparentCompiler.fs

@@ -964,13 +964,14 @@ type internal TransparentCompiler
                 // Prepare the frameworkTcImportsCache
                 let! tcGlobals, frameworkTcImports = ComputeFrameworkImports tcConfig frameworkDLLs nonFrameworkResolutions
 
-                // If the tcGlobals was loaded from a different project, langVersion and realsig may be different
-                // for each cached project.  So here we create a new tcGlobals, with the existing framework values
-                // and updated realsig and langversion
+                // If the tcGlobals was loaded from a different project, langVersion, realsig and pathMap may be
+                // different for each cached project.  So here we create a new tcGlobals, with the existing
+                // framework values and the updated realsig, langversion and pathMap
                 let tcGlobals =
                     if
                         tcGlobals.langVersion <> tcConfig.langVersion
                         || tcGlobals.realsig <> tcConfig.realsig
+                        || tcGlobals.pathMap <> tcConfig.pathMap
                     then
                         TcGlobals(
                             tcGlobals.compilingFSharpCore,
@@ -983,7 +984,7 @@ type internal TransparentCompiler
                             tcGlobals.tryFindSysTypeCcuHelper,
                             tcGlobals.emitDebugInfoInQuotations,
                             tcGlobals.noDebugAttributes,
-                            tcGlobals.pathMap,
+                            tcConfig.pathMap,
                             tcConfig.langVersion,
                             tcConfig.realsig,
                             tcConfig.compilationMode

src/Compiler/Symbols/SymbolHelpers.fs

@@ -230,14 +230,20 @@ module internal SymbolHelpers =
     let fileNameOfItem (g: TcGlobals) qualProjectDir (m: range) h =
         let file = m.FileName
         if verbose then dprintf "file stored in metadata is '%s'\n" file
+
+        // A build that maps its source paths maps the directory it compiled in as well, so that directory
+        // and the file name reach the same unrecorded root on their own: joining them repeats the directory.
+        let underDirectory (dir: string) =
+            if FileSystem.IsPathRootedShim dir then Path.Combine(dir, file) else file
+
         if not (FileSystem.IsPathRootedShim file) then
             match ccuOfItem g h with
             | Some ccu ->
-                Path.Combine(ccu.SourceCodeDirectory, file)
+                underDirectory ccu.SourceCodeDirectory
             | None ->
                 match qualProjectDir with
                 | None     -> file
-                | Some dir -> Path.Combine(dir, file)
+                | Some dir -> underDirectory dir
          else file
 
     let ParamNameAndTypesOfUnaryCustomOperation g minfo =

tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj

@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8"?>
+<?xml version="1.0" encoding="utf-8"?>
 <!-- Copyright (c) Microsoft Corporation.  All Rights Reserved.  See License.txt in the project root for license information. -->
 <Project Sdk="Microsoft.NET.Sdk">
 
@@ -540,6 +540,7 @@
     <Compile Include="Signatures\MemberTests.fs" />
     <Compile Include="StaticLinking\StaticLinking.fs" />
     <Compile Include="FSharpChecker\CommonWorkflows.fs" />
+    <Compile Include="FSharpChecker\PathMap.fs" />
     <Compile Include="FSharpChecker\ProjectSnapshot.fs" />
     <Compile Include="FSharpChecker\TransparentCompiler.fs" />
     <Compile Include="FSharpChecker\SymbolUse.fs" />

tests/FSharp.Compiler.ComponentTests/FSharpChecker/PathMap.fs

@@ -0,0 +1,114 @@
+module FSharpChecker.PathMap
+
+open System.IO
+open System.Threading.Tasks
+open Xunit
+open FSharp.Test.ProjectGeneration
+open FSharp.Compiler.CodeAnalysis
+open FSharp.Compiler.EditorServices
+open FSharp.Compiler.Text
+
+let private checkWith (checker: FSharpChecker) (project: SyntheticProject) =
+    ProjectWorkflowBuilder(project, checker = checker).Yield() |> Async.Ignore
+
+/// The framework imports, and the TcGlobals with them, are cached per framework set; the path map of
+/// the project that filled the cache must not reach the ranges a sibling exposes to its consumers.
+[<Theory>]
+[<InlineData(false)>]
+[<InlineData(true)>]
+let ``a sibling's path map does not reach the ranges of a project without one`` (useTransparentCompiler: bool) : Task =
+    task {
+        let checker = FSharpChecker.Create(useTransparentCompiler = useTransparentCompiler)
+        let library = SyntheticProject.Create("Library", sourceFile "Library" [])
+
+        let mapped =
+            { SyntheticProject.Create("Mapped", sourceFile "Mapped" []) with
+                OtherOptions = [ $"--pathmap:{Path.GetDirectoryName library.ProjectDir}=.\\" ] }
+
+        let app =
+            { SyntheticProject.Create("App", sourceFile "App" [ "Library" ]) with
+                DependsOn = [ library ] }
+
+        do! checkWith checker mapped
+        do! checkWith checker app
+
+        let appFile = app.GetFilePath "App"
+
+        let! _, answer =
+            checker.ParseAndCheckFileInProject(
+                appFile,
+                0,
+                SourceText.ofString (File.ReadAllText appFile),
+                app.GetProjectOptions checker
+            )
+
+        let checkResults =
+            match answer with
+            | FSharpCheckFileAnswer.Succeeded checkResults -> checkResults
+            | FSharpCheckFileAnswer.Aborted -> failwith "the check was aborted"
+
+        let libraryFunction =
+            checkResults.GetAllUsesOfAllSymbolsInFile()
+            |> Seq.tryFind (fun symbolUse -> symbolUse.Symbol.FullName = $"{library.Name}.ModuleLibrary.f")
+            |> Option.defaultWith (fun () ->
+                failwith
+                    $"""ModuleLibrary.f not used; symbols: %A{checkResults.GetAllUsesOfAllSymbolsInFile() |> Seq.map _.Symbol.FullName |> Seq.distinct |> List.ofSeq}""")
+
+        Assert.Equal(library.GetFilePath "Library", libraryFunction.Symbol.DeclarationLocation.Value.FileName)
+    }
+
+/// A build that maps its source paths maps the directory it compiled in as well, so the file name of a
+/// range and that directory both reach the same root on their own. Joining them names the directory twice.
+[<Theory>]
+[<InlineData(false)>]
+[<InlineData(true)>]
+let ``a declaration in a project with a path map is named from the root once`` (useTransparentCompiler: bool) : Task =
+    task {
+        let checker = FSharpChecker.Create(useTransparentCompiler = useTransparentCompiler)
+        let library = SyntheticProject.Create("MappedLibrary", sourceFile "Library" [])
+        let root = Path.GetDirectoryName library.ProjectDir
+
+        let library =
+            { library with
+                OtherOptions = [ $"--pathmap:{root}=.{Path.DirectorySeparatorChar}" ] }
+
+        let app =
+            { SyntheticProject.Create("MappedApp", sourceFile "App" [ "Library" ]) with
+                DependsOn = [ library ] }
+
+        do! checkWith checker library
+        do! checkWith checker app
+
+        let appFile = app.GetFilePath "App"
+        let appLines = File.ReadAllLines appFile
+
+        let! _, answer =
+            checker.ParseAndCheckFileInProject(
+                appFile,
+                0,
+                SourceText.ofString (File.ReadAllText appFile),
+                app.GetProjectOptions checker
+            )
+
+        let checkResults =
+            match answer with
+            | FSharpCheckFileAnswer.Succeeded checkResults -> checkResults
+            | FSharpCheckFileAnswer.Aborted -> failwith "the check was aborted"
+
+        let usage =
+            checkResults.GetAllUsesOfAllSymbolsInFile()
+            |> Seq.find (fun symbolUse -> symbolUse.Symbol.FullName = $"{library.Name}.ModuleLibrary.f")
+
+        let declaration =
+            checkResults.GetDeclarationLocation(
+                usage.Range.EndLine,
+                usage.Range.EndColumn,
+                appLines[usage.Range.EndLine - 1],
+                [ "ModuleLibrary"; "f" ]
+            )
+
+        match declaration with
+        | FindDeclResult.DeclFound range ->
+            Assert.Equal(library.GetFilePath "Library", Path.GetFullPath(Path.Combine(root, range.FileName)))
+        | result -> failwith $"expected the declaration of ModuleLibrary.f, got %A{result}"
+    }