Add new `RSpec/SortMetadata` cop

[leoarnold]

trafficstars

This cop sorts the RSpec metadata alphabetically:

  # bad
  describe 'Something', :b, :a
  context 'Something', foo: 'bar', baz: true
  it 'works', :b, :a, foo: 'bar', baz: true

  # good
  describe 'Something', :a, :b
  context 'Something', baz: true, foo: 'bar'
  it 'works', :a, :b, baz: true, foo: 'bar'

---

Before submitting the PR make sure the following are checked:

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

If you have created a new cop:

• [x] Added the new cop to config/default.yml.
• [x] The cop is configured as Enabled: pending in config/default.yml.
• [x] The cop is configured as Enabled: true in .rubocop.yml.
• [x] The cop documents examples of good and bad code.
• [x] The tests assert both that bad code is reported and that good code is not reported.
• [x] Set VersionAdded in default/config.yml to the next minor version.

If you have modified an existing cop's configuration options:

• [ ] Set VersionChanged in config/default.yml to the next major version.

[pirj]

• RESTRICT_ON_SEND = %i[ Because of the configurable language, it's hard to really restrict the send.

As it might be possible to define different configuration in different directories by putting .rubocop.yml files there There’s this static method: https://github.com/rubocop/rubocop/blob/8cc65d1c693d72afdc4f6b19cf1685d1ea9f3002/lib/rubocop/cop/base.rb#L308 It may be redefined not to memorize, but I have serious doubts that it would be beneficial for performance.

[pirj]

Speaking of the name, I think we can refer to the upcoming Cop naming guidelines. I'd suggest RSpec/UnorderedMetadata.

[pirj]

Checked against real-world-rspec:

57497 files inspected, 230 offenses detected, 230 offenses autocorrectable

1 error occurred:
An error occurred while RSpec/SortMetadata cop was inspecting /Users/pirj/source/real-world-rspec/rspec-core/spec/rspec/core/metadata_spec.rb:51:14.

[bquorning]

Hi @leoarnold, I’m a bit eager to merge this PR and get a release ready before I’m going on vacation – so I added a commit with @Darhazer’s code suggestion, plus support for multiline offenses (even if only the 1st line gets marked). I hope that’s ok 😅

Eventually, we’ll probably squash the two commits into just one before merging.

[leoarnold]

@bquorning I recognize that it is unclear "whose code" it is if you open a PR on somebody else's repository. Nevertheless I am very unhappy with such conduct. I don't understand why there is any objective requirement to rush this PR. And it kinda feels like you are putting pressure on other people solely based on constraints of your private life plans.

[bquorning]

solely based on constraints of your private life plans.

Much open source work is to a certain degree constrained by the maintainers’ private life plans 😄

But @leoarnold, I am sorry for putting undue pressure on you. You are correct that there is no objective requirement to rush this PR. I guess I was getting carried away by this new cop being great and me feeling we were getting close to cutting a new release.

I am very unhappy with such conduct.

I am sorry to hear that, and please feel free to revert my changes.

[pirj]

@leoarnold Is there anything left you want to adjust in this PR before it gets merged?

[Darhazer]

@leoarnold I understand your frustration. Though personally I was more frustrated at the times when my PRs were ignored (one in PHPUnit will turn 7 this year, despite the efforts of the community to push it forward.

Realistically, if we'd merge your original PR, and then add the changes as a separate commit (as they are done) - the end result - code-wise - would be the same.

The difference is that you didn't got the chance to apply - or reply to - the suggestions yourself. I'm sorry about that. I hope you'd accept our - all of the rubocop-rspec maintainers - appology for this rushed decision.

I'm not merging this for the sole reason to give you the chance to respond (well, you could do that in a closed PR as well). Just responding with I'm happy for this PR to be merged would be enough. And I thank you for this great contribution :bow:

[Darhazer]

Also I just noticed there is no changelog entry for the new cop

[bquorning]

As mentioned earlier, I really wish to see this pull request merged. But we have not heard back from the original contributor in several weeks, and I think moving forward now can’t be considered unnecessary rushing.

I am not going to repeat the mistake of pushing (let alone force-pushing) to a contributor’s fork of the repository. Instead, I consider opening a new pull request with @leoarnold’s commit and three other commits with subsequent changes. I hope this is an acceptable solution 🙏🏼

[pirj]

Superseded by #1378

@leoarnold I encourage you to revive the original part of this PR related to changing the style focus: true <-> :focus as a separate cop.