Add new RSpec/RepeatedSubjectCall cop

[drcapulet]

This adds a cop for catching uses of subject whereby you forget that it is memoized, often when trying to test idempotence:

it do
  expect { subject }.to change(Webhook.count).by(1)

  expect { subject }.to not_change(Webhook.count)
end

Had a hard time coming up with a succinct, descriptive name so open to suggestions.

---

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

If you have created a new cop:

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

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

• Set VersionChanged: "<<next>>" in config/default.yml.

[pirj]

Thank you so much! This cop, indeed, is very helpful, especially with the ‘not to change’ case!
Let’s shape it up a bit.

[pirj]

I think we need to detect named subjects and memoized helpers in general, too.

[drcapulet]

Agreed - wasn't sure on the best way to achieve that especially given the complex searching we already do below with just subject.

[pirj]

This should work.

[pirj]

Looks good already.
Leaving it up to you if you want to also cover named subjects, or leave it for a follow-up.
Thank you!

[pirj]

Please rebase

[pirj]

I’m sorry, I broke something. I’m from my mobile. Can you please check?

[pirj]

It's strange, I've added:

  RSpec.describe RuboCop::Cop::RSpec::AlignRightLetBrace do
    # rubocop:disable RSpec/ExampleLength
    it 'registers offense for unaligned braces' do
+     expect { subject }.to change(:a)
+     expect { subject }.to change(:a)

and the cop didn't register that. 🤔

[drcapulet]

@pirj Was a little trickier, but updated to support custom subjects!

[ydah]

Looks good! Thank you great work!

[timon]

This cop seems to bring in a lot of false positives, at least for me.

Here's the failing case:

describe LockManager do
  subject(:lock_manager) { described_class }

  describe '.lock!' do
    it 'requires a lock name and TTL in seconds' do # No offense raised
      expect { lock_manager.lock! }.to raise_error(ArgumentError, %r{wrong number of arguments.*required keyword:.*ttl})
    end

    it 'yields to a block if locking is successful' do # No offense raised
      expect { |block| lock_manager.lock!('some_lock', ttl: 1, &block) }.to yield_control
    end


    it 'raises an exception if locking is not successful', :aggregate_failures do
      lock_manager.lock!('some_lock', ttl: 3) do |_lock_held| # No offense raised
        expect do
          expect { |block| lock_manager.lock!('some_lock', ttl: 1, &block) }.not_to yield_control
          # C: RSpec/RepeatedSubjectCall: Calls to subject are memoized, this block is misleading
        end.to raise_error(Redlock::LockError, %r{failed to acquire.*some_lock})
      end
    end
  end
end

I find the actual error misleading:

• there is no call to subject here
• memoization is actually expected to happen

[pirj]

Doesn’t this PR fix this case for you, @timon ?

[timon]

No, dependabot PR with 2.27.0 just failed CI for me

No offence was registered for expect { |block| lock_manager.lock!(…) } line in previous version

[pirj]

You would have to use edge to get the fix from this PR, @timon

[timon]

Changelog says it was released today
https://github.com/rubocop/rubocop-rspec/blob/master/CHANGELOG.md#2270-2024-03-01

[bquorning]

I think @pirj meant to ask if PR #1822 (and not this PR) fixes the issue. It was merged just about half an hour ago.

[timon]

Yes, I added github: 'rubocop/rubocop-rspec' to my Gemfile and no offenses were registered