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

Handling 'unused parameter' hints #107

Closed
jason0x43 opened this issue Oct 24, 2020 · 12 comments
Closed

Handling 'unused parameter' hints #107

jason0x43 opened this issue Oct 24, 2020 · 12 comments

Comments

@jason0x43
Copy link

jason0x43 commented Oct 24, 2020

Pyright flags unused function parameters with a hint.

Screen Shot 2020-10-24 at 12 11 12 PM

That is often helpful, but when you're trying to satisfy some library's API, you may want to include placeholders for parameters even if a function or method doesn't actually use them. An unused argument warning can be silenced by naming a parameter _, but that only works for one parameter. Names can be dundered (like __name__), but that feels like a hack, and doesn't work for keyword parameters. You could also use *arg and **kwargs, but is a bit less clear that having placeholders.

According to this, the Pyright language server tags unused parameter hints in some way, and the LSP client can tell Pyright that it doesn't support this tag to prevent Pyright from sending these hints. Possibly coc-pyright could provide an option to disable unused argument hints.

@fannheyward
Copy link
Owner

Fixed in neoclide/coc.nvim#2538, upgrade to latest coc.nvim and set diagnostic.showUnused to false.

@lucatpa
Copy link

lucatpa commented Jul 27, 2021

Is there any way to special-case this to only show the parameter grayed out without showing any error information? It seems like a good idea to have a visual hint that a variable is unused but having an error produces a lot of noise. This is what Visual Code seems to do according to this pyright issue where this problem was brought up by several people: microsoft/pyright#1118

@lucatpa
Copy link

lucatpa commented Jul 27, 2021

Also, this new diagnostic.showUnused also disable real unused variables, which is a different beast, as there is hardly a good reason for wanting to keep an unused variable, while, as pointed out here, function parameters are sometimes outside our control, so there are very good reasons to having some of those you can't avoid. So I very much want to get hints (as warnings/errors) about unused variables.

@fannheyward
Copy link
Owner

@lucatpa Upgrade your coc.nvim, the latest coc can gray out the unused var without an error/warning.

@lucatpa
Copy link

lucatpa commented Jul 28, 2021

Thanks @fannheyward!

I just upgraded (using the release branch) and I'm still getting the hint window

image

Is this available only on master or am I misunderstanding what I should expect?

@fannheyward
Copy link
Owner

@lucatpa the unused var will be grayed out, but if you put your cursor on it, the hint message will still showing. It's the expected behavior.

@lucatpa
Copy link

lucatpa commented Jul 28, 2021

I'm caught in the middle of different opinions here, because pyright developers say this should only be a syntax highlight hint and no other visual indication should be shown. For parameters of functions that are fulfilling some API this leaves users in a bad position where they have to chose to have visual noise for irrelevant (and unavoidable) noise (having a popup and a mark in the line) or completely disabling all unused variables hinting.

So is there no way to reconcile both projects views on this one? Should I just give up? And I'm not being cynical here, I'm truly hugely thankful to both projects because they make my life so much easier, this is only a tiny nuance. I just want to avoid wasting your and my time if there will be no agreement between both projects.

@fannheyward
Copy link
Owner

For example:

#!/usr/bin/env python3

def foo(name):
    """docstring for foo"""
    pass

截屏2021-07-29 上午10 42 02

The name parameter is not used. coc will make it gray out, but it's still a diagnostic because Pyright published this:

{
    "range": {
        "start": {
            "line": 2,
            "character": 8
        },
        "end": {
            "line": 2,
            "character": 12
        }
    },
    "message": "\"name\" is not accessed",
    "severity": 4,
    "source": "Pyright",
    "tags": [
        1
    ]
}

The diagnostic level is Hint. coc will sign it as >> on signcolumn.

截屏2021-07-29 上午10 49 26

In VSCode, name also be grayed out, if you put your cursor on it, you can get the diagnostic message.

I've know your problem, IMO the 'unused parameter' hint is a diagnostic, both coc/VSCode will gray out it, but in coc you will still get the sign noise. You can set "diagnostic.hintSign": "" to disable the sign, but it's still a diagnostic in :CocDiagnostics.

I'm not being cynical here

It's OK here.

@lucatpa
Copy link

lucatpa commented Jul 29, 2021

OK, I see. Thanks for getting getting involved in the pyright issues. I think the real solution would be to provide a way to ignore unused parameters for certain functions, as stated in pyright issues, because the problem here (or to me) is getting the editor even notifying me, in any way, noisy or not, about something that is expected and I can't possibly fix. Even a gray-out would still be noise, more manageable but noise.

I will try the "diagnostic.hintSign": "", this might help to reduce the noise. Thanks!

@lithammer
Copy link
Contributor

lithammer commented Sep 9, 2022

Personally, I also had trouble with virtual text. So my solution was this:

{"diagnostic.virtualTextLevel": "information"}

However I'm unsure what other consequences there are for this. I might be missing something useful.

@duongdominhchau
Copy link

This may be a non-solution to others, but personally I "fixed" this by disabling the reporting of unused var from Pyright and use Ruff instead as Ruff can be configured to only report unused vars not starting with _. This way I don't lose the ability to recognize unused variables, but can still suppress it when needed.

@eullerborges
Copy link

eullerborges commented Jul 21, 2023

Note that you also need "diagnostic.level": "information" on pyrightconfig.json for the solution proposed by @duongdominhchau to work. Also, you'll want to add a dummy variable regex (dummy-variable-rgx = "^(_+|(_+[a-zA-Z0-9_]*[a-zA-Z0-9]+?))$") to the ruff configuration (~/.ruff.toml).

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

No branches or pull requests

6 participants