Name a declaration from a path-mapped assembly from its root once - #20518
Open
xperiandri wants to merge 6 commits into
Open
Name a declaration from a path-mapped assembly from its root once#20518xperiandri wants to merge 6 commits into
xperiandri wants to merge 6 commits into
Conversation
…imports are cached FrameworkImportsCache keys the framework imports, and the TcGlobals built with them, by the framework set alone. A project reusing the entry got a fresh TcGlobals only when langVersion or realsig differed, and even then took pathMap from the cached instance. Since TypedTreePickle applies that map to every range it writes, the in-memory reference data of each project carried the --pathmap of whichever project filled the cache first, and a project without a map handed its consumers file names nothing on disk matches. pathMap now takes part in the decision like langVersion and realsig, and the new TcGlobals takes it from the project's own TcConfig, in the incremental builder and the transparent compiler alike. Fixes dotnet#20474 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ing project Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A build that maps its source paths - as DeterministicSourcePaths does - maps the directory it compiled in along with the file names of its ranges, so the two reach the same root and neither records what that root is. `fileNameOfItem` joined them anyway, on the assumption that the directory is absolute and the file name relative to it, and the declaration of a symbol imported from such an assembly came back naming the directory twice. Nothing matches a path like that, so Go To Definition reported the symbol as external and opened generated metadata instead of its source. Join only when the directory is rooted. When it is not, the file name already reaches the root on its own and is as complete as the metadata can make it. The IDE drops the path map from the options it builds, so this is reached through an assembly on disk - which is where a path map is normally applied, and which no change to those options can rewrite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
❗ Release notes requiredYou can open this PR in browser to add release notes: open in github.dev
|
github-actions
Bot
added
the
⚠️ Affects-Design-Time
Contributor
|
🔍 Tooling Safety Check — Affects-Design-Time
|
On Linux and macOS a backslash is part of a file name, so a map to .\ left the declaration's name unresolvable against the root there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Go To Definition into a sibling project of a solution built with a path map — the
<PathMap>$(MSBuildThisFileDirectory)=.\</PathMap>plusDeterministicSourcePathsthat manyDirectory.Build.propsset — opens generated metadata instead of the source. The declarationGetDeclarationLocationreturns names the compile-time directory twice:Nothing matches a path like that, so the editor takes the symbol for an external one.
Why
A build with a path map maps two things into the assembly's F# metadata: the file names of its
ranges, and the directory it compiled in, pickled as
compileTimeWorkingDirand read back asccu.SourceCodeDirectory.SymbolHelpers.fileNameOfItemthen joins the two withPath.Combine,on the assumption that the directory is absolute and the file name relative to it. Under a map
both are relative to the same root, which the assembly never records, so the file name already
reaches that root on its own and the join repeats the directory.
#20470 keeps the map out of the options the IDE builds, which covers references between projects
held in memory. It cannot cover this: a referenced assembly on disk was built by MSBuild with the map
and carries the mapped names permanently.
The change
Join only when the directory is rooted. When it is not, the file name is as complete as the metadata
can make it and is returned unchanged. A build without a map keeps an absolute directory and takes the
same path as before;
SourceCodeDirectory = "", which shared and IL-only CCUs use, gave the file nameback before and still does.
The test puts the map on the referenced project only and checks that the declaration, resolved from
the map's root, is the real file. Without the change it fails with the directory named twice:
fileNameOfItemhas one caller,GetDeclarationLocation. On the branch this was developed on, thefull
FSharp.Compiler.Service.Testsrun passes with it (3798, none failed).The editor still has to resolve a relative name against the solution rather than the process's
current directory; that is #20519.
Base
Stacked on #20476, which adds
FSharpChecker/PathMap.fs; the diff shrinks to its own commit once thatmerges.
🤖 Generated with Claude Code