Skip to content

Conversation

@d4rky-pl
Copy link
Contributor

@d4rky-pl d4rky-pl commented Dec 15, 2025

This PR fixes a false positive in RSpec/ScatteredSetup by ignoring the hooks defined inside class methods.


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).

@d4rky-pl d4rky-pl requested a review from a team as a code owner December 15, 2025 20:34
@bquorning
Copy link
Collaborator

I didn't want to chase this and figure out if this is Ruby version or some specific dependency getting silently updated due to unpinned version so I reverted those changes.

You should use Ruby 3.4 (as specified in .ruby-version), but an Ruby version agnostic alternative has been suggested in #2144. For now, reverting/ignoring the documentation change was the correct call. 👍🏼

@d4rky-pl
Copy link
Contributor Author

@bquorning ah, that explains it, I'm using asdf without legacy_version_file enabled. I can confirm that after switching to 3.4 the docs are no longer updated in the wrong places. I'll remove the note from the description, thanks :)

Copy link
Collaborator

@bquorning bquorning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks useful. But it seems it doesn’t consider the other ways of defining class methods:

class << self
  def foo
  end
end

@d4rky-pl d4rky-pl force-pushed the feat/scattered-setup-in-class-methods branch from ce8a208 to c587a4c Compare December 15, 2025 21:33
@d4rky-pl
Copy link
Contributor Author

@bquorning I feel that the class << self is a bit of an edge case, not sure if anyone would think of using it inside describe block. I've added the support anyway and applied the other suggestion.

Copy link
Collaborator

@bquorning bquorning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ydah What is your opinion?

@d4rky-pl d4rky-pl force-pushed the feat/scattered-setup-in-class-methods branch from c587a4c to 9e60861 Compare December 15, 2025 22:02
@d4rky-pl
Copy link
Contributor Author

@bquorning I had to push another fix because I've realized too late that it won't work with multiline class << self due to parser's implicit begin. It then took me forever to hit 100% coverage and 100% rubocop and have all the tests 😅

The PR should be OK now, apologies for pushing too soon.

Copy link
Member

@ydah ydah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Copy link
Collaborator

@bquorning bquorning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @d4rky-pl 🙏🏼

@bquorning bquorning merged commit a1f68a0 into rubocop:master Dec 15, 2025
27 checks passed
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