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

Send unnecssary/deprecated diagnostics when clients don't support tags #7380

Closed
wants to merge 1 commit into from
Closed

Send unnecssary/deprecated diagnostics when clients don't support tags #7380

wants to merge 1 commit into from

Conversation

the-mikedavis
Copy link
Contributor

Previously if an LSP client didn't declare tagSupport in the client capabilities for publishDiagnostic, pyright didn't send diagnostics for unnecessary or deprecated diagnostics. I think it's still valuable to send these regardless of support for tags. When a client doesn't support tags, we can just skip attaching them.

Previously if an LSP client didn't declare tagSupport in the client
capabilities for publishDiagnostic, pyright didn't send diagnostics for
unnecessary or deprecated diagnostics. I think it's still valuable to
send these regardless of support for tags. When a client doesn't support
tags, we can just skip attaching them.
@the-mikedavis
Copy link
Contributor Author

@microsoft-github-policy-service agree

@erictraut
Copy link
Collaborator

I don't agree with this change. These tagged hints are not meant to be presented as regular diagnostics, and they shouldn't be treated as such. There is no way to suppress them (e.g. through # type: ignore). They are meant to be subtle visual hints to the user. If the client doesn't support these tags, then they should not be generated.

@erictraut erictraut closed this Feb 29, 2024
@the-mikedavis
Copy link
Contributor Author

Can I suggest using an LSP extension for sending these hints, or the semantic tokens part of the spec instead? I don't use Python or Pyright with any regularity so I'm a little out of my depth here, but if these are not regular diagnostics then I believe it's improper to send them through textDocument/publishDiagnostic. Using tags this way is against the spirit of the spec in my interpretation:

export interface Diagnostic {
  // ..

	/**
	 * Additional metadata about the diagnostic.
	 *
	 * @since 3.15.0
	 */
	tags?: DiagnosticTag[];

  // ..
}

I believe tags are meant to be additional context about a diagnostic - the diagnostic is still a diagnostic.

Other clients like Neovim already present them as regular diagnostics with extra styling, and I think there is no way to supress the diagnostics there (though, again, I'm out of my water here - I'm not familiar with # type: ignoreing). Neovim's presentation, from a downstream bug report (helix-editor/helix#9765):

neovim-hint-deprecated

It's not a high priority suggestion at all as supporting tags in clients is very straightward. But I believe semantic tokens (specifically the SemanticTokenModifiers.deprecated member) is a better fit for these hints.

@the-mikedavis the-mikedavis deleted the md-report-diags-without-tags branch March 1, 2024 00:05
@kas2020-commits
Copy link

[tagged hints]... are meant to be subtle visual hints to the user.

@erictraut if that's the case then what is a user supposed to do if they're trying to enforce the deprecateTypingAliases code standard on a code base? The only recourse available is to set typeCheckingMode to strict which seems unnecessarily excessive for a simple deprecation warning.

@erictraut
Copy link
Collaborator

@kas2020-commits, you can enable the reportDeprecated diagnostic check independently of the type checking mode if you want pyright to return real diagnostics (not tagged hints) for the use of deprecated symbols.

@the-mikedavis, I'm not interested in debating this. Please refer to this thread where I made my perspective clear in this matter.

As for adding semantic token support, we have no plans to add any new LSP functionality to pyright at this time. Its LSP feature set is effectively frozen where it is. Pyright is focused on type checking. It has some basic LSP functionality, and that won't go away, but you shouldn't expect any new LSP functionality to be added in pyright. There is a full team at Microsoft building language server functionality on top of pyright in the Pylance language server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants