Skip to content

Offend allow_any_instance_of and expect_any_instance_of in RSpec/SubjectStub #2094

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

- Mark `RSpec/IncludeExamples` as `SafeAutoCorrect: false`. ([@yujideveloper])
- Fix a false positive for `RSpec/LeakyConstantDeclaration` when defining constants in explicit namespaces. ([@naveg])
- Offend `allow_any_instance_of` and `expect_any_instance_of` in `RSpec/SubjectStub`. ([@lovro-bikic])

## 3.6.0 (2025-04-18)

Expand Down
11 changes: 11 additions & 0 deletions docs/modules/ROOT/pages/cops_rspec.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -6150,6 +6150,17 @@ describe Article do
end
end

# bad
describe Article do
subject { Article }

it 'indicates that the author is unknown' do
allow_any_instance_of(subject).to receive(:author).and_return(nil)

expect(subject.new.description).to include('by an unknown author')
end
end

# good
describe Article do
subject(:article) { Article.new(author: nil) }
Expand Down
15 changes: 14 additions & 1 deletion lib/rubocop/cop/rspec/subject_stub.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,17 @@ module RSpec
# end
# end
#
# # bad
# describe Article do
# subject { Article }
Copy link
Member

Choose a reason for hiding this comment

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

I can’t recall if I’ve seen such usage ever before. Is this common?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've seen it a couple times, but I think this change makes even more sense in conjunction with this one #2088, which is part of the reason why I opened this PR.

allow_any_instance_of(described_class).to receive(:author) is common in my experience.

#
# it 'indicates that the author is unknown' do
# allow_any_instance_of(subject).to receive(:author).and_return(nil)
Copy link
Member

Choose a reason for hiding this comment

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

This is a consequence of usage of the above subject definition.

Do you feel that allow_any_instance_of(Article).to receive(:author).and_return(nil) is wrong, and why?

#
# expect(subject.new.description).to include('by an unknown author')
# end
# end
#
# # good
# describe Article do
# subject(:article) { Article.new(author: nil) }
Expand Down Expand Up @@ -93,11 +104,13 @@ class SubjectStub < Base
# expect(foo).to receive(:bar)
# expect(foo).to receive(:bar).with(1)
# expect(foo).to receive(:bar).with(1).and_return(2)
# expect_any_instance_of(foo).to receive(:bar)
# allow_any_instance_of(foo).to receive(:bar)
#
def_node_matcher :message_expectation?, <<~PATTERN
(send
{
(send nil? { :expect :allow } (send nil? %))
(send nil? { :expect :allow :expect_any_instance_of :allow_any_instance_of } (send nil? %))
(send nil? :is_expected)
}
#Runners.all
Expand Down
30 changes: 30 additions & 0 deletions spec/rubocop/cop/rspec/subject_stub_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,23 @@
RUBY
end

it 'flags when any instance of subject is stubbed' do
expect_offense(<<~RUBY)
describe Foo do
subject(:foo) { described_class }

before do
allow_any_instance_of(foo).to receive(:bar).and_return(baz)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not stub methods of the object under test.
end

it 'uses expect twice' do
expect(foo.bar).to eq(baz)
end
end
RUBY
end

it 'flags when subject is mocked' do
expect_offense(<<~RUBY)
describe Foo do
Expand Down Expand Up @@ -126,6 +143,19 @@
RUBY
end

it 'flags when any instance of subject is mocked' do
expect_offense(<<~RUBY)
describe Foo do
subject(:foo) { described_class }

it 'uses named subject' do
expect_any_instance_of(foo).to receive(:bar).and_return(baz)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not stub methods of the object under test.
end
end
RUBY
end

it 'flags one-line expectation syntax' do
expect_offense(<<~RUBY)
describe Foo do
Expand Down