reportUnusedVariable ignore pattern

[smackesey]

Is your feature request related to a problem? Please describe.

PyRight at present seems to have no way to fine tune the reportUnusedVariable setting to ignore variables matching certain patterns. When implementing a function/method with a third-party-specified interface, it often happens that there will be some function arguments that you write out but don't use. It should be possible to suppress diagnostics for these "expected" unused arguments without turning off unused variable warnings entirely. But at present, it appears that reportUnusedVariable can only be toggled entirely on or off.

Describe the solution you'd like

A common convention for this situation is to prefix the unused arguments with _. Other static analysis tools permit specification of a pattern that matches variable names for which to suppress unused warnings (see varsIgnorePattern for eslint's no-unused-vars) A similar option could be implemented in PyRight.

[dsuch]

Good day,

yes, having such an option would be a welcome addition to what otherwise is a truly good tool.

Whether to use an underscore vs. regexp - the latter would be unlikely to see wide usage, this is right.

However, instead of just having an underscore, and instead of a regexp, this could be a list of prefixes:

ignoredPrefixes = _, ignored_, unused_

The default would be:

ignoredPrefixes = _

My use case is that I sometimes use verbose names instead of an underscore for such a name to truly stand out, e.g. unused_user_id, rather than _user_id.

In that way it is much more visible on the one hand and on the other hand, it makes it much easier to find such variables in a large code base, e.g. "Search for everything starting with unused_".

In the default configuration above, the underscore would be simply a prefix and the configuration option would accept a list of prefixes, not just one.

Thank you.

[erictraut]

I've discussed this more with my colleagues. I did receive some pushback on the idea of exempting variables based on names (other than _, which is a well-established convention in Python). I have to agree that it seems odd that there are cases where you have a "dummy" local variable whose value is intentionally unused but you still feel strongly about giving it a name other than _, but it sounds like that's something many of you want to do.

Given the strong response on this issue, we're going to go ahead with option "1a" and exempt variables that start with an underscore. We might consider more flexible configuration options in the future based on feedback, but for now it sounds like this basic solution satisfies the needs of most pyright users.

This change will be included in the next release of pyright.

Thanks for all the feedback and discussion on this issue.

[erictraut]

This is included in pyright 1.1.190, which I just published. It will also be included in the next release of pylance.

[Wsine]

Is this new version working for you? I try the below minimal example, but it has no effect. my current pyright version is 1.1.254.

[erictraut]

@Wsine, what you are seeing here is that your client is requesting "unreferenced code" hints from the language server, but it is then interpreting and displaying these hints as diagnostics. This is a bug in some editors. If they request these hints, they should display them like VS Code does, in a subtle "grayed out" color. If the client isn't able to display these hints correctly, it should not request them from the language server.

[jason0x43]

I was also a bit surprised that function arguments can't be ignored with the underscore convention. Unused but necessary function arguments seem like one of the primary (if not the primary) use cases for this feature.

[MatthewScholefield]

Also, naming function definitions that are registered to something else via a decorator is a common use case where single-underscore identifiers seem especially strange:

@app.register_route('/foo')
def _handle_foo():  # Still reports "_handle_foo" is not accessed
    return {}

@app.register_route('/foo')
def _():  # Looks very strange
    return {}

Note that I guess ideally in the above you could just write handle_foo and it would suppress the ignored variable indicator because the function is passed to a decorator, but the underscore naming would at least be a suitable workaround.

[lewis6991]

@erictraut : I'm trying to fix the rendering in Neovim's LSP implementation to render these diagnostics correctly.

@Wsine, what you are seeing here is that your client is requesting "unreferenced code" hints from the language server, but it is then interpreting and displaying these hints as diagnostics. This is a bug in some editors. If they request these hints, they should display them like VS Code does, in a subtle "grayed out" color. If the client isn't able to display these hints correctly, it should not request them from the language server.

Is there somewhere this is documented. The spec itself just has:

     * Clients are allowed to render diagnostics with this tag faded out
     * instead of having an error squiggle.

However, diagnostics are more than just an error squiggle as they are used in statuslines, inline annotations and the sign column.

From your comments, I interpreted this as diagnostics with these tags should not be reported as diagnostics and should just be faded. Is this correct? And if so can this be further clarified in the LSP spec?

Otherwise, if these are still "real" diagnostics, then we need some way to action them.

[erictraut]

Is this correct?

Yes, that's correct.

And if so can this be further clarified in the LSP spec?

You'd need to talk to someone who controls the LSP spec about this. I recommend looking at VS Code as a reference implementation for a language server client and follow its lead.

[jason0x43]

@lewis6991 The current treatment of unused names as hint diagnostics seems reasonable enough (at least from my perspective). The main issue is that there is currently no way to selectively disable unused hints for specific function arguments (although there is for variables).

When writing code that implements some standard APIs (like, say, writing an extension for Home Assistant), you may end up with many functions that have required arguments that you don't actually need to use in your own code. Those arguments may be unused, but they have to be there, so any indication that they're unused is misleading, and makes unused hints generally less useful. That situation has come up enough for me that I ended up disabling all hint diagnostics coming from pyright.

However, disabling all unused hints obviously isn't ideal, because some of them may be meaningful. I generally do want to know if I've written a function and have unused arguments or an unused variable floating around somewhere. Some sort of visual indicator is very useful in that case. The problem is just that if I have code with 50 instances of necessary-but-unused function arguments and 1 instance that really is unused and could be cleaned up, noticing that 1 legitimate issue can be difficult.

[lewis6991]

@erictraut https://github.com/microsoft/language-server-protocol/issues/1696 indicates that diagnostics with these tags are real diagnostics, but it is up to the client to decide on the exact UI. I think this makes sense.

The issue I'm having is as follows:

I need to provide a function implementation with 4 arguments, but the implementation doesn't need to use any of them. This results in 4 diagnostic hints from Pyright. Renaming all 4 arguments to _ isn't valid, so AFAIK there is no way to resolve these diagnostics.

[erictraut]

This results in 4 diagnostic hints from Pyright

As I've said many times before, these are not meant to be displayed as regular diagnostics. They are intended to be displayed as a subtle "grayed out" text, as they are in VS Code. If your client requests these tagged hints from the language server but doesn't display them as intended, please report the problem to the maintainer of your editor or LSP plugin. In this case, the client should either 1. indicate that it does not support these tagged hints, or 2. display them properly.

[lewis6991]

According to the specification maintainers, this doesn't seem to be correct.

please report the problem to the maintainer of your editor or LSP plugin.

I am the maintainer. I'm trying to determine the correct behaviour here. The behaviour you describe is not consistent with the spec.

2. display them properly.

This should be determined by the spec which only contains:

Clients are allowed to render diagnostics with this tag faded out instead of having an error squiggle

It says nothing about these not being real diagnostics. The spec only says that clients are allowed to show these diagnostics as faded out, not that they must.

[erictraut]

Do you want pyright to report these tagged hints or not? If you do not want pyright to report them, then the client should not request them (i.e. it should report that it doesn't support that tag). If you do want pyright to report these tagged hints, then you should display them in a manner consistent with how they are intended to be used — and in a way that your users find useful. I don't know what else to say here. If the client requests these tagged hints and then displays them in a manner that is inconsistent with how they are intended to be used, your users are not going to be happy with the results.

[lewis6991]

Diagnostics are intended to be used as .... diagnostics, meaning they can be addressed/resolved by the user. Your interpretation here is much closer to semantic tokens in that they only affect UI. In fact, I know several servers that do exactly this.

I want pyright to report these tagged hints, but the hints must be resolvable, the ones pyright are emitting are not. The only solution pyright provides to resolve these currently is to rename such variables to _, however as I have already shown, this isn't always possible. The fact that these hints are not emitted for _ is arguably inconsistent, should _ not be faded?

[jason0x43]

you should display them in a manner consistent with how they are intended to be used — and in a way that your users find useful

The way unused variables are currently presented is useful. The problem isn't that the visual indicator isn't subtle enough, it's that there is any visual indicator in certain cases.

In @lewis6991's example, the four function arguments should not be highlighted as unused, subtly or otherwise. It doesn't convey any useful information to the developer. It can even be counterproductive because if a file has many "false" unused variable highlights, a developer could easy miss ones that truly indicate unused variables.

Tools such as eslint and tsserver provide a mechanism to deal with this case -- a developer can disable a rule in specific cases where the checker is providing results that aren't helpful. This feature was even added to pyright in v1.1.190, but only for local variables (which seems odd, because while there are cases where function arguments may be required but unused, I'm not sure when that would occur for local variables).

[smackesey]

LOL, this issue has such a long and confusing discourse around it because of confusion over what the word "diagnostic" means.

The "tagged hints" @erictraut is referring to are LSP diagnostics, full stop-- they just have severity HINT. I believe Pyright has its own concept of "diagnostic" that doesn't quite align with LSP diagnostics, though in the end Pyright must communicate both its "tagged hints' and own concept of diagnostics over the "diagnostics' channel of LSP. This discrepancy in the meaning of "diagnostic" is probably why @erictraut has earlier in this thread used a different definition of "diagnostic":

what you are seeing here is that your client is requesting "unreferenced code" hints from the language server, but it is then interpreting and displaying these hints as diagnostics. This is a bug in some editors

This is a really confusing statement, because the client is not requesting "unreferenced code hints"-- that's not an LSP concept-- it's an LSP client requesting LSP diagnostics. And displaying them as regular diagnostics is definitely not a bug. A generic LSP client will display any LSP diagnostics as... diagnostics. This is correct behavior.

However, even though these "tagged hints" are reported as LSP diagnostics, default editor diagnostic styling is unsuited to them, because, as so many have pointed out in this thread, it is extremely noisy. Typically the default styling is oriented towards flagging a "problem", but an unused variable is very often not a problem.

So @lewis6991, it basically falls to you as the editor client maintainer to use some special case logic here to instruct the editor to use appropriate styling-- if you try to match VSCode it actually works pretty well (i.e. no gutter flag, just subtle grayed out text). It communicates useful information but does not identify itself as a problem to be solved. I have an ad hoc config to do this in NeoVim and it works well-- this basically mimics VSCode:

Since IIUC you're trying to fix this in Neovim itself, my solution is not going to work for you because it globally overrides DiagnosticUnderlineHint but FWIW this is what I do:

require('lspconfig').pyright.setup({
  ...
  handlers = {
    -- no signs for unused variable hints
    ['textDocument/publishDiagnostics'] = vim.lsp.with(
      vim.lsp.diagnostic.on_publish_diagnostics, {
        virtual_text = false,
        signs = {
          severity = { min = vim.diagnostic.severity.INFO }
        },
      }
    )
  },
})

...

-- Colors *all* diagnostic hints with underline enabled as commented instead.
-- This is to match the VSCode style of unobtrusive "grayed out" unused
-- variables, instead of displaying hints like errors/warnings. This is not
-- ideal, but I'm not sure how to override this on a per-language-server basis.
-- Note that it must be *after* the colorscheme is loaded, because most
-- colorschemes will perform their own link of `DiagnosticUnderlineHint`.
vim.cmd('highlight! link DiagnosticUnderlineHint Comment')

[jason0x43]

LOL, this issue has such a long and confusing discourse around it because of confusion over what the word "diagnostic" means.

That's true, although I'm not sure it's really a problem. The root issue (well, my root issue) hasn't been with how the hints are displayed, it's that some of them are displayed at all.

an unused variable is very often not a problem

That's why developers should be able to selectively hide them! For a hint to be useful, it must convey some information. Otherwise, why show the hint at all? Hints (at least to my understanding) indicate to a developer that something might need their attention. That's very useful for unused variables...the first time you see a hint. However, once you've decided that a particular variable is necessary, the hint starts to be a problem. Now every time you see it you have to decide whether this variable is actually necessary.

[smackesey]

@jason0x43 I agree this should be configurable, I'm the OP of this thread and that's what I initially requested. FWIW I have since adapted to the status quo and now would probably not use such a setting if it were made available, as I find it useful to always see the special "grayed-out" rendering. For me, it stopped feeling like "noise" once I got the rendering correct-- it's more of a way to quickly visually parse what's going on in a function than to identify a potential issue.

One possible solution for you is to use Ruff. Ruff is amazing and complements pyright. It has a language server and supports unused argument identification, though at present it doesn't support an ignore pattern setting. But I'm confident you will have a much easier time getting Ruff's maintainer to add a setting for this than you will getting pyright to support it, Ruff's maintainer is much, much more open to feature requests, especially when they're standard features in similar lint tools.

[jason0x43]

it's more of a way to quickly visually parse what's going on in a function than to identify a potential issue.

That's my goal, too, but having things marked that shouldn't be marked really detracts from the utility of the hints. I'd like to be able to scan through code and have hints indicate things I might want to take a look at. If they're flagging a lot of things that I don't need to look at, it kind of slows down the scanning. I ended up disabling pyright's hints entirely because (in the code I was working with) they usually weren't showing me anything useful (but of course that means I also won't see when they would indicate something useful).

[erictraut]

I agree this should be configurable

Pyright already has a way to enable or disable diagnostics for unused variables (reportUnusedVariable) and imports (reportUnusedImport). If you're interested in receiving these diagnostics, you can enable the corresponding check. If you are not interested in these diagnostics, you can leave them disabled. These diagnostics are independent of the tagged hints.

The tagged hints are configurable through the LSP. A client can choose to specify that it does or doesn't support specific tags. If it indicates that it doesn't support the "unreferenced code" tag, then pyright will not send those tagged hints.

[smackesey]

Pyright already has a way to enable or disable diagnostics for unused variables (reportUnusedVariable) and imports (reportUnusedImport

What I'm referring to (and IIUC what @jason0x43 is seeking) is a way to selectively suppress unused argument diagnostics (does pyright have those independent of the hints?) by providing a e.g. a configuration setting that takes a regex pattern to match names for which the diagnostic will be suppressed. That is a common feature in other static analysis tools that do this in other languages, e.g. eslint.

[jason0x43]

Borrowing @lewis6991's example from earlier:

def _extend_with_null(validator_class):
  validate_type = validator_class. VALIDATORS [' type']

  def _null_type(validator, types, instance, schema):
    if instance is None:
      return
    for e in validate_type(validator, types, instance, schema):
      yield e

  def _null_enum(_validator, _enums, _instance, _schema) :
    # No need to validate enums
    pass

Assume the arguments to _null_enum are required. I should not see unused argument hints for those arguments because they are, in effect, necessary, even though this particular implementation doesn't use them. Having the hints there, even subtly, means that I can't trust those hints to actually be indicating unused arguments (because sometimes the arguments being hinted may be required).

However, just because some arguments are unused-but-necessary doesn't mean they all are. The hints are useful, just not in every case that they're currently being applied to. Ideally I wouldn't have to disable all unused variable/argument hints just because some of them (maybe a significant fraction of them) aren't valid.

by providing a e.g. a configuration setting that takes a regex pattern to match names for which the diagnostic will be suppressed

Really, I'm completely open on this. 😄 It could be a regex, a convention like a leading underscore (_unused_arg), or even a comment to disable the unused check for the next line (not ideal, but it's still better than turning off the check entirely).

  # pyright-disable-next-line reportUnusedArguments
  def _null_enum(_validator, _enums, _instance, _schema) :
    # No need to validate enums
    pass

[lewis6991]

Pyright already has a way to enable or disable diagnostics for unused variables (reportUnusedVariable) and imports (reportUnusedImport). If you're interested in receiving these diagnostics, you can enable the corresponding check. If you are not interested in these diagnostics, you can leave them disabled. These diagnostics are independent of the tagged hints.

I'm getting a very strong sense that the issue here isn't understood.

It's been established that these checks are wanted.

It's been established that for a client to support these tags as intended, they ~need to~ can be shown faded out. This is absolutely fine. There is no confusion about how these are intended to be handled from an LSP specification POV. The specification is quite clear and has further been confirmed. As far as a client is concerned, these are real diagnostics, despite whatever VSCode does.

The problem is that these diagnostics cannot be suppressed like local variables can. Disabling the check entirely is obviously a bad solution, I hope that much is clear.

The tagged hints are configurable through the LSP. A client can choose to specify that it does or doesn't support specific tags. If it indicates that it doesn't support the "unreferenced code" tag, then pyright will not send those tagged hints.

Our client supports these tags just fine, and they are displayed in a way that users are happy with the 200+ servers we have support for. This is very much a pyright specific issue. As far as I've seen, every other LSP that implements similar checks provides ways of resolving such diagnostics.

[erictraut]

As far as I've seen, every other LSP that implements similar checks provides ways of resolving such diagnostics.

That's not consistent with my experience. Pyright's behavior here is the same as the TypeScript language server in VS Code, for example. And this works fine because of how VS Code displays these hints. This is only a problem in editors that choose to display these tagged hints the same as other diagnostics rather than displaying them in a more subtle manner. I don't plan to modify pyright's behavior to work around what I see as a design flaw in these editors.

@lewis6991, do you see a downside to changing the way your editor displays "unreferenced code" tagged hints so it's consistent with VS Code and other editors?

[jason0x43]

Pyright's behavior here is the same as the TypeScript language server in VS Code, for example.

The TypeScript language server allows unused variable hints to be selectively silenced by prepending a variable or argument name with an underscore.

This is only a problem in editors that choose to display these tagged hints the same as other diagnostics rather than displaying them in a more subtle manner.

The problem described in this issue exists in VS Code, too. It's not a question of subtlety; the current way hints are displayed in neovim is fine enough. The issue is that sometimes hints are being applied in situations that they ideally shouldn't be.

[lewis6991]

That's not consistent with my experience. Pyright's behavior here is the same as the TypeScript language server in VS Code, for example.

Are you sure?

Note z2 is faded, and _z1 is not.

TypeScript implements exactly what is being asked for here. Additionally, typescript allows comment annotations to further control diagnostics.

And this works fine because of how VS Code displays these hints. This is only a problem in editors that choose to display these tagged hints the same as other diagnostics rather than displaying them in a more subtle manner. I don't plan to modify pyright's behavior to work around what I see as a design flaw in these editors.

It absolutely is not a design flaw. Our client is abiding by the LSP spec, not whatever VS Code happens to do. A diagnostic is a diagnostic. Just because VSCode chooses to hide diagnostics with the hint severity is a VS Code specific thing, not something every editor must also do.

@lewis6991, do you see a downside to changing the way your editor displays "unreferenced code" tagged hints so it's consistent with VS Code and other editors?

That effectively what I've done here. However, after writing the logic and also asking here, it seems to me this isn't the best way, and I'm confident now these are real diagnostics. If a server wants to be subtle, then I believe semantic tokens are a better fit. Otherwise, these diagnostics must be suppressible, just like Typescript and every other server I've used.

[erictraut]

We seem to be at a impasse here. I think we're going to need to agree to disagree.

[lewis6991]

So to confirm, you're not going to be consistent with typescript?

[erictraut]

You are arguing that all tagged hints should be suppressible, and I disagree with this. TypeScript uses the "unreferenced code" tags for multiple purposes. Not all of them can be suppressed. For example, blocks of code that are provably not executable given the current configuration are also tagged using an "unreferenced code" tagged hint and displayed as grayed out.

You mentioned that TypeScript allows unused parameters to be renamed with an underscore to indicate that they should not be flagged as unused. That's true. However, the TypeScript language doesn't support keyword arguments, so the names of parameters are not relevant to callers. In Python, the names of parameters cannot be modified at a whim because it changes the interface of the function or method, so telling developers to rename their parameters to work around a problem in their editor's UI is not a good solution.

[lewis6991]

You mentioned that TypeScript allows unused parameters to be renamed with an underscore to indicate that they should not be flagged as unused. That's true. However, the TypeScript language doesn't support keyword arguments, so the names of parameters are not relevant to callers. In Python, the names of parameters cannot be modified at a whim because it changes the interface of the function or method, so telling developers to rename their parameters to work around a problem in their editor's UI is not a good solution.

So then why do you allow _, and why is this also not grayed out?

[jason0x43]

In Python, the names of parameters cannot be modified at a whim because it changes the interface of the function or method, so telling developers to rename their parameters to work around a problem in their editor's UI is not a good solution.

That's true, but that's just one example implementation. Pyright could do something different, such as using comments, for example.

around a problem in their editor's UI

All editors (including VS Code) currently exhibit the same behavior.

[lewis6991]

One solution that appears to work is:

        def _null_enum(_validator, _enums, _instance, _schema):
            # shutup pyright!
            _ = _validator, _enums, _instance, _schema

[clason]

around a problem in their editor's UI is not a good solution.

I take umbrage at this. The editor implements the LSP specification as written (which is the whole point of a spec). It's pyright that makes incorrect assumptions about how a diagnostic is to be presented. If you believe that these hints should be presented in a specific way, you should either

1. lobby your colleagues that maintain the spec to change this paragraph from "editor's choice" to "mandatory" or
2. just use semantic tokens for this purpose like other servers.

[smackesey]

The editor implements the LSP specification as written (which is the whole point of a spec).

That's certainly true of NeoVim or any other editor that treats hint diagnostics like other diagnostics. And given that semantic tokens are now part of LSP, it seems pretty clear that this is the correct channel for pyright to communicate this sort of "unresolvable hint" info.

@erictraut is the reason that semantic tokens aren't used simply historical, i.e. they weren't part of LSP at the time pyright's implementation was written? Or is there a more principled reason? Because this does seem like an abuse of the LSP diagnostics channel given the existence of semantic tokens.

[jason0x43]

communicate this sort of "unresolvable hint" info

While I agree that semantic tokens would be a better fit for "unresolvable" hints, I think diagnostics are the right place for unused variable hints, because they should be actionable. Most language servers (that I use) flag unused variables with a hint or warning.

[clason]

Just to be clear, you can have your cake and eat it, too, here:

1. keep reporting actionable unused variable hints as (tagged) diagnostics; but
2. report unactionable unused variable hints as semantic tokens instead.

[erictraut]

I think pyright is using the "unreferenced code" tagged hint in the manner it was intended to be used. This is not an abuse of the diagnostics channel. Tagged hints were added for this very purpose (prior to the introduction of semantic tokens), and other language servers like TypeScript use tagged hints in the same manner and for the same purpose — even though it also supports semantic tokens.

Pyright's implementation of this functionality does indeed predate the introduction of semantic tokens. Pyright started out as both a type checker and a language server, but it is now focused entirely on type checking and type analysis. It serves as the foundation for pylance, which is Microsoft's Python language server for VS Code. The earlier language server functionality in still present in pyright, but there are no plans to add any new language server functionality. A semantic token provider is implemented in pylance, and it's unlikely that the pylance team would decide to port it to pyright. If you'd like to build your own language server on top of pyright (e.g. to convert "unreferenced code" tagged hints into semantic tokens), you're welcome to do so.

The LSP spec does not dictate UI design choices in an editor. However, it does define the semantic intent behind tagged hints. It also provides guidance (not mandatory, but suggested) for how editors should leverage these tagged hints in their UI. VS Code further provides a reference for how such a UI can work. You can certainly make whatever UI choices you want to within your editor implementation, but if those choices deviate from the semantic intent of the spec and from the expectations of your users, you might want to reconsider your choices.

... you can ... keep reporting actionable unused variable hints as (tagged) diagnostics

Actionable unused variable and import hints are already reported by pyright as true diagnostics (errors, warnings, and informations), not as tagged hints. They do not use a tag because these conditions should be rendered as true diagnostics, not as grayed-out text. The "unreferenced code" tagged hint is used only for unactionable things like blocks of unreachable code, unreferenced parameters, other unreferenced symbols when the user has requested not to received diagnostics for this condition, etc.

[lewis6991]

You keep mentioning "intent" like there is one, but AFAIK this is not covered in the spec. If the intent is not documented in the spec, then it does not exist. All that does exist is a suggestion on how to render diagnostics with these tags, we will follow and implement that.

And no, VS Code does not act as a proxy for the intent.

We will stick as closely to the spec as possible.

[clason]

if those choices deviate from the semantic intent of the spec and from the expectations of your users, you might want to reconsider your choices.

I'm sorry, but that is putting the cart before the horse. If the specification does not express the semantic intent clearly enough, it's clearly not doing its job properly. The whole point of a specification is not having to rely on an interpretation of "author intent".

In any case, I have reached the end of my limited patience with "language servers" that treat their own specification like a pirate's code that is subordinate to VS Code's behavior. What I am taking away from this discussion is that pyright is -- by your own admission -- in fact not (intended to be) a language server according to the specification and merely happens to use the protocol.

[erictraut]

You keep mentioning "intent" like there is one, but AFAIK this is not covered in the spec.

Here's what I mean when I say "intent". The spec defines diagnostic tags. It defines numeric values for two tags, and it gives symbolic names to each. The comments in the code (included in the spec and pasted below for reference) further explain the intended meaning of each tag. And it suggests a possible UI treatment that would be appropriate for rendering each tag. Perhaps this could be made clearer, but by my reading it's pretty clear that there was an intended meaning and use case behind each of these tags. Do you see it differently?

/**
 * The diagnostic tags.
 *
 * @since 3.15.0
 */
export namespace {
    /**
     * Unused or unnecessary code.
     *
     * Clients are allowed to render diagnostics with this tag faded out
     * instead of having an error squiggle.
     */
    export const Unnecessary: 1 = 1;
    /**
     * Deprecated or obsolete code.
     *
     * Clients are allowed to rendered diagnostics with this tag strike through.
     */
    export const Deprecated: 2 = 2;
}

We will stick as closely to the spec as possible.

It makes sense that you wouldn't want to violate the spec, but the spec provides latitude in UI design choices — and even suggests a potential UI treatment that would be appropriate in this case. Do you see it as a violation of the spec to adopt this suggestion? Or perhaps you're concerned that the suggested approach has some downsides relative to your current UI treatment for these tags?

Do you have any experimentation capabilities (e.g. for A/B testing)? I wonder if it would be worth trying this treatment as an experiment and getting feedback from users. That's a technique we frequently employ to inform our UX decisions. Just a thought.

[clason]

Do you see it as a violation of the spec to adopt this suggestion?

We see it as a violation of the spec on your part to require (or even assume) that this suggestion is adopted. I don't know how to make it clearer than that.

[clason]

And I appreciate the suggestions, but we are just not interested in tweaking the UX to account for a single language server's idiosyncrasies -- that would defeat the whole purpose of the language server protocol.

[lewis6991]

I think VS Code doesn't show hint diagnostics at all other than a squiggly line, so the subtlety of this check makes sense within that editor.

However other editors (including ours) show hints like all other severities. In general this has been a good default behaviour with servers since diagnostics are usually actionable.

So implementing the suggestion doesn't make sense to us since it becomes very special case because it becomes a function of severity+tag+'UI element' which overall makes configurability very convoluted. By treating each severity equal, we can make configuration flexible. If we just follow the LSP spec, then we only need to change the highlighting.

And as you said, pyright isn't really a language server anymore, so it also wouldn't make much sense to make significant changes to our UI just for a server like this one.

[erictraut]

I don't think pyright is as idiosyncratic as you are stating here. Surely there are other language servers other than pyright and TypeScript that use these tags to indicate blocks of code that cannot be reached, etc.

So implementing the suggestion doesn't make sense to us ...

Fair enough.

[jason0x43]

Pyright isn't alone in using hint tags to indicate unused code. The key difference between pyright and other language servers (and the reason this issue was originally opened) is that pyright doesn't provide the ability to silence specific instances of hints. With the TS language server, I can silence a specific instance of a hint if I know it's not actionable, either with a convention (underscore) or a comment. With pyright, my options are to either show all the hints (which may include a lot of non-actionable hints, obscuring the actionable ones), or turn all the hints off, which means I'm missing out on the benefits of hints.

On a related note to @lewis6991's comment yesterday about assigning unused arguments to _, I saw this recommendation from the PyLint manual, which also works:

        def _null_enum(validator, enums, instance, schema):
            del validator, enums, instance, schema

[joaotavora]

FWIW I also can't understand why prepending my variable's name with _ won't make pyright shut up about it. Sigh. It works if I name the variable _, losing semantic information about what the variable was suppose to mean if it were ever used.

[carschandler]

I'd love to see _unused implemented as a way of ignoring unused variables as well! It's definitely the most common convention, e.g. Rust has this built-in.

[noamraph]

I think that warning on unused variables is very valuable, and I find this pattern to be very useful:

a, b, _c, _d = get_something()

It lets me declare that it's OK that I'm not using the third and fourth return value of a function. I find it much clearer than a, c, _, _ = get_something(), since then I can know what other return values are available, without having to look up the reference.

Why not just make pyright treat _a like it treats _?

[Cutipus]

I also have been wrapping my head around this. signal(SIGINT, lambda signo, frame: self.quit()) complains that signo and frame are unused variables - which is true, but I just have no use for these required variables. I thought adding # pyright: ignore would solve it but apparently that just doesn't work. I discovered that _ removes the issue, but that only works for one variable.

Seeing the discussion here it seems like my code will never have that beautiful green ✅ in my editor.