Skip to content

Commit 7e76bd7

Browse files
committed
RSpec/SubjectStub. Code review. Handle case with several explicit subjects in example group
1 parent 817a77e commit 7e76bd7

File tree

2 files changed

+34
-23
lines changed

2 files changed

+34
-23
lines changed

lib/rubocop/cop/rspec/subject_stub.rb

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -76,20 +76,12 @@ class SubjectStub < Cop
7676
PATTERN
7777

7878
def on_block(node)
79-
# process only describe/context/... example groups
8079
return unless example_group?(node)
81-
82-
# skip already processed example group
83-
# it's processed if is nested in one of the processed example groups
8480
return unless (processed_example_groups & node.ancestors).empty?
8581

86-
# add example group to already processed
8782
processed_example_groups << node
83+
@explicit_subjects = find_all_explicit_subjects(node)
8884

89-
# find all custom subjects e.g. subject(:foo) { ... }
90-
@named_subjects = find_all_named_subjects(node)
91-
92-
# look for method expectation matcher
9385
find_subject_expectations(node) do |stub|
9486
add_offense(stub)
9587
end
@@ -98,32 +90,31 @@ def on_block(node)
9890
private
9991

10092
def processed_example_groups
101-
@processed_example_groups ||= Set[]
93+
@processed_example_groups ||= Set.new
10294
end
10395

104-
def find_all_named_subjects(node)
96+
def find_all_explicit_subjects(node)
10597
node.each_descendant(:block).each_with_object({}) do |child, h|
10698
name = subject(child)
107-
h[child.parent.parent] = name if name
99+
if name
100+
h[child.parent.parent] ||= []
101+
h[child.parent.parent] << name
102+
end
108103
end
109104
end
110105

111-
def find_subject_expectations(node, subject_name = nil, &block)
112-
# if it's a new example group - check whether new named subject is
113-
# defined there
114-
if example_group?(node)
115-
subject_name = @named_subjects[node] || subject_name
106+
def find_subject_expectations(node, subject_names = [], &block)
107+
if example_group?(node) && @explicit_subjects[node]
108+
subject_names = @explicit_subjects[node]
116109
end
117110

118-
# check default :subject and then named one (if it's present)
119-
expectation_detected = message_expectation?(node, :subject) || \
120-
(subject_name && message_expectation?(node, subject_name))
121-
111+
expectation_detected = (subject_names + [:subject]).any? do |name|
112+
message_expectation?(node, name)
113+
end
122114
return yield(node) if expectation_detected
123115

124-
# Recurse through node's children looking for a message expectation.
125116
node.each_child_node do |child|
126-
find_subject_expectations(child, subject_name, &block)
117+
find_subject_expectations(child, subject_names, &block)
127118
end
128119
end
129120
end

spec/rubocop/cop/rspec/subject_stub_spec.rb

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,26 @@
2020
RUBY
2121
end
2222

23+
it 'flags when subject is stubbed and there are several named subjects ' \
24+
'in the same example group', :wip do
25+
expect_offense(<<-RUBY)
26+
describe Foo do
27+
subject(:foo) { described_class.new }
28+
subject(:bar) { described_class.new }
29+
subject(:baz) { described_class.new }
30+
31+
before do
32+
allow(bar).to receive(:bar).and_return(baz)
33+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not stub methods of the object under test.
34+
end
35+
36+
it 'uses expect twice' do
37+
expect(foo.bar).to eq(baz)
38+
end
39+
end
40+
RUBY
41+
end
42+
2343
it 'flags when subject is mocked' do
2444
expect_offense(<<-RUBY)
2545
describe Foo do

0 commit comments

Comments
 (0)