Enable pending cops up to RuboCop v1.63

[bquorning]

I enabled the pending cops up until RuboCop v1.63, and fixed most offenses. We want to keep the NewCops: disable settings, so PRs are not blocked by newly added cops forcing changes to the entire code base. But we should update once in a while, even if it means everyone needs to bundle update. Or we could add rubocop to Gemfile with a version restriction?

I am fixing Performance/MethodObjectAsBlock in two commits, which should be squashed before merging:

1. Fix Performance/MethodObjectAsBlock using numbered parameters:
   I really dislike the foo(&method(:bar)) style, and in my opinion Performance/MethodObjectAsBlock might as well have been a Style cop. I tried fixing the offenses using numbered parameters (_1) but I‘m not a fan of that syntax either, to be honest.
2. Fix Performance/MethodObjectAsBlock using block arguments:
   I think the plain old block arguments are much easier to read than numbered parameters – except maybe in a few cases. E.g. is { |n| let?(n) } really better to read than { let?(_1) }?

The Lint/ToEnumArguments offense listed in the todo file is due to a bug: rubocop/rubocop#12885

---

Before submitting the PR make sure the following are checked:

• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Updated documentation.
• Added an entry to the CHANGELOG.md if the new code introduces user-observable changes.
• The build (bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).

[ydah]

Looks good!

[bquorning]

cc @pirj I would like your opinion on this PR too.

[pirj]

I have the same preferences for the block syntax.

For the pin the rubocop version vs keep NewCops disabled, I’d rather keep it as is, and run our the code in specs with the latest released (or latest cached by bundler?) RuboCop version rather than inspected our code with it.
The necessity to make such a decision comes from the fact that RuboCop is both the base for extensions like ours, and a set of cops. I’m not a huge propeonent of this, and I hoped this will follow the trend if rubocop-ast, but it is what it is.

Thank you!

[bquorning]

I’d rather keep it as is, and run our the code in specs with the latest released (or latest cached by bundler?) RuboCop version rather than inspected our code with it.

I am not sure I understand this part of your response. Can you please elaborate a bit?

I think the three conflicting goals here are:

1. We want to use new cops pretty soon after they are released,
2. we don’t want to require an unnecessarily recent version of RuboCop in our gemspec, and
3. we don’t want pull requests to fail CI because of a new RuboCop version (so no using edge version on CI).

[pirj]

There is one thing we might be missing - it’s to test our code against our minimum specified rubocop version.
Imagine, we have 1.40 in our min, 1.41 introduced some nice helper, 1.42 added an internal investigation cop to enforce the use of this helper, we started using it (because our goal is to inspect our code with the latest rubocop), and our code breaks for our users who updated rubocop-rspec, but stick to rubocop 1.40.

[pirj]

Please correct me, but it feels that 1 is not easy together with 3.
If we have a few PRs open and enable pending cops, this has a good chance of either:

• break prs if they are rebases
• break master if prs are merged without being rebased (with offensive code that wasn’t offensive without new cops)

I don’t have a good solution to this, and think it’s inevitable. If we don’t enable new cops too often -it’s not a big burden.

[bquorning]

Yeah, you’re probably right. We should not enable new cops very often.

And we should start running CI with the minimum allowed RuboCop version.

[bquorning]

There is one thing we might be missing - it’s to test our code against our minimum specified rubocop version.

See #1882.

[bquorning]

Rebased. Are you ok with merging @pirj ?

[pirj]

Yes, please proceed with the merge. All perfect. Thank you!