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

Style/ArgumentsForwarding` should allow or forbid specific argument names #12296

Closed
nevans opened this issue Oct 19, 2023 · 1 comment · Fixed by #12397
Closed

Style/ArgumentsForwarding` should allow or forbid specific argument names #12296

nevans opened this issue Oct 19, 2023 · 1 comment · Fixed by #12397

Comments

@nevans
Copy link
Contributor

nevans commented Oct 19, 2023

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

As it is today, Style/ArgumentsForwarding is great for cutting down on the redundant noise of argument passing. Using anonymous argument names clearly signals that the arguments are either ignored or forwarded. It was the first autocorrection I ran after upgrading to ruby 3.2.

However, argument names can be used to document more than just "this is forwarded". E.g: these names can document what they are represent, and where they are forwarded. This is especially important for gem authors and public APIs. As ruby language servers mature and achieve greater adoption, this documentation becomes more useful even on internal or private methods.

If the names are simple *rest, *args, **kwargs, **opts, and similar, then the names are (generally) not really serving any purpose but noise. But if the names are **scope_options or *authenticator_args and so on, now they might be worth preserving.

Describe the solution you'd like

I would like (at least) two new configurable attributes: AllowedNames and ForbiddenNames. Both options should be an array of either regular expressions or strings. The defaults could be the same as today: forbid all and allow none.

We might add separate configuration options for each of the rest, kwrest, and block parameters, but I personally would prefer to keep it simpler with only these two options.

Describe alternatives you've considered

  1. Just let rubocop autocorrect names that are unique and helpful... but only a little bit helpful.
  2. Add # rubocop:disable Style/ArgumentForwarding every time I feel the names are important to the documentation.

Additional context

When I upgraded to ruby 3.2, I used rubocop to autocorrect almost every single Style/ArgumentsForwarding offense in my codebase. For at least 9 out of ten changes, I agreed completely. For almost all of the rest, it wasn't really worth the effort to deal with them on a case-by-case basis... but if I'd had these configuration attributes, it would have been worth my time to configure.

Thanks!

@koic
Copy link
Member

koic commented Nov 17, 2023

I've opened #12397 to resolve this issue. It's a slightly different approach from the proposal, but it addresses the issue effectively.

koic added a commit to koic/rubocop that referenced this issue Nov 20, 2023
…mentsForwarding`

Fixes rubocop#12296.

This PR supports `RedundantRestArgumentNames`, `RedundantKeywordRestArgumentNames`,
and `RedundantBlockArgumentNames` options for `Style/ArgumentsForwarding`.

Meaningless names that are commonly used can be anonymized by default as part of the new `Redundant*Names` defaults.
e.g., `*args`, `**options`, `&block`, and some argument names.

Names not on this list are likely to be meaningful and are allowed by default.

This approach is designed to only detect generally meaningless names without the need for users to edit the configuration
for each meaningful name, with the expectation that the default settings will work well for most cases.
bbatsov pushed a commit that referenced this issue Nov 20, 2023
…rwarding`

Fixes #12296.

This PR supports `RedundantRestArgumentNames`, `RedundantKeywordRestArgumentNames`,
and `RedundantBlockArgumentNames` options for `Style/ArgumentsForwarding`.

Meaningless names that are commonly used can be anonymized by default as part of the new `Redundant*Names` defaults.
e.g., `*args`, `**options`, `&block`, and some argument names.

Names not on this list are likely to be meaningful and are allowed by default.

This approach is designed to only detect generally meaningless names without the need for users to edit the configuration
for each meaningful name, with the expectation that the default settings will work well for most cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants