Go To Definition from C# and VB into F# without checking the whole project - #20465
xperiandri wants to merge 6 commits into
Conversation
✅ Release notes checked
|
github-actions
Bot
added
the
AI-Tooling-Check-Scanned-Clean
| return ValueNone | ||
| } | ||
|
|
||
| let! candidates = |
There was a problem hiding this comment.
🤖🕵️⏱️🔥
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
This thread was never actually answered — the reply under it is about error-page slugs and belongs to a different repository, so the request still stands.
What the change does structurally, which is what the numbers should show: before, a navigation from C# or Visual Basic into F# checked the whole target project to find one declaration; now the documentation comment id narrows the work to the documents whose parse can declare that entity, and only those are parsed, with the whole-project check left as the fallback when the narrowing finds nothing. So the shape of the measurement is documents parsed, whether the fallback ran at all, and time to the first source result before Roslyn gives up and shows metadata.
I will take it on the solution the report came from, cold, and post the four numbers here: documents parsed, projects checked, whether ParseAndCheckProject ran, and time to the source result. If you would rather see it as a test that asserts the narrowing instead of a timing, the fixtures for it already exist in CrossLanguageSymbolNavigationTests — say which you prefer.
7f76c36 to
c69cc5f
Compare
github-actions
Bot
added
the
⚠️ Affects-Design-Time
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
T-Gro
left a comment
T-Gro
left a comment
There was a problem hiding this comment.
🤖🕵️ 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
Bot
moved this from New
to In Progress
in F# Compiler and Tooling
c69cc5f to
3401152
Compare
T-Gro
added
the
vsintegration-only
a38af57 to
534267d
Compare
github-actions
Bot
added
the
⚠️ Affects-Build-Infra
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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>
One test checks that a composed multi-file project narrows to the single file declaring the entity, among others that do not; the other covers a multi-targeted project whose first instance does not declare the entity at all under conditional compilation, and only a later one does. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
534267d to
de4750f
Compare
|
🔍 Tooling Safety Check — Affects-Design-Time
|
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The navigation and cancellation changes span multiple production and test files, warranting final human review.
Review effort: Lite
Findings: None
What changed in this PR
Improves C#/VB → F# Go To Definition by locating declarations through parse-only scans and checking only the relevant file before falling back to whole-project checking.
Changes:
- Extracts cross-language navigation into a dedicated module.
- Adds cancellable sequential declaration scanning.
- Adds navigation tests and project-file updates.
- Documents the optimization in release notes.
| File | Summary |
|---|---|
vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj |
Includes the new navigation tests. |
vsintegration/tests/FSharp.Editor.Tests/CrossLanguageSymbolNavigationTests.fs |
Adds cross-language navigation coverage. |
vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs |
Removes relocated navigation logic. |
vsintegration/src/FSharp.Editor/Navigation/CrossLanguageSymbolNavigation.fs |
Implements optimized declaration lookup and fallback behavior. |
vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj |
Includes the new navigation module. |
vsintegration/src/FSharp.Editor/Common/CancellableTasks.fs |
Adds sequential cancellable tryPick. |
docs/release-notes/.VisualStudio/18.vNext.md |
Documents the performance improvement. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Go To Definition from a C# or Visual Basic file into an F# symbol opened Roslyn's decompiled view instead of the F# source while Visual Studio was cold, because the lookup ran a full project type-check for every target-framework instance of the F# project before falling back to decompilation on a timeout. Reproduced with the in-box tools on Visual Studio 18 Insiders, on a solution with 135 project instances.
The lookup now finds the declaring file first, from a parse-only scan of the project's already-cached syntax trees, and type-checks only that one file; the previous whole-project check remains as a fallback for the cases the parse-only scan cannot resolve.
F# → C# navigation, the within-F# Go To Definition path, and the ExternalAccess contract are unchanged. No timing numbers are claimed: the change replaces one full project check per target-framework instance with a parse-only scan plus a single file check, with the full check only as a fallback.
🤖 Generated with Claude Code