Skip to content

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

Open
xperiandri wants to merge 7 commits into
dotnet:mainfrom
xperiandri:fix/pathmap-relative-document-names
Open

Match a path-mapped file name against the solution, not the current directory#20519
xperiandri wants to merge 7 commits into
dotnet:mainfrom
xperiandri:fix/pathmap-relative-document-names

Conversation

@xperiandri

Copy link
Copy Markdown
Contributor

Go To Definition, Find All References and Rename on a symbol whose assembly was built with a path map
DeterministicSourcePaths, or a <PathMap> set in Directory.Build.props — open generated
metadata instead of the symbol's source.

Why

A path-mapped assembly names each source file relative to the map's root, and does not record the
root. TryGetDocumentIdFromFSharpRange and FSharpSymbolUse.GetSymbolScope resolved that name with
Path.GetFullPath, that is, against Environment.CurrentDirectory. The current directory belongs to
the process, not to the solution, and holds whatever the last component to set it left there; the
path it produced named a file that does not exist, no document matched, and the symbol was taken for
an external one.

This is the editor half of the path-map fix. #20518 stops the compiler naming the directory twice in
GetDeclarationLocation, which is what reaches the editor in the first place; #20470 keeps the map out
of the options the IDE builds, which covers references between projects held in memory. A referenced
assembly on disk was built by MSBuild with the map, and still carries the mapped names.

The change

Solution.GetDocumentIdsWithFSharpFileName answers which documents a compiler range's file name
denotes:

  • a rooted name goes through the workspace's index, as before, so nothing changes for a build without
    a map;
  • a relative name is matched by its tail against the paths the solution's documents already have,
    anchored on a separator so that it matches whole directories and never the tail of one. The rule is
    isTheFileAt, which also accepts the doubled separator the compiler writes when the map's
    replacement ends in one (.\ + \src\…).

TryGetDocumentIdFromFSharpRange — Go To Definition, and the declaration document Find All
References starts from, through TryGetDocumentFromFSharpRange — and GetSymbolScope, which decides
the projects Find All References and Rename search, now go through it.

The scan over the solution's documents runs only for a relative name, which means only for a symbol
declared in a path-mapped assembly, and once per navigation: the ranges of the references found are
the solution's own and rooted. Paths are compared case-insensitively; they are not identifiers.

Tests: the rule, including that a name inside a segment matches nothing; and a range whose name a map
left relative finding its document. On the branch this was developed on, running them without the
change fails the second with no document is named by .\Library_…\FileLibrary.fs. On this branch
PathMapNavigationTests pass (9), as do the Go To Definition (3) and Find References (7) tests.

Two lookups in other pending work resolve the same kind of name and need the same rule:
GetSolutionDocumentsWithFilePath in #20462, and the comparison FindSymbolDeclarationInDocument
makes in #20492. Whichever of those merges after this will route through the helper.

Base

Stacked on #20470, which adds PathMapNavigationTests.fs; the diff shrinks to its own commit once
that merges.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

❗ Release notes required

You can open this PR in browser to add release notes: open in github.dev


✅ Found changes and release notes in following paths:

Change path Release notes path Description
`vsintegration/src` docs/release-notes/.VisualStudio/18.vNext.md

xperiandri added a commit to xperiandri/fsharp that referenced this pull request Sep 11, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the ⚠️ Affects-Design-Time Tooling check: PR touches type providers or dependency manager label Sep 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Tooling Safety Check — Affects-Design-Time
Affects-Design-Time: Changes IDE symbol navigation behavior.

Generated by PR Tooling Safety Check · gpt56 1.4M ·

else
let projects =
currentDocument.Project.Solution.GetDocumentIdsWithFilePath(filePath)
currentDocument.Project.Solution.GetDocumentIdsWithFSharpFileName loc.FileName

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖🕵️ [P1] Find All References loses the consumer's reference when an unrelated document has the mapped suffix. The executed service returns 0 references here; base returns 1. The suffix match selects only Unrelated and bypasses the assembly-reference fallback. Check the defining assembly before restricting the scope.

// C:\package\Library.fs -> ExternalLibrary.dll, --pathmap:C:\package=.
module ExternalLibrary
let value = 42

// Consumer/App.fs; references ExternalLibrary.dll
module Consumer
let result = ExternalLibrary.value // Find All References on value

// Unrelated/Library.fs; separate solution project, no reference to the DLL
module Unrelated
let value = 99

member self.GetDocumentIdsWithFSharpFileName(fileName: string) =
match fileName with
| null -> []
| rooted when Path.IsPathRooted rooted -> self.GetDocumentIdsWithFilePath(Path.GetFullPathSafe rooted) |> List.ofSeq

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖🕵️ [P2] Foreign mapped filenames now throw during document lookup on the editor's .NET Framework runtime. An imported DLL carrying this filename previously returned no matches; Path.IsPathRooted now throws before GetFullPathSafe can protect the lookup. Keep invalid/non-native filenames on a non-throwing path.

Imported declaration filename: /home/build/a|b/Library.fs
Base: 0 matching documents
HEAD: System.ArgumentException: Illegal characters in path.

[
for project in self.Projects do
for document in project.Documents do
if relative |> isTheFileAt document.FilePath then

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖🕵️ [P2] Repeated external-declaration lookups add seconds and gigabytes of allocations to Find All References. Project.FindFSharpReferencesAsync resolves the same declaration once per searched project, so a missing mapped source repeats this whole-solution scan. On desktop CLR, 201 lookups over 20,000 documents measured 2.72 s and 1.91 GB allocated, versus 0.37 ms and 32 KB before. Normalize the suffix once and reuse lookup results, including misses, across the search.

// ExternalLibrary.dll was built from Library.fs with --pathmap:C:\package=.\
// Solution: 200 projects, 100 documents each, all referencing that DLL.
// Library.fs is not in the solution. Find All References on value:
let result = ExternalLibrary.value
// Repeated declaration lookup input: .\\Library.fs

@T-Gro
T-Gro self-requested a review September 11, 2026 12:59
@T-Gro T-Gro added the AI-reviewed PR reviewed by AI review council label Sep 11, 2026
xperiandri and others added 6 commits September 11, 2026 17:51
…r 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 <noreply@anthropic.com>
A project built with DeterministicSourcePaths or an explicit PathMap hands
the IDE a `--pathmap:` option. FCS applies the map when it pickles the ranges
of the in-memory reference other projects check against, so every symbol
imported from such a project names a mapped, relative file that no workspace
document has, and Go To Definition ends in the generated signature instead of
the source. The map is a property of the build output; the IDE now drops it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@xperiandri
xperiandri force-pushed the fix/pathmap-relative-document-names branch from 1482407 to 8075288 Compare September 11, 2026 16:16
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 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⚠️ Affects-Design-Time Tooling check: PR touches type providers or dependency manager AI-reviewed PR reviewed by AI review council

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

2 participants