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

reportUnusedFunctionParameter? #2046

Closed
bgrounds opened this issue Jul 2, 2021 · 5 comments
Closed

reportUnusedFunctionParameter? #2046

bgrounds opened this issue Jul 2, 2021 · 5 comments
Labels
enhancement request New feature or request

Comments

@bgrounds
Copy link

bgrounds commented Jul 2, 2021

Is your feature request related to a problem? Please describe.
I rely heavily on type checking for refactoring.
I recently extracted some hard-coded values into function parameters and plumbed the function parameter through the call chain.
But then I forgot to actually replace the hard-coded value with the function parameter.
This resulted in a run-time error (since the hard-coded value was no longer valid).
The type checker didn't help here, which was surprising to me.
This is just one example.
I know there are use-cases for not using function parameters, but when I write functions whos signatures I can control (which is nearly all of them), not using a function parameter is a bug.

Describe the solution you'd like
I'd like to have the option to enable reportUnusedFunctionParameter.

Additional context
I notice comments in #1895 and #1118 like this:

Function parameters (even if they are unused) never trigger the reportUnusedVariable diagnostic rule. They will be displayed as "grayed out" if they are unused and your editor supports that feature, but no error or warning is emitted in this case.

Originally posted by @erictraut in #1118 (comment)

Since it's possible to provide hints, seems like it should also be possible to optionally emit warnings or errors, right?

@erictraut
Copy link
Collaborator

I am highly skeptical that such an option would be useful in anything but the most simple and contrived pieces of Python code. There are just way too many cases where parameters must be present but are legitimately unused. This is true for subclass method overrides, parameters for test frameworks, and callback functions.

@hmc-cs-mdrissi
Copy link

I think the rule that would be enough to cover most cases is overrides don't count and arguments starting with an _ can be unused. Most callbacks I encounter don't care about the names of the function arguments.

I'm still -1 as I think this is outside the scope of a type checker and in scope for a linter. pylint supports unused argument warning and type checker does not need to cover all potential style issues a linter should cover.

@bgrounds
Copy link
Author

bgrounds commented Jul 2, 2021

I am highly skeptical that such an option would be useful in anything but the most simple and contrived pieces of Python code. There are just way too many cases where parameters must be present but are legitimately unused. This is true for subclass method overrides, parameters for test frameworks, and callback functions.

As a functional programmer,

  • my programs are composed of many small functions that I define
  • I avoid classes and methods (I use plenty of dataclasses, but they basically serve as tuples with named fields)
  • If I ever have to use classes, I avoid inheritance whenever possible
  • I try to separate the use of third party dependencies from my application code
    • So if I'm forced to ignore a parameter in a 3rd party API, I'd be happy to add a comment suppressing the error for those few specific cases

For code that I write, this feature would be useful.
I don't think my code is contrived, but I do aim for simplicity :)

I realize that functional programmers are not typical, but we're also not unique.
There's also a good deal of practical advice in popular programming books and talks that encourage this style of programming.
So maybe some others would appreciate this feature, too.

Again, I'm not asking that this rule be enforced by default - just that I have the option to enable it.

I think this is outside the scope of a type checker

That's a fair point.
But how is reportUnusedFunctionParameter different than reportUnusedVariable in this regard?
And how much effort would it take to implement and maintain this feature?
Considering that informational diagnostics powering the 'graying out of unused parameters' feature in editors are already emitted, my guess is that it wouldn't be too difficult to implement. But that's just a guess.

pylint supports unused argument warning

Thanks for the tip. I'll check it out :)

type checker does not need to cover all potential style issues a linter should cover

agreed :)
I don't think a type checker should check style concerns, only correctness concerns.
When I write programs, unused function parameters are almost always bugs.

@erictraut
Copy link
Collaborator

Thanks for the additional details.

We generally add new diagnostic checks only when we've received signal from enough users that it's going to be generally useful. I'm going to close this one for now, but if it receives enough upvotes or additional comments, we'll consider reopening it in the future.

@erictraut erictraut added enhancement request New feature or request and removed needs decision labels Jul 2, 2021
@feluxe
Copy link

feluxe commented Nov 18, 2023

This is a problem when you rely on locals() for wrapping interfaces. E.g.:

    def wrapper_function(
        lots: str,
        of: str,
        args: str,
        ...
    ) -> SomeType:
        """
        A function wrapping another function
        """
        stored_kwargs = locals()
        
        # Do stuff
        new_arg = 1
        new_kwarg = 2
       
        return wrapped_function(new_arg, **{**stored_kwargs, "foo": new_kwarg})

We use this pattern a lot and end up with literally hundreds of pyright hints in different repos.

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

No branches or pull requests

4 participants