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

Feature request: new rule (and auto fix?) keyword only parameters #8382

Open
tiangolo opened this issue Oct 31, 2023 · 3 comments
Open

Feature request: new rule (and auto fix?) keyword only parameters #8382

tiangolo opened this issue Oct 31, 2023 · 3 comments
Labels
rule Implementing or modifying a lint rule

Comments

@tiangolo
Copy link

This is something I like to do and request people do as well, but I haven't seen a linter that implements my (personal) rule.

I'm a bit ashamed of coming just to add a new issue to request a new non-existing rule/fix just because I would want it, but anyway, worth trying. 😅

Description

I try to avoid callables (functions, methods) with more than two positional parameters/arguments.

I told @Kludex about it a while ago, and he built https://github.com/Kludex/kwonly-transformer which is very close to what I want.

There is also https://github.com/vchaptsev/flake8-keyword-arguments which is a bit closer but only implements the rule/error, not the fix.

Specification (?)

Here's the description of what I would like.

It applies both to creating a function/method signature and to calling it.

  • If there are at most 2 parameters/arguments, it's fine to leave them positional.
  • If there are more than 2 parameters/arguments:
    • If at most 2 are required and the rest are optional, the two parameters can be positional but the rest should be keyword only.
    • If there are more than 2 parameters/arguments required, all of them have to be keyword only.

I tend to be a bit stricter than that, but I would think that's generalizable enough that it could be useful to others. For example, if there's one required parameter and the rest are optional, I would try to allow only the first one to be positional, all the others would be required to be keyword only. The rationale is that normally it's a function that applies to a specific object and has some parameters/modifications for how to apply it.

The reason for 2 parameters and allowing 2 in most places is mainly for the cases when an operation applies to two things, one is in some way the origin/source and the other is the destiny, so it can be considered starting from the first and going to the second (going left to right, the same direction as when writing or reading code).

Maybe that could be a configurable threshold, could be 2, or could be 0 for people who don't want any positional argument... don't know, for me, it would be fine if it was fixed at 2.

Other ideas

It could be stricter and with more rules, with other things like, if all parameters/arguments are optional, they should all be required, but that (and other potential extra rules) is probably more cumbersome and less flexible.

Auto Fix

Of course, it would be great if it could not just show the error but also auto fix it, but not sure how difficult that is, it would change the calls and hopefully also the function signature.

@zanieb
Copy link
Member

zanieb commented Oct 31, 2023

Thanks for the comprehensive suggestion! There's some discussion at #8137 already too.

I agree with the general sentiment that positional argument use should be limited.

We may have a hard time with the fix, as we don't perform fixes across multiple files yet.

@zanieb zanieb added the rule Implementing or modifying a lint rule label Oct 31, 2023
@tiangolo
Copy link
Author

tiangolo commented Nov 1, 2023

Ah! yes, it seems that's related to the same, sorry, I only checked on issues and not on discussions. 😅 Feel free to close this one!

About the fix, get it, maybe just fixing the calls (and not the signature) would already do most of the work. But in any case, even just the rule detecting it would help a lot.

@jack-mcivor
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

3 participants