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

Invalid hyperlink in JavaScript diagnostics #64145

Closed
MariaSolOs opened this issue Sep 20, 2022 · 4 comments · Fixed by #64242
Closed

Invalid hyperlink in JavaScript diagnostics #64145

MariaSolOs opened this issue Sep 20, 2022 · 4 comments · Fixed by #64242
Assignees
Milestone

Comments

@MariaSolOs
Copy link
Contributor

Version Used: 4.4.0-2.22405.13

Steps to Reproduce:

  1. Create a new JavaScript file in Visual Studio.
  2. Write the following code inside: var # = 0;
  3. Open the error list and click on the TS1127 hyperlink.

Expected Behavior: The browser opens this 404 page.

Actual Behavior: Since the diagnostic doesn't include a valid documentation URL, the error code shouldn't be a hyperlink.

This happens with all JavaScript and TypeScript errors, as well as with some C# errors (like CS8199).

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Analyzers untriaged Issues and PRs which have not yet been triaged by a lead labels Sep 20, 2022
@MariaSolOs
Copy link
Contributor Author

FYI @mavasani

@mavasani mavasani self-assigned this Sep 20, 2022
@mavasani mavasani added Bug and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Sep 20, 2022
@mavasani mavasani added this to the 17.4 milestone Sep 20, 2022
@Youssef1313
Copy link
Member

as well as with some C# errors (like CS8199).

For C#, these are not-yet-documented errors. Links should work normally after they're documented.

@mavasani
Copy link
Contributor

I investigated this a bit and see that the behavior is controlled by the platform side. It queries for a help link, and if available and non-null it navigates to it. Otherwise, it queries for help keyword for F1 help and if available and non-null it navigates to the URL for F1 help, which seems to be the case for TS diagnostics.

This is Roslyn's implementation for the queries for help link and help keyword:

case StandardTableKeyNames.HelpKeyword:
content = data.Id;
return content != null;
case StandardTableKeyNames.HelpLink:
content = data.GetValidHelpLinkUri()?.AbsoluteUri;
return content != null;

We return the diagnostic ID as the help keyword, so basically error list is invoking the global VS Help service with TS diagnostic ID and navigating to it. I checked the history and see that both Roslyn and Error list implementation have been in place for many years, so this is not a recent regression from either of these.

If you want to fix this behavior and prevent F1 help from kicking in from TS diagnostics, we can make a change in Roslyn's implementation. For example, you can add a special custom tag, say "DoesNotSupportF1Help" or some such well-known name, to your diagnostics. Roslyn's implementation I pointed to above will return the diagnostic ID for F1 help keyword only if the above custom tag is not present on the diagnostic. We likely want to expose this special custom tag string via TS ExternalAccess layer to avoid any breaks in case the Roslyn code is changed/refactored.

@MariaSolOs
Copy link
Contributor Author

That sounds like a good plan @mavasani!

mavasani added a commit to mavasani/roslyn that referenced this issue Sep 23, 2022
Fixes dotnet#64145

TS diagnostics do not help corresponding help links, so the default help navigation invokes F1 help for the diagnostic ID. F1 help returns 404 error code for all TS diagnostics, so they wish to avoid showing F1 help. This PR adds a special well-known custom tag to allow diagnostics to skip F1 help navigation.
mavasani added a commit to mavasani/roslyn that referenced this issue Sep 23, 2022
Fixes dotnet#64145

TS diagnostics do not help corresponding help links, so the default help navigation invokes F1 help for the diagnostic ID. F1 help returns 404 error code for all TS diagnostics, so they wish to avoid showing F1 help. This PR adds a special well-known custom tag to allow diagnostics to skip F1 help navigation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants