Option to disable the "self" is not accessed in methods

[felipetrz]

Sometimes it's desirable to write methods that don't use any instance members, such as a stateless implementation of a required interface.

There should be an option to disable the unused argument warning for the self parameter, either globally in the config file, or inline with an annotation.

[erictraut]

Pyright never emits an error or warning for unused "self". It does emit a "hint" diagnostic that clients can either ignore or use to display the unreferenced variable in gray if they choose. VS Code handles this fine, but some clients (editors) may not properly handle this hint and display it as a warning instead. Which client are you using?

[felipetrz]

I'm using the coc-pyright extension for coc.nvim. I'm able to filter the warnings by level globally, but I'd rather be able to disable them only when required, instead of hiding every hint and possibly missing the useful ones.

[erictraut]

OK, that confirms this is a bug in the coc.nvim extension. It is not properly conforming to the language client spec. It should be ignoring these hints or using them to display the corresponding text in gray.

[jakebailey]

Arguably, we should be checking the client capabilities before presenting them (as diagnostic tags are normally behind the flag), but that wouldn't change the fact that the diagnostic would appear unless the entire diagnostic were to hinge on the client having that capability (which seems wrong and is likely to make things worse as the diagnostic could never be used even when you do actually want to know).

[erictraut]

Is there a client capability that covers diagnostic tags specifically? If so, we could filter those if the client claims it doesn't support them.

[jakebailey]

Yes (see here, in tagSupport), but either you don't choose to provide the diagnostic tag and the diagnostic still shows up in the client as it does now (as right now, they'd just ignore the property during deserialization), or you make the entire "unused parameter" or "dead code" diagnostic depend on the tag support, which I think is a net negative as either you have to disable it always, or there's a setting whose default depends on the capability.

RE: self/cls/other required parameters, see: microsoft/pylance-release#194; personally I think we shouldn't be offering these diagnostics on parameters that are always required to be provided.

[erictraut]

I think it's fine to not provide any of the unreferenced/unused code hints if the client doesn't support it.

These are not meant to be "diagnostics". They're meant to be subtle hints to the user that something isn't referenced. It just so happens that the LSP spec requires that we report them as diagnostics. Special-casing "self" and "cls" would be inconsistent, so I'm not in favor of that. There's no harm in displaying these as gray when they're unreferenced.

[jakebailey]

Doing so would probably affect @edricgarran, as they said:

instead of hiding every hint and possibly missing the useful ones.

Which implies that they are interested in the diagnostics in other contexts, though I'm a bit skeptical as to how "nice" it is to have visible squiggles on parameters/imports/code blocks compared to the graying out that's a bit less visually annoying.

[felipetrz]

Checking for client support would likely do nothing in this case, since the diagnostic level is correctly recognized and handled as a hint.
The issue clearly lies in using diagnostics as a workaround to solve a syntax highlighting problem (as you keep mentioning "graying out", which is vscode specific behavior).
If this is the only reason the "hint" level exists, I'll simply ignore it.

[erictraut]

The client capability mechanism apparently allows the client to specify which hint "tags" it supports. In addition to having a "severity" (Error, Warning, Information, Hint), each diagnostic can also specify a series of tags. The spec currently defines two of them:

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

Pyright is using the "Unnecessary" tag, and VS Code uses that to determine which ranges to display in gray.

If the client capabilities indicate that the client doesn't support this tag, then Pyright could simply filter out those Hint diagnostics before returning them to the client.

[jakebailey]

In VSC, IIRC hints are shown as the little "..." in the corner of a word, for example, versus the others which are full squiggles. It's just that the tags add an extra dimension, and "hint + unnecessary" grays out the text and "hint + deprecated" strikes it out.

The visual look of the different levels also depends per client; I wouldn't say that ignoring every "hint" is always the right thing to do either as I'd hope there's some other "minimally annoying" indicator like the mini dots, but I'm thinking most things aren't so simple in the terminal. Graying out / striking through text is something I think can be done at the terminal, but adding little dots maybe not. (But, I've seen squiggles before, so maybe a one character gray one is possible?)

[erictraut]

This is now addressed in Pyright 1.1.66, which I just published. It will no longer report hint diagnostics with the "Unnecessary" tag if the client does not indicate that it supports this tag.

[fannheyward]

Fixed in coc-pyright, with override coc's publishDiagnostics.tagSupport to disable the Unnecessary hint.