Add new Style/NumberedParameters cop

[Hugo-Hache]

Ruby 2.7 introduced numbered parameters. They come in handy for single line block where naming a variable was done only to please the parser but did not bring any value. However for longer blocks they can obfuscate the object manipulated.

This cop allows to restrict their use to single line block.

---

Before submitting the PR make sure the following are checked:

• The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
• Wrote good commit messages.
• Commit message starts with [Fix #issue-number] (if the related issue exists).
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
• Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

[Hugo-Hache]

👋 the Rubocop team, after discussing with my team the way we wanted to use the numbered parameters introduced by Ruby 2.7, I noticed there was no related cop.

Being new to this repo, I checked the docs you wrote for contributors, and tried to base the (really simple) code of this cop on existing ones. However I certainly made rookie mistakes, so please let me know what they are, and I'll do my best to fix them.

[dvandersluis]

This cop is very straightforward, and looks like a good addition. I can see some additional styles being useful (for instance, disallowing numbered params altogether).

My only question is: does having this cop operate on the number of lines the block is vs the number of statements the block contains valid? (FWIW, the cop could still be pretty simple checking for more than one statement, you'd just need to check if the child of the block node is a begin node).

This also could probably autocorrect pretty easily if desired, see Style/BlockDelimiters for a similar example.

[koic]

I think that the cop name Style/NumberedParameters makes users imagine denying numbered parameter.

The cop will probably have an option of always denying numbered parameter.

So, I'm not sure whether it's better to allow only single line by default. And it is doubtful that single lines will be auto-corrected to multi lines instead of being denied. It does not resolve nameless, so this cop may be Naming dept instead of Style dept (but I'm not sure which is the suitable dept yet).

IMO, use of numbered parameter is understandable to me when numbered parameter is used in disposable script, but I'm very worried about using numbered parameter in maintenance code. OTOH, I've met several developers who like numbered parameter.

So, this rule probably needs discussion in the Ruby Style Guide first.
https://github.com/rubocop/ruby-style-guide

[Hugo-Hache]

Thanks so much for your reactivity.

Answers for @dvandersluis:

• Additional style disallow: great idea, just added it
• Number of lines vs statements: I initially chose lines over statements (or complexity) as my concern was focused on readability. Single line put beside the array name and numbered parameter, so it's very easy to follow what the numbered parameter is refering to.
• Autocorrect: not sur how we could auto-correct numbered parameters, adding parameters with default name like a, b, etc? Not sure it would put the code in a much better state than the initial numbered parameters

@koic based on your advice I just opened an issue on the subject in the style guide

[dvandersluis]

IMO, use of numbered parameter is understandable to me when numbered parameter is used in disposable script, but I'm very worried about using numbered parameter in maintenance code. OTOH, I've met several developers who like numbered parameter.

I agree, I would definitely disable numbered parameters in my work codebase it it was possible.

[dvandersluis]

Autocorrect: not sur how we could auto-correct numbered parameters, adding parameters with default name like a, b, etc? Not sure it would put the code in a much better state than the initial numbered parameters

My suggestion for autocorrect was only for changing multiple lines to a single line if we're going that way. Definitely don't change variables, that wouldn't be safe, and wouldn't work well anyways.

[dvandersluis]

I left one minor nitpick comment, but this looks good to me. Once all of @koic's concerns are resolved then I think this is good to go.

[bbatsov]

I guess this needs to be updated to reflect the more general nature of the cop today.

[Hugo-Hache]

Updated, ok for you? (I updated default.yml as well)

[bbatsov]

My only question is: does having this cop operate on the number of lines the block is vs the number of statements the block contains valid? (FWIW, the cop could still be pretty simple checking for more than one statement, you'd just need to check if the child of the block node is a begin node).

I was also thinking about this, but we've already targeted single-line blocks in the past, so I guess the current approach is fine.

I think this cop should also have some limit on the number of numbered params in a block (regardless of the length of the block), as things can get very dicey with 2+ numbered params IMO. This, however, would imply an "allow" style as well, so perhaps a separate cop is in order down the road (e.g. NumberOfNumberedParams or something like this).

[dvandersluis]

This, however, would imply an "allow" style as well, so perhaps a separate cop is in order down the road (e.g. NumberOfNumberedParams or something like this).

I opened #10111 to add a cop for this 😁

[Hugo-Hache]

I'm on the road this week, I'll incorporate your comments on Monday morning. Thanks again for your quick feedbacks!

[bbatsov]

Thanks!

[dvandersluis]

I love how simple this cop is, great job @Hugo-Hache!