Skip to content

Show C# and VB uses of an F# symbol in Find All References - #20463

Open
xperiandri wants to merge 10 commits into
dotnet:mainfrom
xperiandri:feature/find-references-csharp
Open

xperiandri wants to merge 10 commits into
dotnet:mainfrom
xperiandri:feature/find-references-csharp

Conversation

@xperiandri

@xperiandri xperiandri commented Sep 6, 2026 •

Copy link
Copy Markdown
Contributor

Find All References on an F# symbol never listed its call sites in C# or VB projects; Find Implementations and Rename keep the F#-only scope they had.

After the F# search reports its own uses, the same lookup runs against every C# or VB project that references the assembly a project declaring the symbol builds, and the locations are reported under the same F# definition item, after the F# uses, so the window keeps its order. A symbol internal to its project or declared in an external assembly is skipped, and a lookup that cannot resolve — no compilation, unbuilt or stale assembly — yields no C# results rather than failing.

Known limits: a union case resolves to its nested type, so U.NewCase(…) calls are not found; conversion operators and active patterns have no compiled name Roslyn can look up.

The first two commits are shared with #20462 and #20464.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Sep 6, 2026 •

Copy link
Copy Markdown
Contributor

✅ Release notes checked


✅ 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 6, 2026
@github-actions github-actions Bot added ⚠️ Affects-Build-Infra Tooling check: PR touches build infrastructure ⚠️ Affects-Restore Tooling check: PR touches NuGet packages or feeds labels Sep 6, 2026
@github-actions

This comment has been minimized.

@T-Gro T-Gro left a comment

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.

🤖 🕵️ AI review — verify independently.

match! project.GetCompilationAsync cancellationToken with
| null -> return Seq.empty
| compilation ->
match DocumentationCommentId.GetFirstSymbolForDeclarationId(docId, compilation) with

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] Find All References reports the unrelated C# method and misses the F# call. With the F# reference aliased as FSLib, the editor search returns Other()'s Value span instead of Real()'s. Resolve the ID within the declaring assembly rather than taking the compilation-wide first match.

// F# library
namespace Collision
type Widget() =
    static member Value() = 1
// Reference the F# library with alias FSLib.
extern alias FSLib;
namespace Collision {
    public class Widget { public static int Value() => 2; }
}
class Consumer {
    int Real() => FSLib::Collision.Widget.Value(); // missed
    int Other() => Collision.Widget.Value();      // incorrectly reported
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The described shape is real: DocumentationCommentId.GetFirstSymbolForDeclarationId searches the compilation and answers with the first symbol carrying that id, so when a C# project declares a type with the same namespace-qualified name as the F# one it references, the F# symbol can lose the race to the C# one and the search reports uses of the wrong member.

Worth being exact about the scope, though. An extern alias is not what causes it — aliases change name resolution in the C# source, not the identity of the symbols in the compilation — so the collision is simply "two symbols in the referencing compilation share a documentation comment id", which an alias only makes convenient to write. It needs the C# project to declare Collision.Widget.Value itself while also referencing the F# assembly that declares it; that is unusual but nothing prevents it.

The fix is to resolve within the assembly the id came from rather than compilation-wide: take the IAssemblySymbol for the referenced F# project's output from the referencing compilation and ask it for the id, falling back to the compilation-wide lookup only when the reference cannot be identified. I will write it that way, with the two-declaration fixture from your repro as the test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in ec64e8a505, the way you asked: resolved within the declaring assembly rather than compilation-wide.

findRoslynReferences takes the declaring project's AssemblyName — the caller already has that project, since it is the one whose references decide which consumers are searched — and picks from DocumentationCommentId.GetSymbolsForDeclarationId the symbol whose ContainingAssembly is that one. No fallback to the first match: if the id resolves only to a symbol of some other assembly, that symbol is not what the search was asked about, and reporting its uses is the bug this thread is about.

I did not add a test for it. The fixture would need a consumer that declares the colliding type and references the F# assembly, which is a second C# project plus an alias in the test host's compilation options, and AddCSharpProject takes neither today. If you want it covered rather than argued, say so and I will extend the helper — otherwise the enum case above is the one with a test and this one rests on the resolution being restricted by construction.

@T-Gro
T-Gro self-requested a review September 9, 2026 08:58
@T-Gro T-Gro added the AI-reviewed PR reviewed by AI review council label Sep 9, 2026
| docSig when value.LiteralValue.IsSome && docSig.StartsWith("P:", StringComparison.Ordinal) -> $"F:{docSig.Substring 2}"
| docSig -> docSig
| :? FSharpEntity as entity -> entity.XmlDocSig
| :? FSharpField as field -> field.XmlDocSig

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.

🤖🕵️ Enum IDs do not resolve: P:N.Color.Red returns null, but F:N.Color.Red resolves the field.

namespace N
type Color = Red = 0 | Blue = 1

Map enum-field IDs to F: and cover C#/VB callers.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confirmed, and the cause is on our side of the mapping. docCommentIdOf builds the id from the F# symbol's kind, and an enum case comes back as a property (P:) because that is how it is surfaced, while Roslyn's DocumentationCommentId spells an enum member as a field — F:N.Color.Red. So the lookup asks for an id that cannot exist and gets nothing; the case is silently missing from Find All References rather than wrong, which is why it went unnoticed.

Fix is to spell an enum case as F: when building the id, and the test belongs next to the C# and VB caller cases that already exercise the mapping — an F# enum, a C# use of Color.Red, and the assertion that the use is reported.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in ec64e8a505. DocumentationCommentId now spells an enum case as F:, the same way it already spelled a literal, so F:N.Color.Red is what the consumer compilation is asked for.

Covered by a new case in DocumentationCommentId is the compiled form Roslyn resolves — the fixture gained an enum and the theory asserts F:{module}.Color.Red for Red. The 10 tests in FindReferencesFromCSharpTests pass.

xperiandri added a commit to xperiandri/fsharp that referenced this pull request Sep 11, 2026
@xperiandri
xperiandri force-pushed the feature/find-references-csharp branch from 8b67667 to 450d60b Compare September 11, 2026 16:18
@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

This comment has been minimized.

@github-actions

This comment has been minimized.

@T-Gro T-Gro left a comment

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.

🤖🕵️ If this fixes an issue or implements an RFC/suggestion, link it (Fixes #... when applicable). Otherwise, give a short management-level summary in simplified technical English: what user scenario improves and what this achieves.

Please apply this PR-description guidance. Remove the implementation inventory already visible in Files, but keep necessary scope, compatibility, and dependency caveats.

@github-project-automation github-project-automation Bot moved this from New to In Progress in F# Compiler and Tooling Sep 14, 2026
xperiandri added a commit to xperiandri/fsharp that referenced this pull request Sep 14, 2026
@xperiandri
xperiandri force-pushed the feature/find-references-csharp branch from 450d60b to f629e84 Compare September 14, 2026 15:31
@xperiandri
xperiandri requested a review from T-Gro September 14, 2026 15:39
xperiandri added a commit to xperiandri/fsharp that referenced this pull request Sep 22, 2026
@xperiandri
xperiandri force-pushed the feature/find-references-csharp branch from f629e84 to 6ba05f4 Compare September 22, 2026 12:07
xperiandri and others added 4 commits September 26, 2026 03:01
…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>
The IFSharpFindUsagesContext stub of FindReferencesTests moves to
RoslynTestHelpers.CreateFindUsagesContext so other test files can collect the
definitions and references a search reports.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The test host served language services for F# only. It now creates them for
any language from the same export provider, imports the C# workspace parts,
accepts .cs documents, and gains two helpers: CompileToAssembly builds a
synthetic project into its OutputFilename with the checker's options, and
AddCSharpProject adds a C# library referencing the framework of the F# options
and given assemblies. The assemblies are added after AdhocWorkspace.AddProject,
which would otherwise rewrite a reference to a project's output into a project
reference; VS keeps a C# → F# reference as metadata.

The smoke tests check that the C# compilation resolves the documentation
comment id of an F# function, that SymbolFinder finds its call site, and that
ProjectFiltering sees the C# project as a consumer of the F# assembly.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Find All References searched F# documents only: C# and VB projects were never
in scope, so call sites of F# functions from C# were missing. After the F#
uses are reported, the symbol's documentation comment id (its XmlDocSig, the
compiled form Roslyn resolves) is looked up in the compilation of every C# or
VB project whose metadata references include the assembly of a project
declaring the symbol, and SymbolFinder.FindReferencesAsync reports the
locations under the F# definition item. Multi-targeted consumers report each
file span once.

Only Find All References does this: Find Implementations and Rename keep
their F#-only scope, and symbols internal to their project or declared in
external assemblies are skipped.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
xperiandri and others added 4 commits September 26, 2026 03:01
An F# library compiled to its assembly and a C# consumer calling its function:
Find All References on the F# declaration reports the C# call site, Find
Implementations does not, and DocumentationCommentId gives the compiled form
for a module, a function and nothing for a local.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
FCS names a module literal `P:` like any module value, but it compiles to a
const field, so `DocumentationCommentId.GetFirstSymbolForDeclarationId` found
nothing and Find All References showed no C# uses of it. The id now starts
with `F:` for a literal.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The cross-language search ran after the F# one and visited the consumers one
by one, so on a solution where the F# search takes minutes the C# call sites
were the last thing to appear. The search now starts before the F# one and
runs a few consumers at a time, each search building a compilation; the
results are still reported after the F# uses, each file span once, so the
order in the window is unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 26, 2026 01:01
@xperiandri
xperiandri force-pushed the feature/find-references-csharp branch from 6ba05f4 to 0470f62 Compare September 26, 2026 01:01
@github-actions

This comment has been minimized.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Fix exact assembly identity matching and preserve callback failure handling; add VB-specific coverage.

Review effort: Lite
Findings: 1 High severity · 1 Medium severity · 1 Low severity

Open (3)
What changed in this PR

Adds C# and VB cross-language Find All References results for public F# symbols.

Changes:

  • Resolves F# symbols via compiled documentation IDs and searches referencing projects.
  • Adds Roslyn test infrastructure and C# integration coverage.
  • Updates package references and release notes.
File Summary
vsintegration/​tests/​FSharp.Editor.Tests/​Helpers/​RoslynHelpers.fs Adds shared Roslyn test workspace helpers.
vsintegration/​tests/​FSharp.Editor.Tests/​FSharp.Editor.Tests.fsproj Registers tests and Roslyn workspace dependencies.
vsintegration/​tests/​FSharp.Editor.Tests/​FindReferencesTests.fs Updates shared test setup and helper usage.
vsintegration/​tests/​FSharp.Editor.Tests/​FindReferencesFromCSharpTests.fs Adds C# cross-language reference coverage.
vsintegration/​src/​FSharp.Editor/​Navigation/​FindUsagesService.fs Implements cross-language reference discovery and reporting.
vsintegration/​src/​FSharp.Editor/​LanguageService/​Symbols.fs Adds compiled documentation ID resolution.
eng/​Packages.props Updates the central Roslyn package version.
docs/​release-notes/​.VisualStudio/​18.vNext.md Documents the feature.

match declaringProject.OutputFilePath with
| null -> []
| outputFilePath ->
ProjectFiltering.getProjectsReferencingAssembly outputFilePath declaringProject.Solution

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in eea9dba5f2, though not by requiring an exact path everywhere: a consumer normally references the copy of the assembly in its own output, not the file the producer writes, so an exact match would find no consumers at all in the common case, which is why the filter was written on the file name.

What the file name cannot survive is a second project producing one named the same, which is your scenario. So the filter now asks that first — whether any other project of the solution has an OutputFilePath with this file name — and where it does, only the declaring project's own output path counts; where it does not, the name identifies the assembly as before.

Worth noting the second line of defence added in ec64e8a505 for the neighbouring thread: the documentation comment id is resolved among the symbols of the declaring assembly rather than compilation-wide, so even a consumer picked up wrongly no longer yields uses of another assembly's member unless that assembly also carries the same simple name.

let span = location.Location.SourceSpan

if reported.Add(struct (location.Document.FilePath, span)) then
do! onReferenceFoundAsync (FSharpSourceReferenceItem(definitionItem, FSharpDocumentSpan(location.Document, span)))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Correct, and fixed in eea9dba5f2 — the cross-language reporting wraps the callback the same way the F# path does, with the same reason written next to it: the window throws inside Roslyn on an item it will not take, and one such item must not end a search that has already found the rest.

Comment on lines +122 to +126
let ``Find All References on an F# function reports its C# call site`` () =
let context, foundDefinitions, foundReferences =
RoslynTestHelpers.CreateFindUsagesContext()

findUsagesService.FindReferencesAsync(fsharpDocument, declarationPosition, context).Wait()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fair as a statement about coverage, and I would rather be exact about what is and is not language-specific here.

Nothing in the change knows a language. The consumers are chosen by their metadata references, the symbol is found with DocumentationCommentId against whatever Compilation the project produces, and the uses come from SymbolFinder.FindReferencesAsync. The one place a language could differ is how its compilation spells a documentation comment id, and that is Roslyn's own code, shared by C# and Visual Basic.

Adding the coverage is not free: FSharp.Editor.Tests loads Microsoft.CodeAnalysis.CSharp.Workspaces only, so a Visual Basic consumer needs the VB workspaces package in the test host and an AddVisualBasicProject beside AddCSharpProject in RoslynTestHelpers — a test-host dependency added for one assertion.

Happy to do it if the team wants Visual Basic covered rather than reasoned about; otherwise I will narrow the PR text to say the search is language-agnostic and that C# is what the tests exercise. Say which you prefer.

xperiandri and others added 2 commits September 26, 2026 04:16
… field

Two ways the search reached the wrong symbol, or none.

A documentation comment id was resolved against the whole consumer
compilation, which answers with the first symbol carrying it.  A consumer that
declares a type of the same namespace-qualified name as one it references
therefore won the lookup, and the search reported uses of its own declaration
while missing the ones it was asked about.  The id is the F# assembly's, so it
is now asked of the symbols of that assembly.

An enum case compiles to a field, and FCS names it `P:` where Roslyn spells
`F:` - the same disagreement a literal already had.  The id named nothing, so a
C# or Visual Basic use of an F# enum case was quietly absent from the results
rather than wrong.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…g two assemblies

A consumer references the copy of an assembly in its own output rather than the
file the producer writes, so the file name is what identifies it - until another
project of the solution produces one named the same, when a consumer of that
other assembly is picked up instead.  The name is checked for that first now,
and where it is ambiguous only the declaring project's own output path counts.

The F# reporting path wraps the Find All References callback because the window
throws inside Roslyn on an item it will not take; the cross-language path
reported without that guard, so one such item ended a search that had already
found the rest.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Tooling Safety Check — Affects-Build-Infra, Affects-Design-Time, Affects-Restore
Affects-Build-Infra: Changes build scripts or MSBuild configuration.
Affects-Design-Time: Changes editor or language-service behavior.
Affects-Restore: Changes package feeds, references, or restore configuration.

Generated by PR Tooling Safety Check · gpt56 1.9M · ◷

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⚠️ Affects-Build-Infra Tooling check: PR touches build infrastructure ⚠️ Affects-Design-Time Tooling check: PR touches type providers or dependency manager ⚠️ Affects-Restore Tooling check: PR touches NuGet packages or feeds AI-reviewed PR reviewed by AI review council

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

3 participants