Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Navigate to external metadata doesn't work #16761

Closed
kerams opened this issue Feb 25, 2024 · 8 comments · Fixed by #17047
Closed

Navigate to external metadata doesn't work #16761

kerams opened this issue Feb 25, 2024 · 8 comments · Fixed by #17047
Assignees
@vzarytovskii
Labels
Area-LangService-Navigation go to definition, F1 help + the FCS APIs related to it Bug Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. Regression
Milestone
April-2024

Comments

@kerams
Copy link
Contributor

kerams commented Feb 25, 2024

F12 on anything from FSharp.Core or any other .NET library.

VS 17.10 Preview 1
@kerams kerams added Bug Regression Area-LangService-Navigation go to definition, F1 help + the FCS APIs related to it Needs-Triage labels Feb 25, 2024
@github-actions github-actions bot added this to the Backlog milestone Feb 25, 2024
@vzarytovskii
Copy link
Member

F12 on anything from FSharp.Core or any other .NET library.

VS 17.10 Preview 1

Ugh, again...is it only decompiled metadata?

@kerams
Copy link
Contributor Author

kerams commented Feb 25, 2024

Seems so

@vzarytovskii vzarytovskii added Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. and removed Needs-Triage labels Mar 4, 2024
@vzarytovskii vzarytovskii self-assigned this Mar 4, 2024
@vzarytovskii vzarytovskii modified the milestones: Backlog, March-2024 Mar 4, 2024
@vzarytovskii
Copy link
Member

It seems to be the issue because roslyn started calling our new IFSharpFindDefinitionService (related issue: #14675) instead of old service. It will be a bit harder to fix since before we were creating an on-demand metadata document, and now we need to provide a document and span.

@vzarytovskii
Copy link
Member

@CyrusNajmabadi @tmat do you folks happen to know how does roslyn navigate to the external source? Might be good idea for us to properly support it. We'll likely want to do it in following order: show source link if available, then show F# decompilation (or rather high level representation).

@vzarytovskii vzarytovskii modified the milestones: March-2024, April-2024 Apr 10, 2024
@CyrusNajmabadi
Copy link
Member

Adding @davidwengier our expert here

@davidwengier
Copy link

Roslyn does the same basic process (source link, then decompilation, then metadata) but I don't think the decompilation system is extensible for other languages. The Source Link stuff might mostly work, but there are probably some gates on it only being done for C#/VB at the moment. For example, we create a project and rehydrate compilation options from the PDB as part of the process, so that stuff probably just fails.

@vzarytovskii
Copy link
Member

Roslyn does the same basic process (source link, then decompilation, then metadata) but I don't think the decompilation system is extensible for other languages. The Source Link stuff might mostly work, but there are probably some gates on it only being done for C#/VB at the moment. For example, we create a project and rehydrate compilation options from the PDB as part of the process, so that stuff probably just fails.

Ah, I see. Could you please point me to Roslyn's implementation? I see if I can make a workaround meanwhile.

vzarytovskii added a commit to vzarytovskii/fsharp that referenced this issue Apr 15, 2024
@vzarytovskii
Fix dotnet#16761
9264c4a
@vzarytovskii vzarytovskii mentioned this issue Apr 15, 2024
Merged
@davidwengier
Copy link

The main piece of logic for navigating to sourcelink sources starts here: https://github.com/dotnet/roslyn/blob/ca66296efa86bd8078508fe7b38b91b415364f78/src/Features/Core/Portable/PdbSourceDocument/PdbSourceDocumentMetadataAsSourceFileProvider.cs#L66

That call comes in via the IMetadataAsSourceFileService.GetGeneratedFileAsync, which is implemented here, and a File References on that should hopefully reveal where specific features call into it.

KevinRansom pushed a commit that referenced this issue Apr 17, 2024
@vzarytovskii @github-actions
Fix #16761 (#17047)
587bc1e 
* Fix #16761

* Fully async version + ignore cancellation on external navigation

* Automated command ran: fantomas

  Co-authored-by: vzarytovskii <1260985+vzarytovskii@users.noreply.github.com>

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
psfinaki added a commit that referenced this issue Apr 18, 2024
@dotnet-bot @edgarfgp
@T-Gro @brianrourkeboll @psfinaki @vzarytovskii @github-actions @KevinRansom
Merge main to release/dev17.11 (#17064)
27efc28 
* Disallow calling abstract methods directly on interfaces (#17021)

* Disallow calling abstract methods directly on interfaces

* More tests

* IWSAMs are not supported by NET472

* Update src/Compiler/Checking/ConstraintSolver.fs

Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com>

* fix typos

* looking for the right check

* Add comments

* move release notes

* Add a new error number and message

* Update docs/release-notes/.FSharp.Compiler.Service/8.0.400.md

Co-authored-by: Brian Rourke Boll <brianrourkeboll@users.noreply.github.com>

* Update docs/release-notes/.FSharp.Compiler.Service/8.0.400.md

Co-authored-by: Brian Rourke Boll <brianrourkeboll@users.noreply.github.com>

* Improve error message

---------

Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com>
Co-authored-by: Brian Rourke Boll <brianrourkeboll@users.noreply.github.com>

* Always use `typeEquivAux EraseMeasures` (for  integral range optimizations) (#17048)

* Always use `typeEquivAux EraseMeasures`

* Update release notes

* Update baselines

---------

Co-authored-by: Petr <psfinaki@users.noreply.github.com>

* Error message that explicitly disallowed static abstract members in classes. (#17055)

* WIP

* Error message that explicitly disallowed static abstract methods in abstract classes

* release notes

* SynTypeDefnKind.Class

* Fix #16761 (#17047)

* Fix #16761

* Fully async version + ignore cancellation on external navigation

* Automated command ran: fantomas

  Co-authored-by: vzarytovskii <1260985+vzarytovskii@users.noreply.github.com>

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

---------

Co-authored-by: Edgar Gonzalez <edgar.gonzalez@fundourselves.com>
Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com>
Co-authored-by: Brian Rourke Boll <brianrourkeboll@users.noreply.github.com>
Co-authored-by: Petr <psfinaki@users.noreply.github.com>
Co-authored-by: Vlad Zarytovskii <vzaritovsky@hotmail.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Kevin Ransom (msft) <codecutter@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vzarytovskii vzarytovskii

Labels
Area-LangService-Navigation go to definition, F1 help + the FCS APIs related to it Bug Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. Regression
Projects
F# Compiler and Tooling
Archived in project
Milestone
April-2024
Development

Successfully merging a pull request may close this issue.

Fix #16761 vzarytovskii/fsharp
4 participants
@davidwengier @vzarytovskii @CyrusNajmabadi @kerams