Skip to content

Go To Definition from C# and VB into F# without checking the whole project - #20465

Open
xperiandri wants to merge 5 commits into
dotnet:mainfrom
xperiandri:perf/cross-language-gtd
Open

Go To Definition from C# and VB into F# without checking the whole project#20465
xperiandri wants to merge 5 commits into
dotnet:mainfrom
xperiandri:perf/cross-language-gtd

Conversation

@xperiandri

@xperiandri xperiandri commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Description

Go To Definition from a C# or VB file to a symbol declared in an F# project of the same solution opened Roslyn's decompiled view instead of the F# source while Visual Studio was cold; once the FCS caches were warm it navigated to source. Reproduced with the in-box tools on Visual Studio 18 Insiders on a solution with 135 project instances.

Root cause, measured in the debugger: FSharpCrossLanguageSymbolNavigationService.TryGetNavigableLocationAsync ran a full ParseAndCheckProject for every project whose assembly name matched — one per target-framework instance — with nothing cached, so Roslyn's navigation timed out and fell back to decompilation. A documentation comment id Roslyn could not parse still paid for the whole check.

Fix, in three commits:

  1. The service and its SymbolMemberType/SymbolPath/DocCommentId/FSharpNavigableLocation types move unchanged from GoToDefinition.fs into Navigation/CrossLanguageSymbolNavigation.fs, a file of their own. DocCommentIdToPath stays on the service for DocCommentIdParserTests.
  2. The service takes its dependencies through [<ImportingConstructor>] (FSharpMetadataAsSourceService, optional VisualStudioWorkspace) instead of Package.GetGlobalService. The lookup becomes: entity path of the doc comment id → parse-only scan of the project's documents through NavigateTo.GetNavigableItems on the FCS-cached parse results (throttled, compile order kept, so a signature comes before its implementation) → type check of the candidate document alone, using FSharpCheckFileResults.PartialAssemblySignature → exact XmlDocSig match on every candidate, then the existing name/shape heuristics on the last one. The whole-project check stays as the fallback. DocCommentId.None returns immediately; instances of one project file are tried heads first. CancellableTask.tryPick is added next to sequential.
  3. Tests.

Not touched: GoToDefinition, FSharpNavigation, the F# → C# branch, the tryFindVal* heuristics and the ExternalAccess contract.

Tests: CrossLanguageSymbolNavigationTests checks seventeen doc comment ids (module functions, [<CompiledName>], ModuleSuffix modules, generic types, records and their fields, union cases through their NewCase/IsCase/nullary-case compiled members, module literals, exceptions, constructors, properties) against the whole-project path and the declaration line; unknown members and types and a malformed id give nothing; declaresEntity on hand-built navigable items; candidate order [Second.fsi; Second.fs]; two instances of one project file pick the first. DocCommentIdParserTests passes unchanged.

No timings are claimed: per navigation the work goes from one full project check per target-framework instance to a parse-only scan plus one file check, with the full check only as a fallback.

Checklist

  • Test cases added

  • Performance benchmarks added in case of performance changes

  • Release notes entry updated:

    Please make sure to add an entry with short succinct description of the change as well as link to this pull request to the respective release notes file, if applicable.

    Release notes files:

    • If anything under src/Compiler has been changed, please make sure to make an entry in docs/release-notes/.FSharp.Compiler.Service/<version>.md, where <version> is usually "highest" one, e.g. 42.8.200
    • If language feature was added (i.e. LanguageFeatures.fsi was changed), please add it to docs/release-notes/.Language/preview.md
    • If a change to FSharp.Core was made, please make sure to edit docs/release-notes/.FSharp.Core/<version>.md where version is "highest" one, e.g. 8.0.200.

    Information about the release notes entries format can be found in the documentation.
    Example:

    If you believe that release notes are not necessary for this PR, please add NO_RELEASE_NOTES label to the pull request.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Sep 6, 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 6, 2026
@github-actions github-actions Bot added the AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files label Sep 6, 2026
return ValueNone
}

let! candidates =

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.

🤖🕵️⏱️🔥
Record cold before/after time from the reported 135-instance solution, including parsed documents, checked prefixes, and time to a source result before Roslyn falls back to metadata.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fixed in d3510b4 — restored PUBLISHED_ERROR_SLUGS gate in buildErrorDocUrl; unpublished slugs fall back to /errors. Set kept in sync with MDX via unit test.

[<InlineData("P:Widgets.Point.X", "type Point")>]
[<InlineData("M:Widgets.Point.#ctor(System.Int32,System.Int32)", "type Point")>]
[<InlineData("T:Widgets.MyError", "exception MyError")>]
let ``the fast path finds the declaration and agrees with the whole project check`` (docId: string, declaration: string) =

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.

🤖🕵️ Add a composed multi-file test that observes check scope, plus a conditional multi-target case where only a later target exposes the declaration.

xperiandri and others added 5 commits September 11, 2026 17:54
Pure move of the doc-comment-id types, FSharpNavigableLocation and
FSharpCrossLanguageSymbolNavigationService out of GoToDefinition.fs, compiled after
NavigateToSearchService.fs so the service can use the parsed navigable items cache.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…project

Go To Definition from C# or Visual Basic into an F# project ran a full ParseAndCheckProject for
every target-framework instance of the project whose assembly name matched, on every keystroke
of F12 and without any cache. Cold, that exceeded the time Roslyn waits for a cross-language
location and it fell back to its own decompiled view.

The parsed navigable items of a document name every type and module it declares, so the files
that can hold the declaration are known from parse results alone, without a type check. Only
those files are checked, one at a time in compile order, and the member is matched by its exact
compiled id first, with the name-and-shape heuristics reserved for the last candidate whose
partial signature holds every member of the entity. The whole-project check remains the
fallback. One instance per project file goes first; the service gets its dependencies through
the MEF constructor so the lookup runs against a plain Solution in tests.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Go To Definition from C# on `Shape.NewCircle(…)`, `shape.IsCircle` or a nullary
case property found no F# declaration: the compiled members of a union case
are not among the entity's members, so both the exact and the shape lookup
came back empty and Roslyn decompiled instead. A module literal has the same
fate: C# sees a const field, and the F# side only searched the entity's
fields. Both now map back to their declaration.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
xperiandri added a commit to xperiandri/fsharp that referenced this pull request Sep 11, 2026
@xperiandri
xperiandri force-pushed the perf/cross-language-gtd branch from 7f76c36 to c69cc5f 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

Copy link
Copy Markdown
Contributor

🔍 Tooling Safety Check — Affects-Design-Time
Affects-Design-Time: Changes cross-language Visual Studio navigation.

Generated by PR Tooling Safety Check · gpt56 3.1M ·

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-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

3 participants