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.

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

F1 on our source generator errors goes nowhere because we don't set a HelpUri #85181

Closed
danmoseley opened this issue Apr 21, 2023 · 16 comments · Fixed by #90353
Closed

F1 on our source generator errors goes nowhere because we don't set a HelpUri #85181

danmoseley opened this issue Apr 21, 2023 · 16 comments · Fixed by #90353
Assignees
Labels
area-Meta help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@danmoseley
Copy link
Member

I got this
image

clicked on the SYSLIB1052 code, which is hyperlinked, and got an Oops page:

https://learn.microsoft.com/en-us/visualstudio/ide/not-in-toc/default?f1url=%3FappId%3DDev16IDEF1%26l%3DEN-US%26k%3Dk(keywords_removed)%26rd%3Dtrue&view=vs-2022

Not sure if it's just this code.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Apr 21, 2023
@teo-tsirpanis
Copy link
Contributor

BTW clicking many newer compiler error codes also leads to a generic error page like this.

@danmoseley
Copy link
Member Author

@buyaa-n how do we handle ensuring that the SYSLIBxxx codes go someplace?

@danmoseley
Copy link
Member Author

should ours all pass HelpLinkUri or should there be useful default behavior somewhere?

@buyaa-n
Copy link
Contributor

buyaa-n commented Apr 27, 2023

how do we handle ensuring that the SYSLIBxxx codes go someplace?

I don't know how SYSLIBxxx links works, seems it should open https://learn.microsoft.com/en-us/dotnet/fundamentals/syslib-diagnostics/syslib1050-1069, tagging @gewarren who created that page

@gewarren
Copy link
Contributor

None of the SYSLIB warning links seem to work, even the ones that have a dedicated page (I tried SYSLIB0011 for BinaryFormatter.Serialize). I think this is how the code analysis rules hook up to learn.microsoft.com: https://github.com/dotnet/roslyn-analyzers/blob/2420507acd5863bfe30a7dd97993c9362527045b/src/Utilities/Compiler/DiagnosticDescriptorHelper.cs#L36. @mavasani might be able to suggest a solution for the SYSLIB warnings?

@mavasani
Copy link

mavasani commented Apr 28, 2023

should ours all pass HelpLinkUri or should there be useful default behavior somewhere?

You should write a DiagnosticDescriptor creation helper in your repo that builds up the help link based on the diagnostic ID and adds it to the descriptor. We do so in both Roslyn and Roslyn-analyzers repo.

@AaronRobinsonMSFT AaronRobinsonMSFT added area-Infrastructure and removed untriaged New issue has not been triaged by the area owner labels May 1, 2023
@ghost
Copy link

ghost commented May 1, 2023

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details

I got this
image

clicked on the SYSLIB1052 code, which is hyperlinked, and got an Oops page:

https://learn.microsoft.com/en-us/visualstudio/ide/not-in-toc/default?f1url=%3FappId%3DDev16IDEF1%26l%3DEN-US%26k%3Dk(keywords_removed)%26rd%3Dtrue&view=vs-2022

Not sure if it's just this code.

Author: danmoseley
Assignees: -
Labels:

area-Interop-coreclr, area-Infrastructure

Milestone: -

@AaronRobinsonMSFT AaronRobinsonMSFT added untriaged New issue has not been triaged by the area owner and removed area-Interop-coreclr labels May 1, 2023
@AaronRobinsonMSFT
Copy link
Member

You should write a DiagnosticDescriptor creation helper in your repo that builds up the help link based on the diagnostic ID and adds it to the descriptor.

Seems like this is a general analyzer issue for the repo and no specific to interop.

@danmoseley
Copy link
Member Author

@gewarren
Copy link
Contributor

The DiagnosticDescriptorHelper would need to have a mapping table in order to produce URL's like

We can do the mapping on the docs side through the redirection file, e.g.

{
    "source_path_from_root": "/docs/fundamentals/syslib-diagnostics/syslib1055.md",
    "redirect_url": "/dotnet/fundamentals/syslib-diagnostics/syslib1050-1069"
}

@danmoseley
Copy link
Member Author

The DiagnosticDescriptorHelper would need to have a mapping table in order to produce URL's like

We can do the mapping on the docs side through the redirection file, e.g.

Ah, I think that would be much better. So the helper should basically just set HelpUri to https://learn.microsoft.com/dotnet/fundamentals/syslib-diagnostics/<CODE>.md for each code.

@danmoseley danmoseley added the help wanted [up-for-grabs] Good issue for external contributors label May 10, 2023
@danmoseley danmoseley changed the title missing help for some SYSLIBxxx The specified 'MarshalAsAttribute' configuration for param ... F1 on our source generator errors goes nowhere because we don't set a HelpUri May 10, 2023
@danmoseley
Copy link
Member Author

danmoseley commented May 10, 2023

cc @captainsafia as it appears that the RDG is also missing HelpLinkUri eg

https://github.com/danmoseley/aspnetcore/blob/d6d85eda7ed5b7a9c72ef9f36166f8ec9f850fd2/src/Http/Http.Extensions/gen/DiagnosticDescriptors.cs#L10

and will need a help topic and perhaps a redirect like this dotnet/docs#35301

I can open an issue if there's not already a task tracking that.

for reference, aspnetcore's list is at https://github.com/dotnet/aspnetcore/blob/main/docs/list-of-diagnostics.md

@captainsafia
Copy link
Member

I can open an issue if there's not already a task tracking that.

Yep, let's file an issue for this.

We currently have the HelpLink uri configured the analyzers/codefixers in the targeting pack but not the generator.

@ericstj
Copy link
Member

ericstj commented May 12, 2023

The fix for this should add a DiagnosticDescriptorHelper class like the one shared by @gewarren (thanks!) with a create method taking relevant constructor parameters, it should create the helpLink as

string helpLink = $"https://learn.microsoft.com/dotnet/fundamentals/syslib-diagnostics/{id.ToLowerInvariant()}.md";

Then create the DiagnosticDescriptor. Looks like we have a few different sources in the repo -- some may not be syslib.
https://github.com/search?q=repo%3Adotnet%2Fruntime%20DiagnosticDescriptor%20&type=code

@layomia -- maybe you can try this out while you're working on the ConfigurationBinder generator to find a pattern that works well, then apply it across the other generators?

@ericstj ericstj removed the untriaged New issue has not been triaged by the area owner label May 12, 2023
@ericstj ericstj added this to the 8.0.0 milestone May 12, 2023
@ghost
Copy link

ghost commented May 12, 2023

Tagging subscribers to this area: @dotnet/area-meta
See info in area-owners.md if you want to be subscribed.

Issue Details

I got this
image

clicked on the SYSLIB1052 code, which is hyperlinked, and got an Oops page:

https://learn.microsoft.com/en-us/visualstudio/ide/not-in-toc/default?f1url=%3FappId%3DDev16IDEF1%26l%3DEN-US%26k%3Dk(keywords_removed)%26rd%3Dtrue&view=vs-2022

Not sure if it's just this code.

Author: danmoseley
Assignees: -
Labels:

area-Meta, help wanted

Milestone: 8.0.0

@danmoseley
Copy link
Member Author

Note that despite the title it's not just generators of course, anywhere a DiagnosticDescriptor is created, eg analyzer diagnostics.

@layomia layomia self-assigned this May 12, 2023
@ericstj ericstj assigned ericstj and unassigned layomia Aug 10, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 10, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 12, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Meta help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants