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

Add new Lint/NumberedParameterAssignment cop #9326

Merged

Conversation

koic
Copy link
Member

@koic koic commented Jan 3, 2021

This cop checks for uses of numbered parameter assignment.
It emulates the following warning in Ruby 2.7:

% ruby -ve '_1 = :value'
ruby 2.7.2p137 (2020-10-01 revision 5445e04352)
[x86_64-darwin19]
-e:1: warning: `_1' is reserved for numbered parameter; consider another name

Assiging to numbered parameter (from _1 to _9) cause an error in Ruby 3.0.

% ruby -ve '_1 = :value'
ruby 3.0.0p0 (2020-12-25 revision 95aff21468)
[x86_64-darwin19]
-e:1: _1 is reserved for numbered parameter

NOTE: The parametered parameters are from _1 to _9. This cop checks _10 and
above as well to prevent confusion.

# bad
_1 = :value

# good
non_numbered_parameter = :value

This cop checks numbered (block) parameter other than _1 to _9 (e.g.: _0, _10),
it is placed in the Naming department instead of Lint department.


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.

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 4, 2021

I like the cop, but I find the name a bit confusing, as the assignment is just the only place where we can check about the naming reliably, but orthogonal to the naming itself. I'm wondering if this shouldn't be incorporated in Naming/VariableName instead. If we keep it as a separate cop it's probably more of a Lint cop, than I naming cop, though. Not sure what the others think.

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 7, 2021

@rubocop-hq/rubocop-core If anyone has suggestions how to name this better or whether to integrate it into Naming/VariableName, please share them with us. :-)

Rakefile Outdated Show resolved Hide resolved
@jonas054
Copy link
Collaborator

jonas054 commented Jan 7, 2021

@koic You wrote that you put the new cop in Naming instead of Lint, so it sounds like you first considered Lint but then changed your mind. What was your reason for that? To me it seems more natural to put it in Lint and to keep it as a separate cop. For one thing, it reports on severity level Convention now, while it's syntax error in Ruby 3.0.

@koic koic force-pushed the add_new_naming_numbered_parameter_assignment_cop branch from 52d8b74 to f8468fe Compare January 8, 2021 11:59
@koic koic changed the title Add new Naming/NumberedParameterAssignment cop Add new Lint/NumberedParameterAssignment cop Jan 8, 2021
@koic
Copy link
Member Author

koic commented Jan 8, 2021

Yeah, what I originally tried to focus on was that assigning to numbered parameters (_1 to _9) will cause syntax error in Ruby 3.0. OTOH, assigning to _0, _10 doesn't cause syntax error, so I put it in Naming department, but this is a side effect 💦 (It's not essentially the cop's aim).

So I reconsidered Lint as the comfortable department and updated this PR. Thank you for your consideration

This cop checks for uses of numbered parameter assignment.
It emulates the following warning in Ruby 2.7:

```console
% ruby -ve '_1 = :value'
ruby 2.7.2p137 (2020-10-01 revision 5445e04352)
[x86_64-darwin19]
-e:1: warning: `_1' is reserved for numbered parameter; consider another name
```

Assiging to numbered parameter (from `_1` to `_9`) cause an error in Ruby 3.0.

```console
% ruby -ve '_1 = :value'
ruby 3.0.0p0 (2020-12-25 revision 95aff21468)
[x86_64-darwin19]
-e:1: _1 is reserved for numbered parameter
```

NOTE: The parametered parameters are from `_1` to `_9`. This cop checks `_10` and
above as well to prevent confusion.

```ruby
# bad
_1 = :value

# good
non_numbered_parameter = :value
```

This cop checks numbered (block) parameter other than `_1` to `_9` (e.g.: `_0`, `_10`).
@koic koic force-pushed the add_new_naming_numbered_parameter_assignment_cop branch from f8468fe to d47750b Compare January 10, 2021 10:40
@bbatsov
Copy link
Collaborator

bbatsov commented Jan 10, 2021

Let's merge the PR after the next bugfix release. I plan to do it Monday morning.

@bbatsov bbatsov merged commit 02ac59e into rubocop:master Jan 11, 2021
@koic koic deleted the add_new_naming_numbered_parameter_assignment_cop branch January 11, 2021 15:35
jmkoni pushed a commit to standardrb/standard that referenced this pull request May 3, 2021
* Update rubocop from 1.12.1 to [1.13.0](https://github.com/rubocop-hq/rubocop/releases/tag/v1.13.0)
* Update rubocop-performance from 1.9.2 to [1.11.1](https://github.com/rubocop-hq/rubocop-performance/releases/tag/v1.11.1)
* Enabled the following rules:
  * [`Performance/RedundantSplitRegexpArgument`](rubocop/rubocop-performance#190)
  * [`Style/IfWithBooleanLiteralBranches`](rubocop/rubocop#9396)
  * [`Lint/TripleQuotes`](rubocop/rubocop#9402)
  * [`Lint/SymbolConversion`](rubocop/rubocop#9362)
  * [`Lint/OrAssignmentToConstant`](rubocop/rubocop#9363)
  * [`Lint/NumberedParameterAssignment`](rubocop/rubocop#9326)
  * [`Style/HashConversion`](rubocop/rubocop#9478)
  * [`Gemspec/DateAssignment`](rubocop/rubocop#9496)
  * [`Style/StringChars`](rubocop/rubocop#9615)
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

Successfully merging this pull request may close these issues.

3 participants